diff --git a/CHANGELOG.md b/CHANGELOG.md index 8da8e3b9e0..90bb8ddc23 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/textual/_compositor.py b/src/textual/_compositor.py index 1e7ab16035..ca92e525e7 100644 --- a/src/textual/_compositor.py +++ b/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, diff --git a/src/textual/geometry.py b/src/textual/geometry.py index 9b5c09f03c..17e938b1dd 100644 --- a/src/textual/geometry.py +++ b/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( diff --git a/src/textual/widget.py b/src/textual/widget.py index 185d18a64d..8c3f7951d9 100644 --- a/src/textual/widget.py +++ b/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.