Skip to content

Commit

Permalink
Add support for the "faint" graphic rendition attribute (#6873)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request

This PR adds support for the `SGR 2` escape sequence, which enables the
ANSI _faint_ graphic rendition attribute. When a character is output
with this attribute set, it uses a dimmer version of the active
foreground color.

## PR Checklist
* [x] Closes #6703
* [x] CLA signed. 
* [x] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [x] I've discussed this with core contributors already. Issue number where discussion took place: #6703

## Detailed Description of the Pull Request / Additional comments

There was already an `ExtendedAttributes::Faint` flag in the
`TextAttribute` class, but I needed to add `SetFaint` and `IsFaint`
methods to access that flag, and update the `SetGraphicsRendition`
methods of the two dispatchers to set the attribute on receipt of the
`SGR 2` sequence. I also had to update the existing `SGR 22` handler to
reset _Faint_ in addition to _Bold_, since they share the same reset
sequence. For that reason, I thought it a good idea to change the name
of the `SGR 22` enum to `NotBoldOrFaint`.

For the purpose of rendering, I've updated the
`TextAttribute::CalculateRgbColors` method to return a dimmer version of
the foreground color when the _Faint_ attribute is set. This is simply
achieved by dividing each color component by two, which produces a
reasonable effect without being too complicated. Note that the _Faint_
effect is applied before _Reverse Video_, so if the output it reversed,
it's the background that will be faint.

The only other complication was the update of the `Xterm256Engine` in
the VT renderer. As mentioned above, _Bold_ and _Faint_ share the same
reset sequence, so to forward that state over conpty we have to go
through a slightly more complicated process than with other attributes.
We first check whether either attribute needs to be turned off to send
the reset sequence, and then check if the individual attributes need to
be turned on again.

## Validation

I've extended the existing SGR unit tests to cover the new attribute in
the `AdapterTest`, the `ScreenBufferTests`, and the `VtRendererTest`,
and added a test to confirm the color calculations  when _Faint_ is set
in the `TextAttributeTests`.

I've also done a bunch of manual testing with all the different VT color
types and confirmed that our output is comparable to most other
terminals.
  • Loading branch information
j4james authored Jul 13, 2020
1 parent 1c8e83d commit 7d677c5
Show file tree
Hide file tree
Showing 13 changed files with 131 additions and 15 deletions.
14 changes: 14 additions & 0 deletions src/buffer/out/TextAttribute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@ std::pair<COLORREF, COLORREF> TextAttribute::CalculateRgbColors(const std::basic
{
auto fg = _foreground.GetColor(colorTable, defaultFgColor, IsBold());
auto bg = _background.GetColor(colorTable, defaultBgColor);
if (IsFaint())
{
fg = (fg >> 1) & 0x7F7F7F; // Divide foreground color components by two.
}
if (IsReverseVideo() ^ reverseScreenMode)
{
std::swap(fg, bg);
Expand Down Expand Up @@ -211,6 +215,11 @@ bool TextAttribute::IsBold() const noexcept
return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::Bold);
}

bool TextAttribute::IsFaint() const noexcept
{
return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::Faint);
}

bool TextAttribute::IsItalic() const noexcept
{
return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::Italics);
Expand Down Expand Up @@ -252,6 +261,11 @@ void TextAttribute::SetBold(bool isBold) noexcept
WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Bold, isBold);
}

void TextAttribute::SetFaint(bool isFaint) noexcept
{
WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Faint, isFaint);
}

void TextAttribute::SetItalics(bool isItalic) noexcept
{
WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Italics, isItalic);
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 @@ -89,6 +89,7 @@ class TextAttribute final

bool IsLegacy() const noexcept;
bool IsBold() const noexcept;
bool IsFaint() const noexcept;
bool IsItalic() const noexcept;
bool IsBlinking() const noexcept;
bool IsInvisible() const noexcept;
Expand All @@ -98,6 +99,7 @@ class TextAttribute final
bool IsReverseVideo() const noexcept;

void SetBold(bool isBold) noexcept;
void SetFaint(bool isFaint) noexcept;
void SetItalics(bool isItalic) noexcept;
void SetBlinking(bool isBlinking) noexcept;
void SetInvisible(bool isInvisible) noexcept;
Expand Down
20 changes: 20 additions & 0 deletions src/buffer/out/ut_textbuffer/TextAttributeTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ void TextAttributeTests::TestRoundtripExhaustive()
void TextAttributeTests::TestTextAttributeColorGetters()
{
const COLORREF red = RGB(255, 0, 0);
const COLORREF faintRed = RGB(127, 0, 0);
const COLORREF green = RGB(0, 255, 0);
TextAttribute attr(red, green);
auto view = _GetTableView();
Expand All @@ -149,6 +150,25 @@ void TextAttributeTests::TestTextAttributeColorGetters()
VERIFY_ARE_EQUAL(red, attr.GetForeground().GetColor(view, _defaultFg));
VERIFY_ARE_EQUAL(green, attr.GetBackground().GetColor(view, _defaultBg));
VERIFY_ARE_EQUAL(std::make_pair(green, red), attr.CalculateRgbColors(view, _defaultFg, _defaultBg));

// reset the reverse video
attr.SetReverseVideo(false);

// with faint set, the calculated foreground value should be fainter
// while the background and getters stay the same
attr.SetFaint(true);

VERIFY_ARE_EQUAL(red, attr.GetForeground().GetColor(view, _defaultFg));
VERIFY_ARE_EQUAL(green, attr.GetBackground().GetColor(view, _defaultBg));
VERIFY_ARE_EQUAL(std::make_pair(faintRed, green), attr.CalculateRgbColors(view, _defaultFg, _defaultBg));

// with reverse video set, calculated foreground/background values should be
// switched, and the background fainter, while getters stay the same
attr.SetReverseVideo(true);

VERIFY_ARE_EQUAL(red, attr.GetForeground().GetColor(view, _defaultFg));
VERIFY_ARE_EQUAL(green, attr.GetBackground().GetColor(view, _defaultBg));
VERIFY_ARE_EQUAL(std::make_pair(green, faintRed), attr.CalculateRgbColors(view, _defaultFg, _defaultBg));
}

void TextAttributeTests::TestReverseDefaultColors()
Expand Down
6 changes: 5 additions & 1 deletion src/cascadia/TerminalCore/TerminalDispatchGraphics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,12 @@ bool TerminalDispatch::SetGraphicsRendition(const std::basic_string_view<Dispatc
case BoldBright:
attr.SetBold(true);
break;
case UnBold:
case RGBColorOrFaint:
attr.SetFaint(true);
break;
case NotBoldOrFaint:
attr.SetBold(false);
attr.SetFaint(false);
break;
case Italics:
attr.SetItalics(true);
Expand Down
27 changes: 22 additions & 5 deletions src/host/ut_host/ScreenBufferTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5014,14 +5014,16 @@ void ScreenBufferTests::TestExtendedTextAttributes()
// Run this test for each and every possible combination of states.
BEGIN_TEST_METHOD_PROPERTIES()
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: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, italics, blink, invisible, crossedOut;
bool bold, faint, italics, 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"blink", blink));
VERIFY_SUCCEEDED(TestData::TryGetValue(L"invisible", invisible));
Expand All @@ -5043,6 +5045,11 @@ void ScreenBufferTests::TestExtendedTextAttributes()
WI_SetFlag(expectedAttrs, ExtendedAttributes::Bold);
vtSeq += L"\x1b[1m";
}
if (faint)
{
WI_SetFlag(expectedAttrs, ExtendedAttributes::Faint);
vtSeq += L"\x1b[2m";
}
if (italics)
{
WI_SetFlag(expectedAttrs, ExtendedAttributes::Italics);
Expand Down Expand Up @@ -5100,9 +5107,10 @@ void ScreenBufferTests::TestExtendedTextAttributes()

// One-by-one, turn off each of these states with VT, then check that the
// state matched.
if (bold)
if (bold || faint)
{
WI_ClearFlag(expectedAttrs, ExtendedAttributes::Bold);
// The bold and faint attributes share the same reset sequence.
WI_ClearAllFlags(expectedAttrs, ExtendedAttributes::Bold | ExtendedAttributes::Faint);
vtSeq = L"\x1b[22m";
validate(expectedAttrs, vtSeq);
}
Expand Down Expand Up @@ -5147,6 +5155,7 @@ void ScreenBufferTests::TestExtendedTextAttributesWithColors()
// Run this test for each and every possible combination of states.
BEGIN_TEST_METHOD_PROPERTIES()
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:blink", L"{false, true}")
TEST_METHOD_PROPERTY(L"Data:invisible", L"{false, true}")
Expand All @@ -5162,8 +5171,9 @@ void ScreenBufferTests::TestExtendedTextAttributesWithColors()
const int Use256Color = 2;
const int UseRGBColor = 3;

bool bold, italics, blink, invisible, crossedOut;
bool bold, faint, italics, 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"blink", blink));
VERIFY_SUCCEEDED(TestData::TryGetValue(L"invisible", invisible));
Expand All @@ -5189,6 +5199,11 @@ void ScreenBufferTests::TestExtendedTextAttributesWithColors()
expectedAttr.SetBold(true);
vtSeq += L"\x1b[1m";
}
if (faint)
{
expectedAttr.SetFaint(true);
vtSeq += L"\x1b[2m";
}
if (italics)
{
expectedAttr.SetItalics(true);
Expand Down Expand Up @@ -5290,9 +5305,11 @@ void ScreenBufferTests::TestExtendedTextAttributesWithColors()

// One-by-one, turn off each of these states with VT, then check that the
// state matched.
if (bold)
if (bold || faint)
{
// The bold and faint attributes share the same reset sequence.
expectedAttr.SetBold(false);
expectedAttr.SetFaint(false);
vtSeq = L"\x1b[22m";
validate(expectedAttr, vtSeq);
}
Expand Down
16 changes: 14 additions & 2 deletions src/host/ut_host/VtRendererTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -660,13 +660,15 @@ 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: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 italics, blink, invisible, crossedOut;
bool faint, italics, blink, invisible, crossedOut;
VERIFY_SUCCEEDED(TestData::TryGetValue(L"faint", faint));
VERIFY_SUCCEEDED(TestData::TryGetValue(L"italics", italics));
VERIFY_SUCCEEDED(TestData::TryGetValue(L"blink", blink));
VERIFY_SUCCEEDED(TestData::TryGetValue(L"invisible", invisible));
Expand All @@ -676,6 +678,12 @@ void VtRendererTest::Xterm256TestExtendedAttributes()
std::vector<std::string> onSequences, offSequences;

// Collect up a VT sequence to set the state given the method properties
if (faint)
{
desiredAttrs.SetFaint(true);
onSequences.push_back("\x1b[2m");
offSequences.push_back("\x1b[22m");
}
if (italics)
{
desiredAttrs.SetItalics(true);
Expand Down Expand Up @@ -746,7 +754,7 @@ void VtRendererTest::Xterm256TestExtendedAttributes()
void VtRendererTest::Xterm256TestAttributesAcrossReset()
{
BEGIN_TEST_METHOD_PROPERTIES()
TEST_METHOD_PROPERTY(L"Data:renditionAttribute", L"{1, 3, 4, 5, 7, 8, 9, 53}")
TEST_METHOD_PROPERTY(L"Data:renditionAttribute", L"{1, 2, 3, 4, 5, 7, 8, 9, 53}")
END_TEST_METHOD_PROPERTIES()

int renditionAttribute;
Expand Down Expand Up @@ -774,6 +782,10 @@ void VtRendererTest::Xterm256TestAttributesAcrossReset()
Log::Comment(L"----Set Bold Attribute----");
textAttributes.SetBold(true);
break;
case GraphicsOptions::RGBColorOrFaint:
Log::Comment(L"----Set Faint Attribute----");
textAttributes.SetFaint(true);
break;
case GraphicsOptions::Italics:
Log::Comment(L"----Set Italics Attribute----");
textAttributes.SetItalics(true);
Expand Down
2 changes: 1 addition & 1 deletion src/inc/conattrs.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ enum class ExtendedAttributes : BYTE
// TODO:GH#2916 add support for these to the parser as well.
Underlined = 0x20, // _technically_ different from LVB_UNDERSCORE, see TODO:GH#2915
DoublyUnderlined = 0x40, // Included for completeness, but not currently supported.
Faint = 0x80, // Included for completeness, but not currently supported.
Faint = 0x80,
};
DEFINE_ENUM_FLAG_OPERATORS(ExtendedAttributes);

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 @@ -344,6 +344,17 @@ using namespace Microsoft::Console::Render;
return _Write(isBold ? "\x1b[1m" : "\x1b[22m");
}

// Method Description:
// - Formats and writes a sequence to change the faintness of the following text.
// Arguments:
// - isFaint: If true, we'll make the text faint. Otherwise we'll remove the faintness.
// Return Value:
// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write.
[[nodiscard]] HRESULT VtEngine::_SetFaint(const bool isFaint) noexcept
{
return _Write(isFaint ? "\x1b[2m" : "\x1b[22m");
}

// Method Description:
// - Formats and writes a sequence to change the underline of the following text.
// Arguments:
Expand Down
24 changes: 21 additions & 3 deletions src/renderer/vt/Xterm256Engine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,28 @@ Xterm256Engine::Xterm256Engine(_In_ wil::unique_hfile hPipe,
// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write.
[[nodiscard]] HRESULT Xterm256Engine::_UpdateExtendedAttrs(const TextAttribute& textAttributes) noexcept
{
if (textAttributes.IsBold() != _lastTextAttributes.IsBold())
// Turning off Bold and Faint must be handled at the same time,
// since there is only one sequence that resets both of them.
const auto boldTurnedOff = !textAttributes.IsBold() && _lastTextAttributes.IsBold();
const auto faintTurnedOff = !textAttributes.IsFaint() && _lastTextAttributes.IsFaint();
if (boldTurnedOff || faintTurnedOff)
{
RETURN_IF_FAILED(_SetBold(textAttributes.IsBold()));
_lastTextAttributes.SetBold(textAttributes.IsBold());
RETURN_IF_FAILED(_SetBold(false));
_lastTextAttributes.SetBold(false);
_lastTextAttributes.SetFaint(false);
}

// Once we've handled the cases where they need to be turned off,
// we can then check if either should be turned back on again.
if (textAttributes.IsBold() && !_lastTextAttributes.IsBold())
{
RETURN_IF_FAILED(_SetBold(true));
_lastTextAttributes.SetBold(true);
}
if (textAttributes.IsFaint() && !_lastTextAttributes.IsFaint())
{
RETURN_IF_FAILED(_SetFaint(true));
_lastTextAttributes.SetFaint(true);
}

if (textAttributes.IsUnderlined() != _lastTextAttributes.IsUnderlined())
Expand Down
1 change: 1 addition & 0 deletions src/renderer/vt/vtrenderer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ namespace Microsoft::Console::Render
[[nodiscard]] HRESULT _ResizeWindow(const short sWidth, const short sHeight) noexcept;

[[nodiscard]] HRESULT _SetBold(const bool isBold) noexcept;
[[nodiscard]] HRESULT _SetFaint(const bool isFaint) noexcept;
[[nodiscard]] HRESULT _SetUnderline(const bool isUnderlined) noexcept;
[[nodiscard]] HRESULT _SetOverline(const bool isUnderlined) noexcept;
[[nodiscard]] HRESULT _SetItalics(const bool isItalic) noexcept;
Expand Down
2 changes: 1 addition & 1 deletion src/terminal/adapter/DispatchTypes.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace Microsoft::Console::VirtualTerminal::DispatchTypes
Invisible = 8,
CrossedOut = 9,
DoublyUnderlined = 21,
UnBold = 22,
NotBoldOrFaint = 22,
NotItalics = 23,
NoUnderline = 24,
Steady = 25, // _not_ blink
Expand Down
6 changes: 5 additions & 1 deletion src/terminal/adapter/adaptDispatchGraphics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,12 @@ bool AdaptDispatch::SetGraphicsRendition(const std::basic_string_view<DispatchTy
case BoldBright:
attr.SetBold(true);
break;
case UnBold:
case RGBColorOrFaint:
attr.SetFaint(true);
break;
case NotBoldOrFaint:
attr.SetBold(false);
attr.SetFaint(false);
break;
case Italics:
attr.SetItalics(true);
Expand Down
15 changes: 14 additions & 1 deletion src/terminal/adapter/ut_adapter/adapterTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1261,7 +1261,7 @@ class AdapterTest
TEST_METHOD(GraphicsSingleTests)
{
BEGIN_TEST_METHOD_PROPERTIES()
TEST_METHOD_PROPERTY(L"Data:uiGraphicsOptions", L"{0, 1, 4, 7, 24, 27, 30, 31, 32, 33, 34, 35, 36, 37, 39, 40, 41, 42, 43, 44, 45, 46, 47, 49, 53, 55, 90, 91, 92, 93, 94, 95, 96, 97, 100, 101, 102, 103, 104, 105, 106, 107}") // corresponds to options in DispatchTypes::GraphicsOptions
TEST_METHOD_PROPERTY(L"Data:uiGraphicsOptions", L"{0, 1, 2, 4, 7, 22, 24, 27, 30, 31, 32, 33, 34, 35, 36, 37, 39, 40, 41, 42, 43, 44, 45, 46, 47, 49, 53, 55, 90, 91, 92, 93, 94, 95, 96, 97, 100, 101, 102, 103, 104, 105, 106, 107}") // corresponds to options in DispatchTypes::GraphicsOptions
END_TEST_METHOD_PROPERTIES()

Log::Comment(L"Starting test...");
Expand Down Expand Up @@ -1290,6 +1290,12 @@ class AdapterTest
_testGetSet->_expectedAttribute = TextAttribute{ 0 };
_testGetSet->_expectedAttribute.SetBold(true);
break;
case DispatchTypes::GraphicsOptions::RGBColorOrFaint:
Log::Comment(L"Testing graphics 'Faint'");
_testGetSet->_attribute = TextAttribute{ 0 };
_testGetSet->_expectedAttribute = TextAttribute{ 0 };
_testGetSet->_expectedAttribute.SetFaint(true);
break;
case DispatchTypes::GraphicsOptions::Underline:
Log::Comment(L"Testing graphics 'Underline'");
_testGetSet->_attribute = TextAttribute{ 0 };
Expand All @@ -1305,6 +1311,13 @@ class AdapterTest
_testGetSet->_attribute = TextAttribute{ 0 };
_testGetSet->_expectedAttribute = TextAttribute{ COMMON_LVB_REVERSE_VIDEO };
break;
case DispatchTypes::GraphicsOptions::NotBoldOrFaint:
Log::Comment(L"Testing graphics 'No Bold or Faint'");
_testGetSet->_attribute = TextAttribute{ 0 };
_testGetSet->_attribute.SetBold(true);
_testGetSet->_attribute.SetFaint(true);
_testGetSet->_expectedAttribute = TextAttribute{ 0 };
break;
case DispatchTypes::GraphicsOptions::NoUnderline:
Log::Comment(L"Testing graphics 'No Underline'");
_testGetSet->_attribute = TextAttribute{ COMMON_LVB_UNDERSCORE };
Expand Down

0 comments on commit 7d677c5

Please sign in to comment.