Bug 1611331 - Tidy up rect field naming in picture cache code. r=Bert
authorGlenn Watson <gw@intuitionlibrary.com>
Fri, 24 Jan 2020 03:24:35 +0000
changeset 511603 9261d1b05a3259fca73ec05870de8b9eb4639fb4
parent 511602 bb770dc63b34c867eb6c4751827ce0176dbbe150
child 511604 975f76d04c40fe49b8eefc9bf7945fe61cd55cab
push id37053
push usernbeleuzu@mozilla.com
push dateFri, 24 Jan 2020 21:46:30 +0000
treeherdermozilla-central@6f6e201853c8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersBert
bugs1611331
milestone74.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 1611331 - Tidy up rect field naming in picture cache code. r=Bert Differential Revision: https://phabricator.services.mozilla.com/D60921
gfx/wr/webrender/src/composite.rs
gfx/wr/webrender/src/picture.rs
--- a/gfx/wr/webrender/src/composite.rs
+++ b/gfx/wr/webrender/src/composite.rs
@@ -307,17 +307,17 @@ impl CompositeState {
             let tile = &tile_cache.tiles[key];
             if !tile.is_visible {
                 // This can occur when a tile is found to be occluded during frame building.
                 continue;
             }
 
             visible_tile_count += 1;
 
-            let device_rect = (tile.world_rect * global_device_pixel_scale).round();
+            let device_rect = (tile.world_tile_rect * global_device_pixel_scale).round();
             let dirty_rect = (tile.world_dirty_rect * global_device_pixel_scale).round();
             let surface = tile.surface.as_ref().expect("no tile surface set!");
 
             let (surface, is_opaque) = match surface {
                 TileSurface::Color { color } => {
                     (CompositeTileSurface::Color { color: *color }, true)
                 }
                 TileSurface::Clear => {
--- a/gfx/wr/webrender/src/picture.rs
+++ b/gfx/wr/webrender/src/picture.rs
@@ -369,38 +369,38 @@ pub struct SpatialNodeDependency {
     /// The current value retrieved from the clip-scroll tree.
     value: TransformKey,
     /// True if it was changed (or is new) since the last frame build.
     changed: bool,
 }
 
 // Immutable context passed to picture cache tiles during pre_update
 struct TilePreUpdateContext {
-    /// The local rect of the overall picture cache
-    local_rect: PictureRect,
-
-    /// The local clip rect (in picture space) of the entire picture cache
-    local_clip_rect: PictureRect,
-
     /// Maps from picture cache coords -> world space coords.
     pic_to_world_mapper: SpaceMapper<PicturePixel, WorldPixel>,
 
     /// The fractional position of the picture cache, which may
     /// require invalidation of all tiles.
     fract_offset: PictureVector2D,
 
     /// The optional background color of the picture cache instance
     background_color: Option<ColorF>,
 
     /// The visible part of the screen in world coords.
     global_screen_world_rect: WorldRect,
 }
 
 // Immutable context passed to picture cache tiles during post_update
 struct TilePostUpdateContext<'a> {
+    /// The local rect of the overall picture cache
+    local_rect: PictureRect,
+
+    /// The local clip rect (in picture space) of the entire picture cache
+    local_clip_rect: PictureRect,
+
     /// The calculated backdrop information for this cache instance.
     backdrop: BackdropInfo,
 
     /// Information about transform node differences from last frame.
     spatial_nodes: &'a FastHashMap<SpatialNodeIndex, SpatialNodeDependency>,
 
     /// Information about opacity bindings from the picture cache.
     opacity_bindings: &'a FastHashMap<PropertyBindingId, OpacityBindingInfo>,
@@ -633,21 +633,25 @@ pub struct TileCacheInstanceSerializer {
     pub tiles: FastHashMap<TileOffset, TileSerializer>,
     pub background_color: Option<ColorF>,
     pub fract_offset: PictureVector2D,
 }
 
 /// Information about a cached tile.
 pub struct Tile {
     /// The current world rect of this tile.
-    pub world_rect: WorldRect,
+    pub world_tile_rect: WorldRect,
     /// The current local rect of this tile.
-    pub rect: PictureRect,
-    /// The local rect of the tile clipped to the overall picture local rect.
-    clipped_rect: PictureRect,
+    pub local_tile_rect: PictureRect,
+    /// The picture space dirty rect for this tile.
+    local_dirty_rect: PictureRect,
+    /// The world space dirty rect for this tile.
+    /// TODO(gw): We have multiple dirty rects available due to the quadtree above. In future,
+    ///           expose these as multiple dirty rects, which will help in some cases.
+    pub world_dirty_rect: WorldRect,
     /// Uniquely describes the content of this tile, in a way that can be
     /// (reasonably) efficiently hashed and compared.
     pub current_descriptor: TileDescriptor,
     /// The content descriptor for this tile from the previous frame.
     pub prev_descriptor: TileDescriptor,
     /// Handle to the backing surface for this tile.
     pub surface: Option<TileSurface>,
     /// If true, this tile is marked valid, and the existing texture
@@ -664,57 +668,50 @@ pub struct Tile {
     /// The tile id is stable between display lists and / or frames,
     /// if the tile is retained. Useful for debugging tile evictions.
     pub id: TileId,
     /// If true, the tile was determined to be opaque, which means blending
     /// can be disabled when drawing it.
     pub is_opaque: bool,
     /// Root node of the quadtree dirty rect tracker.
     root: TileNode,
-    /// The picture space dirty rect for this tile.
-    dirty_rect: PictureRect,
-    /// The world space dirty rect for this tile.
-    /// TODO(gw): We have multiple dirty rects available due to the quadtree above. In future,
-    ///           expose these as multiple dirty rects, which will help in some cases.
-    pub world_dirty_rect: WorldRect,
     /// The last rendered background color on this tile.
     background_color: Option<ColorF>,
     /// The first reason the tile was invalidated this frame.
     invalidation_reason: Option<InvalidationReason>,
 }
 
 impl Tile {
     /// Construct a new, invalid tile.
     fn new(
         id: TileId,
     ) -> Self {
         Tile {
-            rect: PictureRect::zero(),
-            clipped_rect: PictureRect::zero(),
-            world_rect: WorldRect::zero(),
+            local_tile_rect: PictureRect::zero(),
+            world_tile_rect: WorldRect::zero(),
+            local_dirty_rect: PictureRect::zero(),
+            world_dirty_rect: WorldRect::zero(),
             surface: None,
             current_descriptor: TileDescriptor::new(),
             prev_descriptor: TileDescriptor::new(),
             is_valid: false,
             is_visible: false,
             fract_offset: PictureVector2D::zero(),
             id,
             is_opaque: false,
             root: TileNode::new_leaf(Vec::new()),
-            dirty_rect: PictureRect::zero(),
-            world_dirty_rect: WorldRect::zero(),
             background_color: None,
             invalidation_reason: None,
         }
     }
 
     /// Print debug information about this tile to a tree printer.
     fn print(&self, pt: &mut dyn PrintTreePrinter) {
         pt.new_level(format!("Tile {:?}", self.id));
-        pt.add_item(format!("rect: {}", self.rect));
+        pt.add_item(format!("local_tile_rect: {}", self.local_tile_rect));
         pt.add_item(format!("fract_offset: {:?}", self.fract_offset));
         pt.add_item(format!("background_color: {:?}", self.background_color));
         pt.add_item(format!("invalidation_reason: {:?}", self.invalidation_reason));
         self.current_descriptor.print(pt);
         pt.end_level();
     }
 
     /// Check if the content of the previous and current tile descriptors match
@@ -777,49 +774,44 @@ impl Tile {
         &mut self,
         invalidation_rect: Option<PictureRect>,
         reason: InvalidationReason,
     ) {
         self.is_valid = false;
 
         match invalidation_rect {
             Some(rect) => {
-                self.dirty_rect = self.dirty_rect.union(&rect);
+                self.local_dirty_rect = self.local_dirty_rect.union(&rect);
             }
             None => {
-                self.dirty_rect = self.rect;
+                self.local_dirty_rect = self.local_tile_rect;
             }
         }
 
         if self.invalidation_reason.is_none() {
             self.invalidation_reason = Some(reason);
         }
     }
 
     /// Called during pre_update of a tile cache instance. Allows the
     /// tile to setup state before primitive dependency calculations.
     fn pre_update(
         &mut self,
-        rect: PictureRect,
+        local_tile_rect: PictureRect,
         ctx: &TilePreUpdateContext,
     ) {
-        self.rect = rect;
+        self.local_tile_rect = local_tile_rect;
         self.invalidation_reason  = None;
 
-        self.clipped_rect = self.rect
-            .intersection(&ctx.local_rect)
-            .and_then(|r| r.intersection(&ctx.local_clip_rect))
-            .unwrap_or_else(PictureRect::zero);
-
-        self.world_rect = ctx.pic_to_world_mapper
-            .map(&self.rect)
+        self.world_tile_rect = ctx.pic_to_world_mapper
+            .map(&self.local_tile_rect)
             .expect("bug: map local tile rect");
 
         // Check if this tile is currently on screen.
-        self.is_visible = self.world_rect.intersects(&ctx.global_screen_world_rect);
+        self.is_visible = self.world_tile_rect.intersects(&ctx.global_screen_world_rect);
 
         // If the tile isn't visible, early exit, skipping the normal set up to
         // validate dependencies. Instead, we will only compare the current tile
         // dependencies the next time it comes into view.
         if !self.is_visible {
             return;
         }
 
@@ -839,17 +831,17 @@ impl Tile {
 
         // Clear any dependencies so that when we rebuild them we
         // can compare if the tile has the same content.
         mem::swap(
             &mut self.current_descriptor,
             &mut self.prev_descriptor,
         );
         self.current_descriptor.clear();
-        self.root.clear(rect);
+        self.root.clear(local_tile_rect);
     }
 
     /// Add dependencies for a given primitive to this tile.
     fn add_prim_dependency(
         &mut self,
         info: &PrimitiveDependencyInfo,
     ) {
         // If this tile isn't currently visible, we don't want to update the dependencies
@@ -869,18 +861,18 @@ impl Tile {
 
         // Include any transforms that this primitive depends on.
         self.current_descriptor.transforms.extend_from_slice(&info.spatial_nodes);
 
         // TODO(gw): The origin of background rects produced by APZ changes
         //           in Gecko during scrolling. Consider investigating this so the
         //           hack / workaround below is not required.
         let (prim_origin, prim_clip_rect) = if info.clip_by_tile {
-            let tile_p0 = self.rect.origin;
-            let tile_p1 = self.rect.bottom_right();
+            let tile_p0 = self.local_tile_rect.origin;
+            let tile_p1 = self.local_tile_rect.bottom_right();
 
             let clip_p0 = PicturePoint::new(
                 clampf(info.prim_clip_rect.origin.x, tile_p0.x, tile_p1.x),
                 clampf(info.prim_clip_rect.origin.y, tile_p0.y, tile_p1.y),
             );
 
             let clip_p1 = PicturePoint::new(
                 clampf(info.prim_clip_rect.origin.x + info.prim_clip_rect.size.width, tile_p0.x, tile_p1.x),
@@ -958,17 +950,21 @@ impl Tile {
             }
 
             return false;
         }
 
         // Check if this tile can be considered opaque. Opacity state must be updated only
         // after all early out checks have been performed. Otherwise, we might miss updating
         // the native surface next time this tile becomes visible.
-        self.is_opaque = ctx.backdrop.rect.contains_rect(&self.clipped_rect);
+        let clipped_rect = self.local_tile_rect
+            .intersection(&ctx.local_rect)
+            .and_then(|r| r.intersection(&ctx.local_clip_rect))
+            .unwrap_or_else(PictureRect::zero);
+        self.is_opaque = ctx.backdrop.rect.contains_rect(&clipped_rect);
 
         // Check if the selected composite mode supports dirty rect updates. For Draw composite
         // mode, we can always update the content with smaller dirty rects. For native composite
         // mode, we can only use dirty rects if the compositor supports partial surface updates.
         let (supports_dirty_rects, supports_simple_prims) = match state.composite_state.compositor_kind {
             CompositorKind::Draw { .. } => {
                 (true, true)
             }
@@ -992,22 +988,22 @@ impl Tile {
                 );
             }
         }
 
         // The dirty rect will be set correctly by now. If the underlying platform
         // doesn't support partial updates, and this tile isn't valid, force the dirty
         // rect to be the size of the entire tile.
         if !self.is_valid && !supports_dirty_rects {
-            self.dirty_rect = self.rect;
+            self.local_dirty_rect = self.local_tile_rect;
         }
 
         // Ensure that the dirty rect doesn't extend outside the local tile rect.
-        self.dirty_rect = self.dirty_rect
-            .intersection(&self.rect)
+        self.local_dirty_rect = self.local_dirty_rect
+            .intersection(&self.local_tile_rect)
             .unwrap_or_else(PictureRect::zero);
 
         // See if this tile is a simple color, in which case we can just draw
         // it as a rect, and avoid allocating a texture surface and drawing it.
         // TODO(gw): Initial native compositor interface doesn't support simple
         //           color tiles. We can definitely support this in DC, so this
         //           should be added as a follow up.
         let is_simple_prim =
@@ -2032,18 +2028,16 @@ impl TileCacheInstance {
         self.tile_bounds_p0 = TileOffset::new(x0, y0);
         self.tile_bounds_p1 = TileOffset::new(x1, y1);
 
         let mut world_culling_rect = WorldRect::zero();
 
         mem::swap(&mut self.tiles, &mut self.old_tiles);
 
         let ctx = TilePreUpdateContext {
-            local_rect: self.local_rect,
-            local_clip_rect: self.local_clip_rect,
             pic_to_world_mapper,
             fract_offset: self.fract_offset,
             background_color: self.background_color,
             global_screen_world_rect: frame_context.global_screen_world_rect,
         };
 
         self.tiles.clear();
         for y in y0 .. y1 {
@@ -2081,17 +2075,17 @@ impl TileCacheInstance {
                 // (2) When we need to allocate an off-screen surface for a child picture (for
                 //     example a CSS filter) we clip the size of the GPU surface to the world
                 //     culling rect below (to ensure we draw enough of it to be sampled by any
                 //     tiles that reference it). Making the world culling rect only affected
                 //     by visible tiles (rather than the entire virtual tile display port) can
                 //     result in allocating _much_ smaller GPU surfaces for cases where the
                 //     true off-screen surface size is very large.
                 if tile.is_visible {
-                    world_culling_rect = world_culling_rect.union(&tile.world_rect);
+                    world_culling_rect = world_culling_rect.union(&tile.world_tile_rect);
                 }
 
                 self.tiles.insert(key, tile);
             }
         }
 
         // Any old tiles that remain after the loop above are going to be dropped. For
         // simple composite mode, the texture cache handle will expire and be collected
@@ -2531,16 +2525,18 @@ impl TileCacheInstance {
 
             self.spatial_nodes.insert(spatial_node_index, SpatialNodeDependency {
                 changed,
                 value,
             });
         }
 
         let ctx = TilePostUpdateContext {
+            local_rect: self.local_rect,
+            local_clip_rect: self.local_clip_rect,
             backdrop: self.backdrop,
             spatial_nodes: &self.spatial_nodes,
             opacity_bindings: &self.opacity_bindings,
             current_tile_size: self.current_tile_size,
         };
 
         let mut state = TilePostUpdateState {
             resource_cache: frame_state.resource_cache,
@@ -3847,17 +3843,17 @@ impl PicturePrimitive {
                         let world_clip_rect = map_pic_to_world
                             .map(&local_clip_rect)
                             .expect("bug: unable to map clip rect");
 
                         for key in &tile_cache.tiles_to_draw {
                             let tile = tile_cache.tiles.get_mut(key).expect("bug: no tile found!");
 
                             // Get the world space rect that this tile will actually occupy on screem
-                            let tile_draw_rect = match world_clip_rect.intersection(&tile.world_rect) {
+                            let tile_draw_rect = match world_clip_rect.intersection(&tile.world_tile_rect) {
                                 Some(rect) => rect,
                                 None => {
                                     tile.is_visible = false;
                                     continue;
                                 }
                             };
 
                             // If that draw rect is occluded by some set of tiles in front of it,
@@ -3885,17 +3881,17 @@ impl PicturePrimitive {
                                 tile.root.draw_debug_rects(
                                     &map_pic_to_world,
                                     tile.is_opaque,
                                     scratch,
                                     frame_context.global_device_pixel_scale,
                                 );
 
                                 let label_offset = DeviceVector2D::new(20.0, 30.0);
-                                let tile_device_rect = tile.world_rect * frame_context.global_device_pixel_scale;
+                                let tile_device_rect = tile.world_tile_rect * frame_context.global_device_pixel_scale;
                                 if tile_device_rect.size.height >= label_offset.y {
                                     let surface = tile.surface.as_ref().expect("no tile surface set!");
 
                                     scratch.push_debug_string(
                                         tile_device_rect.origin + label_offset,
                                         debug_colors::RED,
                                         format!("{:?}: s={} is_opaque={} surface={}",
                                                 tile.id,
@@ -3932,17 +3928,17 @@ impl PicturePrimitive {
                                             // There is no current surface allocation, so ensure the entire tile is invalidated
                                             tile.invalidate(None, InvalidationReason::NoSurface);
                                         }
                                     }
                                 }
                             }
 
                             // Update the world dirty rect
-                            tile.world_dirty_rect = map_pic_to_world.map(&tile.dirty_rect).expect("bug");
+                            tile.world_dirty_rect = map_pic_to_world.map(&tile.local_dirty_rect).expect("bug");
 
                             if tile.is_valid {
                                 continue;
                             }
 
                             // Ensure that this texture is allocated.
                             if let TileSurface::Texture { ref mut descriptor, ref mut visibility_mask } = tile.surface.as_mut().unwrap() {
                                 match descriptor {
@@ -4004,25 +4000,25 @@ impl PicturePrimitive {
                                     visibility_mask.set_visible(PrimitiveVisibilityMask::MAX_DIRTY_REGIONS - 1);
 
                                     tile_cache.dirty_region.include_rect(
                                         PrimitiveVisibilityMask::MAX_DIRTY_REGIONS - 1,
                                         tile.world_dirty_rect,
                                     );
                                 }
 
-                                let content_origin_f = tile.world_rect.origin * device_pixel_scale;
+                                let content_origin_f = tile.world_tile_rect.origin * device_pixel_scale;
                                 let content_origin = content_origin_f.round();
                                 debug_assert!((content_origin_f.x - content_origin.x).abs() < 0.01);
                                 debug_assert!((content_origin_f.y - content_origin.y).abs() < 0.01);
 
                                 // Get a task-local scissor rect for the dirty region of this
                                 // picture cache task.
                                 let scissor_rect = tile.world_dirty_rect.translate(
-                                    -tile.world_rect.origin.to_vector()
+                                    -tile.world_tile_rect.origin.to_vector()
                                 );
                                 // The world rect is guaranteed to be device pixel aligned, by the tile
                                 // sizing code in tile::pre_update. However, there might be some
                                 // small floating point accuracy issues (these were observed on ARM
                                 // CPUs). Round the rect here before casting to integer device pixels
                                 // to ensure the scissor rect is correct.
                                 let scissor_rect = (scissor_rect * device_pixel_scale).round();
                                 let surface = descriptor.resolve(
@@ -4061,17 +4057,17 @@ impl PicturePrimitive {
                                         port: render_task_id,
                                     });
 
                                     first = false;
                                 }
                             }
 
                             // Now that the tile is valid, reset the dirty rect.
-                            tile.dirty_rect = PictureRect::zero();
+                            tile.local_dirty_rect = PictureRect::zero();
                             tile.is_valid = true;
                         }
 
                         // If invalidation debugging is enabled, dump the picture cache state to a tree printer.
                         if frame_context.debug_flags.contains(DebugFlags::INVALIDATION_DBG) {
                             tile_cache.print();
                         }
 
@@ -4173,17 +4169,17 @@ impl PicturePrimitive {
                     let mut tile_cache_tiny = TileCacheInstanceSerializer {
                         slice: tile_cache.slice,
                         tiles: FastHashMap::default(),
                         background_color: tile_cache.background_color,
                         fract_offset: tile_cache.fract_offset
                     };
                     for (key, tile) in &tile_cache.tiles {
                         tile_cache_tiny.tiles.insert(*key, TileSerializer {
-                            rect: tile.rect,
+                            rect: tile.local_tile_rect,
                             current_descriptor: tile.current_descriptor.clone(),
                             fract_offset: tile.fract_offset,
                             id: tile.id,
                             root: tile.root.clone(),
                             background_color: tile.background_color,
                             invalidation_reason: tile.invalidation_reason.clone()
                         });
                     }