[Mesa-stable] [Mesa-dev] [PATCH 05/14] meta: Abort meta pbo path if readpixels need signed-unsigned conversion

[Anuj Phogat]

On Mon, Jul 20, 2015 at 5:10 AM, Iago Toral <itoral at igalia.com> wrote:
> On Fri, 2015-06-19 at 13:40 -0700, Anuj Phogat wrote:
>> On Tue, Jun 16, 2015 at 9:21 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>> >
>> > On Jun 16, 2015 11:15, "Anuj Phogat" <anuj.phogat at gmail.com> wrote:
>> >>
>> >> Without this patch, piglit test fbo_integer_readpixels_sint_uint fails,
>> >> when
>> >> forced to use the meta pbo path.
>> >>
>> >> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
>> >> Cc: <mesa-stable at lists.freedesktop.org>
>> >> ---
>> >>  src/mesa/drivers/common/meta_tex_subimage.c | 3 +++
>> >>  1 file changed, 3 insertions(+)
>> >>
>> >> diff --git a/src/mesa/drivers/common/meta_tex_subimage.c
>> >> b/src/mesa/drivers/common/meta_tex_subimage.c
>> >> index 00364f8..84cbc50 100644
>> >> --- a/src/mesa/drivers/common/meta_tex_subimage.c
>> >> +++ b/src/mesa/drivers/common/meta_tex_subimage.c
>> >> @@ -283,6 +283,9 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
>> >> GLuint dims,
>> >>
>> >>        if (_mesa_need_rgb_to_luminance_conversion(rb->Format, format))
>> >>           return false;
>> >> +
>> >> +      if (_mesa_need_signed_unsigned_int_conversion(rb->Format, format,
>> >> type))
>> >> +         return false;
>> >
>> > Hrm... This seems fishy.  Isn't glBlitFramebuffers supposed to handle format
>> > conversion with integers?  If so we should probably fix it rather than just
>> > skip it for the meta pbo path.
>> >
>> As discussed offline, here is relevant text for glBlitFrameBuffer() from
>> OpenGL 4.5 spec, section 18.3.1:
>> "An INVALID_OPERATION error is generated if format conversions are not
>> supported, which occurs under any of the following conditions:
>> -The read buffer contains fixed-point or floating-point values and any draw
>>   buffer contains neither fixed-point nor floating-point values.
>> -The read buffer contains unsigned integer values and any draw buffer does
>>   not contain unsigned integer values.
>> - The read buffer contains signed integer values and any draw buffer does
>>   not contain signed integer values."
>>
>> I'll add a comment here explaining the reason to avoid meta path.
>
> Is this code going to run only for glBlitFramebuffer? I see this
> function being called from code paths that implement glReadPixels and
> glGetTexImage too.
>
_mesa_meta_pbo_GetTexSubImage() is used only for glReadPixels and
glGetTexImage. I quoted the glBliFrameBuffer restriction above because
the function is later using _mesa_meta_BlitFramebuffer(), which doesn't
support some format conversions.

> Iago
>
>> >>     }
>> >>
>> >>     /* For arrays, use a tall (height * depth) 2D texture but taking into
>> >> --
>> >> 1.9.3
>> >>
>> >> _______________________________________________
>> >> mesa-dev mailing list
>> >> mesa-dev at lists.freedesktop.org
>> >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>