Kick off WebGL 2.0 implementation #19028

MortimerGoro · 2017-10-26T16:40:11Z

This PR kicks off the WebGL 2.0 implementation:

Include WebGL2RenderingContext.webidl and comment unimplemented methods
Create WebGL2 struct hierarchy with WebGL 1.0 backwards compatibility
Add WebGL 2.0 entry points to canvas
Select the correct GL Version on GLContext backends (related PR Add support for selecting OpenGL/ES versions surfman#108)
Add WebGL version selection in shader compilations
Create a WebGL 2.0 preference

I tried a complex three.js demo using canvas.getContext("webgl2") and the backwards compatibility worked great.

Next steps:

I'll add WebGLVersion selection/filtering to WebGLExtensions and move some extensions to core in WebGL 2.0 (e.g. VAOs)
I'll add the WebGL 2.0 conformance WPT
I'll create a mega-issue with a the list of all TODO methods for a complete WebGL 2.0 implementation (as @emilio did with WebGL 1.0), so we can start start getting community involvement.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix #__ (github issue number if applicable).

There are tests for these changes OR
These changes do not require tests because I will add the entire webgl 2.0 WPT in a different PR

This change is

highfive · 2017-10-26T16:40:16Z

Heads up! This PR modifies the following files:

@KiChjang: components/script/dom/bindings/codegen/CodegenRust.py, components/script/dom/webglrenderingcontext.rs, components/script/dom/webgl2renderingcontext.rs, components/script/dom/webidls/WebGL2RenderingContext.webidl, components/script/dom/webglshader.rs and 4 more
@fitzgen: components/script/dom/bindings/codegen/CodegenRust.py, components/script/dom/webglrenderingcontext.rs, components/script/dom/webgl2renderingcontext.rs, components/script/dom/webidls/WebGL2RenderingContext.webidl, components/script/dom/webglshader.rs and 4 more
@emilio: components/canvas/webgl_thread.rs, components/script/dom/webglrenderingcontext.rs, components/script/dom/webgl2renderingcontext.rs, components/script/dom/webglshader.rs

highfive · 2017-10-26T16:40:17Z

Warning

These commits modify unsafe code. Please review it carefully!
These commits modify script code, but no tests are modified. Please consider adding a test!

emilio

Huh, it was way simpler to review than what I expected!

r=me, with the nits

emilio · 2017-10-26T17:20:52Z

components/script/dom/htmlcanvaselement.rs

+
+    #[allow(unsafe_code)]
+    fn get_gl_attributes(cx: *mut JSContext, attrs: Option<HandleValue>) -> Option<GLContextAttributes> {
+        if let Some(webgl_attributes) = attrs {


nit: This can read nicer like:

let webgl_attributes = match attrs { Some(attrs) => attrs, None => return Some(GlContextAttributes::default()), }; // ...

emilio · 2017-10-26T17:23:16Z

components/script/dom/webgl2renderingcontext.rs

+}
+
+impl WebGL2RenderingContext {
+    fn new_inherited(window: &Window,


nit: For new code, I think it's preferable to use block indentation for arguments and such, like:

fn new_inherited( window: &Window, canvas: &HTMLCanvasElement, size: Size2D<i32>, attrs: GLContextAttributes, ) -> Option<Self> {

This applies to other parts of the patch, feel free to reformat the signatures you're touching too if you want.

emilio · 2017-10-26T17:24:00Z

components/script/dom/webgl2renderingcontext.rs

+}
+
+impl WebGL2RenderingContextMethods for WebGL2RenderingContext {
+    // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.1


nit: I'd use doc comments (/// instead of //), but no big deal.

emilio · 2017-10-26T17:25:20Z

components/script/dom/webgl2renderingcontext.rs

+    #[allow(unsafe_code)]
+    // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.5
+    unsafe fn BufferData(&self, cx: *mut JSContext, target: u32, data: *mut JSObject, usage: u32) -> Fallible<()> {
+        self.base.BufferData(cx, target, data, usage)


Ugh, it'd be nice to forward all these a bit more easily :(.

Can you file an issue to maybe generate macros from CodegenBinding.py or something like that? Not sure if it's common enough to be worth it, but...

I didn't find this pattern in another API. WebGL 2.0 WebIDL implements both interfaces instead of using inheritance.

At least I generated all this stuff with a local script ;) I'll open that issue to see if it's worth for more APIs

emilio · 2017-10-26T17:33:24Z

@bors-servo delegate+

bors-servo · 2017-10-26T17:33:25Z

✌️ @MortimerGoro can now approve this pull request

MortimerGoro · 2017-10-26T19:23:51Z

@bors-servo r=emilio

bors-servo · 2017-10-26T19:23:51Z

📌 Commit 379a6a0 has been approved by emilio

bors-servo · 2017-10-26T20:05:59Z

⌛ Testing commit 379a6a0 with merge cef0baf...

@emilio

Kick off WebGL 2.0 implementation  This PR kicks off the WebGL 2.0 implementation: - Include WebGL2RenderingContext.webidl and comment unimplemented methods - Create WebGL2 struct hierarchy with WebGL 1.0 backwards compatibility - Add WebGL 2.0 entry points to canvas - Select the correct GL Version on GLContext backends (related PR servo/surfman#108) - Add WebGL version selection in shader compilations - Create a WebGL 2.0 preference I tried a complex three.js demo using canvas.getContext("webgl2") and the backwards compatibility worked great. Next steps: - I'll add WebGLVersion selection/filtering to WebGLExtensions and move some extensions to core in WebGL 2.0 (e.g. VAOs) - I'll add the WebGL 2.0 conformance WPT - I'll create a mega-issue with a the list of all TODO methods for a complete WebGL 2.0 implementation (as @emilio did with WebGL 1.0), so we can start start getting community involvement. ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable).  - [ ] There are tests for these changes OR - [x] These changes do not require tests because I will add the entire webgl 2.0 WPT in a different PR    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19028)

bors-servo · 2017-10-26T20:44:52Z

💔 Test failed - mac-rel-wpt3

MortimerGoro · 2017-10-27T10:55:54Z

Test failures fixed: It was just due to a invalid CanvasContext::WebGL2 enum set in HTMLCanvasElementContext when creating a WebGL1 context.

@bors-servo r=emilio

bors-servo · 2017-10-27T10:55:54Z

📌 Commit ddd6c86 has been approved by emilio

bors-servo · 2017-10-27T10:55:59Z

⌛ Testing commit ddd6c86 with merge c007194...

@emilio

Kick off WebGL 2.0 implementation  This PR kicks off the WebGL 2.0 implementation: - Include WebGL2RenderingContext.webidl and comment unimplemented methods - Create WebGL2 struct hierarchy with WebGL 1.0 backwards compatibility - Add WebGL 2.0 entry points to canvas - Select the correct GL Version on GLContext backends (related PR servo/surfman#108) - Add WebGL version selection in shader compilations - Create a WebGL 2.0 preference I tried a complex three.js demo using canvas.getContext("webgl2") and the backwards compatibility worked great. Next steps: - I'll add WebGLVersion selection/filtering to WebGLExtensions and move some extensions to core in WebGL 2.0 (e.g. VAOs) - I'll add the WebGL 2.0 conformance WPT - I'll create a mega-issue with a the list of all TODO methods for a complete WebGL 2.0 implementation (as @emilio did with WebGL 1.0), so we can start start getting community involvement. ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable).  - [ ] There are tests for these changes OR - [x] These changes do not require tests because I will add the entire webgl 2.0 WPT in a different PR    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19028)

bors-servo · 2017-10-27T10:56:09Z

💡 This pull request was already approved, no need to approve it again.

This pull request is currently being tested. If there's no response from the continuous integration service, you may use retry to trigger a build again.

bors-servo · 2017-10-27T10:56:09Z

📌 Commit ddd6c86 has been approved by emilio

bors-servo · 2017-10-27T10:56:14Z

⌛ Testing commit ddd6c86 with merge d21657a...

@emilio

Kick off WebGL 2.0 implementation  This PR kicks off the WebGL 2.0 implementation: - Include WebGL2RenderingContext.webidl and comment unimplemented methods - Create WebGL2 struct hierarchy with WebGL 1.0 backwards compatibility - Add WebGL 2.0 entry points to canvas - Select the correct GL Version on GLContext backends (related PR servo/surfman#108) - Add WebGL version selection in shader compilations - Create a WebGL 2.0 preference I tried a complex three.js demo using canvas.getContext("webgl2") and the backwards compatibility worked great. Next steps: - I'll add WebGLVersion selection/filtering to WebGLExtensions and move some extensions to core in WebGL 2.0 (e.g. VAOs) - I'll add the WebGL 2.0 conformance WPT - I'll create a mega-issue with a the list of all TODO methods for a complete WebGL 2.0 implementation (as @emilio did with WebGL 1.0), so we can start start getting community involvement. ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable).  - [ ] There are tests for these changes OR - [x] These changes do not require tests because I will add the entire webgl 2.0 WPT in a different PR    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19028)

bors-servo · 2017-10-27T12:28:12Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
Approved by: emilio
Pushing d21657a to master...

highfive assigned metajack Oct 26, 2017

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Oct 26, 2017

emilio approved these changes Oct 26, 2017

View reviewed changes

MortimerGoro force-pushed the webgl2 branch from ea15546 to 379a6a0 Compare October 26, 2017 19:17

highfive assigned emilio and unassigned metajack Oct 26, 2017

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Oct 26, 2017

highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Oct 26, 2017

Kick off WebGL 2.0 implementation

ddd6c86

MortimerGoro force-pushed the webgl2 branch from 379a6a0 to ddd6c86 Compare October 27, 2017 10:53

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Oct 27, 2017

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Oct 27, 2017

bors-servo merged commit ddd6c86 into servo:master Oct 27, 2017

highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 27, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kick off WebGL 2.0 implementation #19028

Kick off WebGL 2.0 implementation #19028

MortimerGoro commented Oct 26, 2017 •

edited

highfive commented Oct 26, 2017

highfive commented Oct 26, 2017

emilio left a comment

emilio Oct 26, 2017

emilio Oct 26, 2017

emilio Oct 26, 2017

emilio Oct 26, 2017

MortimerGoro Oct 26, 2017 •

edited

emilio commented Oct 26, 2017

bors-servo commented Oct 26, 2017

MortimerGoro commented Oct 26, 2017

bors-servo commented Oct 26, 2017

bors-servo commented Oct 26, 2017

bors-servo commented Oct 26, 2017

MortimerGoro commented Oct 27, 2017 •

edited

bors-servo commented Oct 27, 2017

bors-servo commented Oct 27, 2017

bors-servo commented Oct 27, 2017

bors-servo commented Oct 27, 2017

bors-servo commented Oct 27, 2017

bors-servo commented Oct 27, 2017

Kick off WebGL 2.0 implementation #19028

Kick off WebGL 2.0 implementation #19028

Conversation

MortimerGoro commented Oct 26, 2017 • edited

highfive commented Oct 26, 2017

highfive commented Oct 26, 2017

emilio left a comment

Choose a reason for hiding this comment

emilio Oct 26, 2017

Choose a reason for hiding this comment

emilio Oct 26, 2017

Choose a reason for hiding this comment

emilio Oct 26, 2017

Choose a reason for hiding this comment

emilio Oct 26, 2017

Choose a reason for hiding this comment

MortimerGoro Oct 26, 2017 • edited

Choose a reason for hiding this comment

emilio commented Oct 26, 2017

bors-servo commented Oct 26, 2017

MortimerGoro commented Oct 26, 2017

bors-servo commented Oct 26, 2017

bors-servo commented Oct 26, 2017

bors-servo commented Oct 26, 2017

MortimerGoro commented Oct 27, 2017 • edited

bors-servo commented Oct 27, 2017

bors-servo commented Oct 27, 2017

bors-servo commented Oct 27, 2017

bors-servo commented Oct 27, 2017

bors-servo commented Oct 27, 2017

bors-servo commented Oct 27, 2017

MortimerGoro commented Oct 26, 2017 •

edited

MortimerGoro Oct 26, 2017 •

edited

MortimerGoro commented Oct 27, 2017 •

edited