Simplify determining if a batch can be merged / needs scissor.

Previously, we checked if the bounding box of any child primitives
extended outside the rectangle of the allocated task rect. However,
there is a simpler way to calculate this.

If the allocated size of the render task is >= the unclipped size
of the picture bounding rect, no scissor is needed, since we know
that local clip rects will take care of ensuring nothing is
drawn outside the task boundary.

This is an optimization, but the main benefit is removing one more
piece of code that relies on knowledge of screen / device rects,
which simplifies the ongoing work to be able to rasterize in other
coordinate systems.

webrender/src/batch.rs

@@ -263,7 +263,6 @@ impl OpaqueBatchList {
 pub struct BatchList {
     pub alpha_batch_list: AlphaBatchList,
     pub opaque_batch_list: OpaqueBatchList,
-    pub combined_bounding_rect: DeviceIntRect,
 }
 
 impl BatchList {
@@ -275,24 +274,14 @@ impl BatchList {
         BatchList {
             alpha_batch_list: AlphaBatchList::new(),
             opaque_batch_list: OpaqueBatchList::new(batch_area_threshold),
-            combined_bounding_rect: DeviceIntRect::zero(),
         }
     }
 
-    fn add_bounding_rect(
-        &mut self,
-        task_relative_bounding_rect: &DeviceIntRect,
-    ) {
-        self.combined_bounding_rect = self.combined_bounding_rect.union(task_relative_bounding_rect);
-    }
-
     pub fn get_suitable_batch(
         &mut self,
         key: BatchKey,
         task_relative_bounding_rect: &DeviceIntRect,
     ) -> &mut Vec<PrimitiveInstance> {
-        self.add_bounding_rect(task_relative_bounding_rect);
-
         match key.blend_mode {
             BlendMode::None => {
                 self.opaque_batch_list
@@ -409,31 +398,27 @@ pub struct AlphaBatchBuilder {
     pub batch_list: BatchList,
     glyph_fetch_buffer: Vec<GlyphFetchResult>,
     target_rect: DeviceIntRect,
+    can_merge: bool,
 }
 
 impl AlphaBatchBuilder {
     pub fn new(
         screen_size: DeviceIntSize,
         target_rect: DeviceIntRect,
+        can_merge: bool,
     ) -> Self {
         AlphaBatchBuilder {
             batch_list: BatchList::new(screen_size),
             glyph_fetch_buffer: Vec::new(),
             target_rect,
+            can_merge,
         }
     }
 
     pub fn build(mut self, merged_batches: &mut AlphaBatchContainer) -> Option<AlphaBatchContainer> {
         self.batch_list.finalize();
 
-        let task_relative_target_rect = DeviceIntRect::new(
-            DeviceIntPoint::zero(),
-            self.target_rect.size,
-        );
-
-        let can_merge = task_relative_target_rect.contains_rect(&self.batch_list.combined_bounding_rect);
-
-        if can_merge {
+        if self.can_merge {
             merged_batches.merge(self);
             None
         } else {
@@ -1203,8 +1188,6 @@ impl AlphaBatchBuilder {
             brush_flags: BrushFlags::PERSPECTIVE_INTERPOLATION,
         };
 
-        self.batch_list.add_bounding_rect(task_relative_bounding_rect);
-
         let batch_key = BatchKey {
             blend_mode,
             kind: BatchKind::Brush(batch_kind),
@@ -1236,8 +1219,6 @@ impl AlphaBatchBuilder {
             brush_flags: BrushFlags::PERSPECTIVE_INTERPOLATION,
         };
 
-        self.batch_list.add_bounding_rect(task_relative_bounding_rect);
-
         match brush.segment_desc {
             Some(ref segment_desc) => {
                 let alpha_batch_key = BatchKey {
@@ -1317,7 +1298,6 @@ fn add_gradient_tiles(
     base_prim_header: &PrimitiveHeader,
     prim_headers: &mut PrimitiveHeaders,
 ) {
-    batch_list.add_bounding_rect(task_relative_bounding_rect);
     let batch = batch_list.get_suitable_batch(
         BatchKey {
             blend_mode: blend_mode,

webrender/src/frame_builder.rs

@@ -250,6 +250,7 @@ impl FrameBuilder {
 
         let root_render_task = RenderTask::new_picture(
             RenderTaskLocation::Fixed(frame_context.screen_rect),
+            frame_context.screen_rect.size,
             root_prim_index,
             DeviceIntPoint::zero(),
             pic_state.tasks,

webrender/src/glyph_rasterizer/pathfinder.rs

@@ -301,7 +301,7 @@ fn request_render_task_from_pathfinder(glyph_key: &GlyphKey,
     let subpixel_offset = TypedPoint2D::new(glyph_subpixel_offset as f32, 0.0);
     let embolden_amount = compute_embolden_amount(size.to_f32_px());
 
-    let location = RenderTaskLocation::Dynamic(None, Some(*glyph_size));
+    let location = RenderTaskLocation::Dynamic(None, *glyph_size);
     let glyph_render_task = RenderTask::new_glyph(location,
                                                   mesh,
                                                   &glyph_origin,

webrender/src/picture.rs

@@ -372,7 +372,8 @@ impl PicturePrimitive {
                 // relevant transforms haven't changed from frame to frame.
                 let surface = if pic_state_for_children.has_non_root_coord_system {
                     let picture_task = RenderTask::new_picture(
-                        RenderTaskLocation::Dynamic(None, Some(device_rect.size)),
+                        RenderTaskLocation::Dynamic(None, device_rect.size),
+                        prim_screen_rect.unclipped.size,
                         prim_index,
                         device_rect.origin,
                         pic_state_for_children.tasks,
@@ -428,7 +429,8 @@ impl PicturePrimitive {
                             let child_tasks = mem::replace(&mut pic_state_for_children.tasks, Vec::new());
 
                             let picture_task = RenderTask::new_picture(
-                                RenderTaskLocation::Dynamic(None, Some(device_rect.size)),
+                                RenderTaskLocation::Dynamic(None, device_rect.size),
+                                prim_screen_rect.unclipped.size,
                                 prim_index,
                                 device_rect.origin,
                                 child_tasks,
@@ -484,7 +486,8 @@ impl PicturePrimitive {
                 );
 
                 let mut picture_task = RenderTask::new_picture(
-                    RenderTaskLocation::Dynamic(None, Some(device_rect.size)),
+                    RenderTaskLocation::Dynamic(None, device_rect.size),
+                    prim_screen_rect.unclipped.size,
                     prim_index,
                     device_rect.origin,
                     pic_state_for_children.tasks,
@@ -553,7 +556,8 @@ impl PicturePrimitive {
                 );
 
                 let picture_task = RenderTask::new_picture(
-                    RenderTaskLocation::Dynamic(None, Some(prim_screen_rect.clipped.size)),
+                    RenderTaskLocation::Dynamic(None, prim_screen_rect.clipped.size),
+                    prim_screen_rect.unclipped.size,
                     prim_index,
                     prim_screen_rect.clipped.origin,
                     pic_state_for_children.tasks,
@@ -588,7 +592,8 @@ impl PicturePrimitive {
                 );
 
                 let picture_task = RenderTask::new_picture(
-                    RenderTaskLocation::Dynamic(None, Some(prim_screen_rect.clipped.size)),
+                    RenderTaskLocation::Dynamic(None, prim_screen_rect.clipped.size),
+                    prim_screen_rect.unclipped.size,
                     prim_index,
                     prim_screen_rect.clipped.origin,
                     pic_state_for_children.tasks,
@@ -608,7 +613,8 @@ impl PicturePrimitive {
                 );
 
                 let picture_task = RenderTask::new_picture(
-                    RenderTaskLocation::Dynamic(None, Some(prim_screen_rect.clipped.size)),
+                    RenderTaskLocation::Dynamic(None, prim_screen_rect.clipped.size),
+                    prim_screen_rect.unclipped.size,
                     prim_index,
                     prim_screen_rect.clipped.origin,
                     pic_state_for_children.tasks,

webrender/src/render_task.rs

@@ -178,7 +178,7 @@ impl ops::IndexMut<RenderTaskId> for RenderTaskTree {
 #[cfg_attr(feature = "replay", derive(Deserialize))]
 pub enum RenderTaskLocation {
     Fixed(DeviceIntRect),
-    Dynamic(Option<(DeviceIntPoint, RenderTargetIndex)>, Option<DeviceIntSize>),
+    Dynamic(Option<(DeviceIntPoint, RenderTargetIndex)>, DeviceIntSize),
     TextureCache(SourceTexture, i32, DeviceIntRect),
 }
 
@@ -202,6 +202,7 @@ pub struct ClipRegionTask {
 #[cfg_attr(feature = "replay", derive(Deserialize))]
 pub struct PictureTask {
     pub prim_index: PrimitiveIndex,
+    pub can_merge: bool,
     pub content_origin: DeviceIntPoint,
     pub uv_rect_handle: GpuCacheHandle,
     uv_rect_kind: UvRectKind,
@@ -330,7 +331,7 @@ impl RenderTask {
         render_task_sanity_check(&size);
 
         RenderTask {
-            location: RenderTaskLocation::Dynamic(None, Some(size)),
+            location: RenderTaskLocation::Dynamic(None, size),
             children,
             kind,
             clear_mode,
@@ -340,28 +341,30 @@ impl RenderTask {
 
     pub fn new_picture(
         location: RenderTaskLocation,
+        unclipped_size: DeviceIntSize,
         prim_index: PrimitiveIndex,
         content_origin: DeviceIntPoint,
         children: Vec<RenderTaskId>,
         uv_rect_kind: UvRectKind,
     ) -> Self {
         let size = match location {
-            RenderTaskLocation::Dynamic(_, Some(size)) => Some(size),
-            RenderTaskLocation::Fixed(rect) => Some(rect.size),
-            RenderTaskLocation::TextureCache(_, _, rect) => Some(rect.size),
-            _ => None,
+            RenderTaskLocation::Dynamic(_, size) => size,
+            RenderTaskLocation::Fixed(rect) => rect.size,
+            RenderTaskLocation::TextureCache(_, _, rect) => rect.size,
         };
 
-        if let Some(size) = size {
-            render_task_sanity_check(&size);
-        }
+        render_task_sanity_check(&size);
+
+        let can_merge = size.width >= unclipped_size.width &&
+                        size.height >= unclipped_size.height;
 
         RenderTask {
             location,
             children,
             kind: RenderTaskKind::Picture(PictureTask {
                 prim_index,
                 content_origin,
+                can_merge,
                 uv_rect_handle: GpuCacheHandle::new(),
                 uv_rect_kind,
             }),
@@ -770,10 +773,7 @@ impl RenderTask {
     pub fn get_dynamic_size(&self) -> DeviceIntSize {
         match self.location {
             RenderTaskLocation::Fixed(..) => DeviceIntSize::zero(),
-            RenderTaskLocation::Dynamic(_, Some(size)) => size,
-            RenderTaskLocation::Dynamic(_, None) => {
-                panic!("bug: render task must have assigned size by now");
-            }
+            RenderTaskLocation::Dynamic(_, size) => size,
             RenderTaskLocation::TextureCache(_, _, rect) => rect.size,
         }
     }
@@ -798,7 +798,6 @@ impl RenderTask {
             //           to mark a task as unused explicitly. This
             //           would allow us to restore this debug check.
             RenderTaskLocation::Dynamic(Some((origin, target_index)), size) => {
-                let size = size.expect("bug: must be assigned a size by now");
                 (DeviceIntRect::new(origin, size), target_index)
             }
             RenderTaskLocation::Dynamic(None, _) => {
@@ -1096,10 +1095,7 @@ impl RenderTaskCache {
                     RenderTaskLocation::TextureCache(..) => {
                         panic!("BUG: dynamic task was expected");
                     }
-                    RenderTaskLocation::Dynamic(_, None) => {
-                        panic!("BUG: must have assigned size by now");
-                    }
-                    RenderTaskLocation::Dynamic(_, Some(size)) => size,
+                    RenderTaskLocation::Dynamic(_, size) => size,
                 };
 
                 // Select the right texture page to allocate from.

webrender/src/tiling.rs

@@ -351,7 +351,11 @@ impl RenderTarget for ColorRenderTarget {
 
                     let (target_rect, _) = task.get_target_rect();
 
-                    let mut batch_builder = AlphaBatchBuilder::new(self.screen_size, target_rect);
+                    let mut batch_builder = AlphaBatchBuilder::new(
+                        self.screen_size,
+                        target_rect,
+                        pic_task.can_merge,
+                    );
 
                     batch_builder.add_pic_to_batch(
                         pic,
@@ -845,7 +849,6 @@ impl RenderPass {
                                 None
                             }
                             RenderTaskLocation::Dynamic(ref mut origin, size) => {
-                                let size = size.expect("bug: size must be assigned by now");
                                 let alloc_size = DeviceUintSize::new(size.width as u32, size.height as u32);
                                 let (alloc_origin, target_index) =  match target_kind {
                                     RenderTargetKind::Color => color.allocate(alloc_size),