On Mon, 2015-07-20 at 10:56 -0700, Anuj Phogat wrote: > On Mon, Jul 20, 2015 at 5:10 AM, Iago Toral <ito...@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 <ja...@jlekstrand.net> > >> wrote: > >> > > >> > On Jun 16, 2015 11:15, "Anuj Phogat" <anuj.pho...@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.pho...@gmail.com> > >> >> Cc: <mesa-sta...@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.
If this function can be used to resolve ReadPixels and GetTexImage but the checks you add are *specific* to BlitFramebuffer, it does not look like this is the right place for them. Shouldn't you put them inside _mesa_meta_BlitFramebuffer instead? Otherwise they would affect to ReadPixels and GetTexImage too and I don't see the same restrictions applying to ReadPixels for example. Iago > >> >> } > >> >> > >> >> /* For arrays, use a tall (height * depth) 2D texture but taking > >> >> into > >> >> -- > >> >> 1.9.3 > >> >> > >> >> _______________________________________________ > >> >> 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 > > > > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev