Ink extent through the display tree (PR 1/2)

[kostub]

PR 1 of 2 — Ink extent through the display tree

Part 1 of the ink-aware width + device-pixel size rounding work (issues #213, #98, #57). This PR adds an internal ink extent to the display tree; the label-facing sizing/rounding changes land in PR 2 (which targets this branch).

Plan: docs/plans/2026-07-14-ink-aware-width.md
LLD: docs/lld/2026-07-06-ink-aware-width.md

Goal

Every MTDisplay reports an ink extent (inkWidth) that covers trailing glyph-path ink for any leaf or composite, while width, pen advance, and script positions stay byte-for-byte unchanged. No label/reporting behavior changes in this PR — it is validated entirely at the display-tree level.

Approach

Keep width as the pen advance (layout untouched). Add a stored inkMaxX + read-time inkWidth getter (MAX(width, inkMaxX)) on MTDisplay — the horizontal twin of the existing ink-based ascent/descent. Measure it on leaves, roll it up in recomputeDimensions, and compose it recursively in every composite display's inkWidth getter. No MTTypesetter changes.

Commits (one per plan item)

1. [item 1] Add MTDisplay inkMaxX/inkWidth ink-extent surface
2. [item 2] Measure MTCTLineDisplay ink max-x
3. [item 3] Measure MTTextDisplay ink max-x
4. [item 4] Measure MTGlyphDisplay ink max-x from glyph bbox
5. [item 5] Measure MTGlyphConstructionDisplay ink max-x
6. [item 6] Measure MTHorizontalGlyphAssemblyDisplay ink max-x
7. [item 7] Roll trailing ink up through recomputeDimensions
8. [item 8] Compose fraction inkWidth over numerator/denominator
9. [item 9] Compose radical inkWidth over radicand
10. [item 10] Compose over/underline inkWidth over inner
11. [item 11] Compose accent inkWidth over accentee
12. [item 12] Compose large-op-limits inkWidth over nucleus/limits
13. [item 13] Compose stack inkWidth over base/over/under
14. [item 14] Compose inner inkWidth over delimiters/inner
15. [item 15] Guard inkWidth >= width through post-init width mutation

Testing

New iosMathTests/MTInkWidthTest.m (15 tests): leaf-ink measurement, top-level rollup, per-composite guarantee + coordinate-basis-invariance, layout-invariance guard, and a post-init width-mutation guard. Existing MTTypesetterTest (132 tests) stays green, confirming advance/pen/script layout is unchanged. Full suite: 431/431 passing.

Two test inputs (not asserted mechanisms) were adjusted to actually reach the class under test in Latin Modern Math @ 20pt: item 5 deepened the nested fractions to trigger the multi-part MTGlyphConstructionDisplay; item 13 used \overrightarrow{VV} because the arrow variant width saturated a single V's overhang. Both keep the guarantee and basis-invariance assertions intact.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Added accurate ink-width measurement for rendered mathematical content, accounting for glyph overhangs beyond advance width.
  • Improved ink-width propagation across glyph-based and composite constructs (including text, fractions, radicals, accents, large-operator limits, stacks, and inner/left-right layouts) while preserving existing positioning behavior.
  • Ensured reported bounds remain consistent even when layout widths change after initialization.
• Tests
  • Added MTInkWidthTest with deterministic LaTeX-based assertions covering leaf and composite ink-width rollup behavior.

[gemini-code-assist]

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

[kostub]

Code review: ink-aware width (PR 1)

Reviewed the full diff (838cddc..e19e9cf) against the LLD/plan, and empirically probed the composite getters against live glyph metrics (Latin Modern Math @ 20pt). One correctness gap voids the guarantee for a whole atom class; the rest of the surface is sound.

Strengths

• Leaf coverage is complete: every leaf that draws glyphs/text now populates inkMaxX — MTCTLineDisplay, MTTextDisplay, MTGlyphDisplay, MTGlyphConstructionDisplay, MTHorizontalGlyphAssemblyDisplay. No glyph-drawing leaf is left unmeasured.
• Read-time getter design (MAX(width, inkMaxX)) correctly tracks post-init width mutations (e.g. radical setDegree:, inner width recompute). Verified the invariant inkWidth >= width holds structurally.
• Coordinate basis (child.position.x - self.position.x) + child.inkWidth is correct for fraction, radical, line, large-op, inner (children carry absolute positions via their update…Position methods that add self.position.x) and for stack (setPosition: delta-shifts children, preserving absolute positions). The assertComposite basis-invariance test (s.inkWidth - s.width == b.inkWidth - b.width) is a good guard against a wrong basis.
• MTMathBoxDisplay correctly has no override — matches LLD §6 (lap/phantom deliberately lie about extent).
• MTGlyphConstructionDisplay / MTHorizontalGlyphAssembly fold per-part _positions[i].x rather than assuming 0, so they stay correct even where the comment's "x-offsets are 0" assumption wouldn't hold.
• CoreText usage is memory-safe: CTFontGetBoundingRectsForGlyphs returns by value, no allocation/ownership to manage. shiftDown is a y-only adjustment, so it correctly does not affect x-ink.

Issues

Critical — MTAccentDisplay.inkWidth ignores the accent glyph, under-reporting ink

MTMathListDisplay.m:945-949. The getter folds only self.accentee, but MTAccentDisplay also holds _accent (the accent glyph), which the typesetter positions at an x-offset of skew (makeAccent: → accentPosition = CGPointMake(skew, height), MTTypesetter.m:1990-1995) with its own ink extent. Wide/variant accents (\vec, \hat, \tilde, \bar) and skewed accents on narrow/italic letters push the accent glyph's ink well past accentee.inkWidth, and the getter never accounts for it.

Measured true accent-glyph ink-right vs. the getter's inkWidth (Latin Modern Math @ 20pt):

latex	getter inkWidth	true accent ink-right	clipped by
\vec{j}	8.24	11.34	3.10
\vec{f}	11.04	13.44	2.40
\vec{l}	5.96	7.84	1.88
\hat{f}	11.04	12.92	1.88
\bar{f}	9.80→11.04	12.88	1.84
\tilde{l}	5.96	7.38	1.42

This is exactly the defect class the PR exists to prevent: a trailing accented atom will have its accent ink painted outside the frame that a downstream sizing consumer computes from inkWidth. Because recomputeDimensions rolls up atom.position.x + atom.inkWidth, the under-report propagates to the top-level display too (not masked).

Note both the implementation and the LLD miss this: LLD §2.6 lists only "MTAccentDisplay accentee" as the held child, and §3.3 specifies the getter "over the accentee" only. The _accent leaf itself is measured (MTGlyphDisplay init sets inkMaxX), so the fix is just to fold it in:

- (CGFloat)inkWidth {
    CGFloat result = MAX(self.width, (self.accentee.position.x - self.position.x) + self.accentee.inkWidth);
    if (self.accent) {
        result = MAX(result, (self.accent.position.x - self.position.x) + self.accent.inkWidth);
    }
    return result;
}

Caveat when adding the test: the accent glyph's advance width is often 0 and its raw inkMaxX can be negative, so accent.inkWidth == MAX(width, inkMaxX) clamps to 0 and yields accent.position.x. That's still >= the true painted ink-right in the measured cases (since inkMaxX < 0 there), so the guarantee inkWidth >= true ink holds — but assert against the true bbox-right (accent.position.x - self.position.x + accent.inkMaxX), not merely that the number moved.

Minor — MTAccentDisplay.inkWidth comment is now misleading

MTMathListDisplay.m:947. The comment ("width is set to accentee.width by the typesetter; accentee holds an absolute position") describes only why the accentee term works and implicitly justifies ignoring the glyph. Update it once the accent glyph is folded in.

Minor — accent test only exercises the accentee branch

MTInkWidthTest.m:testAccentInk uses \hat{V}, where the trailing overhang comes from the accentee V, not the accent glyph — so it passes even with the accent-glyph term missing. Add an accent case where the accent glyph is the trailing ink source (e.g. \vec{j} / \vec{f}) so the guarantee is actually pinned for this class.

Assessment

Ready to merge? No — with one fix.

Reasoning: The leaf measurement, rollup, and all non-accent composite bases are correct and well-tested, and layout/advance/scripts are genuinely untouched. But MTAccentDisplay.inkWidth silently under-reports by up to ~3pt for common accents, voiding the ink guarantee for every accented atom — the precise failure mode this feature targets. Fix the accent getter (and add a glyph-driven accent test) before merge.

[coderabbitai]

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 53d033e5-30de-472f-a78c-f12c3d77266e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Changes

Ink Width Measurement

Layer / File(s)	Summary
Ink measurement contract and glyph geometry iosMath/render/internal/MTMathListDisplayInternal.h, iosMath/render/MTMathListDisplay.m	Adds inkMaxX and computed inkWidth, deriving leaf and constructed-glyph ink extents from glyph bounds.
Composite ink propagation iosMath/render/MTMathListDisplay.m	Propagates child ink extents through math lists, fractions, radicals, limits, lines, accents, stacks, and inner displays using translated positions.
Ink width regression coverage iosMathTests/MTInkWidthTest.m, iosMath.xcodeproj/project.pbxproj	Tests leaf measurements, composite propagation, display-tree traversal, width mutations, and the inkWidth >= width invariant; registers the test source in the test target.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CTLine
  participant GlyphDisplay
  participant CompositeDisplay
  participant MTMathListDisplay
  participant MTInkWidthTest
  CTLine->>GlyphDisplay: provide glyph-path bounds
  GlyphDisplay->>CompositeDisplay: expose inkWidth
  CompositeDisplay->>MTMathListDisplay: propagate translated ink extent
  MTMathListDisplay->>MTInkWidthTest: expose final inkWidth and inkMaxX
  MTInkWidthTest->>MTInkWidthTest: verify overhang and width invariants

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly reflects the main change: adding ink extent tracking through the display tree.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/ink-aware-width-pr1

---

_{Comment @coderabbitai help to get the list of available commands.}

[kostub]

@CodeRabbit review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[kostub]

Remove comments like this. the lld file isn't checked in. shouldn't be referenced in code.

[kostub]

Removed in 708b018 — dropped the docs/lld/LLD § references from the two files this PR touches (MTMathListDisplayInternal.h, MTMathListDisplay.m). Note the same LLD § refs also exist in MTMathAtomFactory.m, MTMathList.m/.h, and MTMathListBuilder.m from prior merged PRs; left those out of this diff to keep it focused.

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (1)

iosMath/render/MTMathListDisplay.m (1)

611-617: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate glyph bbox-accumulation loop across MTGlyphConstructionDisplay and MTHorizontalGlyphAssemblyDisplay.

The inkMaxX computation loop is byte-for-byte identical in both initializers. Worth extracting into a shared helper to avoid future drift.

♻️ Proposed refactor

static CGFloat MTInkMaxXForGlyphRun(const CGGlyph* glyphs, const CGPoint* positions, NSInteger count, MTFont* font) {
    CGFloat maxX = 0;
    for (NSInteger i = 0; i < count; i++) {
        CGRect bbox = CTFontGetBoundingRectsForGlyphs(font.ctFont, kCTFontOrientationDefault, &glyphs[i], NULL, 1);
        maxX = MAX(maxX, positions[i].x + CGRectGetMaxX(bbox));
    }
    return maxX;
}

Then both call sites reduce to:

-        CGFloat maxX = 0;
-        for (int i = 0; i < _numGlyphs; i++) {
-            CGRect bbox = CTFontGetBoundingRectsForGlyphs(font.ctFont, kCTFontOrientationDefault, &_glyphs[i], NULL, 1);
-            maxX = MAX(maxX, _positions[i].x + CGRectGetMaxX(bbox));
-        }
-        self.inkMaxX = maxX;
+        self.inkMaxX = MTInkMaxXForGlyphRun(_glyphs, _positions, _numGlyphs, font);

Also applies to: 1056-1061

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@iosMath/render/MTMathListDisplay.m` around lines 611 - 617, Extract the
duplicated inkMaxX glyph-bbox accumulation from the initializers of
MTGlyphConstructionDisplay and MTHorizontalGlyphAssemblyDisplay into a shared
MTInkMaxXForGlyphRun helper. Update both call sites to use the helper while
preserving the existing glyph, position, count, and font inputs and resulting
inkMaxX values.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@iosMath/render/MTMathListDisplay.m`:
- Around line 945-950: Update MTAccentDisplay.inkWidth to include the horizontal
ink extent of both self.accentee and self.accent, taking the maximum alongside
self.width. Preserve the existing absolute-position calculation for accentee and
apply equivalent positioning to the accent glyph so overhanging accents
determine the reported ink width.
- Around line 463-473: Update MTRadicalDisplay.inkWidth to include the degree
child in the ink extent calculation, using the same position-relative width
logic already applied to radicand. Preserve the existing radicand handling and
return the maximum extent across the radical’s own width, radicand, and degree.

In `@iosMathTests/MTInkWidthTest.m`:
- Around line 233-239: Update testPostInitWidthMutationGuard to mutate d.width
after displayFor: returns, then invoke inkWidth and assert the invariant through
assertInkInvariant:. Keep the existing nested-display traversal and tolerance
checks so the test specifically verifies inkWidth reflects post-initialization
width changes rather than only initial construction state.

---

Nitpick comments:
In `@iosMath/render/MTMathListDisplay.m`:
- Around line 611-617: Extract the duplicated inkMaxX glyph-bbox accumulation
from the initializers of MTGlyphConstructionDisplay and
MTHorizontalGlyphAssemblyDisplay into a shared MTInkMaxXForGlyphRun helper.
Update both call sites to use the helper while preserving the existing glyph,
position, count, and font inputs and resulting inkMaxX values.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 87afeb5a-5db0-4956-995d-edf23765fbdd

📥 Commits

Reviewing files that changed from the base of the PR and between 838cddc and e19e9cf.

📒 Files selected for processing (3)

• iosMath/render/MTMathListDisplay.m
• iosMath/render/internal/MTMathListDisplayInternal.h
• iosMathTests/MTInkWidthTest.m

[kostub]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[kostub]

Pushed 708b018 addressing the review. Two items that were in review bodies (no inline thread to reply to):

• Dedup nitpick (duplicate glyph-bbox loop in MTGlyphConstructionDisplay / MTHorizontalGlyphAssemblyDisplay): extracted a shared MTInkMaxXForGlyphRun helper; both call sites now use it.
• Test not registered in xcodeproj (found during review): MTInkWidthTest.m was only running under swift test. Registered it in iosMath.xcodeproj; it now runs under xcodebuild too (16/16).

Verification: full SPM suite 438/438; MTInkWidthTest 16/16 under both swift test and xcodebuild.