Skip to content

Fix incomplete system font coverage in the Chinese interface - #1973

Merged
aiongg merged 1 commit into
storytold:mainfrom
yuraizmailov:pr/font-coverage
Oct 23, 2026
Merged

aiongg merged 1 commit into
storytold:mainfrom
yuraizmailov:pr/font-coverage

Conversation

@yuraizmailov

@yuraizmailov yuraizmailov commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Switching to Simplified Chinese can still draw labels as replacement boxes when the system face
chosen by the single Han probe covers only a subset of the catalog. Register additional unique
fallback faces for the actual catalog characters, keeping the original CJK ordering and every UI
font family. No font assets are added.

The existing Simplified Chinese regression failed before this change and passes with catalog-based
fallback coverage. No locale tests are filtered.
This package is independent of mask editing changes.

Validation: CARGO_PROFILE_RELEASE_LTO=off cargo xtask ci (format, release Clippy with warnings denied, complete release
workspace tests, dependency/layer/asset checks and WASM compilation). Native interaction validation
was performed on macOS; Windows/Linux native interaction has not been checked locally.

No dependency or project schema changes. No third-party media assets are added.

LTO is disabled only for local validation to reduce test-executable link time. No tests or gate
phases are excluded, and repository release settings are unchanged.

Local gate result: 3,667 passed, zero failed, 31 ignored across 199 test suites; all gate phases completed.

Summary by CodeRabbit

  • Bug Fixes
    • Improved font fallback for Chinese interfaces by identifying and loading fonts that cover characters used in the translation catalog.
    • Reduced missing or incorrectly rendered CJK characters in the user interface.
    • Improved fallback selection to avoid duplicate font entries and provide more consistent text rendering.

@coderabbitai

coderabbitai Bot commented Oct 20, 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: Repository: storytold/effectcraft/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 70f1730d-9355-4cc2-8b40-841b3735f00a
📥 Commits

Reviewing files that changed from the base of the PR and between 9db7f3b and d4f06b5.

📒 Files selected for processing: 3
  • crates/ui-egui/src/i18n.rs
  • crates/ui-egui/src/i18n/ui.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

Native builds now expose unique characters from translation catalogs. Chinese font fallback setup uses those characters for additional probes, skips characters covered by earlier faces, and assigns each catalog fallback a distinct font-data name.

Changes: Catalog-based CJK font fallbacks

Layer / File(s) Summary
Collect and apply catalog characters
crates/ui-egui/src/i18n/ui.rs, crates/ui-egui/src/i18n.rs, crates/ui-egui/src/theme.rs
The native-only catalog_characters function returns sorted, unique characters from translated catalog values and is re-exported within the crate. For Chinese interfaces, fallback setup probes catalog characters at or above U+2E80 that earlier faces do not cover. Each catalog fallback receives a font-data name containing its probe character. Standard fallback names remain unchanged.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: marcoarnulfo, feiliuem

Merge Risk: ⚪ Minimal · up to d4f06

No actionable merge-blocking risk was identified in the catalog-based fallback changes.

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 and concisely describes the main change: improving system font coverage for the Chinese interface.
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 4 functions across 3 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 commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

This is in integration batch #2257, with a small follow-up: catalog characters in a block that already found no face are skipped, so a machine without a covering font doesn't re-read every font file per block.

@aiongg
aiongg merged commit bc8ddfa into storytold:main Oct 23, 2026
1 check passed
aiongg added a commit that referenced this pull request Oct 23, 2026
…ready found no face (#1973)

On a machine with no font covering the catalog, each uncovered 256-character
block re-read every installed font file on the UI thread; blocks that already
failed are skipped.
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.

2 participants