From 4da79ac69a5ad5f0b8e5544a9adb5b4ebf042d83 Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Mon, 20 Jan 2025 10:51:03 +0100 Subject: [PATCH] Fix layouting of huge ListView with millions of items The problem is that the precision of f32 for coordinate wouldn't be accurate enough with such big viewport to put the elements so that they are next to eachother. So put the elements relative to the Flickable instead of relative to the created moving viewport Rectangle. Fixes #3700 Note that the ListView still use f32 for the scrollbar value, meaning that at some point, the wheel stops working as the wheel increment is smaller than the f32 increment, and scrolling becomes somehow fuzzy. But this only happens after one more billions pixels now, so one can have more than 50 millions of elements without much problems --- api/cpp/include/slint.h | 8 +- internal/compiler/passes/flickable.rs | 49 ++++++- .../compiler/passes/repeater_component.rs | 2 - internal/core/model.rs | 41 +++--- tests/cases/elements/listview-millions.slint | 130 ++++++++++++++++++ 5 files changed, 204 insertions(+), 26 deletions(-) create mode 100644 tests/cases/elements/listview-millions.slint diff --git a/api/cpp/include/slint.h b/api/cpp/include/slint.h index 6d0eaae3c..84874206e 100644 --- a/api/cpp/include/slint.h +++ b/api/cpp/include/slint.h @@ -1190,13 +1190,13 @@ public: void ensure_updated_listview(const Parent *parent, const private_api::Property *viewport_width, const private_api::Property *viewport_height, - [[maybe_unused]] const private_api::Property *viewport_y, + const private_api::Property *viewport_y, float listview_width, [[maybe_unused]] float listview_height) const { // TODO: the rust code in model.rs try to only allocate as many items as visible items ensure_updated(parent); - float h = compute_layout_listview(viewport_width, listview_width); + float h = compute_layout_listview(viewport_width, listview_width, viewport_y->get()); viewport_height->set(h); } @@ -1234,9 +1234,9 @@ public: } float compute_layout_listview(const private_api::Property *viewport_width, - float listview_width) const + float listview_width, float viewport_y) const { - float offset = 0; + float offset = viewport_y; viewport_width->set(listview_width); if (!inner) return offset; diff --git a/internal/compiler/passes/flickable.rs b/internal/compiler/passes/flickable.rs index db3e2ac98..99aaeca94 100644 --- a/internal/compiler/passes/flickable.rs +++ b/internal/compiler/passes/flickable.rs @@ -11,9 +11,10 @@ //! binding reference to fake properties use crate::expression_tree::{BindingExpression, Expression, MinMaxOp, NamedReference}; -use crate::langtype::{ElementType, NativeClass}; +use crate::langtype::{ElementType, NativeClass, Type}; use crate::object_tree::{Component, Element, ElementRc}; use crate::typeregister::TypeRegister; +use core::cell::RefCell; use smol_str::{format_smolstr, SmolStr}; use std::rc::Rc; @@ -43,6 +44,46 @@ pub fn handle_flickable(root_component: &Rc, tr: &TypeRegister) { fn create_viewport_element(flickable: &ElementRc, native_empty: &Rc) { let children = std::mem::take(&mut flickable.borrow_mut().children); + let is_listview = children + .iter() + .any(|c| c.borrow().repeated.as_ref().is_some_and(|r| r.is_listview.is_some())); + + if is_listview { + // Fox Listview, we don't bind the y property to the geometry because for large listview, we want to support coordinate with more precision than f32 + // so the actual geometry is relative to the Flickable instead of the viewport + // We still assign a binding to the y property in case it is read by someone + for c in &children { + if c.borrow().repeated.is_none() { + // Normally should not happen, listview should only have one children, and it should be repeated + continue; + } + let ElementType::Component(base) = c.borrow().base_type.clone() else { continue }; + let inner_elem = &base.root_element; + let new_y = crate::layout::create_new_prop( + inner_elem, + SmolStr::new_static("actual-y"), + Type::LogicalLength, + ); + new_y.mark_as_set(); + inner_elem.borrow_mut().bindings.insert( + "y".into(), + RefCell::new( + Expression::BinaryExpression { + lhs: Expression::PropertyReference(new_y.clone()).into(), + rhs: Expression::PropertyReference(NamedReference::new( + flickable, + SmolStr::new_static("viewport-y"), + )) + .into(), + op: '-', + } + .into(), + ), + ); + inner_elem.borrow_mut().geometry_props.as_mut().unwrap().y = new_y; + } + } + let viewport = Element::make_rc(Element { id: format_smolstr!("{}-viewport", flickable.borrow().id), base_type: ElementType::Native(native_empty.clone()), @@ -53,8 +94,12 @@ fn create_viewport_element(flickable: &ElementRc, native_empty: &Rc }); let element_type = flickable.borrow().base_type.clone(); for prop in element_type.as_builtin().properties.keys() { + // bind the viewport's property to the flickable property, such as: `width <=> parent.viewport-width` if let Some(vp_prop) = prop.strip_prefix("viewport-") { - // bind the viewport's property to the flickable property, such as: `width <=> parent.viewport-width` + if is_listview && matches!(vp_prop, "y" | "height") { + //don't bind viewport-y for ListView because the layout is handled by the runtime + continue; + } viewport.borrow_mut().bindings.insert( vp_prop.into(), BindingExpression::new_two_way(NamedReference::new(flickable, prop.clone())).into(), diff --git a/internal/compiler/passes/repeater_component.rs b/internal/compiler/passes/repeater_component.rs index aa2081643..14eba5ead 100644 --- a/internal/compiler/passes/repeater_component.rs +++ b/internal/compiler/passes/repeater_component.rs @@ -85,8 +85,6 @@ fn create_repeater_components(component: &Rc) { RefCell::new(Expression::PropertyReference(listview.listview_width).into()), ); } - - NamedReference::new(&comp.root_element, SmolStr::new_static("y")).mark_as_set(); } let weak = Rc::downgrade(&comp); diff --git a/internal/core/model.rs b/internal/core/model.rs index 5efb24052..cf8787910 100644 --- a/internal/core/model.rs +++ b/internal/core/model.rs @@ -974,23 +974,24 @@ impl Repeater { viewport_width.set(listview_width); let model = self.model(); let row_count = model.row_count(); + let zero = LogicalLength::zero(); if row_count == 0 { self.0.inner.borrow_mut().instances.clear(); - viewport_height.set(LogicalLength::zero()); - viewport_y.set(LogicalLength::zero()); + viewport_height.set(zero); + viewport_y.set(zero); return; } let listview_height = listview_height.get(); - let mut vp_y = viewport_y.get().min(LogicalLength::zero()); + let mut vp_y = viewport_y.get().min(zero); // We need some sort of estimation of the element height let cached_item_height = self.data().inner.borrow_mut().cached_item_height; - let element_height = if cached_item_height > LogicalLength::zero() { + let element_height = if cached_item_height > zero { cached_item_height } else { - let total_height = Cell::new(LogicalLength::zero()); + let total_height = Cell::new(zero); let count = Cell::new(0); let get_height_visitor = |x: &ItemTreeRc| { let height = x.as_pin_ref().item_geometry(0).height_length(); @@ -1042,12 +1043,12 @@ impl Repeater { // We are jumping more than 1.5 screens, consider this as a random seek. inner.instances.clear(); inner.offset = ((-vp_y / element_height).get().floor() as usize).min(row_count - 1); - (inner.offset, -vp_y) + (inner.offset, zero) } else if vp_y < inner.previous_viewport_y { // we scrolled down, try to find out the new offset. - let mut it_y = first_item_y; + let mut it_y = first_item_y + vp_y; let mut new_offset = inner.offset; - debug_assert!(it_y <= -vp_y); // we scrolled down, the anchor should be hidden + debug_assert!(it_y <= zero); // we scrolled down, the anchor should be hidden for (i, c) in inner.instances.iter_mut().enumerate() { if c.0 == RepeatedInstanceState::Dirty { if c.1.is_none() { @@ -1060,7 +1061,7 @@ impl Repeater { c.0 = RepeatedInstanceState::Clean; } let h = c.1.as_ref().unwrap().as_pin_ref().item_geometry(0).height_length(); - if it_y + h >= -vp_y || new_offset + 1 >= row_count { + if it_y + h >= zero || new_offset + 1 >= row_count { break; } it_y += h; @@ -1069,14 +1070,14 @@ impl Repeater { (new_offset, it_y) } else { // We scrolled up, we'll instantiate items before offset in the loop - (inner.offset, first_item_y) + (inner.offset, first_item_y + vp_y) }; loop { // If there is a gap before the new_offset and the beginning of the visible viewport, // try to fill it with items. First look at items that are before new_offset in the // inner.instances, if any. - while new_offset > inner.offset && new_offset_y > -vp_y { + while new_offset > inner.offset && new_offset_y > zero { new_offset -= 1; new_offset_y -= inner.instances[new_offset - inner.offset] .1 @@ -1088,7 +1089,7 @@ impl Repeater { } // If there is still a gap, fill it with new instances before let mut new_instances = Vec::new(); - while new_offset > 0 && new_offset_y > -vp_y { + while new_offset > 0 && new_offset_y > zero { new_offset -= 1; let new_instance = init(); if let Some(data) = model.row_data(new_offset) { @@ -1137,13 +1138,13 @@ impl Repeater { x.as_pin_ref().listview_layout(&mut y, viewport_width); } idx += 1; - if y >= -vp_y + listview_height { + if y >= listview_height { break; } } // create more items until there is no more room. - while y < -vp_y + listview_height && idx < row_count { + while y < listview_height && idx < row_count { let new_instance = init(); if let Some(data) = model.row_data(idx) { new_instance.update(idx, data); @@ -1153,11 +1154,15 @@ impl Repeater { inner.instances.push((RepeatedInstanceState::Clean, Some(new_instance))); idx += 1; } - if y < -vp_y + listview_height && vp_y < LogicalLength::zero() { + if y < listview_height && vp_y < zero { assert!(idx >= row_count); // we reached the end of the model, and we still have room. scroll a bit up. - vp_y = listview_height - y; - continue; + let new_vp_y = vp_y + (listview_height - y); + // check that we actually did scroll (for very large lists we can exceed the precision of f32 and have an infinite loop) + if new_vp_y != vp_y { + vp_y = new_vp_y; + continue; + } } // Let's cleanup the instances that are not shown. @@ -1187,7 +1192,7 @@ impl Repeater { inner.cached_item_height = (y - new_offset_y) / inner.instances.len() as Coord; inner.anchor_y = inner.cached_item_height * inner.offset as Coord; viewport_height.set(inner.cached_item_height * row_count as Coord); - let new_viewport_y = -inner.anchor_y + vp_y + new_offset_y; + let new_viewport_y = -inner.anchor_y + new_offset_y; viewport_y.set(new_viewport_y); inner.previous_viewport_y = new_viewport_y; break; diff --git a/tests/cases/elements/listview-millions.slint b/tests/cases/elements/listview-millions.slint new file mode 100644 index 000000000..e3939dbf1 --- /dev/null +++ b/tests/cases/elements/listview-millions.slint @@ -0,0 +1,130 @@ +// Copyright © SixtyFPS GmbH +// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-2.0 OR LicenseRef-Slint-Software-3.0 + +// As of now, the C++ optimization doesn't optimize hidden item of Listview so this test would take forever +//ignore:cpp + +import { ListView } from "std-widgets.slint"; +export component TestCase inherits Window { + preferred-width: 300px; + preferred-height: 300px; + + in-out property result; + callback clicked(int); + in-out property viewport-y <=> lv.viewport-y; + + lv := ListView { + for _[num] in 2130000000: Rectangle { + height: 20px; + border-width: 1px; + border-color: red; + Text { text: num; } + TouchArea { + clicked => { result = "|"+num; root.clicked(num) } + } + } + } +} + + +/* + + +```rust +let instance = TestCase::new().unwrap(); +let clicked = std::rc::Rc::new(std::cell::RefCell::new(Vec::::new())); +let clicked2 = clicked.clone(); +instance.on_clicked(move |x| clicked2.borrow_mut().push(x) ); +slint_testing::send_mouse_click(&instance, 5., 250.); +assert_eq!(clicked.borrow().as_slice(), &[12]); +slint_testing::send_mouse_click(&instance, 5., 263.); +slint_testing::send_mouse_click(&instance, 5., 239.); +assert_eq!(clicked.borrow().as_slice(), &[12, 13, 11]); + +instance.set_viewport_y(-20. * 1000.); +clicked.borrow_mut().clear(); +slint_testing::send_mouse_click(&instance, 5., 250.); +assert_eq!(clicked.borrow().as_slice(), &[1012]); +slint_testing::send_mouse_click(&instance, 5., 263.); +slint_testing::send_mouse_click(&instance, 5., 239.); +assert_eq!(clicked.borrow().as_slice(), &[1012, 1013, 1011]); + +instance.set_viewport_y(-20. * 100000.); +clicked.borrow_mut().clear(); +slint_testing::send_mouse_click(&instance, 5., 250.); +assert_eq!(clicked.borrow().as_slice(), &[100012]); +slint_testing::send_mouse_click(&instance, 5., 263.); +slint_testing::send_mouse_click(&instance, 5., 239.); +assert_eq!(clicked.borrow().as_slice(), &[100012, 100013, 100011]); + +instance.set_viewport_y(-20. * 10000000.); +clicked.borrow_mut().clear(); +slint_testing::send_mouse_click(&instance, 5., 250.); +assert_eq!(clicked.borrow().as_slice(), &[10000012]); +slint_testing::send_mouse_click(&instance, 5., 263.); +slint_testing::send_mouse_click(&instance, 5., 239.); +assert_eq!(clicked.borrow().as_slice(), &[10000012, 10000013, 10000011]); + +instance.set_viewport_y(-20. * 210000000.); +clicked.borrow_mut().clear(); +slint_testing::send_mouse_click(&instance, 5., 250.); +assert_eq!(clicked.borrow().as_slice(), &[210000012]); +slint_testing::send_mouse_click(&instance, 5., 263.); +slint_testing::send_mouse_click(&instance, 5., 239.); +assert_eq!(clicked.borrow().as_slice(), &[210000012, 210000013, 210000011]); +``` + +```cpp +auto handle = TestCase::create(); +const TestCase &instance = *handle; +auto clicked = std::make_shared>(); +instance.on_clicked([clicked](int x) { clicked->push_back(x); }); +slint_testing::send_mouse_click(&instance, 5., 250.); +assert(*clicked == std::vector{12}); +``` + +```js +var instance = new slint.TestCase(); +var clicked = new Array(); +instance.clicked = function(x) { clicked.push(x); }; +slintlib.private_api.send_mouse_click(instance, 5., 250.); +assert.deepEqual(clicked, [12]); +slintlib.private_api.send_mouse_click(instance, 5., 263.); +slintlib.private_api.send_mouse_click(instance, 5., 239.); +assert.deepEqual(clicked, [12, 13, 11]); + +instance.viewport_y=(-20. * 1000.); +clicked.length = 0; +slintlib.private_api.send_mouse_click(instance, 5., 250.); +assert.deepEqual(clicked, [1012]); +slintlib.private_api.send_mouse_click(instance, 5., 263.); +slintlib.private_api.send_mouse_click(instance, 5., 239.); +assert.deepEqual(clicked, [1012, 1013, 1011]); + +instance.viewport_y=(-20. * 100000.); +clicked.length = 0; +slintlib.private_api.send_mouse_click(instance, 5., 250.); +assert.deepEqual(clicked, [100012]); +slintlib.private_api.send_mouse_click(instance, 5., 263.); +slintlib.private_api.send_mouse_click(instance, 5., 239.); +assert.deepEqual(clicked, [100012, 100013, 100011]); + +instance.viewport_y=(-20. * 10000000.); +clicked.length = 0; +slintlib.private_api.send_mouse_click(instance, 5., 250.); +assert.deepEqual(clicked, [10000012]); +slintlib.private_api.send_mouse_click(instance, 5., 263.); +slintlib.private_api.send_mouse_click(instance, 5., 239.); +assert.deepEqual(clicked, [10000012, 10000013, 10000011]); + +instance.viewport_y=(-20. * 210000000.); +clicked.length = 0; +slintlib.private_api.send_mouse_click(instance, 5., 250.); +assert.deepEqual(clicked, [210000012]); +slintlib.private_api.send_mouse_click(instance, 5., 263.); +slintlib.private_api.send_mouse_click(instance, 5., 239.); +assert.deepEqual(clicked, [210000012, 210000013, 210000011]); +``` + + +*/