[Mesa-dev] [PATCH V3 3/6] glsl: store uniform slot id in var location field

[Jason Ekstrand]

On Tue, Sep 1, 2015 at 7:44 PM, Timothy Arceri <t_arceri at yahoo.com.au> wrote:
> This will allow us to access the uniform later on without resorting to
> building a name string and looking it up in UniformHash.
>
> V2: store slot number for all non-UBO uniforms to make code more
> consitent, renamed explicit_binding to explicit_location and added
> comment about what it does. Store the location at every shader stage.
> Updated data.location comments in ir/nir.h.
> ---
>  src/glsl/ir.h              |  2 ++
>  src/glsl/link_uniforms.cpp | 27 +++++++++++++++++++++++----
>  src/glsl/nir/nir.h         |  1 +
>  3 files changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/src/glsl/ir.h b/src/glsl/ir.h
> index ede8caa..d06abaf 100644
> --- a/src/glsl/ir.h
> +++ b/src/glsl/ir.h
> @@ -819,6 +819,8 @@ public:
>         *   - Fragment shader output: one of the values from \c gl_frag_result.
>         *   - Uniforms: Per-stage uniform slot number for default uniform block.
>         *   - Uniforms: Index within the uniform block definition for UBO members.
> +       *   - Non-UBO Uniforms: explicit binding until linking then reused to
> +       *     store uniform slot number.
>         *   - Other: This field is not currently used.
>         *
>         * If the variable is a uniform, shader input, or shader output, and the
> diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
> index da8f2f7..f68ea9d 100644
> --- a/src/glsl/link_uniforms.cpp
> +++ b/src/glsl/link_uniforms.cpp
> @@ -528,7 +528,13 @@ public:
>                      var->get_interface_type()->name);
>           else
>              process(var);
> -      } else
> +      } else {
> +         /* Store any explicit location and reset data location so we can
> +          * reuse this variable for storing the uniform slot number.
> +          */
> +         this->explicit_location = current_var->data.location;
> +         current_var->data.location = -1;
> +
>           process(var);
>        }
>        delete this->record_next_sampler;
> @@ -706,6 +712,13 @@ private:
>        handle_images(base_type, &this->uniforms[id]);
>        handle_subroutines(base_type, &this->uniforms[id]);
>
> +      /* For array of arrays or struct arrays the base location may have
> +       * already been set so dont set it again.
> +       */
> +      if (ubo_block_index == -1 && current_var->data.location == -1) {
> +         current_var->data.location = id;
> +      }
> +
>        /* If there is already storage associated with this uniform or if the
>         * uniform is set as builtin, it means that it was set while processing
>         * an earlier shader stage.  For example, we may be processing the
> @@ -720,12 +733,13 @@ private:
>        if (current_var->data.explicit_location) {
>           /* Set sequential locations for struct fields. */
>           if (record_type != NULL) {
> -            const unsigned entries = MAX2(1, this->uniforms[id].array_elements);
> +            const unsigned entries =
> +               MAX2(1, this->uniforms[id].array_elements);

Spurious change?  I'm all for line-wrapping but if that's all this is,
it should be separate.  Other than that,

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

>              this->uniforms[id].remap_location =
> -               current_var->data.location + field_counter;
> +               this->explicit_location + field_counter;
>              field_counter += entries;
>           } else {
> -            this->uniforms[id].remap_location = current_var->data.location;
> +         this->uniforms[id].remap_location = this->explicit_location;
>           }
>        } else {
>           /* Initialize to to indicate that no location is set */
> @@ -791,6 +805,11 @@ private:
>     unsigned next_image;
>     unsigned next_subroutine;
>
> +   /* Used to store the explicit location from current_var so that we can
> +    * reuse the location field for storing the uniform slot id.
> +    */
> +   int explicit_location;
> +
>     /* Stores total struct array elements including nested structs */
>     unsigned record_array_count;
>
> diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
> index 9703372..36fbf5c 100644
> --- a/src/glsl/nir/nir.h
> +++ b/src/glsl/nir/nir.h
> @@ -278,6 +278,7 @@ typedef struct {
>         *   - Fragment shader output: one of the values from \c gl_frag_result.
>         *   - Uniforms: Per-stage uniform slot number for default uniform block.
>         *   - Uniforms: Index within the uniform block definition for UBO members.
> +       *   - Non-UBO Uniforms: uniform slot number.
>         *   - Other: This field is not currently used.
>         *
>         * If the variable is a uniform, shader input, or shader output, and the
> --
> 2.4.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev