Bug 1520687 - Include root content clip rect in picture caching world bounds. r=kvark
authorGlenn Watson <github@intuitionlibrary.com>
Thu, 17 Jan 2019 19:40:06 +0000
changeset 514319 80ea2afaa742b6b1c050767e4a1ff735f66cd2a0
parent 514318 ed73c6c4e5d568ef815fc15597e709a4a4a1be7b
child 514320 3ce47eeea41a4e62bd52be3aea7d720933a8d24a
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskvark
bugs1520687
milestone66.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 1520687 - Include root content clip rect in picture caching world bounds. r=kvark Differential Revision: https://phabricator.services.mozilla.com/D16782
gfx/wr/webrender/src/clip.rs
gfx/wr/webrender/src/picture.rs
gfx/wr/webrender/src/prim_store/mod.rs
gfx/wr/webrender/src/renderer.rs
--- a/gfx/wr/webrender/src/clip.rs
+++ b/gfx/wr/webrender/src/clip.rs
@@ -1029,17 +1029,17 @@ impl ClipItem {
         }
         ClipItem::BoxShadow(source)
     }
 
     // Get an optional clip rect that a clip source can provide to
     // reduce the size of a primitive region. This is typically
     // used to eliminate redundant clips, and reduce the size of
     // any clip mask that eventually gets drawn.
-    fn get_local_clip_rect(&self, local_pos: LayoutPoint) -> Option<LayoutRect> {
+    pub fn get_local_clip_rect(&self, local_pos: LayoutPoint) -> Option<LayoutRect> {
         let size = match *self {
             ClipItem::Rectangle(size, ClipMode::Clip) => Some(size),
             ClipItem::Rectangle(_, ClipMode::ClipOut) => None,
             ClipItem::RoundedRectangle(size, _, ClipMode::Clip) => Some(size),
             ClipItem::RoundedRectangle(_, _, ClipMode::ClipOut) => None,
             ClipItem::Image { repeat, size, .. } => {
                 if repeat {
                     None
--- a/gfx/wr/webrender/src/picture.rs
+++ b/gfx/wr/webrender/src/picture.rs
@@ -11,17 +11,17 @@ use api::{DebugFlags, DeviceVector2D};
 use box_shadow::{BLUR_SAMPLE_SCALE};
 use clip::{ClipStore, ClipChainId, ClipChainNode, ClipItem};
 use clip_scroll_tree::{ROOT_SPATIAL_NODE_INDEX, ClipScrollTree, SpatialNodeIndex, CoordinateSystemId};
 #[cfg(feature = "debug_renderer")]
 use debug_colors;
 use device::TextureFilter;
 use euclid::{TypedScale, vec3, TypedRect, TypedPoint2D, TypedSize2D};
 use euclid::approxeq::ApproxEq;
-use frame_builder::FrameVisibilityContext;
+use frame_builder::{FrameVisibilityContext, FrameVisibilityState};
 use intern::ItemUid;
 use internal_types::{FastHashMap, FastHashSet, PlaneSplitter};
 use frame_builder::{FrameBuildingContext, FrameBuildingState, PictureState, PictureContext};
 use gpu_cache::{GpuCache, GpuCacheAddress, GpuCacheHandle};
 use gpu_types::{TransformPalette, TransformPaletteId, UvRectKind};
 use plane_split::{Clipper, Polygon, Splitter};
 use prim_store::{PictureIndex, PrimitiveInstance, SpaceMapper, VisibleFace, PrimitiveInstanceKind};
 use prim_store::{get_raster_rects, PrimitiveScratchBuffer, VectorKey, PointKey};
@@ -310,22 +310,36 @@ impl TileDescriptor {
         self.image_keys.reset();
         self.transforms.reset();
     }
 
     /// Return true if the content of the tile is the same
     /// as last frame. This doesn't check validity of the
     /// tile based on the currently valid regions.
     fn is_same_content(&self) -> bool {
-        self.image_keys.is_valid() &&
-        self.opacity_bindings.is_valid() &&
-        self.clip_uids.is_valid() &&
-        self.clip_vertices.is_valid() &&
-        self.prims.is_valid() &&
-        self.transforms.is_valid()
+        if !self.image_keys.is_valid() {
+            return false;
+        }
+        if !self.opacity_bindings.is_valid() {
+            return false;
+        }
+        if !self.clip_uids.is_valid() {
+            return false;
+        }
+        if !self.clip_vertices.is_valid() {
+            return false;
+        }
+        if !self.prims.is_valid() {
+            return false;
+        }
+        if !self.transforms.is_valid() {
+            return false;
+        }
+
+        true
     }
 }
 
 /// Represents the dirty region of a tile cache picture.
 /// In future, we will want to support multiple dirty
 /// regions.
 #[derive(Debug)]
 pub struct DirtyRegion {
@@ -366,16 +380,19 @@ pub struct TileCache {
     /// A list of blits from the framebuffer to be applied during this frame.
     pub pending_blits: Vec<TileBlit>,
     /// The current world bounding rect of this tile cache. This is used
     /// to derive a local clip rect, such that we don't obscure in the
     /// z-buffer any items placed earlier in the render order (such as
     /// scroll bars in gecko, when the content overflows under the
     /// scroll bar).
     world_bounding_rect: WorldRect,
+    /// World space clip rect of the root clipping node. Every primitive
+    /// has this as the root of the clip chain attached to the primitive.
+    root_clip_rect: WorldRect,
     /// 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,
 }
 
 /// Stores information about a primitive in the cache that we will
@@ -492,16 +509,17 @@ impl TileCache {
             dirty_region: None,
             needs_update: true,
             world_origin: WorldPoint::zero(),
             world_tile_size: WorldSize::zero(),
             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,
         }
     }
 
     /// Get the tile coordinates for a given rectangle.
     fn get_tile_coords_for_rect(
         &self,
@@ -524,18 +542,17 @@ impl TileCache {
         (p0, p1)
     }
 
     /// Update transforms, opacity bindings and tile rects.
     pub fn pre_update(
         &mut self,
         pic_rect: LayoutRect,
         frame_context: &FrameVisibilityContext,
-        resource_cache: &ResourceCache,
-        retained_tiles: &mut RetainedTiles,
+        frame_state: &mut FrameVisibilityState,
     ) {
         // Work out the scroll offset to apply to the world reference point.
         let scroll_transform = frame_context.clip_scroll_tree.get_relative_transform(
             ROOT_SPATIAL_NODE_INDEX,
             self.spatial_node_index,
         ).expect("bug: unable to get scroll transform");
         let scroll_offset = WorldVector2D::new(
             scroll_transform.m41,
@@ -543,34 +560,34 @@ impl TileCache {
         );
         let scroll_delta = match self.scroll_offset {
             Some(prev) => prev - scroll_offset,
             None => WorldVector2D::zero(),
         };
         self.scroll_offset = Some(scroll_offset);
 
         // Pull any retained tiles from the previous scene.
-        let world_offset = if retained_tiles.tiles.is_empty() {
+        let world_offset = if frame_state.retained_tiles.tiles.is_empty() {
             None
         } else {
             assert!(self.tiles.is_empty());
-            self.tiles = mem::replace(&mut retained_tiles.tiles, Vec::new());
+            self.tiles = mem::replace(&mut frame_state.retained_tiles.tiles, Vec::new());
 
             // Get the positions of the reference primitives for this
             // new display list.
             let mut new_prim_map = FastHashMap::default();
             build_ref_prims(
                 &self.reference_prims.ref_prims,
                 &mut new_prim_map,
                 frame_context.clip_scroll_tree,
             );
 
             // Attempt to correlate them to work out which offset to apply.
             correlate_prim_maps(
-                &retained_tiles.ref_prims,
+                &frame_state.retained_tiles.ref_prims,
                 &new_prim_map,
             )
         }.unwrap_or(WorldVector2D::zero());
 
         // Assume no tiles are valid to draw by default
         self.tiles_to_draw.clear();
 
         self.map_local_to_world = SpaceMapper::new(
@@ -730,25 +747,47 @@ impl TileCache {
             // TODO(gw): Should we explicitly drop the tile texture cache handles here?
         }
 
         // TODO(gw): We don't actually need to update the prim dependencies each frame.
         //           For common cases, such as only being one main scroll root, we could
         //           detect this and skip the dependency update on scroll frames.
         self.needs_update = true;
         self.world_bounding_rect = WorldRect::zero();
+        self.root_clip_rect = WorldRect::max_rect();
+
+        // Calculate the world space of the root clip node, that every primitive has
+        // at the root of its clip chain (this is enforced by the per-pipeline-root
+        // clip node added implicitly during display list flattening). Doing it once
+        // here saves doing it for every primitive during update_prim_dependencies.
+        let root_clip_chain_node = &frame_state
+            .clip_store
+            .clip_chain_nodes[self.root_clip_chain_id.0 as usize];
+        let root_clip_node = &frame_state
+            .resources
+            .clip_data_store[root_clip_chain_node.handle];
+        if let Some(clip_rect) = root_clip_node.item.get_local_clip_rect(root_clip_chain_node.local_pos) {
+            self.map_local_to_world.set_target_spatial_node(
+                root_clip_chain_node.spatial_node_index,
+                frame_context.clip_scroll_tree,
+            );
+
+            if let Some(world_clip_rect) = self.map_local_to_world.map(&clip_rect) {
+                self.root_clip_rect = world_clip_rect;
+            }
+        }
 
         // Do tile invalidation for any dependencies that we know now.
         for tile in &mut self.tiles {
             // Start frame assuming that the tile has the same content.
             tile.is_same_content = true;
 
             // Content has changed if any images have changed
             for image_key in tile.descriptor.image_keys.items() {
-                if resource_cache.is_image_dirty(*image_key) {
+                if frame_state.resource_cache.is_image_dirty(*image_key) {
                     tile.is_same_content = false;
                     break;
                 }
             }
 
             // Content has changed if any opacity bindings changed.
             for binding in tile.descriptor.opacity_bindings.items() {
                 if let OpacityBinding::Binding(id) = binding {
@@ -936,38 +975,39 @@ impl TileCache {
                 ClipItem::Rectangle(size, ClipMode::Clip) => {
                     let clip_spatial_node = &clip_scroll_tree.spatial_nodes[clip_chain_node.spatial_node_index.0 as usize];
 
                     let local_clip_rect = LayoutRect::new(
                         clip_chain_node.local_pos,
                         size,
                     );
 
-                    // If the clip rect is in the same spatial node, it can be handled by the
-                    // local clip rect.
-                    if clip_chain_node.spatial_node_index == prim_instance.spatial_node_index {
-                        culling_rect = culling_rect.intersection(&local_clip_rect).unwrap_or(LayoutRect::zero());
-                        false
-                    } else if clip_spatial_node.coordinate_system_id == CoordinateSystemId(0) {
+                    if clip_spatial_node.coordinate_system_id == CoordinateSystemId(0) {
                         // Clips that are not in the root coordinate system are not axis-aligned,
                         // so we need to treat them as normal style clips with vertices.
                         match self.map_local_to_world.map(&local_clip_rect) {
                             Some(clip_world_rect) => {
                                 // Even if this ends up getting clipped out by the current clip
                                 // stack, we want to ensure the primitive gets added to the tiles
                                 // below, to ensure invalidation isn't tripped up by the wrong
                                 // number of primitives that affect this tile.
                                 world_clip_rect = world_clip_rect
                                     .intersection(&clip_world_rect)
                                     .unwrap_or(WorldRect::zero());
 
-                                world_clips.push((
-                                    clip_world_rect.into(),
-                                    clip_chain_node.spatial_node_index,
-                                ));
+                                // If the clip rect is in the same spatial node, it can be handled by the
+                                // local clip rect.
+                                if clip_chain_node.spatial_node_index == prim_instance.spatial_node_index {
+                                    culling_rect = culling_rect.intersection(&local_clip_rect).unwrap_or(LayoutRect::zero());
+                                } else {
+                                    world_clips.push((
+                                        clip_world_rect.into(),
+                                        clip_chain_node.spatial_node_index,
+                                    ));
+                                }
 
                                 false
                             }
                             None => {
                                 true
                             }
                         }
                     } else {
@@ -979,32 +1019,41 @@ impl TileCache {
                 ClipItem::Image { .. } |
                 ClipItem::BoxShadow(..) => {
                     true
                 }
             };
 
             if add_to_clip_deps {
                 clip_chain_uids.push(clip_chain_node.handle.uid());
-                clip_spatial_nodes.insert(clip_chain_node.spatial_node_index);
+
+                // If the clip has the same spatial node, the relative transform
+                // will always be the same, so there's no need to depend on it.
+                if clip_chain_node.spatial_node_index != self.spatial_node_index {
+                    clip_spatial_nodes.insert(clip_chain_node.spatial_node_index);
+                }
 
                 let local_clip_rect = LayoutRect::new(
                     clip_chain_node.local_pos,
                     LayoutSize::zero(),
                 );
                 if let Some(world_clip_rect) = self.map_local_to_world.map(&local_clip_rect) {
                     clip_vertices.push(world_clip_rect.origin);
                 }
             }
 
             current_clip_chain_id = clip_chain_node.parent_clip_chain_id;
         }
 
         if include_clip_rect {
-            self.world_bounding_rect = self.world_bounding_rect.union(&world_clip_rect);
+            // Intersect the calculated prim bounds with the root clip rect, to save
+            // having to process and transform the root clip rect in every primitive.
+            if let Some(clipped_world_rect) = world_clip_rect.intersection(&self.root_clip_rect) {
+                self.world_bounding_rect = self.world_bounding_rect.union(&clipped_world_rect);
+            }
         }
 
         self.map_local_to_world.set_target_spatial_node(
             prim_instance.spatial_node_index,
             clip_scroll_tree,
         );
         let world_culling_rect = self
             .map_local_to_world
@@ -1057,17 +1106,22 @@ impl TileCache {
                     world_culling_rect,
                 });
                 tile.descriptor.clip_uids.extend_from_slice(&clip_chain_uids);
                 for clip_vertex in &clip_vertices {
                     let clip_vertex = (*clip_vertex - tile.world_rect.origin.to_vector()).round();
                     tile.descriptor.clip_vertices.push(clip_vertex.into());
                 }
 
-                tile.transforms.insert(prim_instance.spatial_node_index);
+                // If the primitive has the same spatial node, the relative transform
+                // will always be the same, so there's no need to depend on it.
+                if prim_instance.spatial_node_index != self.spatial_node_index {
+                    tile.transforms.insert(prim_instance.spatial_node_index);
+                }
+
                 for spatial_node_index in &clip_spatial_nodes {
                     tile.transforms.insert(*spatial_node_index);
                 }
                 for (world_rect, spatial_node_index) in &world_clips {
                     tile.potential_clips.insert(world_rect.clone(), *spatial_node_index);
                 }
             }
         }
@@ -1188,30 +1242,40 @@ impl TileCache {
                         let mut label_pos = tile_device_rect.origin + DeviceVector2D::new(20.0, 30.0);
                         _scratch.push_debug_rect(
                             tile_device_rect,
                             debug_colors::GREEN,
                         );
                         _scratch.push_debug_string(
                             label_pos,
                             debug_colors::RED,
-                            format!("{:?} {:?}", tile.id, tile.handle),
+                            format!("{:?} {:?} {:?}", tile.id, tile.handle, tile.world_rect),
                         );
                         label_pos.y += 20.0;
                         _scratch.push_debug_string(
                             label_pos,
                             debug_colors::RED,
                             format!("same: {} frames", tile.same_frames),
                         );
                     }
                 }
             } else {
                 // Add the tile rect to the dirty rect.
                 dirty_world_rect = dirty_world_rect.union(&visible_rect);
 
+                #[cfg(feature = "debug_renderer")]
+                {
+                    if frame_context.debug_flags.contains(DebugFlags::PICTURE_CACHING_DBG) {
+                        _scratch.push_debug_rect(
+                            visible_rect * frame_context.device_pixel_scale,
+                            debug_colors::RED,
+                        );
+                    }
+                }
+
                 // Only cache tiles that have had the same content for at least two
                 // frames. This skips caching on pages / benchmarks that are changing
                 // every frame, which is wasteful.
                 if tile.same_frames > FRAMES_BEFORE_CACHING {
                     // Ensure that this texture is allocated.
                     resource_cache.texture_cache.update(
                         &mut tile.handle,
                         descriptor,
@@ -1253,26 +1317,16 @@ impl TileCache {
         }
 
         // Store the dirty region for drawing the main scene.
         self.dirty_region = if dirty_world_rect.is_empty() {
             None
         } else {
             let dirty_device_rect = dirty_world_rect * frame_context.device_pixel_scale;
 
-            #[cfg(feature = "debug_renderer")]
-            {
-                if frame_context.debug_flags.contains(DebugFlags::PICTURE_CACHING_DBG) {
-                    _scratch.push_debug_rect(
-                        dirty_device_rect,
-                        debug_colors::RED,
-                    );
-                }
-            }
-
             Some(DirtyRegion {
                 dirty_world_rect,
                 dirty_device_rect: dirty_device_rect.round().to_i32(),
             })
         };
 
         local_clip_rect
     }
--- a/gfx/wr/webrender/src/prim_store/mod.rs
+++ b/gfx/wr/webrender/src/prim_store/mod.rs
@@ -1786,18 +1786,17 @@ impl PrimitiveStore {
                 debug_assert!(frame_state.tile_cache.is_none());
 
                 // If we have a tile cache for this picture, see if any of the
                 // relative transforms have changed, which means we need to
                 // re-map the dependencies of any child primitives.
                 tile_cache.pre_update(
                     pic.local_rect,
                     frame_context,
-                    frame_state.resource_cache,
-                    frame_state.retained_tiles,
+                    frame_state,
                 );
 
                 frame_state.tile_cache = Some(tile_cache);
             }
 
             (prim_list, surface_index, pic.apply_local_clip_rect)
         };
 
--- a/gfx/wr/webrender/src/renderer.rs
+++ b/gfx/wr/webrender/src/renderer.rs
@@ -4246,17 +4246,17 @@ impl Renderer {
         let debug_renderer = match self.debug.get_mut(&mut self.device) {
             Some(render) => render,
             None => return,
         };
 
         for item in items {
             match item {
                 DebugItem::Rect { rect, color } => {
-                    let inner_color = color.scale_alpha(0.1).into();
+                    let inner_color = color.scale_alpha(0.5).into();
                     let outer_color = (*color).into();
 
                     debug_renderer.add_quad(
                         rect.origin.x,
                         rect.origin.y,
                         rect.origin.x + rect.size.width,
                         rect.origin.y + rect.size.height,
                         inner_color,