Auto merge of #636 - JerryShih:handle-external-image-life-time, r=glennw

Handle external image life time in api.delete_image() call

Currently, the delete_image() call just removes the image_key in the hash_map. If the key is associate to an external image, we should release that image using the ExternalImageHandler callbacks.

Since the ExternalImageHandler callbacks are only used in renderer, a pending releasing array is used. Then, pass this array to the renderer for ExternalImageHandler::release() call.

@nical @glennw

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/636)
<!-- Reviewable:end -->

Author: bors-servo

webrender/src/internal_types.rs

@@ -347,6 +347,8 @@ pub enum TextureUpdateOp {
     Grow(u32, u32, ImageFormat, TextureFilter, RenderTargetMode),
 }
 
+pub type ExternalImageUpdateList = Vec<ExternalImageId>;
+
 pub struct TextureUpdate {
     pub id: CacheTextureId,
     pub op: TextureUpdateOp,
@@ -395,9 +397,8 @@ impl RendererFrame {
 }
 
 pub enum ResultMsg {
-    UpdateTextureCache(TextureUpdateList),
     RefreshShader(PathBuf),
-    NewFrame(RendererFrame, BackendProfileCounters),
+    NewFrame(RendererFrame, TextureUpdateList, ExternalImageUpdateList, BackendProfileCounters),
 }
 
 #[repr(u32)]

webrender/src/render_backend.rs

@@ -431,18 +431,15 @@ impl RenderBackend {
                                      &self.scene.pipeline_auxiliary_lists,
                                      self.device_pixel_ratio);
 
-        let pending_update = self.resource_cache.pending_updates();
-        if !pending_update.updates.is_empty() {
-            self.result_tx.send(ResultMsg::UpdateTextureCache(pending_update)).unwrap();
-        }
-
         frame
     }
 
     fn publish_frame(&mut self,
                      frame: RendererFrame,
                      profile_counters: &mut BackendProfileCounters) {
-        let msg = ResultMsg::NewFrame(frame, profile_counters.clone());
+        let pending_update = self.resource_cache.pending_updates();
+        let pending_external_image_update = self.resource_cache.pending_external_image_updates();
+        let msg = ResultMsg::NewFrame(frame, pending_update, pending_external_image_update, profile_counters.clone());
         self.result_tx.send(msg).unwrap();
         profile_counters.reset();
     }

webrender/src/renderer.rs

@@ -16,7 +16,7 @@ use device::{TextureFilter, VAOId, VertexUsageHint, FileWatcherHandler, TextureT
 use euclid::Matrix4D;
 use fnv::FnvHasher;
 use internal_types::{CacheTextureId, RendererFrame, ResultMsg, TextureUpdateOp};
-use internal_types::{TextureUpdateList, PackedVertex, RenderTargetMode};
+use internal_types::{ExternalImageUpdateList, TextureUpdateList, PackedVertex, RenderTargetMode};
 use internal_types::{ORTHO_NEAR_PLANE, ORTHO_FAR_PLANE, SourceTexture};
 use internal_types::{BatchTextures, TextureSampler, GLContextHandleWrapper};
 use profiler::{Profiler, BackendProfileCounters};
@@ -710,10 +710,12 @@ impl Renderer {
         // Pull any pending results and return the most recent.
         while let Ok(msg) = self.result_rx.try_recv() {
             match msg {
-                ResultMsg::UpdateTextureCache(update_list) => {
-                    self.pending_texture_updates.push(update_list);
-                }
-                ResultMsg::NewFrame(frame, profile_counters) => {
+                ResultMsg::NewFrame(frame, texture_update_list, external_image_update_list, profile_counters) => {
+                    self.pending_texture_updates.push(texture_update_list);
+
+                    // When a new frame is ready, we could start to update all pending external image requests here.
+                    self.release_external_images(external_image_update_list);
+
                     self.backend_profile_counters = profile_counters;
 
                     // Update the list of available epochs for use during reftests.
@@ -1262,7 +1264,7 @@ impl Renderer {
                 let props = &deferred_resolve.image_properties;
                 let external_id = props.external_id
                                        .expect("BUG: Deferred resolves must be external images!");
-                let image = handler.get(external_id);
+                let image = handler.lock(external_id);
 
                 let texture_id = match image.source {
                     ExternalImageSource::NativeTexture(texture_id) => TextureId::new(texture_id),
@@ -1277,13 +1279,25 @@ impl Renderer {
         }
     }
 
-    fn release_external_textures(&mut self) {
+    fn unlock_external_images(&mut self) {
         if !self.external_images.is_empty() {
             let handler = self.external_image_handler
                               .as_mut()
                               .expect("Found external image, but no handler set!");
 
             for (external_id, _) in self.external_images.drain() {
+                handler.unlock(external_id);
+            }
+        }
+    }
+
+    fn release_external_images(&mut self, mut pending_external_image_updates: ExternalImageUpdateList) {
+        if !pending_external_image_updates.is_empty() {
+            let handler = self.external_image_handler
+                              .as_mut()
+                              .expect("found external image updates, but no handler set!");
+
+            for external_id in pending_external_image_updates.drain(..) {
                 handler.release(external_id);
             }
         }
@@ -1388,7 +1402,7 @@ impl Renderer {
             }
         }
 
-        self.release_external_textures();
+        self.unlock_external_images();
     }
 
     pub fn debug_renderer<'a>(&'a mut self) -> &'a mut DebugRenderer {
@@ -1427,10 +1441,19 @@ pub struct ExternalImage {
     pub source: ExternalImageSource,
 }
 
-/// Interface that an application can implement
-/// to support providing external image buffers.
+/// The interfaces that an application can implement to support providing
+/// external image buffers.
+/// When the the application passes an external image to WR, it should kepp that
+/// external image life time untile the release() call.
 pub trait ExternalImageHandler {
-    fn get(&mut self, key: ExternalImageId) -> ExternalImage;
+    /// Lock the external image. Then, WR could start to read the image content.
+    /// The WR client should not change the image content until the unlock()
+    /// call.
+    fn lock(&mut self, key: ExternalImageId) -> ExternalImage;
+    /// Unlock the external image. The WR should not read the image content
+    /// after this call.
+    fn unlock(&mut self, key: ExternalImageId);
+    /// Tell the WR client that it could start to release this external image.
     fn release(&mut self, key: ExternalImageId);
 }
 

webrender/src/resource_cache.rs

@@ -6,14 +6,15 @@ use app_units::Au;
 use device::TextureFilter;
 use fnv::FnvHasher;
 use frame::FrameId;
-use internal_types::{FontTemplate, SourceTexture, TextureUpdateList};
+use internal_types::{ExternalImageUpdateList, FontTemplate, SourceTexture, TextureUpdateList};
 use platform::font::{FontContext, RasterizedGlyph};
 use std::cell::RefCell;
 use std::collections::{HashMap, HashSet};
 use std::collections::hash_map::Entry::{self, Occupied, Vacant};
 use std::fmt::Debug;
 use std::hash::BuildHasherDefault;
 use std::hash::Hash;
+use std::mem;
 use std::sync::{Arc, Barrier};
 use std::sync::mpsc::{channel, Receiver, Sender};
 use std::thread;
@@ -207,6 +208,7 @@ pub struct ResourceCache {
     pending_image_requests: Vec<ImageRequest>,
     glyph_cache_tx: Sender<GlyphCacheMsg>,
     glyph_cache_result_queue: Receiver<GlyphCacheResultMsg>,
+    pending_external_image_update_list: ExternalImageUpdateList,
 }
 
 impl ResourceCache {
@@ -228,6 +230,7 @@ impl ResourceCache {
             pending_image_requests: Vec::new(),
             glyph_cache_tx: glyph_cache_tx,
             glyph_cache_result_queue: glyph_cache_result_queue,
+            pending_external_image_update_list: ExternalImageUpdateList::new(),
         }
     }
 
@@ -272,6 +275,14 @@ impl ResourceCache {
                                  bytes: Vec<u8>) {
         let next_epoch = match self.image_templates.get(&image_key) {
             Some(image) => {
+                // This image should not be an external image.
+                match image.data {
+                    ImageData::External(id) => {
+                        panic!("Update an external image with buffer, id={} image_key={:?}", id.0, image_key);
+                    },
+                    _ => {},
+                }
+
                 let Epoch(current_epoch) = image.epoch;
                 Epoch(current_epoch + 1)
             }
@@ -294,7 +305,21 @@ impl ResourceCache {
     }
 
     pub fn delete_image_template(&mut self, image_key: ImageKey) {
-        self.image_templates.remove(&image_key);
+        let value = self.image_templates.remove(&image_key);
+
+        // If the key is associated to an external image, pass the external id to renderer for cleanup.
+        if let Some(image) = value {
+            match image.data {
+                ImageData::External(id) => {
+                    self.pending_external_image_update_list.push(id);
+                },
+                _ => {},
+            }
+
+            return;
+        }
+
+        println!("Delete the non-exist key:{:?}", image_key);
     }
 
     pub fn add_webgl_texture(&mut self, id: WebGLContextId, texture_id: SourceTexture, size: DeviceIntSize) {
@@ -343,6 +368,10 @@ impl ResourceCache {
         self.texture_cache.pending_updates()
     }
 
+    pub fn pending_external_image_updates(&mut self) -> ExternalImageUpdateList {
+        mem::replace(&mut self.pending_external_image_update_list, ExternalImageUpdateList::new())
+    }
+
     pub fn get_glyphs<F>(&self,
                          font_key: FontKey,
                          size: Au,