[Mesa-dev] [PATCH 2/2] i965: Fix scalar vertex shader struct outputs.

[Jason Ekstrand]

On Tue, Nov 24, 2015 at 10:35 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> While we correctly set output[] for composite varyings, we set completely
> bogus values for output_components[], making emit_urb_writes() output
> zeros instead of the actual values.
>
> Unfortunately, our simple approach goes out the window, and we need to
> recurse into structs to get the proper value of vector_elements for each
> field.
>
> Together with the previous patch, this fixes rendering in an upcoming
> game from Feral Interactive.
>
> Cc: "11.1 11.0" <mesa-stable at lists.freedesktop.org>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/brw_fs.h       |  2 ++
>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 40 +++++++++++++++++++++++++-------
>  2 files changed, 34 insertions(+), 8 deletions(-)
>
> Again, happy to switch to pointers if people would prefer.
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
> index aded70b..3a5f334 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -243,6 +243,8 @@ public:
>
>     void emit_nir_code();
>     void nir_setup_inputs();
> +   void nir_setup_single_output_varying(fs_reg &reg, const glsl_type *type,
> +                                        unsigned &location);
>     void nir_setup_outputs();
>     void nir_setup_uniforms();
>     void nir_emit_system_values();
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index a094eef..9528bc5 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -91,6 +91,32 @@ fs_visitor::nir_setup_inputs()
>  }
>
>  void
> +fs_visitor::nir_setup_single_output_varying(fs_reg &reg,
> +                                            const glsl_type *type,
> +                                            unsigned &location)

Again, inout params.  Pointers please.

With that changed,

Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com>

> +{
> +   if (type->is_array() || type->is_matrix()) {
> +      const struct glsl_type *elem_type = glsl_get_array_element(type);
> +      const unsigned length = glsl_get_length(type);
> +
> +      for (unsigned i = 0; i < length; i++) {
> +         nir_setup_single_output_varying(reg, elem_type, location);
> +      }
> +   } else if (type->is_record()) {
> +      for (unsigned i = 0; i < type->length; i++) {
> +         const struct glsl_type *field_type = type->fields.structure[i].type;
> +         nir_setup_single_output_varying(reg, field_type, location);
> +      }
> +   } else {
> +      assert(type->is_scalar() || type->is_vector());
> +      this->outputs[location] = reg;
> +      this->output_components[location] = type->vector_elements;
> +      reg = offset(reg, bld, 4);
> +      location++;
> +   }
> +}
> +
> +void
>  fs_visitor::nir_setup_outputs()
>  {
>     brw_wm_prog_key *key = (brw_wm_prog_key*) this->key;
> @@ -100,17 +126,13 @@ fs_visitor::nir_setup_outputs()
>     nir_foreach_variable(var, &nir->outputs) {
>        fs_reg reg = offset(nir_outputs, bld, var->data.driver_location);
>
> -      int vector_elements = var->type->without_array()->vector_elements;
> -
>        switch (stage) {
>        case MESA_SHADER_VERTEX:
> -      case MESA_SHADER_GEOMETRY:
> -         for (int i = 0; i < type_size_vec4(var->type); i++) {
> -            int output = var->data.location + i;
> -            this->outputs[output] = offset(reg, bld, 4 * i);
> -            this->output_components[output] = vector_elements;
> -         }
> +      case MESA_SHADER_GEOMETRY: {
> +         unsigned location = var->data.location;
> +         nir_setup_single_output_varying(reg, var->type, location);
>           break;
> +      }
>        case MESA_SHADER_FRAGMENT:
>           if (var->data.index > 0) {
>              assert(var->data.location == FRAG_RESULT_DATA0);
> @@ -130,6 +152,8 @@ fs_visitor::nir_setup_outputs()
>           } else if (var->data.location == FRAG_RESULT_SAMPLE_MASK) {
>              this->sample_mask = reg;
>           } else {
> +            int vector_elements = var->type->without_array()->vector_elements;
> +
>              /* gl_FragData or a user-defined FS output */
>              assert(var->data.location >= FRAG_RESULT_DATA0 &&
>                     var->data.location < FRAG_RESULT_DATA0+BRW_MAX_DRAW_BUFFERS);
> --
> 2.6.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev