Repository navigation
fix(core): accented Latin letters draw in a bundled family at its bundled weights - #7299
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Reviewed at 3866e559. Requesting changes: one blocker. The fix itself looks right.
Blocker
- The Comments workflow fails at this head, and the failure comes from this diff. The job "A package file's comment share may not rise" reports:
RAISED comment share: packages/core/src/fonts/deterministicFonts.ts is 9.4% (201/2144), at the base 9.3% (198/2121). Three comment lines were added: the JSDoc onbundledWeightsWithinand the two-line note in the emit loop. Cut them or move the explanation into names. This also conflicts with the checked "Comments follow CONTRIBUTING.md" box.
What I verified
fails without the fix:namesdeterministicFonts-bundledSubset.test.ts> "variable supplement at bundled weights". I ran it locally. It fails withdeterministicFonts.tsfrom main and passes at head. All 24 files insrc/fonts/pass except the@hyperframes/core/fonts/aliasesself-import test, which fails the same way on main because core isn't built locally.- Ordering is safe. The new exact-weight copies are emitted before
bundledRules, so the bundled Latin faces still win where the ranges overlap.
Non-blocking
- The "Why" section doesn't describe the real request. For
css2?family=Outfit:ital,wght@0,100;…;0,900, Google returns a separate face for each weight, all pointing at the same woff2. It does not return one100 900face. The100 900range comes frompartitionWeightRuns:spansCoveredWeightuses strict bounds, so 300→400→500 never breaks, and the latin-ext faces at 400, 700 and 900 merge into one range. The fix still covers this case becausebundledWeightsWithinis inclusive. But the test only mocks the single-range shape. Consider adding the per-weight shape Google actually sends, so the case that really happens is pinned. - Size: each non-Latin variable subset is now embedded at every bundled weight, plus the range. For Inter's normal style the non-Latin subsets are about 180 KB of woff2 together. Without
text=, that adds about 0.5 MB raw per document, more if italics are bundled. Thetext=path keeps this small in the common case. A different fix would avoid it: stop merging a run across a covered endpoint, then drop the range copy. That said, the duplication is a fair price for correct output.
Contributor
There was a problem hiding this comment.
Re-reviewed at c8c021ff. Approved.
- The blocker is resolved. Comments now passes. The diff from
3866e559only removes and shortens comments (1 line added, 3 removed, all indeterministicFonts.ts). The logic and the test are byte-identical, so the earlier result still holds:deterministicFonts-bundledSubset.test.ts> "variable supplement at bundled weights" fails on main and passes with the fix. - My non-blocking notes from the last review still stand. First, the "Why" section describes one
100 900face, but Google actually sends one face per weight, andpartitionWeightRunsmerges them into that range. Second, the fix duplicates each non-Latin subset at every bundled weight. Neither blocks the merge. - At review time, CI, regression, Windows render verification, CodeQL, Player perf and preview-regression were still running on this head. Merge should wait for them.
Contributor
SanHsien
added a commit
to SanHsien/hyperframes
that referenced
this pull request
Nov 6, 2026
…view Adopts heygen-com#7299 (accented Latin letters draw in a bundled family at its bundled weight), heygen-com#7301 (installing Chrome on a busy Windows PC), heygen-com#7262 (cloud render logs a line a minute while a job holds its status), heygen-com#7265 (video starts a second early and catches up hidden) and heygen-com#7305 (check_report names the finding that stopped the check). Player packed budget takes upstream's +1655 from heygen-com#7265 since the player bundles core's media-steer. Defers heygen-com#7302; heygen-com#7261 and heygen-com#7263 do not apply. Watermark advances to 9e2510b. Co-authored-by: Miguel Ángel <miguel.sierra@heygen.com> Co-authored-by: WaterrrForever <miao.yang@heygen.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Letters beyond Latin-1 (č š ć ž đ, ą ę ł ń ś ź ż, ř ě ů, ğ ı İ, ő ű, ă ș ț, Vietnamese, and the rest of Latin Extended) now draw in the requested family at the weights the bundle ships (400, 700, 900 of Inter, Outfit and the other bundled families). Before, they fell back to the next font in the stack (a serif) at those weights while weight 500 drew correctly.
Why
The bundle embeds only Google's
latinsubset per weight. The rest comes from a supplementary Google fetch, which for a variable font answers one face with a weight range (font-weight: 100 900). A browser matches an exact-weight face before it looks at a range face, so at 400, 700 and 900 it picks the embedded face, finds no glyph for the letter, and goes to the next font; it never reaches the range face. Weight 500 has no embedded face, so the range face is the match there.Related work
Builds on the bundled-subset coverage test in
deterministicFonts-bundledSubset.test.ts. No open issue or overlapping PR.How
When a supplement face's weight is a range,
bundledFamilyFaceRulesalso emits it at each bundled weight the range spans, with the same source andunicode-range. Faces with the same family, weight and style form one set in the browser, so the embedded Latin glyphs and the supplement's Latin Extended glyphs are used together.Test plan
fails without the fix:
deterministicFonts-bundledSubset.test.ts> "variable supplement at bundled weights" expects faces at 400, 700, 900 and100 900; main emits only100 900, so it fails there (received[ '100 900' ]).Not exercised: offline runs (no supplement fetch, so those letters have no source at any weight), italics, Windows, macOS.
Size
A lone fix nothing open can carry: no other open PR touches the bundled-family face rules.