Skip to content

Commit

Permalink
Fix nearby fonts for DxEngine again (#16323)
Browse files Browse the repository at this point in the history
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 ✅
  • Loading branch information
lhecker committed Nov 16, 2023
1 parent 86fb9b4 commit b780b44
Showing 1 changed file with 42 additions and 29 deletions.
71 changes: 42 additions & 29 deletions src/renderer/dx/DxFontInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,44 +126,52 @@ void DxFontInfo::SetFromEngine(const std::wstring_view familyName,
try
{
face = _FindFontFace(localeName);
}
CATCH_LOG();

if (!face)
if constexpr (Feature_NearbyFontLoading::IsEnabled())
{
try
{
// If we missed, try looking a little more by trimming the last word off the requested family name a few times.
// Quite often, folks are specifying weights or something in the familyName and it causes failed resolution and
// an unexpected error dialog. We theoretically could detect the weight words and convert them, but this
// is the quick fix for the majority scenario.
// The long/full fix is backlogged to GH#9744
// Also this doesn't count as a fallback because we don't want to annoy folks with the warning dialog over
// this resolution.
while (!face && !_familyName.empty())
if (!face)
{
const auto lastSpace = _familyName.find_last_of(UNICODE_SPACE);

// value is unsigned and npos will be greater than size.
// if we didn't find anything to trim, leave.
if (lastSpace >= _familyName.size())
{
break;
}

// trim string down to just before the found space
// (space found at 6... trim from 0 for 6 length will give us 0-5 as the new string)
_familyName = _familyName.substr(0, lastSpace);

// Try to find it with the shortened family name
_fontCollection = FontCache::GetCached();
face = _FindFontFace(localeName);
}
}
CATCH_LOG();
}
CATCH_LOG();

if constexpr (Feature_NearbyFontLoading::IsEnabled())
if (!face)
{
if (!face)
// If we missed, try looking a little more by trimming the last word off the requested family name a few times.
// Quite often, folks are specifying weights or something in the familyName and it causes failed resolution and
// an unexpected error dialog. We theoretically could detect the weight words and convert them, but this
// is the quick fix for the majority scenario.
// The long/full fix is backlogged to GH#9744
// Also this doesn't count as a fallback because we don't want to annoy folks with the warning dialog over
// this resolution.
while (!face && !_familyName.empty())
{
_fontCollection = FontCache::GetCached();
face = _FindFontFace(localeName);
const auto lastSpace = _familyName.find_last_of(UNICODE_SPACE);

// value is unsigned and npos will be greater than size.
// if we didn't find anything to trim, leave.
if (lastSpace >= _familyName.size())
{
break;
}

// trim string down to just before the found space
// (space found at 6... trim from 0 for 6 length will give us 0-5 as the new string)
_familyName = _familyName.substr(0, lastSpace);

try
{
// Try to find it with the shortened family name
face = _FindFontFace(localeName);
}
CATCH_LOG();
}
}

Expand All @@ -176,7 +184,12 @@ void DxFontInfo::SetFromEngine(const std::wstring_view familyName,
{
_familyName = fallbackFace;

face = _FindFontFace(localeName);
try
{
face = _FindFontFace(localeName);
}
CATCH_LOG();

if (face)
{
_didFallback = true;
Expand Down

0 comments on commit b780b44

Please sign in to comment.