Bug 1547632 - Remove unused local clip rect stored on WR picture primitives. r=emilio,kvark
authorGlenn Watson <github@intuitionlibrary.com>
Mon, 29 Apr 2019 16:16:11 +0000
changeset 530638 fa6be110a460610cfa89efcfeda08d9d57e4915b
parent 530637 2e58f3159dfba4fde76800e1a5f94c9a310406ec
child 530639 5293fd5ae92fe1a2759ea318bdca574fbc7892fd
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio, kvark
bugs1547632
milestone68.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 1547632 - Remove unused local clip rect stored on WR picture primitives. r=emilio,kvark Differential Revision: https://phabricator.services.mozilla.com/D29148
gfx/wr/webrender/src/batch.rs
gfx/wr/webrender/src/display_list_flattener.rs
gfx/wr/webrender/src/picture.rs
gfx/wr/webrender/src/prim_store/mod.rs
--- a/gfx/wr/webrender/src/batch.rs
+++ b/gfx/wr/webrender/src/batch.rs
@@ -1127,17 +1127,17 @@ impl AlphaBatchBuilder {
                                 }
 
                                 // Construct a local clip rect that ensures we only draw pixels where
                                 // the local bounds of the picture extend to within the edge tiles.
                                 let local_clip_rect = prim_info
                                     .combined_local_clip_rect
                                     .intersection(&picture.local_rect)
                                     .and_then(|rect| {
-                                        rect.intersection(&picture.local_clip_rect)
+                                        rect.intersection(&tile_cache.local_clip_rect)
                                     });
 
                                 if let Some(local_clip_rect) = local_clip_rect {
                                     // Step through each tile in the cache, and draw it with an image
                                     // brush primitive if visible.
 
                                     let kind = BatchKind::Brush(
                                         BrushBatchKind::Image(ImageBufferKind::Texture2DArray)
--- a/gfx/wr/webrender/src/display_list_flattener.rs
+++ b/gfx/wr/webrender/src/display_list_flattener.rs
@@ -519,17 +519,16 @@ impl<'a> DisplayListFlattener<'a> {
             Picture3DContext::Out,
             self.scene.root_pipeline_id.unwrap(),
             None,
             true,
             true,
             RasterSpace::Screen,
             prim_list,
             main_scroll_root,
-            LayoutRect::max_rect(),
             Some(tile_cache),
             PictureOptions::default(),
         ));
 
         let instance = PrimitiveInstance::new(
             LayoutPoint::zero(),
             LayoutRect::max_rect(),
             PrimitiveInstanceKind::Picture {
@@ -1644,24 +1643,16 @@ impl<'a> DisplayListFlattener<'a> {
         };
 
         if stacking_context.create_tile_cache {
             self.setup_picture_caching(
                 &mut stacking_context.primitives,
             );
         }
 
-        // An arbitrary large clip rect. For now, we don't
-        // specify a clip specific to the stacking context.
-        // However, now that they are represented as Picture
-        // primitives, we can apply any kind of clip mask
-        // to them, as for a normal primitive. This is needed
-        // to correctly handle some CSS cases (see #1957).
-        let max_clip = LayoutRect::max_rect();
-
         let (leaf_context_3d, leaf_composite_mode, leaf_output_pipeline_id) = match stacking_context.context_3d {
             // TODO(gw): For now, as soon as this picture is in
             //           a 3D context, we draw it to an intermediate
             //           surface and apply plane splitting. However,
             //           there is a large optimization opportunity here.
             //           During culling, we can check if there is actually
             //           perspective present, and skip the plane splitting
             //           completely when that is not the case.
@@ -1695,17 +1686,16 @@ impl<'a> DisplayListFlattener<'a> {
                 true,
                 stacking_context.is_backface_visible,
                 stacking_context.requested_raster_space,
                 PrimitiveList::new(
                     stacking_context.primitives,
                     &self.interners,
                 ),
                 stacking_context.spatial_node_index,
-                max_clip,
                 None,
                 PictureOptions::default(),
             ))
         );
 
         // Create a chain of pictures based on presence of filters,
         // mix-blend-mode and/or 3d rendering context containers.
 
@@ -1743,17 +1733,16 @@ impl<'a> DisplayListFlattener<'a> {
                     true,
                     stacking_context.is_backface_visible,
                     stacking_context.requested_raster_space,
                     PrimitiveList::new(
                         prims,
                         &self.interners,
                     ),
                     stacking_context.spatial_node_index,
-                    max_clip,
                     None,
                     PictureOptions::default(),
                 ))
             );
 
             cur_instance = create_prim_instance(
                 current_pic_index,
                 PictureCompositeKey::Identity,
@@ -1811,17 +1800,16 @@ impl<'a> DisplayListFlattener<'a> {
                     true,
                     stacking_context.is_backface_visible,
                     stacking_context.requested_raster_space,
                     PrimitiveList::new(
                         vec![cur_instance.clone()],
                         &self.interners,
                     ),
                     stacking_context.spatial_node_index,
-                    max_clip,
                     None,
                     PictureOptions::default(),
                 ))
             );
 
             current_pic_index = filter_pic_index;
             cur_instance = create_prim_instance(
                 current_pic_index,
@@ -1866,17 +1854,16 @@ impl<'a> DisplayListFlattener<'a> {
                     true,
                     stacking_context.is_backface_visible,
                     stacking_context.requested_raster_space,
                     PrimitiveList::new(
                         vec![cur_instance.clone()],
                         &self.interners,
                     ),
                     stacking_context.spatial_node_index,
-                    max_clip,
                     None,
                     PictureOptions::default(),
                 ))
             );
 
             current_pic_index = blend_pic_index;
             cur_instance = create_prim_instance(
                 blend_pic_index,
@@ -2120,17 +2107,16 @@ impl<'a> DisplayListFlattener<'a> {
     }
 
     pub fn pop_all_shadows(
         &mut self,
     ) {
         assert!(!self.pending_shadow_items.is_empty(), "popped shadows, but none were present");
 
         let pipeline_id = self.sc_stack.last().unwrap().pipeline_id;
-        let max_clip = LayoutRect::max_rect();
         let mut items = mem::replace(&mut self.pending_shadow_items, VecDeque::new());
 
         //
         // The pending_shadow_items queue contains a list of shadows and primitives
         // that were pushed during the active shadow context. To process these, we:
         //
         // Iterate the list, popping an item from the front each iteration.
         //
@@ -2237,17 +2223,16 @@ impl<'a> DisplayListFlattener<'a> {
                                 is_passthrough,
                                 is_backface_visible,
                                 raster_space,
                                 PrimitiveList::new(
                                     prims,
                                     &self.interners,
                                 ),
                                 pending_shadow.clip_and_scroll.spatial_node_index,
-                                max_clip,
                                 None,
                                 options,
                             ))
                         );
 
                         let shadow_pic_key = PictureKey::new(
                             true,
                             LayoutSize::zero(),
@@ -3009,17 +2994,16 @@ impl FlattenedStackingContext {
                 true,
                 self.is_backface_visible,
                 self.requested_raster_space,
                 PrimitiveList::new(
                     mem::replace(&mut self.primitives, Vec::new()),
                     interners,
                 ),
                 self.spatial_node_index,
-                LayoutRect::max_rect(),
                 None,
                 PictureOptions::default(),
             ))
         );
 
         let prim_instance = create_prim_instance(
             pic_index,
             PictureCompositeKey::Identity,
--- a/gfx/wr/webrender/src/picture.rs
+++ b/gfx/wr/webrender/src/picture.rs
@@ -600,16 +600,18 @@ pub struct TileCache {
     /// List of reference primitive information used for
     /// correlating the position between display lists.
     reference_prims: ReferencePrimitiveList,
     /// The root clip chain for this tile cache.
     root_clip_chain_id: ClipChainId,
     /// If true, this tile cache is enabled. For now, it doesn't
     /// support tile caching if the surface is not the main framebuffer.
     pub is_enabled: bool,
+    /// Local clip rect for this tile cache.
+    pub local_clip_rect: LayoutRect,
 }
 
 /// Stores information about a primitive in the cache that we will
 /// try to use to correlate positions between display lists.
 #[derive(Clone)]
 struct ReferencePrimitive {
     uid: ItemUid,
     local_pos: LayoutPoint,
@@ -724,16 +726,17 @@ impl TileCache {
             tile_count: TileSize::zero(),
             scroll_offset: None,
             pending_blits: Vec::new(),
             world_bounding_rect: WorldRect::zero(),
             root_clip_rect: WorldRect::max_rect(),
             reference_prims,
             root_clip_chain_id,
             is_enabled: true,
+            local_clip_rect: LayoutRect::zero(),
         }
     }
 
     /// Get the tile coordinates for a given rectangle.
     fn get_tile_coords_for_rect(
         &self,
         rect: &WorldRect,
     ) -> (TileOffset, TileOffset) {
@@ -1375,38 +1378,38 @@ impl TileCache {
     /// any late tile invalidations, and sets up the dirty rect and
     /// set of tile blits.
     pub fn post_update(
         &mut self,
         resource_cache: &mut ResourceCache,
         gpu_cache: &mut GpuCache,
         frame_context: &FrameVisibilityContext,
         scratch: &mut PrimitiveScratchBuffer,
-    ) -> LayoutRect {
+    ) {
         self.dirty_region.clear();
         self.pending_blits.clear();
 
         // If the tile cache is disabled, just return a no-op local clip rect.
         if !self.is_enabled {
-            return LayoutRect::max_rect();
+            return;
         }
 
         // Skip all tiles if completely off-screen.
         if !self.world_bounding_rect.intersects(&frame_context.screen_world_rect) {
-            return LayoutRect::zero();
+            return;
         }
 
         let map_surface_to_world: SpaceMapper<LayoutPixel, WorldPixel> = SpaceMapper::new_with_target(
             ROOT_SPATIAL_NODE_INDEX,
             self.spatial_node_index,
             frame_context.screen_world_rect,
             frame_context.clip_scroll_tree,
         );
 
-        let local_clip_rect = map_surface_to_world
+        self.local_clip_rect = map_surface_to_world
             .unmap(&self.world_bounding_rect)
             .expect("bug: unable to map local clip rect");
 
         // Step through each tile and invalidate if the dependencies have changed.
         for (i, tile) in self.tiles.iter_mut().enumerate() {
             // Deal with any potential world clips. Check to see if they are
             // outside the tile cache bounding rect. If they are, they're not
             // relevant and we don't care if they move relative to the content
@@ -1590,18 +1593,16 @@ impl TileCache {
         // If we end up with too many dirty rects, then it's going to be a lot
         // of extra draw calls to submit (since we currently just submit every
         // draw call for every dirty rect). In this case, bail out and work
         // with a single, large dirty rect. In future we can consider improving
         // on this by supporting batching per dirty region.
         if self.dirty_region.dirty_rects.len() > MAX_DIRTY_RECTS {
             self.dirty_region.collapse();
         }
-
-        local_clip_rect
     }
 
     pub fn tile_dimensions(testing: bool) -> DeviceIntSize {
         if testing {
             size2(TILE_SIZE_TESTING, TILE_SIZE_TESTING)
         } else {
             size2(TILE_SIZE_WIDTH, TILE_SIZE_HEIGHT)
         }
@@ -2200,19 +2201,16 @@ pub struct PicturePrimitive {
     pub local_rect: LayoutRect,
 
     /// If false, this picture needs to (re)build segments
     /// if it supports segment rendering. This can occur
     /// if the local rect of the picture changes due to
     /// transform animation and/or scrolling.
     pub segments_are_valid: bool,
 
-    /// Local clip rect for this picture.
-    pub local_clip_rect: LayoutRect,
-
     /// If Some(..) the tile cache that is associated with this picture.
     #[cfg_attr(feature = "capture", serde(skip))] //TODO
     pub tile_cache: Option<TileCache>,
 
     /// The config options for this picture.
     options: PictureOptions,
 }
 
@@ -2221,19 +2219,16 @@ impl PicturePrimitive {
         &self,
         pictures: &[Self],
         self_index: PictureIndex,
         pt: &mut T,
     ) {
         pt.new_level(format!("{:?}", self_index));
         pt.add_item(format!("prim_count: {:?}", self.prim_list.prim_instances.len()));
         pt.add_item(format!("local_rect: {:?}", self.local_rect));
-        if self.apply_local_clip_rect {
-            pt.add_item(format!("local_clip_rect: {:?}", self.local_clip_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 (index, _) in &self.prim_list.pictures {
             pictures[index.0].print(pictures, *index, pt);
         }
 
@@ -2315,17 +2310,16 @@ impl PicturePrimitive {
         context_3d: Picture3DContext<OrderedPictureChild>,
         pipeline_id: PipelineId,
         frame_output_pipeline_id: Option<PipelineId>,
         apply_local_clip_rect: bool,
         is_backface_visible: bool,
         requested_raster_space: RasterSpace,
         prim_list: PrimitiveList,
         spatial_node_index: SpatialNodeIndex,
-        local_clip_rect: LayoutRect,
         tile_cache: Option<TileCache>,
         options: PictureOptions,
     ) -> Self {
         PicturePrimitive {
             prim_list,
             state: None,
             secondary_render_task_id: None,
             requested_composite_mode,
@@ -2334,17 +2328,16 @@ impl PicturePrimitive {
             frame_output_pipeline_id,
             extra_gpu_data_handle: GpuCacheHandle::new(),
             apply_local_clip_rect,
             is_backface_visible,
             pipeline_id,
             requested_raster_space,
             spatial_node_index,
             local_rect: LayoutRect::zero(),
-            local_clip_rect,
             tile_cache,
             options,
             segments_are_valid: false,
         }
     }
 
     pub fn take_context(
         &mut self,
--- a/gfx/wr/webrender/src/prim_store/mod.rs
+++ b/gfx/wr/webrender/src/prim_store/mod.rs
@@ -2015,17 +2015,17 @@ impl PrimitiveStore {
         }
 
         let pic = &mut self.pictures[pic_index.0];
 
         if let Some(RasterConfig { composite_mode: PictureCompositeMode::TileCache { .. }, .. }) = pic.raster_config {
             let mut tile_cache = frame_state.tile_cache.take().unwrap();
 
             // Build the dirty region(s) for this tile cache.
-            pic.local_clip_rect = tile_cache.post_update(
+            tile_cache.post_update(
                 frame_state.resource_cache,
                 frame_state.gpu_cache,
                 frame_context,
                 frame_state.scratch,
             );
 
             pic.tile_cache = Some(tile_cache);
         }