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.pe...@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 <kenn...@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
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
