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

Brian Paul brianp at vmware.com
Mon Jan 29 18:24:15 UTC 2018


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;
>>   }
>>   
>>
> 
> 
> 
> 



More information about the mesa-dev mailing list