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.

Sign up for GitHub

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

Don't throw in GetProposedDimensions #10260

Merged
3 commits merged into from
May 28, 2021
Merged

Don't throw in GetProposedDimensions #10260

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
74ce24a
No way to repro this, but hopefully this should fix #10211
zadjii-msft May 28, 2021
81a9f81
Verify that this actually does work, by throwing an exception in Near…
zadjii-msft May 28, 2021
8b4c7f8
Update src/renderer/dx/DxFontRenderData.cpp
miniksa May 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/cascadia/TerminalControl/TermControl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1845,9 +1845,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// will take up.
// TODO: MSFT:21254947 - use a static function to do this instead of
// instantiating a DxEngine
// GH#10211 - UNDER NO CIRCUMSTANCE should this fail. If it does, the
// whole app will crash instantaneously on launch, which is no good.
auto dxEngine = std::make_unique<::Microsoft::Console::Render::DxEngine>();
THROW_IF_FAILED(dxEngine->UpdateDpi(dpi));
THROW_IF_FAILED(dxEngine->UpdateFont(desiredFont, actualFont));
LOG_IF_FAILED(dxEngine->UpdateDpi(dpi));
LOG_IF_FAILED(dxEngine->UpdateFont(desiredFont, actualFont));

const auto scale = dxEngine->GetScaling();
const auto fontSize = actualFont.GetSize();
Expand Down
108 changes: 61 additions & 47 deletions src/renderer/dx/DxFontRenderData.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -644,63 +644,76 @@ CATCH_RETURN()
{
// First attempt to find exactly what the user asked for.
didFallback = false;
auto face = _FindFontFace(familyName, weight, stretch, style, localeName);
Microsoft::WRL::ComPtr<IDWriteFontFace1> face{ nullptr };

if (!face)
// GH#10211 - wrap this all up in a try/catch. If the nearby fonts are
// corrupted, then we don't want to throw out of this top half of this
// method. We still want to fall back to a font that's reasonable, below.
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())
{
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;
}
face = _FindFontFace(familyName, weight, stretch, style, localeName, true);

// 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
face = _FindFontFace(familyName, weight, stretch, style, localeName);
}

// Alright, if our quick shot at trimming didn't work either...
// move onto looking up a font from our hardcoded list of fonts
// that should really always be available.
if (!face)
{
for (const auto fallbackFace : FALLBACK_FONT_FACES)
// 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())
{
familyName = fallbackFace;
face = _FindFontFace(familyName, weight, stretch, style, localeName);
const auto lastSpace = familyName.find_last_of(UNICODE_SPACE);

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

familyName = fallbackFace;
weight = DWRITE_FONT_WEIGHT_NORMAL;
stretch = DWRITE_FONT_STRETCH_NORMAL;
style = DWRITE_FONT_STYLE_NORMAL;
face = _FindFontFace(familyName, weight, stretch, style, localeName);
// 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);

if (face)
{
didFallback = true;
break;
}
// Try to find it with the shortened family name
face = _FindFontFace(familyName, weight, stretch, style, localeName, true);
}
}
}
CATCH_LOG();

// Alright, if our quick shot at trimming didn't work either...
// move onto looking up a font from our hardcoded list of fonts
// that should really always be available.
if (!face)
{
for (const auto fallbackFace : FALLBACK_FONT_FACES)
{
familyName = fallbackFace;
// With these fonts, don't attempt the nearby lookup. We're looking
// for system fonts only. If one of the nearby fonts is causing us
// problems (like in GH##10211), then we don't want to go anywhere
// near it in this part.
face = _FindFontFace(familyName, weight, stretch, style, localeName, false);

if (face)
{
didFallback = true;
break;
}

familyName = fallbackFace;
weight = DWRITE_FONT_WEIGHT_NORMAL;
stretch = DWRITE_FONT_STRETCH_NORMAL;
style = DWRITE_FONT_STYLE_NORMAL;
face = _FindFontFace(familyName, weight, stretch, style, localeName, false);

if (face)
{
didFallback = true;
break;
}
}
}
Expand All @@ -723,7 +736,8 @@ CATCH_RETURN()
DWRITE_FONT_WEIGHT& weight,
DWRITE_FONT_STRETCH& stretch,
DWRITE_FONT_STYLE& style,
std::wstring& localeName) const
std::wstring& localeName,
const bool withNearbyLookup) const
{
Microsoft::WRL::ComPtr<IDWriteFontFace1> fontFace;

Expand All @@ -735,7 +749,7 @@ CATCH_RETURN()
THROW_IF_FAILED(fontCollection->FindFamilyName(familyName.data(), &familyIndex, &familyExists));

// If the system collection missed, try the files sitting next to our binary.
if (!familyExists)
if (withNearbyLookup && !familyExists)
{
auto&& nearbyCollection = NearbyCollection();

Expand Down
3 changes: 2 additions & 1 deletion src/renderer/dx/DxFontRenderData.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ namespace Microsoft::Console::Render
DWRITE_FONT_WEIGHT& weight,
DWRITE_FONT_STRETCH& stretch,
DWRITE_FONT_STYLE& style,
std::wstring& localeName) const;
std::wstring& localeName,
const bool withNearbyLookup) const;

[[nodiscard]] std::wstring _GetFontFamilyName(gsl::not_null<IDWriteFontFamily*> const fontFamily,
std::wstring& localeName) const;
Expand Down