[Mesa-dev] [PATCH 2/2] glsl: Fix uniform buffer object counting.

[Ian Romanick]

On 05/31/2013 12:37 PM, Eric Anholt wrote:
> We were counting uniforms located in UBOs against the default uniform
> block limit, while not doing any counting against the specific combined
> limit.
>
> Note that I couldn't quite find justification for the way I did this, but
> I think it's the only sensible thing: The spec talks about components, so
> each "float" in a std140 block would count as 1 component and a "vec4"
> would count as 4, though they occupy the same amount of space.  Since GPU
> limits on uniform buffer loads are surely going to be about the size of
> the blocks, I just counted them that way.
>
> Fixes link failures in piglit
> arb_uniform_buffer_object/maxuniformblocksize when ported to geometry
> shaders on Paul's GS branch, since in that case the max block size is
> bigger than the default uniform block component limit.

Oh, the silly max combined components check. :(  I think we got rid of 
that redundant check in some later 4.x version.  That said, this change is

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

> ---
>   src/glsl/link_uniforms.cpp | 12 +++++++++++-
>   src/glsl/linker.cpp        | 25 +++++++++++++++++++++++--
>   src/mesa/main/mtypes.h     | 10 +++++++++-
>   3 files changed, 43 insertions(+), 4 deletions(-)
>
> diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
> index 54d54cf..c4f86c8 100644
> --- a/src/glsl/link_uniforms.cpp
> +++ b/src/glsl/link_uniforms.cpp
> @@ -170,6 +170,7 @@ public:
>
>      void process(ir_variable *var)
>      {
> +      this->is_ubo_var = var->is_in_uniform_block();
>         if (var->is_interface_instance())
>            program_resource_visitor::process(var->interface_type,
>                                              var->interface_type->name);
> @@ -197,6 +198,8 @@ public:
>       */
>      unsigned num_shader_uniform_components;
>
> +   bool is_ubo_var;
> +
>   private:
>      virtual void visit_field(const glsl_type *type, const char *name,
>                               bool row_major)
> @@ -222,7 +225,8 @@ private:
>   	  * Note that samplers do not count against this limit because they
>   	  * don't use any storage on current hardware.
>   	  */
> -	 this->num_shader_uniform_components += values;
> +	 if (!is_ubo_var)
> +	    this->num_shader_uniform_components += values;
>         }
>
>         /* If the uniform is already in the map, there's nothing more to do.
> @@ -682,6 +686,12 @@ link_assign_uniform_locations(struct gl_shader_program *prog)
>
>         sh->num_samplers = uniform_size.num_shader_samplers;
>         sh->num_uniform_components = uniform_size.num_shader_uniform_components;
> +
> +      sh->num_combined_uniform_components = sh->num_uniform_components;
> +      for (unsigned i = 0; i < sh->NumUniformBlocks; i++) {
> +	 sh->num_combined_uniform_components +=
> +	    sh->UniformBlocks[i].UniformBufferSize / 4;
> +      }
>      }
>
>      const unsigned num_user_uniforms = uniform_size.num_active_uniforms;
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index 982fe46..cd8d680 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -1523,12 +1523,18 @@ check_resources(struct gl_context *ctx, struct gl_shader_program *prog)
>         ctx->Const.GeometryProgram.MaxTextureImageUnits
>      };
>
> -   const unsigned max_uniform_components[MESA_SHADER_TYPES] = {
> +   const unsigned max_default_uniform_components[MESA_SHADER_TYPES] = {
>         ctx->Const.VertexProgram.MaxUniformComponents,
>         ctx->Const.FragmentProgram.MaxUniformComponents,
>         ctx->Const.GeometryProgram.MaxUniformComponents
>      };
>
> +   const unsigned max_combined_uniform_components[MESA_SHADER_TYPES] = {
> +      ctx->Const.VertexProgram.MaxCombinedUniformComponents,
> +      ctx->Const.FragmentProgram.MaxCombinedUniformComponents,
> +      ctx->Const.GeometryProgram.MaxCombinedUniformComponents
> +   };
> +
>      const unsigned max_uniform_blocks[MESA_SHADER_TYPES] = {
>         ctx->Const.VertexProgram.MaxUniformBlocks,
>         ctx->Const.FragmentProgram.MaxUniformBlocks,
> @@ -1546,7 +1552,22 @@ check_resources(struct gl_context *ctx, struct gl_shader_program *prog)
>   		      shader_names[i]);
>         }
>
> -      if (sh->num_uniform_components > max_uniform_components[i]) {
> +      if (sh->num_uniform_components > max_default_uniform_components[i]) {
> +         if (ctx->Const.GLSLSkipStrictMaxUniformLimitCheck) {
> +            linker_warning(prog, "Too many %s shader default uniform block "
> +                           "components, but the driver will try to optimize "
> +                           "them out; this is non-portable out-of-spec "
> +			   "behavior\n",
> +                           shader_names[i]);
> +         } else {
> +            linker_error(prog, "Too many %s shader default uniform block "
> +			 "components",
> +                         shader_names[i]);
> +         }
> +      }
> +
> +      if (sh->num_combined_uniform_components >
> +	  max_combined_uniform_components[i]) {
>            if (ctx->Const.GLSLSkipStrictMaxUniformLimitCheck) {
>               linker_warning(prog, "Too many %s shader uniform components, "
>                              "but the driver will try to optimize them out; "
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 186f8a0..d982391 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -2141,13 +2141,21 @@ struct gl_shader
>      gl_texture_index SamplerTargets[MAX_SAMPLERS];
>
>      /**
> -    * Number of uniform components used by this shader.
> +    * Number of default uniform block components used by this shader.
>       *
>       * This field is only set post-linking.
>       */
>      unsigned num_uniform_components;
>
>      /**
> +    * Number of combined uniform components used by this shader.
> +    *
> +    * This field is only set post-linking.  It is the sum of the uniform block
> +    * sizes divided by sizeof(float), and num_uniform_compoennts.
> +    */
> +   unsigned num_combined_uniform_components;
> +
> +   /**
>       * This shader's uniform block information.
>       *
>       * The offsets of the variables are assigned only for shaders in a program's
>