Improve Webrender<->WebGL synchronization

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) {

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.
+        // Insert a OpenGL Fence sync object that sends a signal when all the WebGL commands are finished.
-        info.gl_sync = Some(ctx.gl().fence_sync(gl::SYNC_GPU_COMMANDS_COMPLETE, 0));
+        // 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);
         }

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.

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