On 12 December 2013 08:32, Francisco Jerez <curroje...@riseup.net> wrote:
> 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... > 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. > > > > > >> + 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> >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev