[Mesa-dev] [PATCH v4 1/2] glsl: allow any l-value of an input variable as interpolant in interpolateAt*

Timothy Arceri tarceri at itsqueeze.com
Fri Oct 27 10:37:13 UTC 2017


I meant to review this a while ago. Sorry for the delay.

Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>

On 10/10/17 23:09, Nicolai Hähnle wrote:
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
> 
> The intended rule has been clarified in GLSL 4.60, Section 8.13.2
> (Interpolation Functions):
> 
>     "For all of the interpolation functions, interpolant must be an l-value
>      from an in declaration; this can include a variable, a block or
>      structure member, an array element, or some combination of these.
>      Component selection operators (e.g., .xy) may be used when specifying
>      interpolant."
> 
> For members of interface blocks, var->data.must_be_shader_input must be
> determined on-the-fly after lowering interface blocks, since we don't want
> to disable varying packing for an entire block just because one input in it
> is used in interpolateAt*.
> 
> v2: keep setting must_be_shader_input in ast_function (Ian)
> v3: follow the relaxed rule of GLSL 4.60
> v4: only apply the relaxed rules to desktop GL
>      (the ES WG decided that the relaxed rules may apply in a future version
>       but not retroactively; see also
>       dEQP-GLES31.functional.shaders.multisample_interpolation.interpolate_at_centroid.negative.*)
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101378
> Reviewed-by: Ian Romanick <ian.d.romanick at intel.com> (v1)
> ---
>   src/compiler/glsl/ast_function.cpp                 | 19 ++++++++++++++-----
>   src/compiler/glsl/lower_named_interface_blocks.cpp | 18 ++++++++++++++++++
>   2 files changed, 32 insertions(+), 5 deletions(-)
> 
> diff --git a/src/compiler/glsl/ast_function.cpp b/src/compiler/glsl/ast_function.cpp
> index 46a61e46fd5..d1596c272e6 100644
> --- a/src/compiler/glsl/ast_function.cpp
> +++ b/src/compiler/glsl/ast_function.cpp
> @@ -220,33 +220,42 @@ verify_parameter_modes(_mesa_glsl_parse_state *state,
>            if (val->ir_type == ir_type_swizzle) {
>               if (!state->is_version(440, 0)) {
>                  _mesa_glsl_error(&loc, state,
>                                   "parameter `%s` must not be swizzled",
>                                   formal->name);
>                  return false;
>               }
>               val = ((ir_swizzle *)val)->val;
>            }
>   
> -         while (val->ir_type == ir_type_dereference_array) {
> -            val = ((ir_dereference_array *)val)->array;
> +         for (;;) {
> +            if (val->ir_type == ir_type_dereference_array) {
> +               val = ((ir_dereference_array *)val)->array;
> +            } else if (val->ir_type == ir_type_dereference_record &&
> +                       !state->es_shader) {
> +               val = ((ir_dereference_record *)val)->record;
> +            } else
> +               break;
>            }
>   
> -         if (!val->as_dereference_variable() ||
> -             val->variable_referenced()->data.mode != ir_var_shader_in) {
> +         ir_variable *var = NULL;
> +         if (const ir_dereference_variable *deref_var = val->as_dereference_variable())
> +            var = deref_var->variable_referenced();
> +
> +         if (!var || var->data.mode != ir_var_shader_in) {
>               _mesa_glsl_error(&loc, state,
>                                "parameter `%s` must be a shader input",
>                                formal->name);
>               return false;
>            }
>   
> -         val->variable_referenced()->data.must_be_shader_input = 1;
> +         var->data.must_be_shader_input = 1;
>         }
>   
>         /* Verify that 'out' and 'inout' actual parameters are lvalues. */
>         if (formal->data.mode == ir_var_function_out
>             || formal->data.mode == ir_var_function_inout) {
>            const char *mode = NULL;
>            switch (formal->data.mode) {
>            case ir_var_function_out:   mode = "out";   break;
>            case ir_var_function_inout: mode = "inout"; break;
>            default:                    assert(false);  break;
> diff --git a/src/compiler/glsl/lower_named_interface_blocks.cpp b/src/compiler/glsl/lower_named_interface_blocks.cpp
> index 064694128bf..136352a131b 100644
> --- a/src/compiler/glsl/lower_named_interface_blocks.cpp
> +++ b/src/compiler/glsl/lower_named_interface_blocks.cpp
> @@ -108,20 +108,21 @@ public:
>   
>      flatten_named_interface_blocks_declarations(void *mem_ctx)
>         : mem_ctx(mem_ctx),
>           interface_namespace(NULL)
>      {
>      }
>   
>      void run(exec_list *instructions);
>   
>      virtual ir_visitor_status visit_leave(ir_assignment *);
> +   virtual ir_visitor_status visit_leave(ir_expression *);
>      virtual void handle_rvalue(ir_rvalue **rvalue);
>   };
>   
>   } /* anonymous namespace */
>   
>   void
>   flatten_named_interface_blocks_declarations::run(exec_list *instructions)
>   {
>      interface_namespace = _mesa_hash_table_create(NULL, _mesa_key_hash_string,
>                                                    _mesa_key_string_equal);
> @@ -231,20 +232,37 @@ flatten_named_interface_blocks_declarations::visit_leave(ir_assignment *ir)
>         }
>   
>         ir_variable *lhs_var =  lhs_rec_tmp->variable_referenced();
>         if (lhs_var) {
>            lhs_var->data.assigned = 1;
>         }
>      }
>      return rvalue_visit(ir);
>   }
>   
> +ir_visitor_status
> +flatten_named_interface_blocks_declarations::visit_leave(ir_expression *ir)
> +{
> +   ir_visitor_status status = rvalue_visit(ir);
> +
> +   if (ir->operation == ir_unop_interpolate_at_centroid ||
> +       ir->operation == ir_binop_interpolate_at_offset ||
> +       ir->operation == ir_binop_interpolate_at_sample) {
> +      const ir_rvalue *val = ir->operands[0];
> +
> +      /* This disables varying packing for this input. */
> +      val->variable_referenced()->data.must_be_shader_input = 1;
> +   }
> +
> +   return status;
> +}
> +
>   void
>   flatten_named_interface_blocks_declarations::handle_rvalue(ir_rvalue **rvalue)
>   {
>      if (*rvalue == NULL)
>         return;
>   
>      ir_dereference_record *ir = (*rvalue)->as_dereference_record();
>      if (ir == NULL)
>         return;
>   
> 


More information about the mesa-dev mailing list