-
Notifications
You must be signed in to change notification settings - Fork 171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Initial support for creating WebGL 2 contexts. #310
base: master
Are you sure you want to change the base?
Conversation
Would you be able to rebase this PR on the latest? |
This support is very incomplete and only works for a bare minimum of functionality. It hasn't been tested against the WebGL 2 CTS. To create a WebGL 2 context, use the "createWebGL2Context" attribute in the creation options struct. Implementation notes: - in native code, we use a single native class for both context types - we assign enum constants dynamically, so we replace gl.ENUM with this.ENUM in the context helper functions - only adds VAOs from the set of new WebGL 2 object types This contribution is funded by https://higharc.com/
9fe7d9d
to
31bde7c
Compare
Done, looks like it's passing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did an initial code review pass. Looks good. Thanks again for this effort
@@ -2878,14 +2943,22 @@ class WebGLRenderingContext extends NativeWebGLRenderingContext { | |||
data[0] = value[0] | |||
return super.vertexAttrib4f(index | 0, +value[0], +value[1], +value[2], +value[3]) | |||
} | |||
|
|||
_isWebGL2 () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self: this is a good candidate for a getter
@@ -5,6 +5,10 @@ class WebGLTextureUnit { | |||
this._mode = 0 | |||
this._bind2D = null | |||
this._bindCube = null | |||
if (ctx._isWebGL2) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because this is not a getter (yet), I suspect that this is always truthy
@@ -776,6 +836,15 @@ class WebGLRenderingContext extends NativeWebGLRenderingContext { | |||
return webGlTexture | |||
} | |||
|
|||
createVertexArray () { | |||
// TODO: do not expose VAO methods in WebGL 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To resolve this TODO, should these method be declared only on WebGL2RenderingContext
?
Or will it be tricky for devs to deal with the return type of createContext
and having to check if the context is a WebGL2RenderingContext
} | ||
|
||
setError (error) { | ||
// if (error !== 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK to remove?
TODO for myself: update README with experimental WebGL 2 support section |
This support is very incomplete and only works for a bare minimum of functionality. It hasn't been tested against the WebGL 2 CTS. To create a WebGL 2 context, use the "createWebGL2Context" attribute in the creation options struct.
Implementation notes:
This contribution is funded by https://higharc.com/
@dhritzkiv we're currently using this branch in our headless testing, and I can verify it works for the subset of functionality we have in our CI. At the moment this is the last of the work I had planned, but we'll probably need to add more support as we extend our testing coverage. One of the major hurdles would be migrating the gl-conformance library to run on top of the most recent CTS, and adding WebGL 2 CTS support.