[Mesa-dev] [PATCH 2/2] glsl: Nerf assert about qualifiers in lower_packed_varyings().

[Iago Toral]

On Sat, 2017-02-11 at 02:36 -0800, Kenneth Graunke wrote:
> In ES 3.0, interpret_interpolation_qualifier() defaults unset
> interpolation qualifiers to "smooth"...which has the strange result
> of marking some integer variables "smooth".
> 
> Interpolation qualifiers really only matter for fragment shader
> inputs,
> other than enforcing the linker rules that changed in various
> language
> versions.  By the time we get to lower_packed_varyings, we probably
> don't care anymore.

This is probably true...

> Just ignore the assert for non-FS stages.  It's sketchy, but this
> whole
> pass is pretty sketchy and needs to die...

... but yeah, this is very sketchy. If this is produced in ES shaders
due to the default smooth qualifier rules that you mention, aren't the
shaders in these tests that trigger this assertion actually bogus?

Then again, if we think this a problem that we can see in other
applications frequently (I would not be surprised) then maybe this
makes sense even if the tests are bogus... In that case I think I'd
suggest:

1. Expand the comment in the code to explain why we are skipping
geometry stages from the check.

2. Maybe skip geometry stages from the assert only for ES shaders,
since that is the only case where we get the odd behavior.

Makes sense?

> Fixes dEQP-
> GLES31.functional.program_interface_query.program_input.type.
> separable_geometry.{int,ivec2,ivec3,ivec4,uint,uvec2,uvec3,uvec4}.
> 
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/compiler/glsl/lower_packed_varyings.cpp | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/compiler/glsl/lower_packed_varyings.cpp
> b/src/compiler/glsl/lower_packed_varyings.cpp
> index 13f7e5b52da..989caf63c14 100644
> --- a/src/compiler/glsl/lower_packed_varyings.cpp
> +++ b/src/compiler/glsl/lower_packed_varyings.cpp
> @@ -282,7 +282,8 @@ lower_packed_varyings_visitor::run(struct
> gl_linked_shader *shader)
>         * together when their interpolation mode is "flat".  Treat
> integers as
>         * being flat when the interpolation mode is none.
>         */
> -      assert(var->data.interpolation == INTERP_MODE_FLAT ||
> +      assert(shader->Stage != MESA_SHADER_FRAGMENT ||
> +             var->data.interpolation == INTERP_MODE_FLAT ||
>               var->data.interpolation == INTERP_MODE_NONE ||
>               !var->type->contains_integer());
>