maxmustermann/terminal
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Render the SGR "underlined" attribute in the style of the font (#7148)

Browse source 
This PR updates the rendering of the _underlined_ graphic rendition
attribute, using the style specified in the active font, instead of just
reusing the grid line at the bottom of the character cell.

* Support for drawing the correct underline effect in the grid line
  renderer was added in #7107.

There was already an `ExtendedAttributes` flag defined for the
underlined state, but I needed to update the `SetUnderlined` and
`IsUnderlined` methods in the `TextAttribute`  class to use that flag
now in place of the legacy `LVB_UNDERSCORE` attribute. This enables
underlines set via a VT sequence to be tracked separately from
`LVB_UNDERSCORE` grid lines set via the console API.

I then needed to update the `Renderer::s_GetGridlines` method to
activate the `GridLines::Underline` style when the `Underlined`
attribute was set. The `GridLines::Bottom` style is still triggered by
the `LVB_UNDERSCORE` attribute to produce the bottom grid line effect.

Validation
----------

Because this is a change from the existing behaviour, certain unit tests
that were expecting the `LVB_UNDERSCORE` to be toggled by `SGR 4` and
`SGR 24` have now had to be updated to check the `Underlined` flag
instead.

There were also some UI Automation tests that were checking for `SGR 4`
mapping to `LVB_UNDERSCORE` attribute, which I've now substituted with a
test of the `SGR 53` overline attribute mapping to
`LVB_GRID_HORIZONTAL`. These tests only work with legacy attributes, so
they can't access the extended underline state, and I thought a
replacement test that covered similar ground would be better than
dropping the tests altogether.

As far as the visual rendering is concerned, I've manually confirmed
that the VT underline sequences now draw the underline in the correct
position and style, while grid lines output via the console API are
still displayed in their original form.

Closes #2915
This commit is contained in:
James Holderness 2020-08-03 13:49:25 +01:00 committed by GitHub
parent ef4aed944a
commit 158a1708a6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 25 additions and 20 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
src/buffer/out/TextAttribute.cpp
View file

@ -246,8 +246,7 @@ bool TextAttribute::IsCrossedOut() const noexcept
bool TextAttribute::IsUnderlined() const noexcept
{
// TODO:GH#2915 Treat underline separately from LVB_UNDERSCORE
return WI_IsFlagSet(_wAttrLegacy, COMMON_LVB_UNDERSCORE);
return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::Underlined);
}
bool TextAttribute::IsOverlined() const noexcept
@ -292,8 +291,7 @@ void TextAttribute::SetCrossedOut(bool isCrossedOut) noexcept
void TextAttribute::SetUnderlined(bool isUnderlined) noexcept
{
// TODO:GH#2915 Treat underline separately from LVB_UNDERSCORE
WI_UpdateFlag(_wAttrLegacy, COMMON_LVB_UNDERSCORE, isUnderlined);
WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Underlined, isUnderlined);
}
void TextAttribute::SetOverlined(bool isOverlined) noexcept

16
src/host/ft_uia/VirtualTerminalTests.cs
View file

@ -381,23 +381,23 @@ namespace Conhost.UIA.Tests
ciActual = area.GetCharInfoAt(hConsole, pt);
Verify.AreEqual(ciExpected, ciActual, "Verify that background bright cyan got set.");
Log.Comment("Set underline (SGR.4)");
Log.Comment("Set overline (SGR.53)");
app.FillCursorPosition(hConsole, ref pt);
app.UIRoot.SendKeys("e`");
ciExpected.Attributes |= WinCon.CONSOLE_ATTRIBUTES.COMMON_LVB_UNDERSCORE;
ciExpected.Attributes |= WinCon.CONSOLE_ATTRIBUTES.COMMON_LVB_GRID_HORIZONTAL;
ciActual = area.GetCharInfoAt(hConsole, pt);
Verify.AreEqual(ciExpected, ciActual, "Verify that underline got set.");
Verify.AreEqual(ciExpected, ciActual, "Verify that overline got set.");
Log.Comment("Clear underline (SGR.24)");
Log.Comment("Clear overline (SGR.55)");
app.FillCursorPosition(hConsole, ref pt);
app.UIRoot.SendKeys("d`");
ciExpected.Attributes &= ~WinCon.CONSOLE_ATTRIBUTES.COMMON_LVB_UNDERSCORE;
ciExpected.Attributes &= ~WinCon.CONSOLE_ATTRIBUTES.COMMON_LVB_GRID_HORIZONTAL;
ciActual = area.GetCharInfoAt(hConsole, pt);
Verify.AreEqual(ciExpected, ciActual, "Verify that underline got cleared.");
Verify.AreEqual(ciExpected, ciActual, "Verify that overline got cleared.");
Log.Comment("Set negative image video (SGR.7)");
app.FillCursorPosition(hConsole, ref pt);
@ -426,14 +426,14 @@ namespace Conhost.UIA.Tests
ciActual = area.GetCharInfoAt(hConsole, pt);
Verify.AreEqual(ciExpected, ciActual, "Verify that we got set back to the original state.");
Log.Comment("Set multiple properties in the same message (SGR.1,37,43,4)");
Log.Comment("Set multiple properties in the same message (SGR.1,37,43,53)");
app.FillCursorPosition(hConsole, ref pt);
app.UIRoot.SendKeys("9`");
ciExpected.Attributes |= WinCon.CONSOLE_ATTRIBUTES.FOREGROUND_COLORS;
ciExpected.Attributes |= WinCon.CONSOLE_ATTRIBUTES.FOREGROUND_INTENSITY;
ciExpected.Attributes |= WinCon.CONSOLE_ATTRIBUTES.BACKGROUND_YELLOW;
ciExpected.Attributes |= WinCon.CONSOLE_ATTRIBUTES.COMMON_LVB_UNDERSCORE;
ciExpected.Attributes |= WinCon.CONSOLE_ATTRIBUTES.COMMON_LVB_GRID_HORIZONTAL;
ciActual = area.GetCharInfoAt(hConsole, pt);
Verify.AreEqual(ciExpected, ciActual, "Verify that we set foreground bright white, background yellow, and underscore in the same SGR command.");

4
src/host/ut_host/TextBufferTests.cpp
View file

@ -757,8 +757,8 @@ void TextBufferTests::TestMixedRgbAndLegacyUnderline()
VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrA), std::make_pair(fgColor, bgColor));
VERIFY_ARE_EQUAL(gci.LookupAttributeColors(attrB), std::make_pair(fgColor, bgColor));
VERIFY_ARE_EQUAL(attrA.GetLegacyAttributes() & COMMON_LVB_UNDERSCORE, 0);
VERIFY_ARE_EQUAL(attrB.GetLegacyAttributes() & COMMON_LVB_UNDERSCORE, COMMON_LVB_UNDERSCORE);
VERIFY_ARE_EQUAL(attrA.IsUnderlined(), false);
VERIFY_ARE_EQUAL(attrB.IsUnderlined(), true);
wchar_t* reset = L"\x1b[0m";
stateMachine.ProcessString(reset);

2
src/inc/conattrs.hpp
View file

@ -17,7 +17,7 @@ enum class ExtendedAttributes : BYTE
Invisible = 0x08,
CrossedOut = 0x10,
// TODO:GH#2916 add support for these to the parser as well.
Underlined = 0x20, // _technically_ different from LVB_UNDERSCORE, see TODO:GH#2915
Underlined = 0x20,
DoublyUnderlined = 0x40, // Included for completeness, but not currently supported.
Faint = 0x80,
};

5
src/renderer/base/renderer.cpp
View file

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

6
src/terminal/adapter/ut_adapter/adapterTest.cpp
View file

@ -1298,7 +1298,8 @@ public:
case DispatchTypes::GraphicsOptions::Underline:
Log::Comment(L"Testing graphics 'Underline'");
_testGetSet->_attribute = TextAttribute{ 0 };
_testGetSet->_expectedAttribute = TextAttribute{ COMMON_LVB_UNDERSCORE };
_testGetSet->_expectedAttribute = TextAttribute{ 0 };
_testGetSet->_expectedAttribute.SetUnderlined(true);
break;
case DispatchTypes::GraphicsOptions::Overline:
Log::Comment(L"Testing graphics 'Overline'");
@ -1331,7 +1332,8 @@ public:
break;
case DispatchTypes::GraphicsOptions::NoUnderline:
Log::Comment(L"Testing graphics 'No Underline'");
_testGetSet->_attribute = TextAttribute{ COMMON_LVB_UNDERSCORE };
_testGetSet->_attribute = TextAttribute{ 0 };
_testGetSet->_attribute.SetUnderlined(true);
_testGetSet->_expectedAttribute = TextAttribute{ 0 };
break;
case DispatchTypes::GraphicsOptions::NoOverline:

6
src/tools/vtapp/Program.cs
View file

@ -236,11 +236,11 @@ namespace VTApp
break;
case 'e':
Console.Write(CSI);
Console.Write("4m");
Console.Write("53m");
break;
case 'd':
Console.Write(CSI);
Console.Write("24m");
Console.Write("55m");
break;
case 'r':
Console.Write(CSI);
@ -260,7 +260,7 @@ namespace VTApp
break;
case '9':
Console.Write(CSI);
Console.Write("1;37;43;4m");
Console.Write("1;37;43;53m");
break;
case '(':
Console.Write(CSI);