Support running WebGL in its own thread or on the main thread. #23777

jdm · 2019-07-15T17:00:52Z

This is the final missing piece to support WebGL in ANGLE on Windows. ANGLE doesn't support multiple GL contexts on separate threads using the same underlying Direct3d device, so we need to process all GL operations for WebGL on the same thread as the compositor. These changes try to retain enough flexibility to support both approaches so we can get WebGL working on Windows ASAP.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix #23697
There are tests for these changes

This change is

jdm · 2019-07-15T17:01:03Z

@bors-servo try=wpt

bors-servo · 2019-07-15T17:01:08Z

⌛ Trying commit 12e1fd0 with merge dc95e3b...


        Auto merge of #23777 - jdm:webgl-main-thread, r=<try>

Support running WebGL in its own thread or on the main thread. This is the final missing piece to support WebGL in ANGLE on Windows. ANGLE doesn't support multiple GL contexts on separate threads using the same underlying Direct3d device, so we need to process all GL operations for WebGL on the same thread as the compositor. These changes try to retain enough flexibility to support both approaches so we can get WebGL working on Windows ASAP. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #23697 - [x] There are tests for these changes  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23777)

jdm · 2019-07-15T17:09:16Z

One missing piece - I totally stubbed out the main-thread support for dom2texture in OutputHandler.

bors-servo · 2019-07-15T17:13:29Z

💔 Test failed - linux-rel-wpt

jdm · 2019-07-15T17:25:50Z

@bors-servo try=wpt

bors-servo · 2019-07-15T17:25:54Z

⌛ Trying commit f069ede with merge 82b8d50...


        Auto merge of #23777 - jdm:webgl-main-thread, r=<try>

Support running WebGL in its own thread or on the main thread. This is the final missing piece to support WebGL in ANGLE on Windows. ANGLE doesn't support multiple GL contexts on separate threads using the same underlying Direct3d device, so we need to process all GL operations for WebGL on the same thread as the compositor. These changes try to retain enough flexibility to support both approaches so we can get WebGL working on Windows ASAP. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #23697 - [x] There are tests for these changes  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23777)

bors-servo · 2019-07-15T17:53:45Z

💔 Test failed - linux-rel-css

jdm · 2019-07-15T18:03:09Z

With the off thread tests, lots of gl errors like:

  │ Caught GL error 501 at push_debug_group_khr (thread main, at /home/servo/.cargo/git/checkouts/webrender-c3596abe1cf4f320/2afa0c3/webrender/src/device/gl.rs:1216)
  │ stack backtrace:
  │    0: servo::main::{{closure}}::h814a232d73f86db7 (0x5557b69ee31f)
  │    1: std::panicking::rust_panic_with_hook::h67a63e964d7bd76b (0x5557b9efc188)
  │              at src/libstd/panicking.rs:481
  │    2: std::panicking::continue_panic_fmt::hc2aa9083bcd913bc (0x5557b9efbc21)
  │              at src/libstd/panicking.rs:384
  │    3: std::panicking::begin_panic_fmt::ha59a4c464bc3bdc0 (0x5557b9efbb6e)
  │              at src/libstd/panicking.rs:339
  │    4: webrender::device::gl::Device::new::{{closure}}::hb1efa06e598cbe12 (0x5557b96f2261)
  │    5: <gleam::gl::ErrorReactingGl<F> as gleam::gl::Gl>::push_debug_group_khr::h01ef990601e5a0a0 (0x5557b96eb399)
  │    6: webrender::device::query_gl::GpuMarker::new::h40babc6a64c1be1e (0x5557b95cc4da)
  │    7: webrender::renderer::Renderer::update_gpu_cache::h512fb02ddeefc75a (0x5557b9556c18)
  │    8: webrender::renderer::Renderer::prepare_gpu_cache::h909d8b8a4c09cfd4 (0x5557b9558c18)
  │    9: webrender::profiler::TimeProfileCounter::profile::hbe92c3795fb6a754 (0x5557b958346a)
  │   10: webrender::renderer::Renderer::render_impl::h519e68dfebc29026 (0x5557b9555845)
  │   11: webrender::renderer::Renderer::render::h43504c71ac357f67 (0x5557b95551b2)
  │   12: profile_traits::time::profile::h40823dc5fc42ceb6 (0x5557b69f2e58)
  │   13: compositing::compositor::IOCompositor<Window>::composite_specific_target::h0e1544528da6cae5 (0x5557b6a261d9)
  │   14: compositing::compositor::IOCompositor<Window>::composite::hec72686aff94ecf8 (0x5557b6a2895d)
  │   15: compositing::compositor::IOCompositor<Window>::perform_updates::he19cbf6868b2469f (0x5557b6a20562)
  │   16: servo::Servo<Window>::handle_events::hed4c5bc49dd401b1 (0x5557b6a41f38)
  │   17: servo::app::App::handle_events::h8544f2dc7730ac83 (0x5557b6a1f407)
  │   18: servo::app::App::run::hefdc417524fc33c6 (0x5557b6a1e2f7)
  │   19: servo::main::h7ca501accaf2eb58 (0x5557b69edc7d)
  │   20: std::rt::lang_start::{{closure}}::he93334fd8f41eb00 (0x5557b69e4f42)
  │   21: std::rt::lang_start_internal::{{closure}}::hf6cbc52017534aef (0x5557b9efbaa2)
  │              at src/libstd/rt.rs:49
  │       std::panicking::try::do_call::h536274ef681e761e
  │              at src/libstd/panicking.rs:296
  │   22: __rust_maybe_catch_panic (0x5557b9f05e49)
  │              at src/libpanic_unwind/lib.rs:82
  │   23: std::panicking::try::h1331b216c5aa4938 (0x5557b9efc66c)
  │              at src/libstd/panicking.rs:275
  │       std::panic::catch_unwind::h79dc7bfdf5e927ad
  │              at src/libstd/panic.rs:394
  │       std::rt::lang_start_internal::h28351dfa024a5642
  │              at src/libstd/rt.rs:48
  │   24: main (0x5557b69ee4e1)
  │   25: __libc_start_main (0x7fdcfed9482f)
  │   26: _start (0x5557b695ce28)
  │   27: <unknown> (0x0)

With the main thread tests, lots of gl errors like:

 │ Caught GL error 502 at bind_vertex_array (thread main, at /home/servo/.cargo/git/checkouts/webrender-c3596abe1cf4f320/2afa0c3/webrender/src/device/gl.rs:1216)
  │ stack backtrace:
  │    0: servo::main::{{closure}}::h814a232d73f86db7 (0x55e57863f31f)
  │    1: std::panicking::rust_panic_with_hook::h67a63e964d7bd76b (0x55e57bb4d188)
  │              at src/libstd/panicking.rs:481
  │    2: std::panicking::continue_panic_fmt::hc2aa9083bcd913bc (0x55e57bb4cc21)
  │              at src/libstd/panicking.rs:384
  │    3: std::panicking::begin_panic_fmt::ha59a4c464bc3bdc0 (0x55e57bb4cb6e)
  │              at src/libstd/panicking.rs:339
  │    4: webrender::device::gl::Device::new::{{closure}}::hb1efa06e598cbe12 (0x55e57b343261)
  │    5: <gleam::gl::ErrorReactingGl<F> as gleam::gl::Gl>::bind_vertex_array::he9c1988337fb6068 (0x55e57b33ac49)
  │    6: webrender::renderer::Renderer::draw_instanced_batch::h70e9e2ea2debdf63 (0x55e57b1aac7a)
  │    7: webrender::renderer::Renderer::draw_alpha_batch_container::hfba585142882d4e3 (0x55e57b1ac450)
  │    8: webrender::renderer::Renderer::draw_color_target::h2f47328902c08afe (0x55e57b1aeac8)
  │    9: webrender::renderer::Renderer::draw_tile_frame::h94dcb50da9df0cec (0x55e57b1b1f4e)
  │   10: webrender::profiler::TimeProfileCounter::profile::hbe92c3795fb6a754 (0x55e57b1d44b2)
  │   11: webrender::renderer::Renderer::render_impl::h519e68dfebc29026 (0x55e57b1a6845)
  │   12: webrender::renderer::Renderer::render::h43504c71ac357f67 (0x55e57b1a61b2)
  │   13: profile_traits::time::profile::h40823dc5fc42ceb6 (0x55e578643e58)
  │   14: compositing::compositor::IOCompositor<Window>::composite_specific_target::h0e1544528da6cae5 (0x55e5786771d9)
  │   15: compositing::compositor::IOCompositor<Window>::composite::hec72686aff94ecf8 (0x55e57867995d)
  │   16: compositing::compositor::IOCompositor<Window>::perform_updates::he19cbf6868b2469f (0x55e578671562)
  │   17: servo::Servo<Window>::handle_events::hed4c5bc49dd401b1 (0x55e578692f38)
  │   18: servo::app::App::handle_events::h8544f2dc7730ac83 (0x55e578670407)
  │   19: servo::app::App::run::hefdc417524fc33c6 (0x55e57866f2f7)
  │   20: servo::main::h7ca501accaf2eb58 (0x55e57863ec7d)
  │   21: std::rt::lang_start::{{closure}}::he93334fd8f41eb00 (0x55e578635f42)
  │   22: std::rt::lang_start_internal::{{closure}}::hf6cbc52017534aef (0x55e57bb4caa2)
  │              at src/libstd/rt.rs:49
  │       std::panicking::try::do_call::h536274ef681e761e
  │              at src/libstd/panicking.rs:296
  │   23: __rust_maybe_catch_panic (0x55e57bb56e49)
  │              at src/libpanic_unwind/lib.rs:82
  │   24: std::panicking::try::h1331b216c5aa4938 (0x55e57bb4d66c)
  │              at src/libstd/panicking.rs:275
  │       std::panic::catch_unwind::h79dc7bfdf5e927ad
  │              at src/libstd/panic.rs:394
  │       std::rt::lang_start_internal::h28351dfa024a5642
  │              at src/libstd/rt.rs:48
  │   25: main (0x55e57863f4e1)
  │   26: __libc_start_main (0x7f8eeb57182f)
  │   27: _start (0x55e5785ade28)
  │   28: <unknown> (0x0)
  │ [2019-07-15T17:53:18Z ERROR servo] Caught GL error 502 at bind_vertex_array
  │ Stack trace for thread "main"
  │ stack backtrace:
  │    0: servo::install_crash_handler::handler::h55db92dc8b8683fa (0x55e57863e5b6)
  │    1: _ZL15WasmTrapHandleriP9siginfo_tPv (0x55e57a2710fe)
  │              at /home/servo/.cargo/git/checkouts/mozjs-fc2b6454b6e5027a/5584728/mozjs/js/src/wasm/WasmSignalHandlers.cpp:967
  │    2: <unknown> (0x7f8eecdc738f)
  │    3: _ZN4core3ptr18real_drop_in_place17h18651eda418c5124E.llvm.3806932873906869800 (0x55e57b183341)
  │    4: webrender::renderer::Renderer::draw_color_target::h2f47328902c08afe (0x55e57b1af4b8)
  └    5: <unknown> (0x7f8ed4c3ba87)

jdm · 2019-07-17T15:40:39Z

@bors-servo try=wpt

bors-servo · 2019-07-17T15:40:44Z

⌛ Trying commit 8b91853 with merge 5b2ae1c...


        Auto merge of #23777 - jdm:webgl-main-thread, r=<try>

Support running WebGL in its own thread or on the main thread. This is the final missing piece to support WebGL in ANGLE on Windows. ANGLE doesn't support multiple GL contexts on separate threads using the same underlying Direct3d device, so we need to process all GL operations for WebGL on the same thread as the compositor. These changes try to retain enough flexibility to support both approaches so we can get WebGL working on Windows ASAP. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #23697 - [x] There are tests for these changes  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23777)

bors-servo · 2019-07-17T15:53:44Z

💔 Test failed - linux-rel-css

jdm · 2019-07-17T15:58:18Z

@bors-servo try=wpt

pcwalton · 2019-07-24T21:16:02Z

Would you like me to review this?

asajeffrey

Sigh, this is adding yet more complexity to the main event loop, but there's not much we can do about that. My main concern is the extra thread that's being spawned, that wakes the main event loop up for each GL command. That's a lot of context switching and message forwarding!

Also, doesn't the WebGL thread keep track of the current GL context? Isn't it going to be rather surprised if the current GL context isn't the one it expects?

asajeffrey · 2019-07-25T00:02:30Z

components/canvas/webgl_thread.rs

+        // Interpose a new channel in between the existing WebGL channel endpoints.
+        // This will bounce all WebGL messages through a second thread adding a small
+        // delay, but this will also ensure that the main thread will wake up and
+        // process the WebGL message when it arrives.


Oh that's annoying. It introduces yet another thread, which is context switched in for every GL command, plus it calls the event loop waker on every GL command. This seems quite expensive.

Can we not move this logic into webgl, so the event loop waker is only used on commands like gl.finish()? (Similar to how the canvas is dirtied at the moment).

asajeffrey · 2019-07-25T00:02:30Z

components/canvas_traits/webgl_channel/mod.rs

@@ -71,6 +71,13 @@ where
            WebGLReceiver::Mpsc(ref receiver) => receiver.recv().map_err(|_| ()),
        }
    }
+
+    pub fn try_recv(&self) -> Result<T, ()> {


This version of try_recv doesn't distinguish between failure due to no message being present, and failure due to the channel being closed. Annoying, but possibly inevitable, unless there's some secret ipc-fu.

asajeffrey · 2019-07-25T15:12:57Z

IRC chat: https://mozilla.logbot.info/servo/20190725#c16494926

jdm · 2019-07-26T00:45:25Z

@asajeffrey Done! I tested all four permutations of ipc/main-thread and everything works. I need to rebase to pick up the clone->clone_box changes, but this is ready for review.

asajeffrey · 2019-07-26T01:11:09Z

Okay, this looks good. The only thing I think I'd do differently is make the event loop waker non-optional. At this point you can r=me.


        Support running WebGL in its own thread or on the main thread.


        Make headless window set current GL context before compositing.


        Support waking up headless window event loops while they are sleeping.


        Mark a bunch of webgl tests as long-running.


        Remove extra webgl message pumping thread.

jdm · 2019-07-26T03:15:09Z

@bors-servo r=asajeffrey

bors-servo · 2019-07-26T03:15:10Z

📌 Commit a2ca3dd has been approved by asajeffrey

bors-servo · 2019-07-26T04:10:00Z

⌛ Testing commit a2ca3dd with merge 0922b2c...


        Auto merge of #23777 - jdm:webgl-main-thread, r=asajeffrey

Support running WebGL in its own thread or on the main thread. This is the final missing piece to support WebGL in ANGLE on Windows. ANGLE doesn't support multiple GL contexts on separate threads using the same underlying Direct3d device, so we need to process all GL operations for WebGL on the same thread as the compositor. These changes try to retain enough flexibility to support both approaches so we can get WebGL working on Windows ASAP. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #23697 - [x] There are tests for these changes  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23777)

bors-servo · 2019-07-26T04:54:55Z

💔 Test failed - linux-rel-wpt

CYBAI · 2019-07-26T05:07:58Z

{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "#container 1", 
    "test": "/css/CSS2/linebox/inline-negative-margin-001.html", 
    "line": 297380, 
    "action": "test_result", 
    "expected": "FAIL"
}

jdm · 2019-07-26T13:16:31Z

@bors-servo retry

#23862

bors-servo · 2019-07-26T13:16:36Z

⌛ Testing commit a2ca3dd with merge 8ec2897...


        Auto merge of #23777 - jdm:webgl-main-thread, r=asajeffrey

Support running WebGL in its own thread or on the main thread. This is the final missing piece to support WebGL in ANGLE on Windows. ANGLE doesn't support multiple GL contexts on separate threads using the same underlying Direct3d device, so we need to process all GL operations for WebGL on the same thread as the compositor. These changes try to retain enough flexibility to support both approaches so we can get WebGL working on Windows ASAP. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #23697 - [x] There are tests for these changes  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23777)

bors-servo · 2019-07-26T15:26:57Z

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

highfive assigned avadacatavra Jul 15, 2019

highfive added the S-awaiting-review label Jul 15, 2019

highfive added the S-tests-failed label Jul 15, 2019

jdm force-pushed the jdm:webgl-main-thread branch from 12e1fd0 to 45245a1 Jul 15, 2019

highfive removed the S-tests-failed label Jul 15, 2019

jdm force-pushed the jdm:webgl-main-thread branch 2 times, most recently from c97999d to f069ede Jul 15, 2019

jdm mentioned this pull request Jul 15, 2019

UWP app panics when shutting down with active webgl #23778

Closed

highfive added the S-tests-failed label Jul 15, 2019

jdm removed the S-awaiting-review label Jul 15, 2019

highfive added S-awaiting-review and removed S-tests-failed labels Jul 17, 2019

highfive added the S-tests-failed label Jul 17, 2019

jdm force-pushed the jdm:webgl-main-thread branch from 8b91853 to 184df7f Jul 17, 2019

highfive removed the S-tests-failed label Jul 17, 2019

asajeffrey reviewed Jul 25, 2019

View changes

jdm added 5 commits Jul 11, 2019

Support running WebGL in its own thread or on the main thread.

39d13d1

Make headless window set current GL context before compositing.

72413bd

Support waking up headless window event loops while they are sleeping.

6caf407

Mark a bunch of webgl tests as long-running.

d7269ad

Remove extra webgl message pumping thread.

Loading status checks…

a2ca3dd

jdm force-pushed the jdm:webgl-main-thread branch from cc6e6a8 to a2ca3dd Jul 26, 2019

highfive added S-awaiting-merge and removed S-awaiting-review labels Jul 26, 2019

highfive added S-tests-failed and removed S-awaiting-merge labels Jul 26, 2019

highfive added S-awaiting-merge and removed S-tests-failed labels Jul 26, 2019

bors-servo merged commit a2ca3dd into servo:master Jul 26, 2019
3 checks passed

3 checks passed

Taskcluster (pull_request) TaskGroup: success
Details

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Jul 26, 2019

bors-servo mentioned this pull request Jul 26, 2019

Replace use of callbacks in webxr by channels #23848

Merged

3 of 3 tasks complete

servo / servo

Support running WebGL in its own thread or on the main thread. #23777

Support running WebGL in its own thread or on the main thread. #23777

jdm commented Jul 15, 2019 •

edited by SimonSapin

jdm commented Jul 15, 2019

bors-servo commented Jul 15, 2019

jdm commented Jul 15, 2019 •

edited

bors-servo commented Jul 15, 2019

jdm commented Jul 15, 2019

bors-servo commented Jul 15, 2019

bors-servo commented Jul 15, 2019

jdm commented Jul 15, 2019

jdm commented Jul 17, 2019

bors-servo commented Jul 17, 2019

bors-servo commented Jul 17, 2019

jdm commented Jul 17, 2019

pcwalton commented Jul 24, 2019

asajeffrey left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

asajeffrey commented Jul 25, 2019

jdm commented Jul 26, 2019

asajeffrey commented Jul 26, 2019

jdm commented Jul 26, 2019

bors-servo commented Jul 26, 2019

bors-servo commented Jul 26, 2019

bors-servo commented Jul 26, 2019

CYBAI commented Jul 26, 2019

jdm commented Jul 26, 2019

bors-servo commented Jul 26, 2019

bors-servo commented Jul 26, 2019

servo / servo

Join GitHub today

Support running WebGL in its own thread or on the main thread. #23777

Support running WebGL in its own thread or on the main thread. #23777

Conversation

jdm commented Jul 15, 2019 • edited by SimonSapin

jdm commented Jul 15, 2019

bors-servo commented Jul 15, 2019

jdm commented Jul 15, 2019 • edited

bors-servo commented Jul 15, 2019

jdm commented Jul 15, 2019

bors-servo commented Jul 15, 2019

bors-servo commented Jul 15, 2019

jdm commented Jul 15, 2019

jdm commented Jul 17, 2019

bors-servo commented Jul 17, 2019

bors-servo commented Jul 17, 2019

jdm commented Jul 17, 2019

pcwalton commented Jul 24, 2019

asajeffrey left a comment

This comment has been minimized.

asajeffrey Jul 25, 2019

This comment has been minimized.

asajeffrey Jul 25, 2019

This comment has been minimized.

asajeffrey Jul 25, 2019

asajeffrey commented Jul 25, 2019

jdm commented Jul 26, 2019

asajeffrey commented Jul 26, 2019

jdm commented Jul 26, 2019

bors-servo commented Jul 26, 2019

bors-servo commented Jul 26, 2019

bors-servo commented Jul 26, 2019

CYBAI commented Jul 26, 2019

jdm commented Jul 26, 2019

bors-servo commented Jul 26, 2019

bors-servo commented Jul 26, 2019

jdm commented Jul 15, 2019 •

edited by SimonSapin

jdm commented Jul 15, 2019 •

edited