Auto merge of #3455 - gw3583:batch-scissor-5, r=bholley

Follow ups from the recent picture caching optimizations.

This patch contains a number of fixes and improvements to
the picture caching changes that landed recently to improve
the general performance of picture caching. Specifically:

* Support setting scissor rect for the dirty rect that is
  being drawn. This is a performance win, and also simplifies
  the batching logic to not have to adjust the z_id for the
  tiles that are being drawn.
* Support alpha batch containers having more than one batch list.
  Each batch list supports an optional scissor rect, and a list
  of blits to run after drawing that batch list. This ensures
  that items batched after the cacheable content are not drawn
  into the cache tiles.
* The pending tile blits no longer need to be stored in the render
  task or surface information struct. Instead, they are retrieved
  directly from the picture's tile cache struct during batching.
* Include the local clip rect of the picture when drawing tiles,
  to ensure that the tiles don't write to the z-buffer where the
  scroll bars for a content frame is (they typically come earlier
  in the display list than the content, relying on clipping rather
  than render order).
* Include the tile relative position of clip vertices - this ensures
  that tiles are correctly invalidated in cases where only the
  relative position of a clip node changes between frames (there are
  a couple of reftests that verify this).
* Handle the case of a zero-sized clip mask correctly, to avoid
  trying to allocate a zero sized texture if only one clip task
  exists in a pass.
* Skip pushing / popping clip node collector for tile cache surfaces.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/3455)
<!-- Reviewable:end -->

webrender/src/frame_builder.rs

@@ -339,11 +339,6 @@ impl FrameBuilder {
             .surfaces[ROOT_SURFACE_INDEX.0]
             .take_render_tasks();
 
-        let tile_blits = mem::replace(
-            &mut frame_state.surfaces[ROOT_SURFACE_INDEX.0].tile_blits,
-            Vec::new(),
-        );
-
         let root_render_task = RenderTask::new_picture(
             RenderTaskLocation::Fixed(self.screen_rect.to_i32()),
             self.screen_rect.size.to_f32(),
@@ -353,7 +348,6 @@ impl FrameBuilder {
             UvRectKind::Rect,
             root_spatial_node_index,
             None,
-            tile_blits,
         );
 
         let render_task_id = frame_state.render_tasks.add(root_render_task);

webrender/src/picture.rs

@@ -4,7 +4,7 @@
 
 use api::{DeviceRect, FilterOp, MixBlendMode, PipelineId, PremultipliedColorF, PictureRect, PicturePoint, WorldPoint};
 use api::{DeviceIntRect, DevicePoint, LayoutRect, PictureToRasterTransform, LayoutPixel, PropertyBinding, PropertyBindingId};
-use api::{DevicePixelScale, RasterRect, RasterSpace, ColorF, ImageKey, DirtyRect, WorldSize};
+use api::{DevicePixelScale, RasterRect, RasterSpace, ColorF, ImageKey, DirtyRect, WorldSize, LayoutSize};
 use api::{PicturePixel, RasterPixel, WorldPixel, WorldRect, ImageFormat, ImageDescriptor, WorldVector2D};
 use box_shadow::{BLUR_SAMPLE_SCALE};
 use clip::{ClipNodeCollector, ClipStore, ClipChainId, ClipChainNode};
@@ -19,7 +19,7 @@ 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, CoordinateSpaceMapping};
+use prim_store::{get_raster_rects, CoordinateSpaceMapping, VectorKey};
 use prim_store::{OpacityBindingStorage, ImageInstanceStorage, OpacityBindingIndex};
 use print_tree::PrintTreePrinter;
 use render_backend::FrameResources;
@@ -166,6 +166,11 @@ pub struct TileDescriptor {
     /// to uniquely describe the content of the clip node.
     clip_uids: ComparableVec<ItemUid>,
 
+    /// List of tile relative offsets of the clip node origins. This
+    /// ensures that if a clip node is supplied but has a different
+    /// transform between frames that the tile is invalidated.
+    clip_vertices: ComparableVec<VectorKey>,
+
     /// List of image keys that this tile depends on.
     image_keys: ComparableVec<ImageKey>,
 
@@ -179,6 +184,7 @@ impl TileDescriptor {
         TileDescriptor {
             prims: ComparableVec::new(),
             clip_uids: ComparableVec::new(),
+            clip_vertices: ComparableVec::new(),
             opacity_bindings: ComparableVec::new(),
             image_keys: ComparableVec::new(),
         }
@@ -189,6 +195,7 @@ impl TileDescriptor {
     fn clear(&mut self) {
         self.prims.reset();
         self.clip_uids.reset();
+        self.clip_vertices.reset();
         self.opacity_bindings.reset();
         self.image_keys.reset();
     }
@@ -198,6 +205,7 @@ impl TileDescriptor {
         self.image_keys.is_valid() &&
         self.opacity_bindings.is_valid() &&
         self.clip_uids.is_valid() &&
+        self.clip_vertices.is_valid() &&
         self.prims.is_valid()
     }
 }
@@ -207,7 +215,8 @@ impl TileDescriptor {
 /// regions.
 #[derive(Debug)]
 pub struct DirtyRegion {
-    dirty_world_rect: WorldRect,
+    pub dirty_world_rect: WorldRect,
+    pub dirty_device_rect: DeviceIntRect,
 }
 
 /// Represents a cache of tiles that make up a picture primitives.
@@ -244,7 +253,7 @@ pub struct TileCache {
     /// a new scene arrives.
     scroll_offset: Option<WorldVector2D>,
     /// A list of blits from the framebuffer to be applied during this frame.
-    pending_blits: Vec<TileBlit>,
+    pub pending_blits: Vec<TileBlit>,
     /// Collects the clips that apply to this surface.
     clip_node_collector: ClipNodeCollector,
 }
@@ -614,8 +623,8 @@ impl TileCache {
 
         // Build the list of resources that this primitive has dependencies on.
         let mut opacity_bindings: SmallVec<[PropertyBindingId; 4]> = SmallVec::new();
-        let mut clip_chain_spatial_nodes: SmallVec<[SpatialNodeIndex; 8]> = SmallVec::new();
         let mut clip_chain_uids: SmallVec<[ItemUid; 8]> = SmallVec::new();
+        let mut clip_vertices: SmallVec<[WorldPoint; 8]> = SmallVec::new();
         let mut image_keys: SmallVec<[ImageKey; 8]> = SmallVec::new();
         let mut current_clip_chain_id = prim_instance.clip_chain_id;
 
@@ -686,7 +695,25 @@ impl TileCache {
             if clip_chain_node.spatial_node_index < self.spatial_node_index {
                 self.clip_node_collector.insert(current_clip_chain_id)
             } else {
-                clip_chain_spatial_nodes.push(clip_chain_node.spatial_node_index);
+                // TODO(gw): Constructing a rect here rather than mapping a point
+                //           is wasteful. We can optimize this by extending the
+                //           SpaceMapper struct to support mapping a point.
+                let local_rect = LayoutRect::new(
+                    clip_chain_node.local_pos,
+                    LayoutSize::zero(),
+                );
+
+                self.map_local_to_world.set_target_spatial_node(
+                    clip_chain_node.spatial_node_index,
+                    clip_scroll_tree,
+                );
+
+                let clip_world_rect = self
+                    .map_local_to_world
+                    .map(&local_rect)
+                    .expect("bug: unable to map clip rect to world");
+
+                clip_vertices.push(clip_world_rect.origin);
                 clip_chain_uids.push(clip_chain_node.handle.uid());
             }
             current_clip_chain_id = clip_chain_node.parent_clip_chain_id;
@@ -735,6 +762,17 @@ impl TileCache {
                     clip_count: clip_chain_uids.len() as u16,
                 });
                 tile.descriptor.clip_uids.extend_from_slice(&clip_chain_uids);
+
+                // Store tile relative clip vertices.
+                // TODO(gw): We might need to quantize these to avoid
+                //           invalidations due to FP accuracy.
+                for clip_vertex in &clip_vertices {
+                    let clip_vertex = VectorKey {
+                        x: clip_vertex.x - tile.world_rect.origin.x,
+                        y: clip_vertex.y - tile.world_rect.origin.y,
+                    };
+                    tile.descriptor.clip_vertices.push(clip_vertex);
+                }
             }
         }
     }
@@ -749,9 +787,12 @@ impl TileCache {
         clip_store: &ClipStore,
         frame_context: &FrameBuildingContext,
         resources: &FrameResources,
-    ) {
+    ) -> LayoutRect {
         let mut dirty_world_rect = WorldRect::zero();
 
+        self.dirty_region = None;
+        self.pending_blits.clear();
+
         let descriptor = ImageDescriptor::new(
             TILE_SIZE_WIDTH,
             TILE_SIZE_HEIGHT,
@@ -770,12 +811,23 @@ impl TileCache {
                 frame_context.clip_scroll_tree,
             ) {
             Some(clip_rect) => clip_rect,
-            None => return,
+            None => return LayoutRect::zero(),
         };
 
+        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
+            .unmap(&clip_rect)
+            .expect("bug: unable to map local clip rect");
+
         let clip_rect = match clip_rect.intersection(&frame_context.screen_world_rect) {
             Some(clip_rect) => clip_rect,
-            None => return,
+            None => return LayoutRect::zero(),
         };
 
         let clipped = (clip_rect * frame_context.device_pixel_scale).round().to_i32();
@@ -872,10 +924,14 @@ impl TileCache {
         self.dirty_region = if dirty_world_rect.is_empty() {
             None
         } else {
+            let dirty_device_rect = (dirty_world_rect * frame_context.device_pixel_scale).round().to_i32();
             Some(DirtyRegion {
                 dirty_world_rect,
+                dirty_device_rect,
             })
         };
+
+        local_clip_rect
     }
 }
 
@@ -987,8 +1043,6 @@ pub struct SurfaceInfo {
     pub tasks: Vec<RenderTaskId>,
     /// How much the local surface rect should be inflated (for blur radii).
     pub inflation_factor: f32,
-    /// A list of tile blits to be done after drawing this surface.
-    pub tile_blits: Vec<TileBlit>,
 }
 
 impl SurfaceInfo {
@@ -1023,7 +1077,6 @@ impl SurfaceInfo {
             surface_spatial_node_index,
             tasks: Vec::new(),
             inflation_factor,
-            tile_blits: Vec::new(),
         }
     }
 
@@ -1533,7 +1586,12 @@ impl PicturePrimitive {
             }
         };
 
-        if self.raster_config.is_some() {
+        // Don't bother pushing a clip node collector for a tile cache, it's not
+        // actually an off-screen surface.
+        // TODO(gw): The way this is handled via the picture composite mode is not
+        //           ideal - we should fix this up and then be able to remove hacks
+        //           like this.
+        if self.raster_config.is_some() && self.tile_cache.is_none() {
             frame_state.clip_store
                 .push_surface(surface_spatial_node_index);
         }
@@ -1627,6 +1685,13 @@ impl PicturePrimitive {
         self.prim_list = prim_list;
         self.state = Some((state, context));
 
+        // Don't bother popping a clip node collector for a tile cache, it's not
+        // actually an off-screen surface (see comment when pushing surface for
+        // more information).
+        if self.tile_cache.is_some() {
+            return None;
+        }
+
         self.raster_config.as_ref().map(|_| {
             frame_state.clip_store.pop_surface()
         })
@@ -2074,13 +2139,10 @@ impl PicturePrimitive {
 
         let surface = match raster_config.composite_mode {
             PictureCompositeMode::TileCache { .. } => {
-                let tile_cache = self.tile_cache.as_mut().unwrap();
-
                 // For a picture surface, just push any child tasks and tile
                 // blits up to the parent surface.
                 let surface = &mut surfaces[surface_index.0];
                 surface.tasks.extend(child_tasks);
-                surface.tile_blits.extend(tile_cache.pending_blits.drain(..));
 
                 return true;
             }
@@ -2145,7 +2207,6 @@ impl PicturePrimitive {
                         uv_rect_kind,
                         pic_context.raster_spatial_node_index,
                         None,
-                        Vec::new(),
                     );
 
                     let picture_task_id = frame_state.render_tasks.add(picture_task);
@@ -2203,7 +2264,6 @@ impl PicturePrimitive {
                                 uv_rect_kind,
                                 pic_context.raster_spatial_node_index,
                                 None,
-                                Vec::new(),
                             );
 
                             let picture_task_id = render_tasks.add(picture_task);
@@ -2261,7 +2321,6 @@ impl PicturePrimitive {
                     uv_rect_kind,
                     pic_context.raster_spatial_node_index,
                     None,
-                    Vec::new(),
                 );
                 picture_task.mark_for_saving();
 
@@ -2328,7 +2387,6 @@ impl PicturePrimitive {
                     uv_rect_kind,
                     pic_context.raster_spatial_node_index,
                     None,
-                    Vec::new(),
                 );
 
                 let readback_task_id = frame_state.render_tasks.add(
@@ -2368,7 +2426,6 @@ impl PicturePrimitive {
                     uv_rect_kind,
                     pic_context.raster_spatial_node_index,
                     None,
-                    Vec::new(),
                 );
 
                 let render_task_id = frame_state.render_tasks.add(picture_task);
@@ -2400,7 +2457,6 @@ impl PicturePrimitive {
                     uv_rect_kind,
                     pic_context.raster_spatial_node_index,
                     None,
-                    Vec::new(),
                 );
 
                 let render_task_id = frame_state.render_tasks.add(picture_task);

webrender/src/prim_store/mod.rs

@@ -505,8 +505,8 @@ impl SizeKey {
 #[cfg_attr(feature = "replay", derive(Deserialize))]
 #[derive(Copy, Debug, Clone, PartialEq)]
 pub struct VectorKey {
-    x: f32,
-    y: f32,
+    pub x: f32,
+    pub y: f32,
 }
 
 impl Eq for VectorKey {}
@@ -1731,7 +1731,7 @@ impl PrimitiveStore {
             let mut tile_cache = state.tile_cache.take().unwrap();
 
             // Build the dirty region(s) for this tile cache.
-            tile_cache.post_update(
+            pic.local_clip_rect = tile_cache.post_update(
                 resource_cache,
                 gpu_cache,
                 clip_store,
@@ -3248,6 +3248,11 @@ pub fn get_raster_rects(
         device_pixel_scale,
     );
 
+    // Ensure that we won't try to allocate a zero-sized clip render task.
+    if clipped.is_empty() {
+        return None;
+    }
+
     Some((clipped.to_i32(), unclipped))
 }
 

webrender/src/render_task.rs

@@ -270,7 +270,6 @@ pub struct PictureTask {
     pub uv_rect_handle: GpuCacheHandle,
     pub root_spatial_node_index: SpatialNodeIndex,
     uv_rect_kind: UvRectKind,
-    pub blits: Vec<TileBlit>,
 }
 
 #[derive(Debug)]
@@ -434,7 +433,6 @@ impl RenderTask {
         uv_rect_kind: UvRectKind,
         root_spatial_node_index: SpatialNodeIndex,
         clear_color: Option<ColorF>,
-        blits: Vec<TileBlit>,
     ) -> Self {
         let size = match location {
             RenderTaskLocation::Dynamic(_, size) => size,
@@ -462,7 +460,6 @@ impl RenderTask {
                 uv_rect_handle: GpuCacheHandle::new(),
                 uv_rect_kind,
                 root_spatial_node_index,
-                blits,
             }),
             clear_mode,
             saved_index: None,