On 28/09/15 00:05, Timothy Arceri wrote: > On Fri, 2015-09-25 at 10:24 +0200, Samuel Iglesias Gonsalvez wrote: >> From ARB_program_interface_query: >> >> "For an active shader storage block member declared as an array, an >> entry will be generated only for the first array element, regardless >> of its type. For arrays of aggregate types, the enumeration rules >> are >> applied recursively for the single enumerated array element." >> >> Signed-off-by: Samuel Iglesias Gonsalvez <sigles...@igalia.com> >> --- >> src/glsl/linker.cpp | 56 >> +++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 56 insertions(+) >> >> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp >> index be04f5b..8cc9350 100644 >> --- a/src/glsl/linker.cpp >> +++ b/src/glsl/linker.cpp >> @@ -3134,6 +3134,57 @@ check_explicit_uniform_locations(struct >> gl_context *ctx, >> } >> >> static bool >> +should_add_buffer_variable(struct gl_shader_program *shProg, >> + GLenum type, const char *name) >> +{ >> + bool found_interface = false; >> + const char *block_name = NULL; >> + >> + assert(type == GL_BUFFER_VARIABLE); >> + >> + for (unsigned i = 0; i < shProg->NumUniformShaderStorageBlocks; >> i++) { >> + block_name = shProg->UniformBlocks[i].Name; >> + if (strncmp(block_name, name, strlen(block_name)) == 0) { >> + found_interface = true; >> + break; >> + } >> + } >> + >> + /* We remove the interface name from the buffer variable name, >> + * including the dot that follows it. >> + */ >> + if (found_interface) >> + name = name + strlen(block_name) + 1; >> + >> + /* From: ARB_program_interface_query extension: >> + * >> + * "For an active shader storage block member declared as an >> array, an >> + * entry will be generated only for the first array element, >> regardless >> + * of its type. For arrays of aggregate types, the enumeration >> rules are >> + * applied recursively for the single enumerated array element. >> + */ >> + const char *first_dot = strchr(name, '.'); >> + const char *first_square_bracket = strchr(name, '['); >> + >> + /* The buffer variable is on top level and it is not an array or >> struct */ >> + if (!first_square_bracket && !first_dot) { >> + return true; >> + /* The shader storage block member is a struct, then generate the >> entry */ >> + } else if ((!first_square_bracket || >> + (first_dot && first_dot < first_square_bracket))) { > > I think the above can be simplified to: > > if (!first_square_bracket) { > return true; > /* The shader storage block member is a struct, then generate the > entry */ > } else if (first_dot && first_dot < first_square_bracket)) { >
Yes, thanks. >> + return true; >> + } else { >> + /* Shader storage block member is an array, only generate an >> entry for the >> + * first array element. >> + */ >> + if (strncmp(first_square_bracket, "[0]", 3) == 0) >> + return true; >> + } >> + >> + return false; >> +} >> + >> +static bool >> add_program_resource(struct gl_shader_program *prog, GLenum type, >> const void *data, uint8_t stages) >> { >> @@ -3408,6 +3459,11 @@ build_program_resource_list(struct >> gl_shader_program *shProg) >> >> bool is_shader_storage = shProg >> ->UniformStorage[i].is_shader_storage; >> GLenum type = is_shader_storage ? GL_BUFFER_VARIABLE : >> GL_UNIFORM; >> + if (is_shader_storage && >> + !should_add_buffer_variable(shProg, type, >> + shProg >> ->UniformStorage[i].name)) > > Rather than check is_shader_storage here it would be better to replace > the assert at the top of should_add_buffer_variable() with: > > if (type != GL_BUFFER_VARIABLE) > return false; > Actually, it should return true, as we don't want to skip any uniform. I will add this code to should_add_buffer_variable() together with a comment to explain why it returns true. Sam >> + continue; >> + >> if (!add_program_resource(shProg, type, >> &shProg->UniformStorage[i], >> stageref)) >> return; > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev