Replace use of callbacks in webxr by channels

Cargo.toml

@@ -29,3 +29,7 @@ opt-level = 3
 mio = { git = "https://github.com/servo/mio.git", branch = "servo" }
 iovec = { git = "https://github.com/servo/iovec.git", branch = "servo" }
 cmake = { git = "https://github.com/alexcrichton/cmake-rs" }
+
+[patch."https://github.com/servo/webxr"]
+webxr = { git = "https://github.com/asajeffrey/webxr", branch = "optional-glsync" }
+webxr-api = { git = "https://github.com/asajeffrey/webxr", branch = "optional-glsync" }

components/canvas/Cargo.toml

@@ -37,3 +37,4 @@ servo_config = {path = "../config"}
 webrender = {git = "https://github.com/servo/webrender"}
 webrender_api = {git = "https://github.com/servo/webrender", features = ["ipc"]}
 webrender_traits = {path = "../webrender_traits"}
+webxr-api = {git = "https://github.com/servo/webxr", features = ["ipc"]}

components/canvas/webgl_mode/inprocess.rs

@@ -1,7 +1,6 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
-
 use crate::gl_context::GLContextFactory;
 use crate::webgl_thread::{WebGLMainThread, WebGLThread, WebGLThreadInit};
 use canvas_traits::webgl::webgl_channel;
@@ -17,6 +16,7 @@ use std::default::Default;
 use std::rc::Rc;
 use std::sync::{Arc, Mutex};
 use webrender_traits::{WebrenderExternalImageApi, WebrenderExternalImageRegistry};
+use webxr_api::WebGLExternalImageApi;
 
 /// WebGL Threading API entry point that lives in the constellation.
 pub struct WebGLThreads(WebGLSender<WebGLMsg>);
@@ -38,6 +38,7 @@ impl WebGLThreads {
     ) -> (
         WebGLThreads,
         Option<Rc<WebGLMainThread>>,
+        Box<dyn webxr_api::WebGLExternalImageApi>,
         Box<dyn WebrenderExternalImageApi>,
         Option<Box<dyn webrender::OutputImageHandler>>,
     ) {
@@ -77,6 +78,7 @@ impl WebGLThreads {
         (
             WebGLThreads(sender),
             webgl_thread,
+            external.sendable.clone_box(),
             Box::new(external),
             output_handler.map(|b| b as Box<_>),
         )
@@ -96,9 +98,8 @@ impl WebGLThreads {
     }
 }
 
-/// Bridge between the webrender::ExternalImage callbacks and the WebGLThreads.
-struct WebGLExternalImages {
-    webrender_gl: Rc<dyn gl::Gl>,
+/// Bridge between the webxr_api::ExternalImage callbacks and the WebGLThreads.
+struct SendableWebGLExternalImages {
     webgl_channel: WebGLSender<WebGLMsg>,
     // Used to avoid creating a new channel on each received WebRender request.
     lock_channel: (
@@ -107,24 +108,26 @@ struct WebGLExternalImages {
     ),
 }
 
-impl WebGLExternalImages {
-    fn new(webrender_gl: Rc<dyn gl::Gl>, channel: WebGLSender<WebGLMsg>) -> Self {
-        WebGLExternalImages {
-            webrender_gl,
+impl SendableWebGLExternalImages {
+    fn new(channel: WebGLSender<WebGLMsg>) -> Self {
+        Self {
             webgl_channel: channel,
             lock_channel: webgl_channel().unwrap(),
         }
     }
 }
 
-impl WebrenderExternalImageApi for WebGLExternalImages {
-    fn lock(&mut self, id: u64) -> (u32, Size2D<i32>) {
+impl webxr_api::WebGLExternalImageApi for SendableWebGLExternalImages {
+    fn lock(&self, id: usize) -> (u32, Size2D<i32>, Option<gl::GLsync>) {
         if let Some(main_thread) = WebGLMainThread::on_current_thread() {
             // If we're on the same thread as WebGL, we can get the data directly
-            main_thread
+            let (image_id, size) = main_thread
                 .thread_data
                 .borrow_mut()
-                .handle_lock_unsync(WebGLContextId(id as usize))
+                .handle_lock_unsync(WebGLContextId(id as usize));
+            // We don't need a GLsync object if we're running on the main thread
+            // Might be better to return an option?
+            (image_id, size, None)
         } else {
             // 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.
@@ -135,16 +138,11 @@ impl WebrenderExternalImageApi for WebGLExternalImages {
                 ))
                 .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)
+            (image_id, size, Some(gl_sync as gl::GLsync))
         }
     }
 
-    fn unlock(&mut self, id: u64) {
+    fn unlock(&self, id: usize) {
         if let Some(main_thread) = WebGLMainThread::on_current_thread() {
             // If we're on the same thread as WebGL, we can unlock directly
             main_thread
@@ -157,6 +155,42 @@ impl WebrenderExternalImageApi for WebGLExternalImages {
                 .unwrap()
         }
     }
+
+    fn clone_box(&self) -> Box<dyn webxr_api::WebGLExternalImageApi> {
+        Box::new(Self::new(self.webgl_channel.clone()))
+    }
+}
+
+/// Bridge between the webrender::ExternalImage callbacks and the WebGLThreads.
+struct WebGLExternalImages {
+    webrender_gl: Rc<dyn gl::Gl>,
+    sendable: SendableWebGLExternalImages,
+}
+
+impl WebGLExternalImages {
+    fn new(webrender_gl: Rc<dyn gl::Gl>, channel: WebGLSender<WebGLMsg>) -> Self {
+        Self {
+            webrender_gl,
+            sendable: SendableWebGLExternalImages::new(channel),
+        }
+    }
+}
+
+impl WebrenderExternalImageApi for WebGLExternalImages {
+    fn lock(&mut self, id: u64) -> (u32, Size2D<i32>) {
+        let (image_id, size, gl_sync) = self.sendable.lock(id as usize);
+        // 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.
+        if let Some(gl_sync) = gl_sync {
+            self.webrender_gl.wait_sync(gl_sync, 0, gl::TIMEOUT_IGNORED);
+        }
+        (image_id, size)
+    }
+
+    fn unlock(&mut self, id: u64) {
+        self.sendable.unlock(id as usize);
+    }
 }
 
 /// struct used to implement DOMToTexture feature and webrender::OutputImageHandler trait.

components/canvas/webgl_thread.rs

@@ -10,7 +10,7 @@ use euclid::default::Size2D;
 use fnv::FnvHashMap;
 use gleam::gl;
 use half::f16;
-use ipc_channel::ipc::{self, IpcSender, OpaqueIpcMessage};
+use ipc_channel::ipc::{self, OpaqueIpcMessage};
 use ipc_channel::router::ROUTER;
 use offscreen_gl_context::{DrawBuffer, GLContext, NativeGLContextMethods};
 use pixels::{self, PixelFormat};
@@ -284,9 +284,6 @@ impl WebGLThread {
             WebGLMsg::Lock(ctx_id, sender) => {
                 self.handle_lock(ctx_id, sender);
             },
-            WebGLMsg::LockIPC(ctx_id, sender) => {
-                self.handle_lock_ipc(ctx_id, sender);
-            },
             WebGLMsg::Unlock(ctx_id) => {
                 self.handle_unlock(ctx_id);
             },
@@ -343,21 +340,6 @@ impl WebGLThread {
         context_id: WebGLContextId,
         sender: WebGLSender<(u32, Size2D<i32>, usize)>,
     ) {
-        sender.send(self.handle_lock_sync(context_id)).unwrap();
-    }
-
-    /// handle_lock, but unconditionally IPC (used by webxr)
-    fn handle_lock_ipc(
-        &mut self,
-        context_id: WebGLContextId,
-        sender: IpcSender<(u32, Size2D<i32>, usize)>,
-    ) {
-        sender.send(self.handle_lock_sync(context_id)).unwrap();
-    }
-
-    /// Shared code between handle_lock and handle_lock_ipc, does the actual syncing/flushing
-    /// but the caller must send the response back
-    fn handle_lock_sync(&mut self, context_id: WebGLContextId) -> (u32, Size2D<i32>, usize) {
         let data =
             Self::make_current_if_needed(context_id, &self.contexts, &mut self.bound_context_id)
                 .expect("WebGLContext not found in a WebGLMsg::Lock message");
@@ -374,7 +356,7 @@ impl WebGLThread {
         data.ctx.gl().flush();
         debug_assert!(data.ctx.gl().get_error() == gl::NO_ERROR);
 
-        (info.texture_id, info.size, gl_sync as usize)
+        let _ = sender.send((info.texture_id, info.size, gl_sync as usize));
     }
 
     /// A version of locking that doesn't return a GLsync object,

components/canvas_traits/webgl.rs

@@ -4,10 +4,8 @@
 
 use euclid::default::{Rect, Size2D};
 use gleam::gl;
-use gleam::gl::GLsync;
-use gleam::gl::GLuint;
 use gleam::gl::Gl;
-use ipc_channel::ipc::{self, IpcBytesReceiver, IpcBytesSender, IpcSender, IpcSharedMemory};
+use ipc_channel::ipc::{IpcBytesReceiver, IpcBytesSender, IpcSharedMemory};
 use pixels::PixelFormat;
 use std::borrow::Cow;
 use std::fmt;
@@ -61,8 +59,6 @@ pub enum WebGLMsg {
     /// 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>, usize)>),
-    /// Lock(), but unconditionally IPC (used by webxr)
-    LockIPC(WebGLContextId, IpcSender<(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.
@@ -185,39 +181,6 @@ impl WebGLMsgSender {
     pub fn send_dom_to_texture(&self, command: DOMToTextureCommand) -> WebGLSendResult {
         self.sender.send(WebGLMsg::DOMToTextureCommand(command))
     }
-
-    pub fn webxr_external_image_api(&self) -> impl webxr_api::WebGLExternalImageApi {
-        SerializableWebGLMsgSender {
-            ctx_id: self.ctx_id,
-            sender: self.sender.to_ipc(),
-        }
-    }
-}
-
-// WegGLMsgSender isn't actually serializable, despite what it claims.
-#[derive(Clone, Debug, Deserialize, MallocSizeOf, Serialize)]
-struct SerializableWebGLMsgSender {
-    ctx_id: WebGLContextId,
-    #[ignore_malloc_size_of = "channels are hard"]
-    sender: IpcSender<WebGLMsg>,
-}
-
-#[typetag::serde]
-impl webxr_api::WebGLExternalImageApi for SerializableWebGLMsgSender {
-    fn lock(&self) -> Result<(GLuint, Size2D<i32>, GLsync), webxr_api::Error> {
-        let (sender, receiver) = ipc::channel().or(Err(webxr_api::Error::CommunicationError))?;
-        self.sender
-            .send(WebGLMsg::LockIPC(self.ctx_id, sender))
-            .or(Err(webxr_api::Error::CommunicationError))?;
-        let (texture, size, sync) = receiver
-            .recv()
-            .or(Err(webxr_api::Error::CommunicationError))?;
-        Ok((texture, size, sync as GLsync))
-    }
-
-    fn unlock(&self) {
-        let _ = self.sender.send(WebGLMsg::Unlock(self.ctx_id));
-    }
 }
 
 #[derive(Deserialize, Serialize)]