Re: [Mesa-dev] [PATCH 10/11] vbo: optimize loops in bind_vertex_list(), vbo_exec_bind_arrays()

Brian Paul Mon, 29 Jan 2018 10:24:45 -0800

On 01/27/2018 08:01 AM, Mathias Fröhlich wrote:

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.


Can you elaborate?

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.


Sounds good.

-Brian


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

Re: [Mesa-dev] [PATCH 10/11] vbo: optimize loops in bind_vertex_list(), vbo_exec_bind_arrays()

Reply via email to