On lun, 2016-10-31 at 14:51 -0700, Kenneth Graunke wrote: > On Monday, October 31, 2016 6:22:43 PM PDT Antia Puentes wrote: > > > > The emission of vertex attributes corresponding to dvec3 and dvec4 > > vertex shader input variables was not correct when the <size> > > passed > > to the VertexAttribL* commands was <= 2. > > > > This was because we were using the vertex array size when emitting > > vertices > > to decide if we uploaded a 64-bit floating point attribute as 1 > > slot (128-bits) > > for sizes 1 and 2, or 2 slots (256-bits) for sizes 3 and 4. This > > caused problems > > when mapping the input variables to registers because, for deciding > > which > > registers contain the values uploaded for a certain variable, we > > use the size > > and type given to the variable in the shader, so we will be > > assigning 256-bits > > to dvec3/4 variables, even if we only uploaded 128-bits for them, > > which happened > > when the vertex array size was <= 2. > > > > The patch uses the shader information to only emit as 128-bits > > those 64-bit floating > > point variables that were declared as double or dvec2 in the vertex > > shader. Dvec3 and > > dvec4 variables will be always uploaded as 256-bits, independently > > of the <size> given > > to the VertexAttribL* command. > > > > From the ARB_vertex_attrib_64bit specification: > > > > "For the 64-bit double precision types listed in Table X.1, no > > default > > attribute values are provided if the values of the vertex > > attribute variable > > are specified with fewer components than required for the > > attribute > > variable. For example, the fourth component of a variable of > > type dvec4 > > will be undefined if specified using VertexAttribL3dv or using > > a vertex > > array specified with VertexAttribLPointer and a size of three." > > > > We are filling these unspecified components with zeros, which > > coincidentally is > > also what the GL44-CTS.vertex_attrib_binding.basic-inputL-case1 > > expects. > > > > Fixes: GL44-CTS.vertex_attrib_binding.basic-inputL-case1 test > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97287 > > --- > > src/mesa/drivers/dri/i965/brw_compiler.h | 1 + > > src/mesa/drivers/dri/i965/brw_context.h | 2 +- > > src/mesa/drivers/dri/i965/brw_draw_upload.c | 4 +++- > > src/mesa/drivers/dri/i965/brw_vs.c | 1 + > > src/mesa/drivers/dri/i965/gen8_draw_upload.c | 35 ++++++++++++-- > > -------------- > > 5 files changed, 21 insertions(+), 22 deletions(-) > > > > diff --git a/src/mesa/drivers/dri/i965/brw_compiler.h > > b/src/mesa/drivers/dri/i965/brw_compiler.h > > index 819c7d6..c2400f9 100644 > > --- a/src/mesa/drivers/dri/i965/brw_compiler.h > > +++ b/src/mesa/drivers/dri/i965/brw_compiler.h > > @@ -641,6 +641,7 @@ struct brw_vs_prog_data { > > struct brw_vue_prog_data base; > > > > GLbitfield64 inputs_read; > > + GLbitfield64 double_inputs_read; > > > > unsigned nr_attributes; > > unsigned nr_attribute_slots; > > diff --git a/src/mesa/drivers/dri/i965/brw_context.h > > b/src/mesa/drivers/dri/i965/brw_context.h > > index 308ba99..310372a 100644 > > --- a/src/mesa/drivers/dri/i965/brw_context.h > > +++ b/src/mesa/drivers/dri/i965/brw_context.h > > @@ -535,7 +535,7 @@ struct brw_vertex_element { > > const struct gl_vertex_array *glarray; > > > > int buffer; > > - > > + bool is_dual_slot; > > /** Offset of the first element within the buffer object */ > > unsigned int offset; > > }; > > diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c > > b/src/mesa/drivers/dri/i965/brw_draw_upload.c > > index da13e7a..19c8065 100644 > > --- a/src/mesa/drivers/dri/i965/brw_draw_upload.c > > +++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c > > @@ -472,7 +472,9 @@ brw_prepare_vertices(struct brw_context *brw) > > while (vs_inputs) { > > GLuint index = ffsll(vs_inputs) - 1; > > struct brw_vertex_element *input = &brw->vb.inputs[index]; > > - > > + input->is_dual_slot = > > + (brw->gen >= 8 && _mesa_bitcount_64(vs_prog_data- > > >double_inputs_read & > > + BITFIELD64_BIT(index) > > )); > Bitcount of a single bit? Couldn't you do: > > input->is_dual_slot = brw->gen >= 8 && > (vs_prog_data->double_inputs_read & BITFIELD64_BIT(index)) > != 0;
Will do. Thanks for the review! > With that fixed, this is: > Reviewed-by: Kenneth Graunke <kenn...@whitecape.org> > > Thanks for tracking this down and fixing it! _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
