Hi! Timothy, thanks for your review. Seeing your patch in: http://lists.freedesktop.org/archives/mesa-dev/2015-September/095070.html
The condition in my patch: (var->type->interface_packing != GLSL_INTERFACE_PACKING_PACKED) should also be changed to: (var->get_interface_type()->interface_packing != GLSL_INTERFACE_PACKING_PACKED) Right? On mié, 2015-09-23 at 23:44 +1000, Timothy Arceri wrote: > On Wed, 2015-09-23 at 12:25 +0200, Samuel Iglesias Gonsálvez wrote: > > > > On 21/09/15 13:11, Samuel Iglesias Gonsálvez wrote: > > > On 21/09/15 09:41, Tapani Pälli wrote: > > > > Seems like a nice fix, takes ES3 CTS failures from 116 to 64 on > > > > Haswell > > > > (most failing tests are with ubos). > > > > > > > > Tested-by: Tapani Pälli <tapani.pa...@intel.com> > > > > > > > > > > OK thanks! > > > > > > > This is individual patch not related to just SSBOs, maybe this > > > > could be > > > > pushed separately from the rest? > > > > > > > > > > Yes, this patch can be pushed separately from the rest of patches > > > of the > > > series: we just need an R-b, as usual. > > > > > > We are going to discuss the proper solution with Timothy [0], as he > > > found that we are not covering one case. > > > > > > > Timothy has sent a patch fixing the packed case [0] and he developed > > a > > piglit test for it [1]. > > > > We are going to wait for an R-b of Antía's patch before pushing it. > > I sent a reply to this same email saying almost the same thing but it > seems to have gone missing. > > Anyway I also sent my r-b, this patch looks good to me. > > Reviewed-by: Timothy Arceri <t_arc...@yahoo.com.au> > > > > > > Sam > > > > [0] > > http://lists.freedesktop.org/archives/mesa-dev/2015-September/095070. > > html > > [1] http://lists.freedesktop.org/archives/piglit/2015-September/01724 > > 7.html > > > > > Sam > > > > > > [0] https://bugs.freedesktop.org/show_bug.cgi?id=83508 > > > > > > > > > > // Tapani > > > > > > > > On 09/10/2015 04:36 PM, Iago Toral Quiroga wrote: > > > > > From: Antia Puentes <apuen...@igalia.com> > > > > > > > > > > Commit 1ca25ab (glsl: Do not eliminate 'shared' or 'std140' > > > > > blocks > > > > > or block members) considered as active 'shared' and 'std140' > > > > > uniform > > > > > blocks and uniform block arrays, but did not include the block > > > > > array > > > > > elements. Because of that, it was possible to have an active > > > > > uniform > > > > > block array without any elements marked as used, making the > > > > > assertion > > > > > ((b->num_array_elements > 0) == b->type->is_array()) > > > > > in link_uniform_blocks() fail. > > > > > > > > > > Fixes the following 5 dEQP tests: > > > > > > > > > > * dEQP > > > > > -GLES3.functional.ubo.random.nested_structs_instance_arrays.18 > > > > > * dEQP > > > > > -GLES3.functional.ubo.random.nested_structs_instance_arrays.24 > > > > > * > > > > > dEQP > > > > > -GLES3.functional.ubo.random.nested_structs_arrays_instance_arr > > > > > ays.19 > > > > > * dEQP-GLES3.functional.ubo.random.all_per_block_buffers.49 > > > > > * dEQP-GLES3.functional.ubo.random.all_shared_buffer.36 > > > > > > > > > > Fixes bugzilla: > > > > > https://bugs.freedesktop.org/show_bug.cgi?id=83508 > > > > > --- > > > > > src/glsl/link_uniform_block_active_visitor.cpp | 23 > > > > > +++++++++++++++++++++++ > > > > > 1 file changed, 23 insertions(+) > > > > > > > > > > diff --git a/src/glsl/link_uniform_block_active_visitor.cpp > > > > > b/src/glsl/link_uniform_block_active_visitor.cpp > > > > > index 5102947..fbe79de 100644 > > > > > --- a/src/glsl/link_uniform_block_active_visitor.cpp > > > > > +++ b/src/glsl/link_uniform_block_active_visitor.cpp > > > > > @@ -106,6 +106,22 @@ > > > > > link_uniform_block_active_visitor::visit(ir_variable *var) > > > > > assert(b->num_array_elements == 0); > > > > > assert(b->array_elements == NULL); > > > > > assert(b->type != NULL); > > > > > + assert(!b->type->is_array() || b->has_instance_name); > > > > > + > > > > > + /* For uniform block arrays declared with a shared or > > > > > std140 layout > > > > > + * qualifier, mark all its instances as used. > > > > > + */ > > > > > + if (b->type->is_array() && b->type->length > 0) { > > > > > + b->num_array_elements = b->type->length; > > > > > + b->array_elements = reralloc(this->mem_ctx, > > > > > + b->array_elements, > > > > > + unsigned, > > > > > + b->num_array_elements); > > > > > + > > > > > + for (unsigned i = 0; i < b->num_array_elements; i++) { > > > > > + b->array_elements[i] = i; > > > > > + } > > > > > + } > > > > > > > > > > return visit_continue; > > > > > } > > > > > @@ -147,6 +163,13 @@ > > > > > link_uniform_block_active_visitor::visit_enter(ir_dereference_a > > > > > rray *ir) > > > > > assert((b->num_array_elements == 0) == (b->array_elements > > > > > == NULL)); > > > > > assert(b->type != NULL); > > > > > > > > > > + /* If the block array was declared with a shared or > > > > > + * std140 layout qualifier, all its instances have been > > > > > already > > > > > marked > > > > > + * as used in > > > > > link_uniform_block_active_visitor::visit(ir_variable > > > > > *). > > > > > + */ > > > > > + if (var->type->interface_packing != > > > > > GLSL_INTERFACE_PACKING_PACKED) > > > > > + return visit_continue_with_parent; > > > > > + > > > > > ir_constant *c = ir->array_index->as_constant(); > > > > > > > > > > if (c) { > > > > > > > > > _______________________________________________ > > > > 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 > > > > > _______________________________________________ > > 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 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
