<div dir="ltr">On 7 December 2013 17:11, Francisco Jerez <span dir="ltr"><<a href="mailto:currojerez@riseup.net" target="_blank">currojerez@riseup.net</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="im">Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> writes:<br>

<br>
> On 24 November 2013 21:00, Francisco Jerez <<a href="mailto:currojerez@riseup.net">currojerez@riseup.net</a>> wrote:<br>
</div>>[...]<br>
<div class="im">>> +<br>
>> +   case GL_RGBA16UI:<br>
>> +      return MESA_FORMAT_RGBA_UINT16;<br>
>> +<br>
>> +   case GL_RGB10_A2UI:<br>
>> +      return MESA_FORMAT_ABGR2101010_UINT;<br>
>><br>
><br>
> I don't understand the naming conventions of the GL and MESA formats well<br>
> enough to be sure about this, but I'm troubled by the inconsistency between<br>
> the two case statements above.  In the GL_RGBA16UI case, the MESA format<br>
> lists the components in the same order as the GL format (RGBA).  But in the<br>
> GL_RGB10_A2UI case, the MESA format lists the components in the opposite<br>
> order as the GL format (ABGR instead of RGBA).  Unless there are some<br>
> really counterintuitive naming conventions at work here, it seems like<br>
> these cases can't both be right.<br>
><br>
<br>
</div>Yes.  Mesa's formats mix two naming conventions: MESA_FORMAT_XYZW_N<br>
specifies the XYZW components in memory order, each of N bits, while<br>
MESA_FORMAT_XYZWNMPQ specifies the XYZW components from MSB to LSB in an<br>
endianness-dependent manner.  The GL_RGB10_A2UI internal format refers<br>
to an RGBA_INTEGER format of type UNSIGNED_INT_2_10_10_10_REV, which<br>
matches the Mesa format above -- See table 8.8 in the GL4.4 spec.<br></blockquote><div><br></div><div>Wow, that explains why I never managed to pick up the convention by osmosis.  Thanks for explaining.<br></div><div><br>
BTW, after reading through format_pack.c, I found some exceptions to what you said above.  The following formats are always packed by format_pack.c in reverse memory order, regardless of endianness:<br><br>- MESA_FORMAT_RGB888<br>
</div><div>- MESA_FORMAT_BGR888<br>- MESA_FORMAT_SRGB8<br>- MESA_FORMAT_XBGR16161616_UNORM<br>- MESA_FORMAT_XBGR16161616_SNORM<br>- MESA_FORMAT_XBGR16161616_FLOAT<br>- MESA_FORMAT_XBGR32323232_FLOAT<br></div><div><br></div>
<div>Which means they get packed correctly on little-endian systems but backwards on big-endian systems.  This seems like it's a bug in format_pack.c, so it's not relevant to your patch :)<br></div>><br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div><div class="h5">
> This can't be right, because u->Layer can be much larger than 6, but the<br>
> size of the t->Image[] array is 6.  I believe what we need to do instead is:<br>
><br>
> (a) for cubemaps, we need to validate that 0 <= u->Layer < 6 before<br>
> accessing t->Image, otherwise we will access garbage memory.<br>
><br>
> (b) for non-cubemaps (e.g. 2D arrays), all the layers for each miplevel are<br>
> stored in a single gl_texture_image, so we need to access<br>
> t->Image[0][u->Level], and then check that 0 <= u->Layer < t->Depth.<br>
><br>
> (c) I'm not sure how cubemap arrays are handled by the Mesa front end, so<br>
> I'm not sure what is correct to do for them.<br>
><br>
<br>
</div></div>I've addressed these issues here [1], I can send a v2 to the mailing<br>
list if you have more suggestions specific to the new version.<br></blockquote><div><br></div><div>Personally, I don't need to see a v2.  I looked at what you currently have on your branch (a801b805b3016eb905028726ea20a3338c4d7662), and it looks good to me.  It's<br>
<br></div><div>Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>><br></div><br></div></div></div>