felis-terminal/felis

Watch

Keep a wide pair whole when cells move #23

Merged

natsukium merged 1 commit from fix/wide-pair-shift into main

2026-10-04 18:41:55 +09:00

natsukium commented

2026-10-04 17:50:13 +09:00

Owner

Closes #12. Builds on #21.

The editors that move cells repaired only the span they vacated, never the boundaries of the move, so a wide pair crossing one was split, for a plain 字 too: DCH on its right half left the owner drawing over the next character, and ICH on a full line left an owner with no Spacer at the margin.

Change

Each move first erases the pair that straddles each of its boundaries (erase_pair_across): ICH, DCH, the IRM insert, SL/SR, DECIC/DECDC, DECBI/DECFI, the DECSLRM sub-rectangle scroll, and DECCRA's source and destination edges. The inserted/deleted cell count stays exact, as in kitty. A repair after the move would not work: an owner without a Spacer is also what a refused VS16 widen looks like.
The span a move vacates is blanked without pair widening. The move leaves stale copies of moved cells there, and the #21 probe read one and erased the live pair beside it.
The alternate screen's truncating resize blanks an owner whose Spacer the new edge cuts.
A VS16 widen is refused at the DECSLRM right margin. Reflow lays out a cell as a pair only when a Spacer actually follows, so a refused widen no longer shifts its line right on every resize.
OSC 66: a cut character is cleared by its own block, which covers at least the glyph's natural width (w=1;字 still prints a Spacer). A new sized_block_at replaces find_sized_primary, which found the wrong block in a multi-character run, on the print path too. DECCRA copies a sized block only when its source holds all of it, and erases a destination block it overwrites in part. The saved alternate screen now gets the same unfit-block discard as the live one after a resize.
The widened proptest found stale-tail exposure in four places, all fixed here: SR and DECBI/DECFI copied without materializing the row tail, DECSED raised every row's watermark over it, and the VS16 widen read a stale cell as content.

Out of scope, filed as #24: a single-row shift through an uncut multi-row OSC 66 block still moves only that row of the block. This predates the change.

Verification

snapshot_wide_pairs.rs: 39 new cases cover every repro in the issue, the clamped and outside-margin shifts, the sized-block cases, and the stale-tail regressions. The test renderer now reads cells through the watermark, as every other reader does. 34 of them fail on #21. The other 5 are guards: three shifts that #21's erase widening already handled (ICH on a right half, and ICH and DCH clamped to the line), now handled by the seam erase, and two no-ops with the cursor outside the margins.
every_wide_pair_stays_whole (replaces every_spacer_sits_right_of_a_two_cell_glyph) adds the cell-moving editors, DECSLRM, the alternate screen, DECCRA, and a resize step. It also checks a second invariant: a wide Char is always followed by its Spacer. It passes at 30k cases.
Benches (unicode_throughput, scroll_region): within noise.
GUI before/after (release build): rows with a cut owner no longer push the rest of the row a column right, and the cut glyph is gone.
just check green. sol reviewed the plan in one round and the code in four. Rounds 1 to 3 were BLOCKED on OSC 66 edge cases: a multi-character run, DECCRA, the saved alternate screen, and w narrower than the glyph. Round 4 passed, and its one non-blocking note, u32 bounds arithmetic, is applied.

Closes #12. Builds on #21. The editors that move cells repaired only the span they vacated, never the boundaries of the move, so a wide pair crossing one was split, for a plain `字` too: DCH on its right half left the owner drawing over the next character, and ICH on a full line left an owner with no `Spacer` at the margin. ## Change - Each move first erases the pair that straddles each of its boundaries (`erase_pair_across`): ICH, DCH, the IRM insert, SL/SR, DECIC/DECDC, DECBI/DECFI, the DECSLRM sub-rectangle scroll, and DECCRA's source and destination edges. The inserted/deleted cell count stays exact, as in kitty. A repair after the move would not work: an owner without a `Spacer` is also what a refused VS16 widen looks like. - The span a move vacates is blanked without pair widening. The move leaves stale copies of moved cells there, and the #21 probe read one and erased the live pair beside it. - The alternate screen's truncating resize blanks an owner whose `Spacer` the new edge cuts. - A VS16 widen is refused at the DECSLRM right margin. Reflow lays out a cell as a pair only when a `Spacer` actually follows, so a refused widen no longer shifts its line right on every resize. - OSC 66: a cut character is cleared by its own block, which covers at least the glyph's natural width (`w=1;字` still prints a `Spacer`). A new `sized_block_at` replaces `find_sized_primary`, which found the wrong block in a multi-character run, on the print path too. DECCRA copies a sized block only when its source holds all of it, and erases a destination block it overwrites in part. The saved alternate screen now gets the same unfit-block discard as the live one after a resize. - The widened proptest found stale-tail exposure in four places, all fixed here: SR and DECBI/DECFI copied without materializing the row tail, DECSED raised every row's watermark over it, and the VS16 widen read a stale cell as content. Out of scope, filed as #24: a single-row shift through an uncut multi-row OSC 66 block still moves only that row of the block. This predates the change. ## Verification - `snapshot_wide_pairs.rs`: 39 new cases cover every repro in the issue, the clamped and outside-margin shifts, the sized-block cases, and the stale-tail regressions. The test renderer now reads cells through the watermark, as every other reader does. 34 of them fail on #21. The other 5 are guards: three shifts that #21's erase widening already handled (ICH on a right half, and ICH and DCH clamped to the line), now handled by the seam erase, and two no-ops with the cursor outside the margins. - `every_wide_pair_stays_whole` (replaces `every_spacer_sits_right_of_a_two_cell_glyph`) adds the cell-moving editors, DECSLRM, the alternate screen, DECCRA, and a resize step. It also checks a second invariant: a wide `Char` is always followed by its `Spacer`. It passes at 30k cases. - Benches (`unicode_throughput`, `scroll_region`): within noise. - GUI before/after (release build): rows with a cut owner no longer push the rest of the row a column right, and the cut glyph is gone. - `just check` green. sol reviewed the plan in one round and the code in four. Rounds 1 to 3 were BLOCKED on OSC 66 edge cases: a multi-character run, DECCRA, the saved alternate screen, and `w` narrower than the glyph. Round 4 passed, and its one non-blocking note, u32 bounds arithmetic, is applied.

compare12.png

97 KiB

natsukium added 1 commit

2026-10-04 17:50:13 +09:00

grid: keep a wide pair whole when cells move

darwin / detect relevant changes (pull_request) Successful in 1m11s

Details

darwin / darwin docs-only no-op (pull_request) Has been skipped

Details

windows / detect relevant changes (pull_request) Successful in 1m19s

Details

windows / windows docs-only no-op (pull_request) Has been skipped

Details

darwin / build felis (aarch64-darwin) (pull_request) Successful in 2m11s

Details

fuzz / cargo fuzz smoke (per target) (pull_request) Successful in 3m8s

Details

bench / Criterion full-suite snapshot (pull_request) Has been skipped

Details

fuzz / cargo fuzz nightly long-run (pull_request) Has been skipped

Details

windows / cargo nextest (Windows) (pull_request) Successful in 4m59s

Details

windows / frontend smoke (Windows) (pull_request) Successful in 2m44s

Details

windows / package felis (x86_64-pc-windows-msvc) (pull_request) Has been skipped

Details

pr / nix flake check (pull_request) Successful in 1m10s

Details

pr / wire schema is compatible with the base (pull_request) Successful in 1m13s

Details

bench / Criterion regression gate (pull_request) Successful in 3m35s

Details

pr / cargo build / clippy / test / deny (pull_request) Successful in 2m41s

Details

pr / frontend smoke (x86_64-linux) (pull_request) Successful in 58s

Details

windows / cargo clippy (Windows cross) (pull_request) Successful in 44s

Details

d51bbeecdf

The editors that move cells (ICH, DCH, the IRM insert, SL/SR,
DECIC/DECDC, DECBI/DECFI, the DECSLRM sub-rectangle scroll, DECCRA and
the alternate screen's truncating resize) repaired only the span they
vacated, never the boundaries of the move, so a pair crossing one was
split: DCH on the right half of `字` left the owner drawing over the
next character, and ICH on a full line left an owner with no Spacer
at the margin.

Each move now erases, before it copies, the pair that straddles each
of its boundaries. Repairing afterwards cannot work: an owner without
a Spacer is also what a cluster whose VS16 widen was refused looks
like, so a post-move pass could not tell a cut pair from a legitimate
one-cell glyph. Erasing keeps the inserted/deleted cell count exact,
which curses relies on, as kitty does. An OSC 66 character cut this
way is cleared by its own block. The print path's old primary walk
(`find_sized_primary`) stopped only at a handle boundary, and one run
shares a handle across its characters, so it found the wrong block in
a multi-character run; `sized_block_at` replaces it for both. DECCRA
copies a sized block only when its source holds all of it, and erases
a destination block it overwrites in part. The saved alternate screen
now gets the same unfit-block discard after a resize as the live one,
and the discard blanks a primary whose own natural-width pair the new
edge cut. A sized character's footprint covers at least its natural
width, since `w=1;字` still prints a `Spacer` the scaled block omits.

The vacated span is now blanked without pair widening: the move leaves
stale copies of moved cells in it, and #21's widening probe read a
stale Spacer there and erased the live pair just moved beside it.

Two widening bugs from the same issue: a VS16 at the DECSLRM right
margin put its Spacer outside the margin, and reflow synthesized a
Spacer for a refused widen, shifting the rest of the line right on
every resize. Reflow now takes the pair path only when a Spacer is
actually there.

The widened proptest also found three places that exposed a recycled
row's stale tail: SR and DECBI/DECFI copied cells without materializing
it first, DECSED raised every row's watermark to the full width over
it, and the VS16 widen read a stale tenant as content and refused.

Assisted-by: Claude Code

natsukium merged commit 545992fb3e into main

2026-10-04 18:41:55 +09:00

natsukium deleted branch fix/wide-pair-shift

2026-10-04 18:41:55 +09:00

natsukium referenced this pull request from a commit

2026-10-04 18:41:56 +09:00

Merge pull request 'Keep a wide pair whole when cells move' (#23) from fix/wide-pair-shift into main