Fix noisy missing font fallback logging - #35963
Conversation
Treat missing speculative font candidates as expected fallback misses instead of logging errors, while preserving existing font fallback source behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 35963Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35963" |
|
/review -b feature/enhanced-reviewer |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
MauiBot
left a comment
There was a problem hiding this comment.
AI Review Summary
@jfversluis — new AI review results are available based on this last commit:
6e8408a. To request a fresh review after new comments or commits, comment/review rerun.
🗂️ Review Sessions — click to expand
🚦 Gate — Test Before & After Fix
Gate Result: ⚠️ SKIPPED
Gate skipped (SkipUITests fast test mode). No UI/device tests were run for this pipeline invocation.
📋 Pre-Flight — Context & Validation
Issue: #28502 - Log - Missing Fonts report; #24059 - [Android] AddEmbeddedResourceFont Shows Exception Warning in Output Window
PR: #35963 - Fix noisy missing font fallback logging
Platforms Affected: Windows, Android; iOS requested for this review but PR description explicitly says iOS/MacCatalyst are not in scope after prior investigation.
Files Changed: 3 implementation, 4 test
Key Findings
- PR suppresses noisy missing-font fallback logging for unregistered/system font names while preserving warnings for registered missing app fonts.
- Android fix moves extracted-file handling before asset probing and only logs asset warnings when the font came from a registrar mapping.
- Windows fix avoids
LocalPathon relative URIs and uses app-package resolution before Win2D probing. - Prior iOS gate evidence says candidate tests passed both with and without the PR fix, so iOS does not currently demonstrate this bug.
- GitHub CLI is unauthenticated in this environment; public API/local branch were used for context, and CI status remains undetermined.
Code Review Summary
Verdict: NEEDS_DISCUSSION
Confidence: low
Errors: 0 | Warnings: 0 | Suggestions: 0
Key code review findings:
- Prior gate/test concern remains unresolved/undetermined: public review evidence reports iOS tests pass without the fix and with the fix, so they do not prove the bug.
- Prior inline logger brittleness feedback is fixed:
FontManagerLoggerProvider.IsEnabledcaptures warning-and-above only. - No high-confidence code defects found in the current Windows/Android implementation.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #35963 | Platform-specific Windows/Android suppression of unregistered fallback font logging while preserving registered missing-font warnings | FontManager.Android.cs, FontManager.Windows.cs, FontImageSourceService.Windows.cs, device tests |
Original PR; no iOS code changes. |
🔬 Code Review — Deep Analysis
Code Review — PR #35963
Independent Assessment
What this changes: Reduces noisy font fallback logging on Android/Windows by only warning for registered app-font failures, handling embedded-file paths before Android asset loading, and avoiding Windows relative-URI LocalPath failures for font fallback probing.
Inferred motivation: Unregistered/system fallback names like Segoe UI/OpenSansRegular should not be treated as missing app assets.
Reconciliation with PR Narrative
Author claims: Matches: fixes noisy Windows/Android missing-font fallback logging and Android embedded-resource warnings; no iOS/MacCatalyst scope.
Agreement/disagreement: Code matches the stated scope.
Prior Review Reconciliation
| Prior ❌ Error Finding | Source | Status | Evidence |
|---|---|---|---|
| Gate/tests pass without fix or are insufficient | MauiBot/kubaflo | ❌ Unresolved/undetermined | Latest public reviews still show gate failure on current head; no authenticated CI/test verification available. |
| Inline logger brittleness | Copilot | ✅ Fixed | FontManagerLoggerProvider.IsEnabled now captures warning+ only. |
Blast Radius Assessment
- Runs for all instances: Yes, platform FontManager/font image paths affect all Windows/Android font resolution.
- Startup impact: Possible where controls resolve fonts during initialization.
- Static/shared state: Existing per-manager caches only; no new global state.
CI Status
- Required-check result: undetermined/tool-unavailable (
ghunauthenticated). - Classification: undetermined. Public check-runs show failing
maui-pr, with visible infra-like failures, but required-check status could not be authenticated. - Action taken: attempted
ci-analysis(unavailable), usedazdo-build-investigatorcontext/public API; capped confidence.
Findings
No high-confidence ❌ Error or
Failure-Mode Probing
- Unregistered fallback font: returns fallback candidates without warning.
- Registered missing font: still logs warning.
- Relative Windows font source: no longer calls
LocalPathon relative URI. - Embedded Android extracted font path:
File.Existspath loads before asset warning.
Verdict: NEEDS_DISCUSSION
Confidence: low — platform font plumbing plus unauthenticated/undetermined required CI and unresolved test-gate concern. Code approach appears sound, but this should not be treated as merge-ready without CI/test reconciliation.
🛠️ Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | maui-expert-reviewer | iOS-only registered-font warning gating: only log UIKit font-load warnings when the family resolves through IFontRegistrar. |
❌ Not selected; iOS build-only passed but does not validate the Windows/Android bug | 1 file (FontManager.iOS.cs) |
Different from PR, but PR intentionally removed iOS scope because tests passed without the fix. Not demonstrably better. |
| 2 | maui-expert-reviewer | Windows generation-layer filtering: only emit generated Assets/Fonts/*.ttf/.otf fallback candidates when the package asset exists. |
NETSDK1005; iOS build-only passed but did not compile Windows code |
1 file (FontManager.Windows.cs) |
Higher-level than PR, but behavior riskier because it removes fallback candidates instead of tolerating them. |
| 3 | maui-expert-reviewer | Android asset-existence preflight: use AssetManager.List before Typeface.CreateFromAsset so missing candidates are not exception-driven. |
❌ Not selected; Android build-only passed after namespace fix, iOS build-only passed, but no Android behavioral tests ran | 1 file (FontManager.Android.cs) |
Strongest alternative idea, but still not demonstrably better than PR without device-test proof and carries asset-list semantics risk. |
| PR | PR #35963 | Platform-specific Windows/Android suppression/tolerance: Android checks registrar/file path before asset probing and logs only registered misses; Windows tolerates relative/unresolved font candidates without noisy errors. | 3 implementation files + tests | Original PR remains selected because alternatives were unproven or riskier under available validation. |
Cross-Pollination
| Model/Reviewer | Round | New Ideas? | Details |
|---|---|---|---|
| maui-expert-reviewer | 1 | Yes | Proposed provenance-classified lookup; implemented as try-fix-1 iOS warning gating due requested iOS platform. |
| maui-expert-reviewer | 2 | Yes | Proposed suppressing invalid probes at Windows asset-candidate generation layer after try-fix-1 failed to prove value. |
| maui-expert-reviewer | 3 | Yes | Proposed Android AssetManager.List preflight after try-fix-2 was Windows-validation blocked and potentially riskier. |
| orchestrator | 4 | No | Remaining meaningful approaches would be variations of the same three classes: warning gating, candidate generation filtering, or asset existence preflight. Given iOS cannot reproduce/compile the relevant Windows/Android paths, further variations would not be demonstrably better. |
Exhausted: Yes — for this review environment and requested iOS test platform. The bug is Windows/Android-specific, and iOS builds cannot validate the affected code paths.
Selected Fix: PR's fix — it is narrower and less risky than candidate 2, avoids candidate 3's Android asset-list compatibility risk, and does not add unproven iOS behavior from candidate 1. Candidate 3 is the only alternative worth future consideration if Android behavioral tests can be run.
Per-Candidate Details
try-fix-1
See CustomAgentLogsTmp/PRState/35963/PRAgent/try-fix-1/content.md.
try-fix-2
See CustomAgentLogsTmp/PRState/35963/PRAgent/try-fix-2/content.md.
try-fix-3
See CustomAgentLogsTmp/PRState/35963/PRAgent/try-fix-3/content.md.
Environment Notes
ghwas unauthenticated, so PR context used public GitHub API plus the checked-out review branch.EstablishBrokenBaseline.ps1could not run because unrelated dirty files already existed in the shared worktree. Candidate diffs were applied and restored manually without touching unrelated changes.- The prompt-provided gate result was respected: gate verification was not rerun and
gate/content.mdwas not modified.
🏁 Report — Final Recommendation
Comparative Fix Report — PR #35963
Inputs
pr: submitted PR fix on branchpr-review-35963pr-plus-reviewer: submitted PR fix plus expert reviewer feedbacktry-fix-1: Android asset-existence preflight fromtry-fix-1/content.mdtry-fix-2: iOS registered-font warning gating fromtry-fix-2/content.mdtry-fix-3: Windows asset-candidate filtering fromtry-fix-3/content.md
Note: the aggregate try-fix/content.md table conflicts with the individual try-fix-N/content.md files for candidate numbering. This report uses the individual try-fix-N files as authoritative for candidate IDs.
Candidate Ranking
| Rank | Candidate | Regression status | Assessment |
|---|---|---|---|
| 1 | pr |
Best balance of scope, risk, and relevance. Directly addresses the proven Windows/Android noisy logging roots while preserving registered missing-font warnings and existing fallback behavior. | |
| 2 | pr-plus-reviewer |
pr |
Equivalent to pr because the expert reviewer produced no actionable findings and inline-findings.json is []. Ranked just below pr only because it adds no improvement over the raw submitted fix. |
| 3 | try-fix-1 |
Strongest alternate idea for Android because it avoids exception-driven asset miss probing. Still riskier than the PR because AssetManager.List must exactly match Typeface.CreateFromAsset lookup semantics for root and subfolder assets, and no Android behavioral tests proved it. |
|
| 4 | try-fix-3 |
Plausible Windows-only alternative, but riskier because it removes speculative fallback asset candidates instead of tolerating unresolved ones. Environment did not prove the Windows behavior. | |
| 5 | try-fix-2 |
❌ Not selected; iOS build-only passed but does not reproduce the Windows/Android bug | Does not address the PR's proven Windows/Android root causes. Since prior investigation found iOS tests pass without the fix, this candidate is unproven and out of scope. |
No candidate has stronger regression evidence than the PR. Candidates with blocked, weak, or non-relevant validation are ranked lower than the submitted PR fix.
Winning Candidate
Winner: pr
The submitted PR is the best candidate because it is narrowly targeted to the Windows and Android code paths where the noisy logging root causes were identified. It suppresses unregistered fallback noise without hiding registered app-font registration problems, and it avoids the additional behavioral risk introduced by the alternatives.
Why the Alternatives Do Not Win
pr-plus-reviewer
The expert reviewer found no actionable issues, so this candidate is a no-op relative to pr. It is safe, but it is not a distinct improvement.
try-fix-1
The Android asset preflight approach could be worth future exploration, but it depends on AssetManager.List matching all successful Typeface.CreateFromAsset paths. Without Android behavioral tests for root assets, Fonts/, fonts/, registered misses, unregistered misses, and embedded-resource extraction, it is not safer than the PR.
try-fix-2
The iOS warning-gating approach is out of scope for the demonstrated bug. The PR intentionally removed iOS/MacCatalyst behavior changes because current tests pass without the fix there.
try-fix-3
The Windows asset-candidate filtering approach prevents nonexistent assets from reaching Win2D, but it changes fallback candidate generation itself. The PR's approach is more conservative because it keeps the existing fallback list and only avoids noisy family-name probing for unresolved package assets.
Recommendation
Proceed with the submitted PR fix. Keep inline-findings.json as an empty array and do not apply any reviewer-driven code changes.
🧭 Next Steps — review latest findings
No alternative fix was selected for this run. Review the session findings and CI results before merging.
…tputs are deleted (#33919) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ### Description of Change The Resizetizer copies and registers `MauiFont` / `MauiSplashScreen` assets during the build. Two incremental-build gaps could leave an app packaged **without** its fonts or splash screen: 1. **First build (Android/Tizen)** — item *registration* (`AndroidAsset`, `BundleResource`, …) lived **inside** the incremental `ProcessMauiFonts` target. On a clean build the target's output-inference glob was empty, so the platform items were never registered and fonts were missing until a *second* build. The fix splits registration into an always-run `_CollectMauiFontItems` target that maps font paths predictively from `@(MauiFont)`. 2. **Incremental build (all platforms)** — `ProcessMauiFonts` / `ProcessMauiSplashScreens` tracked freshness with `mauifont.stamp` / `mauisplash.stamp` files. A stamp could stay newer than a generated output that was later deleted (partial `obj` clean or concurrent build), so MSBuild skipped the target and the package shipped without the missing font/splash. The fix replaces stamps with `mauifont.outputs` / `mauisplash.outputs` manifests. `_ReadMauiFontOutputs` / `_ReadMauiSplashOutputs` run before freshness evaluation, delete the manifest when a listed generated output is missing, and each processor uses the manifest as its sole `Outputs`. This makes only the affected processor rerun without re-stamping unchanged generated assets and unnecessarily invalidating downstream consumers such as Android aapt2. This PR **consolidates** #35962 (closed): it drops the font/splash stamps, adds `ProcessMauiSplashScreensDependsOnTargets`, and de-duplicates fonts by intermediate filename before `CreatePartialInfoPlistTask`, so colliding names (for example an app and a `ProjectReference` both shipping `OpenSans.ttf`) do not emit duplicate `UIAppFonts` entries. The related runtime-side symptom (noisy missing-font fallback logging) is intentionally out of scope here and handled separately in #35963. ### Issues Fixed Fixes #23268 Fixes #33092 ### Tests - `ResizetizerTests.FontsAreCopiedToAndroidAssetsOnFirstBuild` — clean Release build of the `maui` template asserts the font lands in the Android `assets` folder on the **first** build, then an incremental build confirms `ProcessMauiFonts` is skipped while the always-run `_CollectMauiFontItems` still registers the asset. - `ResizetizerTests.BuildRegeneratesFontsAndSplashWhenIntermediateOutputsAreMissing` (macOS-gated) — builds Android/iOS/MacCatalyst template targets, deletes only the generated iOS/MacCatalyst `MauiInfo.plist` files and verifies both font processors rerun and restore them, then deletes generated font/splash folders, verifies recovery, and finally verifies a no-op build skips both processors. ### Validation - A focused MSBuild sequence verified initial generation, no-op skipping, regeneration after deleting a recorded output, and a subsequent no-op skip using the same manifest-invalidation protocol. - The full integration workflow could not complete locally because Android workload installation exhausted the shared disk; the updated Build integration test will validate on CI. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Gerald Versluis <939291+jfversluis@users.noreply.github.com> Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
…tputs are deleted (#33919) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ### Description of Change The Resizetizer copies and registers `MauiFont` / `MauiSplashScreen` assets during the build. Two incremental-build gaps could leave an app packaged **without** its fonts or splash screen: 1. **First build (Android/Tizen)** — item *registration* (`AndroidAsset`, `BundleResource`, …) lived **inside** the incremental `ProcessMauiFonts` target. On a clean build the target's output-inference glob was empty, so the platform items were never registered and fonts were missing until a *second* build. The fix splits registration into an always-run `_CollectMauiFontItems` target that maps font paths predictively from `@(MauiFont)`. 2. **Incremental build (all platforms)** — `ProcessMauiFonts` / `ProcessMauiSplashScreens` tracked freshness with `mauifont.stamp` / `mauisplash.stamp` files. A stamp could stay newer than a generated output that was later deleted (partial `obj` clean or concurrent build), so MSBuild skipped the target and the package shipped without the missing font/splash. The fix replaces stamps with `mauifont.outputs` / `mauisplash.outputs` manifests. `_ReadMauiFontOutputs` / `_ReadMauiSplashOutputs` run before freshness evaluation, delete the manifest when a listed generated output is missing, and each processor uses the manifest as its sole `Outputs`. This makes only the affected processor rerun without re-stamping unchanged generated assets and unnecessarily invalidating downstream consumers such as Android aapt2. This PR **consolidates** #35962 (closed): it drops the font/splash stamps, adds `ProcessMauiSplashScreensDependsOnTargets`, and de-duplicates fonts by intermediate filename before `CreatePartialInfoPlistTask`, so colliding names (for example an app and a `ProjectReference` both shipping `OpenSans.ttf`) do not emit duplicate `UIAppFonts` entries. The related runtime-side symptom (noisy missing-font fallback logging) is intentionally out of scope here and handled separately in #35963. ### Issues Fixed Fixes #23268 Fixes #33092 ### Tests - `ResizetizerTests.FontsAreCopiedToAndroidAssetsOnFirstBuild` — clean Release build of the `maui` template asserts the font lands in the Android `assets` folder on the **first** build, then an incremental build confirms `ProcessMauiFonts` is skipped while the always-run `_CollectMauiFontItems` still registers the asset. - `ResizetizerTests.BuildRegeneratesFontsAndSplashWhenIntermediateOutputsAreMissing` (macOS-gated) — builds Android/iOS/MacCatalyst template targets, deletes only the generated iOS/MacCatalyst `MauiInfo.plist` files and verifies both font processors rerun and restore them, then deletes generated font/splash folders, verifies recovery, and finally verifies a no-op build skips both processors. ### Validation - A focused MSBuild sequence verified initial generation, no-op skipping, regeneration after deleting a recorded output, and a subsequent no-op skip using the same manifest-invalidation protocol. - The full integration workflow could not complete locally because Android workload installation exhausted the shared disk; the updated Build integration test will validate on CI. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Gerald Versluis <939291+jfversluis@users.noreply.github.com> Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
|
@kubaflo I could not test my app, but I could test related issue #24059 and I still get the error. Here is the test app with the 35963 PR included. Save and run on Android. |
|
@rokmeglicbit did you test with this: #35963 (comment) ? |
|
@kubaflo yes, it is in the zip project above, the include reference is 35963 Please download .zip and try it yourself. Maybe I did something wrong :( |
…tputs are deleted (#33919) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ### Description of Change The Resizetizer copies and registers `MauiFont` / `MauiSplashScreen` assets during the build. Two incremental-build gaps could leave an app packaged **without** its fonts or splash screen: 1. **First build (Android/Tizen)** — item *registration* (`AndroidAsset`, `BundleResource`, …) lived **inside** the incremental `ProcessMauiFonts` target. On a clean build the target's output-inference glob was empty, so the platform items were never registered and fonts were missing until a *second* build. The fix splits registration into an always-run `_CollectMauiFontItems` target that maps font paths predictively from `@(MauiFont)`. 2. **Incremental build (all platforms)** — `ProcessMauiFonts` / `ProcessMauiSplashScreens` tracked freshness with `mauifont.stamp` / `mauisplash.stamp` files. A stamp could stay newer than a generated output that was later deleted (partial `obj` clean or concurrent build), so MSBuild skipped the target and the package shipped without the missing font/splash. The fix replaces stamps with `mauifont.outputs` / `mauisplash.outputs` manifests. `_ReadMauiFontOutputs` / `_ReadMauiSplashOutputs` run before freshness evaluation, delete the manifest when a listed generated output is missing, and each processor uses the manifest as its sole `Outputs`. This makes only the affected processor rerun without re-stamping unchanged generated assets and unnecessarily invalidating downstream consumers such as Android aapt2. This PR **consolidates** #35962 (closed): it drops the font/splash stamps, adds `ProcessMauiSplashScreensDependsOnTargets`, and de-duplicates fonts by intermediate filename before `CreatePartialInfoPlistTask`, so colliding names (for example an app and a `ProjectReference` both shipping `OpenSans.ttf`) do not emit duplicate `UIAppFonts` entries. The related runtime-side symptom (noisy missing-font fallback logging) is intentionally out of scope here and handled separately in #35963. ### Issues Fixed Fixes #23268 Fixes #33092 ### Tests - `ResizetizerTests.FontsAreCopiedToAndroidAssetsOnFirstBuild` — clean Release build of the `maui` template asserts the font lands in the Android `assets` folder on the **first** build, then an incremental build confirms `ProcessMauiFonts` is skipped while the always-run `_CollectMauiFontItems` still registers the asset. - `ResizetizerTests.BuildRegeneratesFontsAndSplashWhenIntermediateOutputsAreMissing` (macOS-gated) — builds Android/iOS/MacCatalyst template targets, deletes only the generated iOS/MacCatalyst `MauiInfo.plist` files and verifies both font processors rerun and restore them, then deletes generated font/splash folders, verifies recovery, and finally verifies a no-op build skips both processors. ### Validation - A focused MSBuild sequence verified initial generation, no-op skipping, regeneration after deleting a recorded output, and a subsequent no-op skip using the same manifest-invalidation protocol. - The full integration workflow could not complete locally because Android workload installation exhausted the shared disk; the updated Build integration test will validate on CI. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Gerald Versluis <939291+jfversluis@users.noreply.github.com> Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
…tputs are deleted (#33919) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ### Description of Change The Resizetizer copies and registers `MauiFont` / `MauiSplashScreen` assets during the build. Two incremental-build gaps could leave an app packaged **without** its fonts or splash screen: 1. **First build (Android/Tizen)** — item *registration* (`AndroidAsset`, `BundleResource`, …) lived **inside** the incremental `ProcessMauiFonts` target. On a clean build the target's output-inference glob was empty, so the platform items were never registered and fonts were missing until a *second* build. The fix splits registration into an always-run `_CollectMauiFontItems` target that maps font paths predictively from `@(MauiFont)`. 2. **Incremental build (all platforms)** — `ProcessMauiFonts` / `ProcessMauiSplashScreens` tracked freshness with `mauifont.stamp` / `mauisplash.stamp` files. A stamp could stay newer than a generated output that was later deleted (partial `obj` clean or concurrent build), so MSBuild skipped the target and the package shipped without the missing font/splash. The fix replaces stamps with `mauifont.outputs` / `mauisplash.outputs` manifests. `_ReadMauiFontOutputs` / `_ReadMauiSplashOutputs` run before freshness evaluation, delete the manifest when a listed generated output is missing, and each processor uses the manifest as its sole `Outputs`. This makes only the affected processor rerun without re-stamping unchanged generated assets and unnecessarily invalidating downstream consumers such as Android aapt2. This PR **consolidates** #35962 (closed): it drops the font/splash stamps, adds `ProcessMauiSplashScreensDependsOnTargets`, and de-duplicates fonts by intermediate filename before `CreatePartialInfoPlistTask`, so colliding names (for example an app and a `ProjectReference` both shipping `OpenSans.ttf`) do not emit duplicate `UIAppFonts` entries. The related runtime-side symptom (noisy missing-font fallback logging) is intentionally out of scope here and handled separately in #35963. ### Issues Fixed Fixes #23268 Fixes #33092 ### Tests - `ResizetizerTests.FontsAreCopiedToAndroidAssetsOnFirstBuild` — clean Release build of the `maui` template asserts the font lands in the Android `assets` folder on the **first** build, then an incremental build confirms `ProcessMauiFonts` is skipped while the always-run `_CollectMauiFontItems` still registers the asset. - `ResizetizerTests.BuildRegeneratesFontsAndSplashWhenIntermediateOutputsAreMissing` (macOS-gated) — builds Android/iOS/MacCatalyst template targets, deletes only the generated iOS/MacCatalyst `MauiInfo.plist` files and verifies both font processors rerun and restore them, then deletes generated font/splash folders, verifies recovery, and finally verifies a no-op build skips both processors. ### Validation - A focused MSBuild sequence verified initial generation, no-op skipping, regeneration after deleting a recorded output, and a subsequent no-op skip using the same manifest-invalidation protocol. - The full integration workflow could not complete locally because Android workload installation exhausted the shared disk; the updated Build integration test will validate on CI. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Gerald Versluis <939291+jfversluis@users.noreply.github.com> Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
|
Probably already mitigated/fixed by #33919 lets wait for that to land and see if we then still need to do anything here. |
…tputs are deleted (#33919) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ### Description of Change The Resizetizer copies and registers `MauiFont` / `MauiSplashScreen` assets during the build. Two incremental-build gaps could leave an app packaged **without** its fonts or splash screen: 1. **First build (Android/Tizen)** — item *registration* (`AndroidAsset`, `BundleResource`, …) lived **inside** the incremental `ProcessMauiFonts` target. On a clean build the target's output-inference glob was empty, so the platform items were never registered and fonts were missing until a *second* build. The fix splits registration into an always-run `_CollectMauiFontItems` target that maps font paths predictively from `@(MauiFont)`. 2. **Incremental build (all platforms)** — `ProcessMauiFonts` / `ProcessMauiSplashScreens` tracked freshness with `mauifont.stamp` / `mauisplash.stamp` files. A stamp could stay newer than a generated output that was later deleted (partial `obj` clean or concurrent build), so MSBuild skipped the target and the package shipped without the missing font/splash. The fix replaces stamps with `mauifont.outputs` / `mauisplash.outputs` manifests. `_ReadMauiFontOutputs` / `_ReadMauiSplashOutputs` run before freshness evaluation, delete the manifest when a listed generated output is missing, and each processor uses the manifest as its sole `Outputs`. This makes only the affected processor rerun without re-stamping unchanged generated assets and unnecessarily invalidating downstream consumers such as Android aapt2. This PR **consolidates** #35962 (closed): it drops the font/splash stamps, adds `ProcessMauiSplashScreensDependsOnTargets`, and de-duplicates fonts by intermediate filename before `CreatePartialInfoPlistTask`, so colliding names (for example an app and a `ProjectReference` both shipping `OpenSans.ttf`) do not emit duplicate `UIAppFonts` entries. The related runtime-side symptom (noisy missing-font fallback logging) is intentionally out of scope here and handled separately in #35963. ### Issues Fixed Fixes #23268 Fixes #33092 ### Tests - `ResizetizerTests.FontsAreCopiedToAndroidAssetsOnFirstBuild` — clean Release build of the `maui` template asserts the font lands in the Android `assets` folder on the **first** build, then an incremental build confirms `ProcessMauiFonts` is skipped while the always-run `_CollectMauiFontItems` still registers the asset. - `ResizetizerTests.BuildRegeneratesFontsAndSplashWhenIntermediateOutputsAreMissing` (macOS-gated) — builds Android/iOS/MacCatalyst template targets, deletes only the generated iOS/MacCatalyst `MauiInfo.plist` files and verifies both font processors rerun and restore them, then deletes generated font/splash folders, verifies recovery, and finally verifies a no-op build skips both processors. ### Validation - A focused MSBuild sequence verified initial generation, no-op skipping, regeneration after deleting a recorded output, and a subsequent no-op skip using the same manifest-invalidation protocol. - The full integration workflow could not complete locally because Android workload installation exhausted the shared disk; the updated Build integration test will validate on CI. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Gerald Versluis <939291+jfversluis@users.noreply.github.com> Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
…tputs are deleted (#33919) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ### Description of Change The Resizetizer copies and registers `MauiFont` / `MauiSplashScreen` assets during the build. Two incremental-build gaps could leave an app packaged **without** its fonts or splash screen: 1. **First build (Android/Tizen)** — item *registration* (`AndroidAsset`, `BundleResource`, …) lived **inside** the incremental `ProcessMauiFonts` target. On a clean build the target's output-inference glob was empty, so the platform items were never registered and fonts were missing until a *second* build. The fix splits registration into an always-run `_CollectMauiFontItems` target that maps font paths predictively from `@(MauiFont)`. 2. **Incremental build (all platforms)** — `ProcessMauiFonts` / `ProcessMauiSplashScreens` tracked freshness with `mauifont.stamp` / `mauisplash.stamp` files. A stamp could stay newer than a generated output that was later deleted (partial `obj` clean or concurrent build), so MSBuild skipped the target and the package shipped without the missing font/splash. The fix replaces stamps with `mauifont.outputs` / `mauisplash.outputs` manifests. `_ReadMauiFontOutputs` / `_ReadMauiSplashOutputs` run before freshness evaluation, delete the manifest when a listed generated output is missing, and each processor uses the manifest as its sole `Outputs`. This makes only the affected processor rerun without re-stamping unchanged generated assets and unnecessarily invalidating downstream consumers such as Android aapt2. This PR **consolidates** #35962 (closed): it drops the font/splash stamps, adds `ProcessMauiSplashScreensDependsOnTargets`, and de-duplicates fonts by intermediate filename before `CreatePartialInfoPlistTask`, so colliding names (for example an app and a `ProjectReference` both shipping `OpenSans.ttf`) do not emit duplicate `UIAppFonts` entries. The related runtime-side symptom (noisy missing-font fallback logging) is intentionally out of scope here and handled separately in #35963. ### Issues Fixed Fixes #23268 Fixes #33092 ### Tests - `ResizetizerTests.FontsAreCopiedToAndroidAssetsOnFirstBuild` — clean Release build of the `maui` template asserts the font lands in the Android `assets` folder on the **first** build, then an incremental build confirms `ProcessMauiFonts` is skipped while the always-run `_CollectMauiFontItems` still registers the asset. - `ResizetizerTests.BuildRegeneratesFontsAndSplashWhenIntermediateOutputsAreMissing` (macOS-gated) — builds Android/iOS/MacCatalyst template targets, deletes only the generated iOS/MacCatalyst `MauiInfo.plist` files and verifies both font processors rerun and restore them, then deletes generated font/splash folders, verifies recovery, and finally verifies a no-op build skips both processors. ### Validation - A focused MSBuild sequence verified initial generation, no-op skipping, regeneration after deleting a recorded output, and a subsequent no-op skip using the same manifest-invalidation protocol. - The full integration workflow could not complete locally because Android workload installation exhausted the shared disk; the updated Build integration test will validate on CI. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Gerald Versluis <939291+jfversluis@users.noreply.github.com> Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
|
@jfversluis did you try the testing and it works? I guess issue 23268 is duplicate of this pull/35963, so testing above .zip would also confirm issue 23268? |
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Fixes #28502
Fixes #24059
Related reports: #19774, #17872, #13239. Related to the embedded-font warning symptom in #19804, but not the broader NuGet asset packaging issue there.
Description
This reduces noisy missing-font fallback logging when an unregistered font family is resolved as a platform/system fallback rather than as an app font asset.
OpenSansRegularandSegoe UIon the affected Windows/Android font manager paths.Scope
This PR intentionally covers the proven Windows and Android root causes. iOS/MacCatalyst was investigated separately, but the current tests pass without the fix there, so this PR no longer changes iOS/MacCatalyst font manager behavior or claims an iOS/MacCatalyst fix.
Validation
dotnet build src\Core\tests\DeviceTests\Core.DeviceTests.csproj -f net10.0-windows10.0.19041.0 -p:WindowsPackageType=None --no-restore -v:minimal