[Mesa-dev] [PATCH 03/29] mesa: Do not assert on integer<->non-integer direct pack/unpack fast paths
Iago Toral
itoral at igalia.com
Fri Nov 21 05:53:28 PST 2014
On Thu, 2014-11-20 at 21:35 -0800, Jason Ekstrand wrote:
>
>
> On Thu, Nov 20, 2014 at 9:33 PM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
>
>
> 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.
>
>
>
> 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
> > <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
> >
> >
>
>
>
>
>
>
>
More information about the mesa-dev
mailing list