libservo|compositor: Have scroll offset directionality match that of WebRender and the web (#37752)

Previously, our Servo-specific spatial tree scroll offsets were opposite
to
that of WebRender and also the web platform. This is due to the fact,
likely, that `winit` wheel directionality is also flipped. This change
has both the Servo spatial tree and the API take offsets that are
consistent with the web.

Any possible changes to the meaning of wheel directionality will be
handled in a followup change.

This is a breaking change to the Servo API.

Testing: This change updates unit tests.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
Co-authored-by: Oriol Brufau <obrufau@igalia.com>

components/compositing/compositor.rs

@@ -44,7 +44,7 @@ use style_traits::CSSPixel;
 use webrender::{CaptureBits, RenderApi, Transaction};
 use webrender_api::units::{
     DeviceIntPoint, DeviceIntRect, DevicePixel, DevicePoint, DeviceRect, LayoutPoint, LayoutRect,
-    LayoutSize, LayoutVector2D, WorldPoint,
+    LayoutSize, WorldPoint,
 };
 use webrender_api::{
     self, BuiltDisplayList, DirtyRect, DisplayListPayload, DocumentId, Epoch as WebRenderEpoch,
@@ -714,11 +714,9 @@ impl IOCompositor {
                 // is inverted compared to `winit`s wheel delta. Hence,
                 // here we invert the sign to mimic wheel scroll
                 // implementation in `headed_window.rs`.
-                let dx = -dx;
-                let dy = -dy;
                 let delta = WheelDelta {
-                    x: dx,
-                    y: dy,
+                    x: -dx,
+                    y: -dy,
                     z: 0.0,
                     mode: WheelMode::DeltaPixel,
                 };
@@ -768,7 +766,7 @@ impl IOCompositor {
                 txn.set_scroll_offsets(
                     external_scroll_id,
                     vec![SampledScrollOffset {
-                        offset: -offset,
+                        offset,
                         generation: 0,
                     }],
                 );
@@ -1169,7 +1167,6 @@ impl IOCompositor {
                         continue;
                     };
 
-                    let offset = LayoutVector2D::new(-offset.x, -offset.y);
                     transaction.set_scroll_offsets(
                         external_id,
                         vec![SampledScrollOffset {
@@ -1731,11 +1728,10 @@ impl IOCompositor {
                 self.send_root_pipeline_display_list_in_transaction(&mut transaction);
             }
             for update in scroll_offset_updates {
-                let offset = LayoutVector2D::new(-update.offset.x, -update.offset.y);
                 transaction.set_scroll_offsets(
                     update.external_scroll_id,
                     vec![SampledScrollOffset {
-                        offset,
+                        offset: update.offset,
                         generation: 0,
                     }],
                 );

components/compositing/touch.rs

@@ -406,7 +406,9 @@ impl TouchHandler {
                     *velocity /= 2.0;
                     // update the touch point every time when panning.
                     touch_sequence.active_touch_points[idx].point = point;
-                    TouchMoveAction::Scroll(delta, point)
+
+                    // Scroll offsets are opposite to the direction of finger motion.
+                    TouchMoveAction::Scroll(-delta, point)
                 } else if delta.x.abs() > TOUCH_PAN_MIN_SCREEN_PX ||
                     delta.y.abs() > TOUCH_PAN_MIN_SCREEN_PX
                 {
@@ -417,7 +419,9 @@ impl TouchHandler {
                     touch_sequence.prevent_click = true;
                     // update the touch point
                     touch_sequence.active_touch_points[idx].point = point;
-                    TouchMoveAction::Scroll(delta, point)
+
+                    // Scroll offsets are opposite to the direction of finger motion.
+                    TouchMoveAction::Scroll(-delta, point)
                 } else {
                     // We don't update the touchpoint, so multiple small moves can
                     // accumulate and merge into a larger move.
@@ -435,8 +439,11 @@ impl TouchHandler {
                     touch_sequence.active_touch_points[idx].point = point;
                     let (d1, c1) = touch_sequence.pinch_distance_and_center();
                     let magnification = d1 / d0;
+
                     let scroll_delta = c1 - c0 * Scale::new(magnification);
-                    TouchMoveAction::Zoom(magnification, scroll_delta)
+
+                    // Scroll offsets are opposite to the direction of finger motion.
+                    TouchMoveAction::Zoom(magnification, -scroll_delta)
                 } else {
                     // We don't update the touchpoint, so multiple small moves can
                     // accumulate and merge into a larger move.

components/compositing/webview_renderer.rs

@@ -339,7 +339,7 @@ impl WebViewRenderer {
     pub(crate) fn on_vsync(&mut self) {
         if let Some(fling_action) = self.touch_handler.on_vsync() {
             self.on_scroll_window_event(
-                ScrollLocation::Delta(fling_action.delta),
+                ScrollLocation::Delta(-fling_action.delta),
                 fling_action.cursor,
             );
         }

components/layout/layout_impl.rs

@@ -476,7 +476,6 @@ impl Layout for LayoutThread {
             .borrow_mut()
             .as_mut()
             .and_then(|tree| tree.compositor_info.scroll_tree.scroll_offset(id))
-            .map(|scroll_offset| -scroll_offset)
     }
 }
 
@@ -1083,7 +1082,7 @@ impl LayoutThread {
             .scroll_tree
             .set_scroll_offset_for_node_with_external_scroll_id(
                 external_scroll_id,
-                -offset,
+                offset,
                 ScrollType::Script,
             )
         {

components/servo/webview.rs

@@ -434,6 +434,8 @@ impl WebView {
             ))
     }
 
+    /// Ask the [`WebView`] to scroll web content. Note that positive scroll offsets reveal more
+    /// content on the bottom and right of the page.
     pub fn notify_scroll_event(&self, location: ScrollLocation, point: DeviceIntPoint) {
         self.inner()
             .compositor

components/shared/compositing/display_list.rs

@@ -130,11 +130,11 @@ impl ScrollableNodeInfo {
         let original_layer_scroll_offset = self.offset;
 
         if scrollable_size.width > 0. && self.scroll_sensitivity.x.contains(context) {
-            self.offset.x = new_offset.x.min(0.0).max(-scrollable_size.width);
+            self.offset.x = new_offset.x.clamp(0.0, scrollable_size.width);
         }
 
         if scrollable_size.height > 0. && self.scroll_sensitivity.y.contains(context) {
-            self.offset.y = new_offset.y.min(0.0).max(-scrollable_size.height);
+            self.offset.y = new_offset.y.clamp(0.0, scrollable_size.height);
         }
 
         if self.offset != original_layer_scroll_offset {
@@ -158,7 +158,7 @@ impl ScrollableNodeInfo {
         let delta = match scroll_location {
             ScrollLocation::Delta(delta) => delta,
             ScrollLocation::Start => {
-                if self.offset.y.round() >= 0.0 {
+                if self.offset.y.round() <= 0.0 {
                     // Nothing to do on this layer.
                     return None;
                 }
@@ -167,8 +167,8 @@ impl ScrollableNodeInfo {
                 return Some(self.offset);
             },
             ScrollLocation::End => {
-                let end_pos = -self.scrollable_size().height;
-                if self.offset.y.round() <= end_pos {
+                let end_pos = self.scrollable_size().height;
+                if self.offset.y.round() >= end_pos {
                     // Nothing to do on this layer.
                     return None;
                 }
@@ -231,20 +231,9 @@ impl ScrollTreeNode {
 
     /// Set the offset for this node, returns false if this was a
     /// non-scrolling node for which you cannot set the offset.
-    pub fn set_offset(&mut self, new_offset: LayoutVector2D) -> bool {
-        match self.info {
-            SpatialTreeNodeInfo::Scroll(ref mut info) => {
-                let scrollable_size = info.scrollable_size();
-                if scrollable_size.width > 0. {
-                    info.offset.x = (new_offset.x).min(0.0).max(-scrollable_size.width);
-                }
-
-                if scrollable_size.height > 0. {
-                    info.offset.y = (new_offset.y).min(0.0).max(-scrollable_size.height);
-                }
-                true
-            },
-            _ => false,
+    pub fn set_offset(&mut self, new_offset: LayoutVector2D) {
+        if let SpatialTreeNodeInfo::Scroll(ref mut info) = self.info {
+            info.scroll_to_offset(new_offset, ScrollType::Script);
         }
     }
 

components/shared/compositing/tests/compositor.rs

@@ -46,11 +46,11 @@ fn test_scroll_tree_simple_scroll() {
     let (scrolled_id, offset) = scroll_tree
         .scroll_node_or_ancestor(
             &id,
-            ScrollLocation::Delta(LayoutVector2D::new(-20.0, -40.0)),
+            ScrollLocation::Delta(LayoutVector2D::new(20.0, 40.0)),
             ScrollType::Script,
         )
         .unwrap();
-    let expected_offset = LayoutVector2D::new(-20.0, -40.0);
+    let expected_offset = LayoutVector2D::new(20.0, 40.0);
     assert_eq!(scrolled_id, ExternalScrollId(0, pipeline_id));
     assert_eq!(offset, expected_offset);
     assert_eq!(scroll_tree.get_node(&id).offset(), Some(expected_offset));
@@ -58,7 +58,7 @@ fn test_scroll_tree_simple_scroll() {
     let (scrolled_id, offset) = scroll_tree
         .scroll_node_or_ancestor(
             &id,
-            ScrollLocation::Delta(LayoutVector2D::new(20.0, 40.0)),
+            ScrollLocation::Delta(LayoutVector2D::new(-20.0, -40.0)),
             ScrollType::Script,
         )
         .unwrap();
@@ -67,10 +67,10 @@ fn test_scroll_tree_simple_scroll() {
     assert_eq!(offset, expected_offset);
     assert_eq!(scroll_tree.get_node(&id).offset(), Some(expected_offset));
 
-    // Scroll offsets must be negative.
+    // Scroll offsets must be positive.
     let result = scroll_tree.scroll_node_or_ancestor(
         &id,
-        ScrollLocation::Delta(LayoutVector2D::new(20.0, 40.0)),
+        ScrollLocation::Delta(LayoutVector2D::new(-20.0, -40.0)),
         ScrollType::Script,
     );
     assert!(result.is_none());
@@ -99,11 +99,11 @@ fn test_scroll_tree_simple_scroll_chaining() {
     let (scrolled_id, offset) = scroll_tree
         .scroll_node_or_ancestor(
             &unscrollable_child_id,
-            ScrollLocation::Delta(LayoutVector2D::new(-20.0, -40.0)),
+            ScrollLocation::Delta(LayoutVector2D::new(20.0, 40.0)),
             ScrollType::Script,
         )
         .unwrap();
-    let expected_offset = LayoutVector2D::new(-20.0, -40.0);
+    let expected_offset = LayoutVector2D::new(20.0, 40.0);
     assert_eq!(scrolled_id, ExternalScrollId(0, pipeline_id));
     assert_eq!(offset, expected_offset);
     assert_eq!(
@@ -114,11 +114,11 @@ fn test_scroll_tree_simple_scroll_chaining() {
     let (scrolled_id, offset) = scroll_tree
         .scroll_node_or_ancestor(
             &unscrollable_child_id,
-            ScrollLocation::Delta(LayoutVector2D::new(-10.0, -15.0)),
+            ScrollLocation::Delta(LayoutVector2D::new(10.0, 15.0)),
             ScrollType::Script,
         )
         .unwrap();
-    let expected_offset = LayoutVector2D::new(-30.0, -55.0);
+    let expected_offset = LayoutVector2D::new(30.0, 55.0);
     assert_eq!(scrolled_id, ExternalScrollId(0, pipeline_id));
     assert_eq!(offset, expected_offset);
     assert_eq!(
@@ -140,7 +140,7 @@ fn test_scroll_tree_chain_when_at_extent() {
         .scroll_node_or_ancestor(&child_id, ScrollLocation::End, ScrollType::Script)
         .unwrap();
 
-    let expected_offset = LayoutVector2D::new(0.0, -100.0);
+    let expected_offset = LayoutVector2D::new(0.0, 100.0);
     assert_eq!(scrolled_id, ExternalScrollId(1, pipeline_id));
     assert_eq!(offset, expected_offset);
     assert_eq!(
@@ -153,11 +153,11 @@ fn test_scroll_tree_chain_when_at_extent() {
     let (scrolled_id, offset) = scroll_tree
         .scroll_node_or_ancestor(
             &child_id,
-            ScrollLocation::Delta(LayoutVector2D::new(0.0, -10.0)),
+            ScrollLocation::Delta(LayoutVector2D::new(0.0, 10.0)),
             ScrollType::Script,
         )
         .unwrap();
-    let expected_offset = LayoutVector2D::new(0.0, -10.0);
+    let expected_offset = LayoutVector2D::new(0.0, 10.0);
     assert_eq!(scrolled_id, ExternalScrollId(0, pipeline_id));
     assert_eq!(offset, expected_offset);
     assert_eq!(
@@ -187,11 +187,11 @@ fn test_scroll_tree_chain_through_overflow_hidden() {
     let (scrolled_id, offset) = scroll_tree
         .scroll_node_or_ancestor(
             &overflow_hidden_id,
-            ScrollLocation::Delta(LayoutVector2D::new(-20.0, -40.0)),
+            ScrollLocation::Delta(LayoutVector2D::new(20.0, 40.0)),
             ScrollType::InputEvents,
         )
         .unwrap();
-    let expected_offset = LayoutVector2D::new(-20.0, -40.0);
+    let expected_offset = LayoutVector2D::new(20.0, 40.0);
     assert_eq!(scrolled_id, ExternalScrollId(0, pipeline_id));
     assert_eq!(offset, expected_offset);
     assert_eq!(