On 01/23/2013 07:53 PM, Paul Berry wrote:
On 22 January 2013 00:52, Ian Romanick <i...@freedesktop.org <mailto:i...@freedesktop.org>> wrote:From: Ian Romanick <ian.d.roman...@intel.com <mailto:ian.d.roman...@intel.com>> Signed-off-by: Ian Romanick <ian.d.roman...@intel.com <mailto:ian.d.roman...@intel.com>> --- src/glsl/ast_to_hir.cpp | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp index d485bc8..c922a84 100644 --- a/src/glsl/ast_to_hir.cpp +++ b/src/glsl/ast_to_hir.cpp @@ -4259,9 +4259,20 @@ ast_uniform_block::hir(exec_list *instructions, * field selector ( . ) operator (analogously to structures)." */ if (this->instance_name) { - ir_variable *var = new(state) ir_variable(block_type, - this->instance_name, - ir_var_uniform); + ir_variable *var; + + if (this->array_size != NULL) { + const glsl_type *block_array_type = + process_array_type(&loc, block_type, this->array_size, state); + + var = new(state) ir_variable(block_array_type, + this->instance_name, + ir_var_uniform); + } else { + var = new(state) ir_variable(block_type, + this->instance_name, + ir_var_uniform); + } var->interface_type = block_type; state->symbols->add_variable(var); -- 1.7.11.7 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org <mailto:mesa-dev@lists.freedesktop.org> http://lists.freedesktop.org/mailman/listinfo/mesa-dev Immediately below this hunk there's an else branch, to deal with the case where the uniform block doesn't have an instance name. Why don't we need to add similar array-handling logic to the else branch? I'm guessing that the grammar prevents instance blocks without names from being arrays, but since I'm not too familiar with UBO's I'm not very certain about it.
Yes, that's the case. It makes sense, too, because blocks without instance names just have variables at global scope...so there's nothing sensible to put an array subscript on.
If my guess is right, it would be nice to put an explanatory comment in the else branch, and maybe an "assert(this->array_size == NULL);" just to drive the point home.
Certainly never hurts.
But I won't be a stickler about it. With or without my suggested change, this patch is: Reviewed-by: Paul Berry <stereotype...@gmail.com <mailto:stereotype...@gmail.com>> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
