[Mesa-dev] [PATCH v2 03/11] mesa/st: add helper to return pipe_endian based on pipe_format

[Marek Olšák]

On Fri, Apr 15, 2016 at 12:01 PM, Michel Dänzer <michel at daenzer.net> wrote:
> On 14.04.2016 21:18, Oded Gabbay wrote:
>> Add a helper function which receives pipe_format value and returns a
>> matching pipe_endian value. It is needed because almost every call to
>> st_create_texture will use this function to send the pipe_endian value
>>
>> Signed-off-by: Oded Gabbay <oded.gabbay at gmail.com>
>> ---
>>  src/mesa/state_tracker/st_format.c | 51 ++++++++++++++++++++++++++++++++++++++
>>  src/mesa/state_tracker/st_format.h |  3 +++
>>  2 files changed, 54 insertions(+)
>>
>> diff --git a/src/mesa/state_tracker/st_format.c b/src/mesa/state_tracker/st_format.c
>> index 9a280fc..cc44c9a 100644
>> --- a/src/mesa/state_tracker/st_format.c
>> +++ b/src/mesa/state_tracker/st_format.c
>> @@ -2501,3 +2501,54 @@ st_translate_color(const union gl_color_union *colorIn,
>>        }
>>     }
>>  }
>> +
>> +enum pipe_endian
>> +st_get_endian_by_format(enum pipe_format format)
>> +{
>> +   /* return native endianess by default, except for a couple of gallium
>> +    * formats, where gallium provides different formats based on endianess */
>> +   switch (format)
>> +   {
>> +   case PIPE_FORMAT_R8G8B8A8_UNORM:
>> +   case PIPE_FORMAT_R8G8B8X8_UNORM:
>> +   case PIPE_FORMAT_B8G8R8A8_UNORM:
>> +   case PIPE_FORMAT_B8G8R8X8_UNORM:
>> +   case PIPE_FORMAT_A8R8G8B8_UNORM:
>> +   case PIPE_FORMAT_X8R8G8B8_UNORM:
>> +   case PIPE_FORMAT_A8B8G8R8_UNORM:
>> +   case PIPE_FORMAT_X8B8G8R8_UNORM:
>> +   case PIPE_FORMAT_R8G8B8A8_SNORM:
>> +   case PIPE_FORMAT_R8G8B8X8_SNORM:
>> +   case PIPE_FORMAT_A8B8G8R8_SNORM:
>> +   case PIPE_FORMAT_X8B8G8R8_SNORM:
>> +   case PIPE_FORMAT_R8G8B8A8_SRGB:
>> +   case PIPE_FORMAT_R8G8B8X8_SRGB:
>> +   case PIPE_FORMAT_B8G8R8A8_SRGB:
>> +   case PIPE_FORMAT_B8G8R8X8_SRGB:
>> +   case PIPE_FORMAT_A8R8G8B8_SRGB:
>> +   case PIPE_FORMAT_X8R8G8B8_SRGB:
>> +   case PIPE_FORMAT_A8B8G8R8_SRGB:
>> +   case PIPE_FORMAT_X8B8G8R8_SRGB:
>> +   case PIPE_FORMAT_A8L8_UNORM:
>> +   case PIPE_FORMAT_L8A8_UNORM:
>> +   case PIPE_FORMAT_A8L8_SNORM:
>> +   case PIPE_FORMAT_L8A8_SNORM:
>> +   case PIPE_FORMAT_A8L8_SRGB:
>> +   case PIPE_FORMAT_L8A8_SRGB:
>> +   case PIPE_FORMAT_A16L16_UNORM:
>> +   case PIPE_FORMAT_L16A16_UNORM:
>> +   case PIPE_FORMAT_G8R8_UNORM:
>> +   case PIPE_FORMAT_R8G8_UNORM:
>> +   case PIPE_FORMAT_G8R8_SNORM:
>> +   case PIPE_FORMAT_R8G8_SNORM:
>> +   case PIPE_FORMAT_G16R16_UNORM:
>> +   case PIPE_FORMAT_R16G16_UNORM:
>> +   case PIPE_FORMAT_G16R16_SNORM:
>> +   case PIPE_FORMAT_R16G16_SNORM:
>> +      return PIPE_ENDIAN_LITTLE;
>> +
>> +   default:
>> +      return PIPE_ENDIAN_NATIVE;
>> +     }
>
> All of the formats above with 8 bits per component are so-called "array"
> formats which are defined independently from endianness[0], as a fixed
> order of bytes in memory. Attaching any endianness meaning to them makes
> no sense. That you're trying to do such a hack means that some code is
> using formats incorrectly. Such code needs to be identified and fixed.
>
> For the formats with a multiple of 8 bits per component, there's the
> issue of the endianness of each individual component. This is currently
> not explicitly defined but probably mostly implicitly assumed to be CPU
> byte order by non-HW-driver code and GPU byte order by GPU driver code.
> That's just one example of an issue that wasn't fully addressed by the
> last format endianness cleanup effort.
>
> What's needed is a completely consistent format model which covers all
> cases and is used correctly everywhere, instead of adding partial hacks
> on top of each other. I'm sorry that this is painful (I did warn you
> before you started on this journey that it could be long and painful),
> but I don't see any clean way around it.
>
> [0] See the PIPE_ARCH_BIG/LITTLE_ENDIAN sections in p_format.h for
> endian dependent, so-called "packed" formats defined relative to array
> formats.

Michel makes a good point about array formats.

util_format_description::is_array tells you if the format is an array
of components. Such formats must be treated as arrays. If it's not an
array format, it's a packed format, whose components can be read using
bitwise operations only.

The function above can be simplified to:
is_array ? PIPE_ENDIAN_LITTLE : PIPE_ENDIAN_NATIVE
(PIPE_ENDIAN_INDEPENDENT would be more accurate)

If you can simply derive the endianness from the format like that,
it's redundant to be in pipe_resource.

Marek