Skip to content

ui: the interface prefers the platform's UI faces, rendered text keeps the print ones - #1322

Merged
aiongg merged 1 commit into
storytold:mainfrom
FeiLiuEM:font/screen-ui-families
Oct 23, 2026
Merged

aiongg merged 1 commit into
storytold:mainfrom
FeiLiuEM:font/screen-ui-families

Conversation

@FeiLiuEM

@FeiLiuEM FeiLiuEM commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Windows ships two faces per family for what it draws on screen and what it prints — Meiryo/Meiryo UI, Yu Gothic/Yu Gothic UI, MS Gothic/MS UI Gothic, Microsoft YaHei/Microsoft YaHei UI, Microsoft JhengHei/Microsoft JhengHei UI — and its own link table for Segoe UI (FontLink\SystemLink) picks the UI one. Its kana are narrower and its spacing tighter, which is what an interface at 12 px wants.

The first version of this PR put those faces at the head of the lists in crates/text/src/fonts.rs, and @bflatastic was right to stop it: those same lists feed fonts::fallback_for, which shapes composed frames, so the reorder would have changed how existing projects render. The two orders are separate now.

What changed

  • crates/text/src/fonts.rs is untouched, exactly as main has it. A document keeps rendering in the platform's print order, and a new test pins that: Yu Gothic stays ahead of Yu Gothic UI, and no UI name appears in the lists a document renders with.
  • install_script_fallbacks (the interface's own install) has a candidate list of its own: ["Meiryo UI", "Yu Gothic UI", "MS UI Gothic"] for kana and for the Han ideographs of a Japanese interface, ["Microsoft YaHei UI"] for Simplified Chinese, ["Microsoft JhengHei UI", "PMingLiU"] for Traditional. It is used only when the family resolves, is not missing, and covers the probe; otherwise the shared choice stands. Every other script the install probes has an empty list — those faces draw their script correctly at any size, and their order should not depend on the chrome.
  • PMingLiU stays, with the right reason: it is MingLiU's proportional relative, not a screen sibling, and proportional spacing is what makes the chrome read better in it.
  • the_interface_asks_for_the_platforms_ui_faces pins the interface list, including that the other scripts have none.

What this does not do

It does not make the selection testable on Linux: resolve finds no Meiryo UI here, and the test asserts the list rather than the choice. The path the choice goes through (fonts::resolve + FaceId::missing + Face::has_char) is the same one the neighbouring probes already use.

The PingFang SC/PingFang TC side is untouched: macOS ships no UI sibling for either, so there is nothing to prefer there.

Summary by CodeRabbit

  • Improvements
    • Japanese and Chinese interface text can now use platform-specific UI fonts when those fonts are available and support the required characters.
    • If a suitable UI font isn’t available, text continues to use the existing fallback behavior.

@bflatastic

Copy link
Copy Markdown
Member

Thanks @FeiLiuEM! These fallback lists also drive composition text layout (text/src/layout.rs goes through fallback_for), not just the interface. Putting the UI faces first would change how existing projects with Japanese or Chinese text render on Windows (Meiryo UI's kana are narrower). Could the interface order be kept separate from the order used for rendered text? Also, PMingLiU is the proportional MingLiU rather than a screen face.

@FeiLiuEM

Copy link
Copy Markdown
Contributor Author

Both points are right, thank you.

The layout coupling is the one that matters. fallback_for is the same resolver for composition text, so putting the UI faces first would change how existing Windows projects render — Meiryo UI's narrower kana and all — which is exactly what a fallback list should not do. The interface order should be the one that changes, not the shared one, and I would rather redo this PR that way than have it land as it is. The smallest shape I can see: leave the family lists as main has them and give the text engine a way to resolve with an interface-scoped list first, so theme.rs's install passes its own order while the document path keeps fallback_for as it is. If you would rather the UI faces not be preferred at all, say so and I will drop the reorder entirely and keep only the corrections that are not screen-versus-print.

PMingLiU: you are right and my rationale is wrong — it is MingLiU's proportional sibling, not a screen face in the sense Meiryo UI is. The three font-link pairs I would defend are Meiryo/Meiryo UI, MS Gothic/MS UI Gothic and Yu Gothic/Yu Gothic UI; on the Chinese side the honest statement is that Windows ships a UI variant of YaHei and JhengHei, which is why those two went first, and PMingLiU is in the list for its proportional spacing rather than as a sibling of MingLiU. I will fix the comment with whatever this PR becomes.

@FeiLiuEM FeiLiuEM changed the title text: the CJK fallback lists prefer the screen faces over their print siblings ui: the interface prefers the platform's UI faces, rendered text keeps the print ones Oct 16, 2026
@FeiLiuEM

Copy link
Copy Markdown
Contributor Author

Done that way, and thank you for catching it before it landed.

  • crates/text/src/fonts.rs is back to main — a composed frame renders in the platform's print order, unchanged — and a test pins it (Yu Gothic ahead of Yu Gothic UI; no UI name in the lists a document renders with, so a future reorder fails).
  • The interface has its own list in install_script_fallbacks: ["Meiryo UI", "Yu Gothic UI", "MS UI Gothic"] for kana and for the Han of a Japanese interface, ["Microsoft YaHei UI"] for Simplified, ["Microsoft JhengHei UI", "PMingLiU"] for Traditional. It is used only where the family resolves, is not missing and covers the probe; otherwise fallback_for's choice stands. Every other script probes an empty list, so nothing about their order changes.
  • PMingLiU stays for the reason you gave: proportional spacing, not a sibling of MingLiU.

One thing I could not verify here: on Linux none of those families resolve, so the test asserts the list rather than the selection. On a Windows box with the faces installed the choice goes through the same resolve + has_char pair the other probes use.

@echelon

echelon commented Oct 18, 2026

Copy link
Copy Markdown
Member

Thank you for this contribution! We reviewed it for today's community batch, and it needs a few changes before it can land:

Sloppy in places. cjk_locale() is applied twice (the caller passes cjk_locale(language) and the function calls cjk_locale again). The new test was inserted between the #394 doc comment and its test, so it steals that doc comment. The added fonts.rs test uses panic! in a closure (test code only, but noisy).

Suggested next step: Fix the double cjk_locale and move the test so it doesn't split the #394 doc comment. Rebase after #1395. The face preferences are Windows-only and untestable in CI, so they need a maintainer eyeball.

Once it is updated we will take another look. If you have not already, please join us on Discord: https://discord.gg/artcraft

…s the print ones

Windows ships a UI variant of its Japanese and Chinese families (`Meiryo`/`Meiryo UI`, `Yu
Gothic`/`Yu Gothic UI`, `MS Gothic`/`MS UI Gothic`, `Microsoft YaHei`/`Microsoft YaHei UI`,
`Microsoft JhengHei`/`Microsoft JhengHei UI`) whose kana and kanji are drawn for the screen, and its
own link table for `Segoe UI` picks the UI one for everything it draws. The interface should ask for
it - but the same lists feed `fonts::fallback_for`, which shapes composed frames, so reordering them
would change how existing projects render. The two orders are therefore separate: this leaves
`crates/text/src/fonts.rs` exactly as it is and gives the interface its own candidate list in
`install_script_fallbacks`, which falls back to the shared choice when the platform has no UI variant,
none is installed, or it does not cover the probe.

`PMingLiU` is in the interface list for its proportional spacing, not as a sibling of `MingLiU`: it is
MingLiU's proportional relative.

The rendered order is pinned by a test now: the Japanese list keeps `Yu Gothic` ahead of `Yu Gothic
UI`, and no UI name appears in the lists a document renders with. `the_interface_asks_for_the_
platforms_ui_faces` pins the interface list and checks that every other script the install probes has
none, so a script that draws correctly in any face keeps the shared order.

Review fixes: the locale is mapped once (the call passes the language, the helper maps it), the new
test sits after the storytold#394 test it had split from its doc comment, and the fonts.rs test asserts instead
of panicking inside a closure. Rebased on current main; storytold#1395 has not landed yet, so there is nothing
to rebase after it.
@FeiLiuEM

Copy link
Copy Markdown
Contributor Author

Thank you — all three are fixed, in one commit rebased on current main (2fbcc6b):

  • cjk_locale is applied once now: the call passes language and interface_fallback_candidates maps it.
  • the new theme test sits after installed_script_fallbacks_draw_arabic_hebrew_indic_and_thai_names, so the Bug: Arabic text cannot be entered in layer names #394 doc comment stays with its own test — it was mine that split them.
  • the fonts.rs test no longer panics inside the closure: position(..).unwrap_or(usize::MAX), with the list printed in the assertion message.

On rebasing after #1395: it is still open, so there was nothing to rebase after yet — this is on current main (f36b2b3, 0.12.0). If #1395 lands and touches the same function I will rebase again.

You are right that the face preferences cannot be tested in CI: the tests pin the candidate lists and the rendered order, not the selection, and the body says so. A maintainer eyeball on Windows is the right gate for the rest.

Thanks for the Discord link.

@FeiLiuEM
FeiLiuEM force-pushed the font/screen-ui-families branch from d73775a to 2fbcc6b Compare October 18, 2026 14:32
@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c0f799c6-5cb6-4511-857e-91c8da66adeb
📥 Commits

Reviewing files that changed from the base of the PR and between f3f24ef and 2fbcc6b.

📒 Files selected for processing: 2
  • crates/text/src/fonts.rs
  • crates/ui-egui/src/theme.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

Script fallback installation now checks language-specific interface UI fonts before using the existing fallback lists. Tests cover UI-font candidate selection and confirm that Japanese and Chinese fallback lists keep print faces ahead of UI variants.

Changes: Script font fallbacks

Layer / File(s) Summary
Interface UI-font selection
crates/ui-egui/src/theme.rs
install_script_fallbacks checks script-specific UI-font candidates and uses a candidate only if its face is not missing and covers the probe. Otherwise, it uses fonts::fallback_for.
Fallback ordering tests
crates/ui-egui/src/theme.rs, crates/text/src/fonts.rs
Tests check the Japanese and Chinese UI-font candidate orders, confirm other scripts receive no special candidates, and assert that print faces precede UI variants in fallback lists.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: echelon, bflatastic, marcoarnulfo

Merge Risk: ⚪ Minimal · up to 2fbcc

The interface fallback retains the general fallback when no eligible UI font is found. Windows-specific face selection remains platform-dependent, but no concrete material failure is established; the change is mergeable with normal checks.

### Architecture Summary

Architecture risk: 🟡 Medium · up to 2fbcc

The change affects 1 system.

Changed systems: crates

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — crates (service) was modified; 2 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in crates/text/src/fonts.rs: Adds assertions that the Japanese, Simplified Chinese, and Traditional Chinese fallback lists keep print faces ahead of UI variants, with the expected Chinese list heads and specified UI-only or excluded names absent.
  • observed — Modified behavior in crates/ui-egui/src/theme.rs: install_script_fallbacks now tries language-specific interface UI fonts that resolve to non-missing faces covering each probe, falling back to fonts::fallback_for when none qualify; duplicate-face handling and registration remain unchanged.
  • observed — Modified behavior in crates/ui-egui/src/theme.rs: Added the non-wasm interface_fallback_candidates helper. It selects Japanese UI candidates for Japanese kana and Japanese-interface Han, Microsoft YaHei UI for Simplified Chinese Han, and Microsoft JhengHei UI followed by PMingLiU for Traditional Chinese Han; other inputs return an empty list.
  • observed — Modified behavior in crates/ui-egui/src/theme.rs: The script-font test module now imports interface_fallback_candidates for use in its tests.

Reliability and maintainability

  • inferred — Risk-relevant change factors for crates: blast_radius_1; direct_dependents_1
Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly summarizes the primary change: interface fallback prefers platform UI faces while rendered text preserves print-face ordering.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files.
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 a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

🔧 Clippy (1.98.1)

Clippy execution timed out

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

@aiongg
aiongg merged commit 966f9d9 into storytold:main Oct 23, 2026
1 check passed
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.

4 participants