Skip to content
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 the fix for the fix of nearby font loading #16196

Merged
merged 2 commits into from
Nov 6, 2023

Conversation

lhecker
Copy link
Member

@lhecker lhecker commented Oct 19, 2023

I still don't know how to reproduce it properly, but I'm slowly
wrapping my head around how and why it happens. The issue isn't that
FindFamilyName fails with exists=FALSE, but rather that any of the
followup calls like GetDesignGlyphMetrics fails, which results in an
exception and subsequently in an orderly fallback to Consolas.
I've always thought that the issue is that even with the nearby font
collection we get an exists=FALSE... I'm not sure why I thought that.

This changeset also drops the fallback iteration for Lucida Console and
Courier New, because I felt like the code looks neater that way and I
think it's a reasonable expectation that Consolas is always installed.

Closes #16058

@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-Fonts Related to the font Product-Terminal The new Windows Terminal. labels Oct 19, 2023
@DHowett
Copy link
Member

DHowett commented Oct 19, 2023

reasonable expectation that Consolas is always installed.

On Desktop, yes. OneCoreUAP is different, but... we don't talk about that.

@@ -455,30 +455,34 @@ void AtlasEngine::SetWarningCallback(std::function<void(HRESULT)> pfn) 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
{
static constexpr std::array fallbackFaceNames{ static_cast<const wchar_t*>(nullptr), L"Consolas", L"Lucida Console", L"Courier New" };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't see where we handle fallback at all now (?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh duh it's down where we don't pass in a font name

@zadjii-msft zadjii-msft merged commit 9e86c98 into main Nov 6, 2023
17 checks passed
@zadjii-msft zadjii-msft deleted the dev/lhecker/16058-cascadia-mono-404 branch November 6, 2023 21:30
DHowett pushed a commit that referenced this pull request Nov 7, 2023
I still don't know how to reproduce it properly, but I'm slowly
wrapping my head around how and why it happens. The issue isn't that
`FindFamilyName` fails with `exists=FALSE`, but rather that any of the
followup calls like `GetDesignGlyphMetrics` fails, which results in an
exception and subsequently in an orderly fallback to Consolas.
I've always thought that the issue is that even with the nearby font
collection we get an `exists=FALSE`... I'm not sure why I thought that.

This changeset also drops the fallback iteration for Lucida Console and
Courier New, because I felt like the code looks neater that way and I
think it's a reasonable expectation that Consolas is always installed.

Closes #16058

(cherry picked from commit 9e86c98)
Service-Card-Id: 90885610
Service-Version: 1.18
DHowett pushed a commit that referenced this pull request Nov 7, 2023
I still don't know how to reproduce it properly, but I'm slowly
wrapping my head around how and why it happens. The issue isn't that
`FindFamilyName` fails with `exists=FALSE`, but rather that any of the
followup calls like `GetDesignGlyphMetrics` fails, which results in an
exception and subsequently in an orderly fallback to Consolas.
I've always thought that the issue is that even with the nearby font
collection we get an `exists=FALSE`... I'm not sure why I thought that.

This changeset also drops the fallback iteration for Lucida Console and
Courier New, because I felt like the code looks neater that way and I
think it's a reasonable expectation that Consolas is always installed.

Closes #16058

(cherry picked from commit 9e86c98)
Service-Card-Id: 90885607
Service-Version: 1.19
DHowett pushed a commit that referenced this pull request Nov 7, 2023
I still don't know how to reproduce it properly, but I'm slowly
wrapping my head around how and why it happens. The issue isn't that
`FindFamilyName` fails with `exists=FALSE`, but rather that any of the
followup calls like `GetDesignGlyphMetrics` fails, which results in an
exception and subsequently in an orderly fallback to Consolas.
I've always thought that the issue is that even with the nearby font
collection we get an `exists=FALSE`... I'm not sure why I thought that.

This changeset also drops the fallback iteration for Lucida Console and
Courier New, because I felt like the code looks neater that way and I
think it's a reasonable expectation that Consolas is always installed.

Closes #16058

(cherry picked from commit 9e86c98)
Service-Card-Id: 90885610
Service-Version: 1.18
radu-cernatescu pushed a commit to radu-cernatescu/terminal that referenced this pull request Nov 8, 2023
I still don't know how to reproduce it properly, but I'm slowly
wrapping my head around how and why it happens. The issue isn't that
`FindFamilyName` fails with `exists=FALSE`, but rather that any of the
followup calls like `GetDesignGlyphMetrics` fails, which results in an
exception and subsequently in an orderly fallback to Consolas.
I've always thought that the issue is that even with the nearby font
collection we get an `exists=FALSE`... I'm not sure why I thought that.

This changeset also drops the fallback iteration for Lucida Console and
Courier New, because I felt like the code looks neater that way and I
think it's a reasonable expectation that Consolas is always installed.

Closes microsoft#16058
DHowett added a commit that referenced this pull request Jan 12, 2024
- AtlasEngine: Minor bug fixes (GH-16219)
- Fix the fix for the fix of nearby font loading (GH-16196)
- Added selectionBackground to light color schemes (GH-16243)
- Another theoretical fix for a crash (GH-16267)
- Fix tabs being printed in cmd.exe prompts (GH-16273)

Related work items: MSFT-47266988
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Fonts Related to the font Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal.
Projects
Development

Successfully merging this pull request may close these issues.

Cascadia Mono font not found
3 participants