From 0eee93ddf4135de43498d5b6d1e4d6595f420561 Mon Sep 17 00:00:00 2001 From: Ian Romanick Date: Wed, 2 Apr 2014 18:58:54 -0700 Subject: [PATCH] glsl: Work-in-progress fix for layout(binding=N) Information about the binding was being properly communicated from the front-end compiler to the linker. NOTE: With this patch, an UBO with an instance name will trigger this assertion: ../../src/glsl/link_uniform_initializers.cpp:100: void linker::set_uniform_binding(void*, gl_shader_program*, const char*, const glsl_type*, int): Assertion `storage != __null' failed. I know what's causing it, but I haven't decided how to fix it yet. --- src/glsl/ast_to_hir.cpp | 7 +++++++ src/glsl/link_uniform_block_active_visitor.cpp | 8 ++++++++ src/glsl/link_uniform_block_active_visitor.h | 3 +++ src/glsl/link_uniform_blocks.cpp | 24 ++++++++++++++++++++++-- src/glsl/link_uniform_initializers.cpp | 11 ++++++++++- 5 files changed, 50 insertions(+), 3 deletions(-) diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp index 8f6e901..02aedb5 100644 --- a/src/glsl/ast_to_hir.cpp +++ b/src/glsl/ast_to_hir.cpp @@ -5267,6 +5267,13 @@ ast_interface_block::hir(exec_list *instructions, earlier->reinit_interface_type(block_type); delete var; } else { + /* Propagate the "binding" keyword into this UBO's fields; + * the UBO declaration itself doesn't get an ir_variable unless it + * has an instance name. This is ugly. + */ + var->data.explicit_binding = this->layout.flags.q.explicit_binding; + var->data.binding = this->layout.binding; + state->symbols->add_variable(var); instructions->push_tail(var); } diff --git a/src/glsl/link_uniform_block_active_visitor.cpp b/src/glsl/link_uniform_block_active_visitor.cpp index f2f46a2..d19ce20 100644 --- a/src/glsl/link_uniform_block_active_visitor.cpp +++ b/src/glsl/link_uniform_block_active_visitor.cpp @@ -46,6 +46,14 @@ process_block(void *mem_ctx, struct hash_table *ht, ir_variable *var) b->type = block_type; b->has_instance_name = var->is_interface_instance(); + if (var->data.explicit_binding) { + b->has_binding = true; + b->binding = var->data.binding; + } else { + b->has_binding = false; + b->binding = 0; + } + _mesa_hash_table_insert(ht, h, var->get_interface_type()->name, (void *) b); return b; diff --git a/src/glsl/link_uniform_block_active_visitor.h b/src/glsl/link_uniform_block_active_visitor.h index fba628a..d76dbca 100644 --- a/src/glsl/link_uniform_block_active_visitor.h +++ b/src/glsl/link_uniform_block_active_visitor.h @@ -36,7 +36,10 @@ struct link_uniform_block_active { unsigned *array_elements; unsigned num_array_elements; + unsigned binding; + bool has_instance_name; + bool has_binding; }; class link_uniform_block_active_visitor : public ir_hierarchical_visitor { diff --git a/src/glsl/link_uniform_blocks.cpp b/src/glsl/link_uniform_blocks.cpp index 72d6c53..7fd74df 100644 --- a/src/glsl/link_uniform_blocks.cpp +++ b/src/glsl/link_uniform_blocks.cpp @@ -251,7 +251,21 @@ link_uniform_blocks(void *mem_ctx, blocks[i].Name = ralloc_asprintf(blocks, "%s[%u]", name, b->array_elements[j]); blocks[i].Uniforms = &variables[parcel.index]; - blocks[i].Binding = 0; + + /* The GL_ARB_shading_language_420pack spec says: + * + * "If the binding identifier is used with a uniform block + * instanced as an array then the first element of the array + * takes the specified block binding and each subsequent + * element takes the next consecutive uniform block binding + * point." + */ + if (b->has_binding) { + blocks[i].Binding = b->binding + j; + } else { + blocks[i].Binding = 0; + } + blocks[i].UniformBufferSize = 0; blocks[i]._Packing = gl_uniform_block_packing(block_type->interface_packing); @@ -269,7 +283,13 @@ link_uniform_blocks(void *mem_ctx, } else { blocks[i].Name = ralloc_strdup(blocks, block_type->name); blocks[i].Uniforms = &variables[parcel.index]; - blocks[i].Binding = 0; + + if (b->has_binding) { + blocks[i].Binding = b->binding; + } else { + blocks[i].Binding = 0; + } + blocks[i].UniformBufferSize = 0; blocks[i]._Packing = gl_uniform_block_packing(block_type->interface_packing); diff --git a/src/glsl/link_uniform_initializers.cpp b/src/glsl/link_uniform_initializers.cpp index 9d6977d..9a7c1c4 100644 --- a/src/glsl/link_uniform_initializers.cpp +++ b/src/glsl/link_uniform_initializers.cpp @@ -232,7 +232,16 @@ 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, + const char *name = var->name; + + if (var->is_interface_instance()) { + if (var->type->is_array()) + name = var->type->fields.array->name; + else + name = var->type->name; + } + + linker::set_uniform_binding(mem_ctx, prog, name, var->type, var->data.binding); } else if (var->constant_value) { linker::set_uniform_initializer(mem_ctx, prog, var->name, -- 1.8.1.4