Bug 1551520 - Use WR relative transform instead of the world inverse. r=gw a=jcristau
authorDzmitry Malyshau <dmalyshau@mozilla.com>
Thu, 23 May 2019 15:43:33 -0400
changeset 533399 39c14a33468fde1a7316fc135243b647e750014f
parent 533398 fa195c94dc1082186356e97094feb62e4e1f5b01
child 533400 298f7d8bb0c03c97cb5254ca5fbe9835f3d8575b
push id11308
push usernbeleuzu@mozilla.com
push dateFri, 24 May 2019 19:41:29 +0000
treeherdermozilla-beta@298f7d8bb0c0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgw, jcristau
bugs1551520
milestone68.0
Bug 1551520 - Use WR relative transform instead of the world inverse. r=gw a=jcristau Summary: This is a follow-up to https://phabricator.services.mozilla.com/D30600 Previously, I changed changed the space mapper logic to use the world transformations. This was seemingly needed because we requrested the relation between primitives and their clip nodes, which could be in unrelated spatial sub-trees. However, I believe the change was a mistake, since for clips we should not even try to get the relative mapping, and clipping is done in world space for these cases. This change reverts that logic back. Fingers crossed for the try to not show any asserts firing up inside get_relative_transform. Test Plan: pending try https://treeherder.mozilla.org/#/jobs?repo=try&revision=dbd8fe9a797c680db0f64f02b019b2b4eaab4dd4 Reviewers: gw Tags: #secure-revision Bug #: 1551520 Differential Revision: https://phabricator.services.mozilla.com/D32382
gfx/wr/webrender/src/clip_scroll_tree.rs
gfx/wr/webrender/src/display_list_flattener.rs
gfx/wr/webrender/src/picture.rs
gfx/wr/webrender/src/prim_store/mod.rs
--- a/gfx/wr/webrender/src/clip_scroll_tree.rs
+++ b/gfx/wr/webrender/src/clip_scroll_tree.rs
@@ -222,29 +222,16 @@ impl<Src, Dst> CoordinateSpaceMapping<Sr
         match self {
             CoordinateSpaceMapping::Local => CoordinateSpaceMapping::Local,
             CoordinateSpaceMapping::ScaleOffset(scale_offset) => CoordinateSpaceMapping::ScaleOffset(scale_offset),
             CoordinateSpaceMapping::Transform(transform) => CoordinateSpaceMapping::Transform(
                 transform.with_destination::<NewDst>()
             ),
         }
     }
-
-    pub fn post_mul_transform<NewDst>(
-        &self, other: &CoordinateSpaceMapping<Dst, NewDst>
-    ) -> TypedTransform3D<f32, Src, NewDst>
-    where Self: Clone
-    {
-        let matrix = self.clone().into_transform();
-        match *other {
-            CoordinateSpaceMapping::Local => matrix.with_destination::<NewDst>(),
-            CoordinateSpaceMapping::ScaleOffset(ref scale_offset) => matrix.post_mul(&scale_offset.to_transform()),
-            CoordinateSpaceMapping::Transform(ref transform) => matrix.post_mul(transform),
-        }
-    }
 }
 
 impl ClipScrollTree {
     pub fn new() -> Self {
         ClipScrollTree {
             spatial_nodes: Vec::new(),
             coord_systems: Vec::new(),
             pending_scroll_offsets: FastHashMap::default(),
--- a/gfx/wr/webrender/src/display_list_flattener.rs
+++ b/gfx/wr/webrender/src/display_list_flattener.rs
@@ -507,17 +507,16 @@ impl<'a> DisplayListFlattener<'a> {
             }
         );
 
         let tile_cache = TileCache::new(
             main_scroll_root,
             &prim_list.prim_instances,
             *self.pipeline_clip_chain_stack.last().unwrap(),
             &self.prim_store.pictures,
-            &self.clip_scroll_tree,
         );
 
         let pic_index = self.prim_store.pictures.alloc().init(PicturePrimitive::new_image(
             Some(PictureCompositeMode::TileCache { clear_color: ColorF::new(1.0, 1.0, 1.0, 1.0) }),
             Picture3DContext::Out,
             self.scene.root_pipeline_id.unwrap(),
             None,
             true,
--- a/gfx/wr/webrender/src/picture.rs
+++ b/gfx/wr/webrender/src/picture.rs
@@ -698,32 +698,30 @@ fn collect_ref_prims(
 }
 
 impl TileCache {
     pub fn new(
         spatial_node_index: SpatialNodeIndex,
         prim_instances: &[PrimitiveInstance],
         root_clip_chain_id: ClipChainId,
         pictures: &[PicturePrimitive],
-        clip_scroll_tree: &ClipScrollTree,
     ) -> Self {
         // Build the list of reference primitives
         // for this picture cache.
         let reference_prims = ReferencePrimitiveList::new(
             prim_instances,
             pictures,
         );
 
         TileCache {
             spatial_node_index,
             tiles: Vec::new(),
             map_local_to_world: SpaceMapper::new(
                 ROOT_SPATIAL_NODE_INDEX,
                 WorldRect::zero(),
-                clip_scroll_tree,
             ),
             tiles_to_draw: Vec::new(),
             opacity_bindings: FastHashMap::default(),
             dirty_region: DirtyRegion::new(),
             world_origin: WorldPoint::zero(),
             world_tile_size: WorldSize::zero(),
             tile_count: TileSize::zero(),
             scroll_offset: None,
@@ -829,17 +827,16 @@ impl TileCache {
         }.unwrap_or(WorldVector2D::zero());
 
         // Assume no tiles are valid to draw by default
         self.tiles_to_draw.clear();
 
         self.map_local_to_world = SpaceMapper::new(
             ROOT_SPATIAL_NODE_INDEX,
             frame_context.screen_world_rect,
-            frame_context.clip_scroll_tree,
         );
 
         let world_mapper = SpaceMapper::new_with_target(
             ROOT_SPATIAL_NODE_INDEX,
             self.spatial_node_index,
             frame_context.screen_world_rect,
             frame_context.clip_scroll_tree,
         );
@@ -1838,17 +1835,16 @@ impl SurfaceInfo {
 
         let pic_bounds = map_surface_to_world
             .unmap(&map_surface_to_world.bounds)
             .unwrap_or(PictureRect::max_rect());
 
         let map_local_to_surface = SpaceMapper::new(
             surface_spatial_node_index,
             pic_bounds,
-            clip_scroll_tree,
         );
 
         SurfaceInfo {
             rect: PictureRect::zero(),
             map_local_to_surface,
             render_tasks: None,
             raster_spatial_node_index,
             surface_spatial_node_index,
@@ -2396,17 +2392,16 @@ impl PicturePrimitive {
         );
 
         let pic_bounds = map_pic_to_world.unmap(&map_pic_to_world.bounds)
                                          .unwrap_or(PictureRect::max_rect());
 
         let map_local_to_pic = SpaceMapper::new(
             surface_spatial_node_index,
             pic_bounds,
-            frame_context.clip_scroll_tree,
         );
 
         let (map_raster_to_world, map_pic_to_raster) = create_raster_mappers(
             surface_spatial_node_index,
             raster_spatial_node_index,
             frame_context.screen_world_rect,
             frame_context.clip_scroll_tree,
         );
@@ -2530,67 +2525,67 @@ impl PicturePrimitive {
                     }
                     PictureCompositeMode::Filter(Filter::DropShadows(ref shadows)) => {
                         let mut max_std_deviation = 0.0;
                         for shadow in shadows {
                             // TODO(nical) presumably we should compute the clipped rect for each shadow
                             // and compute the union of them to determine what we need to rasterize and blur?
                             max_std_deviation = f32::max(max_std_deviation, shadow.blur_radius * device_pixel_scale.0);
                         }
-        
+
                         max_std_deviation = max_std_deviation.round();
                         let max_blur_range = (max_std_deviation * BLUR_SAMPLE_SCALE).ceil() as i32;
                         let mut device_rect = clipped.inflate(max_blur_range, max_blur_range)
                                 .intersection(&unclipped.to_i32())
                                 .unwrap();
                         device_rect.size = RenderTask::adjusted_blur_source_size(
                             device_rect.size,
                             DeviceSize::new(max_std_deviation, max_std_deviation),
                         );
-        
+
                         let uv_rect_kind = calculate_uv_rect_kind(
                             &pic_rect,
                             &transform,
                             &device_rect,
                             device_pixel_scale,
                             true,
                         );
-        
+
                         let mut picture_task = RenderTask::new_picture(
                             RenderTaskLocation::Dynamic(None, device_rect.size),
                             unclipped.size,
                             pic_index,
                             device_rect.origin,
                             uv_rect_kind,
                             raster_spatial_node_index,
                             device_pixel_scale,
                         );
                         picture_task.mark_for_saving();
-        
+
                         let picture_task_id = frame_state.render_tasks.add(picture_task);
-        
+
                         self.secondary_render_task_id = Some(picture_task_id);
-        
+
                         let mut blur_tasks = BlurTaskCache::default();
-        
+
                         self.extra_gpu_data_handles.resize(shadows.len(), GpuCacheHandle::new());
-        
+
                         let mut blur_render_task_id = picture_task_id;
                         for shadow in shadows {
                             let std_dev = f32::round(shadow.blur_radius * device_pixel_scale.0);
                             blur_render_task_id = RenderTask::new_blur(
                                 DeviceSize::new(std_dev, std_dev),
                                 picture_task_id,
                                 frame_state.render_tasks,
                                 RenderTargetKind::Color,
                                 ClearMode::Transparent,
                                 Some(&mut blur_tasks),
-                            );      
+                            );
                         }
-        
+
                         // TODO(nical) the second one should to be the blur's task id but we have several blurs now
                         (blur_render_task_id, picture_task_id)
                     }
                     PictureCompositeMode::MixBlend(..) if !frame_context.fb_config.gpu_supports_advanced_blend => {
                         let uv_rect_kind = calculate_uv_rect_kind(
                             &pic_rect,
                             &transform,
                             &clipped,
@@ -3393,17 +3388,16 @@ fn build_ref_prims(
     prim_map: &mut FastHashMap<ItemUid, WorldPoint>,
     clip_scroll_tree: &ClipScrollTree,
 ) {
     prim_map.clear();
 
     let mut map_local_to_world = SpaceMapper::new(
         ROOT_SPATIAL_NODE_INDEX,
         WorldRect::zero(),
-        clip_scroll_tree,
     );
 
     for ref_prim in ref_prims {
         map_local_to_world.set_target_spatial_node(
             ref_prim.spatial_node_index,
             clip_scroll_tree,
         );
 
--- a/gfx/wr/webrender/src/prim_store/mod.rs
+++ b/gfx/wr/webrender/src/prim_store/mod.rs
@@ -129,49 +129,42 @@ impl PrimitiveOpacity {
     }
 }
 
 
 #[derive(Debug, Clone)]
 pub struct SpaceMapper<F, T> {
     kind: CoordinateSpaceMapping<F, T>,
     pub ref_spatial_node_index: SpatialNodeIndex,
-    ref_world_inv_transform: CoordinateSpaceMapping<WorldPixel, T>,
     pub current_target_spatial_node_index: SpatialNodeIndex,
     pub bounds: TypedRect<f32, T>,
     visible_face: VisibleFace,
 }
 
 impl<F, T> SpaceMapper<F, T> where F: fmt::Debug {
     pub fn new(
         ref_spatial_node_index: SpatialNodeIndex,
         bounds: TypedRect<f32, T>,
-        clip_scroll_tree: &ClipScrollTree,
     ) -> Self {
         SpaceMapper {
             kind: CoordinateSpaceMapping::Local,
             ref_spatial_node_index,
-            ref_world_inv_transform: clip_scroll_tree
-                .get_world_transform(ref_spatial_node_index)
-                .inverse()
-                .unwrap()
-                .with_destination::<T>(),
             current_target_spatial_node_index: ref_spatial_node_index,
             bounds,
             visible_face: VisibleFace::Front,
         }
     }
 
     pub fn new_with_target(
         ref_spatial_node_index: SpatialNodeIndex,
         target_node_index: SpatialNodeIndex,
         bounds: TypedRect<f32, T>,
         clip_scroll_tree: &ClipScrollTree,
     ) -> Self {
-        let mut mapper = SpaceMapper::new(ref_spatial_node_index, bounds, clip_scroll_tree);
+        let mut mapper = Self::new(ref_spatial_node_index, bounds);
         mapper.set_target_spatial_node(target_node_index, clip_scroll_tree);
         mapper
     }
 
     pub fn set_target_spatial_node(
         &mut self,
         target_node_index: SpatialNodeIndex,
         clip_scroll_tree: &ClipScrollTree,
@@ -187,19 +180,20 @@ impl<F, T> SpaceMapper<F, T> where F: fm
             CoordinateSpaceMapping::Local
         } else if ref_spatial_node.coordinate_system_id == target_spatial_node.coordinate_system_id {
             let scale_offset = ref_spatial_node.coordinate_system_relative_scale_offset
                 .inverse()
                 .accumulate(&target_spatial_node.coordinate_system_relative_scale_offset);
             CoordinateSpaceMapping::ScaleOffset(scale_offset)
         } else {
             let transform = clip_scroll_tree
-                .get_world_transform(target_node_index)
-                .post_mul_transform(&self.ref_world_inv_transform)
-                .with_source::<F>();
+                .get_relative_transform(target_node_index, self.ref_spatial_node_index)
+                .into_transform()
+                .with_source::<F>()
+                .with_destination::<T>();
             CoordinateSpaceMapping::Transform(transform)
         };
 
         self.visible_face = self.kind.visible_face();
         self.current_target_spatial_node_index = target_node_index;
     }
 
     pub fn get_transform(&self) -> TypedTransform3D<f32, F, T> {
@@ -1788,17 +1782,16 @@ impl PrimitiveStore {
             surface.surface_spatial_node_index,
             frame_context.screen_world_rect,
             frame_context.clip_scroll_tree,
         );
 
         let mut map_local_to_raster = SpaceMapper::new(
             surface.raster_spatial_node_index,
             RasterRect::max_rect(),
-            frame_context.clip_scroll_tree,
         );
 
         let mut surface_rect = PictureRect::zero();
 
         for prim_instance in &mut prim_list.prim_instances {
             prim_instance.reset();
 
             if prim_instance.is_chased() {