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

Submitted by Kenneth Graunke on Feb. 11, 2017, 10:36 a.m.

Details

Message ID 20170211103640.19849-2-kenneth@whitecape.org
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Kenneth Graunke Feb. 11, 2017, 10:36 a.m.
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.

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

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@whitecape.org>
---
 src/compiler/glsl/lower_packed_varyings.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

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());
 

Comments

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@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());
>