Yeah, this is nicer :)
Reviewed-by: Iago Toral Quiroga <ito...@igalia.com>
On Tue, 2017-11-07 at 10:20 -0800, Kenneth Graunke wrote:
> According to the GLSL ES 3.20, GLSL 4.50, and GLSL 1.20 specs:
>
> "To force all output variables to be invariant, use the pragma
>
> #pragma STDGL invariant(all)
>
> before all declarations in a shader."
>
> Notably, this is only supposed to affect output
> variables. Furthermore,
>
> "Only variables output from a shader can be candidates for
> invariance."
>
> It looks like this has been wrong since we first supported the pragma
> in
> 2011 (commit 86b4398cd158024f6be9fa830554a11c2a7ebe0c).
>
> Fixes dEQP-
> GLES2.functional.shaders.preprocessor.pragmas.pragma_fragment.
>
> v2: Now that all cases are identical (other than compute shaders,
> which
> have no output variables anyway), we can drop the switch
> statement
> entirely. We also don't need the current_function == NULL check;
> this was a hold over from when we had a single var_mode_out for
> both
> function parameters and shader varyings, in the bad old days.
>
> Reviewed-by: Iago Toral Quiroga <ito...@igalia.com> [v1]
> Reviewed-by: Ilia Mirkin <imir...@alum.mit.edu> [v1]
> ---
> src/compiler/glsl/ast_to_hir.cpp | 26 ++------------------------
> 1 file changed, 2 insertions(+), 24 deletions(-)
>
> Good call, Ilia :) I'd originally had a /* Invariance isn't
> meaningful
> for fragment shader outputs */ comment, but then I looked elsewhere
> in
> the file and realized that it actually was allowed. So, I changed
> the
> case, but didn't think to combine it.
>
> It turns out we can combine all of them...here's a better version.
>
> diff --git a/src/compiler/glsl/ast_to_hir.cpp
> b/src/compiler/glsl/ast_to_hir.cpp
> index 441404f86d3..1794a1af5cb 100644
> --- a/src/compiler/glsl/ast_to_hir.cpp
> +++ b/src/compiler/glsl/ast_to_hir.cpp
> @@ -4077,30 +4077,8 @@ apply_type_qualifier_to_variable(const struct
> ast_type_qualifier *qual,
> }
> }
>
> - if (state->all_invariant && (state->current_function == NULL)) {
> - switch (state->stage) {
> - case MESA_SHADER_VERTEX:
> - if (var->data.mode == ir_var_shader_out)
> - var->data.invariant = true;
> - break;
> - case MESA_SHADER_TESS_CTRL:
> - case MESA_SHADER_TESS_EVAL:
> - case MESA_SHADER_GEOMETRY:
> - if ((var->data.mode == ir_var_shader_in)
> - || (var->data.mode == ir_var_shader_out))
> - var->data.invariant = true;
> - break;
> - case MESA_SHADER_FRAGMENT:
> - if (var->data.mode == ir_var_shader_in)
> - var->data.invariant = true;
> - break;
> - case MESA_SHADER_COMPUTE:
> - /* Invariance isn't meaningful in compute shaders. */
> - break;
> - default:
> - break;
> - }
> - }
> + if (state->all_invariant && var->data.mode == ir_var_shader_out)
> + var->data.invariant = true;
>
> var->data.interpolation =
> interpret_interpolation_qualifier(qual, var->type,
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
