[Mesa-dev] [RFC PATCH 3/4] mesa: Prefer non-swizzled formats when prefer_no_swizzle set

[Chris Forbes]

Actually, after poking around a bit more, I think this is correct...

As of Brian's commit 657436da7ee, the packed formats are described as
being laid out from the LSB. This is consistent with this patch, with
the pack/unpack code, and the hardware documentation:

    mesa: update packed format layout comments

    Update the comments for the packed formats to accurately reflect the
    layout of the bits in the pixel.  For example, for the packed format
    MESA_FORMAT_R8G8B8A8, R is in the least significant position while A
    is in the most-significant position of the 32-bit word.

Unfortunately the example higher up of how a type-P format is laid out
is still backwards, which adds to the confusion.

-- Chris

On Wed, Mar 12, 2014 at 12:36 PM, Chris Forbes <chrisf at ijw.co.nz> wrote:
> Yeah, you're right -- that looks bogus. There's been piles of
> confusion recently in formats.h about which end things are laid out
> from; the truth though is obviously in the pack/unpack code and the
> hardware format documentation.
>
> On Wed, Mar 12, 2014 at 9:12 AM, Francisco Jerez <currojerez at riseup.net> wrote:
>> Chris Forbes <chrisf at ijw.co.nz> writes:
>>
>>> If prefer_no_swizzle is set, try:
>>> - The exact matching format
>>> - Formats with the required components in the correct order, plus a junk
>>>   component
>>> - finally, swizzled formats (BGRA etc)
>>>
>>> Signed-off-by: Chris Forbes <chrisf at ijw.co.nz>
>>> ---
>>>  src/mesa/main/texformat.c | 35 +++++++++++++++++++++++++++++++----
>>>  1 file changed, 31 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/mesa/main/texformat.c b/src/mesa/main/texformat.c
>>> index 33725dc..7cb42bc 100644
>>> --- a/src/mesa/main/texformat.c
>>> +++ b/src/mesa/main/texformat.c
>>> @@ -79,11 +79,13 @@ _mesa_choose_tex_format(struct gl_context *ctx, GLenum target,
>>>        } else if (type == GL_UNSIGNED_INT_2_10_10_10_REV) {
>>>           RETURN_IF_SUPPORTED(MESA_FORMAT_B10G10R10A2_UNORM);
>>>        }
>>> -      RETURN_IF_SUPPORTED(MESA_FORMAT_A8B8G8R8_UNORM);
>>> -      RETURN_IF_SUPPORTED(MESA_FORMAT_B8G8R8A8_UNORM);
>>> -      break;
>>> +      /* fallthrough */
>>>
>>>     case GL_RGBA8:
>>> +      if (prefer_no_swizzle) {
>>> +         RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8A8_UNORM);
>>> +         break;
>>> +      }
>>
>> Are you sure this is correct on little endian machines?  According to
>> the docs, MESA_FORMAT_R8G8B8A8_UNORM would end up with the R component
>> in the most significant byte (highest array index) and the A component
>> in the least significant byte (lowest array index), which seems like the
>> opposite of what GL_RGBA8 is specified to be by
>> ARB_shader_image_load_store.
>>
>>>        RETURN_IF_SUPPORTED(MESA_FORMAT_A8B8G8R8_UNORM);
>>>        RETURN_IF_SUPPORTED(MESA_FORMAT_B8G8R8A8_UNORM);
>>>        break;
>>> @@ -100,6 +102,9 @@ _mesa_choose_tex_format(struct gl_context *ctx, GLenum target,
>>>
>>>     /* deep RGBA formats */
>>>     case GL_RGB10_A2:
>>> +      if (prefer_no_swizzle) {
>>> +         RETURN_IF_SUPPORTED(MESA_FORMAT_R10G10B10A2_UNORM);
>>> +      }
>>
>> This looks correct for any endianness.
>>
>>>        RETURN_IF_SUPPORTED(MESA_FORMAT_B10G10R10A2_UNORM);
>>>        RETURN_IF_SUPPORTED(MESA_FORMAT_B8G8R8A8_UNORM);
>>>        break;
>>> @@ -119,6 +124,11 @@ _mesa_choose_tex_format(struct gl_context *ctx, GLenum target,
>>>        }
>>>        /* fallthrough */
>>>     case GL_RGB8:
>>> +      if (prefer_no_swizzle) {
>>> +         RETURN_IF_SUPPORTED(MESA_FORMAT_RGB_UNORM8);
>>> +         RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8X8_UNORM);
>>> +         RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8A8_UNORM);
>>
>> The last two seem wrong for LE too.
>>
>>> +      }
>>>        RETURN_IF_SUPPORTED(MESA_FORMAT_BGR_UNORM8);
>>>        RETURN_IF_SUPPORTED(MESA_FORMAT_B8G8R8X8_UNORM);
>>>        RETURN_IF_SUPPORTED(MESA_FORMAT_B8G8R8A8_UNORM);
>>> @@ -454,10 +464,19 @@ _mesa_choose_tex_format(struct gl_context *ctx, GLenum target,
>>>        break;
>>>     case GL_RGB_SNORM:
>>>     case GL_RGB8_SNORM:
>>> +      if (prefer_no_swizzle) {
>>> +         RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8X8_SNORM);
>>> +         RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8A8_SNORM);
>>> +      }
>>
>> Same here.
>>
>>>        RETURN_IF_SUPPORTED(MESA_FORMAT_X8B8G8R8_SNORM);
>>> -      /* FALLTHROUGH */
>>> +      RETURN_IF_SUPPORTED(MESA_FORMAT_A8B8G8R8_SNORM);
>>> +      RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8A8_SNORM);
>>> +      break;
>>>     case GL_RGBA_SNORM:
>>>     case GL_RGBA8_SNORM:
>>> +      if (prefer_no_swizzle) {
>>> +         RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8A8_SNORM);
>>> +      }
>>
>> And here.
>>
>>>        RETURN_IF_SUPPORTED(MESA_FORMAT_A8B8G8R8_SNORM);
>>>        RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8A8_SNORM);
>>>        break;
>>> @@ -525,11 +544,19 @@ _mesa_choose_tex_format(struct gl_context *ctx, GLenum target,
>>>
>>>     case GL_SRGB_EXT:
>>>     case GL_SRGB8_EXT:
>>> +      if (prefer_no_swizzle) {
>>> +         /* there is no MESA_FORMAT_RGB_SRGB8 */
>>> +         RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8X8_SRGB);
>>> +         RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8A8_SRGB);
>>
>> And here.
>>
>>> +      }
>>>        RETURN_IF_SUPPORTED(MESA_FORMAT_BGR_SRGB8);
>>>        RETURN_IF_SUPPORTED(MESA_FORMAT_B8G8R8A8_SRGB);
>>>        break;
>>>     case GL_SRGB_ALPHA_EXT:
>>>     case GL_SRGB8_ALPHA8_EXT:
>>> +      if (prefer_no_swizzle) {
>>> +         RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8A8_SRGB);
>>> +      }
>>>        RETURN_IF_SUPPORTED(MESA_FORMAT_A8B8G8R8_SRGB);
>>>        RETURN_IF_SUPPORTED(MESA_FORMAT_B8G8R8A8_SRGB);
>>>        break;
>>> --
>>> 1.9.0
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev