Skip to content

refactor(render): (with-frame ...) — RAII-style synced frame boundaries - #174

Merged
mparrett merged 1 commit into
mainfrom
perf/render-sync-with-frame
Jul 23, 2026
Merged

refactor(render): (with-frame ...) — RAII-style synced frame boundaries#174
mparrett merged 1 commit into
mainfrom
perf/render-sync-with-frame

Conversation

@mparrett

Copy link
Copy Markdown
Collaborator

Stacked on #149 (perf/render-sync-output). Review/merge that first; this PR's diff is only the with-frame refactor on top of it. Opened separately so it doesn't clutter or reset the approval on #149, and so the nesting question below gets its own thread.

What

Replaces the hand-written (sfx/sync-begin!) … (sfx/flush-frame!) pairs at every full-screen frame boundary with a single macro:

(defmacro with-frame
  [& body]
  `(do
     (xsofy.screenfx/sync-begin!)
     (try
       ~@body
       (finally (xsofy.screenfx/flush-frame!)))))

Every frame emitter becomes (sfx/with-frame …): render-full, render-dirty, animate-vfx!, the six modal screens, screenfx/clear-screen, and title's draw-frame + descent loop.

Why

Addresses the review note on #149: a with-frame with a finally makes the sync boundary structural instead of a matched pair a future edit can split.

  • Dangling syncs — fixed deterministically. perf(render): synchronized output (DEC 2026) across all frame renders #149's own design note admits a frame that throws before end-sync "leaves the mode on, but the terminal's own 2026 timeout releases it." The finally closes it immediately, no reliance on that timeout. There's a guard test (with-frame-closes-sync-even-when-body-throws) for exactly this.
  • Begin/end can't drift. They're generated as one form, so an early return or a newly inserted branch can't skip the close.

Notes / to discuss

  • Not nesting-safe, by design. Mode 2026 is a boolean — a single end-sync closes it — so an inner with-frame's finally would close the outer frame early. This preserves perf(render): synchronized output (DEC 2026) across all frame renders #149's "wrap only top-level frames" discipline rather than changing it. If we'd rather make nesting genuinely safe, with-frame would need a depth counter (emit begin on 0→1, end on 1→0). Nothing nests today, so I kept it a boolean; happy to switch to the counter if we want it to be foolproof. Documented in the macro's docstring.
  • commit-camera! hoisted. In render-full/render-dirty it moved just past the frame boundary. It only mutates the @last-camera atom (no terminal output), and both render-dirty branches called it identically, so hoisting it out of the frame is behavior-preserving.
  • Indentation. To keep the diff minimal I left the wrapped let bodies at their original indent rather than re-indenting ~200 lines under with-frame. Valid, just slightly flat at the (let line — can re-indent if preferred.
  • let-go quirk. The macro fully-qualifies xsofy.screenfx/* because let-go's syntax-quote doesn't auto-namespace symbols (let-go#48), matching the existing with-folded-salt convention.

Tests

just check clean; full suite green — 307 tests / 2803 assertions, 0 failures — including the existing render golden tests (which pin the per-cell output and so confirm paren balance across the rewrite) and two new guards: the balanced begin/body/end pair, and end-sync-on-throw.

Made with Cursor

@mparrett
mparrett deleted the branch main July 23, 2026 19:19
@mparrett mparrett closed this Jul 23, 2026
@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-23 19:39 UTC

Replace the hand-written (sync-begin!) … (flush-frame!) pairs at every
full-screen frame boundary with a single (screenfx/with-frame ...) macro
that opens the DEC 2026 sync on entry and always closes it (end-sync +
flush) on exit via try/finally. The begin/end can no longer drift apart
or be skipped by an early return, and — the point — a frame that throws
mid-emit still emits end-sync instead of leaning on the terminal's own
2026 release timeout.

Converts render-full, render-dirty (both branches now share one frame,
with the state-only commit-camera! hoisted past the sync boundary),
animate-vfx!, the six modal screens, screenfx/clear-screen, and title's
draw-frame + descent loop.

Not nesting-safe: mode 2026 is a boolean, so an inner with-frame's
finally would close the outer frame early — documented in the docstring;
the existing top-level-only discipline still holds. Adds guard tests for
the balanced pair and for end-sync-on-throw.

Co-authored-by: Cursor <cursoragent@cursor.com>
@mparrett
mparrett force-pushed the perf/render-sync-with-frame branch from 087ec53 to eba896f Compare July 23, 2026 19:36
@mparrett
mparrett changed the base branch from perf/render-sync-output to main July 23, 2026 19:36
@mparrett
mparrett merged commit 9a9761c into main Jul 23, 2026
8 checks passed
@mparrett
mparrett deleted the perf/render-sync-with-frame branch July 23, 2026 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants