Fix nearby fonts for DxEngine again #16323
Merged
Conversation
This file contains 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
microsoft-github-policy-service
bot
added
the
Issue-Bug
lhecker
commented
Nov 16, 2023
| @@ -126,44 +126,52 @@ void DxFontInfo::SetFromEngine(const std::wstring_view familyName, | |||
| try | |||
| { | |||
| face = _FindFontFace(localeName); | |||
| } | |||
| CATCH_LOG(); | |||
There was a problem hiding this comment.
Suppressing whitespace changes makes this a lot easier.
tl;dr: This PR wraps every single _FindFontFace call into its own try/catch block and moves the font-variant search code down below the nearby search.
DHowett
approved these changes
Nov 16, 2023
zadjii-msft
approved these changes
Nov 16, 2023
DHowett
pushed a commit
that referenced
this pull request
Dec 4, 2023
The nearby font loading has to be outside of the try/catch of the `_FindFontFace` call, because it'll throw for broken font files. But in my previous PR I had overlooked that the font variant loop modifies the only copy of the face name that we got and was in the same try/catch. That's bad, because once we get to the nearby search code, the face name will be invalid. This commit fixes the issue by wrapping each individual `_FindFontFace` call in a try/catch block. Closes #16322 ## Validation Steps Performed * Remove every single copy of Windows Terminal from your system * Manually clean up Cascadia .ttf files because they aren't gone * Destroy your registry by manually removing appx references (fun!) * Put the 4 Cascadia .ttf files into the Dev app AppX directory * Launch * No warning ✅ (cherry picked from commit b780b44) Service-Card-Id: 91114951 Service-Version: 1.19
DHowett
pushed a commit
that referenced
this pull request
Jan 23, 2024
The nearby font loading has to be outside of the try/catch of the `_FindFontFace` call, because it'll throw for broken font files. But in my previous PR I had overlooked that the font variant loop modifies the only copy of the face name that we got and was in the same try/catch. That's bad, because once we get to the nearby search code, the face name will be invalid. This commit fixes the issue by wrapping each individual `_FindFontFace` call in a try/catch block. Closes #16322 ## Validation Steps Performed * Remove every single copy of Windows Terminal from your system * Manually clean up Cascadia .ttf files because they aren't gone * Destroy your registry by manually removing appx references (fun!) * Put the 4 Cascadia .ttf files into the Dev app AppX directory * Launch * No warning ✅ (cherry picked from commit b780b44) Service-Card-Id: 91114950 Service-Version: 1.18
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.
The nearby font loading has to be outside of the try/catch of the
_FindFontFacecall, because it'll throw for broken font files.But in my previous PR I had overlooked that the font variant loop
modifies the only copy of the face name that we got and was in the
same try/catch. That's bad, because once we get to the nearby search
code, the face name will be invalid. This commit fixes the issue by
wrapping each individual
_FindFontFacecall in a try/catch block.Closes #16322
Validation Steps Performed