From a74d6b5f4094156cfc56ec335a17f4b47dbc2248 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Mon, 1 Apr 2024 23:32:06 +0200 Subject: [PATCH 1/3] Fix bugs introduced in #16821 (custom font fallback) --- .../TerminalSettingsEditor/Appearances.cpp | 10 +-- .../ProfileViewModel.cpp | 1 + src/renderer/atlas/AtlasEngine.api.cpp | 65 ++++++++++++------- src/renderer/atlas/AtlasEngine.h | 5 +- 4 files changed, 52 insertions(+), 29 deletions(-) diff --git a/src/cascadia/TerminalSettingsEditor/Appearances.cpp b/src/cascadia/TerminalSettingsEditor/Appearances.cpp index fe9cabc824f..15417166ad2 100644 --- a/src/cascadia/TerminalSettingsEditor/Appearances.cpp +++ b/src/cascadia/TerminalSettingsEditor/Appearances.cpp @@ -419,6 +419,11 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation BOOL hasPowerlineCharacters = FALSE; til::iterate_font_families(fontFace, [&](wil::zwstring_view name) { + if (primaryFontName.empty()) + { + primaryFontName = name; + } + std::wstring* accumulator = nullptr; UINT32 index = 0; @@ -434,11 +439,6 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation break; } - if (primaryFontName.empty()) - { - primaryFontName = name; - } - wil::com_ptr fontFamily; THROW_IF_FAILED(fontCollection->GetFontFamily(index, fontFamily.addressof())); diff --git a/src/cascadia/TerminalSettingsEditor/ProfileViewModel.cpp b/src/cascadia/TerminalSettingsEditor/ProfileViewModel.cpp index a00a077ef5b..d373926b380 100644 --- a/src/cascadia/TerminalSettingsEditor/ProfileViewModel.cpp +++ b/src/cascadia/TerminalSettingsEditor/ProfileViewModel.cpp @@ -187,6 +187,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation UpdateFontList(); } const auto& currentFontList{ CompleteFontList() }; + fallbackFont = currentFontList.First().Current(); for (const auto& font : currentFontList) { if (font.LocalizedName() == name) diff --git a/src/renderer/atlas/AtlasEngine.api.cpp b/src/renderer/atlas/AtlasEngine.api.cpp index c031f67a154..90b3c46bb4e 100644 --- a/src/renderer/atlas/AtlasEngine.api.cpp +++ b/src/renderer/atlas/AtlasEngine.api.cpp @@ -482,32 +482,17 @@ void AtlasEngine::SetWarningCallback(std::functionfont->fontCollection` for a pre-existing font collection, - // before falling back to using the system font collection. This way we can inject our custom one. - // 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()) { - _api.s.write()->font.write()->fontCollection = FontCache::GetCached(); + hr = _updateFont(fontInfoDesired, fontInfo, features, axes); } - CATCH_LOG(); } - try - { - _updateFont(fontInfoDesired, fontInfo, features, axes); - return S_OK; - } - CATCH_RETURN(); + return hr; } void AtlasEngine::UpdateHyperlinkHoveredId(const uint16_t hoveredId) noexcept @@ -536,7 +521,8 @@ void AtlasEngine::_resolveTransparencySettings() noexcept } } -void AtlasEngine::_updateFont(const FontInfoDesired& fontInfoDesired, FontInfo& fontInfo, const std::unordered_map& features, const std::unordered_map& axes) +[[nodiscard]] HRESULT AtlasEngine::_updateFont(const FontInfoDesired& fontInfoDesired, FontInfo& fontInfo, const std::unordered_map& features, const std::unordered_map& axes) noexcept +try { std::vector fontFeatures; if (!features.empty()) @@ -616,9 +602,12 @@ void AtlasEngine::_updateFont(const FontInfoDesired& fontInfoDesired, FontInfo& _resolveFontMetrics(fontInfoDesired, fontInfo, font); font->fontFeatures = std::move(fontFeatures); font->fontAxisValues = std::move(fontAxisValues); + + return S_OK; } +CATCH_RETURN() -void AtlasEngine::_resolveFontMetrics(const FontInfoDesired& fontInfoDesired, FontInfo& fontInfo, FontSettings* fontMetrics) const +void AtlasEngine::_resolveFontMetrics(const FontInfoDesired& fontInfoDesired, FontInfo& fontInfo, FontSettings* fontMetrics) { const auto& faceName = fontInfoDesired.GetFaceName(); const auto requestedFamily = fontInfoDesired.GetFamily(); @@ -659,6 +648,16 @@ void AtlasEngine::_resolveFontMetrics(const FontInfoDesired& fontInfoDesired, Fo BOOL exists = false; THROW_IF_FAILED(fontCollection->FindFamilyName(fontName.c_str(), &index, &exists)); + // In case of a portable build, the given font may not be installed and instead be bundled next to our executable. + if constexpr (Feature_NearbyFontLoading::IsEnabled()) + { + if (!exists && _updateWithNearbyFontCollection()) + { + fontCollection = _api.s->font->fontCollection; + THROW_IF_FAILED(fontCollection->FindFamilyName(fontName.c_str(), &index, &exists)); + } + } + if (!exists) { if (!missingFontNames.empty()) @@ -869,3 +868,25 @@ void AtlasEngine::_resolveFontMetrics(const FontInfoDesired& fontInfoDesired, Fo fontMetrics->colorGlyphs = fontInfoDesired.GetEnableColorGlyphs(); } } + +bool AtlasEngine::_updateWithNearbyFontCollection() +{ + // _resolveFontMetrics() checks `_api.s->font->fontCollection` for a pre-existing font collection, + // before falling back to using the system font collection. This way we can inject our custom one. + // 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. + wil::com_ptr collection; + try + { + collection = FontCache::GetCached(); + } + CATCH_LOG(); + + if (!collection || _api.s->font->fontCollection == collection) + { + return false; + } + + _api.s.write()->font.write()->fontCollection = std::move(collection); + return true; +} diff --git a/src/renderer/atlas/AtlasEngine.h b/src/renderer/atlas/AtlasEngine.h index 5c9f954ae2f..a4269a06f3b 100644 --- a/src/renderer/atlas/AtlasEngine.h +++ b/src/renderer/atlas/AtlasEngine.h @@ -94,8 +94,9 @@ namespace Microsoft::Console::Render::Atlas // AtlasEngine.api.cpp void _resolveTransparencySettings() noexcept; - void _updateFont(const FontInfoDesired& fontInfoDesired, FontInfo& fontInfo, const std::unordered_map& features, const std::unordered_map& axes); - void _resolveFontMetrics(const FontInfoDesired& fontInfoDesired, FontInfo& fontInfo, FontSettings* fontMetrics = nullptr) const; + [[nodiscard]] HRESULT _updateFont(const FontInfoDesired& fontInfoDesired, FontInfo& fontInfo, const std::unordered_map& features, const std::unordered_map& axes) noexcept; + void _resolveFontMetrics(const FontInfoDesired& fontInfoDesired, FontInfo& fontInfo, FontSettings* fontMetrics = nullptr); + bool _updateWithNearbyFontCollection(); // AtlasEngine.r.cpp ATLAS_ATTR_COLD void _recreateAdapter(); From a54e935500dfe028ac48c9729cedf538a830c790 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Mon, 1 Apr 2024 23:42:54 +0200 Subject: [PATCH 2/3] Better comments --- src/renderer/atlas/AtlasEngine.api.cpp | 6 +++++- src/renderer/atlas/AtlasEngine.h | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/renderer/atlas/AtlasEngine.api.cpp b/src/renderer/atlas/AtlasEngine.api.cpp index 90b3c46bb4e..0ec0ed23491 100644 --- a/src/renderer/atlas/AtlasEngine.api.cpp +++ b/src/renderer/atlas/AtlasEngine.api.cpp @@ -869,7 +869,11 @@ void AtlasEngine::_resolveFontMetrics(const FontInfoDesired& fontInfoDesired, Fo } } -bool AtlasEngine::_updateWithNearbyFontCollection() +// Nearby fonts are described a couple times throughout the file. +// This abstraction in particular helps us avoid retrying when it's pointless: +// After all, if the font collection didn't change (no nearby fonts, loading failed, it's already loaded), +// we don't need to try it again. It returns true if retrying is necessary. +[[nodiscard]] bool AtlasEngine::_updateWithNearbyFontCollection() { // _resolveFontMetrics() checks `_api.s->font->fontCollection` for a pre-existing font collection, // before falling back to using the system font collection. This way we can inject our custom one. diff --git a/src/renderer/atlas/AtlasEngine.h b/src/renderer/atlas/AtlasEngine.h index a4269a06f3b..2e787cff5d9 100644 --- a/src/renderer/atlas/AtlasEngine.h +++ b/src/renderer/atlas/AtlasEngine.h @@ -96,7 +96,7 @@ namespace Microsoft::Console::Render::Atlas void _resolveTransparencySettings() noexcept; [[nodiscard]] HRESULT _updateFont(const FontInfoDesired& fontInfoDesired, FontInfo& fontInfo, const std::unordered_map& features, const std::unordered_map& axes) noexcept; void _resolveFontMetrics(const FontInfoDesired& fontInfoDesired, FontInfo& fontInfo, FontSettings* fontMetrics = nullptr); - bool _updateWithNearbyFontCollection(); + [[nodiscard]] bool _updateWithNearbyFontCollection(); // AtlasEngine.r.cpp ATLAS_ATTR_COLD void _recreateAdapter(); From 51fc550e8c8486d08d8e5cb285847a78d1a32d14 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 2 Apr 2024 01:20:41 +0200 Subject: [PATCH 3/3] Fix AuditMode --- src/renderer/atlas/AtlasEngine.api.cpp | 4 ++-- src/renderer/atlas/AtlasEngine.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/renderer/atlas/AtlasEngine.api.cpp b/src/renderer/atlas/AtlasEngine.api.cpp index 0ec0ed23491..9c98a12cf52 100644 --- a/src/renderer/atlas/AtlasEngine.api.cpp +++ b/src/renderer/atlas/AtlasEngine.api.cpp @@ -869,11 +869,11 @@ void AtlasEngine::_resolveFontMetrics(const FontInfoDesired& fontInfoDesired, Fo } } -// Nearby fonts are described a couple times throughout the file. +// Nearby fonts are described a couple of times throughout the file. // This abstraction in particular helps us avoid retrying when it's pointless: // After all, if the font collection didn't change (no nearby fonts, loading failed, it's already loaded), // we don't need to try it again. It returns true if retrying is necessary. -[[nodiscard]] bool AtlasEngine::_updateWithNearbyFontCollection() +[[nodiscard]] bool AtlasEngine::_updateWithNearbyFontCollection() noexcept { // _resolveFontMetrics() checks `_api.s->font->fontCollection` for a pre-existing font collection, // before falling back to using the system font collection. This way we can inject our custom one. diff --git a/src/renderer/atlas/AtlasEngine.h b/src/renderer/atlas/AtlasEngine.h index 2e787cff5d9..6d346d7bfe4 100644 --- a/src/renderer/atlas/AtlasEngine.h +++ b/src/renderer/atlas/AtlasEngine.h @@ -96,7 +96,7 @@ namespace Microsoft::Console::Render::Atlas void _resolveTransparencySettings() noexcept; [[nodiscard]] HRESULT _updateFont(const FontInfoDesired& fontInfoDesired, FontInfo& fontInfo, const std::unordered_map& features, const std::unordered_map& axes) noexcept; void _resolveFontMetrics(const FontInfoDesired& fontInfoDesired, FontInfo& fontInfo, FontSettings* fontMetrics = nullptr); - [[nodiscard]] bool _updateWithNearbyFontCollection(); + [[nodiscard]] bool _updateWithNearbyFontCollection() noexcept; // AtlasEngine.r.cpp ATLAS_ATTR_COLD void _recreateAdapter();