Bug 1558926 - Part 5: Store a reference to cached display item data in DisplayItemRef

Differential Revision: https://phabricator.services.mozilla.com/D60761

--HG--
extra : moz-landing-system : lando

gfx/wr/webrender/src/scene.rs

@@ -2,7 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-use api::{BuiltDisplayList, ColorF, DynamicProperties, Epoch, FontRenderMode};
+use api::{BuiltDisplayList, DisplayItemCache, ColorF, DynamicProperties, Epoch, FontRenderMode};
 use api::{PipelineId, PropertyBinding, PropertyBindingId, MixBlendMode, StackingContext};
 use api::units::*;
 use crate::composite::CompositorKind;
@@ -134,6 +134,7 @@ pub struct ScenePipeline {
     pub content_size: LayoutSize,
     pub background_color: Option<ColorF>,
     pub display_list: BuiltDisplayList,
+    pub display_list_cache: DisplayItemCache,
 }
 
 /// A complete representation of the layout bundling visible pipelines together.
@@ -168,12 +169,20 @@ impl Scene {
         viewport_size: LayoutSize,
         content_size: LayoutSize,
     ) {
+        let pipeline = self.pipelines.remove(&pipeline_id);
+        let mut display_list_cache = pipeline.map_or(Default::default(), |p| {
+            p.display_list_cache
+        });
+
+        display_list_cache.update(&display_list);
+
         let new_pipeline = ScenePipeline {
             pipeline_id,
             viewport_size,
             content_size,
             background_color,
             display_list,
+            display_list_cache,
         };
 
         self.pipelines.insert(pipeline_id, new_pipeline);

gfx/wr/webrender/src/scene_building.rs

@@ -449,8 +449,9 @@ impl<'a> SceneBuilder<'a> {
             device_pixel_scale,
         );
 
+        let cache = &root_pipeline.display_list_cache;
         builder.build_items(
-            &mut root_pipeline.display_list.iter(),
+            &mut root_pipeline.display_list.iter_with_cache(cache),
             root_pipeline.pipeline_id,
             true,
         );
@@ -687,23 +688,45 @@ impl<'a> SceneBuilder<'a> {
         apply_pipeline_clip: bool,
     ) {
         loop {
-            let subtraversal = {
-                let item = match traversal.next() {
-                    Some(item) => item,
-                    None => break,
-                };
+            let item = match traversal.next() {
+                Some(item) => item,
+                None => break,
+            };
 
-                match item.item() {
-                    DisplayItem::PopReferenceFrame |
-                    DisplayItem::PopStackingContext => return,
-                    _ => (),
+            let subtraversal = match item.item() {
+                DisplayItem::PushStackingContext(ref info) => {
+                    let space = self.get_space(&info.spatial_id);
+                    let mut subtraversal = item.sub_iter();
+                    self.build_stacking_context(
+                        &mut subtraversal,
+                        pipeline_id,
+                        &info.stacking_context,
+                        space,
+                        info.origin,
+                        item.filters(),
+                        &item.filter_datas(),
+                        item.filter_primitives(),
+                        info.prim_flags,
+                        apply_pipeline_clip,
+                    );
+                    Some(subtraversal)
                 }
-
-                self.build_item(
-                    item,
-                    pipeline_id,
-                    apply_pipeline_clip,
-                )
+                DisplayItem::PushReferenceFrame(ref info) => {
+                    let parent_space = self.get_space(&info.parent_spatial_id);
+                    let mut subtraversal = item.sub_iter();
+                    self.build_reference_frame(
+                        &mut subtraversal,
+                        pipeline_id,
+                        parent_space,
+                        info.origin,
+                        &info.reference_frame,
+                        apply_pipeline_clip,
+                    );
+                    Some(subtraversal)
+                }
+                DisplayItem::PopReferenceFrame |
+                DisplayItem::PopStackingContext => return,
+                _ => None,
             };
 
             // If build_item created a sub-traversal, we need `traversal` to have the
@@ -711,6 +734,8 @@ impl<'a> SceneBuilder<'a> {
             if let Some(mut subtraversal) = subtraversal {
                 subtraversal.merge_debug_stats_from(traversal);
                 *traversal = subtraversal;
+            } else {
+                self.build_item(item, pipeline_id, apply_pipeline_clip);
             }
         }
 
@@ -958,8 +983,10 @@ impl<'a> SceneBuilder<'a> {
 
         self.rf_mapper.push_scope();
         self.iframe_depth += 1;
+
+        let cache = &pipeline.display_list_cache;
         self.build_items(
-            &mut pipeline.display_list.iter(),
+            &mut pipeline.display_list.iter_with_cache(cache),
             pipeline.pipeline_id,
             true,
         );
@@ -1061,10 +1088,10 @@ impl<'a> SceneBuilder<'a> {
 
     fn build_item<'b>(
         &'b mut self,
-        item: DisplayItemRef<'a, 'b>,
+        item: DisplayItemRef,
         pipeline_id: PipelineId,
         apply_pipeline_clip: bool,
-    ) -> Option<BuiltDisplayListIter<'a>> {
+    ) {
         match *item.item() {
             DisplayItem::Image(ref info) => {
                 let (layout, _, clip_and_scroll) = self.process_common_properties_with_bounds(
@@ -1299,36 +1326,6 @@ impl<'a> SceneBuilder<'a> {
                     item.gradient_stops(),
                 );
             }
-            DisplayItem::PushStackingContext(ref info) => {
-                let space = self.get_space(&info.spatial_id);
-                let mut subtraversal = item.sub_iter();
-                self.build_stacking_context(
-                    &mut subtraversal,
-                    pipeline_id,
-                    &info.stacking_context,
-                    space,
-                    info.origin,
-                    item.filters(),
-                    item.filter_datas(),
-                    item.filter_primitives(),
-                    info.prim_flags,
-                    apply_pipeline_clip,
-                );
-                return Some(subtraversal);
-            }
-            DisplayItem::PushReferenceFrame(ref info) => {
-                let parent_space = self.get_space(&info.parent_spatial_id);
-                let mut subtraversal = item.sub_iter();
-                self.build_reference_frame(
-                    &mut subtraversal,
-                    pipeline_id,
-                    parent_space,
-                    info.origin,
-                    &info.reference_frame,
-                    apply_pipeline_clip,
-                );
-                return Some(subtraversal);
-            }
             DisplayItem::Iframe(ref info) => {
                 let space = self.get_space(&info.space_and_clip.spatial_id);
                 self.build_iframe(
@@ -1456,10 +1453,18 @@ impl<'a> SceneBuilder<'a> {
             DisplayItem::SetFilterData |
             DisplayItem::SetFilterPrimitives => {}
 
+            // Special items that are handled in the parent method
+            DisplayItem::PushStackingContext(..) |
+            DisplayItem::PushReferenceFrame(..) |
             DisplayItem::PopReferenceFrame |
             DisplayItem::PopStackingContext => {
                 unreachable!("Should have returned in parent method.")
             }
+
+            DisplayItem::ReuseItem(..) => {
+                unreachable!("Iterator logic error")
+            }
+
             DisplayItem::PushShadow(info) => {
                 let clip_and_scroll = self.get_clip_and_scroll(
                     &info.space_and_clip.clip_id,
@@ -1473,8 +1478,6 @@ impl<'a> SceneBuilder<'a> {
                 self.pop_all_shadows();
             }
         }
-
-        None
     }
 
     // Given a list of clip sources, a positioning node and

gfx/wr/webrender_api/src/display_item.rs

@@ -163,6 +163,8 @@ pub enum DisplayItem {
     PopReferenceFrame,
     PopStackingContext,
     PopAllShadows,
+
+    ReuseItem(ItemKey),
 }
 
 /// This is a "complete" version of the DisplayItem, with all implicit trailing
@@ -203,6 +205,8 @@ pub enum DebugDisplayItem {
     PopReferenceFrame,
     PopStackingContext,
     PopAllShadows,
+
+    ReuseItem(ItemKey),
 }
 
 #[derive(Clone, Copy, Debug, Default, Deserialize, PartialEq, Serialize, PeekPoke)]
@@ -1476,6 +1480,7 @@ impl DisplayItem {
             DisplayItem::Rectangle(..) => "rectangle",
             DisplayItem::ScrollFrame(..) => "scroll_frame",
             DisplayItem::SetGradientStops => "set_gradient_stops",
+            DisplayItem::ReuseItem(..) => "reuse_item",
             DisplayItem::StickyFrame(..) => "sticky_frame",
             DisplayItem::Text(..) => "text",
             DisplayItem::YuvImage(..) => "yuv_image",

gfx/wr/webrender_api/src/display_list.rs

@@ -223,6 +223,58 @@ pub struct ItemStats {
 
 pub struct DisplayItemRef<'a: 'b, 'b> {
     iter: &'b BuiltDisplayListIter<'a>,
+    cached_item: Option<&'a CachedDisplayItem>,
+}
+
+// Some of these might just become ItemRanges
+impl<'a, 'b> DisplayItemRef<'a, 'b> {
+    fn cached_or_iter_data<T>(
+        &self,
+        data: ItemRange<'a, T>
+    ) -> ItemRange<'a, T> {
+        self.cached_item.map_or(data, |i| i.data_as_item_range())
+    }
+
+    pub fn display_list(&self) -> &BuiltDisplayList {
+        self.iter.display_list()
+    }
+
+    // Creates a new iterator where this element's iterator is, to hack around borrowck.
+    pub fn sub_iter(&self) -> BuiltDisplayListIter<'a> {
+        BuiltDisplayListIter::new(self.iter.list, self.iter.data, self.iter.cache)
+    }
+
+    pub fn item(&self) -> &di::DisplayItem {
+        self.cached_item.map_or(&self.iter.cur_item, |i| i.item())
+    }
+
+    pub fn clip_chain_items(&self) -> ItemRange<di::ClipId> {
+        self.iter.cur_clip_chain_items
+    }
+
+    pub fn complex_clip(&self) -> ItemRange<di::ComplexClipRegion> {
+        self.iter.cur_complex_clip
+    }
+
+    pub fn glyphs(&self) -> ItemRange<GlyphInstance> {
+        self.cached_or_iter_data(self.iter.cur_glyphs)
+    }
+
+    pub fn gradient_stops(&self) -> ItemRange<di::GradientStop> {
+        self.cached_or_iter_data(self.iter.cur_stops)
+    }
+
+    pub fn filters(&self) -> ItemRange<di::FilterOp> {
+        self.iter.cur_filters
+    }
+
+    pub fn filter_datas(&self) -> &Vec<TempFilterData> {
+        &self.iter.cur_filter_data
+    }
+
+    pub fn filter_primitives(&self) -> ItemRange<di::FilterPrimitive> {
+        self.iter.cur_filter_primitives
+    }
 }
 
 #[derive(PartialEq)]
@@ -457,7 +509,18 @@ impl<'a> BuiltDisplayListIter<'a> {
     }
 
     pub fn as_ref<'b>(&'b self) -> DisplayItemRef<'a, 'b> {
-        DisplayItemRef { iter: self }
+        let cached_item = match self.cur_item {
+            di::DisplayItem::ReuseItem(key) => {
+                debug_assert!(self.cache.is_some(), "Cache marker without cache!");
+                self.cache.and_then(|c| c.get_item(key))
+            }
+            _ => None
+        };
+
+        DisplayItemRef {
+            iter: self,
+            cached_item
+        }
     }
 
     pub fn skip_current_stacking_context(&mut self) {
@@ -517,50 +580,6 @@ impl<'a> BuiltDisplayListIter<'a> {
     fn log_item_stats(&mut self) { /* no-op */ }
 }
 
-// Some of these might just become ItemRanges
-impl<'a, 'b> DisplayItemRef<'a, 'b> {
-    pub fn item(&self) -> &di::DisplayItem {
-        &self.iter.cur_item
-    }
-
-    pub fn complex_clip(&self) -> ItemRange<di::ComplexClipRegion> {
-        self.iter.cur_complex_clip
-    }
-
-    pub fn gradient_stops(&self) -> ItemRange<di::GradientStop> {
-        self.iter.cur_stops
-    }
-
-    pub fn glyphs(&self) -> ItemRange<GlyphInstance> {
-        self.iter.cur_glyphs
-    }
-
-    pub fn filters(&self) -> ItemRange<di::FilterOp> {
-        self.iter.cur_filters
-    }
-
-    pub fn filter_datas(&self) -> &Vec<TempFilterData> {
-        &self.iter.cur_filter_data
-    }
-
-    pub fn filter_primitives(&self) -> ItemRange<di::FilterPrimitive> {
-        self.iter.cur_filter_primitives
-    }
-
-    pub fn clip_chain_items(&self) -> ItemRange<di::ClipId> {
-        self.iter.cur_clip_chain_items
-    }
-
-    pub fn display_list(&self) -> &BuiltDisplayList {
-        self.iter.display_list()
-    }
-
-    // Creates a new iterator where this element's iterator is, to hack around borrowck.
-    pub fn sub_iter(&self) -> BuiltDisplayListIter<'a> {
-        BuiltDisplayListIter::new_with_list_and_data(self.iter.list, self.iter.data)
-    }
-}
-
 impl<'a, T> AuxIter<'a, T> {
     pub fn new(item: T, mut data: &'a [u8]) -> Self {
         let mut size = 0usize;
@@ -674,6 +693,7 @@ impl Serialize for BuiltDisplayList {
                 Real::PopReferenceFrame => Debug::PopReferenceFrame,
                 Real::PopStackingContext => Debug::PopStackingContext,
                 Real::PopAllShadows => Debug::PopAllShadows,
+                Real::ReuseItem(k) => Debug::ReuseItem(k),
             };
             seq.serialize_element(&serial_di)?
         }
@@ -777,6 +797,7 @@ impl<'de> Deserialize<'de> for BuiltDisplayList {
                 Debug::PopStackingContext => Real::PopStackingContext,
                 Debug::PopReferenceFrame => Real::PopReferenceFrame,
                 Debug::PopAllShadows => Real::PopAllShadows,
+                Debug::ReuseItem(k) => Real::ReuseItem(k),
             };
             poke_into_vec(&item, &mut data);
             // the aux data is serialized after the item, hence the temporary

gfx/wr/wrench/src/yaml_frame_writer.rs

@@ -1448,6 +1448,10 @@ impl YamlFrameWriter {
                 DisplayItem::PopAllShadows => {
                     str_node(&mut v, "type", "pop-all-shadows");
                 }
+                DisplayItem::ReuseItem(key) => {
+                    str_node(&mut v, "type", "reuse-item");
+                    usize_node(&mut v, "key", key as usize);
+                }
             }
             if !v.is_empty() {
                 list.push(Yaml::Hash(v));