Bug 1647299 - Store a single array of primitives per prim list instead of per-cluster. r=gw
authorNicolas Silva <nsilva@mozilla.com>
Wed, 24 Jun 2020 09:02:50 +0000
changeset 537080 26fbdf4c708744a2f3a4201ba9c5b2823f0e3010
parent 537079 05f582433d24ebe0c3882f88c2a2ef450606529d
child 537081 42e0a75220262188ee410cd2a358a8d9541b4a98
push id119795
push usernsilva@mozilla.com
push dateWed, 24 Jun 2020 09:06:02 +0000
treeherderautoland@eb37b6753d82 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgw
bugs1647299
milestone79.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1647299 - Store a single array of primitives per prim list instead of per-cluster. r=gw After this change, clusters just keep a range of indices in the prim list's instance array. Differential Revision: https://phabricator.services.mozilla.com/D80461
gfx/wr/webrender/src/batch.rs
gfx/wr/webrender/src/picture.rs
gfx/wr/webrender/src/prim_store/mod.rs
gfx/wr/webrender/src/scene_building.rs
--- a/gfx/wr/webrender/src/batch.rs
+++ b/gfx/wr/webrender/src/batch.rs
@@ -755,19 +755,22 @@ impl BatchBuilder {
         prim_headers: &mut PrimitiveHeaders,
         transforms: &mut TransformPalette,
         root_spatial_node_index: SpatialNodeIndex,
         surface_spatial_node_index: SpatialNodeIndex,
         z_generator: &mut ZBufferIdGenerator,
         composite_state: &mut CompositeState,
     ) {
         for cluster in &pic.prim_list.clusters {
-            profile_scope!("cluster");
-            // Add each run in this picture to the batch.
-            for prim_instance in &cluster.prim_instances {
+            for prim_instance in &pic.prim_list.prim_instances[cluster.prim_range()] {
+                if prim_instance.visibility_info == PrimitiveVisibilityIndex::INVALID {
+                    continue;
+                }
+
+                // Add each run in this picture to the batch.
                 self.add_prim_to_batch(
                     prim_instance,
                     cluster.spatial_node_index,
                     ctx,
                     gpu_cache,
                     render_tasks,
                     deferred_resolves,
                     prim_headers,
@@ -862,20 +865,16 @@ impl BatchBuilder {
         deferred_resolves: &mut Vec<DeferredResolve>,
         prim_headers: &mut PrimitiveHeaders,
         transforms: &mut TransformPalette,
         root_spatial_node_index: SpatialNodeIndex,
         surface_spatial_node_index: SpatialNodeIndex,
         z_generator: &mut ZBufferIdGenerator,
         composite_state: &mut CompositeState,
     ) {
-        if prim_instance.visibility_info == PrimitiveVisibilityIndex::INVALID {
-            return;
-        }
-
         #[cfg(debug_assertions)] //TODO: why is this needed?
         debug_assert_eq!(prim_instance.prepared_frame_id, render_tasks.frame_id());
 
         let is_chased = prim_instance.is_chased();
 
         let transform_id = transforms
             .get_id(
                 prim_spatial_node_index,
@@ -1281,18 +1280,17 @@ impl BatchBuilder {
                     specific_prim_address: prim_cache_address,
                     transform_id,
                 };
 
                 match picture.context_3d {
                     // Convert all children of the 3D hierarchy root into batches.
                     Picture3DContext::In { root_data: Some(ref list), .. } => {
                         for child in list {
-                            let cluster = &picture.prim_list.clusters[child.anchor.cluster_index];
-                            let child_prim_instance = &cluster.prim_instances[child.anchor.instance_index];
+                            let child_prim_instance = &picture.prim_list.prim_instances[child.anchor.instance_index];
                             let child_prim_info = &ctx.scratch.prim_info[child_prim_instance.visibility_info.0 as usize];
 
                             let child_pic_index = match child_prim_instance.kind {
                                 PrimitiveInstanceKind::Picture { pic_index, .. } => pic_index,
                                 _ => unreachable!(),
                             };
                             let pic = &ctx.prim_store.pictures[child_pic_index.0];
 
--- a/gfx/wr/webrender/src/picture.rs
+++ b/gfx/wr/webrender/src/picture.rs
@@ -127,16 +127,17 @@ use crate::render_task_graph::RenderTask
 use crate::render_target::RenderTargetKind;
 use crate::render_task::{RenderTask, RenderTaskLocation, BlurTaskCache, ClearMode};
 use crate::resource_cache::{ResourceCache, ImageGeneration};
 use crate::scene::SceneProperties;
 use smallvec::SmallVec;
 use std::{mem, u8, marker, u32};
 use std::sync::atomic::{AtomicUsize, Ordering};
 use std::collections::hash_map::Entry;
+use std::ops::Range;
 use crate::texture_cache::TextureCacheHandle;
 use crate::util::{MaxRect, VecHelper, RectHelpers, MatrixHelpers};
 use crate::filterdata::{FilterDataHandle};
 #[cfg(any(feature = "capture", feature = "replay"))]
 use ron;
 #[cfg(feature = "capture")]
 use crate::scene_builder_thread::InternerUpdates;
 #[cfg(any(feature = "capture", feature = "replay"))]
@@ -3920,32 +3921,33 @@ impl<'a> PictureUpdateState<'a> {
         clip_store: &ClipStore,
         data_stores: &mut DataStores,
     ) {
         if let Some(prim_list) = picture_primitives[pic_index.0].pre_update(
             self,
             frame_context,
         ) {
             for cluster in &prim_list.clusters {
-                if cluster.flags.contains(ClusterFlags::IS_PICTURE) {
-                    for prim_instance in &cluster.prim_instances {
-                        let child_pic_index = match prim_instance.kind {
-                            PrimitiveInstanceKind::Picture { pic_index, .. } => pic_index,
-                            _ => unreachable!(),
-                        };
-
-                        self.update(
-                            child_pic_index,
-                            picture_primitives,
-                            frame_context,
-                            gpu_cache,
-                            clip_store,
-                            data_stores,
-                        );
-                    }
+                if !cluster.flags.contains(ClusterFlags::IS_PICTURE) {
+                    continue;
+                }
+                for prim_instance in &prim_list.prim_instances[cluster.prim_range()] {
+                    let child_pic_index = match prim_instance.kind {
+                        PrimitiveInstanceKind::Picture { pic_index, .. } => pic_index,
+                        _ => unreachable!(),
+                    };
+
+                    self.update(
+                        child_pic_index,
+                        picture_primitives,
+                        frame_context,
+                        gpu_cache,
+                        clip_store,
+                        data_stores,
+                    );
                 }
             }
 
             picture_primitives[pic_index.0].post_update(
                 prim_list,
                 self,
                 frame_context,
                 data_stores,
@@ -3974,28 +3976,29 @@ impl<'a> PictureUpdateState<'a> {
                     surface.raster_spatial_node_index = fallback_raster_spatial_node;
                 }
                 surface.raster_spatial_node_index
             }
             None => fallback_raster_spatial_node,
         };
 
         for cluster in &picture.prim_list.clusters {
-            if cluster.flags.contains(ClusterFlags::IS_PICTURE) {
-                for instance in &cluster.prim_instances {
-                    let child_pic_index = match instance.kind {
-                        PrimitiveInstanceKind::Picture { pic_index, .. } => pic_index,
-                        _ => unreachable!(),
-                    };
-                    self.assign_raster_roots(
-                        child_pic_index,
-                        picture_primitives,
-                        new_fallback,
-                    );
-                }
+            if !cluster.flags.contains(ClusterFlags::IS_PICTURE) {
+                continue;
+            }
+            for instance in &picture.prim_list.prim_instances[cluster.prim_range()] {
+                let child_pic_index = match instance.kind {
+                    PrimitiveInstanceKind::Picture { pic_index, .. } => pic_index,
+                    _ => unreachable!(),
+                };
+                self.assign_raster_roots(
+                    child_pic_index,
+                    picture_primitives,
+                    new_fallback,
+                );
             }
         }
     }
 }
 
 #[derive(Debug, Copy, Clone, PartialEq)]
 #[cfg_attr(feature = "capture", derive(Serialize))]
 pub struct SurfaceIndex(pub usize);
@@ -4268,36 +4271,37 @@ bitflags! {
 pub struct PrimitiveCluster {
     /// The positioning node for this cluster.
     pub spatial_node_index: SpatialNodeIndex,
     /// The bounding rect of the cluster, in the local space of the spatial node.
     /// This is used to quickly determine the overall bounding rect for a picture
     /// during the first picture traversal, which is needed for local scale
     /// determination, and render task size calculations.
     bounding_rect: LayoutRect,
-    /// The list of primitive instances in this cluster.
-    pub prim_instances: Vec<PrimitiveInstance>,
+    /// The range of primitive instance indices associated with this cluster.
+    pub prim_range: Range<usize>,
     /// Various flags / state for this cluster.
     pub flags: ClusterFlags,
     /// An optional scroll root to use if this cluster establishes a picture cache slice.
     pub cache_scroll_root: Option<SpatialNodeIndex>,
 }
 
 impl PrimitiveCluster {
     /// Construct a new primitive cluster for a given positioning node.
     fn new(
         spatial_node_index: SpatialNodeIndex,
         flags: ClusterFlags,
+        first_instance_index: usize,
     ) -> Self {
         PrimitiveCluster {
             bounding_rect: LayoutRect::zero(),
             spatial_node_index,
             flags,
-            prim_instances: Vec::new(),
             cache_scroll_root: None,
+            prim_range: first_instance_index..first_instance_index
         }
     }
 
     /// Return true if this cluster is compatible with the given params
     pub fn is_compatible(
         &self,
         spatial_node_index: SpatialNodeIndex,
         flags: ClusterFlags,
@@ -4307,49 +4311,52 @@ impl PrimitiveCluster {
         // scrollbars into a single slice.
         if self.flags.contains(ClusterFlags::SCROLLBAR_CONTAINER) {
             return false;
         }
 
         self.flags == flags && self.spatial_node_index == spatial_node_index
     }
 
+    pub fn prim_range(&self) -> Range<usize> {
+        self.prim_range.clone()
+    }
+
     /// Add a primitive instance to this cluster, at the start or end
-    fn push(
+    fn add_instance(
         &mut self,
-        prim_instance: PrimitiveInstance,
-        prim_rect: LayoutRect,
+        culling_rect: &LayoutRect,
+        instance_index: usize,
     ) {
-        let culling_rect = prim_instance.local_clip_rect
-            .intersection(&prim_rect)
-            .unwrap_or_else(LayoutRect::zero);
-
-        self.bounding_rect = self.bounding_rect.union(&culling_rect);
-        self.prim_instances.push(prim_instance);
+        debug_assert_eq!(instance_index, self.prim_range.end);
+        self.bounding_rect = self.bounding_rect.union(culling_rect);
+        self.prim_range.end += 1;
     }
 }
 
 /// A list of primitive instances that are added to a picture
 /// This ensures we can keep a list of primitives that
 /// are pictures, for a fast initial traversal of the picture
 /// tree without walking the instance list.
 #[cfg_attr(feature = "capture", derive(Serialize))]
 pub struct PrimitiveList {
     /// List of primitives grouped into clusters.
     pub clusters: Vec<PrimitiveCluster>,
+    pub prim_instances: Vec<PrimitiveInstance>,
 }
 
 impl PrimitiveList {
     /// Construct an empty primitive list. This is
     /// just used during the take_context / restore_context
     /// borrow check dance, which will be removed as the
     /// picture traversal pass is completed.
     pub fn empty() -> Self {
         PrimitiveList {
             clusters: Vec::new(),
+            prim_instances: Vec::new(),
         }
     }
 
     /// Add a primitive instance to the end of the list
     pub fn add_prim(
         &mut self,
         prim_instance: PrimitiveInstance,
         prim_rect: LayoutRect,
@@ -4380,45 +4387,71 @@ impl PrimitiveList {
         if prim_flags.contains(PrimitiveFlags::IS_SCROLLBAR_CONTAINER) {
             flags.insert(ClusterFlags::SCROLLBAR_CONTAINER);
         }
 
         if prim_flags.contains(PrimitiveFlags::PREFER_COMPOSITOR_SURFACE) {
             flags.insert(ClusterFlags::PREFER_COMPOSITOR_SURFACE);
         }
 
+        let culling_rect = prim_instance.local_clip_rect
+            .intersection(&prim_rect)
+            .unwrap_or_else(LayoutRect::zero);
+
+        let instance_index = self.prim_instances.len();
+        self.prim_instances.push(prim_instance);
+
         if let Some(cluster) = self.clusters.last_mut() {
             if cluster.is_compatible(spatial_node_index, flags) {
-                cluster.push(prim_instance, prim_rect);
+                cluster.add_instance(&culling_rect, instance_index);
                 return;
             }
         }
 
         let mut cluster = PrimitiveCluster::new(
             spatial_node_index,
             flags,
+            instance_index,
         );
-        cluster.push(prim_instance, prim_rect);
+
+        cluster.add_instance(&culling_rect, instance_index);
         self.clusters.push(cluster);
     }
 
     /// Returns true if there are no clusters (and thus primitives)
     pub fn is_empty(&self) -> bool {
         self.clusters.is_empty()
     }
 
     /// Add an existing cluster to this prim list
-    pub fn add_cluster(&mut self, cluster: PrimitiveCluster) {
+    pub fn add_cluster(&mut self, mut cluster: PrimitiveCluster, prim_instances: &[PrimitiveInstance]) {
+        let start = self.prim_instances.len();
+        self.prim_instances.extend_from_slice(&prim_instances[cluster.prim_range()]);
+        let end = self.prim_instances.len();
+
+        cluster.prim_range = start..end;
         self.clusters.push(cluster);
     }
 
     /// Merge another primitive list into this one
-    pub fn extend(&mut self, prim_list: PrimitiveList) {
+    pub fn extend(&mut self, mut prim_list: PrimitiveList) {
+        let offset = self.prim_instances.len();
+        for cluster in &mut prim_list.clusters {
+            cluster.prim_range.start += offset;
+            cluster.prim_range.end += offset;
+        }
+
+        self.prim_instances.extend(prim_list.prim_instances);
         self.clusters.extend(prim_list.clusters);
     }
+
+    pub fn clear(&mut self) {
+        self.prim_instances.clear();
+        self.clusters.clear();
+    }
 }
 
 /// Defines configuration options for a given picture primitive.
 #[cfg_attr(feature = "capture", derive(Serialize))]
 pub struct PictureOptions {
     /// If true, WR should inflate the bounding rect of primitives when
     /// using a filter effect that requires inflation.
     pub inflate_if_required: bool,
@@ -4520,24 +4553,25 @@ impl PicturePrimitive {
         pt.add_item(format!("cluster_count: {:?}", self.prim_list.clusters.len()));
         pt.add_item(format!("estimated_local_rect: {:?}", self.estimated_local_rect));
         pt.add_item(format!("precise_local_rect: {:?}", self.precise_local_rect));
         pt.add_item(format!("spatial_node_index: {:?}", self.spatial_node_index));
         pt.add_item(format!("raster_config: {:?}", self.raster_config));
         pt.add_item(format!("requested_composite_mode: {:?}", self.requested_composite_mode));
 
         for cluster in &self.prim_list.clusters {
-            if cluster.flags.contains(ClusterFlags::IS_PICTURE) {
-                for instance in &cluster.prim_instances {
-                    let index = match instance.kind {
-                        PrimitiveInstanceKind::Picture { pic_index, .. } => pic_index,
-                        _ => unreachable!(),
-                    };
-                    pictures[index.0].print(pictures, index, pt);
-                }
+            if !cluster.flags.contains(ClusterFlags::IS_PICTURE) {
+                continue;
+            }
+            for instance in &self.prim_list.prim_instances[cluster.prim_range()] {
+                let index = match instance.kind {
+                    PrimitiveInstanceKind::Picture { pic_index, .. } => pic_index,
+                    _ => unreachable!(),
+                };
+                pictures[index.0].print(pictures, index, pt);
             }
         }
 
         pt.end_level();
     }
 
     /// Returns true if this picture supports segmented rendering.
     pub fn can_use_segments(&self) -> bool {
@@ -6031,17 +6065,17 @@ impl PicturePrimitive {
             if cluster.flags.contains(ClusterFlags::IS_BACKDROP_FILTER) {
                 let backdrop_to_world_mapper = SpaceMapper::new_with_target(
                     ROOT_SPATIAL_NODE_INDEX,
                     cluster.spatial_node_index,
                     LayoutRect::max_rect(),
                     frame_context.spatial_tree,
                 );
 
-                for prim_instance in &mut cluster.prim_instances {
+                for prim_instance in &mut self.prim_list.prim_instances[cluster.prim_range()] {
                     match prim_instance.kind {
                         PrimitiveInstanceKind::Backdrop { data_handle, .. } => {
                             // The actual size and clip rect of this primitive are determined by computing the bounding
                             // box of the projected rect of the backdrop-filter element onto the backdrop.
                             let prim_data = &mut data_stores.backdrop[data_handle];
                             let spatial_node_index = prim_data.kind.spatial_node_index;
 
                             // We cannot use the relative transform between the backdrop and the element because
--- a/gfx/wr/webrender/src/prim_store/mod.rs
+++ b/gfx/wr/webrender/src/prim_store/mod.rs
@@ -1809,19 +1809,17 @@ impl PrimitiveStore {
             );
         }
     }
 
     /// Returns the total count of primitive instances contained in pictures.
     pub fn prim_count(&self) -> usize {
         let mut prim_count = 0;
         for pic in &self.pictures {
-            for cluster in &pic.prim_list.clusters {
-                prim_count += cluster.prim_instances.len();
-            }
+            prim_count += pic.prim_list.prim_instances.len();
         }
         prim_count
     }
 
     /// Update visibility pass - update each primitive visibility struct, and
     /// build the clip chain instance if appropriate.
     pub fn update_visibility(
         &mut self,
@@ -1906,28 +1904,28 @@ impl PrimitiveStore {
                 // Primitive instances are normally reset in the main loop below,
                 // but we must also reset them in the rare case that the cluster
                 // visibility has changed (due to an invalid transform and/or
                 // backface visibility changing for this cluster).
                 // TODO(gw): This is difficult to test for in CI - as a follow up,
                 //           we should add a debug flag that validates the prim
                 //           instance is always reset every frame to catch similar
                 //           issues in future.
-                for prim_instance in &mut cluster.prim_instances {
+                for prim_instance in &mut prim_list.prim_instances[cluster.prim_range()] {
                     prim_instance.reset();
                 }
                 continue;
             }
 
             map_local_to_surface.set_target_spatial_node(
                 cluster.spatial_node_index,
                 frame_context.spatial_tree,
             );
 
-            for prim_instance in &mut cluster.prim_instances {
+            for prim_instance in &mut prim_list.prim_instances[cluster.prim_range()] {
                 prim_instance.reset();
 
                 if prim_instance.is_chased() {
                     #[cfg(debug_assertions)] // needed for ".id" part
                     println!("\tpreparing {:?} in {:?}", prim_instance.id, pic_index);
                     println!("\t{:?}", prim_instance.kind);
                 }
 
@@ -2613,17 +2611,18 @@ impl PrimitiveStore {
         profile_scope!("prepare_primitives");
         for (cluster_index, cluster) in prim_list.clusters.iter_mut().enumerate() {
             profile_scope!("cluster");
             pic_state.map_local_to_pic.set_target_spatial_node(
                 cluster.spatial_node_index,
                 frame_context.spatial_tree,
             );
 
-            for (prim_instance_index, prim_instance) in cluster.prim_instances.iter_mut().enumerate() {
+            for (idx, prim_instance) in (&mut prim_list.prim_instances[cluster.prim_range()]).iter_mut().enumerate() {
+                let prim_instance_index = cluster.prim_range.start + idx;
                 if prim_instance.visibility_info == PrimitiveVisibilityIndex::INVALID {
                     continue;
                 }
 
                 // The original clipped world rect was calculated during the initial visibility pass.
                 // However, it's possible that the dirty rect has got smaller, if tiles were not
                 // dirty. Intersecting with the dirty rect here eliminates preparing any primitives
                 // outside the dirty rect, and reduces the size of any off-screen surface allocations
--- a/gfx/wr/webrender/src/scene_building.rs
+++ b/gfx/wr/webrender/src/scene_building.rs
@@ -492,17 +492,17 @@ impl<'a> SceneBuilder<'a> {
                 };
 
                 // Open up clip chains on the stack on the new slice
                 slices.push(slice);
                 create_slice = false;
             }
 
             // Step through each prim instance, in order to collect shared clips for the slice.
-            for instance in &cluster.prim_instances {
+            for instance in &main_prim_list.prim_instances[cluster.prim_range()] {
                 // If the primitive clip chain is different, then we need to rebuild prim_clips.
                 update_shared_clips |= last_prim_clip_chain_id != instance.clip_chain_id;
                 last_prim_clip_chain_id = instance.clip_chain_id;
 
                 if update_shared_clips {
                     prim_clips.clear();
                     // Update the list of clips that apply to this primitive instance
                     add_clips(
@@ -537,19 +537,21 @@ impl<'a> SceneBuilder<'a> {
             }
 
             // If this cluster creates a slice after, then note that for next cluster
             create_slice |= cluster.flags.intersects(
                 ClusterFlags::CREATE_PICTURE_CACHE_POST | ClusterFlags::IS_CLEAR_PRIMITIVE
             );
 
             // Finally, add this cluster to the current slice
-            slices.last_mut().unwrap().prim_list.add_cluster(cluster);
+            slices.last_mut().unwrap().prim_list.add_cluster(cluster, &main_prim_list.prim_instances);
         }
 
+        main_prim_list.clear();
+
         // Step through the slices, creating picture cache wrapper instances.
         for (slice_index, slice) in slices.drain(..).enumerate() {
             let background_color = if slice_index == 0 {
                 self.config.background_color
             } else {
                 None
             };
 
@@ -3680,17 +3682,17 @@ impl FlattenedStackingContext {
 
                             // A fixed position slice is encountered within a scroll root. Only create
                             // a slice in this case if all the clips referenced by this cluster are also
                             // fixed position. There's no real point in creating slices for these cases,
                             // since we'll have to rasterize them as the scrolling clip moves anyway. It
                             // also allows us to retain subpixel AA in these cases. For these types of
                             // slices, the intra-slice dirty rect handling typically works quite well
                             // (a common case is parallax scrolling effects).
-                            for prim_instance in &cluster.prim_instances {
+                            for prim_instance in &self.prim_list.prim_instances[cluster.prim_range()] {
                                 let mut current_clip_chain_id = prim_instance.clip_chain_id;
 
                                 while current_clip_chain_id != ClipChainId::NONE {
                                     let clip_chain_node = &clip_store
                                         .clip_chain_nodes[current_clip_chain_id.0 as usize];
                                     let spatial_root = spatial_tree.find_scroll_root(clip_chain_node.spatial_node_index);
                                     if spatial_root != ROOT_SPATIAL_NODE_INDEX {
                                         return false;