[Mesa-dev] [PATCH 3/3] mesa: do more thorough target checking in compressed_subtexture_target_check()

[Brian Paul]

On 07/23/2015 08:26 AM, Roland Scheidegger wrote:
> Am 23.07.2015 um 15:57 schrieb Brian Paul:
>> When we're error-checking the target, we also need to check if the
>> corresponding extension is supported.
>> ---
>>   src/mesa/main/teximage.c | 67 +++++++++++++++++++++++++++++-------------------
>>   1 file changed, 41 insertions(+), 26 deletions(-)
>>
>> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
>> index 0726758..11eca5e 100644
>> --- a/src/mesa/main/teximage.c
>> +++ b/src/mesa/main/teximage.c
>> @@ -4555,13 +4555,16 @@ compressed_subtexture_target_check(struct gl_context *ctx, GLenum target,
>>      case 2:
>>         switch (target) {
>>         case GL_TEXTURE_2D:
>> +         targetOK = GL_TRUE;
>> +         break;
>>         case GL_TEXTURE_CUBE_MAP_POSITIVE_X:
>>         case GL_TEXTURE_CUBE_MAP_NEGATIVE_X:
>>         case GL_TEXTURE_CUBE_MAP_POSITIVE_Y:
>>         case GL_TEXTURE_CUBE_MAP_NEGATIVE_Y:
>>         case GL_TEXTURE_CUBE_MAP_POSITIVE_Z:
>>         case GL_TEXTURE_CUBE_MAP_NEGATIVE_Z:
>> -         targetOK = GL_TRUE;
>> +         targetOK = _mesa_is_desktop_gl(ctx)
>> +            && ctx->Extensions.ARB_texture_cube_map;
> Why isn't this valid for ES?

I was following the example from legal_teximage_target() but it looks 
like that was for the proxy target check.  I'll fix that before pushing. 
  Thanks.


>
>
>>            break;
>>         default:
>>            targetOK = GL_FALSE;
>> @@ -4569,31 +4572,40 @@ compressed_subtexture_target_check(struct gl_context *ctx, GLenum target,
>>         }
>>         break;
>>      case 3:
>> -      targetOK = (target == GL_TEXTURE_3D) ||
>> -                 (target == GL_TEXTURE_2D_ARRAY) ||
>> -                 (target == GL_TEXTURE_CUBE_MAP_ARRAY) ||
>> -                 (target == GL_TEXTURE_CUBE_MAP && dsa);
>> -
>> -      /* OpenGL 4.5 spec (30.10.2014) says in Section 8.7 Compressed Texture
>> -       * Images:
>> -       *    "An INVALID_OPERATION error is generated by
>> -       *    CompressedTex*SubImage3D if the internal format of the texture is
>> -       *    one of the EAC, ETC2, or RGTC formats and either border is
>> -       *    non-zero, or the effective target for the texture is not
>> -       *    TEXTURE_2D_ARRAY."
>> -       *
>> -       * NOTE: that's probably a spec error.  It should probably say
>> -       *    "... or the effective target for the texture is not
>> -       *    TEXTURE_2D_ARRAY, TEXTURE_CUBE_MAP, nor GL_TEXTURE_CUBE_MAP_ARRAY."
>> -       * since those targets are 2D images and they support all compression
>> -       * formats.
>> -       *
>> -       * Instead of listing all these, just list those which are allowed,
>> -       * which is (at this time) only bptc. Otherwise we'd say s3tc (and more)
>> -       * are valid here, which they are not, but of course not mentioned by
>> -       * core spec.
>> -       */
>> -      if (target == GL_TEXTURE_3D) {
>> +      switch (target) {
>> +      case GL_TEXTURE_CUBE_MAP:
>> +         targetOK = dsa && ctx->Extensions.ARB_texture_cube_map;
> I think technically the ARB_texture_cube_map check wouldn't be
> necessary, since dsa implies support for it. Should't hurt though...
>
>> +         break;
>> +      case GL_TEXTURE_2D_ARRAY:
>> +         targetOK = _mesa_is_gles3(ctx) ||
>> +            (_mesa_is_desktop_gl(ctx) && ctx->Extensions.EXT_texture_array);
>> +         break;
>> +      case GL_TEXTURE_CUBE_MAP_ARRAY:
>> +         targetOK = ctx->Extensions.ARB_texture_cube_map_array;
>> +         break;
>> +      case GL_TEXTURE_3D:
>> +         targetOK = GL_TRUE;
>> +         /*
>> +          * OpenGL 4.5 spec (30.10.2014) says in Section 8.7 Compressed Texture
>> +          * Images:
>> +          *    "An INVALID_OPERATION error is generated by
>> +          *    CompressedTex*SubImage3D if the internal format of the texture
>> +          *    is one of the EAC, ETC2, or RGTC formats and either border is
>> +          *    non-zero, or the effective target for the texture is not
>> +          *    TEXTURE_2D_ARRAY."
>> +          *
>> +          * NOTE: that's probably a spec error.  It should probably say
>> +          *    "... or the effective target for the texture is not
>> +          *    TEXTURE_2D_ARRAY, TEXTURE_CUBE_MAP, nor
>> +          *    GL_TEXTURE_CUBE_MAP_ARRAY."
>> +          * since those targets are 2D images and they support all compression
>> +          * formats.
>> +          *
>> +          * Instead of listing all these, just list those which are allowed,
>> +          * which is (at this time) only bptc. Otherwise we'd say s3tc (and
>> +          * more) are valid here, which they are not, but of course not
>> +          * mentioned by core spec.
>> +          */
>>            switch (format) {
>>            /* These are the only 3D compression formats supported at this time */
>>            case GL_COMPRESSED_RGBA_BPTC_UNORM:
>> @@ -4610,6 +4622,9 @@ compressed_subtexture_target_check(struct gl_context *ctx, GLenum target,
>>                           _mesa_enum_to_string(format));
>>               return GL_TRUE;
>>            }
>> +         break;
>> +      default:
>> +         targetOK = GL_FALSE;
>>         }
>>
>>         break;
>>
>
> Otherwise, and for the rest of the series,
>
> Reviewed-by: Roland Scheidegger <sroland at vmware.com>

Thanks.

-Brian