<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Aug 25, 2015 at 4:19 PM, Chad Versace <span dir="ltr"><<a href="mailto:chad.versace@intel.com" target="_blank">chad.versace@intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">On Sun 16 Aug 2015, Nanley Chery wrote:<br> > The last line of the commit message should say:<br> ><br> > GL_RGBA4_S3TC (0x83A3) -> COMPRESSED_RGBA_S3TC_DXT5_EXT (0x83F3)<br> <br> </span>There's another weird line too, see below.<br> <span class=""><br> > On Wed, Aug 12, 2015 at 4:19 PM, Nanley Chery <<a href="mailto:nanleychery@gmail.com">nanleychery@gmail.com</a>> wrote:<br> ><br> > > From: Nanley Chery <<a href="mailto:nanley.g.chery@intel.com">nanley.g.chery@intel.com</a>><br> > ><br> > > This function's cases for non-generic compressed formats duplicate<br> > > the GL to MESA translation in _mesa_glenum_to_compressed_format().<br> > > This patch replaces the switch cases with a call to the translation<br> > > function. There are no behavioral changes except for the RGB[A]4 formats:<br> > ><br> > > case GL_RGB4_S3TC:<br> > > return MESA_FORMAT_RGB_DXT1 (old) -> MESA_FORMAT_RGBA_DXT1 (new)<br> </span> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^<br> <br> I have trouble believing this line is correct. It adds an alpha channel.<br> Can you provide the documentation for that translation?<br> <div class=""><div class="h5"><br></div></div></blockquote><div>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:</div><div><br></div><div> * 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 <a href="http://homepage.ntlworld.com/neal.tringham/OpenGLGameDev/s3tcoglext.htm">http://homepage.ntlworld.com/neal.tringham/OpenGLGameDev/s3tcoglext.htm</a>)</div><div><br></div><div>* MESA_FORMAT_RGBA_DXT5 should be reserved for the <span style="color:rgb(0,0,0);white-space:pre-wrap">RGBA[4]_DXT5_S3TC</span> cases we currently aren't handling (<a href="https://www.opengl.org/registry/specs/S3/s3tc.txt">https://www.opengl.org/registry/specs/S3/s3tc.txt</a>)</div><div><br></div><div>* 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[].</div><div><br></div><div>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.</div><div><br></div><div>Thanks,</div><div>Nanley</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class=""><div class="h5"> > > case GL_RGBA4_S3TC:<br> > > return MESA_FORMAT_RGBA_DXT3 (old) -> MESA_FORMAT_RGBA_DXT5 (new)<br> > ><br> > > Although unclear, the old behavior was likely a bug, given that online<br> > > documentation (few and far between) imply a format mapping of the<br> > > following:<br> > ><br> > > GL_RGB_S3TC (0x83A0) -> COMPRESSED_RGB_S3TC_DXT1_EXT (0x83F0)<br> > > GL_RGB4_S3TC (0x83A1) -> COMPRESSED_RGBA_S3TC_DXT1_EXT (0x83F1)<br> > > GL_RGBA_S3TC (0x83A2) -> COMPRESSED_RGBA_S3TC_DXT3_EXT (0x83F2)<br> > > GL_RGBA4_S3TC (0x83A2) -> COMPRESSED_RGBA_S3TC_DXT5_EXT (0x83F2)<br> > ><br> > > Cc: Brian Paul <<a href="mailto:brianp@vmware.com">brianp@vmware.com</a>><br> > > Cc: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a>><br> > > Signed-off-by: Nanley Chery <<a href="mailto:nanley.g.chery@intel.com">nanley.g.chery@intel.com</a>><br> > > ---<br> > > src/mesa/main/texformat.c | 94<br> > > +++++++----------------------------------------<br> > > 1 file changed, 13 insertions(+), 81 deletions(-)<br> </div></div></blockquote></div><br></div></div>