[Mesa-dev] [PATCH 06/14] mesa/varray: split update_array() into validate_array() and update_array()

[Timothy Arceri]

On 13/04/17 12:54, Brian Paul wrote:

> On 04/12/2017 05:42 PM, Timothy Arceri wrote:
>> This will be used for adding KHR_no_error support.
>> ---
>>   src/mesa/main/varray.c | 55 
>> +++++++++++++++++++++++++++++++++++++-------------
>>   1 file changed, 41 insertions(+), 14 deletions(-)
>>
>> diff --git a/src/mesa/main/varray.c b/src/mesa/main/varray.c
>> index 233dc0d..65734df 100644
>> --- a/src/mesa/main/varray.c
>> +++ b/src/mesa/main/varray.c
>> @@ -431,50 +431,45 @@ update_array_format(struct gl_context *ctx,
>>         _mesa_error(ctx, GL_INVALID_OPERATION, "%s(size=%d)", func, 
>> size);
>>         return false;
>>      }
>>
>>      _mesa_update_array_format(ctx, vao, attrib, size, type, format,
>>                                normalized, integer, doubles, 
>> relativeOffset);
>>
>>      return true;
>>   }
>>
>> -
>>   /**
>> - * Do error checking and update state for 
>> glVertex/Color/TexCoord/...Pointer
>> - * functions.
>> + * Do error checking for glVertex/Color/TexCoord/...Pointer functions.
>>    *
>>    * \param func  name of calling function used for error reporting
>>    * \param attrib  the attribute array index to update
>>    * \param legalTypes  bitmask of *_BIT above indicating legal 
>> datatypes
>>    * \param sizeMin  min allowable size value
>>    * \param sizeMax  max allowable size value (may also be BGRA_OR_4)
>>    * \param size  components per element (1, 2, 3 or 4)
>>    * \param type  datatype of each component (GL_FLOAT, GL_INT, etc)
>>    * \param stride  stride between elements, in elements
>>    * \param normalized  are integer types converted to floats in [-1, 
>> 1]?
>>    * \param integer  integer-valued values (will not be normalized to 
>> [-1,1])
>>    * \param doubles  Double values not reduced to floats
>>    * \param ptr  the address (or offset inside VBO) of the array data
>>    */
>>   static void
>> -update_array(struct gl_context *ctx,
>> -             const char *func,
>> -             GLuint attrib, GLbitfield legalTypesMask,
>> -             GLint sizeMin, GLint sizeMax,
>> -             GLint size, GLenum type, GLsizei stride,
>> -             GLboolean normalized, GLboolean integer, GLboolean 
>> doubles,
>> -             const GLvoid *ptr)
>> +validate_array(struct gl_context *ctx, const char *func,
>> +               GLuint attrib, GLbitfield legalTypesMask,
>> +               GLint sizeMin, GLint sizeMax,
>> +               GLint size, GLenum type, GLsizei stride,
>> +               GLboolean normalized, GLboolean integer, GLboolean 
>> doubles,
>> +               const GLvoid *ptr)
>>   {
>>      struct gl_vertex_array_object *vao = ctx->Array.VAO;
>> -   struct gl_array_attributes *array;
>> -   GLsizei effectiveStride;
>>
>>      /* Page 407 (page 423 of the PDF) of the OpenGL 3.0 spec says:
>>       *
>>       *     "Client vertex arrays - all vertex array attribute 
>> pointers must
>>       *     refer to buffer objects (section 2.9.2). The default 
>> vertex array
>>       *     object (the name zero) is also deprecated. Calling
>>       *     VertexAttribPointer when no buffer object or no vertex 
>> array object
>>       *     is bound will generate an INVALID_OPERATION error..."
>>       *
>>       * The check for VBOs is handled below.
>> @@ -507,37 +502,69 @@ update_array(struct gl_context *ctx,
>>       *     * 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 &&
>>          !_mesa_is_bufferobj(ctx->Array.ArrayBufferObj)) {
>>         _mesa_error(ctx, GL_INVALID_OPERATION, "%s(non-VBO array)", 
>> func);
>>         return;
>>      }
>> +}
>> +
>> +
>> +/**
>> + * Update state for glVertex/Color/TexCoord/...Pointer functions.
>> + *
>> + * \param func  name of calling function used for error reporting
>> + * \param attrib  the attribute array index to update
>> + * \param legalTypes  bitmask of *_BIT above indicating legal datatypes
>> + * \param sizeMin  min allowable size value
>> + * \param sizeMax  max allowable size value (may also be BGRA_OR_4)
>> + * \param size  components per element (1, 2, 3 or 4)
>> + * \param type  datatype of each component (GL_FLOAT, GL_INT, etc)
>> + * \param stride  stride between elements, in elements
>> + * \param normalized  are integer types converted to floats in [-1, 1]?
>> + * \param integer  integer-valued values (will not be normalized to 
>> [-1,1])
>> + * \param doubles  Double values not reduced to floats
>> + * \param ptr  the address (or offset inside VBO) of the array data
>> + */
>> +static void
>> +update_array(struct gl_context *ctx,
>> +             const char *func,
>> +             GLuint attrib, GLbitfield legalTypesMask,
>> +             GLint sizeMin, GLint sizeMax,
>> +             GLint size, GLenum type, GLsizei stride,
>> +             GLboolean normalized, GLboolean integer, GLboolean 
>> doubles,
>> +             const GLvoid *ptr)
>> +{
>> +   struct gl_vertex_array_object *vao = ctx->Array.VAO;
>> +
>> +   validate_array(ctx, func, attrib, legalTypesMask, sizeMin, sizeMax,
>> +                  size, type, stride, normalized, integer, doubles, 
>> ptr);
>
> Don't we need validate_array() to return a bool so that we can return 
> here if an error was found?
>
It could be the correct thing to do, however that is a change in the 
current functionality. I just left it to return and continue to the next 
lot of validation like it was previously.