[Mesa-dev] [PATCH 03/29] mesa: Do not assert on integer<->non-integer direct pack/unpack fast paths

[Jason Ekstrand]

On Thu, Nov 20, 2014 at 12:29 AM, Iago Toral <itoral at 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.


>
> 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
> > <itoral at 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 at lists.freedesktop.org
> >         http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> >
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141120/5b9d634a/attachment-0001.html>