On Fri, Aug 12, 2016 at 2:00 PM, Chad Versace <c...@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 <h...@chromium.org>
>> Cc: Chad Versace <chadvers...@chromium.org>
>> Cc: Stéphane Marchesin <marc...@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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
