[Mesa-stable] [Mesa-dev] [RFC 01/21] mesa/texcompress: Restrict FXT1 format to desktop GL subset

[Ian Romanick]

On 10/19/2015 05:58 PM, Nanley Chery wrote:
> 
> 
> On Mon, Oct 19, 2015 at 3:47 PM, Ilia Mirkin <imirkin at alum.mit.edu
> <mailto:imirkin at alum.mit.edu>> wrote:
> 
>     On Mon, Oct 19, 2015 at 6:36 PM, Nanley Chery <nanleychery at gmail.com
>     <mailto:nanleychery at gmail.com>> wrote:
>     > From: Nanley Chery <nanley.g.chery at intel.com
>     <mailto:nanley.g.chery at intel.com>>
>     >
>     > In agreement with the extension spec and commit
>     > dd0eb004874645135b9aaac3ebbd0aaf274079ea, filter FXT1 formats to the
>     > desktop GL profiles. Now we no longer advertise such formats as
>     supported
>     > in an ES context and then throw an INVALID_ENUM error when the client
>     > tries to use such formats with CompressedTexImage2D.
>     >
>     > Fixes the following 26 dEQP tests:
>     > *
>     dEQP-GLES2.functional.negative_api.texture.compressedteximage2d_invalid_border
>     > *
>     dEQP-GLES2.functional.negative_api.texture.compressedteximage2d_invalid_border_cube_neg_x
>     > *
>     dEQP-GLES2.functional.negative_api.texture.compressedteximage2d_invalid_border_cube_neg_y
>     > *
>     dEQP-GLES2.functional.negative_api.texture.compressedteximage2d_invalid_border_cube_neg_z
>     > *
>     dEQP-GLES2.functional.negative_api.texture.compressedteximage2d_invalid_border_cube_pos_x
>     > *
>     dEQP-GLES2.functional.negative_api.texture.compressedteximage2d_invalid_border_cube_pos_y
>     > *
>     dEQP-GLES2.functional.negative_api.texture.compressedteximage2d_invalid_border_cube_pos_z
>     > *
>     dEQP-GLES2.functional.negative_api.texture.compressedteximage2d_invalid_size
>     > *
>     dEQP-GLES2.functional.negative_api.texture.compressedteximage2d_level_max_cube_pos
>     > *
>     dEQP-GLES2.functional.negative_api.texture.compressedteximage2d_level_max_tex2d
>     > *
>     dEQP-GLES2.functional.negative_api.texture.compressedteximage2d_neg_level_cube
>     > *
>     dEQP-GLES2.functional.negative_api.texture.compressedteximage2d_neg_level_tex2d
>     > *
>     dEQP-GLES2.functional.negative_api.texture.compressedteximage2d_neg_width_height_cube_neg_x
>     > *
>     dEQP-GLES2.functional.negative_api.texture.compressedteximage2d_neg_width_height_cube_neg_y
>     > *
>     dEQP-GLES2.functional.negative_api.texture.compressedteximage2d_neg_width_height_cube_neg_z
>     > *
>     dEQP-GLES2.functional.negative_api.texture.compressedteximage2d_neg_width_height_cube_pos_x
>     > *
>     dEQP-GLES2.functional.negative_api.texture.compressedteximage2d_neg_width_height_cube_pos_y
>     > *
>     dEQP-GLES2.functional.negative_api.texture.compressedteximage2d_neg_width_height_cube_pos_z
>     > *
>     dEQP-GLES2.functional.negative_api.texture.compressedteximage2d_neg_width_height_tex2d
>     > *
>     dEQP-GLES2.functional.negative_api.texture.compressedteximage2d_width_height_max_cube_neg_x
>     > *
>     dEQP-GLES2.functional.negative_api.texture.compressedteximage2d_width_height_max_cube_neg_y
>     > *
>     dEQP-GLES2.functional.negative_api.texture.compressedteximage2d_width_height_max_cube_neg_z
>     > *
>     dEQP-GLES2.functional.negative_api.texture.compressedteximage2d_width_height_max_cube_pos_x
>     > *
>     dEQP-GLES2.functional.negative_api.texture.compressedteximage2d_width_height_max_cube_pos_y
>     > *
>     dEQP-GLES2.functional.negative_api.texture.compressedteximage2d_width_height_max_cube_pos_z
>     > *
>     dEQP-GLES2.functional.negative_api.texture.compressedteximage2d_width_height_max_tex2d
>     >
>     > Cc: Ian Romanick <ian.d.romanick at intel.com
>     <mailto:ian.d.romanick at intel.com>>
>     > Cc: Mark Janes <mark.a.janes at intel.com
>     <mailto:mark.a.janes at intel.com>>
>     > Cc: "11.0" <mesa-stable at lists.freedesktop.org
>     <mailto:mesa-stable at lists.freedesktop.org>>
>     > Signed-off-by: Nanley Chery <nanley.g.chery at intel.com
>     <mailto:nanley.g.chery at intel.com>>
>     > ---
>     >  src/mesa/main/texcompress.c | 4 +++-
>     >  1 file changed, 3 insertions(+), 1 deletion(-)
>     >
>     > diff --git a/src/mesa/main/texcompress.c b/src/mesa/main/texcompress.c
>     > index feaf061..1615f4f 100644
>     > --- a/src/mesa/main/texcompress.c
>     > +++ b/src/mesa/main/texcompress.c
>     > @@ -286,7 +286,9 @@ GLuint
>     >  _mesa_get_compressed_formats(struct gl_context *ctx, GLint *formats)
>     >  {
>     >     GLuint n = 0;
>     > -   if (ctx->Extensions.TDFX_texture_compression_FXT1) {
>     > +   if (ctx->Extensions.TDFX_texture_compression_FXT1 &&
>     > +      (ctx->API == API_OPENGL_CORE ||
>     > +      (ctx->API == API_OPENGL_COMPAT && ctx->Version > 10))) {
> 
>     Not sure if you get rid of such helpers down the line in your series,
>     but this is the same thing as _mesa_is_desktop_gl(ctx).
> 
> 
> Later on, I replace these checks with a new helper function. The main
> difference
> between what I've done and _mesa_is_desktop_gl(ctx) is that I'm checking
> that
> the minimum version of the context is 11 (as required by the spec). I
> suppose I
> could've done (_mesa_is_desktop_gl(ctx) && ctx->Version > 10) for
> simplicity.
> The current expression was optimized for me to test how the .text size
> changes
> when moving from open-coded checks to helper functions.

The rest of the series isn't marked for stable, so it doesn't matter.
Also, as I mentioned in my review, it is impossible to have a desktop
OpenGL version less than 1.2.

>     >        if (formats) {
>     >           formats[n++] = GL_COMPRESSED_RGB_FXT1_3DFX;
>     >           formats[n++] = GL_COMPRESSED_RGBA_FXT1_3DFX;
>     > --
>     > 2.6.1
>     >
>     > _______________________________________________
>     > mesa-dev mailing list
>     > mesa-dev at lists.freedesktop.org <mailto:mesa-dev at lists.freedesktop.org>
>     > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev