Skip to content

fix(rendering): don't crash the window when a glyph lands outside the canvas#333

Merged
kshivang merged 2 commits into
masterfrom
fix/drawtext-resize-crash
Jul 11, 2026
Merged

fix(rendering): don't crash the window when a glyph lands outside the canvas#333
kshivang merged 2 commits into
masterfrom
fix/drawtext-resize-crash

Conversation

@kshivang

Copy link
Copy Markdown
Owner

Problem

Resizing the window — or the update banner appearing and squeezing the terminal — sometimes kills the whole window with a native error dialog:

maxWidth must be >= than minWidth, maxHeight must be >= than minHeight, minWidth and minHeight must be >= 0

Root cause

Compose's drawText(textMeasurer, …) derives text-layout constraints from canvasSize - topLeft without clamping (verified in TextPainterKt.textLayoutConstraints bytecode for Compose 1.9.3 — plain fsub/ceil, no coerceAtLeast(0)). Any glyph whose topLeft falls past the right/bottom canvas edge produces negative constraints, and Constraints() throws the message above. Compose Desktop's default WindowExceptionHandler turns that into the fatal error dialog.

TerminalCanvasRenderer can produce such glyphs whenever visual columns outrun buffer columns: emoji / ambiguous-width symbols (Claude Code's ✳ spinner, braille spinners, ⚡ …) occupy one buffer cell but render two cells wide, so x = visualCol * cellWidth can pass size.width even though the render loop clamps buffer columns to visibleCols. At full window width the overshoot stays just inside; shrink the canvas (live resize, update banner reflow) and the draw crosses the edge → crash. The existing whole-canvas guard in ProperTerminal ("prevents drawText constraint failures") only rejects sub-cell canvases, so this hole remained.

Fix

Route all 8 glyph drawText call sites through a new drawTextClipped helper that skips draws whose topLeft is outside the canvas. Those glyphs are clipped by the canvas's clipToBounds() anyway, so skipping is visually identical. Negative topLeft (partially scrolled glyphs) keeps drawing — constraints only go invalid past the right/bottom edge.

Tests

DrawTextClippedTest (compose-ui desktopTest) pins both sides:

  • the framework really does throw IllegalArgumentException on this Compose version for out-of-bounds topLeft (right of / below canvas) — if Compose ever clamps internally, these tests fail and the guard can be dropped
  • drawTextClipped skips out-of-bounds and edge draws instead of crashing, still renders in-bounds glyphs (pixel-checked), and keeps negative-topLeft draws

All compose-ui desktop tests pass.

🤖 Generated with Claude Code

… canvas

Compose's drawText(textMeasurer, ...) derives text-layout constraints from
canvasSize - topLeft without clamping, so any glyph whose topLeft falls past
the right/bottom canvas edge throws IllegalArgumentException ("maxWidth must
be >= than minWidth, maxHeight must be >= than minHeight, minWidth and
minHeight must be >= 0") and kills the whole window.

The terminal canvas can produce such glyphs whenever visual columns outrun
buffer columns: emoji and ambiguous-width symbols occupy one buffer cell but
are rendered two cells wide, so visualCol * cellWidth can pass size.width
even though the render loop clamps buffer columns to visibleCols. At full
window width the overshoot stays (just) inside; shrink the canvas — live
window resize, or the update banner pushing the terminal down — and the draw
crosses the edge, crashing the app.

Route every glyph draw through drawTextClipped, which skips draws whose
topLeft is outside the canvas (they're clipped by clipToBounds anyway, so
this is visually identical) and keeps negative topLeft (partially scrolled
glyphs) drawing as before.

Tests pin both sides: the framework really does throw on this Compose
version for out-of-bounds topLeft (so we notice when the guard becomes
unnecessary), and the clipped variant skips instead of crashing while still
rendering in-bounds glyphs.

Generated with [Claude Code](https://claude.com/claude-code)
@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown

Review: fix(rendering): don't crash the window when a glyph lands outside the canvas

Nicely diagnosed and scoped fix. The root-cause writeup (Compose's drawText deriving maxWidth = canvasWidth - topLeft.x without clamping) matches what I see in the code, and routing all glyph draws through a single guarded helper is the right shape. A few notes below, none blocking.

Strengths

  • Complete call-site coverage. All 8 glyph drawText sites are converted; the only remaining raw drawText in TerminalCanvasRenderer.kt is the one inside drawTextClipped itself.
  • Correct per-axis guard. Compose computes the width/height constraints independently, so a topLeft past either the right or bottom edge is enough to throw. Guarding both axes with || exactly covers the crash condition.
  • Negative topLeft intentionally preserved. Good call keeping partially-scrolled glyphs drawing; constraints only go invalid past the far edge, not the near one.
  • The framework-behavior tests are the best part. frameworkDrawTextThrows... pins the assumption the guard depends on, so if a future Compose bump starts clamping internally the tests flag that the guard is now dead code rather than silently keeping it forever. Right way to encode a workaround for an upstream quirk.

Consideration: scaled draw sites use pre-scale topLeft vs. unscaled size

At the scaled sites (e.g. ~1436 and ~1617) the topLeft passed is Offset(x + (centerX - x) / scaleValue, y) — a pre-scale local coordinate — while the guard compares it against the unscaled size.width. When scaleValue < 1 (down to 0.8 per coerceIn(0.8f, 2.5f)), the pre-scale topLeft.x is inflated relative to where the glyph actually lands after scaling, so a wide glyph in the last column whose rendered center is in-bounds can have a pre-scale topLeft.x beyond size.width and get skipped.

This is not a defect — Compose derives its constraints from that same pre-scale topLeft, so without the guard that exact case crashes today; skip-instead-of-crash is strictly better. But it means the fix can occasionally drop a rightmost/bottommost scaled glyph rather than clip-draw it. Worth a one-line comment at those sites noting the guard is intentionally conservative for scaled draws, so a future reader does not mistake a missing edge glyph for a new bug. (The scaleX-only site at ~1575 is fine — topLeft.x == pivot.x == x, so scaling does not move it.)

Minor

  • topLeft.x >= size.width also skips a glyph sitting exactly on the edge (maxWidth == 0), which Constraints(0, 0) would actually accept without throwing. Harmless (fully clipped anyway) and the test documents it — just flagging that >= is slightly more aggressive than the crash condition strictly requires.
  • Not introduced here, but a NaN topLeft (e.g. a degenerate scaleValue) slips through both >= checks and still reaches drawText. Out of scope; noting only because the helper is now the single choke point where such a guard could live if it ever comes up.

Test coverage

Solid — both directions covered (framework throws / guard skips), in-bounds render pixel-verified, edge and negative cases pinned. Small gap: no test exercises a draw inside a scale{} block, which is where the pre-scale-vs-unscaled-size subtlety lives — a nice-to-have given the behavior there is skip rather than crash either way.

Overall LGTM — good root-cause analysis, minimal surface, and tests that guard the assumption, not just the symptom.

… scaled draw sites

At the two uniform-scale sites the topLeft passed to drawTextClipped is a
pre-scale coordinate compared against the unscaled canvas size — matching
how Compose derives constraints (that exact mismatch is what used to throw),
but for scaleValue < 1 it can skip a rightmost edge glyph whose rendered
position would have been in-bounds. Comment it so a missing edge glyph
isn't mistaken for a new bug. Also document that the guard's >= is slightly
stricter than the throw condition (an exactly-on-edge glyph measures at
zero width without throwing, but renders nothing).

Generated with [Claude Code](https://claude.com/claude-code)
@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown

Review: fix(rendering): don't crash the window when a glyph lands outside the canvas

Excellent PR. The root-cause writeup is unusually thorough, the fix is minimal and funneled through a single choke point, and the tests pin both the framework behavior being guarded against and the guard itself. I verified the changes against the codebase and have only minor, non-blocking notes.

What I verified

  • All 8 glyph drawText sites are routed through drawTextClipped. grep confirms the only remaining raw drawText( is line 416 inside the helper itself; there are no other drawText call sites anywhere in compose-ui/src/desktopMain.
  • The guard exactly matches the throw condition, even inside scale{} blocks. Compose derives constraints from DrawScope.size (the layout size, which scale{} does NOT change — it only mutates the canvas matrix) and the raw topLeft. So the crash condition inside a scaled block is still topLeft.x >= size.width || topLeft.y >= size.height, which is precisely what drawTextClipped checks. The "intentionally conservative for scaleValue < 1" comments are about a purely visual edge case, not the crash — good that they are documented, and the trade-off (skip vs. clip-draw one edge glyph) is the right call.
  • || (not &&) is correct: Compose throws if either maxWidth or maxHeight goes negative, so skipping when either axis is out of bounds is the right match.
  • Negative topLeft correctly keeps drawing: maxWidth = size.width - topLeft.x stays positive for negative topLeft, so partially-scrolled glyphs never hit the throw — the test drawTextClippedKeepsNegativeTopLeft pins this.
  • "Visually identical" claim holds: both Canvas blocks in ProperTerminal use .clipToBounds(), so a glyph whose topLeft is past the edge would have been clipped away regardless.

Minor notes (non-blocking)

  1. Symptom vs. root cause. This is a robust defensive guard at the draw layer, which I think is the right place for it (single choke point, covers future overshoot sources too). But the deeper cause is that visualCol * cellWidth can exceed size.width for double-width glyphs at the last visible column, while only buffer columns are clamped to visibleCols. Worth a follow-up thought on whether clamping visual advance upstream would also help — not needed for this fix.

  2. NaN topLeft. If topLeft.x/topLeft.y were ever NaN (e.g. a degenerate cellWidth), NaN >= size.width is false, so the guard would fall through to drawText and could still throw. Extremely unlikely given the render math, so only worth noting as a theoretical robustness gap — no code needed.

  3. Underline draw (drawLine, ~line 782) is not gated by the same guard, but drawLine does not go through Constraints(), and an underline under an out-of-bounds glyph is itself out of bounds and clipped — so no crash and no visible artifact. Correctly left alone.

Tests

Good coverage. The two framework...Throws tests double as an early-warning tripwire: if a future Compose version starts clamping internally, they fail and signal the guard can be removed — nice touch, and the comments say exactly that.

Nice work — LGTM.

@kshivang
kshivang merged commit bf22437 into master Jul 11, 2026
5 checks passed
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.

1 participant