[Mesa-dev] [PATCH 03/29] mesa: Do not assert on integer<->non-integer direct pack/unpack fast paths
Jason Ekstrand
jason at jlekstrand.net
Thu Nov 20 21:35:26 PST 2014
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.
>
>> 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/48a41440/attachment.html>
More information about the mesa-dev
mailing list