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

[Roland Scheidegger]

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?


>           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>