Fix curlyline rendering in AtlasEngine and GDIRenderer (microsoft#16444)

Fixes Curlyline being drawn as single underline in some cases

**Detailed Description**

- Curlyline is drawn at all font sizes.
- We might render a curlyline that is clipped in cases where we don't
have enough space to draw a full curlyline. This is to give users a
consistent view of Curlylines. Previously in those cases, it was drawn
as a single underline.
- Removed minimum threshold `minCurlyLinePeakHeight` for Curlyline
drawing.
- GDIRender changes:
- Underline offset now points to the (vertical) mid position of the
underline. Removes redundant `underlineMidY` calculation inside the draw
call.

Closes microsoft#16288

Author: tusharsnx

src/renderer/atlas/BackendD3D.cpp

@@ -306,37 +306,35 @@ void BackendD3D::_updateFontDependents(const RenderingPayload& p)
 {
     const auto& font = *p.s->font;
 
-    // The max height of Curly line peak in `em` units.
-    const auto maxCurlyLinePeakHeightEm = 0.075f;
-    // We aim for atleast 1px height, but since we draw 1px smaller curly line,
-    // we aim for 2px height as a result.
-    const auto minCurlyLinePeakHeight = 2.0f;
-
-    // Curlyline uses the gap between cell bottom and singly underline position
-    // as the height of the wave's peak. The baseline for curly-line is at the
-    // middle of singly underline. The gap could be too big, so we also apply
-    // a limit on the peak height.
-    const auto strokeHalfWidth = font.underline.height / 2.0f;
-    const auto underlineMidY = font.underline.position + strokeHalfWidth;
-    const auto cellBottomGap = font.cellSize.y - underlineMidY - strokeHalfWidth;
-    const auto maxCurlyLinePeakHeight = maxCurlyLinePeakHeightEm * font.fontSize;
-    auto curlyLinePeakHeight = std::min(cellBottomGap, maxCurlyLinePeakHeight);
+    // Curlyline is drawn with a desired height relative to the font size. The
+    // baseline of curlyline is at the middle of singly underline. When there's
+    // limited space to draw a curlyline, we apply a limit on the peak height.
+    {
+        // initialize curlyline peak height to a desired value. Clamp it to at
+        // least 1.
+        constexpr auto curlyLinePeakHeightEm = 0.075f;
+        _curlyLinePeakHeight = std::max(1.0f, std::roundf(curlyLinePeakHeightEm * font.fontSize));
+
+        // calc the limit we need to apply
+        const auto strokeHalfWidth = std::floor(font.underline.height / 2.0f);
+        const auto underlineMidY = font.underline.position + strokeHalfWidth;
+        const auto maxDrawableCurlyLinePeakHeight = font.cellSize.y - underlineMidY - font.underline.height;
+
+        // if the limit is <= 0 (no height at all), stick with the desired height.
+        // This is how we force a curlyline even when there's no space, though it
+        // might be clipped at the bottom.
+        if (maxDrawableCurlyLinePeakHeight > 0.0f)
+        {
+            _curlyLinePeakHeight = std::min(_curlyLinePeakHeight, maxDrawableCurlyLinePeakHeight);
+        }
 
-    // When it's too small to be curly, make it straight.
-    if (curlyLinePeakHeight < minCurlyLinePeakHeight)
-    {
-        curlyLinePeakHeight = 0;
+        const auto curlyUnderlinePos = underlineMidY - _curlyLinePeakHeight - font.underline.height;
+        const auto curlyUnderlineWidth = 2.0f * (_curlyLinePeakHeight + font.underline.height);
+        const auto curlyUnderlinePosU16 = gsl::narrow_cast<u16>(lrintf(curlyUnderlinePos));
+        const auto curlyUnderlineWidthU16 = gsl::narrow_cast<u16>(lrintf(curlyUnderlineWidth));
+        _curlyUnderline = { curlyUnderlinePosU16, curlyUnderlineWidthU16 };
     }
 
-    // We draw a smaller curly line (-1px) to avoid clipping due to the rounding.
-    _curlyLineDrawPeakHeight = std::max(0.0f, curlyLinePeakHeight - 1.0f);
-
-    const auto curlyUnderlinePos = font.underline.position - curlyLinePeakHeight;
-    const auto curlyUnderlineWidth = 2.0f * (curlyLinePeakHeight + strokeHalfWidth);
-    const auto curlyUnderlinePosU16 = gsl::narrow_cast<u16>(lrintf(curlyUnderlinePos));
-    const auto curlyUnderlineWidthU16 = gsl::narrow_cast<u16>(lrintf(curlyUnderlineWidth));
-    _curlyUnderline = { curlyUnderlinePosU16, curlyUnderlineWidthU16 };
-
     DWrite_GetRenderParams(p.dwriteFactory.get(), &_gamma, &_cleartypeEnhancedContrast, &_grayscaleEnhancedContrast, _textRenderingParams.put());
     // Clearing the atlas requires BeginDraw(), which is expensive. Defer this until we need Direct2D anyways.
     _fontChangedResetGlyphAtlas = true;
@@ -576,7 +574,7 @@ void BackendD3D::_recreateConstBuffer(const RenderingPayload& p) const
         data.enhancedContrast = p.s->font->antialiasingMode == AntialiasingMode::ClearType ? _cleartypeEnhancedContrast : _grayscaleEnhancedContrast;
         data.underlineWidth = p.s->font->underline.height;
         data.curlyLineWaveFreq = 2.0f * 3.14f / p.s->font->cellSize.x;
-        data.curlyLinePeakHeight = _curlyLineDrawPeakHeight;
+        data.curlyLinePeakHeight = _curlyLinePeakHeight;
         data.curlyLineCellOffset = p.s->font->underline.position + p.s->font->underline.height / 2.0f;
         p.deviceContext->UpdateSubresource(_psConstantBuffer.get(), 0, nullptr, &data, 0, 0);
     }

src/renderer/atlas/BackendD3D.h

@@ -291,7 +291,7 @@ namespace Microsoft::Console::Render::Atlas
         // The bounding rect of _cursorRects in pixels.
         til::rect _cursorPosition;
 
-        f32 _curlyLineDrawPeakHeight = 0;
+        f32 _curlyLinePeakHeight = 0.0f;
         FontDecorationPosition _curlyUnderline;
 
         bool _requiresContinuousRedraw = false;

src/renderer/gdi/paint.cpp

@@ -536,27 +536,30 @@ bool GdiEngine::FontHasWesternScript(HDC hdc)
     const auto DrawLine = [=](const auto x, const auto y, const auto w, const auto h) {
         return PatBlt(_hdcMemoryContext, x, y, w, h, PATCOPY);
     };
-    const auto DrawStrokedLine = [&](const auto x, const auto y, const auto w) {
+    const auto DrawStrokedLine = [&](const til::CoordType x, const til::CoordType y, const unsigned w) {
         RETURN_HR_IF(E_FAIL, !MoveToEx(_hdcMemoryContext, x, y, nullptr));
-        RETURN_HR_IF(E_FAIL, !LineTo(_hdcMemoryContext, x + w, y));
+        RETURN_HR_IF(E_FAIL, !LineTo(_hdcMemoryContext, gsl::narrow_cast<int>(x + w), y));
         return S_OK;
     };
-    const auto DrawCurlyLine = [&](const auto x, const auto y, const auto cCurlyLines) {
+    const auto DrawCurlyLine = [&](const til::CoordType x, const til::CoordType y, const size_t cCurlyLines) {
         const auto curlyLineWidth = fontWidth;
-        const auto curlyLineHalfWidth = lrintf(curlyLineWidth / 2.0f);
-        const auto controlPointHeight = gsl::narrow_cast<long>(std::floor(3.5f * _lineMetrics.curlylinePeakHeight));
+        const auto curlyLineHalfWidth = std::lround(curlyLineWidth / 2.0f);
+        const auto controlPointHeight = std::lround(3.5f * _lineMetrics.curlylinePeakHeight);
+
         // Each curlyLine requires 3 `POINT`s
         const auto cPoints = gsl::narrow<DWORD>(3 * cCurlyLines);
         std::vector<POINT> points;
         points.reserve(cPoints);
+
         auto start = x;
-        for (auto i = 0u; i < cCurlyLines; i++)
+        for (size_t i = 0; i < cCurlyLines; i++)
         {
             points.emplace_back(start + curlyLineHalfWidth, y - controlPointHeight);
             points.emplace_back(start + curlyLineHalfWidth, y + controlPointHeight);
             points.emplace_back(start + curlyLineWidth, y);
             start += curlyLineWidth;
         }
+
         RETURN_HR_IF(E_FAIL, !MoveToEx(_hdcMemoryContext, x, y, nullptr));
         RETURN_HR_IF(E_FAIL, !PolyBezierTo(_hdcMemoryContext, points.data(), cPoints));
         return S_OK;
@@ -619,28 +622,26 @@ bool GdiEngine::FontHasWesternScript(HDC hdc)
     const auto prevPen = wil::SelectObject(_hdcMemoryContext, hpen.get());
     RETURN_HR_IF_NULL(E_FAIL, prevPen.get());
 
-    const auto underlineMidY = std::lround(ptTarget.y + _lineMetrics.underlineOffset + _lineMetrics.underlineWidth / 2.0f);
     if (lines.test(GridLines::Underline))
     {
-        return DrawStrokedLine(ptTarget.x, underlineMidY, widthOfAllCells);
+        return DrawStrokedLine(ptTarget.x, ptTarget.y + _lineMetrics.underlineOffset, widthOfAllCells);
     }
     else if (lines.test(GridLines::DoubleUnderline))
     {
-        const auto doubleUnderlineBottomLineMidY = std::lround(ptTarget.y + _lineMetrics.underlineOffset2 + _lineMetrics.underlineWidth / 2.0f);
-        RETURN_IF_FAILED(DrawStrokedLine(ptTarget.x, underlineMidY, widthOfAllCells));
-        return DrawStrokedLine(ptTarget.x, doubleUnderlineBottomLineMidY, widthOfAllCells);
+        RETURN_IF_FAILED(DrawStrokedLine(ptTarget.x, ptTarget.y + _lineMetrics.underlineOffset, widthOfAllCells));
+        return DrawStrokedLine(ptTarget.x, ptTarget.y + _lineMetrics.underlineOffset2, widthOfAllCells);
     }
     else if (lines.test(GridLines::CurlyUnderline))
     {
-        return DrawCurlyLine(ptTarget.x, underlineMidY, cchLine);
+        return DrawCurlyLine(ptTarget.x, ptTarget.y + _lineMetrics.underlineOffset, cchLine);
     }
     else if (lines.test(GridLines::DottedUnderline))
     {
-        return DrawStrokedLine(ptTarget.x, underlineMidY, widthOfAllCells);
+        return DrawStrokedLine(ptTarget.x, ptTarget.y + _lineMetrics.underlineOffset, widthOfAllCells);
     }
     else if (lines.test(GridLines::DashedUnderline))
     {
-        return DrawStrokedLine(ptTarget.x, underlineMidY, widthOfAllCells);
+        return DrawStrokedLine(ptTarget.x, ptTarget.y + _lineMetrics.underlineOffset, widthOfAllCells);
     }
 
     return S_OK;

src/renderer/gdi/state.cpp

@@ -11,15 +11,6 @@
 
 using namespace Microsoft::Console::Render;
 
-namespace
-{
-    // The max height of Curly line peak in `em` units.
-    constexpr auto MaxCurlyLinePeakHeightEm = 0.075f;
-
-    // The min height of Curly line peak.
-    constexpr auto MinCurlyLinePeakHeight = 2.0f;
-}
-
 // Routine Description:
 // - Creates a new GDI-based rendering engine
 // - NOTE: Will throw if initialization failure. Caller must catch.
@@ -406,29 +397,31 @@ GdiEngine::~GdiEngine()
         _lineMetrics.underlineOffset2 = _lineMetrics.underlineOffset - _lineMetrics.gridlineWidth;
     }
 
-    // Curly line doesn't render properly below 1px stroke width. Make it a straight line.
-    if (_lineMetrics.underlineWidth < 1)
-    {
-        _lineMetrics.curlylinePeakHeight = 0;
-    }
-    else
+    // Since we use GDI pen for drawing, the underline offset should point to
+    // the center of the underline.
+    const auto underlineHalfWidth = gsl::narrow_cast<int>(std::floor(_lineMetrics.underlineWidth / 2.0f));
+    _lineMetrics.underlineOffset += underlineHalfWidth;
+    _lineMetrics.underlineOffset2 += underlineHalfWidth;
+
+    // Curlyline is drawn with a desired height relative to the font size. The
+    // baseline of curlyline is at the middle of singly underline. When there's
+    // limited space to draw a curlyline, we apply a limit on the peak height.
     {
-        // Curlyline uses the gap between cell bottom and singly underline
-        // position as the height of the wave's peak. The baseline for curly
-        // line is at the middle of singly underline. The gap could be too big,
-        // so we also apply a limit on the peak height.
-        const auto strokeHalfWidth = _lineMetrics.underlineWidth / 2.0f;
-        const auto underlineMidY = _lineMetrics.underlineOffset + strokeHalfWidth;
-        const auto cellBottomGap = Font.GetSize().height - underlineMidY - strokeHalfWidth;
-        const auto maxCurlyLinePeakHeight = MaxCurlyLinePeakHeightEm * fontSize;
-        auto curlyLinePeakHeight = std::min(cellBottomGap, maxCurlyLinePeakHeight);
-
-        // When it's too small to be curly, make it a straight line.
-        if (curlyLinePeakHeight < MinCurlyLinePeakHeight)
+        // initialize curlyline peak height to a desired value. Clamp it to at
+        // least 1.
+        constexpr auto curlyLinePeakHeightEm = 0.075f;
+        _lineMetrics.curlylinePeakHeight = gsl::narrow_cast<int>(std::max(1L, std::lround(curlyLinePeakHeightEm * fontSize)));
+
+        // calc the limit we need to apply
+        const auto maxDrawableCurlyLinePeakHeight = Font.GetSize().height - _lineMetrics.underlineOffset - _lineMetrics.underlineWidth;
+
+        // if the limit is <= 0 (no height at all), stick with the desired height.
+        // This is how we force a curlyline even when there's no space, though it
+        // might be clipped at the bottom.
+        if (maxDrawableCurlyLinePeakHeight > 0.0f)
         {
-            curlyLinePeakHeight = 0.0f;
+            _lineMetrics.curlylinePeakHeight = std::min(_lineMetrics.curlylinePeakHeight, maxDrawableCurlyLinePeakHeight);
         }
-        _lineMetrics.curlylinePeakHeight = gsl::narrow_cast<int>(std::floor(curlyLinePeakHeight));
     }
 
     // Now find the size of a 0 in this current font and save it for conversions done later.