diff --git a/ui/dr-ui/src/identity_ui.rs b/ui/dr-ui/src/identity_ui.rs index 68b0803..b543b97 100644 --- a/ui/dr-ui/src/identity_ui.rs +++ b/ui/dr-ui/src/identity_ui.rs @@ -160,9 +160,16 @@ pub fn refresh( ctl.covers.borrow_mut().retain(|id, _| live.contains(id)); } + // Filtered here rather than by the row hiding itself, which is what the + // rail used to do. A row that collapses to 0px is a height that reads the + // model, and the `ListView` drawing the rail cannot virtualise past one — + // see the rule in `widgets.slint`. The toggle reloads either way, so this + // costs a pass over a list that was already in hand. + let show_ignored = ctl.show_ignored.get(); let rows: Vec = view .people .iter() + .filter(|p| show_ignored || !p.ignored) .map(|p| { let cover = store.and_then(|store| { if let Some(img) = ctl.covers.borrow().get(&p.id) { diff --git a/ui/dr-ui/ui/identity.slint b/ui/dr-ui/ui/identity.slint index ace70f6..8cf73dc 100644 --- a/ui/dr-ui/ui/identity.slint +++ b/ui/dr-ui/ui/identity.slint @@ -15,7 +15,7 @@ // the confirm/reject pair sits on the face itself rather than behind a menu. import { Theme } from "theme.slint"; -import { Panel, Button, IconButton, Field } from "widgets.slint"; +import { Panel, Button, IconButton, Field, ListView } from "widgets.slint"; import { SliderRow } from "controls.slint"; import { Icon } from "icons.slint"; @@ -318,6 +318,9 @@ export component IdentityScreen inherits Rectangle { // than disappearing — 132px keeps the 40px cover and elides the // labels, and leaves the faces grid enough width for two columns. width: root.compact ? 132px : 260px; + // The rail scrolls, so its column has to be given the panel's + // height rather than the sum of its parts — see `Panel.fill`. + fill: true; VerticalLayout { padding: Theme.gap; @@ -346,18 +349,30 @@ export component IdentityScreen inherits Rectangle { wrap: word-wrap; } - Flickable { - VerticalLayout { - spacing: 2px; - alignment: start; + // A `ListView`, so the rail costs what is on screen rather than + // what the library holds. See `widgets.slint` — and heed its + // one rule: **the row height below is a constant**, and the + // model arrives already filtered. Hiding a row by collapsing it + // to 0px, which is what this did, is a height that reads the + // model, and it makes Slint build all of them anyway. + // + // A direct child of this layout, with nothing wrapped around + // it. `ListView` carries its own stretch and preferred size — + // it has to, having no natural height to offer — and putting a + // plain Rectangle in between hands the layout that Rectangle's + // constraints instead, which are taken from the list and are + // therefore nothing. The rail comes out empty, with every + // person still in the model. + ListView { + for p[i] in root.people: Rectangle { + // 52 of a row, then the 2px that used to be the + // layout's `spacing`. Virtualisation places row N + // at N × this, so the gap lives inside the row. + height: 54px; - for p[i] in root.people: Rectangle { - // Height and visibility rather than filtering the - // model in Rust: the rail is rebuilt on every - // action, and re-deriving a second filtered list - // per rebuild is work the toggle can do for free. - visible: root.show-ignored || !p.ignored; - height: (root.show-ignored || !p.ignored) ? 52px : 0px; + Rectangle { + y: 0px; + height: 52px; border-radius: Theme.radius; background: p.id == root.selected-person ? Theme.selected @@ -408,7 +423,11 @@ export component IdentityScreen inherits Rectangle { } } - Rectangle { } + // No spacer between the rail and the buttons below it. There + // used to be one, to push them to the bottom of a short rail; + // the rail now stretches into that space itself, and a second + // stretching child would halve the list to make room for + // nothing. // What the "not interested" button has hidden, and the way // back to it. A count with no way to reveal it would make the diff --git a/ui/dr-ui/ui/widgets.slint b/ui/dr-ui/ui/widgets.slint index e7ef6c2..2359f1d 100644 --- a/ui/dr-ui/ui/widgets.slint +++ b/ui/dr-ui/ui/widgets.slint @@ -676,6 +676,19 @@ export component Panel inherits Rectangle { /// `padding` for the layout it may contain, and redeclaring it is a /// compile error rather than an override. in property inset: Theme.gap; + /// Let the contents grow into the panel's full height. + /// + /// The default packs them at the top, which is right for the stacks of + /// controls this was written for: a column of sliders should sit under its + /// heading, not spread out to meet the bottom edge. + /// + /// **A panel holding something that scrolls needs the other answer**, and + /// needs it stated. `alignment: start` gives every child its *preferred* + /// height, and a scrolling view has no preferred height worth the name — + /// its whole purpose is to be smaller than what it holds. So it is given + /// nothing and draws nothing, with no error and no clue: the People rail + /// came out blank this way, its model full and its list 0px tall. + in property fill: false; background: Theme.surface; border-radius: root.flat ? 0px : Theme.radius; @@ -685,7 +698,7 @@ export component Panel inherits Rectangle { VerticalLayout { padding: root.inset; spacing: root.spacing; - alignment: start; + alignment: root.fill ? LayoutAlignment.stretch : LayoutAlignment.start; @children } } @@ -932,3 +945,73 @@ export component EmptyState inherits VerticalLayout { wrap: word-wrap; } } + +// A scrolling list that only builds the rows you can see. +// +// # Why this exists rather than `Flickable { VerticalLayout { for … } }` +// +// That spelling instantiates every row. It is invisible on a list of forty and +// it is the whole cost on a list of fourteen thousand: the Identity rail built +// 14,268 row subtrees on every rebuild, which measures at 4.2 seconds before a +// pixel is drawn — and it paid that again on every action, because every +// action reloads the model. +// +// Slint's compiler has a virtualising path for a `for`, and it is what makes +// `std-widgets`' `ListView` cheap. It keys on the parent element's base being +// *named* `ListView` and exposing the five lengths its layouting code writes +// back — see `parent_is_listview` in `i-slint-compiler`'s `object_tree`. A +// custom base is explicitly allowed, and that is what this is: the +// optimisation without `std-widgets`, whose `ListView` inherits its own +// `ScrollView` and would bring a second style into the file that establishes +// ours. +// +// # The rule a caller must keep +// +// **Every row must be the same, constant height, and that height must not read +// the model.** The virtualisation places row N at `N × height` without +// building rows 0..N, so a height the model can change is a height the layout +// cannot know in advance. Slint's answer to that is to build all of them +// anyway — silently, and at the full cost this component exists to avoid. A +// row that hides itself with `height: cond ? 52px : 0px` is that mistake +// wearing a conditional: to leave a row out, leave it out of the model. +// +// # Why it wraps a `Flickable` instead of being one +// +// A `Flickable` takes its preferred *and maximum* height from its viewport, and +// the viewport of a virtualising list is written by the layouting pass — which +// has not run at the moment the enclosing layout asks how tall this wants to +// be. Inheriting `Flickable` therefore answers "nothing", truthfully, and gets +// nothing: an empty rail with every person still in the model, which is exactly +// what the first attempt at this shipped into a screenshot. +// +// So the sizing is declared here and the `Flickable` is held inside, filling +// it — the same shape, and the same six lines, as `std-widgets`' own +// `ScrollView`. Its scrollbars are the part left out: the rails that use this +// draw their own chrome, or none. +export component ListView { + // Aliases rather than bindings, because the list-view layouting writes + // through them. The compiler requires all five, as lengths, to recognise + // this as a list view at all. + out property visible-width <=> flick.width; + out property visible-height <=> flick.height; + in-out property viewport-width <=> flick.viewport-width; + in-out property viewport-height <=> flick.viewport-height; + in-out property viewport-x <=> flick.viewport-x; + in-out property viewport-y <=> flick.viewport-y; + + callback scrolled <=> flick.flicked; + + min-width: 0px; + min-height: 0px; + horizontal-stretch: 1; + vertical-stretch: 1; + preferred-width: 100%; + preferred-height: 100%; + + flick := Flickable { + width: parent.width; + height: parent.height; + + @children + } +}