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
This commit is contained in:
Olivier Goffart 2025-01-20 10:51:03 +01:00
parent 387e4202ea
commit 4da79ac69a
5 changed files with 204 additions and 26 deletions

View file

@ -1190,13 +1190,13 @@ public:
void ensure_updated_listview(const Parent *parent, void ensure_updated_listview(const Parent *parent,
const private_api::Property<float> *viewport_width, const private_api::Property<float> *viewport_width,
const private_api::Property<float> *viewport_height, const private_api::Property<float> *viewport_height,
[[maybe_unused]] const private_api::Property<float> *viewport_y, const private_api::Property<float> *viewport_y,
float listview_width, [[maybe_unused]] float listview_height) const 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 // TODO: the rust code in model.rs try to only allocate as many items as visible items
ensure_updated(parent); 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); viewport_height->set(h);
} }
@ -1234,9 +1234,9 @@ public:
} }
float compute_layout_listview(const private_api::Property<float> *viewport_width, float compute_layout_listview(const private_api::Property<float> *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); viewport_width->set(listview_width);
if (!inner) if (!inner)
return offset; return offset;

View file

@ -11,9 +11,10 @@
//! binding reference to fake properties //! binding reference to fake properties
use crate::expression_tree::{BindingExpression, Expression, MinMaxOp, NamedReference}; 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::object_tree::{Component, Element, ElementRc};
use crate::typeregister::TypeRegister; use crate::typeregister::TypeRegister;
use core::cell::RefCell;
use smol_str::{format_smolstr, SmolStr}; use smol_str::{format_smolstr, SmolStr};
use std::rc::Rc; use std::rc::Rc;
@ -43,6 +44,46 @@ pub fn handle_flickable(root_component: &Rc<Component>, tr: &TypeRegister) {
fn create_viewport_element(flickable: &ElementRc, native_empty: &Rc<NativeClass>) { fn create_viewport_element(flickable: &ElementRc, native_empty: &Rc<NativeClass>) {
let children = std::mem::take(&mut flickable.borrow_mut().children); 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 { let viewport = Element::make_rc(Element {
id: format_smolstr!("{}-viewport", flickable.borrow().id), id: format_smolstr!("{}-viewport", flickable.borrow().id),
base_type: ElementType::Native(native_empty.clone()), base_type: ElementType::Native(native_empty.clone()),
@ -53,8 +94,12 @@ fn create_viewport_element(flickable: &ElementRc, native_empty: &Rc<NativeClass>
}); });
let element_type = flickable.borrow().base_type.clone(); let element_type = flickable.borrow().base_type.clone();
for prop in element_type.as_builtin().properties.keys() { 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-") { 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( viewport.borrow_mut().bindings.insert(
vp_prop.into(), vp_prop.into(),
BindingExpression::new_two_way(NamedReference::new(flickable, prop.clone())).into(), BindingExpression::new_two_way(NamedReference::new(flickable, prop.clone())).into(),

View file

@ -85,8 +85,6 @@ fn create_repeater_components(component: &Rc<Component>) {
RefCell::new(Expression::PropertyReference(listview.listview_width).into()), 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); let weak = Rc::downgrade(&comp);

View file

@ -974,23 +974,24 @@ impl<C: RepeatedItemTree + 'static> Repeater<C> {
viewport_width.set(listview_width); viewport_width.set(listview_width);
let model = self.model(); let model = self.model();
let row_count = model.row_count(); let row_count = model.row_count();
let zero = LogicalLength::zero();
if row_count == 0 { if row_count == 0 {
self.0.inner.borrow_mut().instances.clear(); self.0.inner.borrow_mut().instances.clear();
viewport_height.set(LogicalLength::zero()); viewport_height.set(zero);
viewport_y.set(LogicalLength::zero()); viewport_y.set(zero);
return; return;
} }
let listview_height = listview_height.get(); 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 // We need some sort of estimation of the element height
let cached_item_height = self.data().inner.borrow_mut().cached_item_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 cached_item_height
} else { } else {
let total_height = Cell::new(LogicalLength::zero()); let total_height = Cell::new(zero);
let count = Cell::new(0); let count = Cell::new(0);
let get_height_visitor = |x: &ItemTreeRc<C>| { let get_height_visitor = |x: &ItemTreeRc<C>| {
let height = x.as_pin_ref().item_geometry(0).height_length(); let height = x.as_pin_ref().item_geometry(0).height_length();
@ -1042,12 +1043,12 @@ impl<C: RepeatedItemTree + 'static> Repeater<C> {
// We are jumping more than 1.5 screens, consider this as a random seek. // We are jumping more than 1.5 screens, consider this as a random seek.
inner.instances.clear(); inner.instances.clear();
inner.offset = ((-vp_y / element_height).get().floor() as usize).min(row_count - 1); 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 { } else if vp_y < inner.previous_viewport_y {
// we scrolled down, try to find out the new offset. // 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; 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() { for (i, c) in inner.instances.iter_mut().enumerate() {
if c.0 == RepeatedInstanceState::Dirty { if c.0 == RepeatedInstanceState::Dirty {
if c.1.is_none() { if c.1.is_none() {
@ -1060,7 +1061,7 @@ impl<C: RepeatedItemTree + 'static> Repeater<C> {
c.0 = RepeatedInstanceState::Clean; c.0 = RepeatedInstanceState::Clean;
} }
let h = c.1.as_ref().unwrap().as_pin_ref().item_geometry(0).height_length(); 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; break;
} }
it_y += h; it_y += h;
@ -1069,14 +1070,14 @@ impl<C: RepeatedItemTree + 'static> Repeater<C> {
(new_offset, it_y) (new_offset, it_y)
} else { } else {
// We scrolled up, we'll instantiate items before offset in the loop // 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 { loop {
// If there is a gap before the new_offset and the beginning of the visible viewport, // 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 // try to fill it with items. First look at items that are before new_offset in the
// inner.instances, if any. // 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 -= 1;
new_offset_y -= inner.instances[new_offset - inner.offset] new_offset_y -= inner.instances[new_offset - inner.offset]
.1 .1
@ -1088,7 +1089,7 @@ impl<C: RepeatedItemTree + 'static> Repeater<C> {
} }
// If there is still a gap, fill it with new instances before // If there is still a gap, fill it with new instances before
let mut new_instances = Vec::new(); 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; new_offset -= 1;
let new_instance = init(); let new_instance = init();
if let Some(data) = model.row_data(new_offset) { if let Some(data) = model.row_data(new_offset) {
@ -1137,13 +1138,13 @@ impl<C: RepeatedItemTree + 'static> Repeater<C> {
x.as_pin_ref().listview_layout(&mut y, viewport_width); x.as_pin_ref().listview_layout(&mut y, viewport_width);
} }
idx += 1; idx += 1;
if y >= -vp_y + listview_height { if y >= listview_height {
break; break;
} }
} }
// create more items until there is no more room. // 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(); let new_instance = init();
if let Some(data) = model.row_data(idx) { if let Some(data) = model.row_data(idx) {
new_instance.update(idx, data); new_instance.update(idx, data);
@ -1153,11 +1154,15 @@ impl<C: RepeatedItemTree + 'static> Repeater<C> {
inner.instances.push((RepeatedInstanceState::Clean, Some(new_instance))); inner.instances.push((RepeatedInstanceState::Clean, Some(new_instance)));
idx += 1; idx += 1;
} }
if y < -vp_y + listview_height && vp_y < LogicalLength::zero() { if y < listview_height && vp_y < zero {
assert!(idx >= row_count); assert!(idx >= row_count);
// we reached the end of the model, and we still have room. scroll a bit up. // we reached the end of the model, and we still have room. scroll a bit up.
vp_y = listview_height - y; let new_vp_y = vp_y + (listview_height - y);
continue; // 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. // Let's cleanup the instances that are not shown.
@ -1187,7 +1192,7 @@ impl<C: RepeatedItemTree + 'static> Repeater<C> {
inner.cached_item_height = (y - new_offset_y) / inner.instances.len() as Coord; inner.cached_item_height = (y - new_offset_y) / inner.instances.len() as Coord;
inner.anchor_y = inner.cached_item_height * inner.offset as Coord; inner.anchor_y = inner.cached_item_height * inner.offset as Coord;
viewport_height.set(inner.cached_item_height * row_count 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); viewport_y.set(new_viewport_y);
inner.previous_viewport_y = new_viewport_y; inner.previous_viewport_y = new_viewport_y;
break; break;

View file

@ -0,0 +1,130 @@
// 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
// 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 <string> result;
callback clicked(int);
in-out property <length> 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::<i32>::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<std::vector<int>>();
instance.on_clicked([clicked](int x) { clicked->push_back(x); });
slint_testing::send_mouse_click(&instance, 5., 250.);
assert(*clicked == std::vector<int>{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]);
```
*/