Bug 1498639 - Give WR the id of the scroll frame perspective scrolls relative to, and compute the right transform based on that. r=kats,kvark a=lizzard
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 28 Jan 2019 23:41:08 +0000
changeset 515689 551c4945683f0fab67725f6268ff78ecb77d7526
parent 515688 575fb73398a7f5ba784c492f1c3583fdd1b32e09
child 515690 b0ffd7b7d7ed7761aff58ef2dee4355380e305ce
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats, kvark, lizzard
bugs1498639
milestone66.0
Bug 1498639 - Give WR the id of the scroll frame perspective scrolls relative to, and compute the right transform based on that. r=kats,kvark a=lizzard I think this is as clean as it can get. Differential Revision: https://phabricator.services.mozilla.com/D17848
gfx/layers/wr/StackingContextHelper.cpp
gfx/webrender_bindings/WebRenderAPI.h
gfx/webrender_bindings/src/bindings.rs
gfx/wr/webrender/src/clip_scroll_tree.rs
gfx/wr/webrender/src/spatial_node.rs
gfx/wr/webrender_api/src/display_item.rs
gfx/wr/wrench/src/yaml_frame_reader.rs
gfx/wr/wrench/src/yaml_frame_writer.rs
layout/painting/nsDisplayList.cpp
layout/reftests/async-scrolling/reftest.list
--- a/gfx/layers/wr/StackingContextHelper.cpp
+++ b/gfx/layers/wr/StackingContextHelper.cpp
@@ -31,17 +31,17 @@ StackingContextHelper::StackingContextHe
       mDeferredTransformItem(aParams.mDeferredTransformItem),
       mIsPreserve3D(aParams.transform_style == wr::TransformStyle::Preserve3D),
       mRasterizeLocally(aParams.mAnimated || aParentSC.mRasterizeLocally) {
   // Compute scale for fallback rendering. We don't try to guess a scale for 3d
   // transformed items
   gfx::Matrix transform2d;
   if (aParams.mBoundTransform &&
       aParams.mBoundTransform->CanDraw2D(&transform2d) &&
-      aParams.reference_frame_kind != wr::ReferenceFrameKind::Perspective &&
+      aParams.reference_frame_kind != wr::WrReferenceFrameKind::Perspective &&
       !aParentSC.mIsPreserve3D) {
     mInheritedTransform = transform2d * aParentSC.mInheritedTransform;
 
     int32_t apd = aContainerFrame->PresContext()->AppUnitsPerDevPixel();
     nsRect r = LayoutDevicePixel::ToAppUnits(aBounds, apd);
     mScale = FrameLayerBuilder::ChooseScale(aContainerFrame, aContainerItem, r,
                                             1.f, 1.f, mInheritedTransform,
                                             /* aCanDraw2D = */ true);
--- a/gfx/webrender_bindings/WebRenderAPI.h
+++ b/gfx/webrender_bindings/WebRenderAPI.h
@@ -312,17 +312,18 @@ class MOZ_RAII AutoTransactionSender {
  * A set of optional parameters for stacking context creation.
  */
 struct MOZ_STACK_CLASS StackingContextParams : public WrStackingContextParams {
   StackingContextParams()
       : WrStackingContextParams{WrStackingContextClip::None(),
                                 nullptr,
                                 nullptr,
                                 wr::TransformStyle::Flat,
-                                wr::ReferenceFrameKind::Transform,
+                                wr::WrReferenceFrameKind::Transform,
+                                nullptr,
                                 /* is_backface_visible = */ true,
                                 /* cache_tiles = */ false,
                                 wr::MixBlendMode::Normal} {}
 
   void SetPreserve3D(bool aPreserve) {
     transform_style =
         aPreserve ? wr::TransformStyle::Preserve3D : wr::TransformStyle::Flat;
   }
--- a/gfx/webrender_bindings/src/bindings.rs
+++ b/gfx/webrender_bindings/src/bindings.rs
@@ -1890,25 +1890,33 @@ pub extern "C" fn wr_dp_restore(state: &
     state.frame_builder.dl_builder.restore();
 }
 
 #[no_mangle]
 pub extern "C" fn wr_dp_clear_save(state: &mut WrState) {
     state.frame_builder.dl_builder.clear_save();
 }
 
+#[repr(u8)]
+#[derive(PartialEq, Eq, Debug)]
+pub enum WrReferenceFrameKind {
+    Transform,
+    Perspective,
+}
+
 /// IMPORTANT: If you add fields to this struct, you need to also add initializers
 /// for those fields in WebRenderAPI.h.
 #[repr(C)]
 pub struct WrStackingContextParams {
     pub clip: WrStackingContextClip,
     pub animation: *const WrAnimationProperty,
     pub opacity: *const f32,
     pub transform_style: TransformStyle,
-    pub reference_frame_kind: ReferenceFrameKind,
+    pub reference_frame_kind: WrReferenceFrameKind,
+    pub scrolling_relative_to: *const u64,
     pub is_backface_visible: bool,
     /// True if picture caching should be enabled for this stacking context.
     pub cache_tiles: bool,
     pub mix_blend_mode: MixBlendMode,
 }
 
 #[no_mangle]
 pub extern "C" fn wr_dp_push_stacking_context(
@@ -1969,22 +1977,36 @@ pub extern "C" fn wr_dp_push_stacking_co
     let wr_clip_id = params.clip.to_webrender(state.pipeline_id);
 
     // Note: 0 has special meaning in WR land, standing for ROOT_REFERENCE_FRAME.
     // However, it is never returned by `push_reference_frame`, and we need to return
     // an option here across FFI, so we take that 0 value for the None semantics.
     // This is resolved into proper `Maybe<WrSpatialId>` inside `WebRenderAPI::PushStackingContext`.
     let mut result = WrSpatialId { id: 0 };
     if let Some(transform_binding) = transform_binding {
+        let scrolling_relative_to = match unsafe { params.scrolling_relative_to.as_ref() } {
+            Some(scroll_id) => {
+                debug_assert_eq!(params.reference_frame_kind, WrReferenceFrameKind::Perspective);
+                Some(ExternalScrollId(*scroll_id, state.pipeline_id))
+            }
+            None => None,
+        };
+
+        let reference_frame_kind = match params.reference_frame_kind {
+            WrReferenceFrameKind::Transform => ReferenceFrameKind::Transform,
+            WrReferenceFrameKind::Perspective => ReferenceFrameKind::Perspective {
+                scrolling_relative_to,
+            },
+        };
         wr_spatial_id = state.frame_builder.dl_builder.push_reference_frame(
             &bounds,
             wr_spatial_id,
             params.transform_style,
             transform_binding,
-            params.reference_frame_kind,
+            reference_frame_kind,
         );
 
         bounds.origin = LayoutPoint::zero();
         result.id = wr_spatial_id.0;
         assert_ne!(wr_spatial_id.0, 0);
     }
 
     let prim_info = LayoutPrimitiveInfo {
--- a/gfx/wr/webrender/src/clip_scroll_tree.rs
+++ b/gfx/wr/webrender/src/clip_scroll_tree.rs
@@ -304,22 +304,23 @@ impl ClipScrollTree {
             current_coordinate_system_id: CoordinateSystemId::root(),
             coordinate_system_relative_scale_offset: ScaleOffset::identity(),
             invertible: true,
         };
         debug_assert!(self.nodes_to_update.is_empty());
         self.nodes_to_update.push((root_node_index, state));
 
         while let Some((node_index, mut state)) = self.nodes_to_update.pop() {
-            let node = match self.spatial_nodes.get_mut(node_index.0 as usize) {
+            let (previous, following) = self.spatial_nodes.split_at_mut(node_index.0 as usize);
+            let node = match following.get_mut(0) {
                 Some(node) => node,
                 None => continue,
             };
 
-            node.update(&mut state, &mut self.coord_systems, scene_properties);
+            node.update(&mut state, &mut self.coord_systems, scene_properties, &*previous);
             if let Some(ref mut palette) = transform_palette {
                 node.push_gpu_data(palette, node_index);
             }
 
             if !node.children.is_empty() {
                 node.prepare_state_for_children(&mut state);
                 self.nodes_to_update.extend(node.children
                     .iter()
@@ -521,17 +522,17 @@ fn add_reference_frame(
     parent: Option<SpatialNodeIndex>,
     transform: LayoutTransform,
     origin_in_parent_reference_frame: LayoutVector2D,
 ) -> SpatialNodeIndex {
     cst.add_reference_frame(
         parent,
         TransformStyle::Preserve3D,
         PropertyBinding::Value(transform),
-        ReferenceFrameKind::Perspective,
+        ReferenceFrameKind::Transform,
         origin_in_parent_reference_frame,
         PipelineId::dummy(),
     )
 }
 
 #[cfg(test)]
 fn test_pt(
     px: f32,
--- a/gfx/wr/webrender/src/spatial_node.rs
+++ b/gfx/wr/webrender/src/spatial_node.rs
@@ -64,16 +64,46 @@ pub struct SpatialNode {
     pub coordinate_system_id: CoordinateSystemId,
 
     /// The transformation from the coordinate system which established our compatible coordinate
     /// system (same coordinate system id) and us. This can change via scroll offsets and via new
     /// reference frame transforms.
     pub coordinate_system_relative_scale_offset: ScaleOffset,
 }
 
+fn compute_offset_from(
+    mut current: Option<SpatialNodeIndex>,
+    external_id: ExternalScrollId,
+    previous_spatial_nodes: &[SpatialNode],
+) -> LayoutVector2D {
+    let mut offset = LayoutVector2D::zero();
+    while let Some(parent_index) = current {
+        let ancestor = &previous_spatial_nodes[parent_index.0 as usize];
+        match ancestor.node_type {
+            SpatialNodeType::ReferenceFrame(..) => {
+                // FIXME(emilio, bug 1523436): Breaking here is technically
+                // wrong and can happen if the perspective frame is transformed
+                // as well.
+                break;
+            },
+            SpatialNodeType::ScrollFrame(ref info) => {
+                if info.external_id == Some(external_id) {
+                    break;
+                }
+                offset += info.offset;
+            },
+            SpatialNodeType::StickyFrame(ref info) => {
+                offset += info.current_offset;
+            },
+        }
+        current = ancestor.parent;
+    }
+    offset
+}
+
 impl SpatialNode {
     pub fn new(
         pipeline_id: PipelineId,
         parent_index: Option<SpatialNodeIndex>,
         node_type: SpatialNodeType,
     ) -> Self {
         SpatialNode {
             world_viewport_transform: LayoutToWorldFastTransform::identity(),
@@ -216,25 +246,26 @@ impl SpatialNode {
         }
     }
 
     pub fn update(
         &mut self,
         state: &mut TransformUpdateState,
         coord_systems: &mut Vec<CoordinateSystem>,
         scene_properties: &SceneProperties,
+        previous_spatial_nodes: &[SpatialNode],
     ) {
         // If any of our parents was not rendered, we are not rendered either and can just
         // quit here.
         if !state.invertible {
             self.mark_uninvertible(state);
             return;
         }
 
-        self.update_transform(state, coord_systems, scene_properties);
+        self.update_transform(state, coord_systems, scene_properties, previous_spatial_nodes);
         self.transform_kind = self.world_content_transform.kind();
 
         // If this node is a reference frame, we check if it has a non-invertible matrix.
         // For non-reference-frames we assume that they will produce only additional
         // translations which should be invertible.
         match self.node_type {
             SpatialNodeType::ReferenceFrame(info) if !info.invertible => {
                 self.mark_uninvertible(state);
@@ -244,34 +275,42 @@ impl SpatialNode {
         }
     }
 
     pub fn update_transform(
         &mut self,
         state: &mut TransformUpdateState,
         coord_systems: &mut Vec<CoordinateSystem>,
         scene_properties: &SceneProperties,
+        previous_spatial_nodes: &[SpatialNode],
     ) {
         match self.node_type {
             SpatialNodeType::ReferenceFrame(ref mut info) => {
                 // Resolve the transform against any property bindings.
                 let source_transform = LayoutFastTransform::from(
                     scene_properties.resolve_layout_transform(&info.source_transform)
                 );
 
                 // Do a change-basis operation on the perspective matrix using
                 // the scroll offset.
                 let source_transform = match info.kind {
-                    ReferenceFrameKind::Perspective => {
-                        // Do a change-basis operation on the perspective matrix
-                        // using the scroll offset.
+                    ReferenceFrameKind::Perspective { scrolling_relative_to: Some(external_id) } => {
+                        let scroll_offset = compute_offset_from(
+                            self.parent,
+                            external_id,
+                            previous_spatial_nodes,
+                        );
+
+                        // Do a change-basis operation on the
+                        // perspective matrix using the scroll offset.
                         source_transform
-                            .pre_translate(&state.parent_accumulated_scroll_offset)
-                            .post_translate(-state.parent_accumulated_scroll_offset)
+                            .pre_translate(&scroll_offset)
+                            .post_translate(-scroll_offset)
                     }
+                    ReferenceFrameKind::Perspective { scrolling_relative_to: None } |
                     ReferenceFrameKind::Transform => source_transform,
                 };
 
                 let resolved_transform =
                     LayoutFastTransform::with_vector(info.origin_in_parent_reference_frame)
                         .pre_mul(&source_transform);
 
                 // The transformation for this viewport in world coordinates is the transformation for
--- a/gfx/wr/webrender_api/src/display_item.rs
+++ b/gfx/wr/webrender_api/src/display_item.rs
@@ -521,20 +521,21 @@ pub struct ReferenceFrameDisplayListItem
 
 /// Provides a hint to WR that it should try to cache the items
 /// within a cache marker context in an off-screen surface.
 #[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)]
 pub struct CacheMarkerDisplayItem {
 }
 
 #[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)]
-#[repr(u8)]
 pub enum ReferenceFrameKind {
     Transform,
-    Perspective,
+    Perspective {
+        scrolling_relative_to: Option<ExternalScrollId>,
+    }
 }
 
 #[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)]
 pub struct ReferenceFrame {
     pub kind: ReferenceFrameKind,
     pub transform_style: TransformStyle,
     /// The transform matrix, either the perspective matrix or the transform
     /// matrix.
@@ -944,16 +945,17 @@ impl SpatialId {
 /// An external identifier that uniquely identifies a scroll frame independent of its ClipId, which
 /// may change from frame to frame. This should be unique within a pipeline. WebRender makes no
 /// attempt to ensure uniqueness. The zero value is reserved for use by the root scroll node of
 /// every pipeline, which always has an external id.
 ///
 /// When setting display lists with the `preserve_frame_state` this id is used to preserve scroll
 /// offsets between different sets of ClipScrollNodes which are ScrollFrames.
 #[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)]
+#[repr(C)]
 pub struct ExternalScrollId(pub u64, pub PipelineId);
 
 impl ExternalScrollId {
     pub fn pipeline_id(&self) -> PipelineId {
         self.1
     }
 
     pub fn is_root(&self) -> bool {
--- a/gfx/wr/wrench/src/yaml_frame_reader.rs
+++ b/gfx/wr/wrench/src/yaml_frame_reader.rs
@@ -1670,17 +1670,17 @@ impl YamlFrameReader {
 
         assert!(
             yaml["transform"].is_badvalue() ||
             yaml["perspective"].is_badvalue(),
             "Should have one of either transform or perspective"
         );
 
         let reference_frame_kind = if !yaml["perspective"].is_badvalue() {
-            ReferenceFrameKind::Perspective
+            ReferenceFrameKind::Perspective { scrolling_relative_to: None }
         } else {
             ReferenceFrameKind::Transform
         };
 
         let transform = yaml["transform"]
             .as_transform(&transform_origin);
 
         let perspective = match yaml["perspective"].as_f32() {
--- a/gfx/wr/wrench/src/yaml_frame_writer.rs
+++ b/gfx/wr/wrench/src/yaml_frame_writer.rs
@@ -191,21 +191,25 @@ fn maybe_radius_yaml(radius: &BorderRadi
 }
 
 fn write_reference_frame(
     parent: &mut Table,
     reference_frame: &ReferenceFrame,
     properties: &SceneProperties,
     clip_id_mapper: &mut ClipIdMapper,
 ) {
+    // FIXME: This ignores the scrolling_relative_to member in
+    // ReferenceFrameKind::Perspective, but it's a bit annoying to fix since the
+    // frame reader abuses `ExternalScrollId`s.
+
     matrix4d_node(
         parent,
         match reference_frame.kind {
             ReferenceFrameKind::Transform => "transform",
-            ReferenceFrameKind::Perspective => "perspective",
+            ReferenceFrameKind::Perspective { .. } => "perspective",
         },
         &properties.resolve_layout_transform(&reference_frame.transform)
     );
 
     usize_node(parent, "id", clip_id_mapper.add_spatial_id(reference_frame.id));
 }
 
 fn write_stacking_context(
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -8508,20 +8508,37 @@ bool nsDisplayPerspective::CreateWebRend
   // other way via the transform-style property on either the transformed frame
   // or the perspective frame in order to not confuse WR's preserve-3d code in
   // very awful ways.
   bool preserve3D =
       mFrame->Extend3DContext() || perspectiveFrame->Extend3DContext();
 
   wr::StackingContextParams params;
   params.mTransformPtr = &perspectiveMatrix;
-  params.reference_frame_kind = wr::ReferenceFrameKind::Perspective;
+  params.reference_frame_kind = wr::WrReferenceFrameKind::Perspective;
   params.is_backface_visible = !BackfaceIsHidden();
   params.SetPreserve3D(preserve3D);
 
+  Maybe<uint64_t> scrollingRelativeTo;
+  for (auto* asr = GetActiveScrolledRoot(); asr; asr = asr->mParent) {
+    if (nsLayoutUtils::IsAncestorFrameCrossDoc(
+          asr->mScrollableFrame->GetScrolledFrame(), perspectiveFrame)) {
+      scrollingRelativeTo.emplace(asr->GetViewId());
+      break;
+    }
+  }
+
+  // We put the perspective reference frame wrapping the transformed frame,
+  // even though there may be arbitrarily nested scroll frames in between.
+  //
+  // We need to know how many ancestor scroll-frames are we nested in, in order
+  // for the async scrolling code in WebRender to calculate the right
+  // transformation for the perspective contents.
+  params.scrolling_relative_to = scrollingRelativeTo.ptrOr(nullptr);
+
   StackingContextHelper sc(aSc, GetActiveScrolledRoot(), mFrame, this, aBuilder,
                            params);
 
   return mList.CreateWebRenderCommands(aBuilder, aResources, sc, aManager,
                                        aDisplayListBuilder);
 }
 
 nsDisplayItemGeometry* nsCharClipDisplayItem::AllocateGeometry(
--- a/layout/reftests/async-scrolling/reftest.list
+++ b/layout/reftests/async-scrolling/reftest.list
@@ -52,17 +52,17 @@ skip-if(!asyncPan) == offscreen-prerende
 fuzzy-if(Android,0-6,0-4) fuzzy-if(skiaContent&&!Android,0-1,0-34) skip-if(!asyncPan) == offscreen-clipped-blendmode-1.html offscreen-clipped-blendmode-ref.html
 fuzzy-if(Android,0-6,0-4) skip-if(!asyncPan) == offscreen-clipped-blendmode-2.html offscreen-clipped-blendmode-ref.html
 fuzzy-if(Android,0-6,0-4) skip == offscreen-clipped-blendmode-3.html offscreen-clipped-blendmode-ref.html # bug 1251588 - wrong AGR on mix-blend-mode item
 fuzzy-if(Android,0-6,0-4) skip-if(!asyncPan) == offscreen-clipped-blendmode-4.html offscreen-clipped-blendmode-ref.html
 fuzzy-if(Android,0-7,0-4) skip-if(!asyncPan) == perspective-scrolling-1.html perspective-scrolling-1-ref.html
 fuzzy-if(Android,0-7,0-4) skip-if(!asyncPan) == perspective-scrolling-2.html perspective-scrolling-2-ref.html
 fuzzy-if(Android,0-7,0-4) skip-if(!asyncPan) == perspective-scrolling-3.html perspective-scrolling-3-ref.html
 fuzzy-if(Android,0-7,0-4) skip-if(!asyncPan) == perspective-scrolling-4.html perspective-scrolling-4-ref.html
-random-if(webrender) skip-if(!asyncPan) == perspective-scrolling-5.html perspective-scrolling-5-ref.html # bug 1498639
+skip-if(!asyncPan) == perspective-scrolling-5.html perspective-scrolling-5-ref.html
 pref(apz.disable_for_scroll_linked_effects,true) skip-if(!asyncPan) == disable-apz-for-sle-pages.html disable-apz-for-sle-pages-ref.html
 fuzzy-if(browserIsRemote&&d2d,0-1,0-22) skip-if(!asyncPan) == background-blend-mode-1.html background-blend-mode-1-ref.html
 skip-if(Android||!asyncPan) != opaque-fractional-displayport-1.html about:blank
 skip-if(Android||!asyncPan) != opaque-fractional-displayport-2.html about:blank
 fuzzy-if(Android,0-6,0-4) skip-if(!asyncPan) == fixed-pos-scrolled-clip-1.html fixed-pos-scrolled-clip-1-ref.html
 fuzzy-if(Android,0-6,0-8) skip-if(!asyncPan) == fixed-pos-scrolled-clip-2.html fixed-pos-scrolled-clip-2-ref.html
 fuzzy-if(Android,0-6,0-8) skip-if(!asyncPan) == fixed-pos-scrolled-clip-3.html fixed-pos-scrolled-clip-3-ref.html
 fuzzy-if(Android,0-6,0-8) skip-if(!asyncPan) == fixed-pos-scrolled-clip-4.html fixed-pos-scrolled-clip-4-ref.html