[Mesa-dev] [PATCH 4/4] mesa: tidy up left over APPLE_vertex_array_object semantics
Timothy Arceri
tarceri at itsqueeze.com
Mon Apr 24 10:28:47 UTC 2017
On 24/04/17 20:13, Nicolai Hähnle wrote:
> On 24.04.2017 07:28, Timothy Arceri wrote:
>> ---
>> src/mesa/main/arrayobj.c | 11 +----------
>> src/mesa/main/attrib.c | 23 +++++++----------------
>> src/mesa/main/mtypes.h | 16 ----------------
>> src/mesa/main/varray.c | 2 +-
>> 4 files changed, 9 insertions(+), 43 deletions(-)
>>
>> diff --git a/src/mesa/main/arrayobj.c b/src/mesa/main/arrayobj.c
>> index c52a07b..82c00fb 100644
>> --- a/src/mesa/main/arrayobj.c
>> +++ b/src/mesa/main/arrayobj.c
>> @@ -443,30 +443,21 @@ _mesa_BindVertexArray( GLuint id )
>> }
>> else {
>> /* non-default array object */
>> newObj = _mesa_lookup_vao(ctx, id);
>> if (!newObj) {
>> _mesa_error(ctx, GL_INVALID_OPERATION,
>> "glBindVertexArray(non-gen name)");
>> return;
>> }
>>
>> - if (!newObj->EverBound) {
>> - /* The "Interactions with APPLE_vertex_array_object" section
>> of the
>> - * GL_ARB_vertex_array_object spec says:
>> - *
>> - * "The first bind call, either BindVertexArray or
>> - * BindVertexArrayAPPLE, determines the semantic of the
>> object."
>> - */
>> - newObj->ARBsemantics = GL_TRUE;
>> - newObj->EverBound = GL_TRUE;
>> - }
>> + newObj->EverBound = GL_TRUE;
>> }
>>
>> if (ctx->Array.DrawMethod == DRAW_ARRAYS) {
>> /* The _DrawArrays pointer is pointing at the VAO being unbound
>> and
>> * that VAO may be in the process of being deleted. If it's not
>> going
>> * to be deleted, this will have no effect, because the pointer
>> needs
>> * to be updated by the VBO module anyway.
>> *
>> * Before the VBO module can update the pointer, we have to set it
>> * to NULL for drivers not to set up arrays which are not bound,
>> diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
>> index 87d3276..dbcfb4e 100644
>> --- a/src/mesa/main/attrib.c
>> +++ b/src/mesa/main/attrib.c
>> @@ -1472,23 +1472,20 @@ copy_pixelstore(struct gl_context *ctx,
>> static void
>> copy_array_object(struct gl_context *ctx,
>> struct gl_vertex_array_object *dest,
>> struct gl_vertex_array_object *src)
>> {
>> GLuint i;
>>
>> /* skip Name */
>> /* skip RefCount */
>>
>> - /* In theory must be the same anyway, but on recreate make sure it
>> matches */
>> - dest->ARBsemantics = src->ARBsemantics;
>> -
>> for (i = 0; i < ARRAY_SIZE(src->VertexAttrib); i++) {
>> _mesa_copy_client_array(ctx, &dest->_VertexAttrib[i],
>> &src->_VertexAttrib[i]);
>> _mesa_copy_vertex_attrib_array(ctx, &dest->VertexAttrib[i],
>> &src->VertexAttrib[i]);
>> _mesa_copy_vertex_buffer_binding(ctx, &dest->BufferBinding[i],
>> &src->BufferBinding[i]);
>> }
>>
>> /* _Enabled must be the same than on push */
>> dest->_Enabled = src->_Enabled;
>> /* The bitmask of bound VBOs needs to match the VertexBinding
>> array */
>> dest->VertexAttribBufferMask = src->VertexAttribBufferMask;
>> @@ -1550,56 +1547,50 @@ save_array_attrib(struct gl_context *ctx,
>> }
>>
>> /**
>> * Restore the content of src to dest.
>> */
>> static void
>> restore_array_attrib(struct gl_context *ctx,
>> struct gl_array_attrib *dest,
>> struct gl_array_attrib *src)
>> {
>> + bool is_vao_name_zero = src->VAO->Name == 0;
>> +
>> /* The ARB_vertex_array_object spec says:
>> *
>> * "BindVertexArray fails and an INVALID_OPERATION error is
>> generated
>> * if array is not a name returned from a previous call to
>> * GenVertexArrays, or if such a name has since been deleted with
>> * DeleteVertexArrays."
>> *
>> * Therefore popping a deleted VAO cannot magically recreate it.
>> - *
>> - * The semantics of objects created using
>> APPLE_vertex_array_objects behave
>> - * differently. These objects expect to be recreated by pop. Alas.
>> */
>> - const bool arb_vao = (src->VAO->Name != 0
>> - && src->VAO->ARBsemantics);
>> -
>> - if (arb_vao && !_mesa_IsVertexArray(src->VAO->Name))
>> + if (!is_vao_name_zero && !_mesa_IsVertexArray(src->VAO->Name))
>> return;
>>
>> _mesa_BindVertexArray(src->VAO->Name);
>>
>> /* Restore or recreate the buffer objects by the names ... */
>> - if (!arb_vao
>> - || src->ArrayBufferObj->Name == 0
>> - || _mesa_IsBuffer(src->ArrayBufferObj->Name)) {
>> + if (is_vao_name_zero || src->ArrayBufferObj->Name == 0 ||
>> + _mesa_IsBuffer(src->ArrayBufferObj->Name)) {
>> /* ... and restore its content */
>> copy_array_attrib(ctx, dest, src, false);
>>
>> _mesa_BindBuffer(GL_ARRAY_BUFFER_ARB,
>> src->ArrayBufferObj->Name);
>> } else {
>> copy_array_attrib(ctx, dest, src, true);
>> }
>>
>> - if (!arb_vao
>> - || src->VAO->IndexBufferObj->Name == 0
>> - || _mesa_IsBuffer(src->VAO->IndexBufferObj->Name))
>> + if (is_vao_name_zero || src->VAO->IndexBufferObj->Name == 0 ||
>> + _mesa_IsBuffer(src->VAO->IndexBufferObj->Name))
>> _mesa_BindBuffer(GL_ELEMENT_ARRAY_BUFFER_ARB,
>> src->VAO->IndexBufferObj->Name);
>> }
>>
>> /**
>> * init/alloc the fields of 'attrib'.
>> * Needs to the init part matching free_array_attrib_data below.
>> */
>> static bool
>> init_array_attrib_data(struct gl_context *ctx,
>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
>> index 2e3a423..002e692 100644
>> --- a/src/mesa/main/mtypes.h
>> +++ b/src/mesa/main/mtypes.h
>> @@ -1502,36 +1502,20 @@ struct gl_vertex_buffer_binding
>> struct gl_vertex_array_object
>> {
>> /** Name of the VAO as received from glGenVertexArray. */
>> GLuint Name;
>>
>> GLint RefCount;
>>
>> GLchar *Label; /**< GL_KHR_debug */
>>
>> /**
>> - * Does the VAO use ARB semantics or Apple semantics?
>> - *
>> - * There are several ways in which ARB_vertex_array_object and
>> - * APPLE_vertex_array_object VAOs have differing semantics. At
>> the very
>> - * least,
>> - *
>> - * - ARB VAOs require that all array data be sourced from
>> vertex buffer
>> - * objects, but Apple VAOs do not.
>> - *
>> - * - ARB VAOs require that names come from GenVertexArrays.
>> - *
>> - * This flag notes which behavior governs this VAO.
>> - */
>> - GLboolean ARBsemantics;
>> -
>> - /**
>> * Has this array object been bound?
>> */
>> GLboolean EverBound;
>>
>> /**
>> * Derived vertex attribute arrays
>> *
>> * This is a legacy data structure created from
>> gl_vertex_attrib_array and
>> * gl_vertex_buffer_binding, for compatibility with existing
>> driver code.
>> */
>> diff --git a/src/mesa/main/varray.c b/src/mesa/main/varray.c
>> index 78dc004..2fb3c18 100644
>> --- a/src/mesa/main/varray.c
>> +++ b/src/mesa/main/varray.c
>> @@ -502,21 +502,21 @@ validate_array(struct gl_context *ctx, const
>> char *func,
>> * "An INVALID_OPERATION error is generated under any of the
>> following
>> * conditions:
>> *
>> * ...
>> *
>> * * any of the *Pointer commands specifying the location and
>> * organization of vertex array data are called while zero
>> is bound
>> * to the ARRAY_BUFFER buffer object binding point (see section
>> * 2.9.6), and the pointer argument is not NULL."
>> */
>> - if (ptr != NULL && vao->ARBsemantics &&
>> + if (ptr != NULL && vao->EverBound &&
>
> The EverBound is an extraneous change and should be unnecessary. With
> that fixed, patches 2-4 are
Actually many many piglit/cts tests end up hitting this error without
adding EverBound here.
>
> Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>
>
>
>> !_mesa_is_bufferobj(ctx->Array.ArrayBufferObj)) {
>> _mesa_error(ctx, GL_INVALID_OPERATION, "%s(non-VBO array)", func);
>> return;
>> }
>> }
>>
>>
>> static bool
>> validate_array_and_format(struct gl_context *ctx, const char *func,
>> GLuint attrib, GLbitfield legalTypes,
>>
>
>
More information about the mesa-dev
mailing list