[Mesa-dev] [PATCH 1/2] mesa: add a workaround for unigine Tropics

Kenneth Graunke kenneth at whitecape.org
Fri Jan 30 01:55:55 PST 2015


On Friday, January 30, 2015 11:50:57 AM Martin Peres wrote:
> 
> On 30/01/15 00:26, Kenneth Graunke wrote:
> > On Thursday, January 29, 2015 11:14:26 PM Martin Peres wrote:
> >> While trying to understand a GLSL pass, curro and I tried running Unigine
> >> Tropics and the GLSL compilers would not compile the shaders.
> >>
> >> The reason is due to the fact that the shaders do not specify the needed GLSL
> >> version but also use in the same shader keywords that could never co-exist
> >> because one got deleted when the other one was added (for instance,
> >> gl_TexCoord and gl_InstanceID). The current solution was to use the
> >> force_glsl_extensions_warn workaround but it broke when GL_ARB_gpu_shader5
> >> got introduced as this workaround also enabled this extension which reserved
> >> the name "sample" which is then used by most of Unigine Tropics' shaders.
> >>
> >> To fix this, the easiest solution seem to introduce a workaround to
> >> disable GL_ARB_gpu_shader5 in the GLSL compiler. This is what this patch
> >> does along with modifying drirc to enable this workaround on tropics.
> >>
> >> This patch has been tested on Haswell. It should also work on Gallium-based
> >> drivers but I did not test it.
> >>
> >> I would like to thank curro for helping me understand the whole issue and
> >> directed me to the right place in the code.
> >>
> >> Signed-off-by: Martin Peres <martin.peres at linux.intel.com>
> > Hi Martin!
> >
> > Thanks for fixing this.  One small comment...
> >
> > [snip]
> >> diff --git a/src/mesa/drivers/dri/i915/intel_screen.c b/src/mesa/drivers/dri/i915/intel_screen.c
> >> index 00d8580..748eee7 100644
> >> --- a/src/mesa/drivers/dri/i915/intel_screen.c
> >> +++ b/src/mesa/drivers/dri/i915/intel_screen.c
> >> @@ -74,6 +74,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_DISABLE_GLSL_EXTENSION_GPU_SHADER5("false")
> >>   
> >>         DRI_CONF_OPT_BEGIN_B(shader_precompile, "true")
> >>   	 DRI_CONF_DESC(en, "Perform code generation at shader link time.")
> > You can probably drop the i915 change - it doesn't support ARB_gpu_shader5
> > anyway.
> 
> I do not think this is a good idea for two reasons:
> - Not initialising this variable will lead to an assert whenever some 
> common code will try to read the parameter.
> - This makes the i965/i915 code paths look different when they should 
> actually be shared. To ease the work of the poor soul who will have to 
> factor out all the drirc configuration parsing for all the classic 
> drivers and gallium.
> 
> > With that removed, you can add:
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82897
> 
> Oh no! I should have checked! It would have saved me quite some time!
> 
> > Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
> Do I still get your RB even if I do not delete it then?

Sure.  It really would be great to move these compiler related things out of
the various drivers and into shared code.

--Ken
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150130/c4a73e40/attachment.sig>


More information about the mesa-dev mailing list