ok, so either (or both) of: --------------- diff --git a/src/mesa/vbo/vbo_exec_api.c b/src/mesa/vbo/vbo_exec_api.c index e02bb90..a7ae50b 100644 --- a/src/mesa/vbo/vbo_exec_api.c +++ b/src/mesa/vbo/vbo_exec_api.c @@ -1278,9 +1278,11 @@ void vbo_exec_FlushVertices( struct gl_context *ctx, GLuint flags )
static void reset_attrfv( struct vbo_exec_context *exec ) { + /* counter-part to trick in vbo_exec_bind_arrays().. */ + if (exec->vtx.active_sz[0]) + exec->vtx.enabled |= (1 << VERT_ATTRIB_POS); while (exec->vtx.enabled) { const int i = u_bit_scan64(&exec->vtx.enabled); - assert(exec->vtx.attrsz[i]); exec->vtx.attrsz[i] = 0; exec->vtx.attrtype[i] = GL_FLOAT; --------------- or --------------- diff --git a/src/mesa/vbo/vbo_exec_draw.c b/src/mesa/vbo/vbo_exec_draw.c index 8d1b2c0..cbac3be 100644 --- a/src/mesa/vbo/vbo_exec_draw.c +++ b/src/mesa/vbo/vbo_exec_draw.c @@ -214,6 +214,7 @@ vbo_exec_bind_arrays( struct gl_context *ctx ) exec->vtx.attrsz[VERT_ATTRIB_GENERIC0] = exec->vtx.attrsz[0]; exec->vtx.attrptr[VERT_ATTRIB_GENERIC0] = exec->vtx.attrptr[0]; exec->vtx.attrsz[0] = 0; + exec->vtx.active_sz[0] = 0; exec->vtx.enabled &= (~BITFIELD64_BIT(VBO_ATTRIB_POS)); exec->vtx.enabled |= BITFIELD64_BIT(VBO_ATTRIB_GENERIC0); } --------------- will fix it... BR, -R On Tue, Jul 5, 2016 at 2:56 PM, Rob Clark <robdcl...@gmail.com> wrote: > So, this is a bit sad, but this breaks things for 0ad.. and maybe > others. I have an api-trace: > > https://people.freedesktop.org/~robclark/0ad-cycladic-archipelago.trace.xz > > The problem is the interaction with the VERT_ATTRIB_POS / > VERT_ATTRIB_GENERIC0 switcharoo in vbo_exec_bind_arrays(), although > not entirely sure what the best thing to do is. At any rate, it > leaves a stale value in exec->vtx.active_sz[0], which results that > vbo_exec_fixup_vertex() never happens.. > > BR, > -R > > On Tue, Jun 14, 2016 at 1:00 AM, <mathias.froehl...@gmx.net> wrote: >> From: Mathias Fröhlich <mathias.froehl...@web.de> >> >> The use of a bitmask makes functions iterating only active >> attributes less visible in profiles. >> >> v2: Use _mesa_bit_scan{,64} instead of open coding. >> v3: Use u_bit_scan{,64} instead of _mesa_bit_scan{,64}. >> >> Reviewed-by: Brian Paul <bri...@vmware.com> >> Signed-off-by: Mathias Fröhlich <mathias.froehl...@web.de> >> --- >> src/mesa/vbo/vbo_exec.h | 1 + >> src/mesa/vbo/vbo_exec_api.c | 146 >> ++++++++++++++++++++++--------------------- >> src/mesa/vbo/vbo_exec_draw.c | 2 + >> 3 files changed, 79 insertions(+), 70 deletions(-) >> >> diff --git a/src/mesa/vbo/vbo_exec.h b/src/mesa/vbo/vbo_exec.h >> index 27bff4a..5e20cf6 100644 >> --- a/src/mesa/vbo/vbo_exec.h >> +++ b/src/mesa/vbo/vbo_exec.h >> @@ -101,6 +101,7 @@ struct vbo_exec_context >> GLuint max_vert; /**< Max number of vertices allowed in buffer */ >> struct vbo_exec_copied_vtx copied; >> >> + GLbitfield64 enabled; /**< mask of enabled vbo arrays. */ >> GLubyte attrsz[VBO_ATTRIB_MAX]; /**< nr. of attrib components >> (1..4) */ >> GLenum attrtype[VBO_ATTRIB_MAX]; /**< GL_FLOAT, GL_DOUBLE, GL_INT, >> etc */ >> GLubyte active_sz[VBO_ATTRIB_MAX]; /**< attrib size (nr. 32-bit >> words) */ >> diff --git a/src/mesa/vbo/vbo_exec_api.c b/src/mesa/vbo/vbo_exec_api.c >> index 7534599..e02bb90 100644 >> --- a/src/mesa/vbo/vbo_exec_api.c >> +++ b/src/mesa/vbo/vbo_exec_api.c >> @@ -42,6 +42,7 @@ USE OR OTHER DEALINGS IN THE SOFTWARE. >> #include "main/api_arrayelt.h" >> #include "main/api_validate.h" >> #include "main/dispatch.h" >> +#include "util/bitscan.h" >> >> #include "vbo_context.h" >> #include "vbo_noop.h" >> @@ -167,54 +168,56 @@ static void vbo_exec_copy_to_current( struct >> vbo_exec_context *exec ) >> { >> struct gl_context *ctx = exec->ctx; >> struct vbo_context *vbo = vbo_context(ctx); >> - GLuint i; >> + GLbitfield64 enabled = exec->vtx.enabled & >> (~BITFIELD64_BIT(VBO_ATTRIB_POS)); >> >> - for (i = VBO_ATTRIB_POS+1 ; i < VBO_ATTRIB_MAX ; i++) { >> - if (exec->vtx.attrsz[i]) { >> - /* Note: the exec->vtx.current[i] pointers point into the >> - * ctx->Current.Attrib and ctx->Light.Material.Attrib arrays. >> - */ >> - GLfloat *current = (GLfloat *)vbo->currval[i].Ptr; >> - fi_type tmp[8]; /* space for doubles */ >> - int dmul = exec->vtx.attrtype[i] == GL_DOUBLE ? 2 : 1; >> - >> - if (exec->vtx.attrtype[i] == GL_DOUBLE) { >> - memset(tmp, 0, sizeof(tmp)); >> - memcpy(tmp, exec->vtx.attrptr[i], exec->vtx.attrsz[i] * >> sizeof(GLfloat)); >> - } else { >> - COPY_CLEAN_4V_TYPE_AS_UNION(tmp, >> - exec->vtx.attrsz[i], >> - exec->vtx.attrptr[i], >> - exec->vtx.attrtype[i]); >> - } >> + while (enabled) { >> + const int i = u_bit_scan64(&enabled); >> + >> + /* Note: the exec->vtx.current[i] pointers point into the >> + * ctx->Current.Attrib and ctx->Light.Material.Attrib arrays. >> + */ >> + GLfloat *current = (GLfloat *)vbo->currval[i].Ptr; >> + fi_type tmp[8]; /* space for doubles */ >> + int dmul = exec->vtx.attrtype[i] == GL_DOUBLE ? 2 : 1; >> + >> + assert(exec->vtx.attrsz[i]); >> + >> + if (exec->vtx.attrtype[i] == GL_DOUBLE) { >> + memset(tmp, 0, sizeof(tmp)); >> + memcpy(tmp, exec->vtx.attrptr[i], exec->vtx.attrsz[i] * >> sizeof(GLfloat)); >> + } else { >> + COPY_CLEAN_4V_TYPE_AS_UNION(tmp, >> + exec->vtx.attrsz[i], >> + exec->vtx.attrptr[i], >> + exec->vtx.attrtype[i]); >> + } >> >> - if (exec->vtx.attrtype[i] != vbo->currval[i].Type || >> - memcmp(current, tmp, 4 * sizeof(GLfloat) * dmul) != 0) { >> - memcpy(current, tmp, 4 * sizeof(GLfloat) * dmul); >> + if (exec->vtx.attrtype[i] != vbo->currval[i].Type || >> + memcmp(current, tmp, 4 * sizeof(GLfloat) * dmul) != 0) { >> + memcpy(current, tmp, 4 * sizeof(GLfloat) * dmul); >> >> - /* Given that we explicitly state size here, there is no need >> - * for the COPY_CLEAN above, could just copy 16 bytes and be >> - * done. The only problem is when Mesa accesses ctx->Current >> - * directly. >> - */ >> - /* Size here is in components - not bytes */ >> - vbo->currval[i].Size = exec->vtx.attrsz[i] / dmul; >> - vbo->currval[i]._ElementSize = vbo->currval[i].Size * >> sizeof(GLfloat) * dmul; >> - vbo->currval[i].Type = exec->vtx.attrtype[i]; >> - vbo->currval[i].Integer = >> - vbo_attrtype_to_integer_flag(exec->vtx.attrtype[i]); >> - vbo->currval[i].Doubles = >> - vbo_attrtype_to_double_flag(exec->vtx.attrtype[i]); >> - >> - /* This triggers rather too much recalculation of Mesa state >> - * that doesn't get used (eg light positions). >> - */ >> - if (i >= VBO_ATTRIB_MAT_FRONT_AMBIENT && >> - i <= VBO_ATTRIB_MAT_BACK_INDEXES) >> - ctx->NewState |= _NEW_LIGHT; >> - >> - ctx->NewState |= _NEW_CURRENT_ATTRIB; >> - } >> + /* Given that we explicitly state size here, there is no need >> + * for the COPY_CLEAN above, could just copy 16 bytes and be >> + * done. The only problem is when Mesa accesses ctx->Current >> + * directly. >> + */ >> + /* Size here is in components - not bytes */ >> + vbo->currval[i].Size = exec->vtx.attrsz[i] / dmul; >> + vbo->currval[i]._ElementSize = vbo->currval[i].Size * >> sizeof(GLfloat) * dmul; >> + vbo->currval[i].Type = exec->vtx.attrtype[i]; >> + vbo->currval[i].Integer = >> + vbo_attrtype_to_integer_flag(exec->vtx.attrtype[i]); >> + vbo->currval[i].Doubles = >> + vbo_attrtype_to_double_flag(exec->vtx.attrtype[i]); >> + >> + /* This triggers rather too much recalculation of Mesa state >> + * that doesn't get used (eg light positions). >> + */ >> + if (i >= VBO_ATTRIB_MAT_FRONT_AMBIENT && >> + i <= VBO_ATTRIB_MAT_BACK_INDEXES) >> + ctx->NewState |= _NEW_LIGHT; >> + >> + ctx->NewState |= _NEW_CURRENT_ATTRIB; >> } >> } >> >> @@ -311,6 +314,7 @@ vbo_exec_wrap_upgrade_vertex(struct vbo_exec_context >> *exec, >> exec->vtx.max_vert = vbo_compute_max_verts(exec); >> exec->vtx.vert_count = 0; >> exec->vtx.buffer_ptr = exec->vtx.buffer_map; >> + exec->vtx.enabled |= BITFIELD64_BIT(attr); >> >> if (unlikely(oldSize)) { >> /* Size changed, recalculate all the attrptr[] values >> @@ -345,34 +349,34 @@ vbo_exec_wrap_upgrade_vertex(struct vbo_exec_context >> *exec, >> if (unlikely(exec->vtx.copied.nr)) { >> fi_type *data = exec->vtx.copied.buffer; >> fi_type *dest = exec->vtx.buffer_ptr; >> - GLuint j; >> >> assert(exec->vtx.buffer_ptr == exec->vtx.buffer_map); >> >> for (i = 0 ; i < exec->vtx.copied.nr ; i++) { >> - for (j = 0 ; j < VBO_ATTRIB_MAX ; j++) { >> + GLbitfield64 enabled = exec->vtx.enabled; >> + while (enabled) { >> + const int j = u_bit_scan64(&enabled); >> GLuint sz = exec->vtx.attrsz[j]; >> - >> - if (sz) { >> - GLint old_offset = old_attrptr[j] - exec->vtx.vertex; >> - GLint new_offset = exec->vtx.attrptr[j] - exec->vtx.vertex; >> - >> - if (j == attr) { >> - if (oldSize) { >> - fi_type tmp[4]; >> - COPY_CLEAN_4V_TYPE_AS_UNION(tmp, oldSize, >> - data + old_offset, >> - exec->vtx.attrtype[j]); >> - COPY_SZ_4V(dest + new_offset, newSize, tmp); >> - } else { >> - fi_type *current = (fi_type *)vbo->currval[j].Ptr; >> - COPY_SZ_4V(dest + new_offset, sz, current); >> - } >> - } >> - else { >> - COPY_SZ_4V(dest + new_offset, sz, data + old_offset); >> - } >> - } >> + GLint old_offset = old_attrptr[j] - exec->vtx.vertex; >> + GLint new_offset = exec->vtx.attrptr[j] - exec->vtx.vertex; >> + >> + assert(sz); >> + >> + if (j == attr) { >> + if (oldSize) { >> + fi_type tmp[4]; >> + COPY_CLEAN_4V_TYPE_AS_UNION(tmp, oldSize, >> + data + old_offset, >> + exec->vtx.attrtype[j]); >> + COPY_SZ_4V(dest + new_offset, newSize, tmp); >> + } else { >> + fi_type *current = (fi_type *)vbo->currval[j].Ptr; >> + COPY_SZ_4V(dest + new_offset, sz, current); >> + } >> + } >> + else { >> + COPY_SZ_4V(dest + new_offset, sz, data + old_offset); >> + } >> } >> >> data += old_vtx_size; >> @@ -1145,6 +1149,7 @@ void vbo_exec_vtx_init( struct vbo_exec_context *exec ) >> vbo_exec_vtxfmt_init( exec ); >> _mesa_noop_vtxfmt_init(&exec->vtxfmt_noop); >> >> + exec->vtx.enabled = 0; >> for (i = 0 ; i < VBO_ATTRIB_MAX ; i++) { >> assert(i < ARRAY_SIZE(exec->vtx.attrsz)); >> exec->vtx.attrsz[i] = 0; >> @@ -1273,9 +1278,10 @@ void vbo_exec_FlushVertices( struct gl_context *ctx, >> GLuint flags ) >> >> static void reset_attrfv( struct vbo_exec_context *exec ) >> { >> - GLuint i; >> + while (exec->vtx.enabled) { >> + const int i = u_bit_scan64(&exec->vtx.enabled); >> + assert(exec->vtx.attrsz[i]); >> >> - for (i = 0 ; i < VBO_ATTRIB_MAX ; i++) { >> exec->vtx.attrsz[i] = 0; >> exec->vtx.attrtype[i] = GL_FLOAT; >> exec->vtx.active_sz[i] = 0; >> diff --git a/src/mesa/vbo/vbo_exec_draw.c b/src/mesa/vbo/vbo_exec_draw.c >> index 0d42618..8d1b2c0 100644 >> --- a/src/mesa/vbo/vbo_exec_draw.c >> +++ b/src/mesa/vbo/vbo_exec_draw.c >> @@ -214,6 +214,8 @@ vbo_exec_bind_arrays( struct gl_context *ctx ) >> exec->vtx.attrsz[VERT_ATTRIB_GENERIC0] = exec->vtx.attrsz[0]; >> exec->vtx.attrptr[VERT_ATTRIB_GENERIC0] = exec->vtx.attrptr[0]; >> exec->vtx.attrsz[0] = 0; >> + exec->vtx.enabled &= (~BITFIELD64_BIT(VBO_ATTRIB_POS)); >> + exec->vtx.enabled |= BITFIELD64_BIT(VBO_ATTRIB_GENERIC0); >> } >> break; >> default: >> -- >> 2.5.5 >> >> _______________________________________________ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev