Bug 1587676 - Allocate picture caching tile surfaces later during the frame. r=nical,kvark
authorGlenn Watson <git@intuitionlibrary.com>
Sun, 13 Oct 2019 20:24:40 +0000
changeset 497380 1af5cb02462a3f01b83cdb48da9644da36ec7e4d
parent 497379 203e2a744ac00183d541da3f4d286be2c2e6729f
child 497381 9dce7195c6edce03982ba144b8d5d0c25856169b
push id97853
push usergwatson@mozilla.com
push dateSun, 13 Oct 2019 20:42:11 +0000
treeherderautoland@1af5cb02462a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnical, kvark
bugs1587676
milestone71.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 1587676 - Allocate picture caching tile surfaces later during the frame. r=nical,kvark Instead of allocating a tile surface from the texture cache during the tile post_update method, this is now deferred until the take_context method of picture. This will allow a simple CPU occlusion culling pass to run after all the tile cache dependency updates (when the visible tiles and rects are know), but before the surfaces are allocated. In this way, we will be able to skip allocating, rasterizing and compositing any tiles that are eliminated by the occlusion culling test. Differential Revision: https://phabricator.services.mozilla.com/D48795
gfx/wr/webrender/src/frame_builder.rs
gfx/wr/webrender/src/picture.rs
gfx/wr/webrender/src/prim_store/mod.rs
--- a/gfx/wr/webrender/src/frame_builder.rs
+++ b/gfx/wr/webrender/src/frame_builder.rs
@@ -388,16 +388,17 @@ impl FrameBuilder {
                 scene.root_pic_index,
                 WorldRect::max_rect(),
                 root_spatial_node_index,
                 root_spatial_node_index,
                 ROOT_SURFACE_INDEX,
                 SubpixelMode::Allow,
                 &mut frame_state,
                 &frame_context,
+                scratch,
             )
             .unwrap();
 
         {
             profile_marker!("PreparePrims");
 
             scene.prim_store.prepare_primitives(
                 &mut prim_list,
--- a/gfx/wr/webrender/src/picture.rs
+++ b/gfx/wr/webrender/src/picture.rs
@@ -267,54 +267,36 @@ struct TilePreUpdateContext {
     fract_changed: bool,
 
     /// The optional background color of the picture cache instance
     background_color: Option<ColorF>,
 }
 
 // Immutable context passed to picture cache tiles during post_update
 struct TilePostUpdateContext<'a> {
-    /// Current set of WR debug flags
-    debug_flags: DebugFlags,
-
-    /// The global scale factor from world -> device coords.
-    global_device_pixel_scale: DevicePixelScale,
-
     /// The visible part of the screen in world coords.
     global_screen_world_rect: WorldRect,
 
     /// 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>,
 
-    /// Helper to map picture coordinates to world space
-    pic_to_world_mapper: SpaceMapper<PicturePixel, WorldPixel>,
-
     /// Current size in device pixels of tiles for this cache
     current_tile_size: DeviceIntSize,
 }
 
 // Mutable state passed to picture cache tiles during post_update
 struct TilePostUpdateState<'a> {
-    /// Scratch buffer for drawing debug information.
-    scratch: &'a mut PrimitiveScratchBuffer,
-
-    /// Current dirty region of this cache.
-    dirty_region: &'a mut DirtyRegion,
-
     /// Allow access to the texture cache for requesting tiles
-    resource_cache: &'a mut ResourceCache,
-
-    /// Needed when requesting tile cache handles.
-    gpu_cache: &'a mut GpuCache,
+    resource_cache: &'a ResourceCache,
 }
 
 /// Information about the dependencies of a single primitive instance.
 struct PrimitiveDependencyInfo {
     /// If true, this instance can be cached.
     is_cacheable: bool,
 
     /// If true, we should clip the prim rect to the tile boundaries.
@@ -655,17 +637,17 @@ impl Tile {
             max_split_level,
         );
 
         // 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.
         let is_simple_prim = self.current_descriptor.prims.len() == 1 && self.is_opaque;
 
         // Set up the backing surface for this tile.
-        let mut surface = if is_simple_prim {
+        let surface = if is_simple_prim {
             // If we determine the tile can be represented by a color, set the
             // surface unconditionally (this will drop any previously used
             // texture cache backing surface).
             match ctx.backdrop.kind {
                 BackdropKind::Color { color } => {
                     TileSurface::Color {
                         color,
                     }
@@ -687,100 +669,16 @@ impl Tile {
                     TileSurface::Texture {
                         handle: TextureCacheHandle::invalid(),
                         visibility_mask: PrimitiveVisibilityMask::empty(),
                     }
                 }
             }
         };
 
-        if let TileSurface::Texture { ref handle, .. } = surface {
-            // Invalidate if the backing texture was evicted.
-            if state.resource_cache.texture_cache.is_allocated(handle) {
-                // Request the backing texture so it won't get evicted this frame.
-                // We specifically want to mark the tile texture as used, even
-                // if it's detected not visible below and skipped. This is because
-                // we maintain the set of tiles we care about based on visibility
-                // during pre_update. If a tile still exists after that, we are
-                // assuming that it's either visible or we want to retain it for
-                // a while in case it gets scrolled back onto screen soon.
-                // TODO(gw): Consider switching to manual eviction policy?
-                state.resource_cache.texture_cache.request(handle, state.gpu_cache);
-            } else {
-                // If the texture was evicted on a previous frame, we need to assume
-                // that the entire tile rect is dirty.
-                self.is_valid = false;
-                self.dirty_rect = self.rect;
-            }
-        }
-
-        // Update the world dirty rect
-        self.world_dirty_rect = ctx.pic_to_world_mapper.map(&self.dirty_rect).expect("bug");
-
-        if ctx.debug_flags.contains(DebugFlags::PICTURE_CACHING_DBG) {
-            self.root.draw_debug_rects(
-                &ctx.pic_to_world_mapper,
-                self.is_opaque,
-                state.scratch,
-                ctx.global_device_pixel_scale,
-            );
-
-            let label_offset = DeviceVector2D::new(20.0, 30.0);
-            let tile_device_rect = self.world_rect * ctx.global_device_pixel_scale;
-            if tile_device_rect.size.height >= label_offset.y {
-                state.scratch.push_debug_string(
-                    tile_device_rect.origin + label_offset,
-                    debug_colors::RED,
-                    format!("{:?}: is_opaque={} surface={}",
-                            self.id,
-                            self.is_opaque,
-                            surface.kind(),
-                    ),
-                );
-            }
-        }
-
-        // Decide how to handle this tile when drawing this frame.
-        if !self.is_valid {
-            // Ensure that this texture is allocated.
-            if let TileSurface::Texture { ref mut handle, ref mut visibility_mask } = surface {
-                if !state.resource_cache.texture_cache.is_allocated(handle) {
-                    state.resource_cache.texture_cache.update_picture_cache(
-                        ctx.current_tile_size,
-                        handle,
-                        state.gpu_cache,
-                    );
-                }
-
-                *visibility_mask = PrimitiveVisibilityMask::empty();
-                let dirty_region_index = state.dirty_region.dirty_rects.len();
-
-                // If we run out of dirty regions, then force the last dirty region to
-                // be a union of any remaining regions. This is an inefficiency, in that
-                // we'll add items to batches later on that are redundant / outside this
-                // tile, but it's really rare except in pathological cases (even on a
-                // 4k screen, the typical dirty region count is < 16).
-                if dirty_region_index < PrimitiveVisibilityMask::MAX_DIRTY_REGIONS {
-                    visibility_mask.set_visible(dirty_region_index);
-
-                    state.dirty_region.push(
-                        self.world_dirty_rect,
-                        *visibility_mask,
-                    );
-                } else {
-                    visibility_mask.set_visible(PrimitiveVisibilityMask::MAX_DIRTY_REGIONS - 1);
-
-                    state.dirty_region.include_rect(
-                        PrimitiveVisibilityMask::MAX_DIRTY_REGIONS - 1,
-                        self.world_dirty_rect,
-                    );
-                }
-            }
-        }
-
         // Store the current surface backing info for use during batching.
         self.surface = Some(surface);
 
         true
     }
 }
 
 /// Defines a key that uniquely identifies a primitive instance.
@@ -1803,39 +1701,26 @@ impl TileCacheInstance {
             }
 
             self.spatial_nodes.insert(spatial_node_index, SpatialNodeDependency {
                 changed,
                 value,
             });
         }
 
-        let pic_to_world_mapper = SpaceMapper::new_with_target(
-            ROOT_SPATIAL_NODE_INDEX,
-            self.spatial_node_index,
-            frame_context.global_screen_world_rect,
-            frame_context.clip_scroll_tree,
-        );
-
         let ctx = TilePostUpdateContext {
-            debug_flags: frame_context.debug_flags,
-            global_device_pixel_scale: frame_context.global_device_pixel_scale,
             global_screen_world_rect: frame_context.global_screen_world_rect,
             backdrop: self.backdrop,
             spatial_nodes: &self.spatial_nodes,
             opacity_bindings: &self.opacity_bindings,
-            pic_to_world_mapper,
             current_tile_size: self.current_tile_size,
         };
 
         let mut state = TilePostUpdateState {
             resource_cache: frame_state.resource_cache,
-            gpu_cache: frame_state.gpu_cache,
-            scratch: frame_state.scratch,
-            dirty_region: &mut self.dirty_region,
         };
 
         // Step through each tile and invalidate if the dependencies have changed.
         for (key, tile) in self.tiles.iter_mut() {
             if tile.post_update(
                 &ctx,
                 &mut state,
             ) {
@@ -2731,16 +2616,17 @@ impl PicturePrimitive {
         pic_index: PictureIndex,
         clipped_prim_bounding_rect: WorldRect,
         surface_spatial_node_index: SpatialNodeIndex,
         raster_spatial_node_index: SpatialNodeIndex,
         parent_surface_index: SurfaceIndex,
         parent_subpixel_mode: SubpixelMode,
         frame_state: &mut FrameBuildingState,
         frame_context: &FrameBuildingContext,
+        scratch: &mut PrimitiveScratchBuffer,
     ) -> Option<(PictureContext, PictureState, PrimitiveList)> {
         if !self.is_visible() {
             return None;
         }
 
         // Extract the raster and surface spatial nodes from the raster
         // config, if this picture establishes a surface. Otherwise just
         // pass in the spatial node indices from the parent context.
@@ -3075,22 +2961,102 @@ impl PicturePrimitive {
                             //           tiles as active. This is a hotfix because video
                             //           images on Mac/Linux are not being correctly detected
                             //           as non-cacheable. We should investigate / fix the
                             //           root cause of this.
                             for image_key in &tile.current_descriptor.image_keys {
                                 frame_state.resource_cache.set_image_active(*image_key);
                             }
 
+                            let surface = tile.surface.as_mut().expect("no tile surface set!");
+
+                            if frame_context.debug_flags.contains(DebugFlags::PICTURE_CACHING_DBG) {
+                                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;
+                                if tile_device_rect.size.height >= label_offset.y {
+                                    scratch.push_debug_string(
+                                        tile_device_rect.origin + label_offset,
+                                        debug_colors::RED,
+                                        format!("{:?}: is_opaque={} surface={}",
+                                                tile.id,
+                                                tile.is_opaque,
+                                                surface.kind(),
+                                        ),
+                                    );
+                                }
+                            }
+
+                            if let TileSurface::Texture { ref handle, .. } = surface {
+                                // Invalidate if the backing texture was evicted.
+                                if frame_state.resource_cache.texture_cache.is_allocated(handle) {
+                                    // Request the backing texture so it won't get evicted this frame.
+                                    // We specifically want to mark the tile texture as used, even
+                                    // if it's detected not visible below and skipped. This is because
+                                    // we maintain the set of tiles we care about based on visibility
+                                    // during pre_update. If a tile still exists after that, we are
+                                    // assuming that it's either visible or we want to retain it for
+                                    // a while in case it gets scrolled back onto screen soon.
+                                    // TODO(gw): Consider switching to manual eviction policy?
+                                    frame_state.resource_cache.texture_cache.request(handle, frame_state.gpu_cache);
+                                } else {
+                                    // If the texture was evicted on a previous frame, we need to assume
+                                    // that the entire tile rect is dirty.
+                                    tile.is_valid = false;
+                                    tile.dirty_rect = tile.rect;
+                                }
+                            }
+
+                            // Update the world dirty rect
+                            tile.world_dirty_rect = map_pic_to_world.map(&tile.dirty_rect).expect("bug");
+
                             if tile.is_valid {
                                 continue;
                             }
 
-                            let surface = tile.surface.as_ref().expect("no tile surface set!");
-                            if let TileSurface::Texture { ref handle, visibility_mask } = surface {
+                            // Ensure that this texture is allocated.
+                            if let TileSurface::Texture { ref mut handle, ref mut visibility_mask } = surface {
+                                if !frame_state.resource_cache.texture_cache.is_allocated(handle) {
+                                    frame_state.resource_cache.texture_cache.update_picture_cache(
+                                        tile_cache.current_tile_size,
+                                        handle,
+                                        frame_state.gpu_cache,
+                                    );
+                                }
+
+                                *visibility_mask = PrimitiveVisibilityMask::empty();
+                                let dirty_region_index = tile_cache.dirty_region.dirty_rects.len();
+
+                                // If we run out of dirty regions, then force the last dirty region to
+                                // be a union of any remaining regions. This is an inefficiency, in that
+                                // we'll add items to batches later on that are redundant / outside this
+                                // tile, but it's really rare except in pathological cases (even on a
+                                // 4k screen, the typical dirty region count is < 16).
+                                if dirty_region_index < PrimitiveVisibilityMask::MAX_DIRTY_REGIONS {
+                                    visibility_mask.set_visible(dirty_region_index);
+
+                                    tile_cache.dirty_region.push(
+                                        tile.world_dirty_rect,
+                                        *visibility_mask,
+                                    );
+                                } else {
+                                    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 = 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(
--- a/gfx/wr/webrender/src/prim_store/mod.rs
+++ b/gfx/wr/webrender/src/prim_store/mod.rs
@@ -2636,16 +2636,17 @@ impl PrimitiveStore {
                         pic_index,
                         clipped_prim_bounding_rect,
                         pic_context.surface_spatial_node_index,
                         pic_context.raster_spatial_node_index,
                         pic_context.surface_index,
                         pic_context.subpixel_mode,
                         frame_state,
                         frame_context,
+                        scratch,
                     ) {
                         Some(info) => Some(info),
                         None => {
                             if prim_instance.is_chased() {
                                 println!("\tculled for carrying an invisible composite filter");
                             }
 
                             prim_instance.visibility_info = PrimitiveVisibilityIndex::INVALID;