Last Comment Bug 728724 - BGRA glReadPixels throws error on PowerVR SGX540
: BGRA glReadPixels throws error on PowerVR SGX540
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla13
Assigned To: Jeff Gilbert [:jgilbert]
:
Mentors:
Depends on:
Blocks: 729726
  Show dependency treegraph
 
Reported: 2012-02-19 12:48 PST by Joe Drew (not getting mail)
Modified: 2012-03-13 04:59 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Check valid read formats before readPixels (5.21 KB, patch)
2012-02-21 15:11 PST, Jeff Gilbert [:jgilbert]
jacob.benoit.1: review+
Details | Diff | Review
Check valid read formats before readPixels (5.21 KB, patch)
2012-02-22 14:30 PST, Jeff Gilbert [:jgilbert]
jgilbert: review+
Details | Diff | Review

Description Joe Drew (not getting mail) 2012-02-19 12:48:32 PST
Bug 727311 discovered that WebGL in readback mode didn't work on the Nexus S and the Galaxy Nexus. The OES_read_format spec[1] says that

    The preferred type and format combination returned may depend
    on the read surface bound to the current GL context.

I suspect this is what is biting us; we query the preferred type and format before we've bound our offscreen framebuffer.

For now, BGRA support is disabled altogether. It'd be nice to be able to use it where it actually works, though.

1. http://www.khronos.org/registry/gles/extensions/OES/OES_read_format.txt
Comment 1 Jeff Gilbert [:jgilbert] 2012-02-21 13:41:33 PST
A note that OES_read_format is not strictly relevant to GLES2, since that behavior has been folded into core. The behavior should be the same though. (Unfortunately, the GLES2 spec is even less well worded than the extension)
Comment 2 Jeff Gilbert [:jgilbert] 2012-02-21 15:11:53 PST
Created attachment 599385 [details] [diff] [review]
Check valid read formats before readPixels
Comment 3 Joe Drew (not getting mail) 2012-02-22 06:53:20 PST
Comment on attachment 599385 [details] [diff] [review]
Check valid read formats before readPixels

We should use GetOptimalReadFormats when creating framebuffers too, since we use RGBA by default all over the place when creating FBOs.
Comment 4 Benoit Jacob [:bjacob] (mostly away) 2012-02-22 12:42:42 PST
Comment on attachment 599385 [details] [diff] [review]
Check valid read formats before readPixels

Review of attachment 599385 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with this comment:

::: gfx/gl/GLContext.cpp
@@ +1707,5 @@
>  
> +static void
> +GetOptimalReadFormats(GLContext* gl, GLenum& format, GLenum& type) {
> +    if (gl->IsGLES2()) {
> +        bool hasBGRA_UBtye = false;

Typo: Btye -> Byte. Also, a _ after 'has' would be nice.
Comment 5 Jeff Gilbert [:jgilbert] 2012-02-22 14:30:43 PST
Created attachment 599777 [details] [diff] [review]
Check valid read formats before readPixels

Fixed and r+ carried forward.
Comment 6 Jeff Gilbert [:jgilbert] 2012-02-22 14:47:18 PST
(In reply to Joe Drew (:JOEDREW!) from comment #3)
> Comment on attachment 599385 [details] [diff] [review]
> Check valid read formats before readPixels
> 
> We should use GetOptimalReadFormats when creating framebuffers too, since we
> use RGBA by default all over the place when creating FBOs.

We can do this, but it's slightly different. All this stuff is only relevant to readPixels. For FBOs, we are working with textures, so we need a different set of extensions. For textures, there is also no hint which would be faster. We could bank on BGRA always being faster, but I don't know as this is a safe bet. Perhaps this is a good thing to blocklist for?

As an aside, ANGLE uses BGRA internally even if you request RGBA.
Comment 7 Jeff Gilbert [:jgilbert] 2012-02-22 14:51:29 PST
Bug for BGRA-texture-backed FBOs is up at bug 729726.
Comment 9 Marco Bonardo [::mak] 2012-03-13 04:59:23 PDT
https://hg.mozilla.org/mozilla-central/rev/c3a8909af707

Note You need to log in before you can comment on or make changes to this bug.