Paul Berry <stereotype...@gmail.com> writes:
> On 26 November 2013 00:02, Francisco Jerez <curroje...@riseup.net> wrote:
>
>> ---
>> src/glsl/link_uniforms.cpp | 13 +++++++++++-
>> src/glsl/linker.cpp | 50
>> ++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 62 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
>> index 0a15739..c75a38c 100644
>> --- a/src/glsl/link_uniforms.cpp
>> +++ b/src/glsl/link_uniforms.cpp
>> @@ -240,7 +240,8 @@ class count_uniform_size : public
>> program_resource_visitor {
>> public:
>> count_uniform_size(struct string_to_uint_map *map)
>> : num_active_uniforms(0), num_values(0), num_shader_samplers(0),
>> - num_shader_uniform_components(0), is_ubo_var(false), map(map)
>> + num_shader_images(0), num_shader_uniform_components(0),
>> + is_ubo_var(false), map(map)
>> {
>> /* empty */
>> }
>> @@ -248,6 +249,7 @@ public:
>> void start_shader()
>> {
>> this->num_shader_samplers = 0;
>> + this->num_shader_images = 0;
>> this->num_shader_uniform_components = 0;
>> }
>>
>> @@ -277,6 +279,11 @@ public:
>> unsigned num_shader_samplers;
>>
>> /**
>> + * Number of images used
>> + */
>> + unsigned num_shader_images;
>> +
>> + /**
>> * Number of uniforms used in the current shader
>> */
>> unsigned num_shader_uniform_components;
>> @@ -303,6 +310,9 @@ private:
>> if (type->contains_sampler()) {
>> this->num_shader_samplers +=
>> type->is_array() ? type->array_size() : 1;
>> + } else if (type->contains_image()) {
>> + this->num_shader_images += values;
>> + this->num_shader_uniform_components += values;
>>
>
> How come images contribute to num_shader_uniform_components but samplers
> don't? There should be a comment explaining this difference.
>
>
>> } else {
>> /* Accumulate the total number of uniform slots used by this
>> shader.
>> * Note that samplers do not count against this limit because they
>> @@ -782,6 +792,7 @@ link_assign_uniform_locations(struct gl_shader_program
>> *prog)
>> }
>>
>> sh->num_samplers = uniform_size.num_shader_samplers;
>> + sh->NumImages = uniform_size.num_shader_images;
>> sh->num_uniform_components =
>> uniform_size.num_shader_uniform_components;
>>
>> sh->num_combined_uniform_components = sh->num_uniform_components;
>> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
>> index fac186a..86a0cae 100644
>> --- a/src/glsl/linker.cpp
>> +++ b/src/glsl/linker.cpp
>> @@ -1980,6 +1980,55 @@ check_resources(struct gl_context *ctx, struct
>> gl_shader_program *prog)
>> }
>> }
>>
>> +/**
>> + * Validate shader image resources.
>> + */
>> +static void
>> +check_image_resources(struct gl_context *ctx, struct gl_shader_program
>> *prog)
>> +{
>> + STATIC_ASSERT(MESA_SHADER_TYPES == 3);
>> + static const char *const shader_names[MESA_SHADER_TYPES] = {
>> + "vertex", "geometry", "fragment"
>> + };
>>
>
> Use _mesa_glsl_shader_target_name() instead of hardcoding this array.
>
Neither of the _mesa_glsl_shader_target_name() overloads accept a
gl_shader_type as argument, the first one takes a GLenum, the second one
takes a _mesa_glsl_parser_targets enumerant -- Yes, the enum values
happen to match Mesa's gl_shader_type, but...
>
>> + const unsigned max_images[MESA_SHADER_TYPES] = {
>> + ctx->Const.VertexProgram.MaxImageUniforms,
>> + ctx->Const.GeometryProgram.MaxImageUniforms,
>> + ctx->Const.FragmentProgram.MaxImageUniforms
>> + };
>>
>
> Rather than doing STATIC_ASSERT(MESA_SHADER_TYPES == 3), I think this would
> be slightly cleaner, since it will be more obvious what needs fixing if the
> assertion ever fires:
>
> const unsigned max_images[] = {
> ctx->Const.VertexProgram.MaxImageUniforms,
> ctx->Const.GeometryProgram.MaxImageUniforms,
> ctx->Const.FragmentProgram.MaxImageUniforms
> };
> STATIC_ASSERT(Elements(max_images) == MESA_SHADER_TYPES);
>
>
> With those changes, the patch is:
>
> Reviewed-by: Paul Berry <stereotype...@gmail.com>
pgpZTDH32WAEy.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
