Skip to content

fix(font): fit oversized fallback glyphs to their cell span - #48

Merged
simota merged 1 commit into
mainfrom
fix/glyph-cell-fit
Jul 24, 2026
Merged

fix(font): fit oversized fallback glyphs to their cell span#48
simota merged 1 commit into
mainfrom
fix/glyph-cell-fit

Conversation

@simota

@simota simota commented Jul 24, 2026

Copy link
Copy Markdown
Owner

Summary

丸数字(①②… U+2460+、East-Asian-Ambiguous幅1)がmacOSフォールバックフォントで約2セル幅のグリフとして解決され、隣接セルへ食い込んで潰れて見える問題を修正。

  • fit-to-cell縮小はfc50f8bでNerd Fontアイコン面限定にゲートされていた(Ghosttyパリティ — 本家も通常テキストはオーバーフローする)。
  • DEC-1: 可読性優先で意図的にGhosttyから逸脱。get_or_raster / raster_shaped の両ゲートで全グリフに Some(span * cell_w) を渡す。advance > fit × 1.1 のときのみ縮小するため、収まるグリフ(通常Latin・幅内CJK)は自然ラスタのまま無変更。
  • 逸脱はゲート2箇所+raster.rs docコメントに明記。

Test plan

  • パリティピンテスト2件を反転: oversized_fallback_glyph_is_fit_to_its_single_cell_span(① ≤ span×1.1)、in_width_glyph_is_not_shrunk_by_fit_to_cell(M)
  • 非縮小回帰を追加: in_width_wide_cjk_glyph_is_not_shrunk_by_fit_to_cell(日, span=2)
  • cargo test -p noa-font 66/66 pass・clippy clean・workspace green(noa-ipc/noa-ptyのsandboxアーティファクト除く、sandbox無効で全pass確認済み)
  • 残: 実機GUIでの①非重なり目視

https://claude.ai/code/session_015dZiLfPko15vV8fPLzm3pQ

Circled digits (U+2460+, East-Asian-Ambiguous, width 1) resolve to macOS
fallback fonts whose glyphs advance ~2 cells, overlapping the neighboring
cell. The fit-to-cell shrink existed but was gated to Nerd Font icon
fallback faces only (fc50f8b) for Ghostty parity — Ghostty indeed lets
ordinary text overflow.

DEC-1: deliberately deviate from Ghostty for readability. Both gate sites
(get_or_raster / raster_shaped) now offer every glyph its allotted span;
the 10% tolerance in rasterize_with_variations keeps in-width glyphs at
their natural raster. Parity pin tests inverted, non-shrink regression
tests added for in-width narrow ('M') and wide CJK glyphs.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c2a080cd37

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

// in `get_or_raster`. Every glyph is offered its allotted span, so an
// oversized fallback glyph is downscaled to fit rather than overflowing
// its neighbor; in-width glyphs (advance <= fit * 1.1) are untouched.
let fit_width = Some(f32::from(span) * self.metrics.cell_w);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve promoted two-cell cluster spans when fitting

When this line starts fitting every shaped glyph, it also trusts the renderer-provided span for clusters that noa-grid widened via VS16 or regional-indicator pairing. In the renderer path, that span is still derived from the base scalar's Unicode width and the next shaped cluster, not from the WIDE/spacer layout attrs, so a promoted two-cell cluster such as ☀️ can be passed as span = 1; with a non-color/outline fallback whose advance exceeds one cell, the new fit path shrinks it back to one cell even though the grid reserved two. Please make the shaped span reflect promoted wide clusters before applying fit-to-cell to all glyphs.

Useful? React with 👍 / 👎.

@simota
simota merged commit 742cb43 into main Jul 24, 2026
1 check passed
@simota
simota deleted the fix/glyph-cell-fit branch July 24, 2026 10:05
@simota simota mentioned this pull request Jul 27, 2026
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