<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Thu, Nov 20, 2014 at 12:29 AM, Iago Toral <<a href="mailto:itoral@igalia.com" target="_blank">itoral@igalia.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">It is explained here: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=84566#c35" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=84566#c35</a> So one example of this was a glReadPixels where we want to store the pixel data as RGBA UINT, but the render buffer format we read from is MESA_FORMAT_B8G8R8A8_UNORM. There are piglit tests that hit this case. </blockquote><div> </div><div>I'm still not seeing how this is allowed. From the 4.2 core spec: "If format is one of RED , GREEN , BLUE , RG , RGB , RGBA , BGR , or BGRA , then red, green, blue, and alpha values are obtained from the selected buffer at each pixel location. If format is an integer format and the color buffer is not an integer format, or if the color buffer is an integer format and format is not an integer format, an INVALID_OPERATION error is generated." </div><div>I also checked the 3.3 compatibility spec and it says the same thing. This seems to indicate that that combination should result in GL_INVALID_OPERATION. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Iago <div class=""><div class="h5"> On Wed, 2014-11-19 at 12:04 -0800, Jason Ekstrand wrote: > Can you remind me again as to what formats hit these paths? I > remember you hitting them, but I'm still not really seeing how it > happens. > > --Jason > > > On Tue, Nov 18, 2014 at 1:23 AM, Iago Toral Quiroga > <<a href="mailto:itoral@igalia.com">itoral@igalia.com</a>> wrote: > We can have conversions from non-integer types to integer > types, so remove > the assertions for these in the pack/unpack fast paths. It > could be that > we do not have all the necessary pack/unpack functions in > these cases though, > so protect these paths with conditionals and let > _mesa_format_convert use > other paths to resolve these kind of conversions if necessary. > --- > src/mesa/main/format_utils.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/src/mesa/main/format_utils.c > b/src/mesa/main/format_utils.c > index 1d65f2b..56a3b8d 100644 > --- a/src/mesa/main/format_utils.c > +++ b/src/mesa/main/format_utils.c > @@ -143,8 +143,8 @@ _mesa_format_convert(void *void_dst, > uint32_t dst_format, size_t dst_stride, > dst += dst_stride; > } > return; > - } else if (dst_array_format.as_uint == > RGBA8888_UBYTE.as_uint) { > - assert(!_mesa_is_format_integer_color(src_format)); > + } else if (dst_array_format.as_uint == > RGBA8888_UBYTE.as_uint && > + !_mesa_is_format_integer_color(src_format)) > { > for (row = 0; row < height; ++row) { > _mesa_unpack_ubyte_rgba_row(src_format, width, > src, (uint8_t > (*)[4])dst); > @@ -152,8 +152,8 @@ _mesa_format_convert(void *void_dst, > uint32_t dst_format, size_t dst_stride, > dst += dst_stride; > } > return; > - } else if (dst_array_format.as_uint == > RGBA8888_UINT.as_uint) { > - assert(_mesa_is_format_integer_color(src_format)); > + } else if (dst_array_format.as_uint == > RGBA8888_UINT.as_uint && > + _mesa_is_format_integer_color(src_format)) { > for (row = 0; row < height; ++row) { > _mesa_unpack_uint_rgba_row(src_format, width, > src, (uint32_t > (*)[4])dst); > @@ -174,8 +174,8 @@ _mesa_format_convert(void *void_dst, > uint32_t dst_format, size_t dst_stride, > dst += dst_stride; > } > return; > - } else if (src_array_format.as_uint == > RGBA8888_UBYTE.as_uint) { > - assert(!_mesa_is_format_integer_color(dst_format)); > + } else if (src_array_format.as_uint == > RGBA8888_UBYTE.as_uint && > + !_mesa_is_format_integer_color(dst_format)) > { > for (row = 0; row < height; ++row) { > _mesa_pack_ubyte_rgba_row(dst_format, width, > (const uint8_t > (*)[4])src, dst); > @@ -183,8 +183,8 @@ _mesa_format_convert(void *void_dst, > uint32_t dst_format, size_t dst_stride, > dst += dst_stride; > } > return; > - } else if (src_array_format.as_uint == > RGBA8888_UINT.as_uint) { > - assert(_mesa_is_format_integer_color(dst_format)); > + } else if (src_array_format.as_uint == > RGBA8888_UINT.as_uint && > + _mesa_is_format_integer_color(dst_format)) { > for (row = 0; row < height; ++row) { > _mesa_pack_uint_rgba_row(dst_format, width, > (const uint32_t > (*)[4])src, dst); > -- > 1.9.1 > > _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> > > </div></div></blockquote></div> </div></div>