Repository: microsoft/terminal (language: cpp)

Issue:
Block Elements not rendering correctly with AtlasEngine enabled
### Windows Terminal version

1.16.2642.0

### Windows build number

10.0.22000.0

### Other Software

_No response_

### Steps to reproduce

In PowerShell with the default Cascadia Mono font, run: ``Write-Host "`u{2595}`u{2594}`n`u{2595}`u{2581}"``

### Expected Behavior

In version 1.15.2525.0 or 1.16.2642.0 with AtlasEngine disabled:
<img width="401" alt="image" src="https://user-images.githubusercontent.com/7351388/192859338-6757f670-6bb3-4993-a349-e93f466929bd.png">


### Actual Behavior

The top is cut off:
<img width="404" alt="image" src="https://user-images.githubusercontent.com/7351388/192859478-619f29e5-f8da-4633-a567-70b9c12ce3a1.png">



Discussion / hints:
Might be same underlying issue as #14022 
Note to self/team: The block character scaling technique in `_getCachedGlyphLayout` is inherently flawed as I realized yesterday. What it does is:
* Get the glyph's advance width/height
* Resize to match the cell size

This doesn't work with fonts that use side bearings for block characters. A better way would be to assume that all fonts that define any block character also probably define the U+2588 Full Block character. That way we can do:
* Get the font's full block glyph
* Get the actual size including side bearings of the full block
* Resize to match the cell size but apply the scale to the given glyph instead

Fix patch:
diff --git a/src/renderer/atlas/AtlasEngine.api.cpp b/src/renderer/atlas/AtlasEngine.api.cpp
index aca166f8524..1e1776559bc 100644
--- a/src/renderer/atlas/AtlasEngine.api.cpp
+++ b/src/renderer/atlas/AtlasEngine.api.cpp
@@ -699,6 +699,7 @@ void AtlasEngine::_resolveFontMetrics(const wchar_t* requestedFaceName, const Fo
         // as we might cause _api to be in an inconsistent state otherwise.
 
         fontMetrics->fontCollection = std::move(fontCollection);
+        fontMetrics->fontFamily = std::move(fontFamily);
         fontMetrics->fontName = std::move(fontName);
         fontMetrics->fontSizeInDIP = fontSizeInDIP;
         fontMetrics->baselineInDIP = baseline / static_cast<float>(_api.dpi) * 96.0f;
diff --git a/src/renderer/atlas/AtlasEngine.cpp b/src/renderer/atlas/AtlasEngine.cpp
index d6d3dd45645..c99c9303b21 100644
--- a/src/renderer/atlas/AtlasEngine.cpp
+++ b/src/renderer/atlas/AtlasEngine.cpp
@@ -1101,6 +1101,10 @@ void AtlasEngine::_recreateFontDependentResources()
                 const auto fontStyle = italic ? DWRITE_FONT_STYLE_ITALIC : DWRITE_FONT_STYLE_NORMAL;
                 auto& textFormat = _r.textFormats[italic][bold];
 
+                wil::com_ptr<IDWriteFont> font;
+                THROW_IF_FAILED(_r.fontMetrics.fontFamily->GetFirstMatchingFont(fontWeight, DWRITE_FONT_STRETCH_NORMAL, fontStyle, font.addressof()));
+                THROW_IF_FAILED(font->CreateFontFace(_r.fontFaces[italic << 1 | bold].put()));
+
                 THROW_IF_FAILED(_sr.dwriteFactory->CreateTextFormat(_api.fontMetrics.fontName.c_str(), _api.fontMetrics.fontCollection.get(), fontWeight, fontStyle, DWRITE_FONT_STRETCH_NORMAL, _api.fontMetrics.fontSizeInDIP, L"", textFormat.put()));
                 THROW_IF_FAILED(textFormat->SetWordWrapping(DWRITE_WORD_WRAPPING_NO_WRAP));
 
diff --git a/src/renderer/atlas/AtlasEngine.h b/src/renderer/atlas/AtlasEngine.h
index fe3b9712bb0..6a1533b4f6e 100644
--- a/src/renderer/atlas/AtlasEngine.h
+++ b/src/renderer/atlas/AtlasEngine.h
@@ -410,6 +410,7 @@ namespace Microsoft::Console::Render
         struct FontMetrics
         {
             wil::com_ptr<IDWriteFontCollection> fontCollection;
+            wil::com_ptr<IDWriteFontFamily> fontFamily;
             std::wstring fontName;
             float baselineInDIP = 0.0f;
             float fontSizeInDIP = 0.0f;
@@ -1010,6 +1011,7 @@ namespace Microsoft::Console::Render
             wil::com_ptr<ID3D11ShaderResourceView> atlasView;
             wil::com_ptr<ID2D1DeviceContext> d2dRenderTarget;
             wil::com_ptr<ID2D1SolidColorBrush> brush;
+            wil::com_ptr<IDWriteFontFace> fontFaces[4];
             wil::com_ptr<IDWriteTextFormat> textFormats[2][2];
             Buffer<DWRITE_FONT_AXIS_VALUE> textFormatAxes[2][2];
             wil::com_ptr<IDWriteTypography> typography;
diff --git a/src/renderer/atlas/AtlasEngine.r.cpp b/src/renderer/atlas/AtlasEngine.r.cpp
index 73d6a709296..a8bc092c0b8 100644
--- a/src/renderer/atlas/AtlasEngine.r.cpp
+++ b/src/renderer/atlas/AtlasEngine.r.cpp
@@ -535,35 +535,44 @@ AtlasEngine::CachedGlyphLayout AtlasEngine::_getCachedGlyphLayout(const wchar_t*
             wil::com_ptr<IDWriteFontFace> fontFace;
             THROW_IF_FAILED(mappedFont->CreateFontFace(fontFace.addressof()));
 
-            DWRITE_FONT_METRICS metrics;
-            fontFace->GetMetrics(&metrics);
-
-            const u32 codePoint = chars[0];
-            u16 glyphIndex;
-            THROW_IF_FAILED(fontFace->GetGlyphIndicesW(&codePoint, 1, &glyphIndex));
-
-            DWRITE_GLYPH_METRICS glyphMetrics;
-            THROW_IF_FAILED(fontFace->GetDesignGlyphMetrics(&glyphIndex, 1, &glyphMetrics));
-
-            const f32x2 boxSize{
-                static_cast<f32>(glyphMetrics.advanceWidth) / static_cast<f32>(metrics.designUnitsPerEm) * _r.fontMetrics.fontSizeInDIP,
-                static_cast<f32>(glyphMetrics.advanceHeight) / static_cast<f32>(metrics.designUnitsPerEm) * _r.fontMetrics.fontSizeInDIP,
-            };
+            // Don't adjust the size of block glyphs that are part of the user's chosen font.
+            if (std::ranges::find(_r.fontFaces, fontFace) == std::end(_r.fontFaces))
+            {
+                DWRITE_FONT_METRICS metrics;
+                fontFace->GetMetrics(&metrics);
 
-            // NOTE: Don't adjust the offset.
-            // Despite these being block characters, some fonts position them really weird with glyphs hanging out
-            // on all sides by up to 0.2em. They still expect the glyphs to be drawn on to their regular baseline.
-            // At the time of writing this can be tested with MesloLGM Nerd Font and U+E0B0 for instance.
+                static constexpr u32 codePoint = L'\u2588'; // Full Block character
+                u16 glyphIndex;
+                THROW_IF_FAILED(fontFace->GetGlyphIndicesW(&codePoint, 1, &glyphIndex));
 
-            scalingRequired = true;
-            // We always want box drawing glyphs to exactly match the size of a terminal cell.
-            // But add 1px to the destination size, so that we don't end up with fractional pixels.
-            scale.x = (layoutBox.x + _r.dipPerPixel) / boxSize.x;
-            scale.y = (layoutBox.y + _r.dipPerPixel) / boxSize.y;
-            // Now that the glyph is in the center of the cell thanks
-            // to the offset, the scaleCenter is center of the cell.
-            scaleCenter.x = layoutBox.x * 0.5f;
-            scaleCenter.y = layoutBox.y * 0.5f;
+                if (glyphIndex)
+                {
+                    DWRITE_GLYPH_METRICS glyphMetrics;
+                    THROW_IF_FAILED(fontFace->GetDesignGlyphMetrics(&glyphIndex, 1, &glyphMetrics));
+
+                    const auto fontScale = _r.fontMetrics.fontSizeInDIP / metrics.designUnitsPerEm;
+
+                    // How-to-DWRITE_OVERHANG_METRICS given a single glyph:
+                    DWRITE_OVERHANG_METRICS overhang;
+                    overhang.left = static_cast<f32>(glyphMetrics.leftSideBearing) * fontScale;
+                    overhang.top = static_cast<f32>(glyphMetrics.verticalOriginY - glyphMetrics.topSideBearing) * fontScale - _r.fontMetrics.baselineInDIP;
+                    overhang.right = static_cast<f32>(gsl::narrow_cast<INT32>(glyphMetrics.advanceWidth) - glyphMetrics.rightSideBearing) * fontScale - layoutBox.x;
+                    overhang.bottom = static_cast<f32>(gsl::narrow_cast<INT32>(glyphMetrics.advanceHeight) - glyphMetrics.verticalOriginY - glyphMetrics.bottomSideBearing) * fontScale + _r.fontMetrics.baselineInDIP - layoutBox.y;
+
+                    scalingRequired = true;
+                    // Center glyphs.
+                    offset.x = (overhang.left - overhang.right) * 0.5f;
+                    offset.y = (overhang.top - overhang.bottom) * 0.5f;
+                    // We always want box drawing glyphs to exactly match the size of a terminal cell.
+                    // But add 1px to the destination size, so that we don't end up with fractional pixels.
+                    scale.x = (layoutBox.x + _r.pixelPerDIP) / (layoutBox.x + overhang.left + overhang.right);
+                    scale.y = (layoutBox.y + _r.pixelPerDIP) / (layoutBox.y + overhang.top + overhang.bottom);
+                    // Now that the glyph is in the center of the cell thanks
+                    // to the offset, the scaleCenter is center of the cell.
+                    scaleCenter.x = layoutBox.x * 0.5f;
+                    scaleCenter.y = layoutBox.y * 0.5f;
+                }
+            }
         }
     }
     else


Test patch: