[Mesa-dev] [PATCH 4/4] mesa: tidy up left over APPLE_vertex_array_object semantics
Nicolai Hähnle
nhaehnle at gmail.com
Mon Apr 24 15:30:00 UTC 2017
On 24.04.2017 12:28, Timothy Arceri wrote:
>
>
> 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.
Okay, I think I understand now. The point is that those tests use a
compatibility profile with user array pointers and no VAO.
Perhaps the test could be
if (ptr != NULL && vao != ctx->Array.DefaultVAO &&
!_mesa_is_bufferobj(ctx->Array.ArrayBufferObj))
I think that should work, and it's a bit more explicit about what the
check means.
Cheers,
Nicolai
>
>>
>> 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,
>>>
>>
>>
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
More information about the mesa-dev
mailing list