From acc1405c2aa2dbc4ac60c47f4d8b7bd4916636dc Mon Sep 17 00:00:00 2001 From: Alexandre Bury Date: Mon, 20 Aug 2018 13:30:42 -0700 Subject: [PATCH] Fix size cache in ScrollView --- src/views/box_view.rs | 66 ++++++++++++++++++++++++++------------ src/views/button.rs | 13 ++++++++ src/views/dialog.rs | 22 +++++++++++++ src/views/dummy.rs | 2 ++ src/views/hideable_view.rs | 17 ++++++++++ src/views/scroll_view.rs | 4 ++- src/views/text_view.rs | 13 ++++---- 7 files changed, 110 insertions(+), 27 deletions(-) diff --git a/src/views/box_view.rs b/src/views/box_view.rs index 823e96c..c0538cd 100644 --- a/src/views/box_view.rs +++ b/src/views/box_view.rs @@ -1,3 +1,4 @@ +use With; use vec::Vec2; use view::{SizeConstraint, View, ViewWrapper}; use XY; @@ -27,8 +28,11 @@ pub struct BoxView { /// /// This means if the required size is less than the computed size, /// consider returning a smaller size. - /// For instance, try to return the child's desires size. - squishable: bool, + /// For instance, try to return the child's desired size. + squishable: bool, // TODO: remove? + + /// Set to `true` whenever we change some settings. Means we should re-layout just in case. + invalidated: bool, /// The actual view we're wrapping. view: T, @@ -44,6 +48,7 @@ impl BoxView { BoxView { size: (width, height).into(), squishable: false, + invalidated: true, view, } } @@ -61,6 +66,7 @@ impl BoxView { /// Leaves the height unchanged. pub fn set_width(&mut self, width: SizeConstraint) { self.size.x = width; + self.invalidate(); } /// Sets the height constraint for this view. @@ -68,6 +74,7 @@ impl BoxView { /// Leaves the width unchanged. pub fn set_height(&mut self, height: SizeConstraint) { self.size.y = height; + self.invalidate(); } /// Sets `self` to be squishable. @@ -78,9 +85,14 @@ impl BoxView { /// /// More specifically, if the available space is less than the size we /// would normally ask for, return the child size. - pub fn squishable(mut self) -> Self { - self.squishable = true; - self + pub fn squishable(self) -> Self { + self.with(|s| s.set_squishable(true)) + } + + /// Controls the "squishability" of `self`. + pub fn set_squishable(&mut self, squishable: bool) { + self.squishable = squishable; + self.invalidate(); } /// Wraps `view` in a new `BoxView` with the given size. @@ -185,6 +197,12 @@ impl BoxView { ) } + /// Should be called anytime something changes. + fn invalidate(&mut self) { + self.invalidated = true; + } + + inner_getters!(self.view: T); } @@ -192,34 +210,42 @@ impl ViewWrapper for BoxView { wrap_impl!(self.view: T); fn wrap_required_size(&mut self, req: Vec2) -> Vec2 { + // This is what the child will see as request. let req = self.size.zip_map(req, SizeConstraint::available); + + // This is the size the child would like to have. let child_size = self.view.required_size(req); + // Some of this request will be granted, but maybe not all. let result = self .size .zip_map(child_size.zip(req), SizeConstraint::result); debug!("{:?}", result); - if self.squishable { + if !self.squishable { + result + } else { + // When we're squishable, special behaviour: + // + // We respect the request if we're less or equal. let respect_req = result.zip_map(req, |res, req| res <= req); - result.zip_map( - respect_req.zip(child_size), - |res, (respect, child)| { - if respect { - // If we respect the request, keep the result - res - } else { - // Otherwise, take the child as squish attempt. - child - } - }, - ) - } else { - result + + // If we respect the request, keep the result + // Otherwise, take the child as squish attempt. + respect_req.select_or(result, child_size) } } + + fn wrap_layout(&mut self, size: Vec2) { + self.invalidated = false; + self.view.layout(size); + } + + fn wrap_needs_relayout(&self) -> bool { + self.invalidated || self.view.needs_relayout() + } } #[cfg(test)] diff --git a/src/views/button.rs b/src/views/button.rs index 5fd4788..8dfec22 100644 --- a/src/views/button.rs +++ b/src/views/button.rs @@ -23,6 +23,8 @@ pub struct Button { callback: Callback, enabled: bool, last_size: Vec2, + + invalidated: bool, } impl Button { @@ -46,6 +48,7 @@ impl Button { callback: Callback::from_fn(cb), enabled: true, last_size: Vec2::zero(), + invalidated: true, } } @@ -121,11 +124,16 @@ impl Button { S: Into, { self.label = label.into(); + self.invalidate(); } fn req_size(&self) -> Vec2 { Vec2::new(self.label.width(), 1) } + + fn invalidate(&mut self) { + self.invalidated = true; + } } impl View for Button { @@ -152,6 +160,7 @@ impl View for Button { fn layout(&mut self, size: Vec2) { self.last_size = size; + self.invalidated = false; } fn required_size(&mut self, _: Vec2) -> Vec2 { @@ -195,4 +204,8 @@ impl View for Button { Rect::from_size((offset, 0), (width, 1)) } + + fn needs_relayout(&self) -> bool { + self.invalidated + } } diff --git a/src/views/dialog.rs b/src/views/dialog.rs index 4477099..f66b7c3 100644 --- a/src/views/dialog.rs +++ b/src/views/dialog.rs @@ -75,6 +75,9 @@ pub struct Dialog { // How to align the buttons under the view. align: Align, + + // `true` when we needs to relayout + invalidated: bool, } new_default!(Dialog); @@ -98,6 +101,7 @@ impl Dialog { padding: Margins::new(1, 1, 0, 0), borders: Margins::new(1, 1, 1, 1), align: Align::top_right(), + invalidated: true, } } @@ -126,6 +130,7 @@ impl Dialog { /// Gets mutable access to the content. pub fn get_content_mut(&mut self) -> &mut View { + self.invalidate(); &mut *self.content.view } @@ -134,6 +139,7 @@ impl Dialog { /// Previous content will be dropped. pub fn set_content(&mut self, view: V) { self.content = SizedView::new(ViewBox::boxed(view)); + self.invalidate(); } /// Convenient method to create a dialog with a simple text content. @@ -164,6 +170,7 @@ impl Dialog { F: 'static + Fn(&mut Cursive), { self.buttons.push(ChildButton::new(label, cb)); + self.invalidate(); } /// Returns the number of buttons on this dialog. @@ -174,6 +181,7 @@ impl Dialog { /// Removes any button from `self`. pub fn clear_buttons(&mut self) { self.buttons.clear(); + self.invalidate(); } /// Removes a button from this dialog. @@ -183,6 +191,7 @@ impl Dialog { /// Panics if `i >= self.buttons_len()`. pub fn remove_button(&mut self, i: usize) { self.buttons.remove(i); + self.invalidate(); } /// Sets the horizontal alignment for the buttons, if any. @@ -224,6 +233,7 @@ impl Dialog { /// Sets the title of the dialog. pub fn set_title>(&mut self, label: S) { self.title = label.into(); + self.invalidate(); } /// Sets the horizontal position of the title in the dialog. @@ -271,6 +281,7 @@ impl Dialog { /// Returns an iterator on this buttons for this dialog. pub fn buttons_mut(&mut self) -> impl Iterator { + self.invalidate(); self.buttons.iter_mut().map(|b| &mut b.button.view) } @@ -504,6 +515,10 @@ impl Dialog { } } } + + fn invalidate(&mut self) { + self.invalidated = true; + } } impl View for Dialog { @@ -583,8 +598,11 @@ impl View for Dialog { if buttons_height > size.y { buttons_height = size.y; } + self.content .layout(size.saturating_sub((0, buttons_height))); + + self.invalidated = false; } fn on_event(&mut self, event: Event) -> EventResult { @@ -627,4 +645,8 @@ impl View for Dialog { + self.borders.top_left() + self.padding.top_left() } + + fn needs_relayout(&self) -> bool { + self.invalidated || self.content.needs_relayout() + } } diff --git a/src/views/dummy.rs b/src/views/dummy.rs index c59cca9..669a180 100644 --- a/src/views/dummy.rs +++ b/src/views/dummy.rs @@ -8,4 +8,6 @@ pub struct DummyView; impl View for DummyView { fn draw(&self, _: &Printer) {} + + fn needs_relayout(&self) -> bool { false } } diff --git a/src/views/hideable_view.rs b/src/views/hideable_view.rs index cab64b7..1286d98 100644 --- a/src/views/hideable_view.rs +++ b/src/views/hideable_view.rs @@ -1,4 +1,5 @@ use view::{Selector, View, ViewWrapper}; +use vec::Vec2; use With; use std::any::Any; @@ -14,6 +15,7 @@ use std::any::Any; pub struct HideableView { view: V, visible: bool, + invalidated: bool, } impl HideableView { @@ -24,12 +26,14 @@ impl HideableView { HideableView { view, visible: true, + invalidated: true, } } /// Sets the visibility for this view. pub fn set_visible(&mut self, visible: bool) { self.visible = visible; + self.invalidate(); } /// Sets the visibility for this view to `false`. @@ -49,6 +53,10 @@ impl HideableView { self.with(Self::hide) } + fn invalidate(&mut self) { + self.invalidated = true; + } + inner_getters!(self.view: V); } @@ -91,4 +99,13 @@ impl ViewWrapper for HideableView { { Ok(self.view) } + + fn wrap_layout(&mut self, size: Vec2) { + self.invalidated = false; + self.with_view_mut(|v| v.layout(size)); + } + + fn wrap_needs_relayout(&self) -> bool { + self.invalidated || (self.visible && self.view.needs_relayout()) + } } diff --git a/src/views/scroll_view.rs b/src/views/scroll_view.rs index b5865fe..4d612ea 100644 --- a/src/views/scroll_view.rs +++ b/src/views/scroll_view.rs @@ -323,11 +323,13 @@ where /// Returns `(inner_size, desired_size)` fn sizes(&mut self, constraint: Vec2, strict: bool) -> (Vec2, Vec2) { // First: try the cache - if self + if !self.inner.needs_relayout() && self .size_cache .map(|cache| cache.zip_map(constraint, SizeCache::accept).both()) .unwrap_or(false) { + // eprintln!("Cache: {:?}; constraint: {:?}", self.size_cache, constraint); + // The new constraint shouldn't change much, // so we can re-use previous values return ( diff --git a/src/views/text_view.rs b/src/views/text_view.rs index 2992d7f..ee158cb 100644 --- a/src/views/text_view.rs +++ b/src/views/text_view.rs @@ -336,12 +336,6 @@ impl TextView { // Desired width self.width = self.rows.iter().map(|row| row.width).max(); - - // The entire "virtual" size (includes all rows) - let my_size = Vec2::new(self.width.unwrap_or(0), self.rows.len()); - - // Build a fresh cache. - content.size_cache = Some(SizeCache::build(my_size, size)); } // Invalidates the cache, so next call will recompute everything. @@ -390,5 +384,12 @@ impl View for TextView { // Compute the text rows. self.last_size = size; self.compute_rows(size); + + // The entire "virtual" size (includes all rows) + let my_size = Vec2::new(self.width.unwrap_or(0), self.rows.len()); + + // Build a fresh cache. + let mut content = self.content.lock().unwrap(); + content.size_cache = Some(SizeCache::build(my_size, size)); } }