Bug 1546286 - Fix external scroll offset handling for perspective elements. r=emilio
authorGlenn Watson <github@intuitionlibrary.com>
Tue, 23 Apr 2019 09:25:39 +0000
changeset 470462 cf05b09d3f3a726f554d2600f3b9b5e5f629972b
parent 470461 80a5deca74380b8c01ae7194aba33ff1e8e3df82
child 470463 696aa652a9e1d48b993802b0c82dbb3f817a57f1
push id35906
push useraciure@mozilla.com
push dateTue, 23 Apr 2019 22:14:56 +0000
treeherdermozilla-central@0ce3633f8b80 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1546286
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1546286 - Fix external scroll offset handling for perspective elements. r=emilio External scroll offsets are not propagated across reference frames. When a perspective element scrolls relative to an ancestor scroll frame, remove the effect of any external scroll offset during that offset calculation. Differential Revision: https://phabricator.services.mozilla.com/D28443
gfx/wr/webrender/src/spatial_node.rs
--- a/gfx/wr/webrender/src/spatial_node.rs
+++ b/gfx/wr/webrender/src/spatial_node.rs
@@ -81,17 +81,20 @@ fn compute_offset_from(
             SpatialNodeType::ReferenceFrame(..) => {
                 // We don't want to scroll across reference frames.
                 break;
             },
             SpatialNodeType::ScrollFrame(ref info) => {
                 if info.external_id == Some(external_id) {
                     break;
                 }
-                offset += info.offset;
+
+                // External scroll offsets are not propagated across
+                // reference frame boundaries, so undo them here.
+                offset += info.offset + info.external_scroll_offset;
             },
             SpatialNodeType::StickyFrame(ref info) => {
                 offset += info.current_offset;
             },
         }
         current = ancestor.parent;
     }
     offset
@@ -765,8 +768,81 @@ impl StickyFrameInfo {
             margins,
             vertical_offset_bounds,
             horizontal_offset_bounds,
             previously_applied_offset,
             current_offset: LayoutVector2D::zero(),
         }
     }
 }
+
+#[test]
+fn test_cst_perspective_relative_scroll() {
+    // Verify that when computing the offset from a perspective transform
+    // to a relative scroll node that any external scroll offset is
+    // ignored. This is because external scroll offsets are not
+    // propagated across reference frame boundaries.
+
+    // It's not currently possible to verify this with a wrench reftest,
+    // since wrench doesn't understand external scroll ids. When wrench
+    // supports this, we could also verify with a reftest.
+
+    use clip_scroll_tree::ClipScrollTree;
+    use euclid::approxeq::ApproxEq;
+
+    let mut cst = ClipScrollTree::new();
+    let pipeline_id = PipelineId::dummy();
+    let ext_scroll_id = ExternalScrollId(1, pipeline_id);
+    let transform = LayoutTransform::create_perspective(100.0);
+
+    let root = cst.add_reference_frame(
+        None,
+        TransformStyle::Flat,
+        PropertyBinding::Value(LayoutTransform::identity()),
+        ReferenceFrameKind::Transform,
+        LayoutVector2D::zero(),
+        pipeline_id,
+    );
+
+    let scroll_frame_1 = cst.add_scroll_frame(
+        root,
+        Some(ext_scroll_id),
+        pipeline_id,
+        &LayoutRect::new(LayoutPoint::zero(), LayoutSize::new(100.0, 100.0)),
+        &LayoutSize::new(100.0, 500.0),
+        ScrollSensitivity::Script,
+        ScrollFrameKind::Explicit,
+        LayoutVector2D::zero(),
+    );
+
+    let scroll_frame_2 = cst.add_scroll_frame(
+        scroll_frame_1,
+        None,
+        pipeline_id,
+        &LayoutRect::new(LayoutPoint::zero(), LayoutSize::new(100.0, 100.0)),
+        &LayoutSize::new(100.0, 500.0),
+        ScrollSensitivity::Script,
+        ScrollFrameKind::Explicit,
+        LayoutVector2D::new(0.0, 50.0),
+    );
+
+    let ref_frame = cst.add_reference_frame(
+        Some(scroll_frame_2),
+        TransformStyle::Preserve3D,
+        PropertyBinding::Value(transform),
+        ReferenceFrameKind::Perspective {
+            scrolling_relative_to: Some(ext_scroll_id),
+        },
+        LayoutVector2D::zero(),
+        pipeline_id,
+    );
+
+    cst.update_tree(WorldPoint::zero(), &SceneProperties::new(), None);
+
+    let scroll_offset = compute_offset_from(
+        cst.spatial_nodes[ref_frame.0 as usize].parent,
+        ext_scroll_id,
+        &cst.spatial_nodes,
+    );
+
+    assert!(scroll_offset.x.approx_eq(&0.0));
+    assert!(scroll_offset.y.approx_eq(&0.0));
+}