servo: Merge #18575 - Improve Webrender<->WebGL synchronization (from MortimerGoro:webgl_flickering); r=glennw

<!-- Please describe your changes on the following line: -->

 Webrender<->WebGL synchronization is not perfect yet, and it has some flickering specially when adding more elements on the page than a single full-screen canvas.

This PR improves the synchronization by using the WR thread to perform the fence wait. All the flickering with multiple elements on the page is gone thanks to this change.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #14235 (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: 941757bafebecd8327fff66e0369c3c5f42d1366

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : 2644bb5199128f8554c183cfcf77c3326e045eff

servo/components/canvas/webgl_mode/inprocess.rs

@@ -8,7 +8,9 @@ use canvas_traits::webgl::{WebGLChan, WebGLContextId, WebGLMsg, WebGLPipeline, W
 use canvas_traits::webgl::{WebGLSender, WebVRCommand, WebVRRenderHandler};
 use canvas_traits::webgl::webgl_channel;
 use euclid::Size2D;
+use gleam::gl;
 use std::marker::PhantomData;
+use std::rc::Rc;
 use webrender;
 use webrender_api;
 
@@ -18,6 +20,7 @@ pub struct WebGLThreads(WebGLSender<WebGLMsg>);
 impl WebGLThreads {
     /// Creates a new WebGLThreads object
     pub fn new(gl_factory: GLContextFactory,
+               webrender_gl: Rc<gl::Gl>,
                webrender_api_sender: webrender_api::RenderApiSender,
                webvr_compositor: Option<Box<WebVRRenderHandler>>)
                -> (WebGLThreads, Box<webrender::ExternalImageHandler>) {
@@ -26,7 +29,7 @@ impl WebGLThreads {
                                          webrender_api_sender,
                                          webvr_compositor.map(|c| WebVRRenderWrapper(c)),
                                          PhantomData);
-        let external = WebGLExternalImageHandler::new(WebGLExternalImages::new(channel.clone()));
+        let external = WebGLExternalImageHandler::new(WebGLExternalImages::new(webrender_gl, channel.clone()));
         (WebGLThreads(channel), Box::new(external))
     }
 
@@ -44,14 +47,16 @@ impl WebGLThreads {
 
 /// Bridge between the webrender::ExternalImage callbacks and the WebGLThreads.
 struct WebGLExternalImages {
+    webrender_gl: Rc<gl::Gl>,
     webgl_channel: WebGLSender<WebGLMsg>,
     // Used to avoid creating a new channel on each received WebRender request.
-    lock_channel: (WebGLSender<(u32, Size2D<i32>)>, WebGLReceiver<(u32, Size2D<i32>)>),
+    lock_channel: (WebGLSender<(u32, Size2D<i32>, usize)>, WebGLReceiver<(u32, Size2D<i32>, usize)>),
 }
 
 impl WebGLExternalImages {
-    fn new(channel: WebGLSender<WebGLMsg>) -> Self {
+    fn new(webrender_gl: Rc<gl::Gl>, channel: WebGLSender<WebGLMsg>) -> Self {
         Self {
+            webrender_gl,
             webgl_channel: channel,
             lock_channel: webgl_channel().unwrap(),
         }
@@ -60,8 +65,15 @@ impl WebGLExternalImages {
 
 impl WebGLExternalImageApi for WebGLExternalImages {
     fn lock(&mut self, ctx_id: WebGLContextId) -> (u32, Size2D<i32>) {
+        // WebGL Thread has it's own GL command queue that we need to synchronize with the WR GL command queue.
+        // The WebGLMsg::Lock message inserts a fence in the WebGL command queue.
         self.webgl_channel.send(WebGLMsg::Lock(ctx_id, self.lock_channel.0.clone())).unwrap();
-        self.lock_channel.1.recv().unwrap()
+        let (image_id, size, gl_sync) = self.lock_channel.1.recv().unwrap();
+        // The next glWaitSync call is run on the WR thread and it's used to synchronize the two
+        // flows of OpenGL commands in order to avoid WR using a semi-ready WebGL texture.
+        // glWaitSync doesn't block WR thread, it affects only internal OpenGL subsystem.
+        self.webrender_gl.wait_sync(gl_sync as gl::GLsync, 0, gl::TIMEOUT_IGNORED);
+        (image_id, size)
     }
 
     fn unlock(&mut self, ctx_id: WebGLContextId) {

servo/components/canvas/webgl_thread.rs

@@ -145,14 +145,19 @@ impl<VR: WebVRRenderHandler + 'static, OB: WebGLThreadObserver> WebGLThread<VR,
     }
 
     /// Handles a lock external callback received from webrender::ExternalImageHandler
-    fn handle_lock(&mut self, context_id: WebGLContextId, sender: WebGLSender<(u32, Size2D<i32>)>) {
+    fn handle_lock(&mut self, context_id: WebGLContextId, sender: WebGLSender<(u32, Size2D<i32>, usize)>) {
         let ctx = Self::make_current_if_needed(context_id, &self.contexts, &mut self.bound_context_id)
                         .expect("WebGLContext not found in a WebGLMsg::Lock message");
         let info = self.cached_context_info.get_mut(&context_id).unwrap();
-        // Use a OpenGL Fence to perform the lock.
-        info.gl_sync = Some(ctx.gl().fence_sync(gl::SYNC_GPU_COMMANDS_COMPLETE, 0));
+        // Insert a OpenGL Fence sync object that sends a signal when all the WebGL commands are finished.
+        // The related gl().wait_sync call is performed in the WR thread. See WebGLExternalImageApi for mor details.
+        let gl_sync = ctx.gl().fence_sync(gl::SYNC_GPU_COMMANDS_COMPLETE, 0);
+        info.gl_sync = Some(gl_sync);
+        // It is important that the fence sync is properly flushed into the GPU's command queue.
+        // Without proper flushing, the sync object may never be signaled.
+        ctx.gl().flush();
 
-        sender.send((info.texture_id, info.size)).unwrap();
+        sender.send((info.texture_id, info.size, gl_sync as usize)).unwrap();
     }
 
     /// Handles an unlock external callback received from webrender::ExternalImageHandler
@@ -161,10 +166,6 @@ impl<VR: WebVRRenderHandler + 'static, OB: WebGLThreadObserver> WebGLThread<VR,
                         .expect("WebGLContext not found in a WebGLMsg::Unlock message");
         let info = self.cached_context_info.get_mut(&context_id).unwrap();
         if let Some(gl_sync) = info.gl_sync.take() {
-            // glFlush must be called before glWaitSync.
-            ctx.gl().flush();
-            // Wait until the GLSync object is signaled.
-            ctx.gl().wait_sync(gl_sync, 0, gl::TIMEOUT_IGNORED);
             // Release the GLSync object.
             ctx.gl().delete_sync(gl_sync);
         }

servo/components/canvas_traits/webgl.rs

@@ -39,7 +39,7 @@ pub enum WebGLMsg {
     /// WR locks a external texture when it wants to use the shared texture contents.
     /// The WR client should not change the shared texture content until the Unlock call.
     /// Currently OpenGL Sync Objects are used to implement the synchronization mechanism.
-    Lock(WebGLContextId, WebGLSender<(u32, Size2D<i32>)>),
+    Lock(WebGLContextId, WebGLSender<(u32, Size2D<i32>, usize)>),
     /// Unlocks a specific WebGLContext. Unlock messages are used for a correct synchronization
     /// with WebRender external image API.
     /// The WR unlocks a context when it finished reading the shared texture contents.

servo/components/servo/lib.rs

@@ -222,7 +222,8 @@ impl<Window> Servo<Window> where Window: WindowMethods + 'static {
                                                                     supports_clipboard,
                                                                     &mut webrender,
                                                                     webrender_document,
-                                                                    webrender_api_sender);
+                                                                    webrender_api_sender,
+                                                                    window.gl());
 
         // Send the constellation's swmanager sender to service worker manager thread
         script::init_service_workers(sw_senders);
@@ -519,7 +520,8 @@ fn create_constellation(user_agent: Cow<'static, str>,
                         supports_clipboard: bool,
                         webrender: &mut webrender::Renderer,
                         webrender_document: webrender_api::DocumentId,
-                        webrender_api_sender: webrender_api::RenderApiSender)
+                        webrender_api_sender: webrender_api::RenderApiSender,
+                        window_gl: Rc<gl::Gl>)
                         -> (Sender<ConstellationMsg>, SWManagerSenders) {
     let bluetooth_thread: IpcSender<BluetoothRequest> = BluetoothThreadFactory::new();
 
@@ -552,6 +554,7 @@ fn create_constellation(user_agent: Cow<'static, str>,
 
     // Initialize WebGL Thread entry point.
     let (webgl_threads, image_handler) = WebGLThreads::new(gl_factory,
+                                                           window_gl,
                                                            webrender_api_sender.clone(),
                                                            webvr_compositor.map(|c| c as Box<_>));
     // Set webrender external image handler for WebGL textures