Fix scroll::core size handling

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.
+    /// Excludes any potential scrollbar.
-    last_size: Vec2,
+    last_available: Vec2,
 
     /// Are we scrollable in each direction?
     enabled: XY<bool>,
@@ -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<XY<SizeCache>>,
+    size_cache: Option<XY<SizeCache<bool>>>,
 
     /// 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) {
+    pub(crate) fn set_last_size(
-        self.last_size = last_size;
+        &mut self,
-    }
+        last_size: Vec2,
-
+        scrolling: XY<bool>,
-    /// Returns the size last given in `set_last_size()`.
+    ) {
-    pub fn last_size(&self) -> Vec2 {
+        self.last_available = last_size.saturating_sub(
-        self.last_size
+            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) {
+    pub(crate) fn build_cache(
-        self.size_cache = Some(SizeCache::build(self_size, last_size));
+        &mut self,
+        self_size: Vec2,
+        last_size: Vec2,
+        scrolling: XY<bool>,
+    ) {
+        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 {
+        if x >= self.offset.x + self.available_size().x {
-            self.offset.x = 1 + x - self.last_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 {
+        if y >= self.offset.y + self.available_size().y {
-            self.offset.y = 1 + y - self.last_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<bool> {
-        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_available
-            self.last_size.saturating_sub(self.scrollbar_size())
-        } else {
-            self.last_size
     }
+
+    /// 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<bool>)> {
         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
             }

cursive-core/src/view/scroll/raw.rs

@@ -92,7 +92,7 @@ fn sizes<Model, GetScroller, RequiredSize>(
     model: &mut Model,
     get_scroller: &mut GetScroller,
     required_size: &mut RequiredSize,
-) -> (Vec2, Vec2)
+) -> (Vec2, Vec2, XY<bool>)
 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<Model, GetScroller, RequiredSize, 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<Model, GetScroller, RequiredSize, 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,

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<T = ()> {
     /// Cached value
     pub value: usize,
     /// `true` if the last size was constrained.
@@ -13,29 +13,19 @@ 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 }
+        SizeCache {
+            value,
+            constrained,
+            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
     }
 
     /// Creates a new bi-dimensional cache.
@@ -56,3 +46,40 @@ impl SizeCache {
         size.zip_map(req, |size, req| SizeCache::new(size, size >= req))
     }
 }
+
+impl<T> SizeCache<T> {
+    /// 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<T>) -> XY<Self> {
+        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
+    }
+}

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| {

cursive-core/src/views/scroll_view.rs

@@ -192,7 +192,8 @@ impl<V> ScrollView<V> {
     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()