LT-22638: Show OpenType feature names and multi-valued variants - #1029
LT-22638: Show OpenType feature names and multi-valued variants#1029jasonleenaylor wants to merge 2 commits into
Conversation
- Read the font-supplied names and named options from GSUB featureParams and the name table so features are comprehensible and character variants are selectable, as with Graphite. - Add OpenTypeFontFeatureInfoReader adapted from Paratext's OpenTypeFeatures.Ttf. - Add OpenTypeFeatureCatalog registered-feature hidden and default-on classification plus English names. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
NUnit Tests 1 files ± 0 1 suites ±0 12m 12s ⏱️ + 3m 59s Results for commit 7a4be53. ± Comparison against base commit ac89f08. This pull request removes 8 and adds 1726 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1029 +/- ##
==========================================
+ Coverage 33.02% 40.91% +7.89%
==========================================
Files 1202 1558 +356
Lines 278232 366731 +88499
Branches 37169 40406 +3237
==========================================
+ Hits 91880 150053 +58173
- Misses 158502 187435 +28933
- Partials 27850 29243 +1393
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Four merge issues; details are inline:
- Preserve
cvNNoption positions when labels cannot decode. - Align default-on features with verified renderer behavior, spec, and docs; hide
size. - Move user-visible catalog names to
.resx. - Add default-on persistence tests: unset stays absent; unchecked writes
liga=0.
Next: address or explicitly defer each inline finding.
|
Three follow-ups:
Next: decide which items belong in this PR versus Jira. |
|
Consolidated into the follow-up summary. |
1 similar comment
|
Consolidated into the follow-up summary. |
There was a problem hiding this comment.
Three follow-up findings; details are inline:
- OpenType character variants after option 31 are unreachable.
- Same-tag records can expose one script's labels and values in another writing system.
s_tableReaderis read outside the lock guarding test-time replacement.
Next: fix or explicitly accept each limitation.
|
Correction incorporated into the follow-up summary. |
2 similar comments
|
Correction incorporated into the follow-up summary. |
|
Correction incorporated into the follow-up summary. |
An undecodable character-variant option name was dropped from the option list, which renumbered every option after it. That index is the value persisted to the writing system, so choosing one variant could apply a different glyph. Slots now keep their position and the UI numbers any the font did not name; a font that names none of them still falls back to a binary toggle. The default-on set held nine tags where the proposal, design and docs name four. Dropped rand, chws, cpsp and halt, and hid size, which references no lookups at all, so size=1 could never have done anything. A test now pins the exact set instead of asserting one tag at a time, which is why five extra tags survived. IsDefaultOn is the one place this provider is not equivalent to the Graphite one. GraphiteFontFeatureProvider reads defaultValue out of the font, and the same font table drives the Graphite shaper, so the checkbox and the renderer cannot disagree. OpenType records no per-feature default anywhere, so this provider asserts one from a static table. Marking a tag on only sets the initial checkbox for a feature the writing system does not mention: wrongly on, it displays as enabled while the text is unaffected and the user's first click writes tag=0, so the gesture meant to enable it disables it; wrongly off, the first click writes tag=1, a no-op if the renderer applies the feature anyway. The OpenType registry does suggest cpsp, rand, chws and halt be on by default, but that instructs an application to apply a feature, which marking a tag default-on prevents by leaving the string silent. Docs/opentype-font-features.md records the reasoning; LT-22774 measures what the renderer actually applies. Moved the 57 remaining visible feature names into FwCoreDlgControls.resx, taking it from 14 keys to 71, so no user-visible name is English-only in C#. AGENTS.md requires this and crowdin.json already globs Src/**/*.resx. Swallowed parse failures now report through the font-feature trace switches, so a malformed font is distinguishable from one with no features. Option counts above the menu ceiling report too rather than truncating in silence; no font we have measured declares more than four. The feature cache handed out its own array behind an IReadOnlyList, which a caller could cast back and write through, so it now stores a ReadOnlyCollection. The table reader and a generation counter are read together under the cache lock, so a discovery that began under one reader cannot seed the cache after the reader changed. Documented the script-blind deduplication as an accepted limitation with the reason: across 192 installed fonts, 176 cv/ss tags appear more than once and every duplicate carries identical metadata.
Comment hygiene (advisory)57 comment-style violation(s) in the lines this branch adds since
32 more not listed here -- see this check's log. Fix them per |
|
Round two. All nine inline comments answered, seven with code changes and two documented as accepted limitations. Details are in the threads; this is the method and the parts that don't belong in any one of them. Fixed: cvNN option positions, the default-on set plus a test pinning it, Font survey. Several of these came down to what fonts actually do, so I measured rather than reasoned. 192 font files: Windows fonts, the per-user font store, Verification. LT-22774 covers what's deferred: confirming the default-on set against measured renderer behavior, and a test that every visible feature resolves a resx label. Fix version FW 9.3, related to LT-22638. The blocker there is that
On your three follow-ups: defaults are aligned with the docs and pinned, with the measurement in LT-22774. Names are in resx, and I don't know whether the bundled fonts carry reusable translations, so treat that as still open. The UI follow-up for a selection summary and grouped live preview isn't filed. Say the word and I'll file it, or take it yourself if you have the shape in mind. Nothing is resolved from my side. Items 7 and 8 are deferrals and 2 is partly one, so they're yours to close or push back on. |
|
Round three. All nine threads are answered and resolved: seven fixes verified in a worktree build ( The follow-up is filed: LT-22792 "Font Features: show the set features together and preview them live", linked Related to LT-22638 and LT-22774. It also carries the note that an Avalonia font dialog will want the same 71 names. Four things before merge, all yours, one cleanup commit:
Next: Jason rebases and pushes the cleanup commit; I re-review the diff of that commit only. |
3 participants
Summary
Fixes LT-22638. Makes OpenType font
features usable in Font Options: features show human-readable names from the
font (or a registered-feature catalog), and character variants with multiple
named alternates are selectable instead of a single On/Off toggle — parity with
the existing Graphite experience, with no change to the dropdown menu, so both
the Writing System and Styles font dialogs inherit it.
What changed
OpenTypeFontFeatureInfoReader(FwUtils) — parses GSUB/GPOS featurelists,
featureParams, and thenametable via a table-source delegate (GDIGetFontDatain the app, font-file bytes in tests). Bounds-checked; malformedfonts degrade to tag-only records. Adapted from Paratext's
OpenTypeFeatures.Ttf.OpenTypeFeatureCatalog(FwUtils) — classifies registered features(hidden / default-on) and supplies English names, seeded from Paratext and
audited against the OpenType registry (
dligvisible,aalthidden,kerndefault-on).
FontFeaturesButton— character variantswith named options become "None + option" submenus stored as
cvNN=k; labelsresolve font-supplied → resx → catalog → numbered fallback; default-on features
initialize enabled; hidden features filtered.
aalt,ccmp) and entries orphaned by the rewrite (ss01–ss05,ValueOff/ValueOn).Docs/opentype-font-features.mdupdated.Storage, rendering, and CSS/Word export are untouched — the renderer-neutral
tag=valuestring already carried multi-values.Testing
cv43"Capital Eng" + 3options,
ss01"Single-story a and g", GPOSmark/mkmk) and ScheherazadeNew (
cv70"Damma").priority, hidden filtering,
cv43=2round-trip, resx↔catalog consistency.catalog/info 100%, reader 86%.
🤖 Generated with Claude Code
This change is