[Mesa-dev] [PATCH v4 1/1] intel: add support for ANGLE_texture_compression_dxt.

Oliver McFadden oliver.mcfadden at linux.intel.com
Fri Sep 28 02:27:25 PDT 2012


On Thu, Sep 27, 2012 at 07:28:47AM -0600, Brian Paul wrote:
> Looks good to me, Oliver.
> 
> Reviewed-by: Brian Paul <brianp at vmware.com>

Thanks Brian.  Please also check Kenneth's comments and my reply; I'd
like your input there.

> 
> One question below...

OK.

> 
> On 09/27/2012 02:56 AM, Oliver McFadden wrote:
> > Signed-off-by: Oliver McFadden<oliver.mcfadden at linux.intel.com>
> > ---
> > v4: Off-by-one on a couple of 'if (ctx->Mesa_DXTn)' lines, which could cause a
> > crash.
> >
> >   src/glx/glxextensions.h                       |    3 ++-
> >   src/mapi/glapi/gen/es_EXT.xml                 |    6 ++++++
> >   src/mesa/drivers/dri/intel/intel_extensions.c |    1 +
> >   src/mesa/main/APIspec.xml                     |    3 +++
> >   src/mesa/main/extensions.c                    |    3 +++
> >   src/mesa/main/glformats.c                     |    6 ++++--
> >   src/mesa/main/glheader.h                      |   11 +++++++++++
> >   src/mesa/main/mtypes.h                        |    1 +
> >   src/mesa/main/texformat.c                     |    9 ++++++---
> >   src/mesa/main/teximage.c                      |   10 ++++++++++
> >   10 files changed, 47 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/glx/glxextensions.h b/src/glx/glxextensions.h
> > index 90c27a7..9e072e4 100644
> > --- a/src/glx/glxextensions.h
> > +++ b/src/glx/glxextensions.h
> > @@ -67,7 +67,8 @@ enum
> >
> >   enum
> >   {
> > -   GL_ARB_depth_texture_bit = 0,
> > +   GL_ANGLE_texture_compression_dxt_bit = 0,
> > +   GL_ARB_depth_texture_bit,
> >      GL_ARB_draw_buffers_bit,
> >      GL_ARB_fragment_program_bit,
> >      GL_ARB_fragment_program_shadow_bit,
> > diff --git a/src/mapi/glapi/gen/es_EXT.xml b/src/mapi/glapi/gen/es_EXT.xml
> > index fc2ec62..2698110 100644
> > --- a/src/mapi/glapi/gen/es_EXT.xml
> > +++ b/src/mapi/glapi/gen/es_EXT.xml
> > @@ -731,4 +731,10 @@
> >       <enum name="RG8_EXT"                                  value="0x822B"/>
> >   </category>
> >
> > +<!-- 111. GL_ANGLE_texture_compression_dxt -->
> > +<category name="ANGLE_texture_compression_dxt" number="111">
> > +<enum name="COMPRESSED_RGBA_S3TC_DXT3_ANGLE"        value="0x83F2"/>
> > +<enum name="COMPRESSED_RGBA_S3TC_DXT5_ANGLE"        value="0x83F3"/>
> > +</category>
> > +
> >   </OpenGLAPI>
> > diff --git a/src/mesa/drivers/dri/intel/intel_extensions.c b/src/mesa/drivers/dri/intel/intel_extensions.c
> > index 89f6c1e..8a46488 100755
> > --- a/src/mesa/drivers/dri/intel/intel_extensions.c
> > +++ b/src/mesa/drivers/dri/intel/intel_extensions.c
> > @@ -182,6 +182,7 @@ intelInitExtensions(struct gl_context *ctx)
> >      }
> >
> >      if (intel->ctx.Mesa_DXTn) {
> > +      ctx->Extensions.ANGLE_texture_compression_dxt = true;
> >         ctx->Extensions.EXT_texture_compression_s3tc = true;
> >         ctx->Extensions.S3_s3tc = true;
> >      }
> > diff --git a/src/mesa/main/APIspec.xml b/src/mesa/main/APIspec.xml
> > index a65c5c5..c396952 100644
> > --- a/src/mesa/main/APIspec.xml
> > +++ b/src/mesa/main/APIspec.xml
> > @@ -2172,6 +2172,9 @@
> >   	<category name="NV_draw_buffers"/>
> >   	<category name="NV_read_buffer"/>
> >
> > +	<!-- GL_ANGLE_texture_compression_dxt -->
> > +	<category name="ANGLE_texture_compression_dxt"/>
> > +
> >   	<function name="DrawBuffersNV" template="DrawBuffers"/>
> >   	<function name="ReadBufferNV" template="ReadBuffer"/>
> >
> > diff --git a/src/mesa/main/extensions.c b/src/mesa/main/extensions.c
> > index bd7c7ba..4971ebc 100644
> > --- a/src/mesa/main/extensions.c
> > +++ b/src/mesa/main/extensions.c
> > @@ -195,6 +195,8 @@ static const struct extension extension_table[] = {
> >      { "GL_EXT_texture3D",                           o(EXT_texture3D),                           GLL,            1996 },
> >      { "GL_EXT_texture_array",                       o(EXT_texture_array),                       GL,             2006 },
> >      { "GL_EXT_texture_compression_dxt1",            o(EXT_texture_compression_s3tc),            GL | ES1 | ES2, 2004 },
> > +   { "GL_ANGLE_texture_compression_dxt3",          o(ANGLE_texture_compression_dxt),           ES2,            2011 },
> > +   { "GL_ANGLE_texture_compression_dxt5",          o(ANGLE_texture_compression_dxt),           ES2,            2011 },
> >      { "GL_EXT_texture_compression_latc",            o(EXT_texture_compression_latc),            GL,             2006 },
> >      { "GL_EXT_texture_compression_rgtc",            o(ARB_texture_compression_rgtc),            GL,             2004 },
> >      { "GL_EXT_texture_compression_s3tc",            o(EXT_texture_compression_s3tc),            GL,             2000 },
> > @@ -480,6 +482,7 @@ _mesa_enable_sw_extensions(struct gl_context *ctx)
> >      ctx->Extensions.EXT_gpu_program_parameters = GL_TRUE;
> >      _mesa_enable_extension(ctx, "GL_3DFX_texture_compression_FXT1");
> >      if (ctx->Mesa_DXTn) {
> > +      ctx->Extensions.ANGLE_texture_compression_dxt = GL_TRUE;
> >         _mesa_enable_extension(ctx, "GL_EXT_texture_compression_s3tc");
> >         _mesa_enable_extension(ctx, "GL_S3_s3tc");
> >      }
> > diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
> > index 04029c0..ccdf56b 100644
> > --- a/src/mesa/main/glformats.c
> > +++ b/src/mesa/main/glformats.c
> > @@ -791,8 +791,10 @@ _mesa_is_compressed_format(struct gl_context *ctx, GLenum format)
> >         return ctx->Extensions.EXT_texture_compression_s3tc;
> >      case GL_COMPRESSED_RGBA_S3TC_DXT3_EXT:
> >      case GL_COMPRESSED_RGBA_S3TC_DXT5_EXT:
> > -      return _mesa_is_desktop_gl(ctx)
> > -&&  ctx->Extensions.EXT_texture_compression_s3tc;
> > +      return (_mesa_is_desktop_gl(ctx)&&
> > +	      ctx->Extensions.EXT_texture_compression_s3tc) ||
> > +	     (ctx->API == API_OPENGLES2&&
> > +	      ctx->Extensions.ANGLE_texture_compression_dxt);
> 
> If an extension like this is marked as being "ES2" in extensions.c why 
> do we need to check ctx->API==API_OPENGLES2?  It seems to me that we 
> should only have to test ctx->Extensions.ANGLE_texture_compression_dxt 
> and not the API (as you did in the _mesa_choose_tex_format() change).
> 
> I think there's other instances of API+extension checks like this in Mesa.

Ian alreaedy replied earlier and answered this; the flags in
extensions.c only control whether the extension string is exported.  If
I would remove the API == ES2 check here, then it's possible that
ANGLE_texture_compression_dxt could be set true by some driver code, and
we'd then expose those formats on e.g. OpenGL (non-ES) 2.x.

_mesa_is_desktop_gl is doing basically the same thing:
src/mesa/main/context.h-   return ctx->API == API_OPENGL || ctx->API == API_OPENGL_CORE

> 
> 
> 
> >      case GL_RGB_S3TC:
> >      case GL_RGB4_S3TC:
> >      case GL_RGBA_S3TC:
> > diff --git a/src/mesa/main/glheader.h b/src/mesa/main/glheader.h
> > index e93ca30..33cda02 100644
> > --- a/src/mesa/main/glheader.h
> > +++ b/src/mesa/main/glheader.h
> > @@ -59,6 +59,17 @@ extern "C" {
> >   #endif
> >
> >
> > +/* GL_ANGLE_texture_compression_dxt3 */
> > +#ifndef GL_ANGLE_texture_compression_dxt3
> > +#define GL_COMPRESSED_RGBA_S3TC_DXT3_ANGLE                      0x83F2
> > +#endif
> > +
> > +/* GL_ANGLE_texture_compression_dxt5 */
> > +#ifndef GL_ANGLE_texture_compression_dxt5
> > +#define GL_COMPRESSED_RGBA_S3TC_DXT5_ANGLE                      0x83F3
> > +#endif
> > +
> > +
> >   /**
> >    * GL_FIXED is defined in glext.h version 64 but these typedefs aren't (yet).
> >    */
> > diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> > index ba43e57..e790e18 100644
> > --- a/src/mesa/main/mtypes.h
> > +++ b/src/mesa/main/mtypes.h
> > @@ -2946,6 +2946,7 @@ struct gl_extensions
> >      GLboolean dummy;  /* don't remove this! */
> >      GLboolean dummy_true;  /* Set true by _mesa_init_extensions(). */
> >      GLboolean dummy_false; /* Set false by _mesa_init_extensions(). */
> > +   GLboolean ANGLE_texture_compression_dxt;
> >      GLboolean ARB_ES2_compatibility;
> >      GLboolean ARB_base_instance;
> >      GLboolean ARB_blend_func_extended;
> > diff --git a/src/mesa/main/texformat.c b/src/mesa/main/texformat.c
> > index 17270ba..cc3687e 100644
> > --- a/src/mesa/main/texformat.c
> > +++ b/src/mesa/main/texformat.c
> > @@ -302,7 +302,8 @@ _mesa_choose_tex_format(struct gl_context *ctx, GLenum target,
> >         }
> >      }
> >
> > -   if (ctx->Extensions.EXT_texture_compression_s3tc) {
> > +   if (ctx->Extensions.EXT_texture_compression_s3tc ||
> > +       ctx->Extensions.ANGLE_texture_compression_dxt) {
> >         switch (internalFormat) {
> >            case GL_COMPRESSED_RGB_S3TC_DXT1_EXT:
> >   	    RETURN_IF_SUPPORTED(MESA_FORMAT_RGB_DXT1);
> > @@ -604,12 +605,14 @@ _mesa_choose_tex_format(struct gl_context *ctx, GLenum target,
> >   	    RETURN_IF_SUPPORTED(MESA_FORMAT_SARGB8);
> >               break;
> >            case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT:
> > -            if (ctx->Extensions.EXT_texture_compression_s3tc)
> > +            if (ctx->Extensions.EXT_texture_compression_s3tc ||
> > +		ctx->Extensions.ANGLE_texture_compression_dxt)
> >   	       RETURN_IF_SUPPORTED(MESA_FORMAT_SRGBA_DXT3);
> >   	    RETURN_IF_SUPPORTED(MESA_FORMAT_SARGB8);
> >               break;
> >            case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT:
> > -            if (ctx->Extensions.EXT_texture_compression_s3tc)
> > +            if (ctx->Extensions.EXT_texture_compression_s3tc ||
> > +		ctx->Extensions.ANGLE_texture_compression_dxt)
> >   	       RETURN_IF_SUPPORTED(MESA_FORMAT_SRGBA_DXT5);
> >   	    RETURN_IF_SUPPORTED(MESA_FORMAT_SARGB8);
> >               break;
> > diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
> > index afda0ea..d297936 100644
> > --- a/src/mesa/main/teximage.c
> > +++ b/src/mesa/main/teximage.c
> > @@ -203,6 +203,16 @@ _mesa_base_tex_format( struct gl_context *ctx, GLint internalFormat )
> >         }
> >      }
> >
> > +   if (ctx->Extensions.ANGLE_texture_compression_dxt) {
> > +      switch (internalFormat) {
> > +         case GL_COMPRESSED_RGBA_S3TC_DXT3_ANGLE:
> > +         case GL_COMPRESSED_RGBA_S3TC_DXT5_ANGLE:
> > +            return GL_RGBA;
> > +         default:
> > +            ; /* fallthrough */
> > +      }
> > +   }
> > +
> >      if (ctx->Extensions.S3_s3tc) {
> >         switch (internalFormat) {
> >            case GL_RGB_S3TC:
> 

-- 
Oliver McFadden.


More information about the mesa-dev mailing list