Fix ListView scrolling with non-uniformly sized items

internal/backends/qt/qt_window.rs

@@ -230,7 +230,9 @@ cpp! {{
             rust!(Slint_mouseWheelEvent [rust_window: &QtWindow as "void*", pos: qttypes::QPointF as "QPointF", delta: qttypes::QPoint as "QPoint", phase: usize as "int"] {
                 let position = LogicalPoint::new(pos.x as _, pos.y as _);
                 let phase = match phase as _ {
-                    key_generated::Qt_ScrollPhase_NoScrollPhase => TouchPhase::Cancelled,
+                    // If we don't know the scroll phase, this is likely a mouse wheel scroll, which
+                    // should be mapped to TouchPhase::Moved to align with the winit backend.
+                    key_generated::Qt_ScrollPhase_NoScrollPhase => TouchPhase::Moved,
                     key_generated::Qt_ScrollPhase_ScrollBegin => TouchPhase::Started,
                     key_generated::Qt_ScrollPhase_ScrollUpdate => TouchPhase::Moved,
                     key_generated::Qt_ScrollPhase_ScrollEnd => TouchPhase::Ended,

internal/core/items/flickable/data_ringbuffer.rs

@@ -1,40 +1,39 @@
 // Copyright © SixtyFPS GmbH <info@slint.dev>
 // SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-2.0 OR LicenseRef-Slint-Software-3.0
 
-//! This module contains a simple ringbuffer to store time and location tuples. It is used in the flickable to
-//! determine the initial velocity of the animation
+//! This module contains a simple ringbuffer to store time and delta tuples.
+//! It is used in the flickable to determine the initial velocity of the animation.
 
 use crate::Coord;
 use crate::animations::Instant;
-use crate::lengths::LogicalPoint;
-use crate::lengths::LogicalPx;
+use crate::lengths::{LogicalPx, LogicalVector};
 use core::time::Duration;
 use euclid::Vector2D;
 
-/// Simple ringbuffer storing time and position tuples
+/// Simple ringbuffer storing time and delta tuples
 #[derive(Debug)]
-pub(crate) struct PositionTimeRingBuffer<const N: usize> {
+pub(crate) struct VelocityRingBuffer<const N: usize> {
     /// Pointing to the next free element
     curr_index: usize,
     /// Indicates if the buffer is full
     full: bool,
-    values: [(Instant, LogicalPoint); N],
+    values: [(Instant, Vector2D<Coord, LogicalPx>); N],
 }
 
-impl<const N: usize> Default for PositionTimeRingBuffer<N> {
+impl<const N: usize> Default for VelocityRingBuffer<N> {
     fn default() -> Self {
-        Self { curr_index: 0, full: false, values: [(Instant::now(), LogicalPoint::default()); N] }
+        Self { curr_index: 0, full: false, values: [(Instant::now(), Vector2D::default()); N] }
     }
 }
 
-impl<const N: usize> PositionTimeRingBuffer<N> {
+impl<const N: usize> VelocityRingBuffer<N> {
     /// Indicates if the buffer is empty
     pub fn empty(&self) -> bool {
         !(self.full || self.curr_index > 0)
     }
 
     /// Add a new element to the ringbuffer
-    pub fn push(&mut self, time: Instant, value: LogicalPoint) {
+    pub fn push(&mut self, time: Instant, value: LogicalVector) {
         if self.curr_index < self.values.len() {
             self.values[self.curr_index] = (time, value);
         }
@@ -50,143 +49,150 @@ impl<const N: usize> PositionTimeRingBuffer<N> {
         if self.curr_index > 0 { self.curr_index - 1 } else { N - 1 }
     }
 
+    fn len(&self) -> usize {
+        if self.full { N } else { self.curr_index }
+    }
+
     /// Returns the last time value added to the buffer if not empty otherwise None
     pub fn last_time(&self) -> Option<Instant> {
         if !self.empty() { Some(self.values[self.latest_index()].0) } else { None }
     }
 
-    /// Returns the difference between the oldest and the newest point
-    pub fn diff(&self) -> (Duration, Vector2D<Coord, LogicalPx>) {
-        if self.full {
-            let oldest = self.values[self.curr_index];
-            let newest = if self.curr_index > 0 {
-                self.values[self.curr_index - 1]
-            } else {
-                self.values[self.values.len() - 1]
-            };
-            (newest.0.duration_since(oldest.0), newest.1 - oldest.1)
-        } else {
-            let oldest = self.values[0];
-            let newest = self.values[usize::max(0, self.curr_index - 1)];
-            (newest.0.duration_since(oldest.0), newest.1 - oldest.1)
+    pub fn mean_velocity(&self) -> LogicalVector {
+        let len = self.len();
+        if len < 2 {
+            return Default::default();
+        }
+
+        let oldest_index = if self.full { self.curr_index } else { 0 };
+        let newest_index = self.latest_index();
+        let duration = self.values[newest_index].0.duration_since(self.values[oldest_index].0);
+        if duration == Duration::ZERO {
+            return Default::default();
         }
+
+        // The oldest recorded delta happened before the oldest timestamp in the covered time span,
+        // so it does not belong to the average velocity between oldest and newest.
+        let mut total_delta = LogicalVector::default();
+        let mut index = (oldest_index + 1) % N;
+        for _ in 1..len {
+            total_delta += self.values[index].1;
+            index = (index + 1) % N;
+        }
+
+        total_delta / duration.as_secs_f32()
     }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
     use crate::animations::Instant;
-    use crate::lengths::LogicalPoint;
     use core::time::Duration;
 
     #[test]
     fn test_empty_buffer() {
-        let buffer: PositionTimeRingBuffer<5> = PositionTimeRingBuffer::default();
+        let buffer: VelocityRingBuffer<5> = VelocityRingBuffer::default();
         assert!(buffer.empty());
         assert_eq!(buffer.curr_index, 0);
         assert!(!buffer.full);
         assert_eq!(buffer.last_time(), None);
+        assert_eq!(buffer.mean_velocity(), Vector2D::default());
     }
 
     #[test]
     fn test_push_single_element() {
-        let mut buffer: PositionTimeRingBuffer<5> = PositionTimeRingBuffer::default();
+        let mut buffer: VelocityRingBuffer<5> = VelocityRingBuffer::default();
         let time = Instant::now();
-        let point = LogicalPoint::new(10.0, 20.0);
+        let delta = Vector2D::new(10.0, 20.0);
 
-        buffer.push(time, point);
+        buffer.push(time, delta);
 
         assert!(!buffer.empty());
         assert_eq!(buffer.curr_index, 1);
         assert!(!buffer.full);
         assert_eq!(buffer.latest_index(), 0);
         assert_eq!(buffer.last_time(), Some(time));
-
-        assert_eq!(buffer.diff(), (Duration::from_millis(0), Vector2D::new(0., 0.)));
+        assert_eq!(buffer.mean_velocity(), Vector2D::default());
     }
 
     /// Buffer not complete full
     #[test]
     fn test_push_two_elements() {
-        let mut buffer: PositionTimeRingBuffer<5> = PositionTimeRingBuffer::default();
+        let mut buffer: VelocityRingBuffer<5> = VelocityRingBuffer::default();
         let time = Instant::now();
 
-        buffer.push(time, LogicalPoint::new(10.0, 20.0));
-        buffer.push(time + Duration::from_millis(13), LogicalPoint::new(13.0, -5.0));
+        buffer.push(time, Vector2D::new(10.0, 20.0));
+        buffer.push(time + Duration::from_millis(100), Vector2D::new(13.0, -5.0));
 
         assert!(!buffer.empty());
         assert_eq!(buffer.curr_index, 2);
         assert!(!buffer.full);
         assert_eq!(buffer.latest_index(), 1);
-        assert_eq!(buffer.last_time(), Some(time + Duration::from_millis(13)));
+        assert_eq!(buffer.last_time(), Some(time + Duration::from_millis(100)));
 
-        assert_eq!(buffer.diff(), (Duration::from_millis(13), Vector2D::new(3., -25.)));
+        assert_eq!(buffer.mean_velocity(), Vector2D::new(130.0, -50.0));
     }
 
     #[test]
     fn test_push_until_full() {
-        let mut buffer: PositionTimeRingBuffer<5> = PositionTimeRingBuffer::default();
+        let mut buffer: VelocityRingBuffer<5> = VelocityRingBuffer::default();
         let base_time = Instant::now();
 
         // Push elements to fill the buffer
         for i in 0..5 {
-            let time = base_time + Duration::from_millis(i * 3 as u64);
-            let point = LogicalPoint::new(i as f32, -2. * i as f32);
-            buffer.push(time, point);
+            let time = base_time + Duration::from_millis(i * 100);
+            buffer.push(time, Vector2D::new(1.0, -2.0));
         }
 
         assert!(!buffer.empty());
         assert_eq!(buffer.curr_index, 0);
         assert!(buffer.full);
-        assert_eq!(buffer.last_time(), Some(base_time + Duration::from_millis(4 * 3)));
+        assert_eq!(buffer.last_time(), Some(base_time + Duration::from_millis(400)));
         assert_eq!(buffer.latest_index(), 4);
 
-        assert_eq!(buffer.diff(), (Duration::from_millis(12), Vector2D::new(4., -8.)));
+        assert_eq!(buffer.mean_velocity(), Vector2D::new(10.0, -20.0));
     }
 
     #[test]
     fn test_push_beyond_capacity() {
         const CAP: usize = 5;
-        let mut buffer: PositionTimeRingBuffer<CAP> = PositionTimeRingBuffer::default();
+        let mut buffer: VelocityRingBuffer<CAP> = VelocityRingBuffer::default();
         let base_time = Instant::now();
 
         // Push more than capacity
         for i in 0..(CAP + 2) {
-            let time = base_time + Duration::from_millis(i as u64);
-            let point = LogicalPoint::new(i as f32, i as f32 * 2. + 100.);
-            buffer.push(time, point);
+            let time = base_time + Duration::from_millis(i as u64 * 100);
+            buffer.push(time, Vector2D::new(1.0, 2.0));
         }
 
         assert!(!buffer.empty());
         assert!(buffer.full);
         assert_eq!(buffer.curr_index, 2);
         assert_eq!(buffer.latest_index(), 1);
-        assert_eq!(buffer.last_time(), Some(base_time + Duration::from_millis(6)));
+        assert_eq!(buffer.last_time(), Some(base_time + Duration::from_millis(600)));
 
-        assert_eq!(buffer.diff(), (Duration::from_millis(4), Vector2D::new(4., 4. * 2.)));
+        assert_eq!(buffer.mean_velocity(), Vector2D::new(10.0, 20.0));
     }
 
     #[test]
     fn test_push_beyond_capacity_wrap_back() {
         const CAP: usize = 5;
-        let mut buffer: PositionTimeRingBuffer<CAP> = PositionTimeRingBuffer::default();
+        let mut buffer: VelocityRingBuffer<CAP> = VelocityRingBuffer::default();
         let base_time = Instant::now();
 
         // Push more than capacity
         for i in 0..CAP {
-            let time = base_time + Duration::from_millis(i as u64);
-            let point = LogicalPoint::new(i as f32 * 3., i as f32 * -2. + 100.);
-            buffer.push(time, point);
+            let time = base_time + Duration::from_millis(i as u64 * 100);
+            buffer.push(time, Vector2D::new(3.0, -2.0));
         }
 
         assert!(!buffer.empty());
         assert!(buffer.full);
         assert_eq!(buffer.curr_index, 0);
         assert_eq!(buffer.latest_index(), CAP - 1);
-        assert_eq!(buffer.last_time(), Some(base_time + Duration::from_millis(4)));
+        assert_eq!(buffer.last_time(), Some(base_time + Duration::from_millis(400)));
 
-        // Wrapping back must be done
-        assert_eq!(buffer.diff(), (Duration::from_millis(4), Vector2D::new(4. * 3., 4. * -2.)));
+        assert_eq!(buffer.mean_velocity(), Vector2D::new(30.0, -20.0));
     }
 }

internal/core/model/repeater.rs

@@ -349,15 +349,17 @@ fn update_visible_instances(
         viewport_height.set(LogicalLength::new(state.cached_item_height * row_count as Coord));
         viewport_width.set(LogicalLength::new(vp_width));
         // If an animation is ongoing we should not interrupt it
-        if !viewport_y.has_binding() {
-            let new_viewport_y = -state.anchor_y + new_offset_y;
-            if new_viewport_y != viewport_y.get().get() {
-                viewport_y.set(LogicalLength::new(new_viewport_y));
-            }
-            state.previous_viewport_y = new_viewport_y;
-        } else {
-            state.previous_viewport_y = viewport_y.get().0;
+        let new_viewport_y = -state.anchor_y + new_offset_y;
+        // Important: Use get_internal here, the viewport_y may have a binding on it (especially
+        // a physical animation).
+        // We must not yet trigger a re-evaluation of that binding, as we have already updated the
+        // viewport_width and viewport_height, but the viewport_y is not yet consistent.
+        // So the physics animations limit value may be inconsistent.
+        if new_viewport_y != viewport_y.get_internal().get() {
+            viewport_y.set(LogicalLength::new(new_viewport_y));
         }
+        state.previous_viewport_y = new_viewport_y;
+
         break;
     }