[Mesa-dev] [PATCH] mesa/texformat: Use format conversion function in _mesa_choose_tex_format

[Nanley Chery]

On Tue, Aug 25, 2015 at 4:19 PM, Chad Versace <chad.versace at intel.com>
wrote:

> On Sun 16 Aug 2015, Nanley Chery wrote:
> > The last line of the commit message should say:
> >
> >    GL_RGBA4_S3TC  (0x83A3)  -> COMPRESSED_RGBA_S3TC_DXT5_EXT (0x83F3)
>
> There's another weird line too, see below.
>
> > On Wed, Aug 12, 2015 at 4:19 PM, Nanley Chery <nanleychery at gmail.com>
> wrote:
> >
> > > From: Nanley Chery <nanley.g.chery at intel.com>
> > >
> > > This function's cases for non-generic compressed formats duplicate
> > > the GL to MESA translation in _mesa_glenum_to_compressed_format().
> > > This patch replaces the switch cases with a call to the translation
> > > function. There are no behavioral changes except for the RGB[A]4
> formats:
> > >
> > >    case GL_RGB4_S3TC:
> > >      return MESA_FORMAT_RGB_DXT1  (old) -> MESA_FORMAT_RGBA_DXT1 (new)
>          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> I have trouble believing this line is correct. It adds an alpha channel.
> Can you provide the documentation for that translation?
>
> Since this mapping is nowhere explicitly defined, I was leaning on the
fact that _mesa_glenum_to_compressed_format() uses this mapping and the
last byte of the enums (coincidentally) match up. I did some more/better
research however and I now think that _mesa_choose_tex_format() had the
correct mapping for several reasons:

 * The GL_RGB4_S3TC and GL_RGBA4_S3TC formats are formats that the driver
can chose if the user passes in GL_RGB_S3TC and GL_RGBA_S3TC respectively.
Therefore, it's not a problem for the formats to map to the same
mesa_format (according to
http://homepage.ntlworld.com/neal.tringham/OpenGLGameDev/s3tcoglext.htm)

* MESA_FORMAT_RGBA_DXT5 should be reserved for the RGBA[4]_DXT5_S3TC cases
we currently aren't handling (
https://www.opengl.org/registry/specs/S3/s3tc.txt)

* The mapping in _mesa_choose_tex_format() can also be found
in nouveau_choose_tex_format(), radeonChooseTextureFormat(), and in
st_format.c's format_map[].

Thank you for the feedback. I will send out another patch to correct
_mesa_glenum_to_compressed_format() and (if R-b'd) will omit the note about
the behavioral change in this patch. I could also send out a v2 with an
updated commit message if you'd prefer.

Thanks,
Nanley

> >    case GL_RGBA4_S3TC:
> > >      return MESA_FORMAT_RGBA_DXT3 (old) -> MESA_FORMAT_RGBA_DXT5 (new)
> > >
> > > Although unclear, the old behavior was likely a bug, given that online
> > > documentation (few and far between) imply a format mapping of the
> > > following:
> > >
> > >    GL_RGB_S3TC    (0x83A0)  -> COMPRESSED_RGB_S3TC_DXT1_EXT  (0x83F0)
> > >    GL_RGB4_S3TC   (0x83A1)  -> COMPRESSED_RGBA_S3TC_DXT1_EXT (0x83F1)
> > >    GL_RGBA_S3TC   (0x83A2)  -> COMPRESSED_RGBA_S3TC_DXT3_EXT (0x83F2)
> > >    GL_RGBA4_S3TC  (0x83A2)  -> COMPRESSED_RGBA_S3TC_DXT5_EXT (0x83F2)
> > >
> > > Cc: Brian Paul <brianp at vmware.com>
> > > Cc: Ian Romanick <ian.d.romanick at intel.com>
> > > Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
> > > ---
> > >  src/mesa/main/texformat.c | 94
> > > +++++++----------------------------------------
> > >  1 file changed, 13 insertions(+), 81 deletions(-)
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150826/5d54441e/attachment.html>