AtlasEngine: Clip box glyphs to their cells (#15343)

Overhangs for box glyphs can produce unsightly effects, where the
antialiased edges of horizontal and vertical lines overlap between
neighboring glyphs and produce "boldened" intersections.
This avoids the issue in most cases by simply clipping the glyph to the
size of a single cell. The downside is that it fails to work well for
custom line heights, etc.

## Validation Steps Performed

* With Cascadia Code, printing ``"`u{2593}`n`u{2593}"`` in pwsh
  doesn't produce a brightened overlap anymore ✅
* ``"`e#3`u{2502}`n`e#4`u{2502}"`` produces a fat vertical line ✅

Author: lhecker

src/inc/til/flat_set.h

@@ -28,14 +28,19 @@ namespace til
     // A basic, hashmap with linear probing. A `LoadFactor` of 2 equals
     // a max. load of roughly 50% and a `LoadFactor` of 4 roughly 25%.
     //
+    // `GrowthExponent` controls how fast the set grows and corresponds to
+    // a rate of 2^GrowthExponent. In other words, a `GrowthExponent` of 3
+    // equals a growth rate of 8x every time the capacity has been reached.
+    //
     // It performs best with:
     // * small and cheap T
     // * >= 50% successful lookups
     // * <= 50% load factor (LoadFactor >= 2, which is the minimum anyways)
-    template<typename T, size_t LoadFactor = 2>
+    template<typename T, size_t LoadFactor = 2, size_t GrowthExponent = 1>
     struct linear_flat_set
     {
         static_assert(LoadFactor >= 2);
+        static_assert(GrowthExponent >= 1);
 
         linear_flat_set() = default;
 
@@ -85,6 +90,30 @@ namespace til
             }
         }
 
+        template<typename U>
+        T* lookup(U&& key) const noexcept
+        {
+            if (!_map)
+            {
+                return nullptr;
+            }
+
+            const auto hash = ::std::hash<T>{}(key) >> _shift;
+
+            for (auto i = hash;; ++i)
+            {
+                auto& slot = _map[i & _mask];
+                if (!slot)
+                {
+                    return nullptr;
+                }
+                if (slot == key) [[likely]]
+                {
+                    return &slot;
+                }
+            }
+        }
+
         template<typename U>
         std::pair<T&, bool> insert(U&& key)
         {
@@ -121,14 +150,15 @@ namespace til
     private:
         __declspec(noinline) void _bumpSize()
         {
+            // For instance at a GrowthExponent of 1:
             // A _shift of 0 would result in a newShift of 0xfffff...
             // A _shift of 1 would result in a newCapacity of 0
-            if (_shift < 2)
+            if (_shift <= GrowthExponent)
             {
                 throw std::bad_array_new_length{};
             }
 
-            const auto newShift = _shift - 1;
+            const auto newShift = _shift - GrowthExponent;
             const auto newCapacity = size_t{ 1 } << (digits - newShift);
             const auto newMask = newCapacity - 1;
             auto newMap = std::make_unique<T[]>(newCapacity);

src/renderer/atlas/BackendD3D.cpp

@@ -36,10 +36,23 @@ TIL_FAST_MATH_BEGIN
 #pragma warning(disable : 26481) // Don't use pointer arithmetic. Use span instead (bounds.1).
 #pragma warning(disable : 26482) // Only index into arrays using constant expressions (bounds.2).
 
+// Initializing large arrays can be very costly compared to how cheap some of these functions are.
+#define ALLOW_UNINITIALIZED_BEGIN _Pragma("warning(push)") _Pragma("warning(disable : 26494)")
+#define ALLOW_UNINITIALIZED_END _Pragma("warning(pop)")
+
 using namespace Microsoft::Console::Render::Atlas;
 
 template<>
-struct ::std::hash<BackendD3D::AtlasGlyphEntry>
+struct std::hash<u16>
+{
+    constexpr size_t operator()(u16 key) const noexcept
+    {
+        return til::flat_set_hash_integer(key);
+    }
+};
+
+template<>
+struct std::hash<BackendD3D::AtlasGlyphEntry>
 {
     constexpr size_t operator()(u16 key) const noexcept
     {
@@ -53,7 +66,7 @@ struct ::std::hash<BackendD3D::AtlasGlyphEntry>
 };
 
 template<>
-struct ::std::hash<BackendD3D::AtlasFontFaceEntry>
+struct std::hash<BackendD3D::AtlasFontFaceEntry>
 {
     using T = BackendD3D::AtlasFontFaceEntry;
 
@@ -982,7 +995,13 @@ void BackendD3D::_drawText(RenderingPayload& p)
         // We need to goto here, because a retry will cause the atlas texture as well as the
         // _glyphCache hashmap to be cleared, and so we'll have to call insert() again.
         drawGlyphRetry:
-            auto& fontFaceEntry = *_glyphAtlasMap.insert(fontFaceKey).first.inner;
+            const auto [fontFaceEntryOuter, fontFaceInserted] = _glyphAtlasMap.insert(fontFaceKey);
+            auto& fontFaceEntry = *fontFaceEntryOuter.inner;
+
+            if (fontFaceInserted)
+            {
+                _initializeFontFaceEntry(fontFaceEntry);
+            }
 
             while (x < m.glyphsTo)
             {
@@ -1124,7 +1143,30 @@ void BackendD3D::_drawTextOverlapSplit(const RenderingPayload& p, u16 y)
     }
 }
 
-bool BackendD3D::_drawGlyph(const RenderingPayload& p, const BackendD3D::AtlasFontFaceEntryInner& fontFaceEntry, BackendD3D::AtlasGlyphEntry& glyphEntry)
+void BackendD3D::_initializeFontFaceEntry(AtlasFontFaceEntryInner& fontFaceEntry)
+{
+    ALLOW_UNINITIALIZED_BEGIN
+    std::array<u32, 0x100> codepoints;
+    std::array<u16, 0x100> indices;
+    ALLOW_UNINITIALIZED_END
+
+    for (u32 i = 0; i < codepoints.size(); ++i)
+    {
+        codepoints[i] = 0x2500 + i;
+    }
+
+    THROW_IF_FAILED(fontFaceEntry.fontFace->GetGlyphIndicesW(codepoints.data(), codepoints.size(), indices.data()));
+
+    for (u32 i = 0; i < indices.size(); ++i)
+    {
+        if (const auto idx = indices[i])
+        {
+            fontFaceEntry.boxGlyphs.insert(idx);
+        }
+    }
+}
+
+bool BackendD3D::_drawGlyph(const RenderingPayload& p, const AtlasFontFaceEntryInner& fontFaceEntry, AtlasGlyphEntry& glyphEntry)
 {
     if (!fontFaceEntry.fontFace)
     {
@@ -1207,22 +1249,20 @@ bool BackendD3D::_drawGlyph(const RenderingPayload& p, const BackendD3D::AtlasFo
 #endif
 
     const auto lineRendition = static_cast<LineRendition>(fontFaceEntry.lineRendition);
-    std::optional<D2D1_MATRIX_3X2_F> transform;
+    const auto needsTransform = lineRendition != LineRendition::SingleWidth;
 
-    if (lineRendition != LineRendition::SingleWidth)
+    static constexpr D2D1_MATRIX_3X2_F identityTransform{ .m11 = 1, .m22 = 1 };
+    D2D1_MATRIX_3X2_F transform = identityTransform;
+
+    if (needsTransform)
     {
-        auto& t = transform.emplace();
-        t.m11 = 2.0f;
-        t.m22 = lineRendition >= LineRendition::DoubleHeightTop ? 2.0f : 1.0f;
-        _d2dRenderTarget->SetTransform(&t);
+        transform.m11 = 2.0f;
+        transform.m22 = lineRendition >= LineRendition::DoubleHeightTop ? 2.0f : 1.0f;
+        _d2dRenderTarget->SetTransform(&transform);
     }
 
     const auto restoreTransform = wil::scope_exit([&]() noexcept {
-        if (transform)
-        {
-            static constexpr D2D1_MATRIX_3X2_F identity{ .m11 = 1, .m22 = 1 };
-            _d2dRenderTarget->SetTransform(&identity);
-        }
+        _d2dRenderTarget->SetTransform(&identityTransform);
     });
 
     // This calculates the black box of the glyph, or in other words,
@@ -1246,7 +1286,7 @@ bool BackendD3D::_drawGlyph(const RenderingPayload& p, const BackendD3D::AtlasFo
     bool isColorGlyph = false;
     D2D1_RECT_F bounds = GlyphRunEmptyBounds;
 
-    const auto cleanup = wil::scope_exit([&]() {
+    const auto antialiasingCleanup = wil::scope_exit([&]() {
         if (isColorGlyph)
         {
             _d2dRenderTarget4->SetTextAntialiasMode(static_cast<D2D1_TEXT_ANTIALIAS_MODE>(p.s->font->antialiasingMode));
@@ -1273,7 +1313,23 @@ bool BackendD3D::_drawGlyph(const RenderingPayload& p, const BackendD3D::AtlasFo
         }
     }
 
-    // box may be empty if the glyph is whitespace.
+    // Overhangs for box glyphs can produce unsightly effects, where the antialiased edges of horizontal
+    // and vertical lines overlap between neighboring glyphs and produce "boldened" intersections.
+    // It looks a little something like this:
+    //   ---+---+---
+    // This avoids the issue in most cases by simply clipping the glyph to the size of a single cell.
+    // The downside is that it fails to work well for custom line heights, etc.
+    const auto isBoxGlyph = fontFaceEntry.boxGlyphs.lookup(glyphEntry.glyphIndex) != nullptr;
+    if (isBoxGlyph)
+    {
+        // NOTE: As mentioned above, the "origin" of a glyph's coordinate system is its baseline.
+        bounds.left = std::max(bounds.left, 0.0f);
+        bounds.top = std::max(bounds.top, static_cast<f32>(-p.s->font->baseline) * transform.m22);
+        bounds.right = std::min(bounds.right, static_cast<f32>(p.s->font->cellSize.x) * transform.m11);
+        bounds.bottom = std::min(bounds.bottom, static_cast<f32>(p.s->font->descender) * transform.m22);
+    }
+
+    // The bounds may be empty if the glyph is whitespace.
     if (bounds.left >= bounds.right || bounds.top >= bounds.bottom)
     {
         return true;
@@ -1299,15 +1355,31 @@ bool BackendD3D::_drawGlyph(const RenderingPayload& p, const BackendD3D::AtlasFo
         static_cast<f32>(rect.y - bt),
     };
 
-    if (transform)
+    _d2dBeginDrawing();
+
+    if (isBoxGlyph)
     {
-        auto& t = *transform;
-        t.dx = (1.0f - t.m11) * baselineOrigin.x;
-        t.dy = (1.0f - t.m22) * baselineOrigin.y;
-        _d2dRenderTarget->SetTransform(&t);
+        const D2D1_RECT_F clipRect{
+            static_cast<f32>(rect.x) / transform.m11,
+            static_cast<f32>(rect.y) / transform.m22,
+            static_cast<f32>(rect.x + rect.w) / transform.m11,
+            static_cast<f32>(rect.y + rect.h) / transform.m22,
+        };
+        _d2dRenderTarget4->PushAxisAlignedClip(&clipRect, D2D1_ANTIALIAS_MODE_ALIASED);
     }
+    const auto boxGlyphCleanup = wil::scope_exit([&]() {
+        if (isBoxGlyph)
+        {
+            _d2dRenderTarget4->PopAxisAlignedClip();
+        }
+    });
 
-    _d2dBeginDrawing();
+    if (needsTransform)
+    {
+        transform.dx = (1.0f - transform.m11) * baselineOrigin.x;
+        transform.dy = (1.0f - transform.m22) * baselineOrigin.y;
+        _d2dRenderTarget->SetTransform(&transform);
+    }
 
     if (!isColorGlyph)
     {

src/renderer/atlas/BackendD3D.h

@@ -151,6 +151,9 @@ namespace Microsoft::Console::Render::Atlas
             LineRendition lineRendition = LineRendition::SingleWidth;
 
             til::linear_flat_set<AtlasGlyphEntry> glyphs;
+            // boxGlyphs gets an increased growth rate of 2^2 = 4x, because presumably fonts either contain very
+            // few or almost all of the box glyphs. This reduces the cost of _initializeFontFaceEntry quite a bit.
+            til::linear_flat_set<u16, 2, 2> boxGlyphs;
         };
 
         struct AtlasFontFaceEntry
@@ -214,6 +217,7 @@ namespace Microsoft::Console::Render::Atlas
         void _uploadBackgroundBitmap(const RenderingPayload& p);
         void _drawText(RenderingPayload& p);
         ATLAS_ATTR_COLD void _drawTextOverlapSplit(const RenderingPayload& p, u16 y);
+        ATLAS_ATTR_COLD static void _initializeFontFaceEntry(AtlasFontFaceEntryInner& fontFaceEntry);
         ATLAS_ATTR_COLD [[nodiscard]] bool _drawGlyph(const RenderingPayload& p, const AtlasFontFaceEntryInner& fontFaceEntry, AtlasGlyphEntry& glyphEntry);
         bool _drawSoftFontGlyph(const RenderingPayload& p, const AtlasFontFaceEntryInner& fontFaceEntry, AtlasGlyphEntry& glyphEntry);
         void _drawGlyphPrepareRetry(const RenderingPayload& p);