AtlasEngine: Harden against invalid soft fonts (#15889)

This is a theoretical improvement for #15553 where Windows Terminal
crashed due to AtlasEngine accessing the soft font bitmap outside of
bounds. The problem is that the soft font cell size was non-zero.
This PR hardens against such situations by checking whether the
requested soft font index is inside the bounds of the bitmaps.
The improvement couldn't be tested as it couldn't be reproduced.

src/renderer/atlas/AtlasEngine.api.cpp

@@ -163,8 +163,9 @@ constexpr HRESULT vec2_narrow(U x, U y, vec2<T>& out) noexcept
 [[nodiscard]] HRESULT AtlasEngine::UpdateSoftFont(const std::span<const uint16_t> bitPattern, const til::size cellSize, const size_t centeringHint) noexcept
 {
     const auto softFont = _api.s.write()->font.write();
-    softFont->softFontPattern = std::vector(bitPattern.begin(), bitPattern.end());
-    softFont->softFontCellSize = cellSize;
+    softFont->softFontPattern.assign(bitPattern.begin(), bitPattern.end());
+    softFont->softFontCellSize.width = std::max(0, cellSize.width);
+    softFont->softFontCellSize.height = std::max(0, cellSize.height);
     return S_OK;
 }
 

src/renderer/atlas/AtlasEngine.cpp

@@ -633,13 +633,12 @@ void AtlasEngine::_flushBufferLine()
 
     auto& row = *_p.rows[_api.lastPaintBufferLineCoord.y];
 
-    wil::com_ptr<IDWriteFontFace2> mappedFontFace;
-
 #pragma warning(suppress : 26494) // Variable 'mappedEnd' is uninitialized. Always initialize an object (type.5).
     for (u32 idx = 0, mappedEnd; idx < _api.bufferLine.size(); idx = mappedEnd)
     {
         u32 mappedLength = 0;
-        _mapCharacters(_api.bufferLine.data() + idx, gsl::narrow_cast<u32>(_api.bufferLine.size()) - idx, &mappedLength, mappedFontFace.put());
+        wil::com_ptr<IDWriteFontFace2> mappedFontFace;
+        _mapCharacters(_api.bufferLine.data() + idx, gsl::narrow_cast<u32>(_api.bufferLine.size()) - idx, &mappedLength, mappedFontFace.addressof());
         mappedEnd = idx + mappedLength;
 
         if (!mappedFontFace)
@@ -945,10 +944,6 @@ void AtlasEngine::_mapReplacementCharacter(u32 from, u32 to, ShapedRow& row)
         return;
     }
 
-    static constexpr auto isSoftFontChar = [](wchar_t ch) noexcept {
-        return ch >= 0xEF20 && ch < 0xEF80;
-    };
-
     auto pos1 = from;
     auto pos2 = pos1;
     size_t col1 = _api.bufferLineColumn[from];

src/renderer/atlas/Backend.h

@@ -69,11 +69,16 @@ namespace Microsoft::Console::Render::Atlas
     // std::clamp<T, Predicate>(T, T, T, Predicate) with std::less{} as the argument,
     // which introduces branching. While not perfect, this is still better than std::clamp.
     template<typename T>
-    constexpr T clamp(T val, T min, T max)
+    constexpr T clamp(T val, T min, T max) noexcept
     {
         return val < min ? min : (max < val ? max : val);
     }
 
+    constexpr bool isSoftFontChar(wchar_t ch) noexcept
+    {
+        return ch >= 0xEF20 && ch < 0xEF80;
+    }
+
     inline constexpr D2D1_RECT_F GlyphRunEmptyBounds{ 1e38f, 1e38f, -1e38f, -1e38f };
     void GlyphRunAccumulateBounds(const ID2D1DeviceContext* d2dRenderTarget, D2D1_POINT_2F baselineOrigin, const DWRITE_GLYPH_RUN* glyphRun, D2D1_RECT_F& bounds);
 

src/renderer/atlas/BackendD3D.cpp

@@ -1498,7 +1498,7 @@ bool BackendD3D::_drawSoftFontGlyph(const RenderingPayload& p, const AtlasFontFa
     if (!_softFontBitmap)
     {
         // Allocating such a tiny texture is very wasteful (min. texture size on GPUs
-        // right now is 64kB), but this is a seldomly used feature so it's fine...
+        // right now is 64kB), but this is a seldom used feature so it's fine...
         const D2D1_SIZE_U size{
             static_cast<UINT32>(p.s->font->softFontCellSize.width),
             static_cast<UINT32>(p.s->font->softFontCellSize.height),
@@ -1511,30 +1511,6 @@ bool BackendD3D::_drawSoftFontGlyph(const RenderingPayload& p, const AtlasFontFa
         THROW_IF_FAILED(_d2dRenderTarget->CreateBitmap(size, nullptr, 0, &bitmapProperties, _softFontBitmap.addressof()));
     }
 
-    {
-        const auto width = static_cast<size_t>(p.s->font->softFontCellSize.width);
-        const auto height = static_cast<size_t>(p.s->font->softFontCellSize.height);
-
-        auto bitmapData = Buffer<u32>{ width * height };
-        const auto glyphIndex = glyphEntry.glyphIndex - 0xEF20u;
-        auto src = p.s->font->softFontPattern.begin() + height * glyphIndex;
-        auto dst = bitmapData.begin();
-
-        for (size_t y = 0; y < height; y++)
-        {
-            auto srcBits = *src++;
-            for (size_t x = 0; x < width; x++)
-            {
-                const auto srcBitIsSet = (srcBits & 0x8000) != 0;
-                *dst++ = srcBitIsSet ? 0xffffffff : 0x00000000;
-                srcBits <<= 1;
-            }
-        }
-
-        const auto pitch = static_cast<UINT32>(width * sizeof(u32));
-        THROW_IF_FAILED(_softFontBitmap->CopyFromMemory(nullptr, bitmapData.data(), pitch));
-    }
-
     const auto interpolation = p.s->font->antialiasingMode == AntialiasingMode::Aliased ? D2D1_INTERPOLATION_MODE_NEAREST_NEIGHBOR : D2D1_INTERPOLATION_MODE_HIGH_QUALITY_CUBIC;
     const D2D1_RECT_F dest{
         static_cast<f32>(rect.x),
@@ -1544,6 +1520,7 @@ bool BackendD3D::_drawSoftFontGlyph(const RenderingPayload& p, const AtlasFontFa
     };
 
     _d2dBeginDrawing();
+    _drawSoftFontGlyphInBitmap(p, glyphEntry);
     _d2dRenderTarget->DrawBitmap(_softFontBitmap.get(), &dest, 1, interpolation, nullptr, nullptr);
 
     glyphEntry.data.shadingType = static_cast<u16>(ShadingType::TextGrayscale);
@@ -1563,6 +1540,51 @@ bool BackendD3D::_drawSoftFontGlyph(const RenderingPayload& p, const AtlasFontFa
     return true;
 }
 
+void BackendD3D::_drawSoftFontGlyphInBitmap(const RenderingPayload& p, const AtlasGlyphEntry& glyphEntry) const
+{
+    if (!isSoftFontChar(glyphEntry.glyphIndex))
+    {
+        // AtlasEngine::_mapReplacementCharacter should have filtered inputs with isSoftFontChar already.
+        assert(false);
+        return;
+    }
+
+    const auto width = static_cast<size_t>(p.s->font->softFontCellSize.width);
+    const auto height = static_cast<size_t>(p.s->font->softFontCellSize.height);
+    const auto& softFontPattern = p.s->font->softFontPattern;
+
+    // The isSoftFontChar() range is [0xEF20,0xEF80).
+    const auto offset = glyphEntry.glyphIndex - 0xEF20u;
+    const auto offsetBeg = offset * height;
+    const auto offsetEnd = offsetBeg + height;
+
+    if (offsetEnd > softFontPattern.size())
+    {
+        // Out of range values should not occur, but they do and it's unknown why: GH#15553
+        assert(false);
+        return;
+    }
+
+    Buffer<u32> bitmapData{ width * height };
+    auto dst = bitmapData.begin();
+    auto it = softFontPattern.begin() + offsetBeg;
+    const auto end = softFontPattern.begin() + offsetEnd;
+
+    while (it != end)
+    {
+        auto srcBits = *it++;
+        for (size_t x = 0; x < width; x++)
+        {
+            const auto srcBitIsSet = (srcBits & 0x8000) != 0;
+            *dst++ = srcBitIsSet ? 0xffffffff : 0x00000000;
+            srcBits <<= 1;
+        }
+    }
+
+    const auto pitch = static_cast<UINT32>(width * sizeof(u32));
+    THROW_IF_FAILED(_softFontBitmap->CopyFromMemory(nullptr, bitmapData.data(), pitch));
+}
+
 void BackendD3D::_drawGlyphPrepareRetry(const RenderingPayload& p)
 {
     THROW_HR_IF_MSG(E_UNEXPECTED, _glyphAtlasMap.empty(), "BackendD3D::_drawGlyph deadlock");

src/renderer/atlas/BackendD3D.h

@@ -220,6 +220,7 @@ namespace Microsoft::Console::Render::Atlas
         ATLAS_ATTR_COLD static void _initializeFontFaceEntry(AtlasFontFaceEntryInner& fontFaceEntry);
         [[nodiscard]] ATLAS_ATTR_COLD bool _drawGlyph(const RenderingPayload& p, const AtlasFontFaceEntryInner& fontFaceEntry, AtlasGlyphEntry& glyphEntry);
         bool _drawSoftFontGlyph(const RenderingPayload& p, const AtlasFontFaceEntryInner& fontFaceEntry, AtlasGlyphEntry& glyphEntry);
+        void _drawSoftFontGlyphInBitmap(const RenderingPayload& p, const AtlasGlyphEntry& glyphEntry) const;
         void _drawGlyphPrepareRetry(const RenderingPayload& p);
         void _splitDoubleHeightGlyph(const RenderingPayload& p, const AtlasFontFaceEntryInner& fontFaceEntry, AtlasGlyphEntry& glyphEntry);
         void _drawGridlines(const RenderingPayload& p, u16 y);