[Mesa-dev] [PATCH 2/2] mesa: Only update enabled VAO gl_vertex_array entries.

[Brian Paul]

On 02/04/2018 02:12 PM, Mathias.Froehlich at gmx.net wrote:
> From: Mathias Fröhlich <mathias.froehlich at web.de>
> 
> Instead of updating all modified gl_vertex_array_object::_VertexArray
> entried just update those ones that are modified and enabled.

"entries"

"those ones that" -> "those that"


> Also release buffer object from the _VertexArray that belong
> to disabled attributes.
> 
> Signed-off-by: Mathias Fröhlich <Mathias.Froehlich at web.de>
> ---
>   src/mesa/main/varray.c |  8 ++++----
>   src/mesa/main/varray.h | 26 +++++++++++++++-----------
>   2 files changed, 19 insertions(+), 15 deletions(-)
> 
> diff --git a/src/mesa/main/varray.c b/src/mesa/main/varray.c
> index 2fd9de630f..a2d1d74798 100644
> --- a/src/mesa/main/varray.c
> +++ b/src/mesa/main/varray.c
> @@ -152,7 +152,7 @@ vertex_attrib_binding(struct gl_context *ctx,
>   
>         array->BufferBindingIndex = bindingIndex;
>   
> -      vao->NewArrays |= array_bit;
> +      vao->NewArrays |= vao->_Enabled & array_bit;
>      }
>   }
>   
> @@ -187,7 +187,7 @@ _mesa_bind_vertex_buffer(struct gl_context *ctx,
>         else
>            vao->VertexAttribBufferMask |= binding->_BoundArrays;
>   
> -      vao->NewArrays |= binding->_BoundArrays;
> +      vao->NewArrays |= vao->_Enabled & binding->_BoundArrays;
>      }
>   }
>   
> @@ -208,7 +208,7 @@ vertex_binding_divisor(struct gl_context *ctx,
>      if (binding->InstanceDivisor != divisor) {
>         FLUSH_VERTICES(ctx, _NEW_ARRAY);
>         binding->InstanceDivisor = divisor;
> -      vao->NewArrays |= binding->_BoundArrays;
> +      vao->NewArrays |= vao->_Enabled & binding->_BoundArrays;
>      }
>   }
>   
> @@ -318,7 +318,7 @@ _mesa_update_array_format(struct gl_context *ctx,
>      array->RelativeOffset = relativeOffset;
>      array->_ElementSize = elementSize;
>   
> -   vao->NewArrays |= VERT_BIT(attrib);
> +   vao->NewArrays |= vao->_Enabled & VERT_BIT(attrib);
>      ctx->NewState |= _NEW_ARRAY;
>   }
>   
> diff --git a/src/mesa/main/varray.h b/src/mesa/main/varray.h
> index fe7eb81631..6e2649830f 100644
> --- a/src/mesa/main/varray.h
> +++ b/src/mesa/main/varray.h
> @@ -58,17 +58,21 @@ _mesa_update_vertex_array(struct gl_context *ctx,
>                             const struct gl_array_attributes *attribs,
>                             const struct gl_vertex_buffer_binding *binding)
>   {
> -   dst->Size = attribs->Size;
> -   dst->Type = attribs->Type;
> -   dst->Format = attribs->Format;
> -   dst->StrideB = binding->Stride;
> -   dst->Ptr = _mesa_vertex_attrib_address(attribs, binding);
> -   dst->Normalized = attribs->Normalized;
> -   dst->Integer = attribs->Integer;
> -   dst->Doubles = attribs->Doubles;
> -   dst->InstanceDivisor = binding->InstanceDivisor;
> -   dst->_ElementSize = attribs->_ElementSize;
> -   _mesa_reference_buffer_object(ctx, &dst->BufferObj, binding->BufferObj);
> +   if (attribs->Enabled) {
> +      dst->Size = attribs->Size;
> +      dst->Type = attribs->Type;
> +      dst->Format = attribs->Format;
> +      dst->StrideB = binding->Stride;
> +      dst->Ptr = _mesa_vertex_attrib_address(attribs, binding);
> +      dst->Normalized = attribs->Normalized;
> +      dst->Integer = attribs->Integer;
> +      dst->Doubles = attribs->Doubles;
> +      dst->InstanceDivisor = binding->InstanceDivisor;
> +      dst->_ElementSize = attribs->_ElementSize;
> +      _mesa_reference_buffer_object(ctx, &dst->BufferObj, binding->BufferObj);
> +   } else {
> +      _mesa_reference_buffer_object(ctx, &dst->BufferObj, NULL);

Maybe also set dst->Ptr = NULL here so that we're more likely to catch 
any bugs where we might reference stale array state.  Maybe set 
dst->Size=0 too.  I'd also put a comment to the effect that the dst 
values should not be consumed by anything when !attribs->Enabled.

Otherwise, for both: Reviewed-by: Brian Paul <brianp at vmware.com>



> +   }
>   }