On 04/09/2014 08:02 AM, Kenneth Graunke wrote: > On 04/04/2014 02:01 PM, Ian Romanick wrote: >> From: Ian Romanick <ian.d.roman...@intel.com> >> >> In the next patch, we'll see that using >> gl_shader_program::UniformStorage is not correct for uniform blocks. >> That means we can't use ::UniformStorage to select between the sampler >> path and the block path. Instead we want to just use the type of the >> variable. That's never passed to set_uniform_binding, and it's easier > > Ehhhmm.....then....
"That" in this instance the variable, not the variable's type. I'll update the commit message. More reply below. >> to just remove the function (especially for later patches in the series) >> than to add another parameter. >> >> Signed-off-by: Ian Romanick <ian.d.roman...@intel.com> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76323 >> Cc: "10.1" <mesa-sta...@lists.freedesktop.org> >> Cc: git...@socker.lepus.uberspace.de >> --- >> src/glsl/link_uniform_initializers.cpp | 33 >> ++++++++++++--------------------- >> 1 file changed, 12 insertions(+), 21 deletions(-) >> >> diff --git a/src/glsl/link_uniform_initializers.cpp >> b/src/glsl/link_uniform_initializers.cpp >> index 6f15e69..bbdeec9 100644 >> --- a/src/glsl/link_uniform_initializers.cpp >> +++ b/src/glsl/link_uniform_initializers.cpp >> @@ -151,25 +151,6 @@ set_block_binding(void *mem_ctx, gl_shader_program >> *prog, >> } >> >> void >> -set_uniform_binding(void *mem_ctx, gl_shader_program *prog, >> - const char *name, const glsl_type *type, int binding) > > ...what exactly is this? ^^^^^^^^^^^^^^^^^^^^^ > >> -{ >> - struct gl_uniform_storage *const storage = >> - get_storage(prog->UniformStorage, prog->NumUserUniformStorage, name); >> - >> - if (storage == NULL) { >> - assert(storage != NULL); >> - return; >> - } >> - >> - if (storage->type->is_sampler()) { >> - set_sampler_binding(mem_ctx, prog, name, type, binding); >> - } else if (storage->block_index != -1) { >> - set_block_binding(mem_ctx, prog, name, type, binding); >> - } >> -} >> - >> -void >> set_uniform_initializer(void *mem_ctx, gl_shader_program *prog, >> const char *name, const glsl_type *type, >> ir_constant *val) >> @@ -268,8 +249,18 @@ link_set_uniform_initializers(struct gl_shader_program >> *prog) >> mem_ctx = ralloc_context(NULL); >> >> if (var->data.explicit_binding) { >> - linker::set_uniform_binding(mem_ctx, prog, var->name, >> - var->type, var->data.binding); >> + const glsl_type *const type = var->type; > > Here you're using type, which is var->type, which is exactly what we > were already passing. > > AFAICT all you needed to do was change: > > if (storage->type->is_sampler()) > > to > > if (type->is_sampler() || (type->is_array() && > type->fields.array->is_sampler())) > > in set_uniform_binding. That would resolve this issue, but patch 5 needs the interface type and not the variable's type. The only way to get the interface type is from the variable. Splitting it out here, then passing the interface type instead of the variable's type in a later patch resulted in what I believed to be better code. Sound reasonable? >> + >> + if (type->is_sampler() >> + || (type->is_array() && type->fields.array->is_sampler())) { >> + linker::set_sampler_binding(mem_ctx, prog, var->name, >> + type, var->data.binding); >> + } else if (var->is_in_uniform_block()) { >> + linker::set_block_binding(mem_ctx, prog, var->name, >> + type, var->data.binding); >> + } else { >> + assert(!"Explicit binding not on a sampler or UBO."); >> + } >> } else if (var->constant_value) { >> linker::set_uniform_initializer(mem_ctx, prog, var->name, >> var->type, var->constant_value); >> > >
signature.asc
Description: OpenPGP digital signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
