<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Aug 12, 2016 at 2:00 PM, Chad Versace <span dir="ltr"><<a href="mailto:chad@kiwitree.net" target="_blank">chad@kiwitree.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">+jekstrand<span class=""><br>
<br>
On 08/12/2016 01:36 PM, Haixia Shi wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
The GL_BGR and GL_UNSIGNED_SHORT_5_6_5_REV are not defined anywhere in<br>
OpenGL ES 3.2 (or earlier) specification, and there are no known extensions<br>
in the Khronos registry that would add these enums as valid responses for<br>
glGetIntegerv(GL_IMPLEMENTATIO<wbr>N_COLOR_READ_TYPE) and<br>
glGetIntegerv(GL_IMPLEMENTATIO<wbr>N_COLOR_READ_FORMAT) queries.<br>
<br>
TEST=dEQP-GLES3.functional.sta<wbr>te_query.integers.*<br>
</blockquote>
<br></span>
Does this patch fix all the tests in that group? Just a few tests? Exactly one?<div><div class="h5"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Signed-off-by: Haixia Shi <<a href="mailto:hshi@chromium.org" target="_blank">hshi@chromium.org</a>><br>
Cc: Chad Versace <<a href="mailto:chadversary@chromium.org" target="_blank">chadversary@chromium.org</a>><br>
Cc: Stéphane Marchesin <<a href="mailto:marcheu@chromium.org" target="_blank">marcheu@chromium.org</a>><br>
<br>
Change-Id: I81bbc8ccdc7e125edaeae443baf6f<wbr>a8fdefcc6b6<br>
---<br>
src/mesa/main/framebuffer.c | 4 ++--<br>
1 file changed, 2 insertions(+), 2 deletions(-)<br>
<br>
diff --git a/src/mesa/main/framebuffer.c b/src/mesa/main/framebuffer.c<br>
index 38bd680..f024f5e 100644<br>
--- a/src/mesa/main/framebuffer.c<br>
+++ b/src/mesa/main/framebuffer.c<br>
@@ -857,7 +857,7 @@ _mesa_get_color_read_format(st<wbr>ruct gl_context *ctx)<br>
if (format == MESA_FORMAT_B8G8R8A8_UNORM)<br>
return GL_BGRA;<br>
else if (format == MESA_FORMAT_B5G6R5_UNORM)<br>
- return GL_BGR;<br>
+ return GL_RGB;<br>
else if (format == MESA_FORMAT_R_UNORM8)<br>
return GL_RED;<br>
<br>
@@ -892,7 +892,7 @@ _mesa_get_color_read_type(stru<wbr>ct gl_context *ctx)<br>
const GLenum data_type = _mesa_get_format_datatype(form<wbr>at);<br>
<br>
if (format == MESA_FORMAT_B5G6R5_UNORM)<br>
- return GL_UNSIGNED_SHORT_5_6_5_REV;<br>
+ return GL_UNSIGNED_SHORT_5_6_5;<br>
<br>
switch (data_type) {<br>
case GL_SIGNED_NORMALIZED:<br>
</blockquote>
<br></div></div>
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<br>
<br>
GL_BGR + GL_UNSIGNED_SHORT_5_6_5_REV<br>
and<br>
<br>
GL_RGB + GL_UNSIGNED_SHORT_5_6_5<br>
<br>
are identical, which is<br>
<br>
msb -> lsb lsb -> msb<br>
R6 G6 B5 == B5 G6 R5 == ISL_FORMAT_B5G6R5_UNORM<br>
<br>
I *really* want to see the commit message briefly state that the patch doesn't change the bit<br>
layout returned by the query. Because that's critical to the patch's correctness, but not<br>
obvious at all.<br></blockquote><div><br></div><div>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...<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
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.<br>
</blockquote></div><br></div><div class="gmail_extra">:p<br></div></div>