Closed Bug 1170855 Opened 5 years ago Closed 5 years ago

WebGL 2 - Expose new state via gl.getParameter

Categories

(Core :: Canvas: WebGL, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: kamidphish, Assigned: kamidphish)

References

()

Details

(Keywords: dev-doc-complete, Whiteboard: [gfx-noted])

Attachments

(12 files)

12.93 KB, patch
jgilbert
: review+
Details | Diff | Splinter Review
5.41 KB, patch
jgilbert
: review+
Details | Diff | Splinter Review
4.08 KB, patch
jgilbert
: review+
Details | Diff | Splinter Review
6.10 KB, patch
jgilbert
: review+
Details | Diff | Splinter Review
3.39 KB, patch
jgilbert
: review+
Details | Diff | Splinter Review
6.18 KB, patch
jgilbert
: review+
Details | Diff | Splinter Review
4.10 KB, patch
smaug
: review+
jgilbert
: review-
Details | Diff | Splinter Review
1.45 KB, patch
jgilbert
: review+
Details | Diff | Splinter Review
2.14 KB, patch
jgilbert
: review+
Details | Diff | Splinter Review
1.24 KB, patch
jgilbert
: review+
Details | Diff | Splinter Review
1.37 KB, patch
jgilbert
: review+
Details | Diff | Splinter Review
7.25 KB, patch
jgilbert
: review+
Details | Diff | Splinter Review
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.
Assignee: nobody → dglastonbury
Status: NEW → ASSIGNED
Correctly detect support for ARB_sync via GLFeature.
Attachment #8617036 - Flags: review?(jgilbert)
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.
Attachment #8617038 - Flags: review?(jgilbert)
Attachment #8617038 - Flags: review?(bugs)
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.
Attachment #8617041 - Flags: review?(jgilbert)
MAX_ELEMENT_INDEX appears in GL 4.3 or via ES3_compatibility. Work
around on OSX 10.10 where max is GL 4.1.
Attachment #8617042 - Flags: review?(jgilbert)
WebGL internals use framebuffers to implement the default
framebuffer. This means that we can't just return the result from
glGetIntegerv(GL_READ_BUFFER, ...)
Attachment #8617043 - Flags: review?(jgilbert)
Comment on attachment 8617038 [details] [diff] [review]
Part 7: Implement MAX_CLIENT_WAIT_TIMEOUT_WEBGL. r=jgilbert, r=smaug

r+ for the .webidl
Attachment #8617038 - Flags: review?(bugs) → review+
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.
Attachment #8617032 - Flags: review?(jgilbert) → review+
Attachment #8617033 - Flags: review?(jgilbert) → review+
Attachment #8617034 - Flags: review?(jgilbert) → review+
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)
Attachment #8617035 - Flags: review?(jgilbert) → review+
Attachment #8617036 - Flags: review?(jgilbert) → review+
Attachment #8617037 - Flags: review?(jgilbert) → review+
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.
Attachment #8617038 - Flags: review?(jgilbert) → review-
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?
Attachment #8617039 - Flags: review?(jgilbert) → review+
Attachment #8617041 - Flags: review?(jgilbert) → review+
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?
Attachment #8617042 - Flags: review?(jgilbert) → review+
Attachment #8617043 - Flags: review?(jgilbert) → review+
(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.
Attachment #8617742 - Flags: review?(jgilbert) → review+
(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.)
You need to log in before you can comment on or make changes to this bug.