<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Dec 19, 2013 at 5:47 PM, Michel Dänzer <span dir="ltr"><<a href="mailto:michel@daenzer.net" target="_blank">michel@daenzer.net</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"><div class="im">On Don, 2013-12-19 at 13:56 -0800, Mark Mueller wrote:<br> > Adjust MESA_FORMAT color component ordering to match API docs, driver<br> > specific formats (e.g. PIPE_FORMATs),<br> <br> </div>Actually, there are a couple of examples of other format definitions<br> which match the Mesa formats before your change but no longer after it,<br> e.g. in the DRI and i915, nouveau and radeon driver code.<br></blockquote><div><br></div><div>It's just not fair to hold the mesa formats to the varied conventions in the DRI driver code because they are all over the place across all the different drivers, past, present, and future (the confusion with MESA_FORMATs may be partly to blame). Moving the mesa formats closer to those of the API, instead of a nasty mix in-between, brings at least some clarity to the situation and the documentation burden can be left to the API specs - minus a few outliers.</div> <div><br></div><div>I'm pretty confident that this naming confusion is resulting in some unnecessary texture re-packing work due to <span style="color:rgb(0,0,0)">_mesa_choose_tex_format returning less then optimal formats, though by name they look to be the best choice. My next series attempts to address that.</span></div> <div><span style="color:rgb(0,0,0)"><br></span></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"> <br> Changing the Mesa format definitions will be confusing for people<br> switching between branches with and without your change.<br> <br> Also, because these Mesa formats are defined as packed values, you're<br> essentially changing the notation from big endian (aka human readable)<br> to little endian. It's unfortunate that the packed PIPE_FORMATs are<br> named in little endian order, that's a concession we had to make when<br> adding them.<br></blockquote><div><br></div><div>Are they all really big endian, currently it looks like a mix of who knows what. The main role the MESA_FORMATs are serving is to fortify the GLenums coming from the API, which are close to useless on their own. I don't think there is any question that there is a lack of convention and using the conventions already provided by the API offers the best fit for lots of reasons.</div> <div><br></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"> <br> Overall, I'm afraid this change doesn't look very good at all to me. At<br> the very least though, you'd also have to change the order of component<br> sizes for formats such as MESA_FORMAT_BGRA2101010_UNORM or<br> MESA_FORMAT_BGRA1555_UNORM, otherwise they're just plain wrong.<br> <div class="im"><br> <br> > and actual use on common platforms.<br> <br> </div>What does that mean?<br></blockquote><div><br></div><div>I'm referring to PIPE_FORMAT, BRW_SURFACEFORMAT, RADEON_TXFORMAT, etc here. </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="im"><br> <br> > Remove comments giving MESA_FORMAT color packings, some of which are<br> > misleading.<br> <br> </div>Which ones are misleading, and how?<br></blockquote><div><br></div><div>Bottom line, if the MESA_FORMATs follow the API's convention, then the reader can go to the API for clarification and we don't have to maintain that in the code. There are some special cases which can be dealt with in the code. </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="im"><br> <br> > diff --git a/src/mesa/main/formats.h b/src/mesa/main/formats.h<br> > index 94fc7a0..f224ed5 100644<br> > --- a/src/mesa/main/formats.h<br> > +++ b/src/mesa/main/formats.h<br> > @@ -62,67 +62,68 @@ typedef enum<br> > MESA_FORMAT_NONE = 0,<br> ><br> > /**<br> > - * \name Basic hardware formats<br> > + * \name Basic API user space data formats<br> <br> </div>All of Mesa is in user space. :) </blockquote><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="im"><br> > + * Please refer to API documentation for more information on format<br> > + * packing<br> > */<br> <br> </div>What API documentation?<br></blockquote><div><br></div><div><a href="http://opengl.org/wiki/Pixel_Transfer">opengl.org/wiki/Pixel_Transfer</a></div><div><a href="http://opengl.org/wiki/Image_Format">opengl.org/wiki/Image_Format</a></div> <div><a href="http://msdn.microsoft.com/en-us/library/windows/desktop/bb172558(v=vs.85).aspx">http://msdn.microsoft.com/en-us/library/windows/desktop/bb172558(v=vs.85).aspx</a> </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"> <span class=""><font color="#888888"></font></span></blockquote></div><br></div><div class="gmail_extra"><br></div><div class="gmail_extra">Thanks for your input Michel. Do you still think this is a bad idea?</div><div class="gmail_extra"> <br></div><div class="gmail_extra">Mark</div></div>