Add support for the "doubly underlined" graphic rendition attribute (#…

…7223)

This PR adds support for the ANSI _doubly underlined_ graphic rendition
attribute, which is enabled by the `SGR 21` escape sequence.

There was already an `ExtendedAttributes::DoublyUnderlined` flag in the
`TextAttribute` class, but I needed to add `SetDoublyUnderlined` and
`IsDoublyUnderlined` methods to access that flag, and update the
`SetGraphicsRendition` methods of the two dispatchers to set the
attribute on receipt of the `SGR 21` sequence. I also had to update the
existing `SGR 24` handler to reset _DoublyUnderlined_ in addition to
_Underlined_, since they share the same reset sequence.

For the rendering, I've added a new grid line type, which essentially
just draws an additional line with the same thickness as the regular
underline, but slightly below it - I found a gap of around 0.05 "em"
between the lines looked best. If there isn't enough space in the cell
for that gap, the second line will be clamped to overlap the first, so
you then just get a thicker line. If there isn't even enough space below
for a thicker line, we move the offset _above_ the first line, but just
enough to make it thicker.

The only other complication was the update of the `Xterm256Engine` in
the VT renderer. As mentioned above, the two underline attributes share
the same reset sequence, so to forward that state over conpty we require
a slightly more complicated process than with most other attributes
(similar to _Bold_ and _Faint_). We first check whether either underline
attribute needs to be turned off to send the reset sequence, and then
check individually if each of them needs to be turned back on again.

## Validation Steps Performed

For testing, I've extended the existing attribute tests in
`AdapterTest`, `VTRendererTest`, and `ScreenBufferTests`, to make sure
we're covering both the _Underlined_ and _DoublyUnderlined_ attributes.

I've also manually tested the `SGR 21` sequence in conhost and Windows
Terminal, with a variety of fonts and font sizes, to make sure the
rendering was reasonably distinguishable from a single underline.

Closes #2916

src/buffer/out/TextAttribute.cpp

@@ -249,6 +249,11 @@ bool TextAttribute::IsUnderlined() const noexcept
     return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::Underlined);
 }
 
+bool TextAttribute::IsDoublyUnderlined() const noexcept
+{
+    return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::DoublyUnderlined);
+}
+
 bool TextAttribute::IsOverlined() const noexcept
 {
     return WI_IsFlagSet(_wAttrLegacy, COMMON_LVB_GRID_HORIZONTAL);
@@ -294,6 +299,11 @@ void TextAttribute::SetUnderlined(bool isUnderlined) noexcept
     WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Underlined, isUnderlined);
 }
 
+void TextAttribute::SetDoublyUnderlined(bool isDoublyUnderlined) noexcept
+{
+    WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::DoublyUnderlined, isDoublyUnderlined);
+}
+
 void TextAttribute::SetOverlined(bool isOverlined) noexcept
 {
     WI_UpdateFlag(_wAttrLegacy, COMMON_LVB_GRID_HORIZONTAL, isOverlined);

src/buffer/out/TextAttribute.hpp

@@ -95,6 +95,7 @@ class TextAttribute final
     bool IsInvisible() const noexcept;
     bool IsCrossedOut() const noexcept;
     bool IsUnderlined() const noexcept;
+    bool IsDoublyUnderlined() const noexcept;
     bool IsOverlined() const noexcept;
     bool IsReverseVideo() const noexcept;
 
@@ -105,6 +106,7 @@ class TextAttribute final
     void SetInvisible(bool isInvisible) noexcept;
     void SetCrossedOut(bool isCrossedOut) noexcept;
     void SetUnderlined(bool isUnderlined) noexcept;
+    void SetDoublyUnderlined(bool isDoublyUnderlined) noexcept;
     void SetOverlined(bool isOverlined) noexcept;
     void SetReverseVideo(bool isReversed) noexcept;
 

src/cascadia/TerminalCore/TerminalDispatchGraphics.cpp

@@ -158,8 +158,12 @@ bool TerminalDispatch::SetGraphicsRendition(const gsl::span<const DispatchTypes:
         case Underline:
             attr.SetUnderlined(true);
             break;
+        case DoublyUnderlined:
+            attr.SetDoublyUnderlined(true);
+            break;
         case NoUnderline:
             attr.SetUnderlined(false);
+            attr.SetDoublyUnderlined(false);
             break;
         case Overline:
             attr.SetOverlined(true);

src/host/ut_host/ScreenBufferTests.cpp

@@ -5016,15 +5016,19 @@ void ScreenBufferTests::TestExtendedTextAttributes()
         TEST_METHOD_PROPERTY(L"Data:bold", L"{false, true}")
         TEST_METHOD_PROPERTY(L"Data:faint", L"{false, true}")
         TEST_METHOD_PROPERTY(L"Data:italics", L"{false, true}")
+        TEST_METHOD_PROPERTY(L"Data:underlined", L"{false, true}")
+        TEST_METHOD_PROPERTY(L"Data:doublyUnderlined", L"{false, true}")
         TEST_METHOD_PROPERTY(L"Data:blink", L"{false, true}")
         TEST_METHOD_PROPERTY(L"Data:invisible", L"{false, true}")
         TEST_METHOD_PROPERTY(L"Data:crossedOut", L"{false, true}")
     END_TEST_METHOD_PROPERTIES()
 
-    bool bold, faint, italics, blink, invisible, crossedOut;
+    bool bold, faint, italics, underlined, doublyUnderlined, blink, invisible, crossedOut;
     VERIFY_SUCCEEDED(TestData::TryGetValue(L"bold", bold));
     VERIFY_SUCCEEDED(TestData::TryGetValue(L"faint", faint));
     VERIFY_SUCCEEDED(TestData::TryGetValue(L"italics", italics));
+    VERIFY_SUCCEEDED(TestData::TryGetValue(L"underlined", underlined));
+    VERIFY_SUCCEEDED(TestData::TryGetValue(L"doublyUnderlined", doublyUnderlined));
     VERIFY_SUCCEEDED(TestData::TryGetValue(L"blink", blink));
     VERIFY_SUCCEEDED(TestData::TryGetValue(L"invisible", invisible));
     VERIFY_SUCCEEDED(TestData::TryGetValue(L"crossedOut", crossedOut));
@@ -5055,6 +5059,16 @@ void ScreenBufferTests::TestExtendedTextAttributes()
         WI_SetFlag(expectedAttrs, ExtendedAttributes::Italics);
         vtSeq += L"\x1b[3m";
     }
+    if (underlined)
+    {
+        WI_SetFlag(expectedAttrs, ExtendedAttributes::Underlined);
+        vtSeq += L"\x1b[4m";
+    }
+    if (doublyUnderlined)
+    {
+        WI_SetFlag(expectedAttrs, ExtendedAttributes::DoublyUnderlined);
+        vtSeq += L"\x1b[21m";
+    }
     if (blink)
     {
         WI_SetFlag(expectedAttrs, ExtendedAttributes::Blinking);
@@ -5120,6 +5134,13 @@ void ScreenBufferTests::TestExtendedTextAttributes()
         vtSeq = L"\x1b[23m";
         validate(expectedAttrs, vtSeq);
     }
+    if (underlined || doublyUnderlined)
+    {
+        // The two underlined attributes share the same reset sequence.
+        WI_ClearAllFlags(expectedAttrs, ExtendedAttributes::Underlined | ExtendedAttributes::DoublyUnderlined);
+        vtSeq = L"\x1b[24m";
+        validate(expectedAttrs, vtSeq);
+    }
     if (blink)
     {
         WI_ClearFlag(expectedAttrs, ExtendedAttributes::Blinking);
@@ -5157,6 +5178,8 @@ void ScreenBufferTests::TestExtendedTextAttributesWithColors()
         TEST_METHOD_PROPERTY(L"Data:bold", L"{false, true}")
         TEST_METHOD_PROPERTY(L"Data:faint", L"{false, true}")
         TEST_METHOD_PROPERTY(L"Data:italics", L"{false, true}")
+        TEST_METHOD_PROPERTY(L"Data:underlined", L"{false, true}")
+        TEST_METHOD_PROPERTY(L"Data:doublyUnderlined", L"{false, true}")
         TEST_METHOD_PROPERTY(L"Data:blink", L"{false, true}")
         TEST_METHOD_PROPERTY(L"Data:invisible", L"{false, true}")
         TEST_METHOD_PROPERTY(L"Data:crossedOut", L"{false, true}")
@@ -5171,10 +5194,12 @@ void ScreenBufferTests::TestExtendedTextAttributesWithColors()
     const int Use256Color = 2;
     const int UseRGBColor = 3;
 
-    bool bold, faint, italics, blink, invisible, crossedOut;
+    bool bold, faint, italics, underlined, doublyUnderlined, blink, invisible, crossedOut;
     VERIFY_SUCCEEDED(TestData::TryGetValue(L"bold", bold));
     VERIFY_SUCCEEDED(TestData::TryGetValue(L"faint", faint));
     VERIFY_SUCCEEDED(TestData::TryGetValue(L"italics", italics));
+    VERIFY_SUCCEEDED(TestData::TryGetValue(L"underlined", underlined));
+    VERIFY_SUCCEEDED(TestData::TryGetValue(L"doublyUnderlined", doublyUnderlined));
     VERIFY_SUCCEEDED(TestData::TryGetValue(L"blink", blink));
     VERIFY_SUCCEEDED(TestData::TryGetValue(L"invisible", invisible));
     VERIFY_SUCCEEDED(TestData::TryGetValue(L"crossedOut", crossedOut));
@@ -5209,6 +5234,16 @@ void ScreenBufferTests::TestExtendedTextAttributesWithColors()
         expectedAttr.SetItalic(true);
         vtSeq += L"\x1b[3m";
     }
+    if (underlined)
+    {
+        expectedAttr.SetUnderlined(true);
+        vtSeq += L"\x1b[4m";
+    }
+    if (doublyUnderlined)
+    {
+        expectedAttr.SetDoublyUnderlined(true);
+        vtSeq += L"\x1b[21m";
+    }
     if (blink)
     {
         expectedAttr.SetBlinking(true);
@@ -5319,6 +5354,14 @@ void ScreenBufferTests::TestExtendedTextAttributesWithColors()
         vtSeq = L"\x1b[23m";
         validate(expectedAttr, vtSeq);
     }
+    if (underlined || doublyUnderlined)
+    {
+        // The two underlined attributes share the same reset sequence.
+        expectedAttr.SetUnderlined(false);
+        expectedAttr.SetDoublyUnderlined(false);
+        vtSeq = L"\x1b[24m";
+        validate(expectedAttr, vtSeq);
+    }
     if (blink)
     {
         expectedAttr.SetBlinking(false);

src/host/ut_host/VtRendererTests.cpp

@@ -661,14 +661,18 @@ void VtRendererTest::Xterm256TestExtendedAttributes()
     // Run this test for each and every possible combination of states.
     BEGIN_TEST_METHOD_PROPERTIES()
         TEST_METHOD_PROPERTY(L"Data:faint", L"{false, true}")
+        TEST_METHOD_PROPERTY(L"Data:underlined", L"{false, true}")
+        TEST_METHOD_PROPERTY(L"Data:doublyUnderlined", L"{false, true}")
         TEST_METHOD_PROPERTY(L"Data:italics", L"{false, true}")
         TEST_METHOD_PROPERTY(L"Data:blink", L"{false, true}")
         TEST_METHOD_PROPERTY(L"Data:invisible", L"{false, true}")
         TEST_METHOD_PROPERTY(L"Data:crossedOut", L"{false, true}")
     END_TEST_METHOD_PROPERTIES()
 
-    bool faint, italics, blink, invisible, crossedOut;
+    bool faint, underlined, doublyUnderlined, italics, blink, invisible, crossedOut;
     VERIFY_SUCCEEDED(TestData::TryGetValue(L"faint", faint));
+    VERIFY_SUCCEEDED(TestData::TryGetValue(L"underlined", underlined));
+    VERIFY_SUCCEEDED(TestData::TryGetValue(L"doublyUnderlined", doublyUnderlined));
     VERIFY_SUCCEEDED(TestData::TryGetValue(L"italics", italics));
     VERIFY_SUCCEEDED(TestData::TryGetValue(L"blink", blink));
     VERIFY_SUCCEEDED(TestData::TryGetValue(L"invisible", invisible));
@@ -684,6 +688,23 @@ void VtRendererTest::Xterm256TestExtendedAttributes()
         onSequences.push_back("\x1b[2m");
         offSequences.push_back("\x1b[22m");
     }
+    if (underlined)
+    {
+        desiredAttrs.SetUnderlined(true);
+        onSequences.push_back("\x1b[4m");
+        offSequences.push_back("\x1b[24m");
+    }
+    if (doublyUnderlined)
+    {
+        desiredAttrs.SetDoublyUnderlined(true);
+        onSequences.push_back("\x1b[21m");
+        // The two underlines share the same off sequence, so we
+        // only add it here if that hasn't already been done.
+        if (!underlined)
+        {
+            offSequences.push_back("\x1b[24m");
+        }
+    }
     if (italics)
     {
         desiredAttrs.SetItalic(true);
@@ -754,7 +775,7 @@ void VtRendererTest::Xterm256TestExtendedAttributes()
 void VtRendererTest::Xterm256TestAttributesAcrossReset()
 {
     BEGIN_TEST_METHOD_PROPERTIES()
-        TEST_METHOD_PROPERTY(L"Data:renditionAttribute", L"{1, 2, 3, 4, 5, 7, 8, 9, 53}")
+        TEST_METHOD_PROPERTY(L"Data:renditionAttribute", L"{1, 2, 3, 4, 5, 7, 8, 9, 21, 53}")
     END_TEST_METHOD_PROPERTIES()
 
     int renditionAttribute;
@@ -794,6 +815,10 @@ void VtRendererTest::Xterm256TestAttributesAcrossReset()
         Log::Comment(L"----Set Underline Attribute----");
         textAttributes.SetUnderlined(true);
         break;
+    case GraphicsOptions::DoublyUnderlined:
+        Log::Comment(L"----Set Doubly Underlined Attribute----");
+        textAttributes.SetDoublyUnderlined(true);
+        break;
     case GraphicsOptions::Overline:
         Log::Comment(L"----Set Overline Attribute----");
         textAttributes.SetOverlined(true);

src/inc/conattrs.hpp

@@ -16,9 +16,8 @@ enum class ExtendedAttributes : BYTE
     Blinking = 0x04,
     Invisible = 0x08,
     CrossedOut = 0x10,
-    // TODO:GH#2916 add support for these to the parser as well.
     Underlined = 0x20,
-    DoublyUnderlined = 0x40, // Included for completeness, but not currently supported.
+    DoublyUnderlined = 0x40,
     Faint = 0x80,
 };
 DEFINE_ENUM_FLAG_OPERATORS(ExtendedAttributes);

src/renderer/base/renderer.cpp

@@ -870,6 +870,11 @@ IRenderEngine::GridLines Renderer::s_GetGridlines(const TextAttribute& textAttri
     {
         lines |= IRenderEngine::GridLines::Underline;
     }
+
+    if (textAttribute.IsDoublyUnderlined())
+    {
+        lines |= IRenderEngine::GridLines::DoubleUnderline;
+    }
     return lines;
 }
 

src/renderer/dx/DxRenderer.cpp

@@ -1544,14 +1544,20 @@ try
     // In the case of the underline and strikethrough offsets, the stroke width
     // is already accounted for, so they don't require further adjustments.
 
-    if (lines & GridLines::Underline)
+    if (lines & (GridLines::Underline | GridLines::DoubleUnderline))
     {
         const auto halfUnderlineWidth = _lineMetrics.underlineWidth / 2.0f;
         const auto startX = target.x + halfUnderlineWidth;
         const auto endX = target.x + fullRunWidth - halfUnderlineWidth;
         const auto y = target.y + _lineMetrics.underlineOffset;
 
         DrawLine(startX, y, endX, y, _lineMetrics.underlineWidth);
+
+        if (lines & GridLines::DoubleUnderline)
+        {
+            const auto y2 = target.y + _lineMetrics.underlineOffset2;
+            DrawLine(startX, y2, endX, y2, _lineMetrics.underlineWidth);
+        }
     }
 
     if (lines & GridLines::Strikethrough)
@@ -2283,9 +2289,28 @@ CATCH_RETURN();
         lineMetrics.underlineOffset = fullPixelAscent - lineMetrics.underlineOffset;
         lineMetrics.strikethroughOffset = fullPixelAscent - lineMetrics.strikethroughOffset;
 
-        // We also add half the stroke width to the offset, since the line
+        // For double underlines we need a second offset, just below the first,
+        // but with a bit of a gap (about double the grid line width).
+        lineMetrics.underlineOffset2 = lineMetrics.underlineOffset +
+                                       lineMetrics.underlineWidth +
+                                       std::round(fontSize * 0.05f);
+
+        // However, we don't want the underline to extend past the bottom of the
+        // cell, so we clamp the offset to fit just inside.
+        const auto maxUnderlineOffset = lineSpacing.height - _lineMetrics.underlineWidth;
+        lineMetrics.underlineOffset2 = std::min(lineMetrics.underlineOffset2, maxUnderlineOffset);
+
+        // But if the resulting gap isn't big enough even to register as a thicker
+        // line, it's better to place the second line slightly above the first.
+        if (lineMetrics.underlineOffset2 < lineMetrics.underlineOffset + lineMetrics.gridlineWidth)
+        {
+            lineMetrics.underlineOffset2 = lineMetrics.underlineOffset - lineMetrics.gridlineWidth;
+        }
+
+        // We also add half the stroke width to the offsets, since the line
         // coordinates designate the center of the line.
         lineMetrics.underlineOffset += lineMetrics.underlineWidth / 2.0f;
+        lineMetrics.underlineOffset2 += lineMetrics.underlineWidth / 2.0f;
         lineMetrics.strikethroughOffset += lineMetrics.strikethroughWidth / 2.0f;
     }
     CATCH_RETURN();

src/renderer/dx/DxRenderer.hpp

@@ -146,6 +146,7 @@ namespace Microsoft::Console::Render
         {
             float gridlineWidth;
             float underlineOffset;
+            float underlineOffset2;
             float underlineWidth;
             float strikethroughOffset;
             float strikethroughWidth;

src/renderer/gdi/gdirenderer.hpp

@@ -100,6 +100,7 @@ namespace Microsoft::Console::Render
         {
             int gridlineWidth;
             int underlineOffset;
+            int underlineOffset2;
             int underlineWidth;
             int strikethroughOffset;
             int strikethroughWidth;

src/renderer/gdi/paint.cpp

@@ -501,10 +501,16 @@ using namespace Microsoft::Console::Render;
         RETURN_HR_IF(E_FAIL, !DrawLine(ptTarget.x, y, widthOfAllCells, _lineMetrics.gridlineWidth));
     }
 
-    if (lines & GridLines::Underline)
+    if (lines & (GridLines::Underline | GridLines::DoubleUnderline))
     {
         const auto y = ptTarget.y + _lineMetrics.underlineOffset;
         RETURN_HR_IF(E_FAIL, !DrawLine(ptTarget.x, y, widthOfAllCells, _lineMetrics.underlineWidth));
+
+        if (lines & GridLines::DoubleUnderline)
+        {
+            const auto y2 = ptTarget.y + _lineMetrics.underlineOffset2;
+            RETURN_HR_IF(E_FAIL, !DrawLine(ptTarget.x, y2, widthOfAllCells, _lineMetrics.underlineWidth));
+        }
     }
 
     if (lines & GridLines::Strikethrough)

src/renderer/gdi/state.cpp

@@ -269,6 +269,24 @@ GdiEngine::~GdiEngine()
     _lineMetrics.underlineOffset = ascent - _lineMetrics.underlineOffset;
     _lineMetrics.strikethroughOffset = ascent - _lineMetrics.strikethroughOffset;
 
+    // For double underlines we need a second offset, just below the first,
+    // but with a bit of a gap (about double the grid line width).
+    _lineMetrics.underlineOffset2 = _lineMetrics.underlineOffset +
+                                    _lineMetrics.underlineWidth +
+                                    std::lround(fontSize * 0.05);
+
+    // However, we don't want the underline to extend past the bottom of the
+    // cell, so we clamp the offset to fit just inside.
+    const auto maxUnderlineOffset = Font.GetSize().Y - _lineMetrics.underlineWidth;
+    _lineMetrics.underlineOffset2 = std::min(_lineMetrics.underlineOffset2, maxUnderlineOffset);
+
+    // But if the resulting gap isn't big enough even to register as a thicker
+    // line, it's better to place the second line slightly above the first.
+    if (_lineMetrics.underlineOffset2 < _lineMetrics.underlineOffset + _lineMetrics.gridlineWidth)
+    {
+        _lineMetrics.underlineOffset2 = _lineMetrics.underlineOffset - _lineMetrics.gridlineWidth;
+    }
+
     // Now find the size of a 0 in this current font and save it for conversions done later.
     _coordFontLast = Font.GetSize();
 

src/renderer/inc/IRenderEngine.hpp

@@ -37,7 +37,8 @@ namespace Microsoft::Console::Render
             Left = 0x4,
             Right = 0x8,
             Underline = 0x10,
-            Strikethrough = 0x20
+            DoubleUnderline = 0x20,
+            Strikethrough = 0x40
         };
 
         virtual ~IRenderEngine() = 0;

src/renderer/vt/VtSequences.cpp

@@ -366,6 +366,17 @@ using namespace Microsoft::Console::Render;
     return _Write(isUnderlined ? "\x1b[4m" : "\x1b[24m");
 }
 
+// Method Description:
+// - Formats and writes a sequence to change the double underline of the following text.
+// Arguments:
+// - isUnderlined: If true, we'll doubly underline the text. Otherwise we'll remove the underline.
+// Return Value:
+// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write.
+[[nodiscard]] HRESULT VtEngine::_SetDoublyUnderlined(const bool isUnderlined) noexcept
+{
+    return _Write(isUnderlined ? "\x1b[21m" : "\x1b[24m");
+}
+
 // Method Description:
 // - Formats and writes a sequence to change the overline of the following text.
 // Arguments: