Fix bugs introduced in #16821 (custom font fallback)#16993
Conversation
| if (primaryFontName.empty()) | ||
| { | ||
| primaryFontName = name; | ||
| } |
| UpdateFontList(); | ||
| } | ||
| const auto& currentFontList{ CompleteFontList() }; | ||
| fallbackFont = currentFontList.First().Current(); |
| return S_OK; | ||
| } | ||
| CATCH_RETURN(); | ||
| return hr; |
There was a problem hiding this comment.
HRESULT makes it simpler to write the if failed and necessary to retry condition IMO.
| fontCollection = _api.s->font->fontCollection; | ||
| THROW_IF_FAILED(fontCollection->FindFamilyName(fontName.c_str(), &index, &exists)); | ||
| } | ||
| } |
There was a problem hiding this comment.
okay so now, on the first failed font in the list, we'll fall back to the nearby font collection for all the remaining ones
zadjii-msft
left a comment
There was a problem hiding this comment.
assuming the instrumentation build passes, of course
| // Doing it this way is a bit hacky, but it does have the benefit that we can cache a font collection | ||
| // instance across font changes, like when zooming the font size rapidly using the scroll wheel. | ||
| try | ||
| if (FAILED(hr) && _updateWithNearbyFontCollection()) |
There was a problem hiding this comment.
do we technically need this at all anymore? the first failed font will automatically use the nearby collection, down in _resolveFontMetrics, right?
There was a problem hiding this comment.
I tried that at one point and it didn't work. My current leading theory is that DWrite's internal font cache thinks the font exists, tells us it exists, and only once it actually tries to use the font - for instance when we call GetMetrics() - it notices that the font doesn't actually exist / is inaccessible, and then it returns a bad HRESULT.
|
|
FindFontWithLocalizedNameis broken (intentionally andtemporarily until Any use of
FindFontWithLocalizedNameis broken and requires refactoring #16943 is fixed) we have to be extra be carefulnot to return a nullptr
Font.the given font (Cascadia Mono for instance) installed. This requires
us to load the nearby fonts even if there aren't any exceptions.
Validation Steps Performed
src/cascadia/CascadiaResources.build.itemsand remove the
Conditionfor .ttf filesSettings > Defaults > Appearance,enter a non-existing font and hit Save