Introduce WebrenderExternalImageRegistry

components/canvas/webgl_mode/inprocess.rs

@@ -13,7 +13,8 @@ use fnv::FnvHashMap;
 use gleam::gl;
 use servo_config::pref;
 use std::rc::Rc;
-use webrender_traits::WebrenderExternalImageApi;
+use std::sync::{Arc, Mutex};
+use webrender_traits::{WebrenderExternalImageApi, WebrenderExternalImageRegistry};
 
 /// WebGL Threading API entry point that lives in the constellation.
 pub struct WebGLThreads(WebGLSender<WebGLMsg>);
@@ -25,6 +26,7 @@ impl WebGLThreads {
         webrender_gl: Rc<dyn gl::Gl>,
         webrender_api_sender: webrender_api::RenderApiSender,
         webvr_compositor: Option<Box<dyn WebVRRenderHandler>>,
+        external_images: Arc<Mutex<WebrenderExternalImageRegistry>>,
     ) -> (
         WebGLThreads,
         Box<dyn WebrenderExternalImageApi>,
@@ -35,6 +37,7 @@ impl WebGLThreads {
             gl_factory,
             webrender_api_sender,
             webvr_compositor.map(|c| WebVRRenderWrapper(c)),
+            external_images,
         );
         let output_handler = if pref!(dom.webgl.dom_to_texture.enabled) {
             Some(Box::new(OutputHandler::new(

components/canvas/webgl_thread.rs

@@ -12,7 +12,9 @@ use half::f16;
 use offscreen_gl_context::{DrawBuffer, GLContext, NativeGLContextMethods};
 use pixels::{self, PixelFormat};
 use std::borrow::Cow;
+use std::sync::{Arc, Mutex};
 use std::thread;
+use webrender_traits::{WebrenderExternalImageRegistry, WebrenderImageHandlerType};
 
 /// WebGL Threading API entry point that lives in the constellation.
 /// It allows to get a WebGLThread handle for each script pipeline.
@@ -58,29 +60,31 @@ pub struct WebGLThread<VR: WebVRRenderHandler + 'static> {
     cached_context_info: FnvHashMap<WebGLContextId, WebGLContextInfo>,
     /// Current bound context.
     bound_context_id: Option<WebGLContextId>,
-    /// Id generator for new WebGLContexts.
-    next_webgl_id: usize,
     /// Handler user to send WebVR commands.
     webvr_compositor: Option<VR>,
     /// Texture ids and sizes used in DOM to texture outputs.
     dom_outputs: FnvHashMap<webrender_api::PipelineId, DOMToTextureData>,
+    /// List of registered webrender external images.
+    /// We use it to get an unique ID for new WebGLContexts.
+    external_images: Arc<Mutex<WebrenderExternalImageRegistry>>,
 }
 
 impl<VR: WebVRRenderHandler + 'static> WebGLThread<VR> {
     pub fn new(
         gl_factory: GLContextFactory,
         webrender_api_sender: webrender_api::RenderApiSender,
         webvr_compositor: Option<VR>,
+        external_images: Arc<Mutex<WebrenderExternalImageRegistry>>,
     ) -> Self {
         WebGLThread {
             gl_factory,
             webrender_api: webrender_api_sender.create_api(),
             contexts: Default::default(),
             cached_context_info: Default::default(),
             bound_context_id: None,
-            next_webgl_id: 0,
             webvr_compositor,
             dom_outputs: Default::default(),
+            external_images,
         }
     }
 
@@ -90,14 +94,19 @@ impl<VR: WebVRRenderHandler + 'static> WebGLThread<VR> {
         gl_factory: GLContextFactory,
         webrender_api_sender: webrender_api::RenderApiSender,
         webvr_compositor: Option<VR>,
+        external_images: Arc<Mutex<WebrenderExternalImageRegistry>>,
     ) -> WebGLSender<WebGLMsg> {
         let (sender, receiver) = webgl_channel::<WebGLMsg>().unwrap();
         let result = sender.clone();
         thread::Builder::new()
             .name("WebGLThread".to_owned())
             .spawn(move || {
-                let mut renderer =
-                    WebGLThread::new(gl_factory, webrender_api_sender, webvr_compositor);
+                let mut renderer = WebGLThread::new(
+                    gl_factory,
+                    webrender_api_sender,
+                    webvr_compositor,
+                    external_images,
+                );
                 let webgl_chan = WebGLChan(sender);
                 loop {
                     let msg = receiver.recv().unwrap();
@@ -287,9 +296,14 @@ impl<VR: WebVRRenderHandler + 'static> WebGLThread<VR> {
             })
             .map_err(|msg: &str| msg.to_owned())?;
 
-        let id = WebGLContextId(self.next_webgl_id);
+        let id = WebGLContextId(
+            self.external_images
+                .lock()
+                .unwrap()
+                .next_id(WebrenderImageHandlerType::WebGL)
+                .0 as usize,
+        );
         let (size, texture_id, limits) = ctx.get_info();
-        self.next_webgl_id += 1;
         self.contexts.insert(
             id,
             GLContextData {

components/media/lib.rs

@@ -21,7 +21,8 @@ use crate::media_channel::{GLPlayerChan, GLPlayerPipeline, GLPlayerReceiver, GLP
 use crate::media_thread::GLPlayerThread;
 use euclid::Size2D;
 use servo_media::player::context::{GlApi, GlContext, NativeDisplay, PlayerGLContext};
-use webrender_traits::WebrenderExternalImageApi;
+use std::sync::{Arc, Mutex};
+use webrender_traits::{WebrenderExternalImageApi, WebrenderExternalImageRegistry};
 
 /// These are the messages that the GLPlayer thread will forward to
 /// the video player which lives in htmlmediaelement
@@ -99,8 +100,10 @@ impl PlayerGLContext for WindowGLContext {
 pub struct GLPlayerThreads(GLPlayerSender<GLPlayerMsg>);
 
 impl GLPlayerThreads {
-    pub fn new() -> (GLPlayerThreads, Box<dyn WebrenderExternalImageApi>) {
-        let channel = GLPlayerThread::start();
+    pub fn new(
+        external_images: Arc<Mutex<WebrenderExternalImageRegistry>>,
+    ) -> (GLPlayerThreads, Box<dyn WebrenderExternalImageApi>) {
+        let channel = GLPlayerThread::start(external_images);
         let external = GLPlayerExternalImages::new(channel.clone());
         (GLPlayerThreads(channel), Box::new(external))
     }

components/media/media_thread.rs

@@ -7,31 +7,36 @@ use crate::media_channel::{glplayer_channel, GLPlayerSender};
 /// constellation.
 use crate::{GLPlayerMsg, GLPlayerMsgForward};
 use fnv::FnvHashMap;
+use std::sync::{Arc, Mutex};
 use std::thread;
+use webrender_traits::{WebrenderExternalImageRegistry, WebrenderImageHandlerType};
 
 /// A GLPlayerThrx1ead manages the life cycle and message multiplexign of
 /// a set of video players with GL render.
 pub struct GLPlayerThread {
     // Map of live players.
     players: FnvHashMap<u64, GLPlayerSender<GLPlayerMsgForward>>,
-    /// Id generator for new WebGLContexts.
-    next_player_id: u64,
+    /// List of registered webrender external images.
+    /// We use it to get an unique ID for new players.
+    external_images: Arc<Mutex<WebrenderExternalImageRegistry>>,
 }
 
 impl GLPlayerThread {
-    pub fn new() -> Self {
+    pub fn new(external_images: Arc<Mutex<WebrenderExternalImageRegistry>>) -> Self {
         GLPlayerThread {
             players: Default::default(),
-            next_player_id: 1,
+            external_images,
         }
     }
 
-    pub fn start() -> GLPlayerSender<GLPlayerMsg> {
+    pub fn start(
+        external_images: Arc<Mutex<WebrenderExternalImageRegistry>>,
+    ) -> GLPlayerSender<GLPlayerMsg> {
         let (sender, receiver) = glplayer_channel::<GLPlayerMsg>().unwrap();
         thread::Builder::new()
             .name("GLPlayerThread".to_owned())
             .spawn(move || {
-                let mut renderer = GLPlayerThread::new();
+                let mut renderer = GLPlayerThread::new(external_images);
                 loop {
                     let msg = receiver.recv().unwrap();
                     let exit = renderer.handle_msg(msg);
@@ -51,12 +56,20 @@ impl GLPlayerThread {
         trace!("processing {:?}", msg);
         match msg {
             GLPlayerMsg::RegisterPlayer(sender) => {
-                let id = self.next_player_id;
+                let id = self
+                    .external_images
+                    .lock()
+                    .unwrap()
+                    .next_id(WebrenderImageHandlerType::Media)
+                    .0;
                 self.players.insert(id, sender.clone());
                 sender.send(GLPlayerMsgForward::PlayerId(id)).unwrap();
-                self.next_player_id += 1;
             },
             GLPlayerMsg::UnregisterPlayer(id) => {
+                self.external_images
+                    .lock()
+                    .unwrap()
+                    .remove(&webrender_api::ExternalImageId(id));
                 if self.players.remove(&id).is_none() {
                     warn!("Tried to remove an unknown player");
                 }

components/servo/lib.rs

@@ -113,7 +113,7 @@ use std::cmp::max;
 use std::path::PathBuf;
 use std::rc::Rc;
 use webrender::{RendererKind, ShaderPrecacheFlags};
-use webrender_traits::{WebrenderExternalImageHandler, WebrenderImageHandlerType};
+use webrender_traits::{WebrenderExternalImageHandlers, WebrenderImageHandlerType};
 use webvr::{VRServiceManager, WebVRCompositorHandler, WebVRThread};
 
 pub use gleam::gl;
@@ -690,7 +690,8 @@ fn create_constellation(
         GLContextFactory::current_native_handle(&compositor_proxy)
     };
 
-    let mut webrender_external_image_handler = Box::new(WebrenderExternalImageHandler::new());
+    let (external_image_handlers, external_images) = WebrenderExternalImageHandlers::new();
+    let mut external_image_handlers = Box::new(external_image_handlers);
 
     // Initialize WebGL Thread entry point.
     let webgl_threads = gl_factory.map(|factory| {
@@ -699,11 +700,11 @@ fn create_constellation(
             window_gl,
             webrender_api_sender.clone(),
             webvr_compositor.map(|c| c as Box<_>),
+            external_images.clone(),
         );
 
         // Set webrender external image handler for WebGL textures
-        webrender_external_image_handler
-            .set_handler(image_handler, WebrenderImageHandlerType::WebGL);
+        external_image_handlers.set_handler(image_handler, WebrenderImageHandlerType::WebGL);
 
         // Set DOM to texture handler, if enabled.
         if let Some(output_handler) = output_handler {
@@ -716,14 +717,13 @@ fn create_constellation(
     let glplayer_threads = match player_context.gl_context {
         GlContext::Unknown => None,
         _ => {
-            let (glplayer_threads, image_handler) = GLPlayerThreads::new();
-            webrender_external_image_handler
-                .set_handler(image_handler, WebrenderImageHandlerType::Media);
+            let (glplayer_threads, image_handler) = GLPlayerThreads::new(external_images);
+            external_image_handlers.set_handler(image_handler, WebrenderImageHandlerType::Media);
             Some(glplayer_threads)
         },
     };
 
-    webrender.set_external_image_handler(webrender_external_image_handler);
+    webrender.set_external_image_handler(external_image_handlers);
 
     let player_context = WindowGLContext {
         glplayer_chan: glplayer_threads.as_ref().map(|threads| threads.pipeline()),

components/webrender_traits/lib.rs

@@ -8,6 +8,7 @@
 
 use euclid::Size2D;
 use std::collections::HashMap;
+use std::sync::{Arc, Mutex};
 
 /// This trait is used as a bridge between the different GL clients
 /// in Servo that handles WebRender ExternalImages and the WebRender
@@ -26,23 +27,66 @@ pub enum WebrenderImageHandlerType {
     Media,
 }
 
-/// WebRender External Image Handler implementation.
-pub struct WebrenderExternalImageHandler {
-    webgl_handler: Option<Box<dyn WebrenderExternalImageApi>>,
-    media_handler: Option<Box<dyn WebrenderExternalImageApi>>,
-    //XXX(ferjm) register external images.
+/// List of Webrender external images to be shared among all external image
+/// consumers (WebGL, Media).
+/// It ensures that external image identifiers are unique.
+pub struct WebrenderExternalImageRegistry {
+    /// Map of all generated external images.
     external_images: HashMap<webrender_api::ExternalImageId, WebrenderImageHandlerType>,
+    /// Id generator for the next external image identifier.
+    next_image_id: u64,
 }
 
-impl WebrenderExternalImageHandler {
+impl WebrenderExternalImageRegistry {
     pub fn new() -> Self {
         Self {
-            webgl_handler: None,
-            media_handler: None,
             external_images: HashMap::new(),
+            next_image_id: 0,
         }
     }
 
+    pub fn next_id(
+        &mut self,
+        handler_type: WebrenderImageHandlerType,
+    ) -> webrender_api::ExternalImageId {
+        self.next_image_id += 1;
+        let key = webrender_api::ExternalImageId(self.next_image_id);
+        self.external_images.insert(key, handler_type);
+        key
+    }
+
+    pub fn remove(&mut self, key: &webrender_api::ExternalImageId) {
+        self.external_images.remove(key);
+    }
+
+    pub fn get(&self, key: &webrender_api::ExternalImageId) -> Option<&WebrenderImageHandlerType> {
+        self.external_images.get(key)
+    }
+}
+
+/// WebRender External Image Handler implementation.
+pub struct WebrenderExternalImageHandlers {
+    /// WebGL handler.
+    webgl_handler: Option<Box<dyn WebrenderExternalImageApi>>,
+    /// Media player handler.
+    media_handler: Option<Box<dyn WebrenderExternalImageApi>>,
+    /// Webrender external images.
+    external_images: Arc<Mutex<WebrenderExternalImageRegistry>>,
+}
+
+impl WebrenderExternalImageHandlers {
+    pub fn new() -> (Self, Arc<Mutex<WebrenderExternalImageRegistry>>) {
+        let external_images = Arc::new(Mutex::new(WebrenderExternalImageRegistry::new()));
+        (
+            Self {
+                webgl_handler: None,
+                media_handler: None,
+                external_images: external_images.clone(),
+            },
+            external_images,
+        )
+    }
+
     pub fn set_handler(
         &mut self,
         handler: Box<dyn WebrenderExternalImageApi>,
@@ -55,7 +99,7 @@ impl WebrenderExternalImageHandler {
     }
 }
 
-impl webrender::ExternalImageHandler for WebrenderExternalImageHandler {
+impl webrender::ExternalImageHandler for WebrenderExternalImageHandlers {
     /// Lock the external image. Then, WR could start to read the
     /// image content.
     /// The WR client should not change the image content until the
@@ -66,10 +110,7 @@ impl webrender::ExternalImageHandler for WebrenderExternalImageHandler {
         _channel_index: u8,
         _rendering: webrender_api::ImageRendering,
     ) -> webrender::ExternalImage {
-        if let Some(handler_type) = self.external_images.get(&key) {
-            // It is safe to unwrap the handlers here because we forbid registration
-            // for specific types that has no handler set.
-            // XXX(ferjm) make this ^ true.
+        if let Some(handler_type) = self.external_images.lock().unwrap().get(&key) {
             let (texture_id, size) = match handler_type {
                 WebrenderImageHandlerType::WebGL => {
                     self.webgl_handler.as_mut().unwrap().lock(key.0)
@@ -90,9 +131,7 @@ impl webrender::ExternalImageHandler for WebrenderExternalImageHandler {
     /// Unlock the external image. The WR should not read the image
     /// content after this call.
     fn unlock(&mut self, key: webrender_api::ExternalImageId, _channel_index: u8) {
-        if let Some(handler_type) = self.external_images.get(&key) {
-            // It is safe to unwrap the handlers here because we forbid registration
-            // for specific types that has no handler set.
+        if let Some(handler_type) = self.external_images.lock().unwrap().get(&key) {
             match handler_type {
                 WebrenderImageHandlerType::WebGL => {
                     self.webgl_handler.as_mut().unwrap().unlock(key.0)