Refactored VirtualList

These set of changes attempt to resolve a few complexities from the
original implementation: sizing and how to dynamically update the
content in the list.

On the sizing front, manually specifying the width and height of the
rows felt like it was more complex than measuring the first widget and
using that for all other widgets. This allows a user who wants to force
an explicit size to use the Resize widget, while also supporting
SizeToFit flows. Additionally, this paves the way for us to add
horizontal scrolling to this list, but this commit was already complex
enough I held off on that change for now.

One workflow I wanted to see supported was going from 0 rows to 50 rows.
When the item count comes from a trait, it was pretty complicated to
determine how to tell the list to ask for a new row count. By having the
user provide a Value<usize>, they can provide a `Dynamic<usize>` that
can be updated with a new row count whenever the application determines
there is new data. We still need to figure out a way to force a refresh
of the data even if the row count doesn't change.

Ultimately changing this allowed removing the trait and seemingly
simplified the basic usage in addition to adding more flexibility.

examples/virtual-list.rs

@@ -1,29 +1,9 @@
-use cushy::styles::Dimension;
 use cushy::widget::MakeWidget;
-use cushy::widgets::virtual_list::{VirtualList, VirtualListContent};
+use cushy::widgets::VirtualList;
 use cushy::Run;
-use figures::units::Lp;
-
-#[derive(Debug)]
-struct TestVirtualList;
-
-impl VirtualListContent for TestVirtualList {
-    fn item_count(&self) -> impl cushy::value::IntoValue<usize> {
-        50
-    }
-    fn item_height(&self) -> impl cushy::value::IntoValue<cushy::styles::Dimension> {
-        cushy::styles::Dimension::Lp(Lp::inches_f(0.5))
-    }
-    fn widget_at(&self, index: usize) -> impl MakeWidget {
-        format!("Item {}", index)
-    }
-    fn width(&self) -> impl cushy::value::IntoValue<cushy::styles::Dimension> {
-        Dimension::Lp(Lp::inches_f(10.))
-    }
-}
 
 fn list() -> impl MakeWidget {
-    VirtualList::new(TestVirtualList).expand()
+    VirtualList::new(50, |index| format!("Item {}", index)).expand()
 }
 
 fn main() -> cushy::Result {

guide/guide-examples/examples/virtual-list.rs

@@ -1,32 +1,10 @@
-use cushy::figures::units::Lp;
-use cushy::styles::Dimension;
 use cushy::widget::MakeWidget;
-use cushy::widgets::virtual_list::{VirtualList, VirtualListContent};
+use cushy::widgets::VirtualList;
 use cushy::Run;
 
-// ANCHOR: implementation
-#[derive(Debug)]
-struct TestVirtualList;
-
-impl VirtualListContent for TestVirtualList {
-    fn item_count(&self) -> impl cushy::value::IntoValue<usize> {
-        50
-    }
-    fn item_height(&self) -> impl cushy::value::IntoValue<cushy::styles::Dimension> {
-        cushy::styles::Dimension::Lp(Lp::inches_f(0.5))
-    }
-    fn widget_at(&self, index: usize) -> impl MakeWidget {
-        format!("Item {}", index)
-    }
-    fn width(&self) -> impl cushy::value::IntoValue<cushy::styles::Dimension> {
-        Dimension::Lp(Lp::inches_f(10.))
-    }
-}
-// ANCHOR_END: implementation
-
 // ANCHOR: list
 fn list() -> impl MakeWidget {
-    VirtualList::new(TestVirtualList).expand()
+    VirtualList::new(50, |index| format!("Item {}", index)).expand()
 }
 // ANCHOR_END: list
 

src/lib.rs

@@ -195,6 +195,19 @@ impl ConstraintLimit {
             ConstraintLimit::SizeToFit(_) => measured.into_unsigned(),
         }
     }
+
+    /// When `self` is `SizeToFit`, the smallest of the constraint and
+    /// `measured` will be returned. When `self` is `Fill`, the fill size will
+    /// be returned.
+    pub fn fill_or_fit<Unit>(self, measured: Unit) -> UPx
+    where
+        Unit: IntoUnsigned<Unsigned = UPx>,
+    {
+        match self {
+            ConstraintLimit::Fill(size) => size,
+            ConstraintLimit::SizeToFit(size) => size.min(measured.into_unsigned()),
+        }
+    }
 }
 
 /// An extension trait for `Size<ConstraintLimit>`.

src/widget.rs

@@ -1348,7 +1348,7 @@ pub trait MakeWidget: Sized {
 
     /// Creates a [`WidgetRef`] for use as child widget.
     #[must_use]
-    fn widget_ref(self) -> WidgetRef {
+    fn into_ref(self) -> WidgetRef {
         WidgetRef::new(self)
     }
 

src/widgets.rs

@@ -25,7 +25,6 @@ pub mod progress;
 pub mod radio;
 mod resize;
 pub mod scroll;
-pub mod virtual_list;
 pub mod select;
 pub mod shortcuts;
 pub mod slider;
@@ -36,6 +35,7 @@ mod switcher;
 mod themed;
 mod tilemap;
 pub mod validated;
+mod virtual_list;
 pub mod wrap;
 
 pub use self::align::Align;
@@ -70,4 +70,5 @@ pub use self::switcher::Switcher;
 pub use self::themed::Themed;
 pub use self::tilemap::TileMap;
 pub use self::validated::Validated;
+pub use self::virtual_list::VirtualList;
 pub use self::wrap::Wrap;

src/widgets/button.rs

@@ -137,7 +137,7 @@ impl Button {
     /// Returns a new button with the provided label.
     pub fn new(content: impl MakeWidget) -> Self {
         Self {
-            content: content.widget_ref(),
+            content: content.into_ref(),
             on_click: None,
             per_window: WindowLocal::default(),
             kind: Value::Constant(ButtonKind::default()),

src/widgets/expand.rs

@@ -107,7 +107,17 @@ impl WrapperWidget for Expand {
         available_space: Size<ConstraintLimit>,
         context: &mut LayoutContext<'_, '_, '_, '_>,
     ) -> WrappedLayout {
-        let available_space = available_space.map(|lim| ConstraintLimit::Fill(lim.max()));
+        let available_space = match &self.kind {
+            ExpandKind::Weighted(_) => available_space.map(|lim| ConstraintLimit::Fill(lim.max())),
+            ExpandKind::Horizontal => Size::new(
+                ConstraintLimit::Fill(available_space.width.max()),
+                ConstraintLimit::SizeToFit(available_space.height.max()),
+            ),
+            ExpandKind::Vertical => Size::new(
+                ConstraintLimit::SizeToFit(available_space.width.max()),
+                ConstraintLimit::Fill(available_space.height.max()),
+            ),
+        };
         let child = self.child.mounted(&mut context.as_event_context());
         let size = context.for_other(&child).layout(available_space);
 

src/widgets/virtual_list.rs

@@ -1,22 +1,38 @@
-use std::{collections::VecDeque, fmt::Debug, ops::Range};
+use std::collections::VecDeque;
+use std::fmt::Debug;
+use std::ops::Range;
 
-use crate::{context::{AsEventContext, EventContext}, figures::{units::{Px, UPx}, IntoSigned, Point, Rect, Round, ScreenScale, Size, Zero}, kludgine::app::winit::{event::{ MouseScrollDelta, TouchPhase}, window::CursorIcon}, styles::Dimension, value::{Destination, Dynamic, DynamicReader, IntoDynamic, IntoValue, Source}, widget::{EventHandling, MakeWidget, MountedWidget, Widget, HANDLED, IGNORED}, widgets::scroll::ScrollBar, window::DeviceId, ConstraintLimit};
+use cushy::context::LayoutContext;
+use cushy::ConstraintLimit;
+use figures::UnscaledUnit;
 
 use super::scroll::OwnedWidget;
+use crate::context::{AsEventContext, EventContext};
+use crate::figures::units::{Px, UPx};
+use crate::figures::{IntoSigned, Point, Rect, Round, ScreenScale, Size, Zero};
+use crate::kludgine::app::winit::event::{MouseScrollDelta, TouchPhase};
+use crate::kludgine::app::winit::window::CursorIcon;
+use crate::value::{Destination, Dynamic, DynamicReader, IntoDynamic, IntoValue, Source};
+use crate::widget::{
+    Callback, EventHandling, MakeWidget, MountedWidget, Widget, WidgetInstance, HANDLED, IGNORED,
+};
+use crate::widgets::scroll::ScrollBar;
+use crate::window::DeviceId;
 
-/// A virtual list contents.
+#[derive(Debug)]
-/// This simple virtual list assumes that all items have the same height, width and that the item count is known.
+struct RowMaker(Callback<usize, WidgetInstance>);
-/// All the values are dynamic, so the list will update when the values change.
+
-pub trait VirtualListContent: Debug {
+impl RowMaker {
-    /// Single item height
+    fn make_row(
-    fn item_height(&self) -> impl IntoValue<Dimension>;
+        &mut self,
-    /// Width of the items
+        index: usize,
-    fn width(&self) -> impl IntoValue<Dimension>;
+        context: &mut LayoutContext<'_, '_, '_, '_>,
-    /// Number of items
+    ) -> VirtualListItem {
-    fn item_count(&self) -> impl IntoValue<usize>;
+        VirtualListItem {
-    /// Create a widget for the item at the given index.
+            index,
-    /// This is called when the widget comes into view. The widget may be removed at any moment (by scrolling it out of view) and recreated later.
+            mounted: context.push_child(self.0.invoke(index)),
-    fn widget_at(&self, index: usize) -> impl MakeWidget;
+        }
+    }
 }
 
 #[derive(Debug)]
@@ -26,37 +42,47 @@ struct VirtualListItem {
 }
 
 #[derive(Debug)]
-/// A virtual list widget.
+/// A virtuallized list view
-/// Requires a [VirtualListContent] trait implementation to render the items.
+///
-/// Items are lazily recreated as they go in and out of view.
+/// This widget allows scrolling a list of rows by lazily loading only the rows
-pub struct VirtualList<T: VirtualListContent + Send + 'static> {
+/// that are currently being displayed to the screen.
-    virtual_list: T,
+pub struct VirtualList {
+    make_row: RowMaker,
     vertical_scroll: OwnedWidget<ScrollBar>,
     items: VecDeque<VirtualListItem>,
     content_size: Dynamic<Size<UPx>>,
-    /// Maximum scroll value - max_scroll.y + control_size.height should be the height of the content.
+    /// Maximum scroll value - `max_scroll.y` + `control_size.height` should be the height of the content.
     pub max_scroll: DynamicReader<Point<UPx>>,
-    /// Current scroll value. The x value is always 0. Change the value to scroll the widget programmatically.
+    /// Current scroll value. Changes to this dynamic will scroll the list
+    /// programmatically.
     pub scroll: Dynamic<Point<UPx>>,
     control_size: Dynamic<Size<UPx>>,
 
-    /// Height of an item. Based on [VirtualListContent::item_height].
+    item_count: DynamicReader<usize>,
-    pub item_height: DynamicReader<Dimension>,
+    item_size: Dynamic<Size<UPx>>,
-    /// Width of the items. Based on [VirtualListContent::width].
-    pub width: DynamicReader<Dimension>,
-    /// Number of items. Based on [VirtualListContent::item_count].
-    pub item_count: DynamicReader<usize>,
 
-    visible_range: Dynamic<Range<usize>>
+    visible_range: Dynamic<Range<usize>>,
 }
 
-impl<T: VirtualListContent + Send + 'static> VirtualList<T> {
+impl VirtualList {
-    /// Creates a new [VirtualList] based on the given [VirtualListContent].
+    /// Creates a new [`VirtualList`] that displays `item_count` rows, loading
-    pub fn new(virtual_list: T) -> Self {
+    /// each row as needed by invoking `make_row`.
+    ///
+    /// `make_row` will be called each time a new row becomes visible. As rows
+    /// are no longer visible, they will be freed, ensuring a minimum number of
+    /// widgets is kept in memory at any given time.
+    ///
+    /// Each row will be sized to match the first visible row. To ensure all
+    /// rows have a consistent size, use the [`Resize`](../Resize) widget.
+    pub fn new<MakeRow, Row>(item_count: impl IntoValue<usize>, mut make_row: MakeRow) -> Self
+    where
+        MakeRow: FnMut(usize) -> Row + Send + 'static,
+        Row: MakeWidget,
+    {
+        let make_row = RowMaker(Callback::new(move |row| make_row(row).make_widget()));
         let scroll = Dynamic::<Point<UPx>>::default();
-        let item_height = virtual_list.item_height().into_value().into_dynamic().create_reader();
+        let item_size = Dynamic::new(Size::ZERO);
-        let width = virtual_list.width().into_value().into_dynamic().create_reader();
+        let item_count = item_count.into_value().into_dynamic().into_reader();
-        let item_count = virtual_list.item_count().into_value().into_dynamic().create_reader();
         let content_size = Dynamic::new(Size::default());
 
         let y = scroll.map_each_cloned(|scroll| scroll.y);
@@ -71,14 +97,14 @@ impl<T: VirtualListContent + Send + 'static> VirtualList<T> {
             }
         })
         .persist();
-        let vertical =
+        let vertical = ScrollBar::new(content_size.map_each_cloned(|size| size.height), y, true);
-            ScrollBar::new(content_size.map_each_cloned(|size| size.height), y, true);
+        let max_scroll = vertical
-        let max_scroll = (&vertical.max_scroll())
+            .max_scroll()
             .map_each_cloned(|y| Point::new(UPx::ZERO, y))
             .into_reader();
 
         Self {
-            virtual_list,
+            make_row,
             vertical_scroll: OwnedWidget::new(vertical),
             items: VecDeque::new(),
             control_size: Dynamic::new(Size::default()),
@@ -86,10 +112,9 @@ impl<T: VirtualListContent + Send + 'static> VirtualList<T> {
             max_scroll,
             scroll,
 
-            item_height,
+            item_size,
-            width,
             item_count,
-            visible_range: Default::default()
+            visible_range: Dynamic::default(),
         }
     }
 
@@ -134,9 +159,140 @@ impl<T: VirtualListContent + Send + 'static> VirtualList<T> {
             .expect("a ScrollBar")
             .hide(context);
     }
+
+    fn layout_rows(
+        &mut self,
+        item_count: usize,
+        available_space: Size<ConstraintLimit>,
+        context: &mut LayoutContext<'_, '_, '_, '_>,
+    ) -> Size<UPx> {
+        let mut item_size = self.calculate_item_size(available_space, context).ceil();
+
+        let content_height = item_size.height * u32::try_from(item_count).unwrap_or(u32::MAX);
+        let content_height = content_height.into_upx(context.gfx.scale());
+
+        let new_control_size = Size::new(
+            available_space.width.fill_or_fit(item_size.width),
+            available_space.height.fill_or_fit(content_height),
+        )
+        .ceil();
+        if item_size.width < new_control_size.width {
+            item_size.width = new_control_size.width;
+        }
+
+        let vertical = self
+            .vertical_scroll
+            .make_if_needed()
+            .mounted(&mut context.as_event_context());
+        let scrollbar_layout = context.for_other(&vertical).layout(available_space);
+        context.set_child_layout(
+            &vertical,
+            Rect::new(
+                Point::new(
+                    available_space
+                        .width
+                        .fit_measured(new_control_size.width)
+                        .saturating_sub(scrollbar_layout.width)
+                        .into_signed(),
+                    Px::ZERO,
+                ),
+                scrollbar_layout.into_signed(),
+            ),
+        );
+        let scroll = self.scroll.get_tracking_invalidate(context);
+
+        let start_item = (scroll.y.floor() / item_size.height).floor().get() as usize;
+        let end_item = ((scroll.y.ceil() + new_control_size.height) / item_size.height)
+            .ceil()
+            .get() as usize;
+        let end_item = end_item.min(item_count - 1);
+
+        self.visible_range.set(start_item..end_item);
+
+        let first = self.items.front().map(|t| t.index);
+        let last = self.items.back().map(|t| t.index);
+
+        if self.items.is_empty() || first.unwrap() > end_item || last.unwrap() < start_item {
+            for item in self.items.drain(..) {
+                context.remove_child(&item.mounted);
+            }
+            self.items.extend(
+                (start_item..=end_item).map(|index| self.make_row.make_row(index, context)),
+            );
+        } else {
+            let first = first.expect("List is not empty");
+            let last = last.expect("List is not empty");
+            while self
+                .items
+                .front()
+                .map_or(false, |item| item.index < start_item)
+            {
+                context.remove_child(&self.items.pop_front().expect("at least one item").mounted);
+            }
+            while self
+                .items
+                .back()
+                .map_or(false, |item| item.index > end_item)
+            {
+                self.items.pop_back();
+            }
+            // no extend front :(
+            for item in (start_item..first).rev() {
+                self.items.push_front(self.make_row.make_row(item, context));
+            }
+            self.items.extend(
+                ((last + 1)..=end_item).map(|index| self.make_row.make_row(index, context)),
+            );
+        }
+
+        // TODO add % to Figures
+        let mut y =
+            -UPx::from_unscaled(scroll.y.into_unscaled() % item_size.height.into_unscaled())
+                .into_signed();
+        let constraint = item_size.map(ConstraintLimit::SizeToFit);
+        for item in &self.items {
+            let child_size = context.for_other(&item.mounted).layout(constraint);
+
+            context.set_child_layout(
+                &item.mounted,
+                Rect::new(
+                    Point::new(Px::ZERO, y),
+                    item_size.min(child_size).into_signed(),
+                ),
+            );
+            y += item_size.height.into_signed();
+        }
+
+        self.control_size.set(new_control_size);
+        self.content_size
+            .set(Size::new(item_size.width, content_height));
+        self.item_size.set(item_size);
+
+        new_control_size
+    }
+
+    fn calculate_item_size(
+        &mut self,
+        available_space: Size<ConstraintLimit>,
+        context: &mut LayoutContext<'_, '_, '_, '_>,
+    ) -> Size<UPx> {
+        if self.items.is_empty() {
+            self.items.push_front(self.make_row.make_row(0, context));
+        }
+
+        context
+            .for_other(
+                &self
+                    .items
+                    .front()
+                    .expect("at least one mounted item")
+                    .mounted,
+            )
+            .layout(available_space.map(|space| ConstraintLimit::SizeToFit(space.max())))
+    }
 }
 
-impl<T: VirtualListContent + Send + 'static> Widget for VirtualList<T> {
+impl Widget for VirtualList {
     fn hit_test(&mut self, _location: Point<Px>, _context: &mut EventContext<'_>) -> bool {
         true
     }
@@ -173,104 +329,16 @@ impl<T: VirtualListContent + Send + 'static> Widget for VirtualList<T> {
     }
 
     fn layout(
-            &mut self,
+        &mut self,
-            available_space: Size<cushy::ConstraintLimit>,
+        available_space: Size<ConstraintLimit>,
-            context: &mut cushy::context::LayoutContext<'_, '_, '_, '_>,
+        context: &mut LayoutContext<'_, '_, '_, '_>,
-        ) -> Size<UPx> {
+    ) -> Size<UPx> {
-        let item_height = self.item_height.get_tracking_invalidate(context);
-        let item_height_upx = item_height.into_upx(context.gfx.scale());
         let item_count = self.item_count.get_tracking_invalidate(context);
-        let content_height = item_height * item_count as i32;
+        if item_count == 0 {
-        let content_height = content_height.into_upx(context.gfx.scale());
+            return available_space.map(ConstraintLimit::min);
-        let width = self.width.get_tracking_invalidate(context);
-        let width = width.into_upx(context.gfx.scale());
-
-        let new_control_size = Size::new(
-            width,
-            constrain_child(available_space.height, content_height),
-        );
-
-        let vertical = self
-            .vertical_scroll
-            .make_if_needed()
-            .mounted(&mut context.as_event_context());
-        let scrollbar_layout = context.for_other(&vertical).layout(available_space);
-        context.set_child_layout(
-            &vertical,
-            Rect::new(
-                Point::new(
-                    available_space.width
-                        .fit_measured(new_control_size.width)
-                        .saturating_sub(scrollbar_layout.width)
-                        .into_signed(),
-                    Px::ZERO,
-                ),
-                scrollbar_layout.into_signed(),
-            ),
-        );
-        let scroll = self.scroll.get_tracking_invalidate(context);
-
-        let start_item = (scroll.y / item_height_upx).floor().get() as usize;
-        let end_item = ((scroll.y + new_control_size.height) / item_height_upx).ceil().get() as usize;
-        let end_item = end_item.min(item_count-1);
-
-        self.visible_range.set(start_item..end_item);
-        
-        let first = self.items.front().map(|t| t.index);
-        let last = self.items.back().map(|t| t.index);
-        let mut closure = |index| {
-            let widget = self.virtual_list.widget_at(index);
-            let mut widget = widget.widget_ref();
-            let mounted = widget.mounted(&mut context.as_event_context());
-            VirtualListItem { index, mounted }
-        };
-        if self.items.is_empty() || first.unwrap() > end_item || last.unwrap() < start_item {
-            self.items.clear();
-            self.items.extend((start_item..=end_item).map(closure));
-        } else {
-            let first = first.expect("List is not empty");
-            let last = last.expect("List is not empty");
-            if first < start_item {
-                while self.items.front().is_some() && self.items.front().expect("Checked is some").index < start_item {
-                    self.items.pop_front();
-                }
-            }
-            if last > end_item {
-                while self.items.back().is_some() && self.items.back().expect("Checked is some").index > end_item {
-                    self.items.pop_back();
-                }
-            }
-            // no extend front :(
-            for item in (start_item..first).rev() {
-                self.items.push_front(closure(item));
-            }
-            self.items.extend(((last+1)..=end_item).map(closure));
         }
 
-        let item_size = Size::new(width, item_height_upx);
+        self.layout_rows(item_count, available_space, context)
-        let constraint = item_size.map(ConstraintLimit::Fill);
-
-        for item in &self.items {
-            context.for_other(&item.mounted).layout(constraint);
-        }
-
-        let item_size = item_size.into_signed();
-        let scroll = self.scroll.get_tracking_invalidate(context).into_signed();
-
-        for item in &self.items {
-            context.set_child_layout(
-                &item.mounted,
-                Rect::new(
-                    Point::new(Px::ZERO, (item_height_upx * item.index as f32).into_signed() - scroll.y),
-                    item_size,
-                )
-            );
-        }
-        
-        self.control_size.set(new_control_size);
-        self.content_size.set(Size::new(width, content_height));
-
-        new_control_size
     }
 
     fn mouse_wheel(
@@ -299,11 +367,3 @@ impl<T: VirtualListContent + Send + 'static> Widget for VirtualList<T> {
         }
     }
 }
-
-fn constrain_child(constraint: ConstraintLimit, measured: UPx) -> UPx {
-    match constraint {
-        ConstraintLimit::Fill(size) => size.min(measured),
-        // change from Scroll widget: returning just measured here would break the functionality (render too many items)
-        ConstraintLimit::SizeToFit(size) => size.min(measured),
-    }
-}