From b2973eb573860f2f7ce46e77dc26edfe08cd40ec Mon Sep 17 00:00:00 2001 From: James Holderness Date: Tue, 14 Jul 2020 15:11:03 +0100 Subject: [PATCH] Add support for the "concealed" graphic rendition attribute (#6907) ## Summary of the Pull Request This PR adds support for the `SGR 8` and `SGR 28` escape sequences, which enable and disable the _concealed/invisible_ graphic rendition attribute. When a character is output with this attribute set, it is rendered with the same foreground and background colors, so the text is essentially invisible. ## PR Checklist * [x] Closes #6876 * [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: #6876 ## Detailed Description of the Pull Request / Additional comments Most of the framework for this attribute was already implemented, so it was just a matter of updating the `TextAttribute::CalculateRgbColors` method to make the foreground the same as the background when the _Invisible_ flag was set. Note that this has to happen after the _Reverse Video_ attribute is applied, so if you have white-on-black text that is reversed and invisible, it should be all white, rather than all black. ## Validation Steps Performed There were already existing SGR unit tests covering this attribute in the `ScreenBufferTests`, and the `VtRendererTest`. But I've added to the `AdapterTest` which verifies the SGR sequences for setting and resetting the attribute, and I've extended the `TextAttributeTests` to verify that the color calculations return the correct values when the attribute is set. I've also manually confirmed that we now render the _concealed text_ values correctly in the _ISO 6429_ tests in Vttest. And I've manually tested the output of _concealed_ when combined with other attributes, and made sure that we're matching the behaviour of most other terminals. --- src/buffer/out/TextAttribute.cpp | 4 ++++ .../out/ut_textbuffer/TextAttributeTests.cpp | 20 +++++++++++++++++++ .../adapter/ut_adapter/adapterTest.cpp | 14 ++++++++++++- 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/buffer/out/TextAttribute.cpp b/src/buffer/out/TextAttribute.cpp index 73d774308b..dd4ef45031 100644 --- a/src/buffer/out/TextAttribute.cpp +++ b/src/buffer/out/TextAttribute.cpp @@ -105,6 +105,10 @@ std::pair TextAttribute::CalculateRgbColors(const std::basic { std::swap(fg, bg); } + if (IsInvisible()) + { + fg = bg; + } return { fg, bg }; } diff --git a/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp b/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp index ae9535f675..13472d4437 100644 --- a/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp +++ b/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp @@ -169,6 +169,26 @@ 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, faintRed), attr.CalculateRgbColors(view, _defaultFg, _defaultBg)); + + // reset the reverse video and faint attributes + attr.SetReverseVideo(false); + attr.SetFaint(false); + + // with invisible set, the calculated foreground value should match the + // background, while getters stay the same + attr.SetInvisible(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, green), attr.CalculateRgbColors(view, _defaultFg, _defaultBg)); + + // with reverse video set, the calculated background value should match + // the foreground, 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(red, red), attr.CalculateRgbColors(view, _defaultFg, _defaultBg)); } void TextAttributeTests::TestReverseDefaultColors() diff --git a/src/terminal/adapter/ut_adapter/adapterTest.cpp b/src/terminal/adapter/ut_adapter/adapterTest.cpp index 944baa8008..c65947503a 100644 --- a/src/terminal/adapter/ut_adapter/adapterTest.cpp +++ b/src/terminal/adapter/ut_adapter/adapterTest.cpp @@ -1261,7 +1261,7 @@ public: TEST_METHOD(GraphicsSingleTests) { BEGIN_TEST_METHOD_PROPERTIES() - 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 + TEST_METHOD_PROPERTY(L"Data:uiGraphicsOptions", L"{0, 1, 2, 4, 7, 8, 22, 24, 27, 28, 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..."); @@ -1311,6 +1311,12 @@ public: _testGetSet->_attribute = TextAttribute{ 0 }; _testGetSet->_expectedAttribute = TextAttribute{ COMMON_LVB_REVERSE_VIDEO }; break; + case DispatchTypes::GraphicsOptions::Invisible: + Log::Comment(L"Testing graphics 'Invisible'"); + _testGetSet->_attribute = TextAttribute{ 0 }; + _testGetSet->_expectedAttribute = TextAttribute{ 0 }; + _testGetSet->_expectedAttribute.SetInvisible(true); + break; case DispatchTypes::GraphicsOptions::NotBoldOrFaint: Log::Comment(L"Testing graphics 'No Bold or Faint'"); _testGetSet->_attribute = TextAttribute{ 0 }; @@ -1333,6 +1339,12 @@ public: _testGetSet->_attribute = TextAttribute{ COMMON_LVB_REVERSE_VIDEO }; _testGetSet->_expectedAttribute = TextAttribute{ 0 }; break; + case DispatchTypes::GraphicsOptions::Visible: + Log::Comment(L"Testing graphics 'Visible'"); + _testGetSet->_attribute = TextAttribute{ 0 }; + _testGetSet->_attribute.SetInvisible(true); + _testGetSet->_expectedAttribute = TextAttribute{ 0 }; + break; case DispatchTypes::GraphicsOptions::ForegroundBlack: Log::Comment(L"Testing graphics 'Foreground Color Black'"); _testGetSet->_attribute = TextAttribute{ FOREGROUND_RED | FOREGROUND_GREEN | FOREGROUND_BLUE | FOREGROUND_INTENSITY };