<div dir="ltr">On 12 December 2013 08:32, Francisco Jerez <<a href="mailto:currojerez@riseup.net" target="_blank">currojerez@riseup.net</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> writes: > On 26 November 2013 00:02, Francisco Jerez <<a href="mailto:currojerez@riseup.net">currojerez@riseup.net</a>> 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. > </div></div>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... </blockquote><div> </div><div>Argh, that's frustrating. There's no good reason to have two enums. I'll send out a patch series that consolidates them into one. </div> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div class="HOEnZb"><div class="h5"> > >> + 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 <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> </div></div></blockquote></div> </div></div>