Attachment 96816 Details for Bug 76323 – Patch that fixes part of the problem

[patch] Patch that fixes part of the problem

0001-glsl-Work-in-progress-fix-for-layout-binding-N.patch (text/plain), 5.65 KB, created by Ian Romanick on 2014-04-03 02:04:01 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ian Romanick

Created: 2014-04-03 02:04:01 UTC

Size: 5.65 KB

patch

obsolete

>From 0eee93ddf4135de43498d5b6d1e4d6595f420561 Mon Sep 17 00:00:00 2001
>From: Ian Romanick <ian.d.romanick@intel.com>
>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
>

Actions: View | Splinter Review

Attachments on bug 76323: 96005 | 96816

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.