[Mesa-dev] [PATCH] mesa: make vertex array type error checking a little more efficient

[Brian Paul]

On 08/05/2014 10:35 AM, Roland Scheidegger wrote:
> Am 30.07.2014 19:08, schrieb Brian Paul:
>> Compute the bitmask of supported array types once instead of every
>> time we call a GL vertex array function.
>> ---
>>   src/mesa/main/mtypes.h |    3 ++
>>   src/mesa/main/varray.c |   86 +++++++++++++++++++++++++++++++-----------------
>>   2 files changed, 59 insertions(+), 30 deletions(-)
>>
>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
>> index 3f60a55..f5ce360 100644
>> --- a/src/mesa/main/mtypes.h
>> +++ b/src/mesa/main/mtypes.h
>> @@ -1693,6 +1693,9 @@ struct gl_array_attrib
>>
>>      /** One of the DRAW_xxx flags, not consumed by drivers */
>>      gl_draw_method DrawMethod;
>> +
>> +   /** Legal array datatypes */
>> +   GLbitfield LegalTypesMask;
>>   };
>>
>>
>> diff --git a/src/mesa/main/varray.c b/src/mesa/main/varray.c
>> index 46956ef..0356858 100644
>> --- a/src/mesa/main/varray.c
>> +++ b/src/mesa/main/varray.c
>> @@ -179,6 +179,53 @@ vertex_binding_divisor(struct gl_context *ctx, GLuint bindingIndex,
>>
>>
>>   /**
>> + * Examine the API profile and extensions to determine which types are legal
>> + * for vertex arrays.  This is called once from update_array_format().
>> + */
>> +static GLbitfield
>> +get_legal_types_mask(const struct gl_context *ctx)
>> +{
>> +   GLbitfield legalTypesMask = ~0u; /* all */
> I think it would be better to list all possible values explicitly here,
> otherwise you have lots of impossible bits in there in the end. It
> should not make any difference really, though for instance for debugging
> the legalTypesMask might look a little confusing otherwise.

OK, I can do that in a follow-up.


>
>> +
>> +   if (_mesa_is_gles(ctx)) {
>> +      legalTypesMask &= ~(FIXED_GL_BIT |
>> +                          DOUBLE_BIT |
>> +                          UNSIGNED_INT_10F_11F_11F_REV_BIT);
>> +
>> +      /* GL_INT and GL_UNSIGNED_INT data is not allowed in OpenGL ES until
>> +       * 3.0.  The 2_10_10_10 types are added in OpenGL ES 3.0 or
>> +       * GL_OES_vertex_type_10_10_10_2.  GL_HALF_FLOAT data is not allowed
>> +       * until 3.0 or with the GL_OES_vertex_half float extension, which isn't
>> +       * quite as trivial as we'd like because it uses a different enum value
>> +       * for GL_HALF_FLOAT_OES.
>> +       */
>> +      if (ctx->Version < 30) {
>> +         legalTypesMask &= ~(UNSIGNED_INT_BIT |
>> +                             INT_BIT |
>> +                             UNSIGNED_INT_2_10_10_10_REV_BIT |
>> +                             INT_2_10_10_10_REV_BIT |
>> +                             HALF_BIT);
>> +      }
>> +   }
>> +   else {
>> +      legalTypesMask &= ~FIXED_ES_BIT;
>> +
>> +      if (!ctx->Extensions.ARB_ES2_compatibility)
>> +         legalTypesMask &= ~FIXED_GL_BIT;
>> +
>> +      if (!ctx->Extensions.ARB_vertex_type_2_10_10_10_rev)
>> +         legalTypesMask &= ~(UNSIGNED_INT_2_10_10_10_REV_BIT |
>> +                             INT_2_10_10_10_REV_BIT);
>> +
>> +      if (!ctx->Extensions.ARB_vertex_type_10f_11f_11f_rev)
>> +         legalTypesMask &= ~UNSIGNED_INT_10F_11F_11F_REV_BIT;
>> +   }
>> +
>> +   return legalTypesMask;
>> +}
>> +
>> +
>> +/**
>>    * Does error checking and updates the format in an attrib array.
>>    *
>>    * Called by update_array() and VertexAttrib*Format().
>> @@ -208,40 +255,19 @@ update_array_format(struct gl_context *ctx,
>>      GLuint elementSize;
>>      GLenum format = GL_RGBA;
>>
>> -   if (_mesa_is_gles(ctx)) {
>> -      legalTypesMask &= ~(FIXED_GL_BIT | DOUBLE_BIT | UNSIGNED_INT_10F_11F_11F_REV_BIT);
>> -
>> -      /* GL_INT and GL_UNSIGNED_INT data is not allowed in OpenGL ES until
>> -       * 3.0.  The 2_10_10_10 types are added in OpenGL ES 3.0 or
>> -       * GL_OES_vertex_type_10_10_10_2.  GL_HALF_FLOAT data is not allowed
>> -       * until 3.0 or with the GL_OES_vertex_half float extension, which isn't
>> -       * quite as trivial as we'd like because it uses a different enum value
>> -       * for GL_HALF_FLOAT_OES.
>> +   if (ctx->Array.LegalTypesMask == 0) {
>> +      /* One-time initialization.  We can't do this in _mesa_init_varrays()
>> +       * below because extensions are not yet enabled at that point.
>>          */
>> -      if (ctx->Version < 30) {
>> -         legalTypesMask &= ~(UNSIGNED_INT_BIT
>> -                             | INT_BIT
>> -                             | UNSIGNED_INT_2_10_10_10_REV_BIT
>> -                             | INT_2_10_10_10_REV_BIT
>> -                             | HALF_BIT);
>> -      }
>> +      ctx->Array.LegalTypesMask = get_legal_types_mask(ctx);
>> +   }
>> +
>> +   legalTypesMask &= ctx->Array.LegalTypesMask;
>>
>> +   if (_mesa_is_gles(ctx) && sizeMax == BGRA_OR_4) {
>>         /* BGRA ordering is not supported in ES contexts.
>>          */
>> -      if (sizeMax == BGRA_OR_4)
>> -         sizeMax = 4;
>> -   } else {
>> -      legalTypesMask &= ~FIXED_ES_BIT;
>> -
>> -      if (!ctx->Extensions.ARB_ES2_compatibility)
>> -         legalTypesMask &= ~FIXED_GL_BIT;
>> -
>> -      if (!ctx->Extensions.ARB_vertex_type_2_10_10_10_rev)
>> -         legalTypesMask &= ~(UNSIGNED_INT_2_10_10_10_REV_BIT |
>> -                             INT_2_10_10_10_REV_BIT);
>> -
>> -      if (!ctx->Extensions.ARB_vertex_type_10f_11f_11f_rev)
>> -         legalTypesMask &= ~UNSIGNED_INT_10F_11F_11F_REV_BIT;
>> +      sizeMax = 4;
>>      }
>>
>>      typeBit = type_to_bit(ctx, type);
>>
>
> Otherwise,
>
> Reviewed-by: Roland Scheidegger <sroland at vmware.com>
>
> Did this have a measurable impact somewhere?

I didn't measure anything.  It just seemed silly to recompute this for 
every vertex array call.

-Brian