Dialogs: full-repaint on scroll to clear a smear off the static frame
The real scroll-smear cause, and why the previous fix could not work. A floating dialog draws a small box over a static background: the frame around it and the box's own border are identical frame-to-frame, so prompt_toolkit's differential renderer never rewrites them. Its paint is relative-positioned (cursor_forward/backward from pt's own assumed column, advanced by pt's own width model). A glyph the terminal renders wider than pt models — a route graph's braille lanes, node-type marks, a chart's braille — desyncs the terminal cursor and pushes a stray cell onto that static frame, where it lingers because pt believes the cell is unchanged. Full-frame screens masked this: their body redraws wholesale on scroll, healing the smear itself (the map scrubs only its static edge). The prior attempt (822db52) filled pt's cached frame with a sentinel to force an in-place rewrite. But that keeps previous_screen non-null, so pt never takes the erase_down + cursor-reset-to-origin branch — it repaints the same desynced garbage. Only dropping the cached frame triggers the full erase_down that blanks the terminal and re-syncs the cursor. It also only covered three route-graph screens, missing scrolled result windows. Fix, structural rather than per-screen: when a scroll/movement action reaches a screen that is floating over a backdrop, drop pt's remembered frame so the next paint clears the whole terminal and redraws clean. No smears flag, no geometry — any floating dialog, any wide glyph. This is the "clear the screen before redraw" the symptom calls for; the cost is one full repaint per dialog scroll, negligible for a modal box. Removes the smears flag and _reflush_frame; the map's own edge scrub is untouched. Full suite: 807 pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
1 parent
82ed64ffcd
commit
9ce1e63dc9
6 files changed
+36
-74
No files matched your search
@@ -51,10 +51,6 @@ _EDGE_UNUSED = (110, 110, 110)
|
||||
class MessagePathsScreen(Screen):
|
||||
"""A floating dialog: one message's arrivals as a path graph over a row list."""
|
||||
|
||||
#: The path graph is braille, which the terminal may render wider than modelled, so a
|
||||
#: scroll reflushes the box to scrub any smeared border cell (see :attr:`Screen.smears`).
|
||||
smears = True
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
message: ChatMessage,
|
||||
|
||||
@@ -241,9 +241,6 @@ class PacketViewer(Screen):
|
||||
"""
|
||||
|
||||
grow_only = True
|
||||
#: A relayed packet's route graph is braille, which the terminal may render wider than
|
||||
#: modelled — so a paging/scroll reflushes the box to scrub any smeared border cell.
|
||||
smears = True
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
|
||||
@@ -120,10 +120,6 @@ class RecordDialog(Screen):
|
||||
removes this one record behind a red Cancel/Delete data-loss confirm.
|
||||
"""
|
||||
|
||||
#: The braille mini-map and route graph can render wider than modelled, so a scroll
|
||||
#: reflushes the box to scrub any smeared border cell (see :attr:`Screen.smears`).
|
||||
smears = True
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
record: DiscoveredPath,
|
||||
|
||||
@@ -48,13 +48,6 @@ class Screen:
|
||||
base screen. Ignored while ``chrome`` is ``True``.
|
||||
footnote: A short line drawn muted and centered *below* a chromeless base screen's
|
||||
box (e.g. a copyright notice). Ignored while ``chrome`` is ``True``.
|
||||
smears: Whether this screen paints braille or other glyphs the terminal may render
|
||||
wider than prompt_toolkit models (a route graph's fan lanes, the node-type
|
||||
marks). Such a glyph's double-width fallback can smear a static cell — a dialog
|
||||
border — that the differential renderer never rewrites, so the session reflushes
|
||||
the frame after a scroll (see :meth:`~meshterm.ui.tui.session.TuiSession._reflush_frame`)
|
||||
to scrub the leftover. A full-frame braille screen that already cleans its own
|
||||
edge (the map, via :meth:`consume_edge_scrub`) leaves this ``False``.
|
||||
"""
|
||||
|
||||
title: str = ""
|
||||
@@ -64,7 +57,6 @@ class Screen:
|
||||
chrome: bool = True
|
||||
banner: Optional[Sequence[str]] = None
|
||||
footnote: Optional[str] = None
|
||||
smears: bool = False
|
||||
|
||||
def __init__(self) -> None:
|
||||
"""Initialize scroll state and the (later-assigned) result future."""
|
||||
|
||||
+18
-27
@@ -90,10 +90,10 @@ _RECLAIM_LAST_COLUMN = os.environ.get("MESHTERM_FULL_WIDTH", "1") != "0"
|
||||
#: real nesting — a tool's list, an item's detail popup, and a confirm over that is only three.
|
||||
_MAX_DIALOG_LAYERS = 8
|
||||
|
||||
#: Scroll/pan actions after which a :attr:`~meshterm.ui.tui.screen.Screen.smears` screen
|
||||
#: reflushes the frame (see :meth:`TuiSession._reflush_frame`) — every key that can move
|
||||
#: wide-glyph content, so a leftover fragment is scrubbed the moment its row shifts.
|
||||
_REFLUSH_ACTIONS = frozenset({
|
||||
#: Movement/scroll actions after which a floating dialog forces a full repaint (see
|
||||
#: :meth:`TuiSession._dispatch`) — every key that can shift the dialog's body, so a
|
||||
#: wide-glyph smear pushed onto the static frame around it is cleared as its row moves.
|
||||
_SCROLL_ACTIONS = frozenset({
|
||||
"up", "down", "pageup", "pagedown", "home", "end",
|
||||
"ctrl_home", "ctrl_end", "ctrl_pageup", "ctrl_pagedown",
|
||||
"left", "right", "ctrl_left", "ctrl_right",
|
||||
@@ -291,22 +291,6 @@ class TuiSession:
|
||||
cols, _ = self._size()
|
||||
self._scrub_columns(cols - count, cols)
|
||||
|
||||
def _reflush_frame(self) -> None:
|
||||
"""Force the next paint to rewrite every cell in place — no erase, no flicker.
|
||||
|
||||
Fills pt's remembered last frame with a sentinel so the differential renderer reads
|
||||
every cell as changed and redraws it (and erase-to-end-of-lines each row's tail),
|
||||
overwriting a double-width fallback glyph's smeared halves wherever they landed. A
|
||||
screen that draws braille or other glyphs the terminal may paint wider than pt models
|
||||
(a route-graph dialog — see :attr:`~meshterm.ui.tui.screen.Screen.smears`) reflushes
|
||||
after a scroll, the way the map scrubs its edge after a pan. Unlike
|
||||
:meth:`request_full_repaint` this keeps the cached frame, so pt overwrites in place
|
||||
rather than erasing first — no blank flash. Scrubbing the whole frame sidesteps the
|
||||
centred box's geometry entirely: the smear is cleaned wherever it sits.
|
||||
"""
|
||||
cols, _ = self._size()
|
||||
self._scrub_columns(0, cols)
|
||||
|
||||
# --- async prompt helpers ------------------------------------------------
|
||||
|
||||
async def run_screen(self, screen: Screen) -> Any:
|
||||
@@ -1090,17 +1074,24 @@ class TuiSession:
|
||||
def _dispatch(self, action: str, data: str = "") -> None:
|
||||
"""Forward an action to the top screen and repaint.
|
||||
|
||||
A screen that paints braille or other glyphs the terminal may render wider than
|
||||
prompt_toolkit models (a route-graph dialog, :attr:`~meshterm.ui.tui.screen.Screen.
|
||||
smears`) can leave a smeared fragment on a static cell — a dialog border — that the
|
||||
differential renderer never rewrites. After a scroll moves the offending row away,
|
||||
reflush the frame so the leftover is scrubbed (see :meth:`_reflush_frame`).
|
||||
A floating dialog draws a small box over a static background: the frame around it and
|
||||
the box's own border don't change frame-to-frame, so prompt_toolkit's differential
|
||||
paint never rewrites them. If a glyph in the dialog body is one the terminal renders
|
||||
wider than pt models (a route graph's braille lanes or node-type marks, a chart's
|
||||
braille), it desyncs the terminal cursor and pushes stray cells onto that static
|
||||
frame — where they linger, because pt believes those cells are unchanged. After a
|
||||
scroll, drop pt's cached frame so the next paint clears the whole terminal and
|
||||
redraws from a clean slate: an ``erase_down`` from the origin, the one reset that
|
||||
re-syncs the cursor (see :meth:`_invalidate_last_frame`). A full-frame screen with no
|
||||
float keeps the fast differential paint — its body redraws wholesale as it scrolls,
|
||||
so a smear there heals itself (the map scrubs only its static edge, via
|
||||
:meth:`consume_edge_scrub`).
|
||||
"""
|
||||
top = self.top
|
||||
if top is not None:
|
||||
top.handle(action, data)
|
||||
if action in _REFLUSH_ACTIONS and getattr(top, "smears", False):
|
||||
self._reflush_frame()
|
||||
if action in _SCROLL_ACTIONS and self._has_float():
|
||||
self._invalidate_last_frame()
|
||||
self.invalidate()
|
||||
|
||||
|
||||
|
||||
+18
-28
@@ -1480,9 +1480,12 @@ def test_session_background_is_the_topmost_full_frame_screen() -> None:
|
||||
assert session._float_layers() == [dialog]
|
||||
|
||||
|
||||
def test_scrolling_a_smear_prone_screen_reflushes_the_whole_frame() -> None:
|
||||
"""A braille screen force-rewrites every cell on scroll, so a wide-glyph smear on any
|
||||
static cell (a dialog border) is scrubbed — where a plain screen keeps the fast diff."""
|
||||
def test_scrolling_a_floating_dialog_forces_a_full_repaint() -> None:
|
||||
"""A floating dialog leaves the frame around it (and its own border) static between
|
||||
frames, so a wide-glyph smear pushed onto those cells would linger under the differential
|
||||
paint. Scrolling one drops pt's cached frame, so the next paint clears the whole terminal
|
||||
and redraws from the origin — the only reset that re-syncs the cursor. A full-frame base
|
||||
with no float keeps the efficient differential paint (its body heals a smear itself)."""
|
||||
import types
|
||||
|
||||
from prompt_toolkit.data_structures import Size
|
||||
@@ -1496,9 +1499,6 @@ def test_scrolling_a_smear_prone_screen_reflushes_the_whole_frame() -> None:
|
||||
invalidate=lambda: None,
|
||||
)
|
||||
|
||||
class _Braille(ScrollScreen):
|
||||
smears = True
|
||||
|
||||
def _filled() -> PtScreen:
|
||||
screen = PtScreen()
|
||||
for row in range(10):
|
||||
@@ -1506,32 +1506,22 @@ def test_scrolling_a_smear_prone_screen_reflushes_the_whole_frame() -> None:
|
||||
screen.data_buffer[row][x] = Char("A")
|
||||
return screen
|
||||
|
||||
# A smear-prone screen: a scroll fills the remembered frame with the scrub sentinel, so
|
||||
# the next diff rewrites every cell in place (no cell is left for the smear to hide in).
|
||||
# A dialog floating over a base: a scroll drops the remembered frame so the next paint
|
||||
# does a full erase_down + redraw, scrubbing anything a smear left on the static frame.
|
||||
session = TuiSession()
|
||||
session._app = _fake_app(_filled())
|
||||
session.push(ScrollScreen(Text("base"), floating=False))
|
||||
session.push(ScrollScreen(Text("dialog"))) # floats over the base
|
||||
session._dispatch("down")
|
||||
assert session._app.renderer._last_screen is None
|
||||
|
||||
# A full-frame base with nothing floating over it keeps the fast differential paint.
|
||||
session = TuiSession()
|
||||
last = _filled()
|
||||
session._app = _fake_app(last)
|
||||
session.push(_Braille(Text("x")))
|
||||
session.push(ScrollScreen(Text("base"), floating=False))
|
||||
session._dispatch("down")
|
||||
assert all(last.data_buffer[r][x].char == "" for r in range(10) for x in range(60))
|
||||
|
||||
# A plain screen keeps the efficient differential paint — the frame is left untouched.
|
||||
session = TuiSession()
|
||||
last = _filled()
|
||||
session._app = _fake_app(last)
|
||||
session.push(ScrollScreen(Text("x"))) # smears defaults False
|
||||
session._dispatch("down")
|
||||
assert last.data_buffer[0][0].char == "A"
|
||||
|
||||
|
||||
def test_smear_prone_route_graph_dialogs_opt_into_the_reflush() -> None:
|
||||
"""The braille route-graph dialogs declare ``smears`` so a scroll scrubs their borders."""
|
||||
from meshterm.ui.message_paths_screen import MessagePathsScreen
|
||||
from meshterm.ui.packet_viewer import PacketViewer
|
||||
from meshterm.ui.records_screen import RecordDialog
|
||||
|
||||
assert PacketViewer.smears and MessagePathsScreen.smears and RecordDialog.smears
|
||||
assert Screen.smears is False # the default: plain screens keep the fast diff
|
||||
assert session._app.renderer._last_screen is last
|
||||
|
||||
|
||||
def test_floating_text_prompt_is_a_popup_over_a_blank_base() -> None:
|
||||
|
||||
Reference in new issue
Block a user