fix(text): resolve app-bundled font families by passing a DirectWrite font collection - #16339
Open
Collin Schneide (FaithfulAudio) wants to merge 3 commits into
Open
Conversation
… font collection WindowsTextLayoutManager::GetTextLayout passed nullptr as the font collection to CreateTextFormat, which restricts resolution to the system collection. Every app-bundled font family therefore failed to resolve and every codepoint fell back to Segoe UI glyph 0 (.notdef) - icon fonts render as blank or tofu. Measured with a standalone DirectWrite probe replaying this exact call sequence against the eight stock react-native-vector-icons TTFs: with a collection that contains the font each draws a real glyph index (40, 1, 1, 13, 4, 4, 4, 2); with nullptr every one resolves to Segoe UI glyph 0. Adds DWriteAppFontCollection() to DWriteHelpers - the system font set merged with every *.ttf/*.otf under the app's Assets\ and Assets\Fonts\, built once via a magic static, failing closed to nullptr so behaviour is unchanged for apps that bundle no fonts - and passes it at the CreateTextFormat call site. Per-fragment SetFontFamilyName inherits the layout's collection, so only that one call site needed changing. Fixes microsoft#16306. Fixes microsoft#16308 (same root cause - the checksum and space-in-name diagnoses in those issues were both refuted by the probe).
Contributor
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
This was referenced Jul 26, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes Fabric text layout on Windows failing to resolve app-bundled font families by ensuring DirectWrite text formats are created with a font collection that includes both the system font set and fonts shipped with the app under Assets\ and Assets\Fonts\.
Changes:
- Introduces
Microsoft::ReactNative::DWriteAppFontCollection()to build (once) a merged DirectWrite font collection from the system font set plus app-bundled*.ttf/*.otffiles. - Updates Fabric text layout creation (
WindowsTextLayoutManager::GetTextLayout) to pass the merged collection toCreateTextFormatinstead ofnullptr(system-only). - Adds a change file documenting the behavioral fix for the
react-native-windowspackage.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| vnext/Microsoft.ReactNative/Fabric/platform/react/renderer/textlayoutmanager/WindowsTextLayoutManager.cpp | Uses the new app+system DirectWrite font collection when creating text formats so bundled font families resolve. |
| vnext/Microsoft.ReactNative/Fabric/DWriteHelpers.h | Declares DWriteAppFontCollection() helper for reuse. |
| vnext/Microsoft.ReactNative/Fabric/DWriteHelpers.cpp | Implements one-time creation of a merged DirectWrite font collection by enumerating bundled font files. |
| change/react-native-windows-fix-app-bundled-fonts.json | Records the fix in the repo’s change tracking. |
Contributor
|
/azp run |
Andrew Coates (acoates-ms)
approved these changes
Aug 3, 2026
Contributor
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Performance Test ResultsBranch: ✅ Passed161 scenario(s) across 28 suite(s) — no regressionsSectionList
FlatList
TouchableOpacity
ScrollView
TouchableHighlight
Pressable
Modal
Image
ActivityIndicator
Switch
Button
TextInput
View
Text
SectionList.native-perf-test.ts
FlatList.native-perf-test.ts
TouchableHighlight.native-perf-test.ts
TouchableOpacity.native-perf-test.ts
Pressable.native-perf-test.ts
ScrollView.native-perf-test.ts
ActivityIndicator.native-perf-test.ts
TextInput.native-perf-test.ts
Switch.native-perf-test.ts
Button.native-perf-test.ts
Modal.native-perf-test.ts
Image.native-perf-test.ts
View.native-perf-test.ts
Text.native-perf-test.ts
|
Andrew Coates (acoates-ms)
requested changes
Aug 4, 2026
Andrew Coates (acoates-ms)
left a comment
Andrew Coates (acoates-ms)
left a comment
Contributor
There was a problem hiding this comment.
Upon further reflection, the DWriteAppFontCollection is likely accessed at least on two threads in the first use. Can we cache list of fonts so that we can at least avoid doing the file searches the 2nd time?
microsoft-github-policy-service
Bot
added
the
Needs: Author Feedback
… hot path acoates-ms asked whether the font list can be cached so the file searches are not repeated, noting DWriteAppFontCollection is likely reached from more than one thread on first use. The directory enumeration already runs exactly once: s_appFontCollection is a function-local static with a dynamic initializer, so it is initialized a single time and concurrent first callers wait for that initialization rather than racing or repeating it ([stmt.dcl]/4). No call after the first touches the file system. That guarantee was load-bearing but only implied by the old comment, so it is now stated explicitly - including the thread-safety, which was the part worth being able to read off the page. What the old signature *did* cost on every call: GetTextLayout() invokes this once per text measure, and returning winrt::com_ptr by value put an AddRef/Release pair on that path for a pointer whose lifetime is already static and process-long. The accessor now returns a non-owning raw pointer, so the per-measure path has no refcount traffic at all; the static keeps the only reference. The call site drops its .get() accordingly. Not changed, and worth calling out in case it is the concern behind the question: the first call still does the enumeration inline, so whichever thread arrives first pays that cost and any thread arriving during it blocks. Moving that work off the measure path entirely (eager construction at instance setup) is a larger change and would be a behavioral one - happy to do it if that is what you would prefer here.
microsoft-github-policy-service
Bot
removed
the
Needs: Author Feedback
Copilot started reviewing on behalf of
Collin Schneide (FaithfulAudio)
August 4, 2026 17:55
View session
Collin Schneide (FaithfulAudio)
added a commit
to FacilitronWorks/react-native-windows
that referenced
this pull request
Aug 4, 2026
… hot path Twin of the same change on main (microsoft#16339), kept byte-identical so the two branches cannot drift. The directory enumeration already ran exactly once: s_appFontCollection is a function-local static with a dynamic initializer, so it is initialized a single time and concurrent first callers wait for that initialization rather than racing or repeating it ([stmt.dcl]/4). That guarantee is now stated explicitly instead of merely implied. What the old signature did cost on every call: GetTextLayout() invokes this once per text measure, and returning winrt::com_ptr by value put an AddRef/Release pair on that path for a pointer whose lifetime is already static and process-long. The accessor now returns a non-owning raw pointer; the call site drops its .get().
Contributor
Author
|
Thanks — pushed, though with one correction to the premise, so please sanity-check my reasoning here. The file searches already happen exactly once. There was a real per-call cost, just not that one. What I did not change, in case it is the concern behind your question. The first call still does the enumeration inline, so whichever thread arrives first pays it, and a thread arriving during that window blocks. Caching wouldn't help that either — it is a first-use cost, not a repeat cost. Moving it off the measure path entirely (eager construction at instance setup) is a larger and behavioral change, so I would rather you tell me you want it than assume. If the two-thread observation was about that stall rather than repeated I/O, say so and I will do it that way. The 0.83-stable twin (#16344) has the identical change; I diffed the three files to confirm they are byte-identical across the two branches. |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (1)
vnext/Microsoft.ReactNative/Fabric/DWriteHelpers.cpp:35
- Executable paths longer than
MAX_PATHmake this helper return an empty directory, so bundled fonts still fail to resolve for otherwise valid long-path installations.GetModuleFileNameWsupports a larger buffer; use a dynamically sized buffer (or the maximum long-path size) instead of treating truncation as “no app fonts.”
wchar_t modulePath[MAX_PATH]{};
const DWORD length = ::GetModuleFileNameW(nullptr, modulePath, MAX_PATH);
if (length == 0 || length >= MAX_PATH) {
Contributor
|
/azp run |
Contributor
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Member
|
Collin Schneide (@FaithfulAudio) , thank you for your PR! |
…ad race
Two changes, one per reading of the review comment.
1. The list of bundled font files is now its own cached static
(AppFontFilePaths): the directory searches run exactly once per process and
every consumer - including any future path that rebuilds a collection -
reads the cached list and never touches the file system again.
CreateAppFontCollection() contains no enumeration by construction.
2. The genuine first-use thread race was in DWriteFactory() itself: the
existing `if (!s_dwriteFactory) { assign }` lazy-init is a data race when
two threads make first use concurrently - and DWriteAppFontCollection() is
reachable from more than one thread on first use, which makes that race
live rather than theoretical. Converted to a function-local static with a
dynamic initializer (thread-safe by [stmt.dcl]/4; /Zc:threadSafeInit is on
by default and nothing in the RNW build disables it), so concurrent first
callers wait for one initialization instead of racing it.
Also removes the stray `#pragma once` this .cpp carried.
Contributor
Author
|
Done — and digging into it, I think your instinct was pointing at something realer than my earlier reply gave it credit for. The font-file list is now cached in its own right. The genuine first-use thread race was one function up. Also removed the stray The 0.83 twin (#16344) has the identical change — files verified byte-identical across the two branches. Vladimir Morozov (@vmoroz) thanks for the heads-up on the feed lockdown — no urgency on our side; these will keep. Good luck with #16350. |
Copilot started reviewing on behalf of
Collin Schneide (FaithfulAudio)
August 5, 2026 19:50
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (1)
vnext/Microsoft.ReactNative/Fabric/DWriteHelpers.cpp:124
- The system set is added before bundled files, but
IDWriteFontSetBuilderkeeps the first-added family when names collide. As a result, an installed font with the same family name wins and the app’s bundled version (potentially with different icon glyph mappings) is ignored. Add bundled files first, then append the system set so bundled assets take precedence while all other system families remain available.
winrt::com_ptr<::IDWriteFontSet> systemFontSet;
winrt::check_hresult(factory5->GetSystemFontSet(systemFontSet.put()));
winrt::check_hresult(builder->AddFontSet(systemFontSet.get()));
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.
Resolve app-bundled fonts in Fabric text layout
Fixes #16306. Fixes #16308. (Both issues turn out to have this single root cause — see below; both of
their filed diagnoses were wrong, and I am the person who filed them.)
Problem
On the new architecture, no app-bundled font family ever resolves.
fontFamily: 'Ionicons','Material Icons','FontAwesome'and friends all fall back to Segoe UI and render glyph 0(
.notdef), whether the app ships the TTF inAssets\Fonts\or registers it through thewindows.sharedFontspackage-manifest extension. Only fonts installed system-wide work.Root cause
WindowsTextLayoutManager::GetTextLayoutpassesnullptras the font collection toCreateTextFormat(WindowsTextLayoutManager.cppL119):nullptrmeans "resolve against the system font collection only". An app's own font files are not init, so the family is never found, DirectWrite falls back, and every icon codepoint maps to glyph 0.
That is the whole bug. It also explains why the two issues I filed looked like two different bugs and
were both misdiagnosed:
the font,
"Material Icons"resolves and draws glyph 782, and so do"material icons"and"MATERIAL ICONS". DirectWrite's family lookup is case-insensitive and perfectly happy withinterior spaces.
react-native-vector-icons@10.3.0TTFs, byte-unmodified, passIDWriteFontFile::Analyzewithsupported=1, are accepted byIDWriteFontSetBuilder1::AddFontFilewithhr=0, and draw realglyph indices.
Segoe UIglyph 0. Whether.notdefpaints a hollow box or nothing at all is a property of thefallback face, not evidence of two different failures.
Fix
Add
Microsoft::ReactNative::DWriteAppFontCollection()toDWriteHelpers: on first use, build aDirectWrite collection from the system font set merged with every
*.ttf/*.otfunder the appdirectory's
Assets\andAssets\Fonts\, and pass it at theCreateTextFormatcall site instead ofnullptr.Properties that matter for review:
nullptron any failure and when the app bundles no fonts, andnullptris exactly the current behaviour — so an app that ships no fonts, or a machine where thecollection cannot be built, behaves precisely as it does today.
static const(thread-safe magic static). Bundled assetscannot change during the process lifetime.
CreateTextFormatcall needed changing. The per-fragmentSetFontFamilyNameat L274 resolves against the collection the layout already inherited from thetext format, so it picks the app collection up for free.
Validation
What was actually measured. I wrote a standalone DirectWrite probe that replays this exact call
sequence —
CreateTextFormat(including the emptylocaleNamethe real code passes) →CreateTextLayout→Drawwith a recordingIDWriteTextRendererthat reports the resolved face andglyph indices — against the real stock TTFs. Built with VS 2022 (x64) and run on Windows 11
10.0.26200. Probe sources and raw output are attachable; here is the substance.
Every stock
react-native-vector-icons@10.3.0font, unmodified:AnalyzesupportedAddFontFilenullptr(today)hr=0acceptedMaterial Iconshr=0acceptedMaterial Design Iconshr=0acceptedIoniconshr=0acceptedFontAwesomehr=0acceptedFeatherhr=0acceptedOcticonshr=0acceptedEvilIconshr=0acceptedanticonThe codepoint in each row is taken from the face's own
IDWriteFontFace1::GetUnicodeRanges, so it isguaranteed to be one the font claims to map.
Family-string variants, holding the collection and the font constant and changing only the requested
string (MaterialIcons.ttf, drawing U+E7FD):
fontFamilyFindFamilyNameMaterial Iconsmaterial iconsMATERIAL ICONSMaterialIcons" Material Icons"(leading space)"Material Icons "(trailing space)Material Icons(doubled interior space)Interior spaces are fine; case does not matter; leading/trailing/duplicated whitespace is not
normalised by DirectWrite and RNW does not trim the string either. Worth knowing, but not fixed
here — a separate, arguable question.
The API sequence this patch uses (
IDWriteFactory5::CreateFontSetBuilder→GetSystemFontSet→AddFontSet→AddFontFile→CreateFontSet→CreateFontCollectionFromFontSet→ query forIDWriteFontCollection) is the same sequence the probe compiled and ran successfully, so the APIusage is exercised rather than merely plausible.
Honest limits — nothing in RNW was compiled or run:
yarn lint,yarn format:verify,clang-format, typecheck and the build are all unrun. The probe is separate standalone code, not
this patch.
raw.githubusercontent.combytes atc69cf55f67f9b03f467502dac1007ac2d9ebe209;git apply --checkagainst a pristine scratch copy withcore.autocrlf false→ exit 0; realgit apply→ exit 0 with 0 CRLF sequences in all threefiles;
git apply --checkagainst a CRLF-converted working copy withcore.autocrlf true(what anormal checkout produces, since
.gitattributesdeclares*.cpp text eol=crlf) → exit 0.Longest added line is 107 columns, within the 120-column
ColumnLimit— hand-counted, notclang-format-verified.
windows.sharedFontsstate. The probe machine has none ofthese fonts installed, so its
nullptrrows fail for the simple reason that the font is absentfrom the system collection. I therefore cannot claim to have measured the exact configuration in
DirectWrite text layout cannot resolve font families whose names contain spaces — 'Material Icons' renders tofu, same font renamed 'MaterialIcons' renders #16306/Registered icon-font TTFs render blank glyphs until table checksums/table directory are recomputed — silent failure in the DWrite font path #16308, only to have shown that neither spaces nor checksums are the mechanism and that
nullptrcannot resolve a font that is not installed.Assets\/Assets\Fonts\convention is a judgement call, not a measurement. It matcheswhere RNW app templates put font assets and where our production app puts them, but if maintainers
would rather this be an explicit API (an app-settable collection, or something aligned with
Implement IProvideFontInfo to unify font loading #15750's
IProvideFontInfodirection) than a directory convention, that is a reasonable objectionand I will rework it.
GetSystemFontSetplus building a merged collection happens once,on the first text layout. I have not profiled it.
AppDirectory()uses aMAX_PATHbuffer and returns empty (→nullptr→ today's behaviour) if themodule path is longer.
TextInputgoes through RichEdit, not DirectWrite textlayout, so app-bundled fonts there are unaffected by this change. The placeholder is fixed,
because
CreatePlaceholderLayoutroutes throughWindowsTextLayoutManager::GetTextLayout. ThenullptrinScrollViewComponentView.cppL474 is fine as-is — it requests the system fontSegoe Fluent Icons.Prior art
Our production app (Facilitron FIT, RNW 0.83.2) ships a near-identical patch as a
yarnpatch and itresolves app-bundled font families without the
windows.sharedFontsmanifest extension — whichmatters because that extension can be rejected during Store package acceptance. This PR is that patch
generalised and cleaned up for upstream. Related: #15316, #15750, #3463.
Change file
change/react-native-windows-fix-app-bundled-fonts.json,"type": "prerelease"(correct formain,whose
vnext/package.jsonversion is0.0.0-canary.1057; the repo's beachball transform downgradesprereleasetopatchautomatically on released branches).Microsoft Reviewers: Open in CodeFlow