fix(rendering): don't crash the window when a glyph lands outside the canvas#333
Conversation
… 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)
|
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
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
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)
|
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
Minor notes (non-blocking)
Tests Good coverage. The two Nice work — LGTM. |
Problem
Resizing the window — or the update banner appearing and squeezing the terminal — sometimes kills the whole window with a native error dialog:
Root cause
Compose's
drawText(textMeasurer, …)derives text-layout constraints fromcanvasSize - topLeftwithout clamping (verified inTextPainterKt.textLayoutConstraintsbytecode for Compose 1.9.3 — plainfsub/ceil, nocoerceAtLeast(0)). Any glyph whosetopLeftfalls past the right/bottom canvas edge produces negative constraints, andConstraints()throws the message above. Compose Desktop's defaultWindowExceptionHandlerturns that into the fatal error dialog.TerminalCanvasRenderercan 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, sox = visualCol * cellWidthcan passsize.widtheven though the render loop clamps buffer columns tovisibleCols. 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 inProperTerminal("prevents drawText constraint failures") only rejects sub-cell canvases, so this hole remained.Fix
Route all 8 glyph
drawTextcall sites through a newdrawTextClippedhelper that skips draws whosetopLeftis outside the canvas. Those glyphs are clipped by the canvas'sclipToBounds()anyway, so skipping is visually identical. NegativetopLeft(partially scrolled glyphs) keeps drawing — constraints only go invalid past the right/bottom edge.Tests
DrawTextClippedTest(compose-ui desktopTest) pins both sides:IllegalArgumentExceptionon this Compose version for out-of-boundstopLeft(right of / below canvas) — if Compose ever clamps internally, these tests fail and the guard can be droppeddrawTextClippedskips out-of-bounds and edge draws instead of crashing, still renders in-bounds glyphs (pixel-checked), and keeps negative-topLeftdrawsAll compose-ui desktop tests pass.
🤖 Generated with Claude Code