diff --git a/src/textual/_compositor.py b/src/textual/_compositor.py index 9b8dbbd6df..5460d60912 100644 --- a/src/textual/_compositor.py +++ b/src/textual/_compositor.py @@ -171,6 +171,8 @@ class SubtreeMapGeometry(NamedTuple): geometry: MapGeometry clip_bounds: tuple[Region, ...] + parent: Widget | None + """Original rendered parent within this capture, not the mutating DOM.""" @property def region(self) -> Region: @@ -228,22 +230,26 @@ def _spatial_map(self) -> SpatialMap[tuple[int, Widget]]: return spatial_map @abstractmethod - def matches(self, key: SubtreeGeometryKey, *, require_complete: bool = False) -> bool: + def matches(self, key: SubtreeGeometryKey, *, require_complete: bool = False, + source_held: bool = False) -> bool: ... def restore_into( self, geometry: CompositorMap, widgets: set[Widget], invisible_widgets: set[Widget], key: SubtreeGeometryKey, clip: SceneClip, clips: dict[Widget, SceneClip], root: Widget, *, visible_only: bool, retained: set[Widget], bounds: Region, + source_held: bool = False, ) -> None: self.project_into(geometry, key, clip, clips, root, - visible_only=visible_only, retained=retained, bounds=bounds) + visible_only=visible_only, retained=retained, bounds=bounds, + source_held=source_held) widgets.update(self.widgets) invisible_widgets.update(self.invisible_widgets) def project_into(self, geometry: CompositorMap, key: SubtreeGeometryKey, clip: SceneClip, clips: dict[Widget, SceneClip], root: Widget, - *, visible_only: bool, retained: set[Widget], bounds: Region) -> None: + *, visible_only: bool, retained: set[Widget], bounds: Region, + source_held: bool = False) -> None: """Publish the requested scene without retiring complete source geometry. A viewport needs exposed descendants and its explicit geometry targets. @@ -260,7 +266,7 @@ def project_into(self, geometry: CompositorMap, key: SubtreeGeometryKey, for _, node in sorted(candidates.items())) else: entries = ((node, entry) for node, (_, entry) in self.geometry.items()) - for node, entry in entries: + for node, entry in self._paint_entries(entries, source_held=source_held): required = not visible_only or node is root or node in retained # Clip intersection can only reduce these original rectangle bounds. # Reject an offscreen, unrequired entry before resolving its clip @@ -272,6 +278,10 @@ def project_into(self, geometry: CompositorMap, key: SubtreeGeometryKey, geometry[node] = placement clips[node] = node_clip + def _paint_entries(self, entries, *, source_held: bool): + """Placed sources retain their original native participation.""" + return entries + @abstractmethod def _project_entry(self, entry: GeometryEntry, key: SubtreeGeometryKey, clip: SceneClip, *, root: bool) -> tuple[MapGeometry, SceneClip]: @@ -287,7 +297,8 @@ def references_retired(self, owner: Widget, retired: set[Widget]) -> bool: class PlacedSubtreeGeometry(SubtreeGeometry[MapGeometry]): """A culled or screen-dependent arrangement keeps its exact placement.""" - def matches(self, key: SubtreeGeometryKey, *, require_complete: bool = False) -> bool: + def matches(self, key: SubtreeGeometryKey, *, require_complete: bool = False, + source_held: bool = False) -> bool: if self.key.visible_only and require_complete: return False return self.key == key or ( @@ -320,17 +331,47 @@ def capture(cls, key, geometry, widgets, invisible_widgets, clips, clip, screen_ if scope is not clip: return PlacedSubtreeGeometry.capture( key, geometry, widgets, invisible_widgets, clips, clip, screen_coordinates) - intrinsic[node] = SubtreeMapGeometry(entry, bounds) + parent = node.parent + parent = parent._render_widget if isinstance(parent, Widget) else None + intrinsic[node] = SubtreeMapGeometry( + entry, bounds, parent if parent in geometry and parent is not node else None, + ) return super().capture(key, intrinsic, widgets, invisible_widgets, clips, clip, screen_coordinates) - def matches(self, key: SubtreeGeometryKey, *, require_complete: bool = False) -> bool: + def matches(self, key: SubtreeGeometryKey, *, require_complete: bool = False, + source_held: bool = False) -> bool: + if source_held: + # The mutation owner lends the original source, not the changing + # DOM. Internal scroll and all arrangement inputs still agree; + # only source revisions belong to the ongoing mutation. + key = key._replace(geometry_revision=self.key.geometry_revision, + nodes_revision=self.key.nodes_revision) return self.key.intrinsic() == key.intrinsic() def _project_entry(self, entry: SubtreeMapGeometry, key: SubtreeGeometryKey, clip: SceneClip, *, root: bool) -> tuple[MapGeometry, SceneClip]: return entry.project(self.key, key, clip, root=root) + def _paint_entries(self, entries, *, source_held: bool): + if not source_held: + return entries + participation: dict[Widget, bool] = {} + + def participates(node: Widget) -> bool: + if node not in participation: + parent = self.geometry[node][1].parent + participation[node] = node._render_widget is node and ( + parent is None or (parent.is_container and participates(parent)) + ) + return participation[node] + + # A retained-paint owner can now render itself rather than its old + # native children. Consult that declaration along captured ancestry, + # never walk or arrange the changing child tree. Each parent answers + # once within this projection; the resource remains immutable. + return ((node, entry) for node, entry in entries if participates(node)) + class CompositorUpdate: """An update generated by the compositor, which also doubles as console renderables.""" @@ -885,7 +926,7 @@ def _arrange_root( for node, geometry in previous.items(): for owner in mutation_roots.intersection(node.walk_ancestors(with_self=True)): held[owner][node] = geometry - for owner in mutation_paths: + for owner in mutation_paths - mutation_roots: self._subtree_geometry.pop(owner, None) map: CompositorMap = {} # Transient declaration paths manufacture the one retained resource. @@ -916,7 +957,10 @@ def store_geometry(node: Widget, geometry: MapGeometry, clip: SceneClip) -> None default_layers = {"default": 0} retained_paths: set[Widget] = set() if visible_only: - for target in retain_geometry: + # A held root still needs its new outer placement when scrolling + # takes it offscreen. Its resource owns descendants; this acquires + # only the ancestor path, not the mutating subtree's arrangement. + for target in (*retain_geometry, *mutation_roots): path: list[Widget] = [] node = target # Existing members already own a path to this same root. @@ -1144,15 +1188,39 @@ def arrange_widget( def add_widget(widget, virtual_region, region, order, layer_order, clip, visible, dock_gutter, complete, inherited_layers): nonlocal map, widgets, invisible_widgets - if widget in held: - # Retain exact hit/paint placement. Moving or resizing this - # subtree belongs to the mutation owner's next layout. - for node, geometry in held[widget].items(): - store_geometry(node, geometry, RootSceneClip(geometry.clip)) - widgets.update(held[widget]) - return if inherited_layers is None and (order_names := widget._get_layer_order((widget,))) is not None: inherited_layers = {name: index for index, name in enumerate(order_names)} + if widget in held: + resource = self._subtree_geometry.get(widget) + key = SubtreeGeometryKey.from_widget( + widget, virtual_region, region, order, layer_order, clip.region, + visible, dock_gutter, size, False, + tuple(inherited_layers.items()) if inherited_layers is not None else (), + ) + if resource is not None and widget._render_widget in resource.geometry and resource.matches( + key, require_complete=True, source_held=True, + ): + # Complete intrinsic geometry retains original descendants + # and clip declarations. Ancestor scrolling changes only + # their placement; no mutating DOM arrangement is read. + resource.restore_into( + map, widgets, invisible_widgets, key, clip, clips, + widget._render_widget, visible_only=visible_only and not complete, + retained=retained_paths, bounds=root_geometry.region, + source_held=True, + ) + # The complete resource has replaced these old viewport + # entries, including nested holds. Do not merge them back + # after projection and resurrect stale absolute placement. + for owner in held.keys() & resource.widgets: + held[owner].clear() + else: + # Placed, resized or missing sources cannot authorize a + # projection. Keep their exact committed hit/paint bounds. + for node, geometry in held[widget].items(): + store_geometry(node, geometry, RootSceneClip(geometry.clip)) + widgets.update(held[widget]) + return if (widget in mutation_paths or not self.max_subtree_geometry_entries or not widget.CACHE_SUBTREE_GEOMETRY or not widget._is_mounted): arrange_widget(widget, virtual_region, region, order, layer_order, clip, visible, dock_gutter, diff --git a/src/textual/screen.py b/src/textual/screen.py index a0e50ca64d..b8095fa4d5 100644 --- a/src/textual/screen.py +++ b/src/textual/screen.py @@ -1396,6 +1396,14 @@ def _invoke_and_clear_callbacks(self, roots: tuple[Widget, ...] | None = None) - # None means not acquired; an empty borrowed tuple is a valid cohort. if roots is None: roots = tuple(root for _, roots in self._prepare_visible_screens() for root in roots) if self.is_current else () + if self.is_current: + # Original retained pixels may paint during a source mutation. + # That publication does not complete the mutating sender's work. + # Acquire source custody once for this synchronous admission, + # separately from the original paint-readiness roots. + roots = (*roots, *(root + for screen in (*self.app._background_screens, self) + for root in screen._layout_mutation_roots())) index = 0 for _ in range(len(self._callbacks)): if self.app._batch_count or index >= len(self._callbacks): diff --git a/tests/test_batch_paint_admission.py b/tests/test_batch_paint_admission.py index dfc4f19798..138c3b3941 100644 --- a/tests/test_batch_paint_admission.py +++ b/tests/test_batch_paint_admission.py @@ -361,7 +361,11 @@ async def test_partial_publication_keeps_geometry_and_owner_callbacks(): await pilot.pause() assert compositor.get_widget_at(15, 1)[0] is original_hit holder.refresh(layout=True) - await asyncio.wait_for(scene_done.wait(), 2) + # Admission returns each callback to its original sender pump; the + # App callback cannot establish completion of the two widget tasks. + await asyncio.wait_for(asyncio.gather( + scene_done.wait(), held_done.wait(), parent_done.wait(), + ), 2) assert held_done.is_set() and parent_done.is_set() assert screen.publications[-1] == () assert not compositor._dirty_regions diff --git a/tests/test_viewport_geometry_targets.py b/tests/test_viewport_geometry_targets.py index ca926deee8..b6d0770592 100644 --- a/tests/test_viewport_geometry_targets.py +++ b/tests/test_viewport_geometry_targets.py @@ -1,3 +1,4 @@ +import asyncio from unittest.mock import patch from textual import errors @@ -25,6 +26,234 @@ def compose(self): yield Static("second line " * 5) +async def test_held_complete_body_projects_original_scene_during_ancestor_scroll(): + """Real layout, paint and hit readers borrow the pre-mutation resource.""" + from textual._compositor import Compositor, IntrinsicSubtreeGeometry + + class HeldBody(VerticalGroup): + CACHE_SUBTREE_GEOMETRY = True + + def arrange(self, size, optimal=False): + assert not self.lock.is_locked, "Read changing body layout" + return super().arrange(size, optimal=optimal) + + rows = [Static(f"COMMITTED_ROW_{index:02}", id=f"held-row-{index}") + for index in range(40)] + body = HeldBody(Static("FIXED_HEADER", id="held-heading"), *rows, id="held-body") + + class HeldScreen(TargetedScreen): + CSS = """ + #held-heading { dock: top; height: 1; } + #held-body Static { height: 2; } + #outside-body { height: 30; } + """ + + def _layout_mutation_roots(self): + return (body,) if body.lock.is_locked else () + + def compose(self): + with VerticalScroll(id="history"): + yield body + yield Static("\n".join(["OUTSIDE_BODY"] * 30), id="outside-body") + + app = App() + async with app.run_test(size=(40, 10)) as pilot: + screen = HeldScreen() + screen.targets = (body,) + await app.push_screen(screen) + await pilot.pause() + history = screen.query_one("#history", VerticalScroll) + compositor = screen._compositor + resource = compositor._subtree_geometry[body] + assert isinstance(resource, IntrinsicSubtreeGeometry) + assert all(row in resource.geometry for row in rows) + # Holding revisions does not permit resizing or changing the source's + # own scroll scope. Those answers still require a fresh arrangement. + assert not resource.matches( + resource.key._replace(region=resource.key.region.grow((0, 1, 0, 0))), + source_held=True, + ) + assert not resource.matches( + resource.key._replace(scroll_offset=resource.key.scroll_offset + (0, 1)), + source_held=True, + ) + + # Acquire the unchanged source at the real destination before mutation. + # This is an ordinary compositor, not a reconstructed translation oracle. + expected = {} + for position in (0, 16, 4, 100): + history.scroll_to(y=position, animate=False, immediate=True) + await pilot.pause() + reference = Compositor(max_subtree_geometry_entries=0) + reference.reflow_visible(screen, app.size, retain_geometry=(body,)) + expected[position] = { + node: geometry for node, geometry in reference._visible_map.items() + if node is body or geometry.visible_region.overlaps(app.size.region) + } + history.scroll_to(y=0, animate=False, immediate=True) + await pilot.pause() + screen.targets = () + + completed = asyncio.Event() + unrelated = asyncio.Event() + async with body.lock: + incomplete = Static("UNCOMMITTED_CHILD", id="incomplete") + await body.mount(incomplete) + body.call_after_refresh(completed.set) + screen.query_one("#outside-body").call_after_refresh(unrelated.set) + await pilot.pause() + for position in (16, 100, 4, 0): + history.scroll_to(y=position, animate=False, immediate=True) + await pilot.pause() + assert compositor._subtree_geometry[body] is resource + assert incomplete not in compositor._published_map + for node, geometry in expected[position].items(): + assert compositor._published_map[node] == geometry, (position, node.id) + # Actual strip rendering and hit testing consume that same scene. + strips = compositor.render_strips() + rendered = "\n".join(strip.text for strip in strips) + assert "UNCOMMITTED_CHILD" not in rendered + if position == 100: + assert "COMMITTED_ROW" not in rendered + assert "OUTSIDE_BODY" in rendered + else: + assert "COMMITTED_ROW" in rendered + for row in rows: + geometry = compositor._published_map.get(row) + if geometry is not None and geometry.visible_region: + x, y = geometry.visible_region.offset + hit, _ = compositor.get_widget_at(x, y) + assert hit is row + break + assert not completed.is_set() + assert unrelated.is_set() + + body.refresh(layout=True) + await pilot.pause() + assert compositor._subtree_geometry[body] is not resource + assert incomplete in compositor.widgets + assert completed.is_set() + assert not app._exception + + +async def test_held_screen_relative_source_keeps_placement_and_retires_children(): + from textual._compositor import PlacedSubtreeGeometry + + class HeldBody(VerticalGroup): + CACHE_SUBTREE_GEOMETRY = True + + def arrange(self, size, optimal=False): + assert not self.lock.is_locked, "Read changing overlay layout" + return super().arrange(size, optimal=optimal) + + overlay = Static("SCREEN_OVERLAY", id="overlay") + retired = Static("RETIRE_THIS", id="retired") + body = HeldBody(overlay, retired, *(Static(f"row {i}") for i in range(30))) + + class HeldScreen(TargetedScreen): + CSS = "#overlay { overlay: screen; width: 12; height: 1; offset: 2 3; }" + + def compose(self): + with VerticalScroll(id="history"): + yield body + + def _layout_mutation_roots(self): + return (body,) if body.lock.is_locked else () + + app = App() + async with app.run_test(size=(40, 10)) as pilot: + screen = HeldScreen() + screen.targets = (body,) + await app.push_screen(screen) + await pilot.pause() + compositor = screen._compositor + resource = compositor._subtree_geometry[body] + assert isinstance(resource, PlacedSubtreeGeometry) + original = dict(compositor._published_map) + async with body.lock: + await body.mount(Static("INCOMPLETE")) + screen.query_one("#history").scroll_to(y=10, animate=False, immediate=True) + await pilot.pause() + for node, geometry in original.items(): + if node in resource.geometry: + assert compositor._published_map[node] == geometry + await retired.remove() + await pilot.pause() + assert body not in compositor._subtree_geometry + assert retired not in compositor._published_map + assert retired not in compositor.widgets + assert compositor._published_map[overlay] == original[overlay] + body.refresh(layout=True) + await pilot.pause() + assert not app._exception + + +async def test_held_original_ancestry_honors_a_nested_self_painting_owner(): + from textual.content import Content + + class HeldGroup(VerticalGroup): + CACHE_SUBTREE_GEOMETRY = True + + def arrange(self, size, optimal=False): + assert not self.lock.is_locked, "Read changing child topology" + return super().arrange(size, optimal=optimal) + + class ContentBody(Static): + # This application declares a native child view for its initial text + # and a self-painted view when it acquires its actual Content resource. + @property + def is_container(self): + return not isinstance(self.content, Content) + + holder = HeldGroup(id="holder") + + class ChangingChild(Static): + def render_lines(self, crop): + assert not holder.lock.is_locked, "Read changing descendant paint" + return super().render_lines(crop) + + body = ContentBody("INITIAL_NATIVE_PARENT", id="content-body") + old = ChangingChild("OLD_NATIVE_CHILD") + + class ContentScreen(TargetedScreen): + CSS = "#content-body { height: 18; } Static { height: 2; }" + + def compose(self): + with VerticalScroll(id="history"): + yield holder + + def _layout_mutation_roots(self): + return (holder,) if holder.lock.is_locked else () + + app = App() + async with app.run_test(size=(40, 10)) as pilot: + screen = ContentScreen() + screen.targets = (holder,) + await app.push_screen(screen) + await holder.mount(body) + await body.mount(old) + await pilot.pause() + compositor = screen._compositor + resource = compositor._subtree_geometry[holder] + assert old in resource.geometry + assert resource.geometry[old][1].parent is body + async with holder.lock: + body.update(Content("\n".join(["ACQUIRED_ROOT_PAINT"] * 18))) + await body.mount(ChangingChild("UNCOMMITTED_DESCENDANT")) + screen.query_one("#history").scroll_to(y=4, animate=False, immediate=True) + await pilot.pause() + assert compositor._subtree_geometry[holder] is resource + assert old not in compositor._published_map + assert body in compositor._published_map + painted = "\n".join(strip.text for strip in compositor.render_strips()) + assert "ACQUIRED_ROOT_PAINT" in painted + assert "OLD_NATIVE_CHILD" not in painted + assert "UNCOMMITTED_DESCENDANT" not in painted + holder.refresh(layout=True) + await pilot.pause() + assert not app._exception + + async def test_cached_overlay_and_fixed_children_match_the_original_scene(): from textual._compositor import Compositor, PlacedSubtreeGeometry