Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions evidence/scroll-parent-coordinate-acquisition-20261007/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Nested scrolling parent coordinates

Widget.scroll_to_widget now acquires the parent virtual_region_with_margin once
per scrolling step, after the parent's scroll_to_region call. Translation and
intersection derive from that same immutable Region. Previously each step read
the property twice, repeating Screen/Compositor lookup and margin growth.
The original first-read order, ancestor traversal, style access, dock admission,
scroll arguments, callback ownership and custom geometry property remain intact.
A custom property is still invoked; translation and clipping use its one answer.

Existing refactor-audit Package parsed all 249 production modules, zero omissions.
The complete consumer family is Screen focus scrolling, Widget focus/scroll
helpers, ListView, Markdown and RadioSet. All use the same unchanged method API.
The parent-coordinate read in this method drops from two sites to one. The
initial target acquisition remains independent.

Real nested App control passed (1 test, 0.35s): bordered/margined containers both
scroll, target ends visible through both, each parent getter called once.
The same application exercised App.run_async with original headless driver:
terminal 0, 42.29ms operation including Pilot's idle wait. This is correctness
and reduced lookup work, not a sidebar latency result or physical UI acceptance.
No provider, package, installed pin or public App operation.

First run_async driver attempt incorrectly awaited Pilot.pause from inside an
after-refresh callback. It produced no result and was stopped with SIGINT;
original process3491947 joined at exit0. Corrected driver signals readiness in
that callback and executes the scrolling check from its external controlling
coroutine; original App task is joined on exit. No native callback repair was
inferred from that driver error.

Tree resize was also traced: rebuilding remeasures get_label_width/render_label,
whose custom implementations can depend on viewport size. No resize invalidation
was removed. The reported 45ms reflow remains unqualified by this small change.
4 changes: 3 additions & 1 deletion src/textual/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ def __init__(self, widget: Widget) -> None:
self.widget = widget

def can_replace(self, message: Message) -> bool:
return isinstance(message, Layout)
# Screen retains requests by source while a subtree borrows committed
# geometry. Another widget's request cannot consume that source.
return isinstance(message, Layout) and self.widget is message.widget


@rich.repr.auto
Expand Down
26 changes: 19 additions & 7 deletions src/textual/screen.py
Original file line number Diff line number Diff line change
Expand Up @@ -1467,6 +1467,15 @@ def _held_layout_requests(self) -> dict[DOMNode, set[Widget]]:
})
} if mutation_roots else {}

def _has_actionable_layout_requests(
self, held: dict[DOMNode, set[Widget]]
) -> bool:
"""Whether the original pending source requests can affect geometry."""
return any(
not members <= held.get(owner, set())
for owner, members in self._layout_widgets.items()
)

def _refresh_layout(self, size: Size | None = None, scroll: bool = False) -> None:
"""Refresh the layout (can change size and positions of widgets)."""
size = self.outer_size if size is None else size
Expand All @@ -1484,10 +1493,7 @@ def _refresh_layout(self, size: Size | None = None, scroll: bool = False) -> Non
# Held requests remain pending for release. They cannot alter the
# committed subtree during scrolling; only actionable requests
# require layout beyond the original visible-scroll traversal.
if scroll and all(
members <= held_layout.get(owner, set())
for owner, members in self._layout_widgets.items()
):
if scroll and not self._has_actionable_layout_requests(held_layout):
exposed_widgets = self._compositor.reflow_visible(
self, size, retain_geometry=geometry_targets,
)
Expand Down Expand Up @@ -1591,9 +1597,15 @@ async def _on_layout(self, message: messages.Layout) -> None:
break
widget = ancestor

if layout_required and not self._layout_required:
self._layout_required = True
self.check_idle()
if not self._layout_required:
# A prior frame keeps held requests but consumes its timer intent.
# Membership is not a new-source notification: after release the
# SAME pending request becomes actionable under current roots.
if layout_required or self._has_actionable_layout_requests(
self._held_layout_requests()
):
self._layout_required = True
self.check_idle()

async def _on_update_scroll(self, message: messages.UpdateScroll) -> None:
message.stop()
Expand Down
17 changes: 10 additions & 7 deletions src/textual/widget.py
Original file line number Diff line number Diff line change
Expand Up @@ -3862,14 +3862,17 @@ def scroll_to_widget(
# Adjust the region by the amount we just scrolled it, and convert to
# its parent's virtual coordinate system.
region = (
(
region.translate(-scroll_offset)
.translate(container.styles.margin.top_left)
.translate(container.styles.border.spacing.top_left)
.translate(container.virtual_region_with_margin.offset)
)
region.translate(-scroll_offset)
.translate(container.styles.margin.top_left)
.translate(container.styles.border.spacing.top_left)
)
# Translate and clip against the same acquired parent geometry,
# after its scroll operation has completed.
container_region = container.virtual_region_with_margin
region = (
region.translate(container_region.offset)
.grow(container.styles.margin)
.intersection(container.virtual_region_with_margin)
.intersection(container_region)
)

widget = container
Expand Down
31 changes: 31 additions & 0 deletions tests/test_batch_paint_admission.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,37 @@ def compose(self):
yield Label("SIDEBAR_OLD", id="sidebar")


async def test_released_pending_layout_resumes_on_its_existing_owner_request():
app = IndependentApp()
completed = asyncio.Event()
async with app.run_test(size=(40, 8)) as pilot:
await pilot.pause()
screen = app.screen
holder = app.query_one(HeldVertical)
original = app.query_one("#original")
committed = screen._compositor.find_widget(original)
async with holder.lock:
new = Label("RELEASED_SOURCE", id="released")
await holder.mount(new)
# A child source change makes the holder itself a pending owner,
# while mount alone may publish only its parent's holder request.
new.styles.height = 4
await pilot.pause()
assert holder in screen._layout_widgets
assert not screen._layout_required
assert screen._compositor.find_widget(original) == committed
assert new not in screen._compositor._published_map

# Subscribe only after the held frame, then exercise the SAME owner's
# refresh. No sidebar update, geometry query or manual timer wakeup.
screen.screen_layout_refresh_signal.subscribe(app, lambda _: completed.set())
holder.refresh(layout=True)
await asyncio.wait_for(completed.wait(), 2)
assert new in screen._compositor._published_map
assert not screen._layout_widgets
assert not app._exception


@pytest.mark.parametrize("held", [False, True])
async def test_idle_shares_acquired_roots_and_paint_reacquires(held):
from textual import events
Expand Down
9 changes: 9 additions & 0 deletions tests/test_message_pump.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,15 @@
from textual.widgets import Button, Input, Label


def test_layout_replacement_preserves_distinct_source_requests():
from textual.messages import Layout

first, second = Widget(), Widget()
request = Layout(first)
assert request.can_replace(Layout(first))
assert not request.can_replace(Layout(second))


class ValidWidget(Widget):
called_by = None

Expand Down
47 changes: 47 additions & 0 deletions tests/test_scroll_parent_coordinates.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
from textual.app import App, ComposeResult
from textual.containers import VerticalScroll
from textual.geometry import Region
from textual.pilot import Pilot
from textual.widgets import Label


class ObservedScroll(VerticalScroll):
coordinate_reads = 0

@property
def virtual_region_with_margin(self) -> Region:
self.coordinate_reads += 1
return super().virtual_region_with_margin


class NestedScrollApp(App[None]):
CSS = """
#outer { height: 10; border: solid green; }
#inner { height: 15; margin: 1; border: solid blue; }
Label { height: 3; }
"""

def compose(self) -> ComposeResult:
with ObservedScroll(id="outer"):
with ObservedScroll(id="inner"):
for index in range(12):
yield Label(f"Row {index}", id=f"row{index}")


async def test_nested_scroll_uses_one_parent_coordinate_per_step() -> None:
app = NestedScrollApp()
async with app.run_test(size=(60, 20)) as pilot:
await check_nested_scroll(app, pilot)


async def check_nested_scroll(app: NestedScrollApp, pilot: Pilot) -> None:
outer = app.query_one("#outer", ObservedScroll)
inner = app.query_one("#inner", ObservedScroll)
target = app.query_one("#row11", Label)
outer.coordinate_reads = inner.coordinate_reads = 0
assert app.screen.scroll_to_widget(target, animate=False, immediate=True)
assert (inner.coordinate_reads, outer.coordinate_reads) == (1, 1)
await pilot.pause()
assert inner.scroll_y > 0 and outer.scroll_y > 0
assert target.region.intersection(inner.scrollable_content_region)
assert target.region.intersection(outer.scrollable_content_region)
Loading