maxmustermann/terminal
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Don't throw in GetProposedDimensions (#10260)

Browse source 
I cannot for the life of me repro the original bug. I've got fonts with bad permissions SxS, I've tried installing a font twice, I've tried stopping the font cache service. No idea how to manually repro the original bug.

BUT theoretically, this function should never throw. So lets just switch this to a `LOG_IF_FAILED`, and hope that this goes away?

* [x] Fixes #10211?
* [x] built & ran manually.

Unclear if this can get cherry-picked trivially to 1.8. Code's pretty trivial though so if we need another PR for that, it can be arranged.

(cherry picked from commit 89ca2ae05f)
This commit is contained in:
Mike Griese 2021-05-28 16:57:34 -05:00 committed by Dustin Howett
parent 414adf16a9
commit c8ac63e78e
No known key found for this signature in database
GPG key ID: 0719BB71B334EE77
3 changed files with 69 additions and 51 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
src/cascadia/TerminalControl/TermControl.cpp
View file

@ -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();

111
src/renderer/dx/DxFontRenderData.cpp
View file

@ -644,63 +644,77 @@ 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 };
// 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
{
face = _FindFontFace(familyName, weight, stretch, style, localeName, true);
if (!face)
{
// 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;
}
// 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, 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)
{
// 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())
for (const auto fallbackFace : FALLBACK_FONT_FACES)
{
const auto lastSpace = familyName.find_last_of(UNICODE_SPACE);
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
// value is unsigned and npos will be greater than size.
// if we didn't find anything to trim, leave.
if (lastSpace >= familyName.size())
// near it in this part.
face = _FindFontFace(familyName, weight, stretch, style, localeName, false);
if (face)
{
didFallback = true;
break;
}
// 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);
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);
// 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 (face)
{
familyName = fallbackFace;
face = _FindFontFace(familyName, weight, stretch, style, localeName);
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);
if (face)
{
didFallback = true;
break;
}
didFallback = true;
break;
}
}
}
@ -723,7 +737,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;
@ -735,7 +750,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();

3
src/renderer/dx/DxFontRenderData.h
View file

@ -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;