From db9a3c32b1df5e2c2ec51f0668d69e22db61265a Mon Sep 17 00:00:00 2001 From: Alexandre Bury Date: Mon, 6 Jul 2020 10:23:49 -0700 Subject: [PATCH] Fix scroll::core size handling --- cursive-core/src/view/scroll/core.rs | 75 ++++++++++++++++----------- cursive-core/src/view/scroll/raw.rs | 18 +++---- cursive-core/src/view/size_cache.rs | 65 ++++++++++++++++------- cursive-core/src/views/menu_popup.rs | 2 +- cursive-core/src/views/scroll_view.rs | 3 +- 5 files changed, 103 insertions(+), 60 deletions(-) diff --git a/cursive-core/src/view/scroll/core.rs b/cursive-core/src/view/scroll/core.rs index 1d3deb5..c5944b8 100644 --- a/cursive-core/src/view/scroll/core.rs +++ b/cursive-core/src/view/scroll/core.rs @@ -71,10 +71,10 @@ pub struct Core { /// Our `(0,0)` will be inner's `offset` offset: Vec2, - /// What was our own size last time we checked. + /// What was the size available to print the child last time? /// - /// This includes scrollbars, if any. - last_size: Vec2, + /// Excludes any potential scrollbar. + last_available: Vec2, /// Are we scrollable in each direction? enabled: XY, @@ -97,7 +97,7 @@ pub struct Core { thumb_grab: Option<(Orientation, usize)>, /// We keep the cache here so it can be busted when we change the content. - size_cache: Option>, + size_cache: Option>>, /// Defines how to update the offset when the view size changes. scroll_strategy: ScrollStrategy, @@ -115,7 +115,7 @@ impl Core { Core { inner_size: Vec2::zero(), offset: Vec2::zero(), - last_size: Vec2::zero(), + last_available: Vec2::zero(), enabled: XY::new(false, true), show_scrollbars: true, scrollbar_padding: Vec2::new(1, 0), @@ -345,13 +345,16 @@ impl Core { } /// Specifies the size given in a layout phase. - pub(crate) fn set_last_size(&mut self, last_size: Vec2) { - self.last_size = last_size; - } - - /// Returns the size last given in `set_last_size()`. - pub fn last_size(&self) -> Vec2 { - self.last_size + pub(crate) fn set_last_size( + &mut self, + last_size: Vec2, + scrolling: XY, + ) { + self.last_available = last_size.saturating_sub( + scrolling + .swap() + .select_or(self.scrollbar_padding + (1, 1), Vec2::zero()), + ); } /// Specifies the size allocated to the content. @@ -360,8 +363,14 @@ impl Core { } /// Rebuild the cache with the given parameters. - pub(crate) fn build_cache(&mut self, self_size: Vec2, last_size: Vec2) { - self.size_cache = Some(SizeCache::build(self_size, last_size)); + pub(crate) fn build_cache( + &mut self, + self_size: Vec2, + last_size: Vec2, + scrolling: XY, + ) { + self.size_cache = + Some(SizeCache::build_extra(self_size, last_size, scrolling)); } /// Makes sure the viewport is within the content. @@ -532,7 +541,7 @@ impl Core { /// Try to keep the given `rect` in view. pub fn keep_in_view(&mut self, rect: Rect) { - let min = rect.bottom_right().saturating_sub(self.last_size); + let min = rect.bottom_right().saturating_sub(self.available_size()); let max = rect.top_left(); let (min, max) = (Vec2::min(min, max), Vec2::max(min, max)); @@ -558,7 +567,7 @@ impl Core { /// Scroll until the given point is visible. pub fn scroll_to(&mut self, pos: Vec2) { // The furthest top-left we can go - let min = pos.saturating_sub(self.last_size); + let min = pos.saturating_sub(self.available_size()); // How far to the bottom-right we can go let max = pos; @@ -567,8 +576,8 @@ impl Core { /// Scroll until the given column is visible. pub fn scroll_to_x(&mut self, x: usize) { - if x >= self.offset.x + self.last_size.x { - self.offset.x = 1 + x - self.last_size.x; + if x >= self.offset.x + self.available_size().x { + self.offset.x = 1 + x - self.available_size().x; } else if x < self.offset.x { self.offset.x = x; } @@ -576,8 +585,8 @@ impl Core { /// Scroll until the given row is visible. pub fn scroll_to_y(&mut self, y: usize) { - if y >= self.offset.y + self.last_size.y { - self.offset.y = 1 + y - self.last_size.y; + if y >= self.offset.y + self.available_size().y { + self.offset.y = 1 + y - self.available_size().y; } else if y < self.offset.y { self.offset.y = y; } @@ -616,7 +625,7 @@ impl Core { /// Returns for each axis if we are scrolling. pub fn is_scrolling(&self) -> XY { - self.inner_size.zip_map(self.last_size, |i, s| i > s) + self.inner_size.zip_map(self.available_size(), |i, s| i > s) } /// Stops grabbing the scrollbar. @@ -637,11 +646,12 @@ impl Core { /// Returns the size available for the child view. fn available_size(&self) -> Vec2 { - if self.show_scrollbars { - self.last_size.saturating_sub(self.scrollbar_size()) - } else { - self.last_size - } + self.last_available + } + + /// Returns the last size given by `layout`. + pub fn last_outer_size(&self) -> Vec2 { + self.available_size() + self.scrollbar_size() } /// Starts scrolling from the cursor position. @@ -649,7 +659,7 @@ impl Core { /// Returns `true` if the event was consumed. fn start_drag(&mut self, position: Vec2) -> bool { // For each scrollbar, how far it is. - let scrollbar_pos = self.last_size.saturating_sub((1, 1)); + let scrollbar_pos = self.last_outer_size().saturating_sub((1, 1)); let lengths = self.scrollbar_thumb_lengths(); let offsets = self.scrollbar_thumb_offsets(lengths); let available = self.available_size(); @@ -725,10 +735,17 @@ impl Core { /// Tries to apply the cache to the current constraint. /// /// Returns the cached value if it works, or `None`. - pub(crate) fn try_cache(&self, constraint: Vec2) -> Option<(Vec2, Vec2)> { + pub(crate) fn try_cache( + &self, + constraint: Vec2, + ) -> Option<(Vec2, Vec2, XY)> { self.size_cache.and_then(|cache| { if cache.zip_map(constraint, SizeCache::accept).both() { - Some((self.inner_size, cache.map(|c| c.value))) + Some(( + self.inner_size, + cache.map(|c| c.value), + cache.map(|c| c.extra), + )) } else { None } diff --git a/cursive-core/src/view/scroll/raw.rs b/cursive-core/src/view/scroll/raw.rs index c74e087..c8c18b7 100644 --- a/cursive-core/src/view/scroll/raw.rs +++ b/cursive-core/src/view/scroll/raw.rs @@ -92,7 +92,7 @@ fn sizes( model: &mut Model, get_scroller: &mut GetScroller, required_size: &mut RequiredSize, -) -> (Vec2, Vec2) +) -> (Vec2, Vec2, XY) where Model: ?Sized, GetScroller: FnMut(&mut Model) -> &mut scroll::Core, @@ -128,7 +128,7 @@ where ); if scrolling == new_scrolling { // Yup, scrolling did it. We're good to go now. - (inner_size, size) + (inner_size, size, scrolling) } else { // Again? We're now scrolling in a new direction? // There is no end to this! @@ -143,12 +143,12 @@ where // That's enough. If the inner view changed again, ignore it! // That'll teach it. - (inner_size, size) + (inner_size, size, new_scrolling) } } else { // We're not showing any scrollbar, either because we don't scroll // or because scrollbars are hidden. - (inner_size, size) + (inner_size, size, scrolling) } } @@ -166,10 +166,8 @@ pub fn layout( RequiredSize: FnMut(&mut Model, Vec2) -> Vec2, Layout: FnMut(&mut Model, Vec2), { - get_scroller(model).set_last_size(size); - // This is what we'd like - let (inner_size, self_size) = sizes( + let (inner_size, self_size, scrolling) = sizes( size, true, needs_relayout, @@ -177,9 +175,9 @@ pub fn layout( &mut get_scroller, &mut required_size, ); - + get_scroller(model).set_last_size(self_size, scrolling); get_scroller(model).set_inner_size(inner_size); - get_scroller(model).build_cache(self_size, size); + get_scroller(model).build_cache(self_size, size, scrolling); layout(model, inner_size); @@ -199,7 +197,7 @@ where GetScroller: FnMut(&mut Model) -> &mut scroll::Core, RequiredSize: FnMut(&mut Model, Vec2) -> Vec2, { - let (_, size) = sizes( + let (_, size, _) = sizes( constraint, false, needs_relayout, diff --git a/cursive-core/src/view/size_cache.rs b/cursive-core/src/view/size_cache.rs index 429716f..0a8417a 100644 --- a/cursive-core/src/view/size_cache.rs +++ b/cursive-core/src/view/size_cache.rs @@ -5,7 +5,7 @@ use crate::XY; /// /// This is not a View, but something to help you if you create your own Views. #[derive(PartialEq, Debug, Clone, Copy)] -pub struct SizeCache { +pub struct SizeCache { /// Cached value pub value: usize, /// `true` if the last size was constrained. @@ -13,31 +13,21 @@ pub struct SizeCache { /// If unconstrained, any request larger than this value /// would return the same size. pub constrained: bool, + + /// Extra field. + pub extra: T, } -impl SizeCache { +impl SizeCache<()> { /// Creates a new sized cache pub fn new(value: usize, constrained: bool) -> Self { - SizeCache { value, constrained } - } - - /// Returns `true` if `self` is still valid for the given `request`. - pub fn accept(self, request: usize) -> bool { - match (request, self.value) { - // Request a smaller size than last time? Hell no! - (r, v) if r < v => false, - // Request exactly what we had last time? Sure! - (r, v) if r == v => true, - // Request more than we had last time? Maybe? - _ => !self.constrained, + SizeCache { + value, + constrained, + extra: (), } } - /// Returns the value in the cache. - pub fn value(self) -> usize { - self.value - } - /// Creates a new bi-dimensional cache. /// /// It will stay valid for the same request, and compatible ones. @@ -56,3 +46,40 @@ impl SizeCache { size.zip_map(req, |size, req| SizeCache::new(size, size >= req)) } } + +impl SizeCache { + /// Creates a new sized cache + pub fn new_extra(value: usize, constrained: bool, extra: T) -> Self { + Self { + value, + constrained, + extra, + } + } + + /// Creates a new bi-dimensional cache. + /// + /// Similar to `build()`, but includes the extra field. + pub fn build_extra(size: Vec2, req: Vec2, extra: XY) -> XY { + XY::zip3(size, req, extra).map(|(size, req, extra)| { + SizeCache::new_extra(size, size >= req, extra) + }) + } + + /// Returns `true` if `self` is still valid for the given `request`. + pub fn accept(self, request: usize) -> bool { + match (request, self.value) { + // Request a smaller size than last time? Hell no! + (r, v) if r < v => false, + // Request exactly what we had last time? Sure! + (r, v) if r == v => true, + // Request more than we had last time? Maybe? + _ => !self.constrained, + } + } + + /// Returns the value in the cache. + pub fn value(self) -> usize { + self.value + } +} diff --git a/cursive-core/src/views/menu_popup.rs b/cursive-core/src/views/menu_popup.rs index c29feb6..1047491 100644 --- a/cursive-core/src/views/menu_popup.rs +++ b/cursive-core/src/views/menu_popup.rs @@ -397,7 +397,7 @@ impl View for MenuPopup { // Mouse clicks outside of the popup should dismiss it. if !position.fits_in_rect( offset, - self.scroll_core.last_size() + (2, 2), + self.scroll_core.last_outer_size() + (2, 2), ) { let dismiss_cb = self.on_dismiss.clone(); return EventResult::with_cb(move |s| { diff --git a/cursive-core/src/views/scroll_view.rs b/cursive-core/src/views/scroll_view.rs index 8f2955b..4c59d27 100644 --- a/cursive-core/src/views/scroll_view.rs +++ b/cursive-core/src/views/scroll_view.rs @@ -192,7 +192,8 @@ impl ScrollView { where V: View, { - let important_area = self.inner.important_area(self.core.last_size()); + let important_area = + self.inner.important_area(self.core.last_outer_size()); self.core.scroll_to_rect(important_area); self.on_scroll_callback()