[Mesa-dev] [PATCH 28/29] vbo: Use a bitmask to track the active arrays in vbo_exec*.
Rob Clark
robdclark at gmail.com
Tue Jul 5 19:12:15 UTC 2016
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 <robdclark at 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.Froehlich at gmx.net> wrote:
>> From: Mathias Fröhlich <mathias.froehlich at 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 <brianp at vmware.com>
>> Signed-off-by: Mathias Fröhlich <Mathias.Froehlich at 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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list