728724 - BGRA glReadPixels throws error on PowerVR SGX540

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Joe Drew (not getting mail)]

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

[Kelsey Gilbert [:jgilbert]]

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

[Kelsey Gilbert [:jgilbert]]

Attachment: Check valid read formats before readPixels

Comment 3

[Joe Drew (not getting mail)]

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)]

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

[Kelsey Gilbert [:jgilbert]]

Attachment: Check valid read formats before readPixels

Fixed and r+ carried forward.

Comment 6

[Kelsey Gilbert [:jgilbert]]

(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

[Kelsey Gilbert [:jgilbert]]

Bug for BGRA-texture-backed FBOs is up at bug 729726.

Comment 8

[Kelsey Gilbert [:jgilbert]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c3a8909af707

Comment 9

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/c3a8909af707