Bug 1499908 - Uplift WebRender PR #3234 to Firefox 64. r=kats, a=RyanVM
authorWR Updater Bot <graphics-team@mozilla.staktrace.com>
Mon, 29 Oct 2018 00:29:46 +0000
changeset 500944 bc7548ec328bb2ece1a2a0d519fd466cf2353ad1
parent 500943 3931ebbd558836838926001aef24f4c4d1120653
child 500945 a78fa2f251c0f213f7b06f45e7488e4f22a9a429
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats, RyanVM
bugs1499908
milestone64.0
Bug 1499908 - Uplift WebRender PR #3234 to Firefox 64. r=kats, a=RyanVM Differential Revision: https://phabricator.services.mozilla.com/D10021
gfx/webrender/src/device/gl.rs
gfx/webrender/src/resource_cache.rs
gfx/webrender_bindings/revision.txt
--- a/gfx/webrender/src/device/gl.rs
+++ b/gfx/webrender/src/device/gl.rs
@@ -724,17 +724,17 @@ pub enum ShaderError {
 /// the device.
 struct SharedDepthTarget {
     /// The Render Buffer Object representing the depth target.
     rbo_id: RBOId,
     /// Reference count. When this drops to zero, the RBO is deleted.
     refcount: usize,
 }
 
-#[cfg(feature = "debug")]
+#[cfg(debug_assertions)]
 impl Drop for SharedDepthTarget {
     fn drop(&mut self) {
         debug_assert!(thread::panicking() || self.refcount == 0);
     }
 }
 
 pub struct Device {
     gl: Rc<gl::Gl>,
--- a/gfx/webrender/src/resource_cache.rs
+++ b/gfx/webrender/src/resource_cache.rs
@@ -151,16 +151,24 @@ impl ImageTemplates {
     }
 }
 
 #[cfg_attr(feature = "capture", derive(Serialize))]
 #[cfg_attr(feature = "replay", derive(Deserialize))]
 struct CachedImageInfo {
     texture_cache_handle: TextureCacheHandle,
     dirty_rect: Option<DeviceUintRect>,
+    manual_eviction: bool,
+}
+
+#[cfg(debug_assertions)]
+impl Drop for CachedImageInfo {
+    fn drop(&mut self) {
+        debug_assert!(!self.manual_eviction, "Manual eviction requires cleanup");
+    }
 }
 
 #[cfg_attr(feature = "capture", derive(Serialize))]
 #[cfg_attr(feature = "replay", derive(Deserialize))]
 pub struct ResourceClassCache<K: Hash + Eq, V, U: Default> {
     resources: FastHashMap<K, V>,
     pub user_data: U,
 }
@@ -225,18 +233,18 @@ where
     pub fn try_get(&self, key: &K) -> Option<&V> {
         self.resources.get(key)
     }
 
     pub fn insert(&mut self, key: K, value: V) {
         self.resources.insert(key, value);
     }
 
-    pub fn remove(&mut self, key: &K) {
-        self.resources.remove(key);
+    pub fn remove(&mut self, key: &K) -> Option<V> {
+        self.resources.remove(key)
     }
 
     pub fn get_mut(&mut self, key: &K) -> &mut V {
         self.resources.get_mut(key)
             .expect("Didn't find a cached resource with that ID!")
     }
 
     pub fn try_get_mut(&mut self, key: &K) -> Option<&mut V> {
@@ -250,30 +258,16 @@ where
     pub fn iter_mut(&mut self) -> IterMut<K, V> {
         self.resources.iter_mut()
     }
 
     pub fn clear(&mut self) {
         self.resources.clear();
     }
 
-    fn clear_keys<F>(&mut self, key_fun: F)
-    where
-        for<'r> F: Fn(&'r &K) -> bool,
-    {
-        let resources_to_destroy = self.resources
-            .keys()
-            .filter(&key_fun)
-            .cloned()
-            .collect::<Vec<_>>();
-        for key in resources_to_destroy {
-            let _ = self.resources.remove(&key).unwrap();
-        }
-    }
-
     pub fn retain<F>(&mut self, f: F)
     where
         F: FnMut(&K, &mut V) -> bool,
     {
         self.resources.retain(f);
     }
 }
 
@@ -336,16 +330,35 @@ pub enum ImageCacheError {
 #[cfg_attr(feature = "capture", derive(Serialize))]
 #[cfg_attr(feature = "replay", derive(Deserialize))]
 enum ImageResult {
     UntiledAuto(CachedImageInfo),
     Multi(ResourceClassCache<CachedImageKey, CachedImageInfo, ()>),
     Err(ImageCacheError),
 }
 
+impl ImageResult {
+    /// Releases any texture cache entries held alive by this ImageResult.
+    fn drop_from_cache(&mut self, texture_cache: &mut TextureCache) {
+        match *self {
+            ImageResult::UntiledAuto(ref mut entry) => {
+                texture_cache.mark_unused(&entry.texture_cache_handle);
+                entry.manual_eviction = false;
+            },
+            ImageResult::Multi(ref mut entries) => {
+                for (_, entry) in &mut entries.resources {
+                    texture_cache.mark_unused(&entry.texture_cache_handle);
+                    entry.manual_eviction = false;
+                }
+            },
+            ImageResult::Err(_) => {},
+        }
+    }
+}
+
 type ImageCache = ResourceClassCache<ImageKey, ImageResult, ()>;
 pub type FontInstanceMap = Arc<RwLock<FastHashMap<FontInstanceKey, FontInstance>>>;
 
 #[derive(Default)]
 struct Resources {
     font_templates: FastHashMap<FontKey, FontTemplate>,
     font_instances: FontInstanceMap,
     image_templates: ImageTemplates,
@@ -842,32 +855,24 @@ impl ResourceCache {
                 (Some(rect), None) => Some(rect),
                 (None, _) => None,
             },
             viewport_tiles: image.viewport_tiles,
         };
     }
 
     pub fn delete_image_template(&mut self, image_key: ImageKey) {
+        // Remove the template.
         let value = self.resources.image_templates.remove(image_key);
 
-        match self.cached_images.try_get(&image_key) {
-            Some(&ImageResult::UntiledAuto(ref entry)) => {
-                self.texture_cache.mark_unused(&entry.texture_cache_handle);
-            }
-            Some(&ImageResult::Multi(ref entries)) => {
-                for (_, entry) in &entries.resources {
-                    self.texture_cache.mark_unused(&entry.texture_cache_handle);
-                }
-            }
-            _ => {}
+        // Release the corresponding texture cache entry, if any.
+        if let Some(mut cached) = self.cached_images.remove(&image_key) {
+            cached.drop_from_cache(&mut self.texture_cache);
         }
 
-        self.cached_images.remove(&image_key);
-
         match value {
             Some(image) => if image.data.is_blob() {
                 self.blob_image_handler.as_mut().unwrap().delete(image_key);
                 self.blob_image_templates.remove(&image_key);
                 self.rasterized_blob_images.remove(&image_key);
             },
             None => {
                 warn!("Delete the non-exist key");
@@ -916,16 +921,17 @@ impl ResourceCache {
                 // the tiled/multi-entry variant.
                 let entry = e.into_mut();
                 if !request.is_untiled_auto() {
                     let untiled_entry = match entry {
                         &mut ImageResult::UntiledAuto(ref mut entry) => {
                             Some(mem::replace(entry, CachedImageInfo {
                                 texture_cache_handle: TextureCacheHandle::new(),
                                 dirty_rect: None,
+                                manual_eviction: false,
                             }))
                         }
                         _ => None
                     };
 
                     if let Some(untiled_entry) = untiled_entry {
                         let mut entries = ResourceClassCache::new();
                         let untiled_key = CachedImageKey {
@@ -938,32 +944,34 @@ impl ResourceCache {
                 }
                 entry
             }
             Vacant(entry) => {
                 entry.insert(if request.is_untiled_auto() {
                     ImageResult::UntiledAuto(CachedImageInfo {
                         texture_cache_handle: TextureCacheHandle::new(),
                         dirty_rect: Some(template.descriptor.full_rect()),
+                        manual_eviction: false,
                     })
                 } else {
                     ImageResult::Multi(ResourceClassCache::new())
                 })
             }
         };
 
         // If this image exists in the texture cache, *and* the dirty rect
         // in the cache is empty, then it is valid to use as-is.
         let entry = match *storage {
             ImageResult::UntiledAuto(ref mut entry) => entry,
             ImageResult::Multi(ref mut entries) => {
                 entries.entry(request.into())
                     .or_insert(CachedImageInfo {
                         texture_cache_handle: TextureCacheHandle::new(),
                         dirty_rect: Some(template.descriptor.full_rect()),
+                        manual_eviction: false,
                     })
             },
             ImageResult::Err(_) => panic!("Errors should already have been handled"),
         };
 
         let needs_upload = self.texture_cache.request(&entry.texture_cache_handle, gpu_cache);
 
         if !needs_upload && entry.dirty_rect.is_none() {
@@ -1566,16 +1574,17 @@ impl ResourceCache {
                             TextureFilter::Trilinear
                         } else {
                             TextureFilter::Linear
                         }
                     }
                 };
 
                 let eviction = if image_template.data.is_blob() {
+                    entry.manual_eviction = true;
                     Eviction::Manual
                 } else {
                     Eviction::Auto
                 };
 
                 //Note: at this point, the dirty rectangle is local to the descriptor space
                 self.texture_cache.update(
                     &mut entry.texture_cache_handle,
@@ -1595,17 +1604,19 @@ impl ResourceCache {
 
     pub fn end_frame(&mut self) {
         debug_assert_eq!(self.state, State::QueryResources);
         self.state = State::Idle;
     }
 
     pub fn clear(&mut self, what: ClearCache) {
         if what.contains(ClearCache::IMAGES) {
-            self.cached_images.clear();
+            for (_key, mut cached) in self.cached_images.resources.drain() {
+                cached.drop_from_cache(&mut self.texture_cache);
+            }
         }
         if what.contains(ClearCache::GLYPHS) {
             self.cached_glyphs.clear();
         }
         if what.contains(ClearCache::GLYPH_DIMENSIONS) {
             self.cached_glyph_dimensions.clear();
         }
         if what.contains(ClearCache::RENDER_TASKS) {
@@ -1615,26 +1626,17 @@ impl ResourceCache {
             self.texture_cache.clear();
         }
         if what.contains(ClearCache::RASTERIZED_BLOBS) {
             self.rasterized_blob_images.clear();
         }
     }
 
     pub fn clear_namespace(&mut self, namespace: IdNamespace) {
-        self.resources
-            .image_templates
-            .images
-            .retain(|key, _| key.0 != namespace);
-        self.cached_images
-            .clear_keys(|key| key.0 == namespace);
-
-        self.blob_image_templates.retain(|key, _| key.0 != namespace);
-
-        self.rasterized_blob_images.retain(|key, _| key.0 != namespace);
+        self.clear_images(|k| k.0 == namespace);
 
         self.resources.font_instances
             .write()
             .unwrap()
             .retain(|key, _| key.0 != namespace);
         for &key in self.resources.font_templates.keys().filter(|key| key.0 == namespace) {
             self.glyph_rasterizer.delete_font(key);
         }
@@ -1682,16 +1684,38 @@ impl ResourceCache {
                 RasterizedBlob::Tiled(map) => map.values().for_each(&mut accumulate),
                 RasterizedBlob::NonTiled(vec) => vec.iter().for_each(&mut accumulate),
             };
         }
         */
 
         report
     }
+
+    /// Properly deletes all images matching the predicate.
+    fn clear_images<F: Fn(&ImageKey) -> bool>(&mut self, f: F) {
+        let keys = self.resources.image_templates.images.keys().filter(|k| f(*k))
+            .cloned().collect::<SmallVec<[ImageKey; 16]>>();
+
+        for key in keys {
+            self.delete_image_template(key);
+        }
+
+        debug_assert!(!self.resources.image_templates.images.keys().any(&f));
+        debug_assert!(!self.cached_images.resources.keys().any(&f));
+        debug_assert!(!self.blob_image_templates.keys().any(&f));
+        debug_assert!(!self.rasterized_blob_images.keys().any(&f));
+
+    }
+}
+
+impl Drop for ResourceCache {
+    fn drop(&mut self) {
+        self.clear_images(|_| true);
+    }
 }
 
 pub fn get_blob_tiling(
     tiling: Option<TileSize>,
     descriptor: &ImageDescriptor,
     max_texture_size: u32,
 ) -> Option<TileSize> {
     if tiling.is_none() &&
--- a/gfx/webrender_bindings/revision.txt
+++ b/gfx/webrender_bindings/revision.txt
@@ -1,1 +1,4 @@
 15656cb497303703b4d541d3e14292259e4c5343
+
+Additional fixes cherry-picked from the upstream WebRender repository:
+https://github.com/servo/webrender/pull/3234