1229210 - Handle the new formats required by WebGL2 in ReadPixels

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

This is required for deqp/functional/gles3/builtinprecision00.html to pass on Linux

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Attachment: WIP to get things working

Comment 3

[Jeff Muizelaar [:jrmuizel]]

Attachment: Add LOCAL_GL_INT as well

jgilbert, is this approach sane?

Comment 4

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8698557 [details] [diff] [review]
Add LOCAL_GL_INT as well

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

::: dom/canvas/WebGLContextGL.cpp
@@ +1458,5 @@
> +    if (IsWebGL2()) {
> +      if (!IsFormatAndTypeUnpackable2(format, type))
> +        return ErrorInvalidEnum("readPixels: Bad format or type.");
> +    } else {
> +      if (!IsFormatAndTypeUnpackable(format, type))

In other places, we have functions like these as virtuals. We should *probably* go that route here as well.

@@ +1499,5 @@
>  
> +    case LOCAL_GL_UNSIGNED_INT:
> +    case LOCAL_GL_INT:
> +        bytesPerPixel = 4;
> +        requiredDataType = js::Scalar::Uint32;

Shouldn't GL_INT require Int32?

Comment 5

[Jeff Muizelaar [:jrmuizel]]

Attachment: Handle the new formats required by WebGL2 in ReadPixels

I decided against doing a virtual function for a couple of reasons:
1. The existing function is static
2. I wanted to avoid the code duplication
3. I wanted to make the fact that WebGL2 is a superset of format more obvious
4. I wanted to keep the format checking code closer together
5. I find having the WebGL2 conditional easier to read because you don't have to know that the function is virtual to see what's going on.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/05b0e3cb50a4

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/05b0e3cb50a4