Open qarmin opened 11 months ago
Thanks for your bug report. I can reproduce the problem.
One problem is that we use f32 for the coordinate, and that the content-y of the underlying ListView can become really big, so big that we get rounding error on the f32 computations when computing the location of the items within the viewport.
In particular the problem is there: https://github.com/slint-ui/slint/blob/6cd68e84a7be8a43baa060565f0bbe2a11b9101b/internal/core/model.rs#L1081
the size of the row is 42.0px. So y
in that expression, which is the position in pixel within the viewport, is going to be 42 * some_index, where some_index can be millions. So we are way over 100M pixels
The listview_layout function will position the element at y
, and add 42px to it. The problem is that in f32, y+42
is actually a few pixel off as that number might not be representable in a f32.
hence the wrong gaps.
I don't explain the panic yet. It tries to query the vector of instance with a completely wrong index, as it iterate over widgets in the accessibility code.
The panics happen because the view is relayouted between the range check and the call to get_subtree:
We query the range line 429, and check the validity of index line 432, but the get_subtree
notice that the tree is dirty (not sure why) and redo the layout. And the item computation turns out to be something different because the floating point computation give different result, and we just query the components outside of the range.
(next_subtree_index is outside of the new range which is recomputed by get_subtree itself)
https://github.com/slint-ui/slint/pull/3709 is fixing the panic.
thread 'main' panicked at /home/rafal/Projekty/Rust/czkawka/target/debug/build/czkawka_slint-3baf4a5d90d5188c/out/main_window.rs:6791:159:
called `Option::unwrap()` on a `None` value
stack backtrace:
4: czkawka_slint::slint_generatedMainWindow::InnerComponent_text_54::item_geometry
5: <czkawka_slint::slint_generatedMainWindow::InnerComponent_text_54 as i_slint_core::item_tree::ItemTree_vtable_mod::ItemTree>::item_geometry
6: <czkawka_slint::slint_generatedMainWindow::InnerComponent_text_54 as const_field_offset::PinnedDrop>::drop::VT::item_geometry
7: i_slint_core::item_tree::ItemTree_vtable_mod::ItemTreeTO::item_geometry
8: i_slint_core::item_tree::ItemRc::geometry
9: i_slint_backend_winit::accesskit::AccessKitAdapter::build_node_without_children
10: i_slint_backend_winit::accesskit::AccessKitAdapter::build_node_for_item_recursively::{{closure}}
11: i_slint_core::properties::CURRENT_BINDING::<impl i_slint_core::properties::CURRENT_BINDING>::set
12: i_slint_core::properties::PropertyTracker<DirtyHandler>::evaluate_as_dependency_root
13: i_slint_core::properties::PropertyTracker<DirtyHandler>::evaluate
14: i_slint_backend_winit::accesskit::AccessKitAdapter::build_node_for_item_recursively
15: i_slint_backend_winit::accesskit::AccessKitAdapter::build_node_for_item_recursively::{{closure}}
Which is just another symptom of https://github.com/slint-ui/slint/issues/3464 as the accessibility iterate over items, but while iterating the listview relayout code is executed that produces different result and delete the parent element.
We have a combination of several problem:
The main issue is that we represent coordinate in f32, and the maximum integer that can fit in a f32 is 16777216. But the coordinate of the viewport relative to the flickable can be much larger than that when having many millions entries. And therefore their position is not accurately representable.
We do relayout the list view more often than needed. Commit 4d0568873069b7ca7134a5e3f18c68ff3a1f54e9 actually removed the listview_geometry_tracker.evaluate_if_dirty
which would only relayout when the listview is dirty. So we relayout all the time when traversing the elements, which is a lot. Fortunately, the relayout is fairly fast as there are never so many elements visible at the same time. But still.
If you sum it up, we get the layout redone too often wand because of point 1. it result in different amount of element which result in item being deleted which cause a panic because of #3464
Also the fact that accessibility is rebuilding the tree even if there is no accessibility backend which is a bug in accesskit.
What can we do?
First thought was to use f64 for coordinates on the desktop. We already have a switch in https://github.com/slint-ui/slint/blob/5bf2c7192b6264947e5e80e7e85f831f4ea3f852/internal/core/lib.rs#L82 for the MCUs, we could as well allow to switch to f64. But it turns out that even doing so wouldn't help fully because the femtovg and skia renderer use f32 internally and we do translate the canvas when we get into items. Instead of translating the canvas like this, we could maintain offsets in f64 within slint, but that's some extra refactoring.
Another possibility would be to not have the listview items as chilren of the viewport, but instead, have them as sibling of the viewport. And so the layouting code can just give them a y position relative to the ListView instead of relative to the viewport. I think this is possible, but require some refactoring in the runtime and compiler.
Couldn't this be fixed with what I hinted at in #5758 (some modulo-like calculation), instead of letting a number grow uncontrollably? Scroll deltas should always be whole physical pixels, right? Then you should actually be able to maintain the illusion of a continuous list, while a rendered-items block is regularly shifted during scrolling (its data together with its position), without getting graphical-rendering imperfections that suddenly show on content block shift. I'm not sure whether these mid-animation shifts would require extra support from the animation system.
Of course, there's still the issue of how things are communicated to the user. For my needs in the linked discussion, the component would communicate a duration
(a 64-bit value) that the content block should start with; meaning, the backend would provide appropriate data that the component then renders. This would be my precise "actual position in the data". For cases with an overly long list with a scrollbar, you could also establish a callback that tells the user to adapt the window into the data that the backing model then would represent (using integer indices). This means, you'd have the actual model, and an integer pair of model start in the virtual data and virtual-model length.
CC: @ezschemi
Slint 1.2.2
When trying to add 10 millions rows to model, after scrolling down to ~5.6 million record I got crash
Looks that after 2/3 millions of items it creates empty space between items
Scroll bar after 3.3 million record go back to ~2.8 million record, probably due adding space after items
Selection dissapears when selected row is not in screen - this happens even with 10 rows
Project to test - czkawka_slint_gui.zip
Video with visible problems:
https://github.com/slint-ui/slint/assets/41945903/d146dc8a-68ec-46bc-8a63-8be2169b22c9
Panic