Hi Brian, The change should not change the current behavior. Nevertheless the current behavior as well as past your change the code has a corner case that will not work correctly.
I do have a hand full of unpublished changes here that will fix that potential problem and achieve something similar to what you are doing now with different means. So, if you don't mind not applying that one I that saves me from intrusively rebasing my series here. best Mathias On Thursday, 25 January 2018 00:20:34 CET Brian Paul wrote: > Instead of looping over 32 attributes, just scan the enabled attrib bits. > This involves manipulating the 'enabled' attributes variable depending > on whether we're using legacy GL or a vertex shader. > > We can remove the varying_inputs variable since it's the same as > the new 'enabled' bitfield. > --- > src/mesa/vbo/vbo_exec_draw.c | 37 ++++++++++++++++++++++++++++++++----- > src/mesa/vbo/vbo_save_draw.c | 36 +++++++++++++++++++++++++++++++----- > 2 files changed, 63 insertions(+), 10 deletions(-) > > diff --git a/src/mesa/vbo/vbo_exec_draw.c b/src/mesa/vbo/vbo_exec_draw.c > index 5cea7fe..fcda235 100644 > --- a/src/mesa/vbo/vbo_exec_draw.c > +++ b/src/mesa/vbo/vbo_exec_draw.c > @@ -174,9 +174,9 @@ vbo_exec_bind_arrays(struct gl_context *ctx) > struct vbo_context *vbo = vbo_context(ctx); > struct vbo_exec_context *exec = &vbo->exec; > struct gl_vertex_array *arrays = exec->vtx.arrays; > - const GLubyte *map; > + const GLubyte *map; /** map from VERT_ATTRIB_x to VBO_ATTRIB_x */ > GLuint attr; > - GLbitfield varying_inputs = 0x0; > + GLbitfield64 enabled = exec->vtx.enabled; > bool swap_pos = false; > > /* Install the default (ie Current) attributes first */ > @@ -194,6 +194,10 @@ vbo_exec_bind_arrays(struct gl_context *ctx) > &vbo->currval[VBO_ATTRIB_MAT_FRONT_AMBIENT+attr]; > } > map = vbo->map_vp_none; > + > + /* shift material attrib flags into generic attribute positions */ > + enabled = (enabled & VBO_ATTRIBS_LEGACY) > + | ((enabled & VBO_ATTRIBS_MATERIALS) >> VBO_MATERIAL_SHIFT); > break; > case VP_SHADER: > for (attr = 0; attr < VERT_ATTRIB_GENERIC_MAX; attr++) { > @@ -203,6 +207,9 @@ vbo_exec_bind_arrays(struct gl_context *ctx) > } > map = vbo->map_vp_arb; > > + /* per-vertex materials are to be ignored when using a VS */ > + enabled &= ~VBO_ATTRIBS_MATERIALS; > + > /* check if VERT_ATTRIB_POS is not read but VERT_BIT_GENERIC0 is read. > * In that case we effectively need to route the data from > * glVertexAttrib(0, val) calls to feed into the GENERIC0 input. > @@ -218,13 +225,34 @@ vbo_exec_bind_arrays(struct gl_context *ctx) > exec->vtx.attrtype[VERT_ATTRIB_GENERIC0] = exec->vtx.attrtype[0]; > exec->vtx.attrptr[VERT_ATTRIB_GENERIC0] = exec->vtx.attrptr[0]; > exec->vtx.attrsz[0] = 0; > + enabled &= ~BITFIELD64_BIT(VBO_ATTRIB_POS); > + enabled |= BITFIELD64_BIT(VBO_ATTRIB_GENERIC0); > } > break; > default: > unreachable("Bad vertex program mode"); > } > > - for (attr = 0; attr < VERT_ATTRIB_MAX ; attr++) { > +#ifdef DEBUG > + /* verify the enabled bitmask is consistent with attribute size info */ > + { > + GLbitfield64 used = 0x0; > + for (attr = 0; attr < VERT_ATTRIB_MAX; attr++) { > + const GLuint src = map[attr]; > + if (exec->vtx.attrsz[src]) { > + assert(enabled & VERT_BIT(attr)); > + used |= VERT_BIT(attr); > + } else { > + assert((enabled & VERT_BIT(attr)) == 0); > + } > + } > + assert(used == enabled); > + } > +#endif > + > + GLbitfield64 enabled_scan = enabled; > + while (enabled_scan) { > + const int attr = u_bit_scan64(&enabled_scan); > const GLuint src = map[attr]; > > if (exec->vtx.attrsz[src]) { > @@ -258,7 +286,6 @@ vbo_exec_bind_arrays(struct gl_context *ctx) > &arrays[attr].BufferObj, > exec->vtx.bufferobj); > > - varying_inputs |= VERT_BIT(attr); > } > } > > @@ -272,7 +299,7 @@ vbo_exec_bind_arrays(struct gl_context *ctx) > exec->vtx.attrsz[VERT_ATTRIB_GENERIC0] = 0; > } > > - _mesa_set_varying_vp_inputs(ctx, varying_inputs); > + _mesa_set_varying_vp_inputs(ctx, enabled); > ctx->NewDriverState |= ctx->DriverFlags.NewArray; > } > > diff --git a/src/mesa/vbo/vbo_save_draw.c b/src/mesa/vbo/vbo_save_draw.c > index c86efcb..5f5dcbe 100644 > --- a/src/mesa/vbo/vbo_save_draw.c > +++ b/src/mesa/vbo/vbo_save_draw.c > @@ -138,11 +138,11 @@ bind_vertex_list(struct gl_context *ctx, > struct vbo_save_context *save = &vbo->save; > struct gl_vertex_array *arrays = save->arrays; > GLuint buffer_offset = node->buffer_offset; > - const GLubyte *map; > + const GLubyte *map; /** map from VERT_ATTRIB_x to VBO_ATTRIB_x */ > GLuint attr; > GLubyte node_attrsz[VBO_ATTRIB_MAX]; /* copy of node->attrsz[] */ > GLenum node_attrtype[VBO_ATTRIB_MAX]; /* copy of node->attrtype[] */ > - GLbitfield varying_inputs = 0x0; > + GLbitfield64 enabled = node->enabled; > > memcpy(node_attrsz, node->attrsz, sizeof(node->attrsz)); > memcpy(node_attrtype, node->attrtype, sizeof(node->attrtype)); > @@ -173,6 +173,10 @@ bind_vertex_list(struct gl_context *ctx, > &vbo->currval[VBO_ATTRIB_MAT_FRONT_AMBIENT+attr]; > } > map = vbo->map_vp_none; > + > + /* shift material attrib flags into generic attribute positions */ > + enabled = (enabled & VBO_ATTRIBS_LEGACY) > + | ((enabled & VBO_ATTRIBS_MATERIALS) >> VBO_MATERIAL_SHIFT); > break; > case VP_SHADER: > for (attr = 0; attr < VERT_ATTRIB_GENERIC_MAX; attr++) { > @@ -181,6 +185,9 @@ bind_vertex_list(struct gl_context *ctx, > } > map = vbo->map_vp_arb; > > + /* per-vertex materials are to be ignored when using a VS */ > + enabled &= ~VBO_ATTRIBS_MATERIALS; > + > /* check if VERT_ATTRIB_POS is not read but VERT_BIT_GENERIC0 is read. > * In that case we effectively need to route the data from > * glVertexAttrib(0, val) calls to feed into the GENERIC0 input. > @@ -193,13 +200,33 @@ bind_vertex_list(struct gl_context *ctx, > node_attrsz[VERT_ATTRIB_GENERIC0] = node_attrsz[0]; > node_attrtype[VERT_ATTRIB_GENERIC0] = node_attrtype[0]; > node_attrsz[0] = 0; > + enabled = (enabled & ~VERT_BIT_POS) | VERT_BIT_GENERIC0; > } > break; > default: > unreachable("Bad vertex program mode"); > } > > - for (attr = 0; attr < VERT_ATTRIB_MAX; attr++) { > +#ifdef DEBUG > + /* verify the enabled bitmask is consistent with attribute size info */ > + { > + GLbitfield64 used = 0x0; > + for (attr = 0; attr < VERT_ATTRIB_MAX; attr++) { > + const GLuint src = map[attr]; > + if (node_attrsz[src]) { > + assert(enabled & VERT_BIT(attr)); > + used |= VERT_BIT(attr); > + } else { > + assert((enabled & VERT_BIT(attr)) == 0); > + } > + } > + assert(used == enabled); > + } > +#endif > + > + GLbitfield64 enabled_scan = enabled; > + while (enabled_scan) { > + const int attr = u_bit_scan64(&enabled_scan); > const GLuint src = map[attr]; > > if (node_attrsz[src]) { > @@ -222,11 +249,10 @@ bind_vertex_list(struct gl_context *ctx, > assert(array->BufferObj->Name); > > buffer_offset += node_attrsz[src] * sizeof(GLfloat); > - varying_inputs |= VERT_BIT(attr); > } > } > > - _mesa_set_varying_vp_inputs(ctx, varying_inputs); > + _mesa_set_varying_vp_inputs(ctx, enabled); > ctx->NewDriverState |= ctx->DriverFlags.NewArray; > } > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev