Bound a placeholder rectangle to its picture, and drop a call from the print loop

[JohnCampionJr]

Two things found while trial-merging #37 against #38. One is a bug that stops a picture appearing; the other is a call in the print loop that does not need to be there.

A placeholder rectangle was unbounded

A placeholder cell continued the run before it whenever it sat below and to the right of that run's origin — with nothing said about how far. So the second appearance of one image anywhere further down the screen kept the first appearance's origin, worked out its tile as the distance from it, found that outside the picture, and printed the placeholder as a visible character.

The second picture simply did not appear.

That is not an edge case. It is what any client does when it shows one image in two places, and what image.nvim does every time it draws a thumbnail lower down than the last one — which is the use placeholders exist for.

The fix is the missing half of the same test: a cell continues the rectangle only while it falls inside the picture measured from the origin. Past its last row or last column is a new picture starting here, which is what "anything else is a new picture" already meant. Writing tiles out of reading order still works within one rectangle, which is what the combining marks need.

It repairs the sideways case too — two copies side by side along a row had the same problem for the same reason.

Two tests, one per direction. Both fail without the bound.

A call per printed character

TryApplyPlaceholderDiacritic is called for every printed character, ahead of the combining machinery, and its own first line returns false when no placeholder was just written — which is every character of ordinary text. The method is far too big to inline, so that answer cost a real call each time.

Hoisting its own test to the call site is the same test somewhere the JIT can use it. Measured against #38, where Print is the slow path and this sits at the top of it:

	alt-redraw
without the call	181 MiB/s
with the call	158
with the guard	178

Three runs each, same machine, back to back. On main today the loop is slower so the share is smaller, but the call is no more necessary here than there. Behaviour is unchanged — the guard is the method's own precondition, moved.

Tests

1071 pass.

Note on #38

I trial-merged #37 with the perf branch before this landed: two conflicting files, one hunk each, both pure adjacency. EscapeSequenceParser.cs and BufferLine.cs auto-merged despite both branches rewriting them, and SetSingleWidthRun splits with includeOverlays defaulting to false, which is right under the new rule that printing splits only a Sixel. 1118 tests passed on the merge. I will rebase #38 onto current main next.

🤖 Generated with Claude Code

[tomlm]

The bound is a real fix for a real bug, and it is my bug — that unbounded test came in with #37. Verified rather than taken on trust: reverting just the two added clauses on this branch makes both new tests fail with exactly the symptom described, the second rectangle drew nothing, and the full suite passes at 1071 with them.

The reasoning holds too. Two full-size rectangles of one image cannot start within Rows of each other without overlapping on screen, so for the case that matters the bound is exact rather than approximate. The residual — a client writing a rectangle smaller than the picture and repeating it inside that distance — is not distinguishable from a taller single rectangle by any rule that infers position, and a client that means something else has the explicit tile marks to say so. Worth knowing, not worth guarding.

One thing worth fixing on the second commit before it lands.

_placeholderCell is only ever assigned, never cleared — line 1040 and line 1154, and nowhere else. So the guard short-circuits only until the first placeholder is printed, and from then on it is non-null for the life of the terminal and every printed character pays the call again.

That means the 12% is real but narrower than the commit message claims. It holds for a session that never shows a placeholder, which is nearly all of them and is exactly the case worth protecting — a text-only terminal should not pay for a graphics feature it never uses. It does not hold for a terminal running image.nvim, which is the motivating use case in the commit above it. The alt-redraw corpus has no placeholders in it, so the measurement only ever exercised the first case.

Not a correctness problem: TryApplyPlaceholderDiacritic re-checks the row and column and then the run's serial, so a stale value fails those tests rather than misapplying a mark.

The guard's own precondition is "a placeholder was just written", and just is the part not enforced. Clearing the field when a printed character turns out not to be one would make the guard mean what it says and keep the win after the first picture:

if (_placeholderCell is not null && TryApplyPlaceholderDiacritic(codePoint))
    return;

// Whatever this character is, it is not a mark applying to the cell before it, so the next one
// cannot be either. Clearing here is what keeps the guard above meaning "just written".
_placeholderCell = null;

placed after the diacritic attempt and before the ordinary print path, with TryPrintKittyPlaceholder setting it again as it already does. Worth measuring on the same corpus with a placeholder printed first, which should be flat before and match after.

Two smaller notes, neither blocking:

• The hoisted test is half the method's precondition — it also needs _placeholderOrigin. Harmless, since _placeholderCell is the necessary one and the method still checks both, but the comment reads as though the whole guard moved.
• origin.Image is the image as of the origin, and the third diacritic can replace it mid-rectangle; that path updates _placeholderOrigin with the new image, so the bound follows it. Correct as written — noting it because it is the one way Rows/Cols could have gone stale.

Approving on the first commit. The second is right in intent and I would take it either way, but it only does what it says for half the sessions until the field is cleared.