On Thu, 2014-11-20 at 21:35 -0800, Jason Ekstrand wrote: > > > On Thu, Nov 20, 2014 at 9:33 PM, Jason Ekstrand <ja...@jlekstrand.net> > wrote: > > > On Thu, Nov 20, 2014 at 12:29 AM, Iago Toral > <ito...@igalia.com> wrote: > It is explained here: > https://bugs.freedesktop.org/show_bug.cgi?id=84566#c35 > > 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. > > > 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." > > > 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. > > > > I also just CC'd Ian, our local spec expert. Maybe he can shed a > little light on this.
No need. I have reverted the commit and run piglit again on i965 and swrast and I don't hit the assert any more, so I guess that when I was hitting that it was because of a bug somewhere in the GL->Mesa type mapping that I must have fixed after added this patch. I'll remove the patch in the second version of the series. > > > Iago > > 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 > > <ito...@igalia.com> 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 > > mesa-dev@lists.freedesktop.org > > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > > > > > > > > > > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev