-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Windows 10 support for nearby font loading #12554
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,8 @@ | |
#include "pch.h" | ||
#include "AtlasEngine.h" | ||
|
||
#include "../base/FontCache.h" | ||
|
||
// #### NOTE #### | ||
// If you see any code in here that contains "_r." you might be seeing a race condition. | ||
// The AtlasEngine::Present() method is called on a background thread without any locks, | ||
|
@@ -227,7 +229,7 @@ try | |
} | ||
#endif | ||
|
||
_resolveFontMetrics(fontInfoDesired, fontInfo); | ||
_resolveFontMetrics(nullptr, fontInfoDesired, fontInfo); | ||
return S_OK; | ||
} | ||
CATCH_RETURN() | ||
|
@@ -401,7 +403,50 @@ void AtlasEngine::ToggleShaderEffects() noexcept | |
} | ||
|
||
[[nodiscard]] HRESULT AtlasEngine::UpdateFont(const FontInfoDesired& fontInfoDesired, FontInfo& fontInfo, const std::unordered_map<std::wstring_view, uint32_t>& features, const std::unordered_map<std::wstring_view, float>& axes) noexcept | ||
try | ||
{ | ||
static constexpr std::array fallbackFaceNames{ static_cast<const wchar_t*>(nullptr), L"Consolas", L"Lucida Console", L"Courier New" }; | ||
auto it = fallbackFaceNames.begin(); | ||
const auto end = fallbackFaceNames.end(); | ||
|
||
for (;;) | ||
{ | ||
try | ||
{ | ||
_updateFont(*it, fontInfoDesired, fontInfo, features, axes); | ||
return S_OK; | ||
} | ||
catch (...) | ||
{ | ||
++it; | ||
if (it == end) | ||
{ | ||
RETURN_CAUGHT_EXCEPTION(); | ||
} | ||
else | ||
{ | ||
LOG_CAUGHT_EXCEPTION(); | ||
} | ||
} | ||
} | ||
} | ||
|
||
void AtlasEngine::UpdateHyperlinkHoveredId(const uint16_t hoveredId) noexcept | ||
{ | ||
_api.hyperlinkHoveredId = hoveredId; | ||
} | ||
|
||
#pragma endregion | ||
|
||
void AtlasEngine::_resolveAntialiasingMode() noexcept | ||
{ | ||
// If the user asks for ClearType, but also for a transparent background | ||
// (which our ClearType shader doesn't simultaneously support) | ||
// then we need to sneakily force the renderer to grayscale AA. | ||
const auto forceGrayscaleAA = _api.antialiasingMode == D2D1_TEXT_ANTIALIAS_MODE_CLEARTYPE && !_api.backgroundOpaqueMixin; | ||
_api.realizedAntialiasingMode = forceGrayscaleAA ? D2D1_TEXT_ANTIALIAS_MODE_GRAYSCALE : _api.antialiasingMode; | ||
} | ||
Comment on lines
+433
to
+447
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code isn't new. It's just moved up from down below to group the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this makes it difficult to review the diff -- can you point out the part that fixed the fallback issue you mentioned? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, saying that I moved it might've been an overstatement. |
||
|
||
void AtlasEngine::_updateFont(const wchar_t* faceName, const FontInfoDesired& fontInfoDesired, FontInfo& fontInfo, const std::unordered_map<std::wstring_view, uint32_t>& features, const std::unordered_map<std::wstring_view, float>& axes) | ||
{ | ||
std::vector<DWRITE_FONT_FEATURE> fontFeatures; | ||
if (!features.empty()) | ||
|
@@ -478,7 +523,7 @@ try | |
} | ||
|
||
const auto previousCellSize = _api.fontMetrics.cellSize; | ||
_resolveFontMetrics(fontInfoDesired, fontInfo, &_api.fontMetrics); | ||
_resolveFontMetrics(faceName, fontInfoDesired, fontInfo, &_api.fontMetrics); | ||
_api.fontFeatures = std::move(fontFeatures); | ||
_api.fontAxisValues = std::move(fontAxisValues); | ||
|
||
|
@@ -489,37 +534,21 @@ try | |
_api.cellCount = _api.sizeInPixel / _api.fontMetrics.cellSize; | ||
WI_SetFlag(_api.invalidations, ApiInvalidations::Size); | ||
} | ||
|
||
return S_OK; | ||
} | ||
CATCH_RETURN() | ||
|
||
void AtlasEngine::UpdateHyperlinkHoveredId(const uint16_t hoveredId) noexcept | ||
{ | ||
_api.hyperlinkHoveredId = hoveredId; | ||
} | ||
|
||
#pragma endregion | ||
|
||
void AtlasEngine::_resolveAntialiasingMode() noexcept | ||
void AtlasEngine::_resolveFontMetrics(const wchar_t* requestedFaceName, const FontInfoDesired& fontInfoDesired, FontInfo& fontInfo, FontMetrics* fontMetrics) const | ||
{ | ||
// If the user asks for ClearType, but also for a transparent background | ||
// (which our ClearType shader doesn't simultaneously support) | ||
// then we need to sneakily force the renderer to grayscale AA. | ||
const auto forceGrayscaleAA = _api.antialiasingMode == D2D1_TEXT_ANTIALIAS_MODE_CLEARTYPE && !_api.backgroundOpaqueMixin; | ||
_api.realizedAntialiasingMode = forceGrayscaleAA ? D2D1_TEXT_ANTIALIAS_MODE_GRAYSCALE : _api.antialiasingMode; | ||
} | ||
|
||
void AtlasEngine::_resolveFontMetrics(const FontInfoDesired& fontInfoDesired, FontInfo& fontInfo, FontMetrics* fontMetrics) const | ||
{ | ||
auto requestedFaceName = fontInfoDesired.GetFaceName().c_str(); | ||
const auto requestedFamily = fontInfoDesired.GetFamily(); | ||
auto requestedWeight = fontInfoDesired.GetWeight(); | ||
auto requestedSize = fontInfoDesired.GetEngineSize(); | ||
|
||
if (!requestedFaceName) | ||
{ | ||
requestedFaceName = L"Consolas"; | ||
requestedFaceName = fontInfoDesired.GetFaceName().c_str(); | ||
if (!requestedFaceName) | ||
{ | ||
requestedFaceName = L"Consolas"; | ||
} | ||
} | ||
if (!requestedSize.Y) | ||
{ | ||
|
@@ -530,16 +559,15 @@ void AtlasEngine::_resolveFontMetrics(const FontInfoDesired& fontInfoDesired, Fo | |
requestedWeight = DWRITE_FONT_WEIGHT_NORMAL; | ||
} | ||
|
||
wil::com_ptr<IDWriteFontCollection> systemFontCollection; | ||
THROW_IF_FAILED(_sr.dwriteFactory->GetSystemFontCollection(systemFontCollection.addressof(), false)); | ||
auto fontCollection = FontCache::GetCached(); | ||
|
||
u32 index = 0; | ||
BOOL exists = false; | ||
THROW_IF_FAILED(systemFontCollection->FindFamilyName(requestedFaceName, &index, &exists)); | ||
THROW_IF_FAILED(fontCollection->FindFamilyName(requestedFaceName, &index, &exists)); | ||
THROW_HR_IF(DWRITE_E_NOFONT, !exists); | ||
|
||
wil::com_ptr<IDWriteFontFamily> fontFamily; | ||
THROW_IF_FAILED(systemFontCollection->GetFontFamily(index, fontFamily.addressof())); | ||
THROW_IF_FAILED(fontCollection->GetFontFamily(index, fontFamily.addressof())); | ||
|
||
wil::com_ptr<IDWriteFont> font; | ||
THROW_IF_FAILED(fontFamily->GetFirstMatchingFont(static_cast<DWRITE_FONT_WEIGHT>(requestedWeight), DWRITE_FONT_STRETCH_NORMAL, DWRITE_FONT_STYLE_NORMAL, font.addressof())); | ||
|
@@ -601,6 +629,7 @@ void AtlasEngine::_resolveFontMetrics(const FontInfoDesired& fontInfoDesired, Fo | |
// NOTE: From this point onward no early returns or throwing code should exist, | ||
// as we might cause _api to be in an inconsistent state otherwise. | ||
|
||
fontMetrics->fontCollection = std::move(fontCollection); | ||
fontMetrics->fontName = std::move(fontName); | ||
fontMetrics->fontSizeInDIP = static_cast<float>(fontSizeInPx / static_cast<double>(_api.dpi) * 96.0); | ||
fontMetrics->baselineInDIP = static_cast<float>(baseline / static_cast<double>(_api.dpi) * 96.0); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
// Copyright (c) Microsoft Corporation. | ||
// Licensed under the MIT license. | ||
|
||
#pragma once | ||
|
||
namespace Microsoft::Console::Render::FontCache | ||
{ | ||
namespace details | ||
{ | ||
inline const std::vector<wil::com_ptr<IDWriteFontFile>>& getNearbyFontFiles(IDWriteFactory5* factory5) | ||
{ | ||
static const auto fontFiles = [=]() { | ||
std::vector<wil::com_ptr<IDWriteFontFile>> files; | ||
|
||
const std::filesystem::path module{ wil::GetModuleFileNameW<std::wstring>(nullptr) }; | ||
const auto folder{ module.parent_path() }; | ||
|
||
for (const auto& p : std::filesystem::directory_iterator(folder)) | ||
{ | ||
if (til::ends_with(p.path().native(), L".ttf")) | ||
{ | ||
wil::com_ptr<IDWriteFontFile> fontFile; | ||
if (SUCCEEDED_LOG(factory5->CreateFontFileReference(p.path().c_str(), nullptr, fontFile.addressof()))) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so it's safe to use these objects on different threads because it is a SHARED-type factory? neato There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
{ | ||
files.emplace_back(std::move(fontFile)); | ||
} | ||
} | ||
} | ||
|
||
files.shrink_to_fit(); | ||
return files; | ||
}(); | ||
return fontFiles; | ||
} | ||
|
||
inline wil::com_ptr<IDWriteFontCollection> getFontCollection(bool forceUpdate) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm kinda just trusting that this bit of the code will work as expected. I don't totally know the mechanical differences between a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. BTW the new code is what |
||
{ | ||
wil::com_ptr<IDWriteFactory> factory; | ||
THROW_IF_FAILED(DWriteCreateFactory(DWRITE_FACTORY_TYPE_SHARED, __uuidof(factory), reinterpret_cast<::IUnknown**>(factory.addressof()))); | ||
|
||
wil::com_ptr<IDWriteFontCollection> systemFontCollection; | ||
THROW_IF_FAILED(factory->GetSystemFontCollection(systemFontCollection.addressof(), forceUpdate)); | ||
|
||
if constexpr (Feature_NearbyFontLoading::IsEnabled()) | ||
{ | ||
// IDWriteFactory5 is supported since Windows 10, build 15021. | ||
const auto factory5 = factory.try_query<IDWriteFactory5>(); | ||
if (!factory5) | ||
{ | ||
return systemFontCollection; | ||
} | ||
|
||
const auto& nearbyFontFiles = getNearbyFontFiles(factory5.get()); | ||
if (nearbyFontFiles.empty()) | ||
{ | ||
return systemFontCollection; | ||
} | ||
|
||
wil::com_ptr<IDWriteFontSet> systemFontSet; | ||
// IDWriteFontCollection1 is supported since Windows 7. | ||
THROW_IF_FAILED(systemFontCollection.query<IDWriteFontCollection1>()->GetFontSet(systemFontSet.addressof())); | ||
|
||
wil::com_ptr<IDWriteFontSetBuilder1> fontSetBuilder; | ||
THROW_IF_FAILED(factory5->CreateFontSetBuilder(fontSetBuilder.addressof())); | ||
THROW_IF_FAILED(fontSetBuilder->AddFontSet(systemFontSet.get())); | ||
|
||
for (const auto& file : nearbyFontFiles) | ||
{ | ||
LOG_IF_FAILED(fontSetBuilder->AddFontFile(file.get())); | ||
} | ||
|
||
wil::com_ptr<IDWriteFontSet> fontSet; | ||
THROW_IF_FAILED(fontSetBuilder->CreateFontSet(fontSet.addressof())); | ||
|
||
wil::com_ptr<IDWriteFontCollection1> fontCollection; | ||
THROW_IF_FAILED(factory5->CreateFontCollectionFromFontSet(fontSet.get(), fontCollection.addressof())); | ||
|
||
return std::move(fontCollection); | ||
} | ||
else | ||
{ | ||
return systemFontCollection; | ||
} | ||
} | ||
} | ||
|
||
inline wil::com_ptr<IDWriteFontCollection> GetCached() | ||
{ | ||
return details::getFontCollection(false); | ||
} | ||
|
||
inline wil::com_ptr<IDWriteFontCollection> GetFresh() | ||
{ | ||
return details::getFontCollection(true); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Windows Terminal ignores the HRESULT this function returns and will simply crash with a division by 0 error if it wasn't for this new code. This implements font fallback in a similar way as DxEngine does.
In the future Terminal shouldn't ignore the HRESULT and call UpdateFont() with fallback fonts itself until this method succeeds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we track this work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a short note in #9999.