From 53fcf3b458e328c51ecb974df4a6594489d3941b Mon Sep 17 00:00:00 2001 From: Murray Stevenson <50844517+murraystevenson@users.noreply.github.com> Date: Tue, 2 Apr 2024 15:48:52 -0700 Subject: [PATCH 1/2] ViewportGadget : Avoid rendering selection ids with `GL_BLEND` enabled Enabling GL_BLEND for selection renders on Intel graphics hardware leads to corrupted ids in the selection buffer, so we now ensure it is disabled for selection renders in `renderInternal`. We also consolidate the enabling of GL_BLEND so it only occurs in `renderInternal`, rather than in various parts of StandardStyle and in `ViewportGadget::render`. This fixes #901 and #2788 on the Intel graphics hardware we have available to test with. --- Changes.md | 1 + src/GafferUI/StandardStyle.cpp | 2 -- src/GafferUI/ViewportGadget.cpp | 8 +++++--- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/Changes.md b/Changes.md index ca07af8878f..1f85cfb85f2 100644 --- a/Changes.md +++ b/Changes.md @@ -21,6 +21,7 @@ Fixes - Dispatcher : Fixed shutdown crashes caused by Python slots connected to the dispatch signals [^1]. - Display : Fixed shutdown crashes caused by Python slots connected to `driverCreatedSignal()` and `imageReceivedSignal()` [^1]. - LightPositionTool : Fixed crash when changing the tool mode with nothing selected [^1]. +- ViewportGadget : Fixed selection issues with Intel GPUs (#901, #2788). API --- diff --git a/src/GafferUI/StandardStyle.cpp b/src/GafferUI/StandardStyle.cpp index a386de4ec8d..0fb19d19b1d 100644 --- a/src/GafferUI/StandardStyle.cpp +++ b/src/GafferUI/StandardStyle.cpp @@ -581,7 +581,6 @@ void StandardStyle::bind( const Style *currentStyle ) const return; } - glEnable( GL_BLEND ); glBlendFuncSeparate( GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA, GL_ONE, GL_ONE_MINUS_SRC_ALPHA ); glUseProgram( shader()->program() ); @@ -1130,7 +1129,6 @@ void StandardStyle::renderImage( const Imath::Box2f &box, const IECoreGL::Textur glPushAttrib( GL_COLOR_BUFFER_BIT ); // As the image is already pre-multiplied we need to change our blend mode. - glEnable( GL_BLEND ); if( !IECoreGL::Selector::currentSelector() ) { // Some users have reported crashes that were traced back to this call diff --git a/src/GafferUI/ViewportGadget.cpp b/src/GafferUI/ViewportGadget.cpp index 43471d67acb..3092c8a4fd5 100644 --- a/src/GafferUI/ViewportGadget.cpp +++ b/src/GafferUI/ViewportGadget.cpp @@ -1193,7 +1193,6 @@ void ViewportGadget::render() const glClearColor( 0.26f, 0.26f, 0.26f, 0.0f ); glClearDepth( 1.0f ); glClear( GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT ); - glEnable( GL_BLEND ); // Set up the camera to world matrix in gl_TextureMatrix[0] so that we can // reference world space positions in shaders @@ -1288,13 +1287,16 @@ void ViewportGadget::renderInternal( RenderReason reason, Gadget::Layer filterLa if( reason != RenderReason::Draw ) { // We're doing selection so post-processing doesn't matter. Just - // render direct to output buffer. + // render direct to output buffer without blending as that can + // corrupt the selection buffer on some graphics hardware. + glDisable( GL_BLEND ); renderLayerInternal( reason, layer, viewTransform, bound, selector ); continue; } - // Render to intemediate framebuffer. + // Render to intermediate framebuffer. + glEnable( GL_BLEND ); glBindFramebuffer( GL_DRAW_FRAMEBUFFER, acquireFramebuffer() ); glClearColor( 0.0f, 0.0f, 0.0f, 0.0f ); glClear( GL_COLOR_BUFFER_BIT ); From 43a95959764d804bbf2a2feb9eaf06a5a02e62f0 Mon Sep 17 00:00:00 2001 From: Murray Stevenson <50844517+murraystevenson@users.noreply.github.com> Date: Tue, 2 Apr 2024 16:05:35 -0700 Subject: [PATCH 2/2] StandardStyle : Remove Selector test when changing blend mode This should no longer be necessary now that we ensure `GL_BLEND` is disabled for selection renders. --- src/GafferUI/StandardStyle.cpp | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/GafferUI/StandardStyle.cpp b/src/GafferUI/StandardStyle.cpp index 0fb19d19b1d..fadf798dce4 100644 --- a/src/GafferUI/StandardStyle.cpp +++ b/src/GafferUI/StandardStyle.cpp @@ -1129,16 +1129,7 @@ void StandardStyle::renderImage( const Imath::Box2f &box, const IECoreGL::Textur glPushAttrib( GL_COLOR_BUFFER_BIT ); // As the image is already pre-multiplied we need to change our blend mode. - if( !IECoreGL::Selector::currentSelector() ) - { - // Some users have reported crashes that were traced back to this call - // when used on the GL_R32UI data type the `IDRender` selection mode - // uses. The `IDRender` buffer would get corrupted with values that - // didn't correspond to actual gadgets. - // Don't change it when rendering the selection pass since - // blending should not be applied to an integer buffer anyways. - glBlendFunc( GL_ONE, GL_ONE_MINUS_SRC_ALPHA ); - } + glBlendFunc( GL_ONE, GL_ONE_MINUS_SRC_ALPHA ); glEnable( GL_TEXTURE_2D ); glActiveTexture( GL_TEXTURE0 );