On 04/09/2014 09:10 AM, Kenneth Graunke wrote: > On 04/04/2014 02:01 PM, Ian Romanick wrote: >> From: Ian Romanick <[email protected]> >> >> The two code paths are quite different, and there are some problems in >> the handling of uniform blocks. Future changes will cause these paths >> to diverge further. Ultimately, selecting between the two functions >> will happen at the set_uniform_binding call site, and >> set_uniform_binding will be deleted. >> >> NOTE: This patch just moves code around. >> >> Signed-off-by: Ian Romanick <[email protected]> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76323 >> Cc: "10.1" <[email protected]> >> Cc: [email protected] >> --- >> src/glsl/link_uniform_initializers.cpp | 42 >> +++++++++++++++++++++++++++++++--- >> 1 file changed, 39 insertions(+), 3 deletions(-) > > Assuming you have a reasonable response to my comment on patch 5, this > series is: > > Reviewed-by: Kenneth Graunke <[email protected]> > > though, I'm not sure how much that's worth - I had to re-read the GLSL > rules and re-discover how our compiler IR for this stuff works. The > code seems right, but I could be totally missing something obvious. > > On that note...is it just me, or is the compiler IR for uniform blocks > rather ugly and messy?
Yes. Part of the problem is that we implemented things one "easy" way
for GL_ARB_uniform_buffer_objects / OpenGL 3.1, but the addition of 3.2
and OpenGL ES 3.0 features made the easy implementation not work.
Rather that completely gut everything, I refactored a bunch of stuff
and, basically, added a parallel implementation for the new bits.
> Anyway, thanks a ton for doing this, Ian. Sorry for dropping the ball
> when we first implemented 420pack.
>
>> diff --git a/src/glsl/link_uniform_initializers.cpp
>> b/src/glsl/link_uniform_initializers.cpp
>> index 9d6977d..9a10350 100644
>> --- a/src/glsl/link_uniform_initializers.cpp
>> +++ b/src/glsl/link_uniform_initializers.cpp
>> @@ -84,7 +84,7 @@ copy_constant_to_storage(union gl_constant_value *storage,
>> }
>>
>> void
>> -set_uniform_binding(void *mem_ctx, gl_shader_program *prog,
>> +set_sampler_binding(void *mem_ctx, gl_shader_program *prog,
>> const char *name, const glsl_type *type, int binding)
>> {
>> struct gl_uniform_storage *const storage =
>> @@ -95,7 +95,7 @@ set_uniform_binding(void *mem_ctx, gl_shader_program *prog,
>> return;
>> }
>>
>> - if (storage->type->is_sampler()) {
>> + {
>> unsigned elements = MAX2(storage->array_elements, 1);
>>
>> /* From section 4.4.4 of the GLSL 4.20 specification:
>> @@ -118,7 +118,24 @@ set_uniform_binding(void *mem_ctx, gl_shader_program
>> *prog,
>> }
>> }
>> }
>> - } else if (storage->block_index != -1) {
>> + }
>> +
>> + storage->initialized = true;
>> +}
>> +
>> +void
>> +set_block_binding(void *mem_ctx, gl_shader_program *prog,
>> + const char *name, const glsl_type *type, int binding)
>> +{
>> + struct gl_uniform_storage *const storage =
>> + get_storage(prog->UniformStorage, prog->NumUserUniformStorage, name);
>> +
>> + if (storage == NULL) {
>> + assert(storage != NULL);
>> + return;
>> + }
>> +
>> + if (storage->block_index != -1) {
>> /* This is a field of a UBO. val is the binding index. */
>> for (int i = 0; i < MESA_SHADER_STAGES; i++) {
>> int stage_index =
>> prog->UniformBlockStageIndex[i][storage->block_index];
>> @@ -134,6 +151,25 @@ set_uniform_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)
>> +{
>> + 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)
>>
signature.asc
Description: OpenPGP digital signature
_______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
