Bug 1554502 - Configurable lookback count r=gw a=jcristau
☠☠ backed out by 73ae511ea895 ☠ ☠
authorDzmitry Malyshau <dmalyshau@mozilla.com>
Thu, 30 May 2019 01:29:43 +0000
changeset 536778 4ab0defb9012f8377cc29b5ee6da821699f2d16f
parent 536777 08677012f38a518182681e55151f31706c018449
child 536779 e4b071df2c1c3359916ca49ad964817ec9e724ff
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgw, jcristau
bugs1554502
milestone68.0
Bug 1554502 - Configurable lookback count r=gw a=jcristau We've had a constant of 10 hard-coded there since early days. Turning it into a configurable number allows us to easier tune it and debug related issues. Differential Revision: https://phabricator.services.mozilla.com/D32761
gfx/wr/webrender/src/batch.rs
gfx/wr/webrender/src/frame_builder.rs
gfx/wr/webrender/src/renderer.rs
gfx/wr/webrender/src/tiling.rs
--- a/gfx/wr/webrender/src/batch.rs
+++ b/gfx/wr/webrender/src/batch.rs
@@ -134,137 +134,150 @@ fn textures_compatible(t1: TextureSource
 }
 
 pub struct AlphaBatchList {
     pub batches: Vec<PrimitiveBatch>,
     pub item_rects: Vec<Vec<PictureRect>>,
     current_batch_index: usize,
     current_z_id: ZBufferId,
     break_advanced_blend_batches: bool,
+    lookback_count: usize,
 }
 
 impl AlphaBatchList {
-    fn new(break_advanced_blend_batches: bool) -> Self {
+    fn new(break_advanced_blend_batches: bool, lookback_count: usize) -> Self {
         AlphaBatchList {
             batches: Vec::new(),
             item_rects: Vec::new(),
             current_z_id: ZBufferId::invalid(),
             current_batch_index: usize::MAX,
             break_advanced_blend_batches,
+            lookback_count,
         }
     }
 
     pub fn set_params_and_get_batch(
         &mut self,
         key: BatchKey,
-        bounding_rect: &PictureRect,
+        // The bounding box of everything at this Z plane. We expect potentially
+        // multiple primitive segments coming with the same `z_id`.
+        z_bounding_rect: &PictureRect,
         z_id: ZBufferId,
     ) -> &mut Vec<PrimitiveInstanceData> {
         if z_id != self.current_z_id ||
            self.current_batch_index == usize::MAX ||
            !self.batches[self.current_batch_index].key.is_compatible_with(&key)
         {
             let mut selected_batch_index = None;
 
             match key.blend_mode {
                 BlendMode::SubpixelWithBgColor => {
-                    'outer_multipass: for (batch_index, batch) in self.batches.iter().enumerate().rev().take(10) {
+                    'outer_multipass: for (batch_index, batch) in self.batches.iter().enumerate().rev().take(self.lookback_count) {
                         // Some subpixel batches are drawn in two passes. Because of this, we need
                         // to check for overlaps with every batch (which is a bit different
                         // than the normal batching below).
                         for item_rect in &self.item_rects[batch_index] {
-                            if item_rect.intersects(bounding_rect) {
+                            if item_rect.intersects(z_bounding_rect) {
                                 break 'outer_multipass;
                             }
                         }
 
                         if batch.key.is_compatible_with(&key) {
                             selected_batch_index = Some(batch_index);
                             break;
                         }
                     }
                 }
                 BlendMode::Advanced(_) if self.break_advanced_blend_batches => {
                     // don't try to find a batch
                 }
                 _ => {
-                    'outer_default: for (batch_index, batch) in self.batches.iter().enumerate().rev().take(10) {
+                    'outer_default: for (batch_index, batch) in self.batches.iter().enumerate().rev().take(self.lookback_count) {
                         // For normal batches, we only need to check for overlaps for batches
                         // other than the first batch we consider. If the first batch
                         // is compatible, then we know there isn't any potential overlap
                         // issues to worry about.
                         if batch.key.is_compatible_with(&key) {
                             selected_batch_index = Some(batch_index);
                             break;
                         }
 
                         // check for intersections
                         for item_rect in &self.item_rects[batch_index] {
-                            if item_rect.intersects(bounding_rect) {
+                            if item_rect.intersects(z_bounding_rect) {
                                 break 'outer_default;
                             }
                         }
                     }
                 }
             }
 
             if selected_batch_index.is_none() {
                 let new_batch = PrimitiveBatch::new(key);
                 selected_batch_index = Some(self.batches.len());
                 self.batches.push(new_batch);
                 self.item_rects.push(Vec::new());
             }
 
             self.current_batch_index = selected_batch_index.unwrap();
-            self.item_rects[self.current_batch_index].push(*bounding_rect);
+            self.item_rects[self.current_batch_index].push(*z_bounding_rect);
             self.current_z_id = z_id;
+        } else if cfg!(debug_assertions) {
+            // If it's a different segment of the same (larger) primitive, we expect the bounding box
+            // to be the same - coming from the primitive itself, not the segment.
+            assert_eq!(self.item_rects[self.current_batch_index].last(), Some(z_bounding_rect));
         }
 
         &mut self.batches[self.current_batch_index].instances
     }
 }
 
 pub struct OpaqueBatchList {
     pub pixel_area_threshold_for_new_batch: f32,
     pub batches: Vec<PrimitiveBatch>,
     pub current_batch_index: usize,
+    lookback_count: usize,
 }
 
 impl OpaqueBatchList {
-    fn new(pixel_area_threshold_for_new_batch: f32) -> Self {
+    fn new(pixel_area_threshold_for_new_batch: f32, lookback_count: usize) -> Self {
         OpaqueBatchList {
             batches: Vec::new(),
             pixel_area_threshold_for_new_batch,
             current_batch_index: usize::MAX,
+            lookback_count,
         }
     }
 
     pub fn set_params_and_get_batch(
         &mut self,
         key: BatchKey,
-        bounding_rect: &PictureRect,
+        // The bounding box of everything at the current Z, whatever it is. We expect potentially
+        // multiple primitive segments produced by a primitive, which we allow to check
+        // `current_batch_index` instead of iterating the batches.
+        z_bounding_rect: &PictureRect,
     ) -> &mut Vec<PrimitiveInstanceData> {
         if self.current_batch_index == usize::MAX ||
            !self.batches[self.current_batch_index].key.is_compatible_with(&key) {
             let mut selected_batch_index = None;
-            let item_area = bounding_rect.size.area();
+            let item_area = z_bounding_rect.size.area();
 
             // If the area of this primitive is larger than the given threshold,
             // then it is large enough to warrant breaking a batch for. In this
             // case we just see if it can be added to the existing batch or
             // create a new one.
             if item_area > self.pixel_area_threshold_for_new_batch {
                 if let Some(batch) = self.batches.last() {
                     if batch.key.is_compatible_with(&key) {
                         selected_batch_index = Some(self.batches.len() - 1);
                     }
                 }
             } else {
                 // Otherwise, look back through a reasonable number of batches.
-                for (batch_index, batch) in self.batches.iter().enumerate().rev().take(10) {
+                for (batch_index, batch) in self.batches.iter().enumerate().rev().take(self.lookback_count) {
                     if batch.key.is_compatible_with(&key) {
                         selected_batch_index = Some(batch_index);
                         break;
                     }
                 }
             }
 
             if selected_batch_index.is_none() {
@@ -302,24 +315,25 @@ pub struct BatchList {
 }
 
 impl BatchList {
     pub fn new(
         screen_size: DeviceIntSize,
         regions: Vec<DeviceIntRect>,
         tile_blits: Vec<TileBlit>,
         break_advanced_blend_batches: bool,
+        lookback_count: usize,
     ) -> Self {
         // The threshold for creating a new batch is
         // one quarter the screen size.
         let batch_area_threshold = (screen_size.width * screen_size.height) as f32 / 4.0;
 
         BatchList {
-            alpha_batch_list: AlphaBatchList::new(break_advanced_blend_batches),
-            opaque_batch_list: OpaqueBatchList::new(batch_area_threshold),
+            alpha_batch_list: AlphaBatchList::new(break_advanced_blend_batches, lookback_count),
+            opaque_batch_list: OpaqueBatchList::new(batch_area_threshold, lookback_count),
             regions,
             tile_blits,
         }
     }
 
     pub fn push_single_instance(
         &mut self,
         key: BatchKey,
@@ -461,52 +475,57 @@ struct SegmentInstanceData {
     user_data: i32,
 }
 
 /// Encapsulates the logic of building batches for items that are blended.
 pub struct AlphaBatchBuilder {
     pub batch_lists: Vec<BatchList>,
     screen_size: DeviceIntSize,
     break_advanced_blend_batches: bool,
+    lookback_count: usize,
     render_task_id: RenderTaskId,
 }
 
 impl AlphaBatchBuilder {
     pub fn new(
         screen_size: DeviceIntSize,
         break_advanced_blend_batches: bool,
+        lookback_count: usize,
         render_task_id: RenderTaskId,
     ) -> Self {
         let batch_lists = vec![
             BatchList::new(
                 screen_size,
                 Vec::new(),
                 Vec::new(),
                 break_advanced_blend_batches,
+                lookback_count,
             ),
         ];
 
         AlphaBatchBuilder {
             batch_lists,
             screen_size,
             break_advanced_blend_batches,
+            lookback_count,
             render_task_id,
         }
     }
 
     fn push_new_batch_list(
         &mut self,
         regions: Vec<DeviceIntRect>,
         tile_blits: Vec<TileBlit>,
     ) {
         self.batch_lists.push(BatchList::new(
             self.screen_size,
             regions,
             tile_blits,
             self.break_advanced_blend_batches,
+            self.lookback_count,
         ));
     }
 
     fn current_batch_list(&mut self) -> &mut BatchList {
         self.batch_lists.last_mut().unwrap()
     }
 
     fn can_merge(&self) -> bool {
@@ -1005,20 +1024,20 @@ impl BatchBuilder {
                     specific_prim_address: prim_cache_address,
                     transform_id,
                 };
 
                 match picture.context_3d {
                     // Convert all children of the 3D hierarchy root into batches.
                     Picture3DContext::In { root_data: Some(ref list), .. } => {
                         for child in list {
-                            let prim_instance = &picture.prim_list.prim_instances[child.anchor];
-                            let prim_info = &ctx.scratch.prim_info[prim_instance.visibility_info.0 as usize];
-
-                            let child_pic_index = match prim_instance.kind {
+                            let child_prim_instance = &picture.prim_list.prim_instances[child.anchor];
+                            let child_prim_info = &ctx.scratch.prim_info[child_prim_instance.visibility_info.0 as usize];
+
+                            let child_pic_index = match child_prim_instance.kind {
                                 PrimitiveInstanceKind::Picture { pic_index, .. } => pic_index,
                                 PrimitiveInstanceKind::LineDecoration { .. } |
                                 PrimitiveInstanceKind::TextRun { .. } |
                                 PrimitiveInstanceKind::NormalBorder { .. } |
                                 PrimitiveInstanceKind::ImageBorder { .. } |
                                 PrimitiveInstanceKind::Rectangle { .. } |
                                 PrimitiveInstanceKind::YuvImage { .. } |
                                 PrimitiveInstanceKind::Image { .. } |
@@ -1027,23 +1046,23 @@ impl BatchBuilder {
                                 PrimitiveInstanceKind::Clear { .. } => {
                                     unreachable!();
                                 }
                             };
                             let pic = &ctx.prim_store.pictures[child_pic_index.0];
 
                             // Get clip task, if set, for the picture primitive.
                             let clip_task_address = ctx.get_prim_clip_task_address(
-                                prim_info.clip_task_index,
+                                child_prim_info.clip_task_index,
                                 render_tasks,
                             ).unwrap_or(OPAQUE_TASK_ADDRESS);
 
                             let prim_header = PrimitiveHeader {
                                 local_rect: pic.snapped_local_rect,
-                                local_clip_rect: prim_info.combined_local_clip_rect,
+                                local_clip_rect: child_prim_info.combined_local_clip_rect,
                                 snap_offsets,
                                 specific_prim_address: GpuCacheAddress::INVALID,
                                 transform_id: transforms
                                     .get_id(
                                         child.spatial_node_index,
                                         root_spatial_node_index,
                                         ctx.clip_scroll_tree,
                                     ),
--- a/gfx/wr/webrender/src/frame_builder.rs
+++ b/gfx/wr/webrender/src/frame_builder.rs
@@ -56,16 +56,17 @@ pub struct FrameBuilderConfig {
     pub dual_source_blending_is_enabled: bool,
     pub chase_primitive: ChasePrimitive,
     pub enable_picture_caching: bool,
     /// True if we're running tests (i.e. via wrench).
     pub testing: bool,
     pub gpu_supports_fast_clears: bool,
     pub gpu_supports_advanced_blend: bool,
     pub advanced_blend_is_coherent: bool,
+    pub batch_lookback_count: usize,
 }
 
 /// A set of common / global resources that are retained between
 /// new display lists, such that any GPU cache handles can be
 /// persisted even when a new display list arrives.
 #[cfg_attr(feature = "capture", derive(Serialize))]
 pub struct FrameGlobalResources {
     /// The image shader block for the most common / default
@@ -221,16 +222,17 @@ impl FrameBuilder {
                 dual_source_blending_is_enabled: true,
                 dual_source_blending_is_supported: false,
                 chase_primitive: ChasePrimitive::Nothing,
                 enable_picture_caching: false,
                 testing: false,
                 gpu_supports_fast_clears: false,
                 gpu_supports_advanced_blend: false,
                 advanced_blend_is_coherent: false,
+                batch_lookback_count: 0,
             },
         }
     }
 
     /// Provide any cached surface tiles from the previous frame builder
     /// to a new frame builder. These will be consumed or dropped the
     /// first time a new frame builder creates a frame.
     pub fn set_retained_resources(
@@ -587,16 +589,17 @@ impl FrameBuilder {
             for pass in &mut passes {
                 let mut ctx = RenderTargetContext {
                     global_device_pixel_scale,
                     prim_store: &self.prim_store,
                     resource_cache,
                     use_dual_source_blending,
                     use_advanced_blending: self.config.gpu_supports_advanced_blend,
                     break_advanced_blend_batches: !self.config.advanced_blend_is_coherent,
+                    batch_lookback_count: self.config.batch_lookback_count,
                     clip_scroll_tree,
                     data_stores,
                     surfaces: &surfaces,
                     scratch,
                     screen_world_rect,
                     globals: &self.globals,
                 };
 
--- a/gfx/wr/webrender/src/renderer.rs
+++ b/gfx/wr/webrender/src/renderer.rs
@@ -112,16 +112,18 @@ use time::precise_time_ns;
 
 cfg_if! {
     if #[cfg(feature = "debugger")] {
         use serde_json;
         use crate::debug_server;
     }
 }
 
+const DEFAULT_BATCH_LOOKBACK_COUNT: usize = 10;
+
 /// Is only false if no WR instances have ever been created.
 static HAS_BEEN_INITIALIZED: AtomicBool = AtomicBool::new(false);
 
 /// Returns true if a WR instance has ever been initialized in this process.
 pub fn wr_has_been_initialized() -> bool {
     HAS_BEEN_INITIALIZED.load(Ordering::SeqCst)
 }
 
@@ -2213,16 +2215,17 @@ impl Renderer {
             dual_source_blending_is_enabled: true,
             dual_source_blending_is_supported: use_dual_source_blending,
             chase_primitive: options.chase_primitive,
             enable_picture_caching: options.enable_picture_caching,
             testing: options.testing,
             gpu_supports_fast_clears: options.gpu_supports_fast_clears,
             gpu_supports_advanced_blend: ext_blend_equation_advanced,
             advanced_blend_is_coherent: ext_blend_equation_advanced_coherent,
+            batch_lookback_count: options.batch_lookback_count,
         };
         info!("WR {:?}", config);
 
         let device_pixel_ratio = options.device_pixel_ratio;
         let debug_flags = options.debug_flags;
         let payload_rx_for_backend = payload_rx.to_mpsc_receiver();
         let size_of_op = options.size_of_op;
         let enclosing_size_of_op = options.enclosing_size_of_op;
@@ -5671,16 +5674,19 @@ pub struct RendererOptions {
     pub gpu_supports_fast_clears: bool,
     pub allow_dual_source_blending: bool,
     pub allow_advanced_blend_equation: bool,
     /// If true, allow WR to use pixel local storage if the device supports it.
     /// For now, this defaults to false since the code is still experimental
     /// and not complete. This option will probably be removed once support is
     /// complete, and WR can implicitly choose whether to make use of PLS.
     pub allow_pixel_local_storage_support: bool,
+    /// Number of batches to look back in history for adding the current
+    /// transparent instance into.
+    pub batch_lookback_count: usize,
     /// Start the debug server for this renderer.
     pub start_debug_server: bool,
 }
 
 impl Default for RendererOptions {
     fn default() -> Self {
         RendererOptions {
             device_pixel_ratio: 1.0,
@@ -5714,16 +5720,17 @@ impl Default for RendererOptions {
             support_low_priority_transactions: false,
             namespace_alloc_by_client: false,
             enable_picture_caching: false,
             testing: false,
             gpu_supports_fast_clears: false,
             allow_dual_source_blending: true,
             allow_advanced_blend_equation: false,
             allow_pixel_local_storage_support: false,
+            batch_lookback_count: DEFAULT_BATCH_LOOKBACK_COUNT,
             // For backwards compatibility we set this to true by default, so
             // that if the debugger feature is enabled, the debug server will
             // be started automatically. Users can explicitly disable this as
             // needed.
             start_debug_server: true,
         }
     }
 }
--- a/gfx/wr/webrender/src/tiling.rs
+++ b/gfx/wr/webrender/src/tiling.rs
@@ -53,16 +53,17 @@ pub struct RenderTargetIndex(pub usize);
 
 pub struct RenderTargetContext<'a, 'rc> {
     pub global_device_pixel_scale: DevicePixelScale,
     pub prim_store: &'a PrimitiveStore,
     pub resource_cache: &'rc mut ResourceCache,
     pub use_dual_source_blending: bool,
     pub use_advanced_blending: bool,
     pub break_advanced_blend_batches: bool,
+    pub batch_lookback_count: usize,
     pub clip_scroll_tree: &'a ClipScrollTree,
     pub data_stores: &'a DataStores,
     pub surfaces: &'a [SurfaceInfo],
     pub scratch: &'a PrimitiveScratchBuffer,
     pub screen_world_rect: WorldRect,
     pub globals: &'a FrameGlobalResources,
 }
 
@@ -434,16 +435,17 @@ impl RenderTarget for ColorRenderTarget 
                         None
                     } else {
                         Some(target_rect)
                     };
 
                     let mut alpha_batch_builder = AlphaBatchBuilder::new(
                         self.screen_size,
                         ctx.break_advanced_blend_batches,
+                        ctx.batch_lookback_count,
                         *task_id,
                     );
 
                     self.batch_builder.add_pic_to_batch(
                         pic,
                         &mut alpha_batch_builder,
                         ctx,
                         gpu_cache,