Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for the "doubly underlined" graphic rendition attribute #7223

Merged
4 commits merged into from
Aug 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/buffer/out/TextAttribute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions src/buffer/out/TextAttribute.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;

Expand Down
4 changes: 4 additions & 0 deletions src/cascadia/TerminalCore/TerminalDispatchGraphics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
47 changes: 45 additions & 2 deletions src/host/ut_host/ScreenBufferTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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}")
Expand All @@ -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));
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
29 changes: 27 additions & 2 deletions src/host/ut_host/VtRendererTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
3 changes: 1 addition & 2 deletions src/inc/conattrs.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 5 additions & 0 deletions src/renderer/base/renderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
29 changes: 27 additions & 2 deletions src/renderer/dx/DxRenderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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;
}
Comment on lines +2302 to +2308
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love how thoughtful this is. Thank you.


// 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();
Expand Down
1 change: 1 addition & 0 deletions src/renderer/dx/DxRenderer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ namespace Microsoft::Console::Render
{
float gridlineWidth;
float underlineOffset;
float underlineOffset2;
float underlineWidth;
float strikethroughOffset;
float strikethroughWidth;
Expand Down
1 change: 1 addition & 0 deletions src/renderer/gdi/gdirenderer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ namespace Microsoft::Console::Render
{
int gridlineWidth;
int underlineOffset;
int underlineOffset2;
int underlineWidth;
int strikethroughOffset;
int strikethroughWidth;
Expand Down
8 changes: 7 additions & 1 deletion src/renderer/gdi/paint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
18 changes: 18 additions & 0 deletions src/renderer/gdi/state.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
3 changes: 2 additions & 1 deletion src/renderer/inc/IRenderEngine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ namespace Microsoft::Console::Render
Left = 0x4,
Right = 0x8,
Underline = 0x10,
Strikethrough = 0x20
DoubleUnderline = 0x20,
Strikethrough = 0x40
};

virtual ~IRenderEngine() = 0;
Expand Down
11 changes: 11 additions & 0 deletions src/renderer/vt/VtSequences.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading