[Mesa-dev] [PATCH] mesa: change state query return value for RGB565

[Jason Ekstrand]

On Fri, Aug 12, 2016 at 2:00 PM, Chad Versace <chad at kiwitree.net> wrote:

> +jekstrand
>
> On 08/12/2016 01:36 PM, Haixia Shi wrote:
>
>> The GL_BGR and GL_UNSIGNED_SHORT_5_6_5_REV are not defined anywhere in
>> OpenGL ES 3.2 (or earlier) specification, and there are no known
>> extensions
>> in the Khronos registry that would add these enums as valid responses for
>> glGetIntegerv(GL_IMPLEMENTATION_COLOR_READ_TYPE) and
>> glGetIntegerv(GL_IMPLEMENTATION_COLOR_READ_FORMAT) queries.
>>
>> TEST=dEQP-GLES3.functional.state_query.integers.*
>>
>
> Does this patch fix all the tests in that group? Just a few tests? Exactly
> one?
>
>
> Signed-off-by: Haixia Shi <hshi at chromium.org>
>> Cc: Chad Versace <chadversary at chromium.org>
>> Cc: Stéphane Marchesin <marcheu at chromium.org>
>>
>> Change-Id: I81bbc8ccdc7e125edaeae443baf6fa8fdefcc6b6
>> ---
>>  src/mesa/main/framebuffer.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/main/framebuffer.c b/src/mesa/main/framebuffer.c
>> index 38bd680..f024f5e 100644
>> --- a/src/mesa/main/framebuffer.c
>> +++ b/src/mesa/main/framebuffer.c
>> @@ -857,7 +857,7 @@ _mesa_get_color_read_format(struct gl_context *ctx)
>>        if (format == MESA_FORMAT_B8G8R8A8_UNORM)
>>           return GL_BGRA;
>>        else if (format == MESA_FORMAT_B5G6R5_UNORM)
>> -         return GL_BGR;
>> +         return GL_RGB;
>>        else if (format == MESA_FORMAT_R_UNORM8)
>>           return GL_RED;
>>
>> @@ -892,7 +892,7 @@ _mesa_get_color_read_type(struct gl_context *ctx)
>>        const GLenum data_type = _mesa_get_format_datatype(format);
>>
>>        if (format == MESA_FORMAT_B5G6R5_UNORM)
>> -         return GL_UNSIGNED_SHORT_5_6_5_REV;
>> +         return GL_UNSIGNED_SHORT_5_6_5;
>>
>>        switch (data_type) {
>>        case GL_SIGNED_NORMALIZED:
>>
>
> I'm convinced that the original code and hshi's patch are both correct
> because, as defined by the GL spec, the bit layout of
>
>     GL_BGR + GL_UNSIGNED_SHORT_5_6_5_REV
> and
>
>     GL_RGB + GL_UNSIGNED_SHORT_5_6_5
>
> are identical, which is
>
>     msb -> lsb     lsb -> msb
>     R6 G6 B5    == B5 G6 R5    == ISL_FORMAT_B5G6R5_UNORM
>
> I *really* want to see the commit message briefly state that the patch
> doesn't change the bit
> layout returned by the query. Because that's critical to the patch's
> correctness, but not
> obvious at all.
>

Yes, they are the same.  And it's obvious, in the best possible "math
professor" sense of the word. :-)  What's not obvious is whether or not
this is going to throw off well-meaning applications...


> I'm not 100% confident in my conclusion, though. I would like to see a 2nd
> opinion on mesa-dev, preferably by jekstrand who is the Format Layout King.
>

:p
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160812/bb8b8abe/attachment.html>