Fix immersive mode panic on three.js rollercoaster on hololens #24242

jdm · 2019-09-19T14:44:09Z

We have some special logic about texture formats when creating drawing buffers for WebGL that needs to be shared with the code that creates a separate framebuffer for immersive mode.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix #24083 and fix #20595.
These changes do not require tests because no CI for hololens; tested manually in the emulator.

This change is

highfive · 2019-09-19T14:44:14Z

Heads up! This PR modifies the following files:

@asajeffrey: components/script/dom/webglframebuffer.rs, components/script/dom/webglrenderingcontext.rs, components/script/dom/xrwebgllayer.rs, components/script/dom/bindings/trace.rs
@KiChjang: components/script/dom/webglframebuffer.rs, components/script/dom/webglrenderingcontext.rs, components/script/dom/xrwebgllayer.rs, components/script/dom/bindings/trace.rs

highfive · 2019-09-19T14:44:16Z

Warning

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

jdm · 2019-09-19T14:44:34Z

r? @asajeffrey

asajeffrey

I'm a bit uncomfortable that this is causing panic, since AFAICT all of the original code could be replicated in user WebGL. That said, this is a definite improvement. @bors-servo r+

asajeffrey · 2019-09-19T15:14:25Z

Ah, does bors not listen to review comments? @bors-servo r+

bors-servo · 2019-09-19T15:14:26Z

📌 Commit 38351d5 has been approved by asajeffrey

bors-servo · 2019-09-19T16:14:14Z

⌛ Testing commit 38351d5 with merge 4c90df7...


        Auto merge of #24242 - jdm:xr-webgllayer-format, r=asajeffrey

Fix immersive mode panic on three.js rollercoaster on hololens We have some special logic about texture formats when creating drawing buffers for WebGL that needs to be shared with the code that creates a separate framebuffer for immersive mode. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #24083 (GitHub issue number if applicable) - [x] These changes do not require tests because no CI for hololens; tested manually in the emulator.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/24242)

jdm · 2019-09-19T16:14:20Z

That's a good point. I should put together some testcases to experiment; filed #24244 to do that.

bors-servo · 2019-09-19T16:41:11Z

💔 Test failed - linux-rel-css

jdm · 2019-09-19T21:46:40Z

@bors-servo try=wpt

bors-servo · 2019-09-19T21:46:45Z

⌛ Trying commit 4edec82 with merge c7ad5bc...


        Auto merge of #24242 - jdm:xr-webgllayer-format, r=<try>

Fix immersive mode panic on three.js rollercoaster on hololens We have some special logic about texture formats when creating drawing buffers for WebGL that needs to be shared with the code that creates a separate framebuffer for immersive mode. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #24083 (GitHub issue number if applicable) - [x] These changes do not require tests because no CI for hololens; tested manually in the emulator.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/24242)

jdm · 2019-09-19T21:47:40Z

r? @asajeffrey

The addition of RGB to the framebuffer completion checks exposed some additional implementation holes which were easy enough to clean up while I was investigating them.

asajeffrey

LGTM

asajeffrey · 2019-09-19T23:03:13Z

tests/wpt/webgl/meta/conformance/extensions/oes-texture-half-float.html.ini

-    expected: FAIL
-
-  [WebGL test #82: getError expected: INVALID_OPERATION. Was NO_ERROR : IMPLEMENTATION_COLOR_READ_FORMAT should fail for incomplete framebuffers.]
-    expected: FAIL


jdm · 2019-09-23T17:12:57Z

@bors-servo retry

jdm · 2019-09-23T17:19:24Z

@bors-servo retry

jdm · 2019-09-23T17:20:10Z

@bors-servo r-

jdm · 2019-09-23T17:20:24Z

@bors-servo r=nox,asajeffrey

bors-servo · 2019-09-23T17:20:25Z

📌 Commit ce12723 has been approved by nox,asajeffrey

jdm · 2019-09-23T17:56:53Z

@bors-servo r-

jdm · 2019-09-23T17:58:44Z

@bors-servo r=asajeffrey,nox

bors-servo · 2019-09-23T17:58:46Z

📌 Commit ea715a7 has been approved by asajeffrey,nox

bors-servo · 2019-09-23T18:24:47Z

⌛ Testing commit ea715a7 with merge 8c55623...


        Auto merge of #24242 - jdm:xr-webgllayer-format, r=asajeffrey,nox

Fix immersive mode panic on three.js rollercoaster on hololens We have some special logic about texture formats when creating drawing buffers for WebGL that needs to be shared with the code that creates a separate framebuffer for immersive mode. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #24083 and fix #20595. - [x] These changes do not require tests because no CI for hololens; tested manually in the emulator.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/24242)

bors-servo · 2019-09-23T21:00:22Z

💔 Test failed - status-taskcluster

jdm · 2019-09-23T21:50:53Z

@bors-servo retry

bors-servo · 2019-09-23T23:34:12Z

⌛ Testing commit ea715a7 with merge 6ca62aa...


        Auto merge of #24242 - jdm:xr-webgllayer-format, r=asajeffrey,nox

Fix immersive mode panic on three.js rollercoaster on hololens We have some special logic about texture formats when creating drawing buffers for WebGL that needs to be shared with the code that creates a separate framebuffer for immersive mode. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #24083 and fix #20595. - [x] These changes do not require tests because no CI for hololens; tested manually in the emulator.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/24242)

bors-servo · 2019-09-24T02:39:48Z

☀️ Test successful - linux-rel-css, linux-rel-wpt, status-taskcluster
Approved by: asajeffrey,nox
Pushing 6ca62aa to master...

highfive assigned emilio Sep 19, 2019

highfive added the S-awaiting-review label Sep 19, 2019

highfive assigned asajeffrey and unassigned emilio Sep 19, 2019

asajeffrey approved these changes Sep 19, 2019

View changes

highfive added S-awaiting-merge and removed S-awaiting-review labels Sep 19, 2019

jdm mentioned this pull request Sep 19, 2019

Creating webgl textures with RGB or RGBA formats can cause GL errors #24244

Open

highfive added S-tests-failed and removed S-awaiting-merge labels Sep 19, 2019

jdm force-pushed the jdm:xr-webgllayer-format branch from 38351d5 to f6727ed Sep 19, 2019

highfive added S-awaiting-review and removed S-tests-failed labels Sep 19, 2019

jdm force-pushed the jdm:xr-webgllayer-format branch from f6727ed to 4edec82 Sep 19, 2019

jdm force-pushed the jdm:xr-webgllayer-format branch 2 times, most recently from d65be06 to 368e030 Sep 19, 2019

asajeffrey approved these changes Sep 19, 2019

View changes

highfive added the S-awaiting-merge label Sep 23, 2019

highfive assigned nox and unassigned asajeffrey Sep 23, 2019

jdm closed this Sep 23, 2019

jdm reopened this Sep 23, 2019

highfive assigned asajeffrey and unassigned nox Sep 23, 2019

highfive added S-tests-failed and removed S-awaiting-merge labels Sep 23, 2019

highfive added S-awaiting-merge and removed S-tests-failed labels Sep 23, 2019

bors-servo merged commit ea715a7 into servo:master Sep 24, 2019
1 of 3 checks passed

1 of 3 checks passed

Taskcluster (pull_request) TaskGroup: failure
Details

continuous-integration/appveyor/pr AppVeyor build failed
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Sep 24, 2019

This was referenced Sep 24, 2019

Initial implementation of WebGLSync #24250

Merged

Initial implementation of WebGLQueries #24178

Merged

servo / servo

Fix immersive mode panic on three.js rollercoaster on hololens #24242

Fix immersive mode panic on three.js rollercoaster on hololens #24242

jdm commented Sep 19, 2019 •

edited

highfive commented Sep 19, 2019

highfive commented Sep 19, 2019

jdm commented Sep 19, 2019

asajeffrey left a comment

asajeffrey commented Sep 19, 2019

bors-servo commented Sep 19, 2019

bors-servo commented Sep 19, 2019

jdm commented Sep 19, 2019

bors-servo commented Sep 19, 2019

jdm commented Sep 19, 2019

bors-servo commented Sep 19, 2019

jdm commented Sep 19, 2019

asajeffrey left a comment

This comment has been minimized.

jdm commented Sep 23, 2019

jdm commented Sep 23, 2019

jdm commented Sep 23, 2019

jdm commented Sep 23, 2019

bors-servo commented Sep 23, 2019

jdm commented Sep 23, 2019

jdm commented Sep 23, 2019

bors-servo commented Sep 23, 2019

bors-servo commented Sep 23, 2019

bors-servo commented Sep 23, 2019

jdm commented Sep 23, 2019

bors-servo commented Sep 23, 2019

bors-servo commented Sep 24, 2019

servo / servo

Join GitHub today

Fix immersive mode panic on three.js rollercoaster on hololens #24242

Fix immersive mode panic on three.js rollercoaster on hololens #24242

Conversation

jdm commented Sep 19, 2019 • edited

highfive commented Sep 19, 2019

highfive commented Sep 19, 2019

jdm commented Sep 19, 2019

asajeffrey left a comment

asajeffrey commented Sep 19, 2019

bors-servo commented Sep 19, 2019

bors-servo commented Sep 19, 2019

jdm commented Sep 19, 2019

bors-servo commented Sep 19, 2019

jdm commented Sep 19, 2019

bors-servo commented Sep 19, 2019

jdm commented Sep 19, 2019

asajeffrey left a comment

This comment has been minimized.

asajeffrey Sep 19, 2019

jdm commented Sep 23, 2019

jdm commented Sep 23, 2019

jdm commented Sep 23, 2019

jdm commented Sep 23, 2019

bors-servo commented Sep 23, 2019

jdm commented Sep 23, 2019

jdm commented Sep 23, 2019

bors-servo commented Sep 23, 2019

bors-servo commented Sep 23, 2019

bors-servo commented Sep 23, 2019

jdm commented Sep 23, 2019

bors-servo commented Sep 23, 2019

bors-servo commented Sep 24, 2019

jdm commented Sep 19, 2019 •

edited