Merge pull request #1524 from Textualize/scroll-optimization

Scroll optimization

CHANGELOG.md

@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 - `MouseScrollUp` and `MouseScrollDown` now inherit from `MouseEvent` and have attached modifier keys. https://github.com/Textualize/textual/pull/1458
 - Fail-fast and print pretty tracebacks for Widget compose errors https://github.com/Textualize/textual/pull/1505
+- Added Widget._refresh_scroll to avoid expensive layout when scrolling https://github.com/Textualize/textual/pull/1524
 
 ### Fixed
 

src/textual/_compositor.py

@@ -253,8 +253,8 @@ def reflow(self, parent: Widget, size: Size) -> ReflowResult:
         # Keep a copy of the old map because we're going to compare it with the update
         old_map = self.map.copy()
         old_widgets = old_map.keys()
-        map, widgets = self._arrange_root(parent, size)
 
+        map, widgets = self._arrange_root(parent, size)
         new_widgets = map.keys()
 
         # Newly visible widgets
@@ -266,20 +266,21 @@ def reflow(self, parent: Widget, size: Size) -> ReflowResult:
         self.map = map
         self.widgets = widgets
 
-        screen = size.region
+        # Contains widgets + geometry for every widget that changed (added, removed, or updated)
+        changes = map.items() ^ old_map.items()
+
+        # Widgets in both new and old
+        common_widgets = old_widgets & new_widgets
 
         # Widgets with changed size
         resized_widgets = {
             widget
-            for widget, (region, *_) in map.items()
-            if widget in old_widgets and old_map[widget].region.size != region.size
+            for widget, (region, *_) in changes
+            if (widget in common_widgets and old_map[widget].region[2:] != region[2:])
         }
 
-        # Gets pairs of tuples of (Widget, MapGeometry) which have changed
-        # i.e. if something is moved / deleted / added
-
-        if screen not in self._dirty_regions:
-            changes = map.items() ^ old_map.items()
+        screen_region = size.region
+        if screen_region not in self._dirty_regions:
             regions = {
                 region
                 for region in (
@@ -346,6 +347,7 @@ def add_widget(
             layer_order: int,
             clip: Region,
             visible: bool,
+            _MapGeometry=MapGeometry,
         ) -> None:
             """Called recursively to place a widget and its children in the map.
 
@@ -392,50 +394,57 @@ def add_widget(
                     )
                     widgets.update(arranged_widgets)
 
-                    # An offset added to all placements
-                    placement_offset = container_region.offset
-                    placement_scroll_offset = placement_offset - widget.scroll_offset
-
-                    _layers = widget.layers
-                    layers_to_index = {
-                        layer_name: index for index, layer_name in enumerate(_layers)
-                    }
-                    get_layer_index = layers_to_index.get
+                    if placements:
+                        # An offset added to all placements
+                        placement_offset = container_region.offset
+                        placement_scroll_offset = (
+                            placement_offset - widget.scroll_offset
+                        )
 
-                    # Add all the widgets
-                    for sub_region, margin, sub_widget, z, fixed in reversed(
-                        placements
-                    ):
-                        # Combine regions with children to calculate the "virtual size"
-                        if fixed:
-                            widget_region = sub_region + placement_offset
-                        else:
-                            total_region = total_region.union(
-                                sub_region.grow(spacing + margin)
+                        _layers = widget.layers
+                        layers_to_index = {
+                            layer_name: index
+                            for index, layer_name in enumerate(_layers)
+                        }
+                        get_layer_index = layers_to_index.get
+
+                        # Add all the widgets
+                        for sub_region, margin, sub_widget, z, fixed in reversed(
+                            placements
+                        ):
+                            # Combine regions with children to calculate the "virtual size"
+                            if fixed:
+                                widget_region = sub_region + placement_offset
+                            else:
+                                total_region = total_region.union(
+                                    sub_region.grow(spacing + margin)
+                                )
+                                widget_region = sub_region + placement_scroll_offset
+
+                            widget_order = (
+                                *order,
+                                get_layer_index(sub_widget.layer, 0),
+                                z,
+                                layer_order,
                             )
-                            widget_region = sub_region + placement_scroll_offset
-
-                        widget_order = order + (
-                            (get_layer_index(sub_widget.layer, 0), z, layer_order),
-                        )
 
-                        add_widget(
-                            sub_widget,
-                            sub_region,
-                            widget_region,
-                            widget_order,
-                            layer_order,
-                            sub_clip,
-                            visible,
-                        )
-                        layer_order -= 1
+                            add_widget(
+                                sub_widget,
+                                sub_region,
+                                widget_region,
+                                widget_order,
+                                layer_order,
+                                sub_clip,
+                                visible,
+                            )
+                            layer_order -= 1
 
                 if visible:
                     # Add any scrollbars
                     for chrome_widget, chrome_region in widget._arrange_scrollbars(
                         container_region
                     ):
-                        map[chrome_widget] = MapGeometry(
+                        map[chrome_widget] = _MapGeometry(
                             chrome_region + layout_offset,
                             order,
                             clip,
@@ -444,7 +453,7 @@ def add_widget(
                             chrome_region,
                         )
 
-                    map[widget] = MapGeometry(
+                    map[widget] = _MapGeometry(
                         region + layout_offset,
                         order,
                         clip,
@@ -455,7 +464,7 @@ def add_widget(
 
             elif visible:
                 # Add the widget to the map
-                map[widget] = MapGeometry(
+                map[widget] = _MapGeometry(
                     region + layout_offset,
                     order,
                     clip,

src/textual/geometry.py

@@ -684,6 +684,7 @@ def clip(self, width: int, height: int) -> Region:
         )
         return new_region
 
+    @lru_cache(maxsize=4096)
     def grow(self, margin: tuple[int, int, int, int]) -> Region:
         """Grow a region by adding spacing.
 
@@ -704,6 +705,7 @@ def grow(self, margin: tuple[int, int, int, int]) -> Region:
             height=max(0, height + top + bottom),
         )
 
+    @lru_cache(maxsize=4096)
     def shrink(self, margin: tuple[int, int, int, int]) -> Region:
         """Shrink a region by subtracting spacing.
 
@@ -713,7 +715,8 @@ def shrink(self, margin: tuple[int, int, int, int]) -> Region:
         Returns:
             Region: The new, smaller region.
         """
-
+        if not any(margin):
+            return self
         top, right, bottom, left = margin
         x, y, width, height = self
         return Region(

src/textual/widget.py

@@ -802,15 +802,17 @@ def watch_hover_style(
         if self.auto_links:
             self.highlight_link_id = hover_style.link_id
 
-    def watch_scroll_x(self, new_value: float) -> None:
+    def watch_scroll_x(self, old_value: float, new_value: float) -> None:
         if self.show_horizontal_scrollbar:
-            self.horizontal_scrollbar.position = int(new_value)
-            self.refresh(layout=True, repaint=False)
+            self.horizontal_scrollbar.position = round(new_value)
+            if round(old_value) != round(new_value):
+                self._refresh_scroll()
 
-    def watch_scroll_y(self, new_value: float) -> None:
+    def watch_scroll_y(self, old_value: float, new_value: float) -> None:
         if self.show_vertical_scrollbar:
-            self.vertical_scrollbar.position = int(new_value)
-            self.refresh(layout=True, repaint=False)
+            self.vertical_scrollbar.position = round(new_value)
+            if round(old_value) != round(new_value):
+                self._refresh_scroll()
 
     def validate_scroll_x(self, value: float) -> float:
         return clamp(value, 0, self.max_scroll_x)
@@ -1147,7 +1149,7 @@ def scroll_offset(self) -> Offset:
         Returns:
             Offset: Offset a container has been scrolled by.
         """
-        return Offset(int(self.scroll_x), int(self.scroll_y))
+        return Offset(round(self.scroll_x), round(self.scroll_y))
 
     @property
     def is_transparent(self) -> bool:
@@ -2155,8 +2157,16 @@ async def _forward_event(self, event: events.Event) -> None:
         event._set_forwarded()
         await self.post_message(event)
 
+    def _refresh_scroll(self) -> None:
+        """Refreshes the scroll position."""
+        self._layout_required = True
+        self.check_idle()
+
     def refresh(
-        self, *regions: Region, repaint: bool = True, layout: bool = False
+        self,
+        *regions: Region,
+        repaint: bool = True,
+        layout: bool = False,
     ) -> None:
         """Initiate a refresh of the widget.