On 7 December 2013 17:11, Francisco Jerez <curroje...@riseup.net> wrote:
> Paul Berry <stereotype...@gmail.com> writes: > > > On 24 November 2013 21:00, Francisco Jerez <curroje...@riseup.net> > wrote: > >[...] > >> + > >> + case GL_RGBA16UI: > >> + return MESA_FORMAT_RGBA_UINT16; > >> + > >> + case GL_RGB10_A2UI: > >> + return MESA_FORMAT_ABGR2101010_UINT; > >> > > > > I don't understand the naming conventions of the GL and MESA formats well > > enough to be sure about this, but I'm troubled by the inconsistency > between > > the two case statements above. In the GL_RGBA16UI case, the MESA format > > lists the components in the same order as the GL format (RGBA). But in > the > > GL_RGB10_A2UI case, the MESA format lists the components in the opposite > > order as the GL format (ABGR instead of RGBA). Unless there are some > > really counterintuitive naming conventions at work here, it seems like > > these cases can't both be right. > > > > Yes. Mesa's formats mix two naming conventions: MESA_FORMAT_XYZW_N > specifies the XYZW components in memory order, each of N bits, while > MESA_FORMAT_XYZWNMPQ specifies the XYZW components from MSB to LSB in an > endianness-dependent manner. The GL_RGB10_A2UI internal format refers > to an RGBA_INTEGER format of type UNSIGNED_INT_2_10_10_10_REV, which > matches the Mesa format above -- See table 8.8 in the GL4.4 spec. > Wow, that explains why I never managed to pick up the convention by osmosis. Thanks for explaining. 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: - MESA_FORMAT_RGB888 - MESA_FORMAT_BGR888 - MESA_FORMAT_SRGB8 - MESA_FORMAT_XBGR16161616_UNORM - MESA_FORMAT_XBGR16161616_SNORM - MESA_FORMAT_XBGR16161616_FLOAT - MESA_FORMAT_XBGR32323232_FLOAT 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 :) > > > This can't be right, because u->Layer can be much larger than 6, but the > > size of the t->Image[] array is 6. I believe what we need to do instead > is: > > > > (a) for cubemaps, we need to validate that 0 <= u->Layer < 6 before > > accessing t->Image, otherwise we will access garbage memory. > > > > (b) for non-cubemaps (e.g. 2D arrays), all the layers for each miplevel > are > > stored in a single gl_texture_image, so we need to access > > t->Image[0][u->Level], and then check that 0 <= u->Layer < t->Depth. > > > > (c) I'm not sure how cubemap arrays are handled by the Mesa front end, so > > I'm not sure what is correct to do for them. > > > > I've addressed these issues here [1], I can send a v2 to the mailing > list if you have more suggestions specific to the new version. > 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 Reviewed-by: Paul Berry <stereotype...@gmail.com>
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
