1170855 - WebGL 2 - Expose new state via gl.getParameter

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[u480271]

Not all state queries are implemented:

FAIL context.getParameter(context.PACK_ROW_LENGTH) should be 0 (of type number). Was null (of type object).
FAIL context.getParameter(context.PACK_SKIP_PIXELS) should be 0 (of type number). Was null (of type object).
FAIL context.getParameter(context.PACK_SKIP_ROWS) should be 0 (of type number). Was null (of type object).
FAIL context.getParameter(context.RASTERIZER_DISCARD) should be false (of type boolean). Was null (of type object).
FAIL context.getParameter(context.READ_BUFFER) should be 0 (of type number). Was null (of type object).
FAIL context.getParameter(context.SAMPLE_ALPHA_TO_COVERAGE) should be false (of type boolean). Was null (of type object).
FAIL context.getParameter(context.SAMPLE_COVERAGE) should be false (of type boolean). Was null (of type object).
FAIL context.getParameter(context.TRANSFORM_FEEDBACK_ACTIVE) should be false (of type boolean). Was null (of type object).
FAIL context.getParameter(context.TRANSFORM_FEEDBACK_PAUSED) should be false (of type boolean). Was null (of type object).
FAIL context.getParameter(context.UNPACK_IMAGE_HEIGHT) should be 0 (of type number). Was null (of type object).
FAIL context.getParameter(context.UNPACK_ROW_LENGTH) should be 0 (of type number). Was null (of type object).
FAIL context.getParameter(context.UNPACK_SKIP_IMAGES) should be false (of type boolean). Was null (of type object).
FAIL context.getParameter(context.UNPACK_SKIP_PIXELS) should be false (of type boolean). Was null (of type object).
FAIL context.getParameter(context.UNPACK_SKIP_ROWS) should be false (of type boolean). Was null (of type object).

FAIL context.getParameter(context.MAX_3D_TEXTURE_SIZE) should be >= 256. Was null (of type object).
FAIL context.getParameter(context.MAX_3D_TEXTURE_SIZE) is not an instance of Number
FAIL context.getParameter(context.MAX_ARRAY_TEXTURE_LAYERS) should be >= 256. Was null (of type object).
FAIL context.getParameter(context.MAX_ARRAY_TEXTURE_LAYERS) is not an instance of Number
FAIL context.getParameter(context.MAX_ARRAY_TEXTURE_LAYERS) is not an instance of Number
FAIL context.getParameter(context.MAX_COMBINED_UNIFORM_BLOCKS) should be >= 24. Was null (of type object).
FAIL context.getParameter(context.MAX_COMBINED_UNIFORM_BLOCKS) is not an instance of Number
FAIL context.getParameter(context.MAX_ELEMENT_INDEX) should be >= 16777215. Was null (of type object).
FAIL context.getParameter(context.MAX_ELEMENT_INDEX) is not an instance of Number
FAIL context.getParameter(context.MAX_ELEMENTS_INDICES) is not an instance of Number
FAIL context.getParameter(context.MAX_ELEMENTS_VERTICES) is not an instance of Number
FAIL context.getParameter(context.MAX_FRAGMENT_INPUT_COMPONENTS) should be >= 60. Was null (of type object).
FAIL context.getParameter(context.MAX_FRAGMENT_INPUT_COMPONENTS) is not an instance of Number
FAIL context.getParameter(context.MAX_FRAGMENT_UNIFORM_BLOCKS) should be >= 12. Was null (of type object).
FAIL context.getParameter(context.MAX_FRAGMENT_INPUT_COMPONENTS) is not an instance of Number
FAIL context.getParameter(context.MAX_FRAGMENT_UNIFORM_COMPONENTS) should be >= 896. Was null (of type object).
FAIL context.getParameter(context.MAX_FRAGMENT_UNIFORM_COMPONENTS) is not an instance of Number
FAIL context.getParameter(context.MAX_PROGRAM_TEXEL_OFFSET) should be >= 7. Was null (of type object).
FAIL context.getParameter(context.MAX_PROGRAM_TEXEL_OFFSET) is not an instance of Number
FAIL context.getParameter(context.MAX_PROGRAM_TEXEL_OFFSET) is not an instance of Number
FAIL context.getParameter(context.MAX_SERVER_WAIT_TIMEOUT) is not an instance of Number
FAIL context.getParameter(context.MAX_TEXTURE_LOD_BIAS) should be >= 2.0. Was null (of type object).
FAIL context.getParameter(context.MAX_TEXTURE_LOD_BIAS) is not an instance of Number
FAIL context.getParameter(context.MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS) should be >= 64. Was null (of type object).
FAIL context.getParameter(context.MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS) is not an instance of Number
FAIL context.getParameter(context.MAX_TRANSFORM_FEEDBACK_SEPARATE_COMPONENTS) should be >= 4. Was null (of type object).
FAIL context.getParameter(context.MAX_TRANSFORM_FEEDBACK_SEPARATE_COMPONENTS) is not an instance of Number
FAIL context.getParameter(context.MAX_UNIFORM_BUFFER_BINDINGS) should be >= 24. Was null (of type object).
FAIL context.getParameter(context.MAX_UNIFORM_BUFFER_BINDINGS) is not an instance of Number
FAIL context.getParameter(context.MAX_VARYING_COMPONENTS) should be >= 60. Was null (of type object).
FAIL context.getParameter(context.MAX_VARYING_COMPONENTS) is not an instance of Number
FAIL context.getParameter(context.MAX_VERTEX_OUTPUT_COMPONENTS) should be >= 64. Was null (of type object).
FAIL context.getParameter(context.MAX_VERTEX_OUTPUT_COMPONENTS) is not an instance of Number
FAIL context.getParameter(context.MAX_VERTEX_UNIFORM_BLOCKS) should be >= 12. Was null (of type object).
FAIL context.getParameter(context.MAX_VERTEX_UNIFORM_BLOCKS) is not an instance of Number
FAIL context.getParameter(context.MIN_PROGRAM_TEXEL_OFFSET) is not an instance of Number
FAIL context.getParameter(context.UNIFORM_BUFFER_OFFSET_ALIGNMENT) should be >= 1. Was null (of type object).
FAIL context.getParameter(context.UNIFORM_BUFFER_OFFSET_ALIGNMENT) is not an instance of Number
FAIL context.getParameter(context.MAX_COMBINED_FRAGMENT_UNIFORM_COMPONENTS) is not an instance of Number
FAIL context.getParameter(context.MAX_COMBINED_VERTEX_UNIFORM_COMPONENTS) should be >= 4096. Was null (of type object).
FAIL context.getParameter(context.MAX_COMBINED_VERTEX_UNIFORM_COMPONENTS) is not an instance of Number
FAIL context.getError() should be 0. Was 1280.

Comment 1

[u480271]

Attachment: Part 1: Extract WebGL 2 specific pnames.

Comment 2

[u480271]

Attachment: Part 2: Be consistent when handling pnames from extensions.

Comment 3

[u480271]

Attachment: Part 3: Cleanup and better comments.

Comment 4

[u480271]

Attachment: Part 4: Pour in the WebGL 2 pnames.

Comment 5

[u480271]

Attachment: Part 5: Correctly load glGetInteger64v.

Correctly detect support for ARB_sync via GLFeature.

Comment 6

[u480271]

Attachment: Part 6: Implement Sampler binding tracking.

Comment 7

[u480271]

Attachment: Part 7: Implement MAX_CLIENT_WAIT_TIMEOUT_WEBGL. r=jgilbert, r=smaug

This is a special addition for WebGL 2. I've updated webidl to match the
spec.

I've set the value to be 0 to match Chrome. If I query the underlying
GL, I get -1 back. (On OSX). We can discuss what a better value would
be.

Comment 8

[u480271]

Attachment: Part 8: MAX_SERVER_WAIT_TIMEOUT is unsigned.

Comment 9

[u480271]

Attachment: Part 9: MAX_VARYING_COMPONENTS workaround.

Turns of querying MAX_VARYING_COMPONENTS on OS X 10.10 is buggy. Always
returns 1. The spec says that the value is 4 times MAX_VARYING_VECTORS
so work around using that method.

Comment 10

[u480271]

Attachment: Part A: Don't error on MAX_ELEMENT_INDEX.

MAX_ELEMENT_INDEX appears in GL 4.3 or via ES3_compatibility. Work
around on OSX 10.10 where max is GL 4.1.

Comment 11

[u480271]

Attachment: Part B: READ_BUFFER requires emulation for default FB.

WebGL internals use framebuffers to implement the default
framebuffer. This means that we can't just return the result from
glGetIntegerv(GL_READ_BUFFER, ...)

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8617038 [details] [diff] [review]
Part 7: Implement MAX_CLIENT_WAIT_TIMEOUT_WEBGL. r=jgilbert, r=smaug

r+ for the .webidl

Comment 13

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8617032 [details] [diff] [review]
Part 1: Extract WebGL 2 specific pnames.

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

::: dom/canvas/WebGLTimerQuery.h
@@ +8,5 @@
>  #define WEBGL_TIMER_QUERY_H_
>  
>  #include "nsWrapperCache.h"
>  #include "WebGLObjectModel.h"
> +#include "GLConsts.h"

Please keep these alpha-sorted.

Comment 14

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8617035 [details] [diff] [review]
Part 4: Pour in the WebGL 2 pnames.

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

::: dom/canvas/WebGL2ContextState.cpp
@@ +36,5 @@
> +      gl->fGetBooleanv(pname, &b);
> +      return JS::BooleanValue(bool(b));
> +    }
> +
> +      /* GLenum */

Keep the intent level for these comments consistent. (see GLboolean)

Comment 15

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8617038 [details] [diff] [review]
Part 7: Implement MAX_CLIENT_WAIT_TIMEOUT_WEBGL. r=jgilbert, r=smaug

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

::: dom/canvas/WebGL2ContextState.cpp
@@ +75,5 @@
>      }
>  
>        // GLint64
> +    case LOCAL_GL_MAX_CLIENT_WAIT_TIMEOUT_WEBGL:
> +      return JS::NumberValue(0); // TODO

I think we should allow up to UINT32_MAX here. It's TODO though, so maybe you changed it later.

::: gfx/gl/GLConsts.h
@@ +2278,5 @@
>  #define LOCAL_GL_MAX_ATOMIC_COUNTER_BUFFER_SIZE              0x92D8
>  #define LOCAL_GL_MAX_ATTRIB_STACK_DEPTH                      0x0D35
>  #define LOCAL_GL_MAX_BINDABLE_UNIFORM_SIZE_EXT               0x8DED
>  #define LOCAL_GL_MAX_CLIENT_ATTRIB_STACK_DEPTH               0x0D3B
> +#define LOCAL_GL_MAX_CLIENT_WAIT_TIMEOUT_WEBGL               0x9247

This is a generated file. We'll have to put this define elsewhere.
Since it's WebGL-specific, it should be able to sit in WebGLContext.h.

Comment 16

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8617039 [details] [diff] [review]
Part 8: MAX_SERVER_WAIT_TIMEOUT is unsigned.

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

::: dom/canvas/WebGL2ContextState.cpp
@@ -76,3 @@
>  
>        // GLint64
>      case LOCAL_GL_MAX_CLIENT_WAIT_TIMEOUT_WEBGL:

Shouldn't CLIENT_WAIT_TIMEOUT be unsigned if SERVER_WAIT_TIMEOUT is?

Comment 17

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8617042 [details] [diff] [review]
Part A: Don't error on MAX_ELEMENT_INDEX.

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

::: dom/canvas/WebGL2ContextState.cpp
@@ +88,5 @@
>      case LOCAL_GL_MAX_ELEMENT_INDEX:
> +      // GL_MAX_ELEMENT_INDEX becomes available in GL 4.3 or via ES3
> +      // compatibility
> +      if (!gl->IsSupported(gl::GLFeature::ES3_compatibility))
> +        return JS::DoubleValue(0.0);

JS::NumberValue(0), surely?

And shouldn't this have a default to something like UINT32_MAX?

Comment 18

[u480271]

(In reply to Jeff Gilbert [:jgilbert] from comment #16)
> Comment on attachment 8617039 [details] [diff] [review]
> Part 8: MAX_SERVER_WAIT_TIMEOUT is unsigned.
> 
> Review of attachment 8617039 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/canvas/WebGL2ContextState.cpp
> @@ -76,3 @@
> >  
> >        // GLint64
> >      case LOCAL_GL_MAX_CLIENT_WAIT_TIMEOUT_WEBGL:
> 
> Shouldn't CLIENT_WAIT_TIMEOUT be unsigned if SERVER_WAIT_TIMEOUT is?

GLuint64 for GL_SERVER_WAIT_TIMEOUT is how it's queried. It's returned as Number and, according to the WebGL2 spec, a GLint64. This could be an error in the spec or intended to convert 0xFFFFFFFFFFFFFFFF into -1 so it can be tested via JavaScript?

WebGL2 spec states MAX_CLIENT_WAIT_TIMEOUT_WEBGL is GLint64.

Comment 19

[u480271]

Attachment: Part C: Move _WEBGL GLenums from GLConsts.h to WebGLContext.h.

Comment 20

[u480271]

(In reply to Jeff Gilbert [:jgilbert] from comment #15)
> Comment on attachment 8617038 [details] [diff] [review]
> Part 7: Implement MAX_CLIENT_WAIT_TIMEOUT_WEBGL. r=jgilbert, r=smaug
> > +    case LOCAL_GL_MAX_CLIENT_WAIT_TIMEOUT_WEBGL:
> > +      return JS::NumberValue(0); // TODO

I'll implement the change from 0 to 0xFFFFFFFF in a follow up patch. (Since the timeout value will need to be checked in the client call.)

Comment 21

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d38fb3aea20a
https://hg.mozilla.org/integration/mozilla-inbound/rev/046422ea849f
https://hg.mozilla.org/integration/mozilla-inbound/rev/c666d71f4899
https://hg.mozilla.org/integration/mozilla-inbound/rev/4ee1883ffc65
https://hg.mozilla.org/integration/mozilla-inbound/rev/835570bc63bf
https://hg.mozilla.org/integration/mozilla-inbound/rev/cca48a1bc6ef
https://hg.mozilla.org/integration/mozilla-inbound/rev/1fd9140044be
https://hg.mozilla.org/integration/mozilla-inbound/rev/1fbb0ef29363
https://hg.mozilla.org/integration/mozilla-inbound/rev/ae7c769cb78d
https://hg.mozilla.org/integration/mozilla-inbound/rev/49284df5294e
https://hg.mozilla.org/integration/mozilla-inbound/rev/86e3fb3a2295
https://hg.mozilla.org/integration/mozilla-inbound/rev/47070f494c9e

Comment 22

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/862dd46b819f for Windows warning-as-error bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=10636567&repo=mozilla-inbound

Comment 23

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/16be2af03b54
https://hg.mozilla.org/integration/mozilla-inbound/rev/51731eb6855d
https://hg.mozilla.org/integration/mozilla-inbound/rev/29d6561e03cb
https://hg.mozilla.org/integration/mozilla-inbound/rev/f044d07d3996
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc75c2fe1b42
https://hg.mozilla.org/integration/mozilla-inbound/rev/b833745e5981
https://hg.mozilla.org/integration/mozilla-inbound/rev/9225d09342f2
https://hg.mozilla.org/integration/mozilla-inbound/rev/349b053438b3
https://hg.mozilla.org/integration/mozilla-inbound/rev/eb6b86397048
https://hg.mozilla.org/integration/mozilla-inbound/rev/dfa20a123ec4
https://hg.mozilla.org/integration/mozilla-inbound/rev/5b7031ee6b0d
https://hg.mozilla.org/integration/mozilla-inbound/rev/321a1148b604

Comment 24

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/16be2af03b54
https://hg.mozilla.org/mozilla-central/rev/51731eb6855d
https://hg.mozilla.org/mozilla-central/rev/29d6561e03cb
https://hg.mozilla.org/mozilla-central/rev/f044d07d3996
https://hg.mozilla.org/mozilla-central/rev/dc75c2fe1b42
https://hg.mozilla.org/mozilla-central/rev/b833745e5981
https://hg.mozilla.org/mozilla-central/rev/9225d09342f2
https://hg.mozilla.org/mozilla-central/rev/349b053438b3
https://hg.mozilla.org/mozilla-central/rev/eb6b86397048
https://hg.mozilla.org/mozilla-central/rev/dfa20a123ec4
https://hg.mozilla.org/mozilla-central/rev/5b7031ee6b0d
https://hg.mozilla.org/mozilla-central/rev/321a1148b604

Comment 25

[Florian Scholz (Open Web Docs)]

https://developer.mozilla.org/en-US/docs/Web/API/WebGLRenderingContext/getParameter#WebGL_2