I've reported this bug to Mozilla (https://bugzilla.mozilla.org/show_bug.cgi?id=743585), but I think Mesa may be needlessly going through the glReadPixels slow path so I thought I'd best report the bug here too. I think what's happening is Firefox is creating a RGBA backing store of some kind, and then attempting to read it back BGRA. This is then in turn causing fast_read_rgba_pixels_memcpy (in /src/mesa/main/readpix.c) to fail and instead fall back to the slow path. As Firefox will read back the whole frame buffer each frame, this signifcantly damages performance. I assume another fast path could be added which does the read back with a swizzle? In the mozilla bug report I included a patch to mesa that does this (which is a horrible hack). I'd be happy to try and turn that hack into a more general solution. Is that worth doing? If so, what's the best line of attack?
Created attachment 59889 [details] [review] do red/blue swizzle in glReadPixels Can you try this patch for Mesa/master? If it works I can back port it to the 8.0 branch.
(In reply to comment #1) > Created attachment 59889 [details] [review] [review] > do red/blue swizzle in glReadPixels > > Can you try this patch for Mesa/master? If it works I can back port it to the > 8.0 branch. Applied the patch with master. Built with: ./configure --disable-dri --disable-egl --enable-xlib-glx --with-gallium-drivers=swrast This does boost performance when the backing store is MESA_FORMAT_RGBA8888_REV . I found that this is not always the case with Firefox. If you initialise your webgl context with canvas.getContext("experimental-webgl",{alpha: false }) you end up with an MESA_FORMAT_XRGB8888 render buffer.
Created attachment 60068 [details] [review] add another fast path for glReadPixels Here's an updated patch that handles the MESA_FORMAT_XRGB888 buffer format. I'm assuming that the format/type params to glReadPixels are still GL_BGRA/GL_UNSIGNED_INT_8_8_8_8_REV.
(In reply to comment #3) > Created attachment 60068 [details] [review] [review] > add another fast path for glReadPixels > > Here's an updated patch that handles the MESA_FORMAT_XRGB888 buffer format. > I'm assuming that the format/type params to glReadPixels are still > GL_BGRA/GL_UNSIGNED_INT_8_8_8_8_REV. The patch seems to work if I change if (rb->Format == MESA_FORMAT_XRGB8888_REV ... to if (rb->Format == MESA_FORMAT_XRGB8888 .... Incidentally, couldn't llvmpipe do all this for you? From what I can tell, there are a load of routines generated from lp_tile_soa.py that can do all the conversion for you. I think these conversion routines are used to generate the buffer returned by ctx->Driver.MapRenderbuffer anyway. Or would it be a case of moving the glReadPixels implementaion into Gallium (in the case of llvmpipe)? If so, I'm not sure if I have the skills to do it myself, but I'll have a look around and see what I can do.
(In reply to comment #4) > (In reply to comment #3) > > Created attachment 60068 [details] [review] [review] [review] > > add another fast path for glReadPixels > > > > Here's an updated patch that handles the MESA_FORMAT_XRGB888 buffer format. > > I'm assuming that the format/type params to glReadPixels are still > > GL_BGRA/GL_UNSIGNED_INT_8_8_8_8_REV. > > The patch seems to work if I change > > if (rb->Format == MESA_FORMAT_XRGB8888_REV ... > > to > > if (rb->Format == MESA_FORMAT_XRGB8888 .... OK, thanks, I guess I did a bad copy&paste. > Incidentally, couldn't llvmpipe do all this for you? From what I can tell, > there are a load of routines generated from lp_tile_soa.py that can do all the > conversion for you. I think these conversion routines are used to generate the > buffer returned by ctx->Driver.MapRenderbuffer anyway. Or would it be a case of > moving the glReadPixels implementaion into Gallium (in the case of llvmpipe)? > If so, I'm not sure if I have the skills to do it myself, but I'll have a look > around and see what I can do. Doing this change in readpix.c means that all drivers can benefit. Besides, the MapRenderbuffer call has no way of telling the driver that a specific surface format is desired. I'll clean up my patch and post it for review before committing. I think I can back-port this to the 8.0 branch too.
Patch committed to mesa with commit a5e95a419e4f6ad93e35a960113d97ae2de27476. This includes a better approach to the swizzling code which in my test was 44% faster than before. Also applied to the 8.0 branch: 49ed43b6de98482c898334a9abfc574720391c9f
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.