Reviewed-by: Marek Olšák <marek.ol...@amd.com> So that's why Gallium wasn't affected. Both (loc = 1, index = 0) and (loc = 0, index = 1) map to COLOR[1] in Gallium.
Marek On Thu, Jan 21, 2016 at 5:35 AM, Kenneth Graunke <kenn...@whitecape.org> wrote: > OpenGL's dual color blending feature was specified so that an > implementation could support both multiple render targets (MRT) and > dual source blending. Fragment shader outputs specify both "location" > (the render target number) and "index" (either color 0 or 1). > > I believe DirectX only has the notion of "location" - if using dual > color blending, location 0 or 1 will specify the operands. If not, > then location means the render target index. The two features can't > be used together. > > As such, some applications mistakenly try to use <loc = 0, index = 0> > and <loc = 1, index = 0> in a shader used for dual color blending with > a single render target, rather than the correct <loc = 0, index = 0> > and <loc = 0, index = 1>. > > In particular, Unigine Heaven 4.0 and Valley 1.0 suffer from this bug. > Unigine is aware of the problem, and quickly developed a fix, but has > not bothered to change the download link on their website to a working > copy in over a year. People were still using the broken version and > complaining. We tried working around this by disabling dual color > blending, but that apparently hurts performance, and people were once > again unhappy. > > On i965, dual source blending is achieved by using different framebuffer > write messages than normal rendering. So, we have to compile different > code for the two cases. We're not being pedantic: we actually have to > know in order to function. > > Normally, dual source blending is detectable in the shader: if a shader > has an output with index = 1, then it's meant for blending, not MRT. > With the broken inputs, they're indistinguishable, so we can only tell > by looking at the current GL state. > > This patch implements a new drirc workaround: > > export dual_color_blend_by_location=true > > which makes the i965 driver detect when OpenGL state is configured for > dual source blending, and recompile the fragment shader to use the right > messages. In that case, we allow either location = 1 or index = 1 to > specify the second source for the blending equations. > > It also re-enables GL_ARB_blend_func_extended for Unigine. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92233 > Signed-off-by: Kenneth Graunke <kenn...@whitecape.org> > Cc: Marek Olšák <marek.ol...@amd.com> > Cc: Ilia Mirkin <imir...@alum.mit.edu> > --- > src/mesa/drivers/dri/common/drirc | 16 ++++++++-------- > src/mesa/drivers/dri/common/xmlpool/t_options.h | 5 +++++ > src/mesa/drivers/dri/i965/brw_compiler.h | 1 + > src/mesa/drivers/dri/i965/brw_context.c | 3 +++ > src/mesa/drivers/dri/i965/brw_context.h | 1 + > src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 6 +++++- > src/mesa/drivers/dri/i965/brw_wm.c | 4 ++++ > src/mesa/drivers/dri/i965/intel_screen.c | 1 + > 8 files changed, 28 insertions(+), 9 deletions(-) > > diff --git a/src/mesa/drivers/dri/common/drirc > b/src/mesa/drivers/dri/common/drirc > index e1874c3..183a1dc 100644 > --- a/src/mesa/drivers/dri/common/drirc > +++ b/src/mesa/drivers/dri/common/drirc > @@ -37,26 +37,26 @@ TODO: document the other workarounds. > > <application name="Unigine Heaven (32-bit)" executable="heaven_x86"> > <option name="allow_glsl_extension_directive_midshader" > value="true" /> > - <!-- remove disable_blend_func_extended if 4.1 ever comes out --> > - <option name="disable_blend_func_extended" value="true" /> > + <!-- remove dual_color_blend_by_location if 4.1 ever comes out > --> > + <option name="dual_color_blend_by_location" value="true" /> > </application> > > <application name="Unigine Heaven (64-bit)" executable="heaven_x64"> > <option name="allow_glsl_extension_directive_midshader" > value="true" /> > - <!-- remove disable_blend_func_extended if 4.1 ever comes out --> > - <option name="disable_blend_func_extended" value="true" /> > + <!-- remove dual_color_blend_by_location if 4.1 ever comes out > --> > + <option name="dual_color_blend_by_location" value="true" /> > </application> > > <application name="Unigine Valley (32-bit)" executable="valley_x86"> > <option name="allow_glsl_extension_directive_midshader" > value="true" /> > - <!-- remove disable_blend_func_extended if 1.1 ever comes out --> > - <option name="disable_blend_func_extended" value="true" /> > + <!-- remove dual_color_blend_by_location if 1.1 ever comes out > --> > + <option name="dual_color_blend_by_location" value="true" /> > </application> > > <application name="Unigine Valley (64-bit)" executable="valley_x64"> > <option name="allow_glsl_extension_directive_midshader" > value="true" /> > - <!-- remove disable_blend_func_extended if 1.1 ever comes out --> > - <option name="disable_blend_func_extended" value="true" /> > + <!-- remove dual_color_blend_by_location if 1.1 ever comes out > --> > + <option name="dual_color_blend_by_location" value="true" /> > </application> > > <application name="Unigine OilRush (32-bit)" > executable="OilRush_x86"> > diff --git a/src/mesa/drivers/dri/common/xmlpool/t_options.h > b/src/mesa/drivers/dri/common/xmlpool/t_options.h > index 4e5a721..55e926b 100644 > --- a/src/mesa/drivers/dri/common/xmlpool/t_options.h > +++ b/src/mesa/drivers/dri/common/xmlpool/t_options.h > @@ -90,6 +90,11 @@ DRI_CONF_OPT_BEGIN_B(disable_blend_func_extended, def) \ > DRI_CONF_DESC(en,gettext("Disable dual source blending")) \ > DRI_CONF_OPT_END > > +#define DRI_CONF_DUAL_COLOR_BLEND_BY_LOCATION(def) \ > +DRI_CONF_OPT_BEGIN_B(dual_color_blend_by_location, def) \ > + DRI_CONF_DESC(en,gettext("Identify dual color blending sources by > location rather than index")) \ > +DRI_CONF_OPT_END > + > #define DRI_CONF_DISABLE_GLSL_LINE_CONTINUATIONS(def) \ > DRI_CONF_OPT_BEGIN_B(disable_glsl_line_continuations, def) \ > DRI_CONF_DESC(en,gettext("Disable backslash-based line continuations > in GLSL source")) \ > diff --git a/src/mesa/drivers/dri/i965/brw_compiler.h > b/src/mesa/drivers/dri/i965/brw_compiler.h > index 224ddb1..748ffe5 100644 > --- a/src/mesa/drivers/dri/i965/brw_compiler.h > +++ b/src/mesa/drivers/dri/i965/brw_compiler.h > @@ -246,6 +246,7 @@ struct brw_wm_prog_key { > bool compute_sample_id:1; > unsigned line_aa:2; > bool high_quality_derivatives:1; > + bool force_dual_color_blend:1; > > uint16_t drawable_height; > uint64_t input_slots_valid; > diff --git a/src/mesa/drivers/dri/i965/brw_context.c > b/src/mesa/drivers/dri/i965/brw_context.c > index 9ba3339..1032e5a 100644 > --- a/src/mesa/drivers/dri/i965/brw_context.c > +++ b/src/mesa/drivers/dri/i965/brw_context.c > @@ -750,6 +750,9 @@ brw_process_driconf_options(struct brw_context *brw) > > ctx->Const.AllowGLSLExtensionDirectiveMidShader = > driQueryOptionb(options, "allow_glsl_extension_directive_midshader"); > + > + brw->dual_color_blend_by_location = > + driQueryOptionb(options, "dual_color_blend_by_location"); > } > > GLboolean > diff --git a/src/mesa/drivers/dri/i965/brw_context.h > b/src/mesa/drivers/dri/i965/brw_context.h > index 2a29dfe..55d6723 100644 > --- a/src/mesa/drivers/dri/i965/brw_context.h > +++ b/src/mesa/drivers/dri/i965/brw_context.h > @@ -836,6 +836,7 @@ struct brw_context > bool always_flush_cache; > bool disable_throttling; > bool precompile; > + bool dual_color_blend_by_location; > > driOptionCache optionCache; > /** @} */ > diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > index d7bcc1c..f9df2a4 100644 > --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > @@ -130,7 +130,11 @@ fs_visitor::nir_setup_outputs() > break; > } > case MESA_SHADER_FRAGMENT: > - if (var->data.index > 0) { > + if (key->force_dual_color_blend && > + var->data.location == FRAG_RESULT_DATA1) { > + this->dual_src_output = reg; > + this->do_dual_src = true; > + } else if (var->data.index > 0) { > assert(var->data.location == FRAG_RESULT_DATA0); > assert(var->data.index == 1); > this->dual_src_output = reg; > diff --git a/src/mesa/drivers/dri/i965/brw_wm.c > b/src/mesa/drivers/dri/i965/brw_wm.c > index 39d644e..78846dc 100644 > --- a/src/mesa/drivers/dri/i965/brw_wm.c > +++ b/src/mesa/drivers/dri/i965/brw_wm.c > @@ -515,6 +515,10 @@ brw_wm_populate_key(struct brw_context *brw, struct > brw_wm_prog_key *key) > /* _NEW_BUFFERS */ > key->nr_color_regions = ctx->DrawBuffer->_NumColorDrawBuffers; > > + /* _NEW_COLOR */ > + key->force_dual_color_blend = brw->dual_color_blend_by_location && > + (ctx->Color.BlendEnabled & 1) && ctx->Color.Blend[0]._UsesDualSrc; > + > /* _NEW_MULTISAMPLE, _NEW_COLOR, _NEW_BUFFERS */ > key->replicate_alpha = ctx->DrawBuffer->_NumColorDrawBuffers > 1 && > (ctx->Multisample.SampleAlphaToCoverage || ctx->Color.AlphaEnabled); > diff --git a/src/mesa/drivers/dri/i965/intel_screen.c > b/src/mesa/drivers/dri/i965/intel_screen.c > index e1e1e62..bca783a 100644 > --- a/src/mesa/drivers/dri/i965/intel_screen.c > +++ b/src/mesa/drivers/dri/i965/intel_screen.c > @@ -79,6 +79,7 @@ DRI_CONF_BEGIN > DRI_CONF_FORCE_GLSL_EXTENSIONS_WARN("false") > DRI_CONF_DISABLE_GLSL_LINE_CONTINUATIONS("false") > DRI_CONF_DISABLE_BLEND_FUNC_EXTENDED("false") > + DRI_CONF_DUAL_COLOR_BLEND_BY_LOCATION("false") > DRI_CONF_ALLOW_GLSL_EXTENSION_DIRECTIVE_MIDSHADER("false") > > DRI_CONF_OPT_BEGIN_B(shader_precompile, "true") > -- > 2.7.0 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
