From dbf25c4849ff7aeebb5dfc03b26a72ec59877ebf Mon Sep 17 00:00:00 2001 From: James Holderness Date: Tue, 14 Apr 2020 12:49:56 +0100 Subject: [PATCH 01/10] Get rid of unused legacy methods in ATTR_ROW and OutputCell. --- src/buffer/out/AttrRow.cpp | 20 -------------------- src/buffer/out/AttrRow.hpp | 1 - src/buffer/out/OutputCell.cpp | 26 -------------------------- src/buffer/out/OutputCell.hpp | 2 -- 4 files changed, 49 deletions(-) diff --git a/src/buffer/out/AttrRow.cpp b/src/buffer/out/AttrRow.cpp index 89cd1e1d441..b12a2860a9b 100644 --- a/src/buffer/out/AttrRow.cpp +++ b/src/buffer/out/AttrRow.cpp @@ -190,26 +190,6 @@ bool ATTR_ROW::SetAttrToEnd(const UINT iStart, const TextAttribute attr) return SUCCEEDED(InsertAttrRuns({ &run, 1 }, iStart, _cchRowWidth - 1, _cchRowWidth)); } -// Routine Description: -// - Replaces all runs in the row with the given wToBeReplacedAttr with the new -// attribute wReplaceWith. This method is used for replacing specifically -// legacy attributes. -// Arguments: -// - wToBeReplacedAttr - the legacy attribute to replace in this row. -// - wReplaceWith - the new value for the matching runs' attributes. -// Return Value: -// -void ATTR_ROW::ReplaceLegacyAttrs(_In_ WORD wToBeReplacedAttr, _In_ WORD wReplaceWith) noexcept -{ - TextAttribute ToBeReplaced; - ToBeReplaced.SetFromLegacy(wToBeReplacedAttr); - - TextAttribute ReplaceWith; - ReplaceWith.SetFromLegacy(wReplaceWith); - - ReplaceAttrs(ToBeReplaced, ReplaceWith); -} - // Method Description: // - Replaces all runs in the row with the given toBeReplacedAttr with the new // attribute replaceWith. diff --git a/src/buffer/out/AttrRow.hpp b/src/buffer/out/AttrRow.hpp index 2aba2a772de..29f0ee1e470 100644 --- a/src/buffer/out/AttrRow.hpp +++ b/src/buffer/out/AttrRow.hpp @@ -42,7 +42,6 @@ class ATTR_ROW final size_t* const pApplies) const; bool SetAttrToEnd(const UINT iStart, const TextAttribute attr); - void ReplaceLegacyAttrs(const WORD wToBeReplacedAttr, const WORD wReplaceWith) noexcept; void ReplaceAttrs(const TextAttribute& toBeReplacedAttr, const TextAttribute& replaceWith) noexcept; void Resize(const size_t newWidth); diff --git a/src/buffer/out/OutputCell.cpp b/src/buffer/out/OutputCell.cpp index ad30b996bdb..fef9dcb0288 100644 --- a/src/buffer/out/OutputCell.cpp +++ b/src/buffer/out/OutputCell.cpp @@ -44,15 +44,6 @@ OutputCell::OutputCell(const std::wstring_view charData, _setFromStringView(charData); } -OutputCell::OutputCell(const CHAR_INFO& charInfo) : - _text{ UNICODE_INVALID }, - _dbcsAttribute{}, - _textAttribute{ InvalidTextAttribute }, - _behavior{ TextAttributeBehavior::Stored } -{ - _setFromCharInfo(charInfo); -} - OutputCell::OutputCell(const OutputCellView& cell) { _setFromOutputCellView(cell); @@ -84,23 +75,6 @@ void OutputCell::_setFromBehavior(const TextAttributeBehavior behavior) THROW_HR_IF(E_INVALIDARG, behavior == TextAttributeBehavior::Stored); } -void OutputCell::_setFromCharInfo(const CHAR_INFO& charInfo) -{ - _text = charInfo.Char.UnicodeChar; - - if (WI_IsFlagSet(charInfo.Attributes, COMMON_LVB_LEADING_BYTE)) - { - _dbcsAttribute.SetLeading(); - } - else if (WI_IsFlagSet(charInfo.Attributes, COMMON_LVB_TRAILING_BYTE)) - { - _dbcsAttribute.SetTrailing(); - } - _textAttribute.SetFromLegacy(charInfo.Attributes); - - _behavior = TextAttributeBehavior::Stored; -} - void OutputCell::_setFromStringView(const std::wstring_view view) { _text = view; diff --git a/src/buffer/out/OutputCell.hpp b/src/buffer/out/OutputCell.hpp index df64e0b461e..da9482e5d93 100644 --- a/src/buffer/out/OutputCell.hpp +++ b/src/buffer/out/OutputCell.hpp @@ -44,7 +44,6 @@ class OutputCell final const DbcsAttribute dbcsAttribute, const TextAttribute textAttribute); - OutputCell(const CHAR_INFO& charInfo); OutputCell(const OutputCellView& view); OutputCell(const OutputCell&) = default; @@ -86,7 +85,6 @@ class OutputCell final TextAttributeBehavior _behavior; void _setFromBehavior(const TextAttributeBehavior behavior); - void _setFromCharInfo(const CHAR_INFO& charInfo); void _setFromStringView(const std::wstring_view view); void _setFromOutputCellView(const OutputCellView& cell); }; From 562bdece15bdf7561d702acd05614292a21a32b5 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Tue, 14 Apr 2020 13:15:49 +0100 Subject: [PATCH 02/10] Replace unnecessary usage of TextAttributeRun::SetAttributesFromLegacy with SetAttributes. --- src/buffer/out/TextAttributeRun.cpp | 11 ----------- src/buffer/out/TextAttributeRun.hpp | 1 - src/host/ut_host/AttrRowTests.cpp | 12 ++++++------ 3 files changed, 6 insertions(+), 18 deletions(-) diff --git a/src/buffer/out/TextAttributeRun.cpp b/src/buffer/out/TextAttributeRun.cpp index a2045eaaea1..97dec13f4df 100644 --- a/src/buffer/out/TextAttributeRun.cpp +++ b/src/buffer/out/TextAttributeRun.cpp @@ -45,14 +45,3 @@ void TextAttributeRun::SetAttributes(const TextAttribute textAttribute) noexcept { _attributes = textAttribute; } - -// Routine Description: -// - Sets the attributes of this run to the given legacy attributes -// Arguments: -// - wNew - the new value for this run's attributes -// Return Value: -// -void TextAttributeRun::SetAttributesFromLegacy(const WORD wNew) noexcept -{ - _attributes.SetFromLegacy(wNew); -} diff --git a/src/buffer/out/TextAttributeRun.hpp b/src/buffer/out/TextAttributeRun.hpp index ff0c9179949..426cfddf189 100644 --- a/src/buffer/out/TextAttributeRun.hpp +++ b/src/buffer/out/TextAttributeRun.hpp @@ -35,7 +35,6 @@ class TextAttributeRun final const TextAttribute& GetAttributes() const noexcept; void SetAttributes(const TextAttribute textAttribute) noexcept; - void SetAttributesFromLegacy(const WORD wNew) noexcept; private: size_t _cchLength; diff --git a/src/host/ut_host/AttrRowTests.cpp b/src/host/ut_host/AttrRowTests.cpp index 15293aa5a6e..55ebc8ce5ba 100644 --- a/src/host/ut_host/AttrRowTests.cpp +++ b/src/host/ut_host/AttrRowTests.cpp @@ -110,7 +110,7 @@ class AttrRowTests { TextAttributeRun* pRun = &pChain->_list[iChain]; - pRun->SetAttributesFromLegacy(iChain); // Just use the chain position as the value + pRun->SetAttributes(iChain); // Just use the chain position as the value pRun->SetLength(sChainSegLength); } @@ -269,11 +269,11 @@ class AttrRowTests ATTR_ROW originalRow{ static_cast(_sDefaultLength), _DefaultAttr }; originalRow._list.resize(3); originalRow._cchRowWidth = 10; - originalRow._list[0].SetAttributesFromLegacy('R'); + originalRow._list[0].SetAttributes('R'); originalRow._list[0].SetLength(3); - originalRow._list[1].SetAttributesFromLegacy('B'); + originalRow._list[1].SetAttributes('B'); originalRow._list[1].SetLength(5); - originalRow._list[2].SetAttributesFromLegacy('G'); + originalRow._list[2].SetAttributes('G'); originalRow._list[2].SetLength(2); LogChain(L"Original: ", originalRow._list); @@ -286,11 +286,11 @@ class AttrRowTests std::vector insertRow; insertRow.resize(cInsertRow); - insertRow[0].SetAttributesFromLegacy(ch1); + insertRow[0].SetAttributes(ch1); insertRow[0].SetLength(uiChar1Length); if (fUseStr2) { - insertRow[1].SetAttributesFromLegacy(ch2); + insertRow[1].SetAttributes(ch2); insertRow[1].SetLength(uiChar2Length); } From aeb1c7f64a4bc062f3a366a1f89288933fc74520 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Tue, 14 Apr 2020 13:24:40 +0100 Subject: [PATCH 03/10] Replace unnecessary usage of TextAttribute::SetFromLegacy with TextAttribute constructor. --- src/buffer/out/OutputCellIterator.cpp | 3 +-- src/buffer/out/TextAttribute.cpp | 10 ---------- src/buffer/out/TextAttribute.hpp | 2 -- src/host/ut_host/OutputCellIteratorTests.cpp | 18 ++++++------------ src/host/ut_host/TextBufferTests.cpp | 3 +-- 5 files changed, 8 insertions(+), 28 deletions(-) diff --git a/src/buffer/out/OutputCellIterator.cpp b/src/buffer/out/OutputCellIterator.cpp index 57eac270251..12ac7ad5081 100644 --- a/src/buffer/out/OutputCellIterator.cpp +++ b/src/buffer/out/OutputCellIterator.cpp @@ -518,8 +518,7 @@ OutputCellView OutputCellIterator::s_GenerateView(const CHAR_INFO& charInfo) noe dbcsAttr.SetTrailing(); } - TextAttribute textAttr; - textAttr.SetFromLegacy(charInfo.Attributes); + const TextAttribute textAttr(charInfo.Attributes); const auto behavior = TextAttributeBehavior::Stored; return OutputCellView(glyph, dbcsAttr, textAttr, behavior); diff --git a/src/buffer/out/TextAttribute.cpp b/src/buffer/out/TextAttribute.cpp index f8f3f75e499..dd0c4ea28c5 100644 --- a/src/buffer/out/TextAttribute.cpp +++ b/src/buffer/out/TextAttribute.cpp @@ -85,16 +85,6 @@ void TextAttribute::SetBackground(const COLORREF rgbBackground) noexcept _background = TextColor(rgbBackground); } -void TextAttribute::SetFromLegacy(const WORD wLegacy) noexcept -{ - _wAttrLegacy = gsl::narrow_cast(wLegacy & META_ATTRS); - WI_ClearAllFlags(_wAttrLegacy, COMMON_LVB_SBCSDBCS); - const BYTE fgIndex = gsl::narrow_cast(wLegacy & FG_ATTRS); - const BYTE bgIndex = gsl::narrow_cast(wLegacy & BG_ATTRS) >> 4; - _foreground = TextColor(fgIndex); - _background = TextColor(bgIndex); -} - void TextAttribute::SetLegacyAttributes(const WORD attrs, const bool setForeground, const bool setBackground, diff --git a/src/buffer/out/TextAttribute.hpp b/src/buffer/out/TextAttribute.hpp index 76fdd49e17d..771b9d56b16 100644 --- a/src/buffer/out/TextAttribute.hpp +++ b/src/buffer/out/TextAttribute.hpp @@ -107,8 +107,6 @@ class TextAttribute final void SetLeftVerticalDisplayed(const bool isDisplayed) noexcept; void SetRightVerticalDisplayed(const bool isDisplayed) noexcept; - void SetFromLegacy(const WORD wLegacy) noexcept; - void SetLegacyAttributes(const WORD attrs, const bool setForeground, const bool setBackground, diff --git a/src/host/ut_host/OutputCellIteratorTests.cpp b/src/host/ut_host/OutputCellIteratorTests.cpp index c379f06ce71..2596c5bbb53 100644 --- a/src/host/ut_host/OutputCellIteratorTests.cpp +++ b/src/host/ut_host/OutputCellIteratorTests.cpp @@ -102,8 +102,7 @@ class OutputCellIteratorTests { SetVerifyOutput settings(VerifyOutputSettings::LogOnlyFailures); - TextAttribute attr; - attr.SetFromLegacy(FOREGROUND_RED | BACKGROUND_BLUE); + const TextAttribute attr(FOREGROUND_RED | BACKGROUND_BLUE); const size_t limit = 5; @@ -128,8 +127,7 @@ class OutputCellIteratorTests { SetVerifyOutput settings(VerifyOutputSettings::LogOnlyFailures); - TextAttribute attr; - attr.SetFromLegacy(FOREGROUND_RED | BACKGROUND_BLUE); + const TextAttribute attr(FOREGROUND_RED | BACKGROUND_BLUE); OutputCellIterator it(attr); @@ -154,8 +152,7 @@ class OutputCellIteratorTests const wchar_t wch = L'Q'; - TextAttribute attr; - attr.SetFromLegacy(FOREGROUND_RED | BACKGROUND_BLUE); + const TextAttribute attr(FOREGROUND_RED | BACKGROUND_BLUE); const size_t limit = 5; @@ -182,8 +179,7 @@ class OutputCellIteratorTests const wchar_t wch = L'Q'; - TextAttribute attr; - attr.SetFromLegacy(FOREGROUND_RED | BACKGROUND_BLUE); + const TextAttribute attr(FOREGROUND_RED | BACKGROUND_BLUE); OutputCellIterator it(wch, attr); @@ -314,8 +310,7 @@ class OutputCellIteratorTests SetVerifyOutput settings(VerifyOutputSettings::LogOnlyFailures); const std::wstring testText(L"The quick brown fox jumps over the lazy dog."); - TextAttribute color; - color.SetFromLegacy(FOREGROUND_GREEN | FOREGROUND_INTENSITY); + const TextAttribute color(FOREGROUND_GREEN | FOREGROUND_INTENSITY); OutputCellIterator it(testText, color); @@ -339,8 +334,7 @@ class OutputCellIteratorTests SetVerifyOutput settings(VerifyOutputSettings::LogOnlyFailures); const std::wstring testText(L"\x30a2\x30a3\x30a4\x30a5\x30a6"); - TextAttribute color; - color.SetFromLegacy(FOREGROUND_GREEN | FOREGROUND_INTENSITY); + const TextAttribute color(FOREGROUND_GREEN | FOREGROUND_INTENSITY); OutputCellIterator it(testText, color); diff --git a/src/host/ut_host/TextBufferTests.cpp b/src/host/ut_host/TextBufferTests.cpp index 563b73e71c3..ac4cbfab1c7 100644 --- a/src/host/ut_host/TextBufferTests.cpp +++ b/src/host/ut_host/TextBufferTests.cpp @@ -1758,8 +1758,7 @@ void TextBufferTests::ResizeTraditional() VERIFY_SUCCEEDED(TestData::TryGetValue(L"shrinkY", shrinkY), L"Shrink Y = true, Grow Y = false"); const COORD smallSize = { 5, 5 }; - TextAttribute defaultAttr; - defaultAttr.SetFromLegacy(0); + const TextAttribute defaultAttr(0); TextBuffer buffer(smallSize, defaultAttr, 12, _renderTarget); From 0f0858135363ca597b0c4f288d5fab5245afda8d Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sat, 18 Apr 2020 21:38:27 +0100 Subject: [PATCH 04/10] Add methods to the TextAttribute class to update attributes with less dependence on internal details. --- src/buffer/out/TextAttribute.cpp | 128 +++++++++++------- src/buffer/out/TextAttribute.hpp | 48 +++---- .../out/ut_textbuffer/TextAttributeTests.cpp | 14 +- src/cascadia/TerminalCore/TerminalApi.cpp | 25 +--- .../TerminalCore/terminalrenderdata.cpp | 2 +- .../ConptyRoundtripTests.cpp | 8 +- src/host/getset.cpp | 9 +- src/host/ut_host/ScreenBufferTests.cpp | 73 +++++----- 8 files changed, 157 insertions(+), 150 deletions(-) diff --git a/src/buffer/out/TextAttribute.cpp b/src/buffer/out/TextAttribute.cpp index dd0c4ea28c5..76a907713e8 100644 --- a/src/buffer/out/TextAttribute.cpp +++ b/src/buffer/out/TextAttribute.cpp @@ -18,7 +18,7 @@ COLORREF TextAttribute::CalculateRgbForeground(std::basic_string_view COLORREF defaultFgColor, COLORREF defaultBgColor) const noexcept { - return _IsReverseVideo() ? _GetRgbBackground(colorTable, defaultBgColor) : _GetRgbForeground(colorTable, defaultFgColor); + return IsReverseVideo() ? _GetRgbBackground(colorTable, defaultBgColor) : _GetRgbForeground(colorTable, defaultFgColor); } // Routine Description: @@ -31,7 +31,7 @@ COLORREF TextAttribute::CalculateRgbBackground(std::basic_string_view COLORREF defaultFgColor, COLORREF defaultBgColor) const noexcept { - return _IsReverseVideo() ? _GetRgbForeground(colorTable, defaultFgColor) : _GetRgbBackground(colorTable, defaultBgColor); + return IsReverseVideo() ? _GetRgbForeground(colorTable, defaultFgColor) : _GetRgbBackground(colorTable, defaultBgColor); } // Routine Description: @@ -66,15 +66,6 @@ void TextAttribute::SetMetaAttributes(const WORD wMeta) noexcept WI_ClearAllFlags(_wAttrLegacy, COMMON_LVB_SBCSDBCS); } -WORD TextAttribute::GetMetaAttributes() const noexcept -{ - WORD wMeta = _wAttrLegacy; - WI_ClearAllFlags(wMeta, FG_ATTRS); - WI_ClearAllFlags(wMeta, BG_ATTRS); - WI_ClearAllFlags(wMeta, COMMON_LVB_SBCSDBCS); - return wMeta; -} - void TextAttribute::SetForeground(const COLORREF rgbForeground) noexcept { _foreground = TextColor(rgbForeground); @@ -106,31 +97,14 @@ void TextAttribute::SetLegacyAttributes(const WORD attrs, } } -// Method Description: -// - Sets the foreground and/or background to a particular index in the 256color -// table. If either parameter is nullptr, it's ignored. -// This method can be used to set the colors to indexes in the range [0, 255], -// as opposed to SetLegacyAttributes, which clamps them to [0,15] -// Arguments: -// - foreground: nullptr if we should ignore this attr, else a pointer to a byte -// value to use as an index into the 256-color table. -// - background: nullptr if we should ignore this attr, else a pointer to a byte -// value to use as an index into the 256-color table. -// Return Value: -// - -void TextAttribute::SetIndexedAttributes(const std::optional foreground, - const std::optional background) noexcept +void TextAttribute::SetIndexedForeground(const BYTE fgIndex) noexcept { - if (foreground) - { - const BYTE fgIndex = (*foreground) & 0xFF; - _foreground = TextColor(fgIndex); - } - if (background) - { - const BYTE bgIndex = (*background) & 0xFF; - _background = TextColor(bgIndex); - } + _foreground = TextColor(fgIndex); +} + +void TextAttribute::SetIndexedBackground(const BYTE bgIndex) noexcept +{ + _background = TextColor(bgIndex); } void TextAttribute::SetColor(const COLORREF rgbColor, const bool fIsForeground) noexcept @@ -185,14 +159,81 @@ void TextAttribute::SetRightVerticalDisplayed(const bool isDisplayed) noexcept WI_UpdateFlag(_wAttrLegacy, COMMON_LVB_GRID_RVERTICAL, isDisplayed); } -void TextAttribute::Embolden() noexcept +bool TextAttribute::IsBold() const noexcept { - _SetBoldness(true); + return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::Bold); } -void TextAttribute::Debolden() noexcept +bool TextAttribute::IsItalic() const noexcept { - _SetBoldness(false); + return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::Italics); +} + +bool TextAttribute::IsBlinking() const noexcept +{ + return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::Blinking); +} + +bool TextAttribute::IsInvisible() const noexcept +{ + return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::Invisible); +} + +bool TextAttribute::IsCrossedOut() const noexcept +{ + return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::CrossedOut); +} + +bool TextAttribute::IsUnderlined() const noexcept +{ + // TODO:GH#2915 Treat underline separately from LVB_UNDERSCORE + return WI_IsFlagSet(_wAttrLegacy, COMMON_LVB_UNDERSCORE); +} + +bool TextAttribute::IsReverseVideo() const noexcept +{ + return WI_IsFlagSet(_wAttrLegacy, COMMON_LVB_REVERSE_VIDEO); +} + +void TextAttribute::SetBold(bool isBold) noexcept +{ + WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Bold, isBold); +} + +void TextAttribute::SetItalics(bool isItalic) noexcept +{ + WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Italics, isItalic); +} + +void TextAttribute::SetBlinking(bool isBlinking) noexcept +{ + WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Blinking, isBlinking); +} + +void TextAttribute::SetInvisible(bool isInvisible) noexcept +{ + WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Invisible, isInvisible); +} + +void TextAttribute::SetCrossedOut(bool isCrossedOut) noexcept +{ + WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::CrossedOut, isCrossedOut); +} + +void TextAttribute::SetUnderline(bool isUnderlined) noexcept +{ + // TODO:GH#2915 Treat underline separately from LVB_UNDERSCORE + WI_UpdateFlag(_wAttrLegacy, COMMON_LVB_UNDERSCORE, isUnderlined); +} + +void TextAttribute::SetReverseVideo(bool isReversed) noexcept +{ + WI_UpdateFlag(_wAttrLegacy, COMMON_LVB_REVERSE_VIDEO, isReversed); +} + +ExtendedAttributes TextAttribute::GetExtendedAttributes() const noexcept +{ + return _extendedAttrs; } void TextAttribute::SetExtendedAttributes(const ExtendedAttributes attrs) noexcept @@ -207,11 +248,6 @@ void TextAttribute::Invert() noexcept WI_ToggleFlag(_wAttrLegacy, COMMON_LVB_REVERSE_VIDEO); } -void TextAttribute::_SetBoldness(const bool isBold) noexcept -{ - WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Bold, isBold); -} - void TextAttribute::SetDefaultForeground() noexcept { _foreground = TextColor(); @@ -257,6 +293,6 @@ bool TextAttribute::BackgroundIsDefault() const noexcept // requires for most erasing and filling operations. void TextAttribute::SetStandardErase() noexcept { - SetExtendedAttributes(ExtendedAttributes::Normal); - SetMetaAttributes(0); + _extendedAttrs = ExtendedAttributes::Normal; + _wAttrLegacy = 0; } diff --git a/src/buffer/out/TextAttribute.hpp b/src/buffer/out/TextAttribute.hpp index 771b9d56b16..30c7a2b2738 100644 --- a/src/buffer/out/TextAttribute.hpp +++ b/src/buffer/out/TextAttribute.hpp @@ -112,14 +112,7 @@ class TextAttribute final const bool setBackground, const bool setMeta) noexcept; - void SetIndexedAttributes(const std::optional foreground, - const std::optional background) noexcept; - void SetMetaAttributes(const WORD wMeta) noexcept; - WORD GetMetaAttributes() const noexcept; - - void Embolden() noexcept; - void Debolden() noexcept; void Invert() noexcept; @@ -131,21 +124,29 @@ class TextAttribute final friend constexpr bool operator!=(const WORD& legacyAttr, const TextAttribute& attr) noexcept; bool IsLegacy() const noexcept; - - constexpr bool IsBold() const noexcept - { - return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::Bold); - } - - constexpr ExtendedAttributes GetExtendedAttributes() const noexcept - { - return _extendedAttrs; - } - + bool IsBold() const noexcept; + bool IsItalic() const noexcept; + bool IsBlinking() const noexcept; + bool IsInvisible() const noexcept; + bool IsCrossedOut() const noexcept; + bool IsUnderlined() const noexcept; + bool IsReverseVideo() const noexcept; + + void SetBold(bool isBold) noexcept; + void SetItalics(bool isItalic) noexcept; + void SetBlinking(bool isBlinking) noexcept; + void SetInvisible(bool isInvisible) noexcept; + void SetCrossedOut(bool isCrossedOut) noexcept; + void SetUnderline(bool isUnderlined) noexcept; + void SetReverseVideo(bool isReversed) noexcept; + + ExtendedAttributes GetExtendedAttributes() const noexcept; void SetExtendedAttributes(const ExtendedAttributes attrs) noexcept; void SetForeground(const COLORREF rgbForeground) noexcept; void SetBackground(const COLORREF rgbBackground) noexcept; + void SetIndexedForeground(const BYTE fgIndex) noexcept; + void SetIndexedBackground(const BYTE bgIndex) noexcept; void SetColor(const COLORREF rgbColor, const bool fIsForeground) noexcept; void SetDefaultForeground() noexcept; @@ -163,13 +164,13 @@ class TextAttribute final // This returns whether this attribute, if printed directly next to another attribute, for the space // character, would look identical to the other one. - constexpr bool HasIdenticalVisualRepresentationForBlankSpace(const TextAttribute& other, const bool inverted = false) const noexcept + bool HasIdenticalVisualRepresentationForBlankSpace(const TextAttribute& other, const bool inverted = false) const noexcept { // sneaky-sneaky: I'm using xor here // inverted is whether there's a global invert; Reverse is a local one. // global ^ local == true : the background attribute is actually the visible foreground, so we care about the foregrounds being identical // global ^ local == false: the foreground attribute is the visible foreground, so we care about the backgrounds being identical - const auto checkForeground = (inverted != _IsReverseVideo()); + const auto checkForeground = (inverted != IsReverseVideo()); return !IsAnyGridLineEnabled() && // grid lines have a visual representation // crossed out, doubly and singly underlined have a visual representation WI_AreAllFlagsClear(_extendedAttrs, ExtendedAttributes::CrossedOut | ExtendedAttributes::DoublyUnderlined | ExtendedAttributes::Underlined) && @@ -191,13 +192,6 @@ class TextAttribute final COLORREF _GetRgbBackground(std::basic_string_view colorTable, COLORREF defaultColor) const noexcept; - constexpr bool _IsReverseVideo() const noexcept - { - return WI_IsFlagSet(_wAttrLegacy, COMMON_LVB_REVERSE_VIDEO); - } - - void _SetBoldness(const bool isBold) noexcept; - WORD _wAttrLegacy; TextColor _foreground; TextColor _background; diff --git a/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp b/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp index b825686061c..b4cc587dd80 100644 --- a/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp +++ b/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp @@ -138,7 +138,7 @@ void TextAttributeTests::TestTextAttributeColorGetters() // verify that calculated foreground/background are the same as the direct // values when reverse video is not set - VERIFY_IS_FALSE(attr._IsReverseVideo()); + VERIFY_IS_FALSE(attr.IsReverseVideo()); VERIFY_ARE_EQUAL(red, attr._GetRgbForeground(view, _defaultFg)); VERIFY_ARE_EQUAL(red, attr.CalculateRgbForeground(view, _defaultFg, _defaultBg)); @@ -148,7 +148,7 @@ void TextAttributeTests::TestTextAttributeColorGetters() // with reverse video set, calculated foreground/background values should be // switched while getters stay the same - attr.SetMetaAttributes(COMMON_LVB_REVERSE_VIDEO); + attr.SetReverseVideo(true); VERIFY_ARE_EQUAL(red, attr._GetRgbForeground(view, _defaultFg)); VERIFY_ARE_EQUAL(green, attr.CalculateRgbForeground(view, _defaultFg, _defaultBg)); @@ -166,7 +166,7 @@ void TextAttributeTests::TestReverseDefaultColors() // verify that calculated foreground/background are the same as the direct // values when reverse video is not set - VERIFY_IS_FALSE(attr._IsReverseVideo()); + VERIFY_IS_FALSE(attr.IsReverseVideo()); VERIFY_ARE_EQUAL(_defaultFg, attr._GetRgbForeground(view, _defaultFg)); VERIFY_ARE_EQUAL(_defaultFg, attr.CalculateRgbForeground(view, _defaultFg, _defaultBg)); @@ -176,8 +176,8 @@ void TextAttributeTests::TestReverseDefaultColors() // with reverse video set, calculated foreground/background values should be // switched while getters stay the same - attr.SetMetaAttributes(COMMON_LVB_REVERSE_VIDEO); - VERIFY_IS_TRUE(attr._IsReverseVideo()); + attr.SetReverseVideo(true); + VERIFY_IS_TRUE(attr.IsReverseVideo()); VERIFY_ARE_EQUAL(_defaultFg, attr._GetRgbForeground(view, _defaultFg)); VERIFY_ARE_EQUAL(_defaultBg, attr.CalculateRgbForeground(view, _defaultFg, _defaultBg)); @@ -186,7 +186,7 @@ void TextAttributeTests::TestReverseDefaultColors() VERIFY_ARE_EQUAL(_defaultFg, attr.CalculateRgbBackground(view, _defaultFg, _defaultBg)); attr.SetForeground(red); - VERIFY_IS_TRUE(attr._IsReverseVideo()); + VERIFY_IS_TRUE(attr.IsReverseVideo()); VERIFY_ARE_EQUAL(red, attr._GetRgbForeground(view, _defaultFg)); VERIFY_ARE_EQUAL(_defaultBg, attr.CalculateRgbForeground(view, _defaultFg, _defaultBg)); @@ -195,7 +195,7 @@ void TextAttributeTests::TestReverseDefaultColors() VERIFY_ARE_EQUAL(red, attr.CalculateRgbBackground(view, _defaultFg, _defaultBg)); attr.Invert(); - VERIFY_IS_FALSE(attr._IsReverseVideo()); + VERIFY_IS_FALSE(attr.IsReverseVideo()); attr.SetDefaultForeground(); attr.SetBackground(green); diff --git a/src/cascadia/TerminalCore/TerminalApi.cpp b/src/cascadia/TerminalCore/TerminalApi.cpp index c2196399959..4189e07b1d0 100644 --- a/src/cascadia/TerminalCore/TerminalApi.cpp +++ b/src/cascadia/TerminalCore/TerminalApi.cpp @@ -44,7 +44,7 @@ bool Terminal::SetTextToDefaults(bool foreground, bool background) noexcept bool Terminal::SetTextForegroundIndex(BYTE colorIndex) noexcept { TextAttribute attrs = _buffer->GetCurrentAttributes(); - attrs.SetIndexedAttributes({ colorIndex }, {}); + attrs.SetIndexedForeground(colorIndex); _buffer->SetCurrentAttributes(attrs); return true; } @@ -52,7 +52,7 @@ bool Terminal::SetTextForegroundIndex(BYTE colorIndex) noexcept bool Terminal::SetTextBackgroundIndex(BYTE colorIndex) noexcept { TextAttribute attrs = _buffer->GetCurrentAttributes(); - attrs.SetIndexedAttributes({}, { colorIndex }); + attrs.SetIndexedBackground(colorIndex); _buffer->SetCurrentAttributes(attrs); return true; } @@ -68,14 +68,7 @@ bool Terminal::SetTextRgbColor(COLORREF color, bool foreground) noexcept bool Terminal::BoldText(bool boldOn) noexcept { TextAttribute attrs = _buffer->GetCurrentAttributes(); - if (boldOn) - { - attrs.Embolden(); - } - else - { - attrs.Debolden(); - } + attrs.SetBold(boldOn); _buffer->SetCurrentAttributes(attrs); return true; } @@ -83,11 +76,7 @@ bool Terminal::BoldText(bool boldOn) noexcept bool Terminal::UnderlineText(bool underlineOn) noexcept { TextAttribute attrs = _buffer->GetCurrentAttributes(); - WORD metaAttrs = attrs.GetMetaAttributes(); - - WI_UpdateFlag(metaAttrs, COMMON_LVB_UNDERSCORE, underlineOn); - - attrs.SetMetaAttributes(metaAttrs); + attrs.SetUnderline(underlineOn); _buffer->SetCurrentAttributes(attrs); return true; } @@ -95,11 +84,7 @@ bool Terminal::UnderlineText(bool underlineOn) noexcept bool Terminal::ReverseText(bool reversed) noexcept { TextAttribute attrs = _buffer->GetCurrentAttributes(); - WORD metaAttrs = attrs.GetMetaAttributes(); - - WI_UpdateFlag(metaAttrs, COMMON_LVB_REVERSE_VIDEO, reversed); - - attrs.SetMetaAttributes(metaAttrs); + attrs.SetReverseVideo(reversed); _buffer->SetCurrentAttributes(attrs); return true; } diff --git a/src/cascadia/TerminalCore/terminalrenderdata.cpp b/src/cascadia/TerminalCore/terminalrenderdata.cpp index 0b6dfdb3fda..22251bfb5c9 100644 --- a/src/cascadia/TerminalCore/terminalrenderdata.cpp +++ b/src/cascadia/TerminalCore/terminalrenderdata.cpp @@ -60,7 +60,7 @@ const COLORREF Terminal::GetBackgroundColor(const TextAttribute& attr) const noe const auto bgColor = attr.CalculateRgbBackground({ _colorTable.data(), _colorTable.size() }, _defaultFg, _defaultBg); // We only care about alpha for the default BG (which enables acrylic) // If the bg isn't the default bg color, or reverse video is enabled, make it fully opaque. - if (!attr.BackgroundIsDefault() || WI_IsFlagSet(attr.GetMetaAttributes(), COMMON_LVB_REVERSE_VIDEO)) + if (!attr.BackgroundIsDefault() || attr.IsReverseVideo()) { return 0xff000000 | bgColor; } diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index 4dfc16e8166..9e769165a38 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -2885,11 +2885,11 @@ void ConptyRoundtripTests::NewLinesAtBottomWithBackground() auto conhostBlueAttrs = defaultAttrs; // Conhost and Terminal store attributes in different bits. - conhostBlueAttrs.SetIndexedAttributes({ static_cast(FOREGROUND_GREEN) }, - { static_cast(FOREGROUND_BLUE) }); + conhostBlueAttrs.SetIndexedForeground(FOREGROUND_GREEN); + conhostBlueAttrs.SetIndexedBackground(FOREGROUND_BLUE); auto terminalBlueAttrs = TextAttribute(); - terminalBlueAttrs.SetIndexedAttributes({ static_cast(XTERM_GREEN_ATTR) }, - { static_cast(XTERM_BLUE_ATTR) }); + terminalBlueAttrs.SetIndexedForeground(XTERM_GREEN_ATTR); + terminalBlueAttrs.SetIndexedBackground(XTERM_BLUE_ATTR); const size_t width = static_cast(TerminalViewWidth); diff --git a/src/host/getset.cpp b/src/host/getset.cpp index 3c006755ccf..8123b33a594 100644 --- a/src/host/getset.cpp +++ b/src/host/getset.cpp @@ -992,14 +992,7 @@ void DoSrvPrivateBoldText(SCREEN_INFORMATION& screenInfo, const bool bolded) { auto& buffer = screenInfo.GetActiveBuffer(); auto attrs = buffer.GetAttributes(); - if (bolded) - { - attrs.Embolden(); - } - else - { - attrs.Debolden(); - } + attrs.SetBold(bolded); buffer.SetAttributes(attrs); } diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index 57be6a3e99c..2ca2f63e018 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -1400,8 +1400,9 @@ void ScreenBufferTests::VtNewlinePastViewport() // Set the attributes that will be used to initialize new rows. auto fillAttr = TextAttribute{ RGB(12, 34, 56), RGB(78, 90, 12) }; - fillAttr.SetExtendedAttributes(ExtendedAttributes::CrossedOut); - fillAttr.SetMetaAttributes(COMMON_LVB_REVERSE_VIDEO | COMMON_LVB_UNDERSCORE); + fillAttr.SetCrossedOut(true); + fillAttr.SetReverseVideo(true); + fillAttr.SetUnderline(true); si.SetAttributes(fillAttr); // But note that the meta attributes are expected to be cleared. auto expectedFillAttr = fillAttr; @@ -1476,8 +1477,9 @@ void ScreenBufferTests::VtNewlinePastEndOfBuffer() // Set the attributes that will be used to initialize new rows. auto fillAttr = TextAttribute{ RGB(12, 34, 56), RGB(78, 90, 12) }; - fillAttr.SetExtendedAttributes(ExtendedAttributes::CrossedOut); - fillAttr.SetMetaAttributes(COMMON_LVB_REVERSE_VIDEO | COMMON_LVB_UNDERSCORE); + fillAttr.SetCrossedOut(true); + fillAttr.SetReverseVideo(true); + fillAttr.SetUnderline(true); si.SetAttributes(fillAttr); // But note that the meta attributes are expected to be cleared. auto expectedFillAttr = fillAttr; @@ -2408,9 +2410,9 @@ void ScreenBufferTests::ReverseResetWithDefaultBackground() VERIFY_ARE_EQUAL(false, attrB.IsLegacy()); VERIFY_ARE_EQUAL(false, attrC.IsLegacy()); - VERIFY_ARE_EQUAL(false, WI_IsFlagSet(attrA.GetMetaAttributes(), COMMON_LVB_REVERSE_VIDEO)); - VERIFY_ARE_EQUAL(true, WI_IsFlagSet(attrB.GetMetaAttributes(), COMMON_LVB_REVERSE_VIDEO)); - VERIFY_ARE_EQUAL(false, WI_IsFlagSet(attrC.GetMetaAttributes(), COMMON_LVB_REVERSE_VIDEO)); + VERIFY_ARE_EQUAL(false, attrA.IsReverseVideo()); + VERIFY_ARE_EQUAL(true, attrB.IsReverseVideo()); + VERIFY_ARE_EQUAL(false, attrC.IsReverseVideo()); VERIFY_ARE_EQUAL(expectedDefaults, attrA); VERIFY_ARE_EQUAL(expectedReversed, attrB); @@ -3389,8 +3391,9 @@ void ScreenBufferTests::ScrollOperations() // Set the attributes that will be used to fill the revealed area. auto fillAttr = TextAttribute{ RGB(12, 34, 56), RGB(78, 90, 12) }; - fillAttr.SetExtendedAttributes(ExtendedAttributes::CrossedOut); - fillAttr.SetMetaAttributes(COMMON_LVB_REVERSE_VIDEO | COMMON_LVB_UNDERSCORE); + fillAttr.SetCrossedOut(true); + fillAttr.SetReverseVideo(true); + fillAttr.SetUnderline(true); si.SetAttributes(fillAttr); // But note that the meta attributes are expected to be cleared. auto expectedFillAttr = fillAttr; @@ -3509,8 +3512,9 @@ void ScreenBufferTests::InsertChars() // Set the attributes that will be used to fill the revealed area. auto fillAttr = TextAttribute{ RGB(12, 34, 56), RGB(78, 90, 12) }; - fillAttr.SetExtendedAttributes(ExtendedAttributes::CrossedOut); - fillAttr.SetMetaAttributes(COMMON_LVB_REVERSE_VIDEO | COMMON_LVB_UNDERSCORE); + fillAttr.SetCrossedOut(true); + fillAttr.SetReverseVideo(true); + fillAttr.SetUnderline(true); si.SetAttributes(fillAttr); // But note that the meta attributes are expected to be cleared. auto expectedFillAttr = fillAttr; @@ -3668,8 +3672,9 @@ void ScreenBufferTests::DeleteChars() // Set the attributes that will be used to fill the revealed area. auto fillAttr = TextAttribute{ RGB(12, 34, 56), RGB(78, 90, 12) }; - fillAttr.SetExtendedAttributes(ExtendedAttributes::CrossedOut); - fillAttr.SetMetaAttributes(COMMON_LVB_REVERSE_VIDEO | COMMON_LVB_UNDERSCORE); + fillAttr.SetCrossedOut(true); + fillAttr.SetReverseVideo(true); + fillAttr.SetUnderline(true); si.SetAttributes(fillAttr); // But note that the meta attributes are expected to be cleared. auto expectedFillAttr = fillAttr; @@ -3899,8 +3904,9 @@ void ScreenBufferTests::EraseTests() // Set the attributes that will be used to fill the erased area. auto fillAttr = TextAttribute{ RGB(12, 34, 56), RGB(78, 90, 12) }; - fillAttr.SetExtendedAttributes(ExtendedAttributes::CrossedOut); - fillAttr.SetMetaAttributes(COMMON_LVB_REVERSE_VIDEO | COMMON_LVB_UNDERSCORE); + fillAttr.SetCrossedOut(true); + fillAttr.SetReverseVideo(true); + fillAttr.SetUnderline(true); si.SetAttributes(fillAttr); // But note that the meta attributes are expected to be cleared. auto expectedFillAttr = fillAttr; @@ -5189,33 +5195,32 @@ void ScreenBufferTests::TestExtendedTextAttributesWithColors() auto& cursor = tbi.GetCursor(); TextAttribute expectedAttr{ si.GetAttributes() }; - ExtendedAttributes expectedExtendedAttrs{ ExtendedAttributes::Normal }; std::wstring vtSeq = L""; // Collect up a VT sequence to set the state given the method properties if (bold) { - WI_SetFlag(expectedExtendedAttrs, ExtendedAttributes::Bold); + expectedAttr.SetBold(true); vtSeq += L"\x1b[1m"; } if (italics) { - WI_SetFlag(expectedExtendedAttrs, ExtendedAttributes::Italics); + expectedAttr.SetItalics(true); vtSeq += L"\x1b[3m"; } if (blink) { - WI_SetFlag(expectedExtendedAttrs, ExtendedAttributes::Blinking); + expectedAttr.SetBlinking(true); vtSeq += L"\x1b[5m"; } if (invisible) { - WI_SetFlag(expectedExtendedAttrs, ExtendedAttributes::Invisible); + expectedAttr.SetInvisible(true); vtSeq += L"\x1b[8m"; } if (crossedOut) { - WI_SetFlag(expectedExtendedAttrs, ExtendedAttributes::CrossedOut); + expectedAttr.SetCrossedOut(true); vtSeq += L"\x1b[9m"; } @@ -5227,7 +5232,7 @@ void ScreenBufferTests::TestExtendedTextAttributesWithColors() } else if (setForegroundType == Use16Color) { - expectedAttr.SetIndexedAttributes({ static_cast(2) }, std::nullopt); + expectedAttr.SetIndexedForeground(2); vtSeq += L"\x1b[32m"; } else if (setForegroundType == Use256Color) @@ -5249,7 +5254,7 @@ void ScreenBufferTests::TestExtendedTextAttributesWithColors() } else if (setBackgroundType == Use16Color) { - expectedAttr.SetIndexedAttributes(std::nullopt, { static_cast(2) }); + expectedAttr.SetIndexedBackground(2); vtSeq += L"\x1b[42m"; } else if (setBackgroundType == Use256Color) @@ -5263,8 +5268,6 @@ void ScreenBufferTests::TestExtendedTextAttributesWithColors() vtSeq += L"\x1b[48;2;1;2;3m"; } - expectedAttr.SetExtendedAttributes(expectedExtendedAttrs); - // Helper lambda to write a VT sequence, then an "X", then check that the // attributes of the "X" match what we think they should be. auto validate = [&](const TextAttribute attr, @@ -5303,36 +5306,31 @@ void ScreenBufferTests::TestExtendedTextAttributesWithColors() // state matched. if (bold) { - WI_ClearFlag(expectedExtendedAttrs, ExtendedAttributes::Bold); - expectedAttr.SetExtendedAttributes(expectedExtendedAttrs); + expectedAttr.SetBold(false); vtSeq = L"\x1b[22m"; validate(expectedAttr, vtSeq); } if (italics) { - WI_ClearFlag(expectedExtendedAttrs, ExtendedAttributes::Italics); - expectedAttr.SetExtendedAttributes(expectedExtendedAttrs); + expectedAttr.SetItalics(false); vtSeq = L"\x1b[23m"; validate(expectedAttr, vtSeq); } if (blink) { - WI_ClearFlag(expectedExtendedAttrs, ExtendedAttributes::Blinking); - expectedAttr.SetExtendedAttributes(expectedExtendedAttrs); + expectedAttr.SetBlinking(false); vtSeq = L"\x1b[25m"; validate(expectedAttr, vtSeq); } if (invisible) { - WI_ClearFlag(expectedExtendedAttrs, ExtendedAttributes::Invisible); - expectedAttr.SetExtendedAttributes(expectedExtendedAttrs); + expectedAttr.SetInvisible(false); vtSeq = L"\x1b[28m"; validate(expectedAttr, vtSeq); } if (crossedOut) { - WI_ClearFlag(expectedExtendedAttrs, ExtendedAttributes::CrossedOut); - expectedAttr.SetExtendedAttributes(expectedExtendedAttrs); + expectedAttr.SetCrossedOut(false); vtSeq = L"\x1b[29m"; validate(expectedAttr, vtSeq); } @@ -5787,7 +5785,8 @@ void ScreenBufferTests::ScreenAlignmentPattern() // Set the initial attributes. auto initialAttr = TextAttribute{ RGB(12, 34, 56), RGB(78, 90, 12) }; - initialAttr.SetMetaAttributes(COMMON_LVB_REVERSE_VIDEO | COMMON_LVB_UNDERSCORE); + initialAttr.SetReverseVideo(true); + initialAttr.SetUnderline(true); si.SetAttributes(initialAttr); // Set some margins. @@ -5818,7 +5817,7 @@ void ScreenBufferTests::ScreenAlignmentPattern() Log::Comment(L"Meta/rendition attributes should be reset."); auto expectedAttr = initialAttr; - expectedAttr.SetMetaAttributes(0); + expectedAttr.SetStandardErase(); VERIFY_ARE_EQUAL(expectedAttr, si.GetAttributes()); } From 249003fa8d4eb37bda12f31514c490858d7985ff Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sun, 19 Apr 2020 01:13:19 +0100 Subject: [PATCH 05/10] Update SetColorTableEntry API to take XTerm indices and add corresponding GetColorTableEntry API. --- src/host/getset.cpp | 26 +++++++- src/host/getset.h | 3 +- src/host/outputStream.cpp | 15 ++++- src/host/outputStream.hpp | 3 +- src/terminal/adapter/adaptDispatch.cpp | 7 +-- src/terminal/adapter/conGetSet.hpp | 3 +- .../adapter/ut_adapter/adapterTest.cpp | 61 +++---------------- 7 files changed, 55 insertions(+), 63 deletions(-) diff --git a/src/host/getset.cpp b/src/host/getset.cpp index 8123b33a594..fd93962157f 100644 --- a/src/host/getset.cpp +++ b/src/host/getset.cpp @@ -2100,6 +2100,28 @@ void DoSrvPrivateMoveToBottom(SCREEN_INFORMATION& screenInfo) screenInfo.GetActiveBuffer().MoveToBottom(); } +// Method Description: +// - Retrieve the color table value at the specified index. +// Arguments: +// - index: the index in the table to retrieve. +// - value: receives the RGB value for the color at that index in the table. +// Return Value: +// - E_INVALIDARG if index is >= 256, else S_OK +[[nodiscard]] HRESULT DoSrvPrivateGetColorTableEntry(const size_t index, COLORREF& value) noexcept +{ + RETURN_HR_IF(E_INVALIDARG, index >= 256); + try + { + Globals& g = ServiceLocator::LocateGlobals(); + CONSOLE_INFORMATION& gci = g.getConsoleInformation(); + + value = gci.GetColorTableEntry(::Xterm256ToWindowsIndex(index)); + + return S_OK; + } + CATCH_RETURN(); +} + // Method Description: // - Sets the color table value in index to the color specified in value. // Can be used to set the 256-color table as well as the 16-color table. @@ -2111,7 +2133,7 @@ void DoSrvPrivateMoveToBottom(SCREEN_INFORMATION& screenInfo) // Notes: // Does not take a buffer parameter. The color table for a console and for // terminals as well is global, not per-screen-buffer. -[[nodiscard]] HRESULT DoSrvPrivateSetColorTableEntry(const short index, const COLORREF value) noexcept +[[nodiscard]] HRESULT DoSrvPrivateSetColorTableEntry(const size_t index, const COLORREF value) noexcept { RETURN_HR_IF(E_INVALIDARG, index >= 256); try @@ -2119,7 +2141,7 @@ void DoSrvPrivateMoveToBottom(SCREEN_INFORMATION& screenInfo) Globals& g = ServiceLocator::LocateGlobals(); CONSOLE_INFORMATION& gci = g.getConsoleInformation(); - gci.SetColorTableEntry(index, value); + gci.SetColorTableEntry(::Xterm256ToWindowsIndex(index), value); // Update the screen colors if we're not a pty // No need to force a redraw in pty mode. diff --git a/src/host/getset.h b/src/host/getset.h index de25e12e7ee..1fad9018e80 100644 --- a/src/host/getset.h +++ b/src/host/getset.h @@ -84,7 +84,8 @@ void DoSrvPrivateInsertLines(const size_t count); void DoSrvPrivateMoveToBottom(SCREEN_INFORMATION& screenInfo); -[[nodiscard]] HRESULT DoSrvPrivateSetColorTableEntry(const short index, const COLORREF value) noexcept; +[[nodiscard]] HRESULT DoSrvPrivateGetColorTableEntry(const size_t index, COLORREF& value) noexcept; +[[nodiscard]] HRESULT DoSrvPrivateSetColorTableEntry(const size_t index, const COLORREF value) noexcept; [[nodiscard]] HRESULT DoSrvPrivateSetDefaultForegroundColor(const COLORREF value) noexcept; diff --git a/src/host/outputStream.cpp b/src/host/outputStream.cpp index 2f21d06320f..c6b25064ff3 100644 --- a/src/host/outputStream.cpp +++ b/src/host/outputStream.cpp @@ -733,6 +733,19 @@ bool ConhostInternalGetSet::MoveToBottom() const return true; } +// Method Description: +// - Connects the PrivateGetColorTableEntry call directly into our Driver Message servicing +// call inside Conhost.exe +// Arguments: +// - index: the index in the table to retrieve. +// - value: receives the RGB value for the color at that index in the table. +// Return Value: +// - true if successful (see DoSrvPrivateGetColorTableEntry). false otherwise. +bool ConhostInternalGetSet::PrivateGetColorTableEntry(const size_t index, COLORREF& value) const noexcept +{ + return SUCCEEDED(DoSrvPrivateGetColorTableEntry(index, value)); +} + // Method Description: // - Connects the PrivateSetColorTableEntry call directly into our Driver Message servicing // call inside Conhost.exe @@ -741,7 +754,7 @@ bool ConhostInternalGetSet::MoveToBottom() const // - value: the new RGB value to use for that index in the color table. // Return Value: // - true if successful (see DoSrvPrivateSetColorTableEntry). false otherwise. -bool ConhostInternalGetSet::PrivateSetColorTableEntry(const short index, const COLORREF value) const noexcept +bool ConhostInternalGetSet::PrivateSetColorTableEntry(const size_t index, const COLORREF value) const noexcept { return SUCCEEDED(DoSrvPrivateSetColorTableEntry(index, value)); } diff --git a/src/host/outputStream.hpp b/src/host/outputStream.hpp index ea900d83bd1..30686e01abd 100644 --- a/src/host/outputStream.hpp +++ b/src/host/outputStream.hpp @@ -144,7 +144,8 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal:: bool MoveToBottom() const override; - bool PrivateSetColorTableEntry(const short index, const COLORREF value) const noexcept override; + bool PrivateGetColorTableEntry(const size_t index, COLORREF& value) const noexcept override; + bool PrivateSetColorTableEntry(const size_t index, const COLORREF value) const noexcept override; bool PrivateSetDefaultForeground(const COLORREF value) const noexcept override; diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index d76d3a75cd9..6d16c2d1376 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -2078,12 +2078,7 @@ bool AdaptDispatch::SetCursorColor(const COLORREF cursorColor) // True if handled successfully. False otherwise. bool AdaptDispatch::SetColorTableEntry(const size_t tableIndex, const DWORD dwColor) { - bool success = tableIndex < 256; - if (success) - { - const auto realIndex = ::Xterm256ToWindowsIndex(tableIndex); - success = _pConApi->PrivateSetColorTableEntry(realIndex, dwColor); - } + const bool success = _pConApi->PrivateSetColorTableEntry(tableIndex, dwColor); // If we're a conpty, always return false, so that we send the updated color // value to the terminal. Still handle the sequence so apps that use diff --git a/src/terminal/adapter/conGetSet.hpp b/src/terminal/adapter/conGetSet.hpp index 229bb74bf59..2697f34d93e 100644 --- a/src/terminal/adapter/conGetSet.hpp +++ b/src/terminal/adapter/conGetSet.hpp @@ -100,7 +100,8 @@ namespace Microsoft::Console::VirtualTerminal virtual bool MoveToBottom() const = 0; - virtual bool PrivateSetColorTableEntry(const short index, const COLORREF value) const = 0; + virtual bool PrivateGetColorTableEntry(const size_t index, COLORREF& value) const = 0; + virtual bool PrivateSetColorTableEntry(const size_t index, const COLORREF value) const = 0; virtual bool PrivateSetDefaultForeground(const COLORREF value) const = 0; virtual bool PrivateSetDefaultBackground(const COLORREF value) const = 0; diff --git a/src/terminal/adapter/ut_adapter/adapterTest.cpp b/src/terminal/adapter/ut_adapter/adapterTest.cpp index 628f212d201..fe412754a2c 100644 --- a/src/terminal/adapter/ut_adapter/adapterTest.cpp +++ b/src/terminal/adapter/ut_adapter/adapterTest.cpp @@ -613,7 +613,13 @@ class TestGetSet final : public ConGetSet return _moveToBottomResult; } - bool PrivateSetColorTableEntry(const short index, const COLORREF value) const noexcept override + bool PrivateGetColorTableEntry(const size_t /*index*/, COLORREF& /*value*/) const noexcept override + { + Log::Comment(L"PrivateGetColorTableEntry MOCK called..."); + return _privateGetColorTableEntryResult; + } + + bool PrivateSetColorTableEntry(const size_t index, const COLORREF value) const noexcept override { Log::Comment(L"PrivateSetColorTableEntry MOCK called..."); if (_privateSetColorTableEntryResult) @@ -901,8 +907,9 @@ class TestGetSet final : public ConGetSet bool _privateSetDefaultAttributesResult = false; bool _moveToBottomResult = false; + bool _privateGetColorTableEntryResult = false; bool _privateSetColorTableEntryResult = false; - short _expectedColorTableIndex = -1; + size_t _expectedColorTableIndex = SIZE_MAX; COLORREF _expectedColorValue = INVALID_COLOR; bool _privateSetDefaultForegroundResult = false; @@ -2148,55 +2155,7 @@ class AdapterTest const auto testColor = RGB(1, 2, 3); _testGetSet->_expectedColorValue = testColor; - _testGetSet->_expectedColorTableIndex = 0; // Windows DARK_BLACK - VERIFY_IS_TRUE(_pDispatch.get()->SetColorTableEntry(0, testColor)); - - _testGetSet->_expectedColorTableIndex = 4; // Windows DARK_RED - VERIFY_IS_TRUE(_pDispatch.get()->SetColorTableEntry(1, testColor)); - - _testGetSet->_expectedColorTableIndex = 2; // Windows DARK_GREEN - VERIFY_IS_TRUE(_pDispatch.get()->SetColorTableEntry(2, testColor)); - - _testGetSet->_expectedColorTableIndex = 6; // Windows DARK_YELLOW - VERIFY_IS_TRUE(_pDispatch.get()->SetColorTableEntry(3, testColor)); - - _testGetSet->_expectedColorTableIndex = 1; // Windows DARK_BLUE - VERIFY_IS_TRUE(_pDispatch.get()->SetColorTableEntry(4, testColor)); - - _testGetSet->_expectedColorTableIndex = 5; // Windows DARK_MAGENTA - VERIFY_IS_TRUE(_pDispatch.get()->SetColorTableEntry(5, testColor)); - - _testGetSet->_expectedColorTableIndex = 3; // Windows DARK_CYAN - VERIFY_IS_TRUE(_pDispatch.get()->SetColorTableEntry(6, testColor)); - - _testGetSet->_expectedColorTableIndex = 7; // Windows DARK_WHITE - VERIFY_IS_TRUE(_pDispatch.get()->SetColorTableEntry(7, testColor)); - - _testGetSet->_expectedColorTableIndex = 8; // Windows BRIGHT_BLACK - VERIFY_IS_TRUE(_pDispatch.get()->SetColorTableEntry(8, testColor)); - - _testGetSet->_expectedColorTableIndex = 12; // Windows BRIGHT_RED - VERIFY_IS_TRUE(_pDispatch.get()->SetColorTableEntry(9, testColor)); - - _testGetSet->_expectedColorTableIndex = 10; // Windows BRIGHT_GREEN - VERIFY_IS_TRUE(_pDispatch.get()->SetColorTableEntry(10, testColor)); - - _testGetSet->_expectedColorTableIndex = 14; // Windows BRIGHT_YELLOW - VERIFY_IS_TRUE(_pDispatch.get()->SetColorTableEntry(11, testColor)); - - _testGetSet->_expectedColorTableIndex = 9; // Windows BRIGHT_BLUE - VERIFY_IS_TRUE(_pDispatch.get()->SetColorTableEntry(12, testColor)); - - _testGetSet->_expectedColorTableIndex = 13; // Windows BRIGHT_MAGENTA - VERIFY_IS_TRUE(_pDispatch.get()->SetColorTableEntry(13, testColor)); - - _testGetSet->_expectedColorTableIndex = 11; // Windows BRIGHT_CYAN - VERIFY_IS_TRUE(_pDispatch.get()->SetColorTableEntry(14, testColor)); - - _testGetSet->_expectedColorTableIndex = 15; // Windows BRIGHT_WHITE - VERIFY_IS_TRUE(_pDispatch.get()->SetColorTableEntry(15, testColor)); - - for (short i = 16; i < 256; i++) + for (size_t i = 0; i < 256; i++) { _testGetSet->_expectedColorTableIndex = i; VERIFY_IS_TRUE(_pDispatch.get()->SetColorTableEntry(i, testColor)); From 95343370aa5c2f7e36934a4b903ab3c435053e29 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sun, 19 Apr 2020 03:14:30 +0100 Subject: [PATCH 06/10] Rewrite the AdaptDispatch::SetGraphicsRendition method with a simpler implementation. --- src/terminal/adapter/adaptDispatch.cpp | 3 - src/terminal/adapter/adaptDispatch.hpp | 18 +- .../adapter/adaptDispatchGraphics.cpp | 692 ++++++------------ .../ut_adapter/Adapter.UnitTests.vcxproj | 5 +- 4 files changed, 215 insertions(+), 503 deletions(-) diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index 6d16c2d1376..54c208caaf1 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -31,9 +31,6 @@ AdaptDispatch::AdaptDispatch(std::unique_ptr pConApi, _usingAltBuffer(false), _isOriginModeRelative(false), // by default, the DECOM origin mode is absolute. _isDECCOLMAllowed(false), // by default, DECCOLM is not allowed. - _changedBackground(false), - _changedForeground(false), - _changedMetaAttrs(false), _termOutput() { THROW_HR_IF_NULL(E_INVALIDARG, _pConApi.get()); diff --git a/src/terminal/adapter/adaptDispatch.hpp b/src/terminal/adapter/adaptDispatch.hpp index d2c28d59105..8bdd40b72ee 100644 --- a/src/terminal/adapter/adaptDispatch.hpp +++ b/src/terminal/adapter/adaptDispatch.hpp @@ -135,13 +135,10 @@ namespace Microsoft::Console::VirtualTerminal bool _EraseSingleLineHelper(const CONSOLE_SCREEN_BUFFER_INFOEX& csbiex, const DispatchTypes::EraseType eraseType, const size_t lineId) const; - void _SetGraphicsOptionHelper(const DispatchTypes::GraphicsOptions opt, WORD& attr); bool _EraseScrollback(); bool _EraseAll(); bool _InsertDeleteHelper(const size_t count, const bool isInsert) const; bool _ScrollMovement(const ScrollDirection dir, const size_t distance) const; - static void s_DisableAllColors(WORD& attr, const bool isForeground) noexcept; - static void s_ApplyColors(WORD& attr, const WORD applyThis, const bool isForeground) noexcept; bool _DoSetTopBottomScrollingMargins(const size_t topMargin, const size_t bottomMargin); @@ -179,17 +176,8 @@ namespace Microsoft::Console::VirtualTerminal bool _isDECCOLMAllowed; - bool _changedForeground; - bool _changedBackground; - bool _changedMetaAttrs; - - bool _SetRgbColorsHelper(const std::basic_string_view options, - COLORREF& rgbColor, - bool& isForeground, - size_t& optionsConsumed); - - bool _SetBoldColorHelper(const DispatchTypes::GraphicsOptions option); - bool _SetDefaultColorHelper(const DispatchTypes::GraphicsOptions option); - bool _SetExtendedTextAttributeHelper(const DispatchTypes::GraphicsOptions option); + size_t _SetRgbColorsHelper(const std::basic_string_view options, + TextAttribute& attr, + const bool isForeground); }; } diff --git a/src/terminal/adapter/adaptDispatchGraphics.cpp b/src/terminal/adapter/adaptDispatchGraphics.cpp index 9553cf0eef1..a648478d136 100644 --- a/src/terminal/adapter/adaptDispatchGraphics.cpp +++ b/src/terminal/adapter/adaptDispatchGraphics.cpp @@ -13,318 +13,28 @@ using namespace Microsoft::Console::VirtualTerminal; using namespace Microsoft::Console::VirtualTerminal::DispatchTypes; -// Routine Description: -// - Small helper to disable all color flags within a given font attributes field -// Arguments: -// - attr - Font attributes field to adjust -// - isForeground - True if we're modifying the FOREGROUND colors. False if we're doing BACKGROUND. -// Return Value: -// - -void AdaptDispatch::s_DisableAllColors(WORD& attr, const bool isForeground) noexcept -{ - if (isForeground) - { - WI_ClearAllFlags(attr, FG_ATTRS); - } - else - { - WI_ClearAllFlags(attr, BG_ATTRS); - } -} - -// Routine Description: -// - Small helper to help mask off the appropriate foreground/background bits in the colors bitfield. -// Arguments: -// - attr - Font attributes field to adjust -// - applyThis - Color values to apply to the low or high word of the font attributes field. -// - isForeground - TRUE = foreground color. FALSE = background color. -// Specifies which half of the bit field to reset and then apply wApplyThis -// upon. -// Return Value: -// - -void AdaptDispatch::s_ApplyColors(WORD& attr, const WORD applyThis, const bool isForeground) noexcept -{ -#pragma warning(suppress : 26496) // SA is wrong. This variable is assigned more than once. - // Copy the new attribute to apply - WORD wNewColors = applyThis; - - // Mask off only the foreground or background - if (isForeground) - { - WI_UpdateFlagsInMask(attr, FG_ATTRS, wNewColors); - } - else - { - WI_UpdateFlagsInMask(attr, BG_ATTRS, wNewColors); - } -} - -// Routine Description: -// - Helper to apply the actual flags to each text attributes field. -// - Placed as a helper so it can be recursive/re-entrant for some of the -// convenience flag methods that perform similar/multiple operations in one -// command. -// Arguments: -// - opt - Graphics option sent to us by the parser/requestor. -// - attr - Font attribute field to adjust -// Return Value: -// - -void AdaptDispatch::_SetGraphicsOptionHelper(const DispatchTypes::GraphicsOptions opt, WORD& attr) -{ - switch (opt) - { - case DispatchTypes::GraphicsOptions::Off: - FAIL_FAST_MSG("GraphicsOptions::Off should be handled by _SetDefaultColorHelper"); - break; - // MSFT:16398982 - These two are now handled by _SetBoldColorHelper - // case DispatchTypes::GraphicsOptions::BoldBright: - // case DispatchTypes::GraphicsOptions::UnBold: - case DispatchTypes::GraphicsOptions::Negative: - WI_SetFlag(attr, COMMON_LVB_REVERSE_VIDEO); - _changedMetaAttrs = true; - break; - case DispatchTypes::GraphicsOptions::Underline: - // TODO:GH#2915 Treat underline separately from LVB_UNDERSCORE - WI_SetFlag(attr, COMMON_LVB_UNDERSCORE); - _changedMetaAttrs = true; - break; - case DispatchTypes::GraphicsOptions::Positive: - WI_ClearFlag(attr, COMMON_LVB_REVERSE_VIDEO); - _changedMetaAttrs = true; - break; - case DispatchTypes::GraphicsOptions::NoUnderline: - WI_ClearFlag(attr, COMMON_LVB_UNDERSCORE); - _changedMetaAttrs = true; - break; - case DispatchTypes::GraphicsOptions::ForegroundBlack: - s_DisableAllColors(attr, true); // turn off all color flags first. - _changedForeground = true; - break; - case DispatchTypes::GraphicsOptions::ForegroundBlue: - s_DisableAllColors(attr, true); // turn off all color flags first. - WI_SetFlag(attr, FOREGROUND_BLUE); - _changedForeground = true; - break; - case DispatchTypes::GraphicsOptions::ForegroundGreen: - s_DisableAllColors(attr, true); // turn off all color flags first. - WI_SetFlag(attr, FOREGROUND_GREEN); - _changedForeground = true; - break; - case DispatchTypes::GraphicsOptions::ForegroundCyan: - s_DisableAllColors(attr, true); // turn off all color flags first. - WI_SetAllFlags(attr, FOREGROUND_BLUE | FOREGROUND_GREEN); - _changedForeground = true; - break; - case DispatchTypes::GraphicsOptions::ForegroundRed: - s_DisableAllColors(attr, true); // turn off all color flags first. - WI_SetFlag(attr, FOREGROUND_RED); - _changedForeground = true; - break; - case DispatchTypes::GraphicsOptions::ForegroundMagenta: - s_DisableAllColors(attr, true); // turn off all color flags first. - WI_SetAllFlags(attr, FOREGROUND_BLUE | FOREGROUND_RED); - _changedForeground = true; - break; - case DispatchTypes::GraphicsOptions::ForegroundYellow: - s_DisableAllColors(attr, true); // turn off all color flags first. - WI_SetAllFlags(attr, FOREGROUND_GREEN | FOREGROUND_RED); - _changedForeground = true; - break; - case DispatchTypes::GraphicsOptions::ForegroundWhite: - s_DisableAllColors(attr, true); // turn off all color flags first. - WI_SetAllFlags(attr, FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED); - _changedForeground = true; - break; - case DispatchTypes::GraphicsOptions::ForegroundDefault: - FAIL_FAST_MSG("GraphicsOptions::ForegroundDefault should be handled by _SetDefaultColorHelper"); - break; - case DispatchTypes::GraphicsOptions::BackgroundBlack: - s_DisableAllColors(attr, false); // turn off all color flags first. - _changedBackground = true; - break; - case DispatchTypes::GraphicsOptions::BackgroundBlue: - s_DisableAllColors(attr, false); // turn off all color flags first. - WI_SetFlag(attr, BACKGROUND_BLUE); - _changedBackground = true; - break; - case DispatchTypes::GraphicsOptions::BackgroundGreen: - s_DisableAllColors(attr, false); // turn off all color flags first. - WI_SetFlag(attr, BACKGROUND_GREEN); - _changedBackground = true; - break; - case DispatchTypes::GraphicsOptions::BackgroundCyan: - s_DisableAllColors(attr, false); // turn off all color flags first. - WI_SetAllFlags(attr, BACKGROUND_BLUE | BACKGROUND_GREEN); - _changedBackground = true; - break; - case DispatchTypes::GraphicsOptions::BackgroundRed: - s_DisableAllColors(attr, false); // turn off all color flags first. - WI_SetFlag(attr, BACKGROUND_RED); - _changedBackground = true; - break; - case DispatchTypes::GraphicsOptions::BackgroundMagenta: - s_DisableAllColors(attr, false); // turn off all color flags first. - WI_SetAllFlags(attr, BACKGROUND_BLUE | BACKGROUND_RED); - _changedBackground = true; - break; - case DispatchTypes::GraphicsOptions::BackgroundYellow: - s_DisableAllColors(attr, false); // turn off all color flags first. - WI_SetAllFlags(attr, BACKGROUND_GREEN | BACKGROUND_RED); - _changedBackground = true; - break; - case DispatchTypes::GraphicsOptions::BackgroundWhite: - s_DisableAllColors(attr, false); // turn off all color flags first. - WI_SetAllFlags(attr, BACKGROUND_BLUE | BACKGROUND_GREEN | BACKGROUND_RED); - _changedBackground = true; - break; - case DispatchTypes::GraphicsOptions::BackgroundDefault: - FAIL_FAST_MSG("GraphicsOptions::BackgroundDefault should be handled by _SetDefaultColorHelper"); - break; - case DispatchTypes::GraphicsOptions::BrightForegroundBlack: - _SetGraphicsOptionHelper(GraphicsOptions::ForegroundBlack, attr); - WI_SetFlag(attr, FOREGROUND_INTENSITY); - _changedForeground = true; - break; - case DispatchTypes::GraphicsOptions::BrightForegroundBlue: - _SetGraphicsOptionHelper(GraphicsOptions::ForegroundBlue, attr); - WI_SetFlag(attr, FOREGROUND_INTENSITY); - _changedForeground = true; - break; - case DispatchTypes::GraphicsOptions::BrightForegroundGreen: - _SetGraphicsOptionHelper(GraphicsOptions::ForegroundGreen, attr); - WI_SetFlag(attr, FOREGROUND_INTENSITY); - _changedForeground = true; - break; - case DispatchTypes::GraphicsOptions::BrightForegroundCyan: - _SetGraphicsOptionHelper(GraphicsOptions::ForegroundCyan, attr); - WI_SetFlag(attr, FOREGROUND_INTENSITY); - _changedForeground = true; - break; - case DispatchTypes::GraphicsOptions::BrightForegroundRed: - _SetGraphicsOptionHelper(GraphicsOptions::ForegroundRed, attr); - WI_SetFlag(attr, FOREGROUND_INTENSITY); - _changedForeground = true; - break; - case DispatchTypes::GraphicsOptions::BrightForegroundMagenta: - _SetGraphicsOptionHelper(GraphicsOptions::ForegroundMagenta, attr); - WI_SetFlag(attr, FOREGROUND_INTENSITY); - _changedForeground = true; - break; - case DispatchTypes::GraphicsOptions::BrightForegroundYellow: - _SetGraphicsOptionHelper(GraphicsOptions::ForegroundYellow, attr); - WI_SetFlag(attr, FOREGROUND_INTENSITY); - _changedForeground = true; - break; - case DispatchTypes::GraphicsOptions::BrightForegroundWhite: - _SetGraphicsOptionHelper(GraphicsOptions::ForegroundWhite, attr); - WI_SetFlag(attr, FOREGROUND_INTENSITY); - _changedForeground = true; - break; - case DispatchTypes::GraphicsOptions::BrightBackgroundBlack: - _SetGraphicsOptionHelper(GraphicsOptions::BackgroundBlack, attr); - WI_SetFlag(attr, BACKGROUND_INTENSITY); - _changedBackground = true; - break; - case DispatchTypes::GraphicsOptions::BrightBackgroundBlue: - _SetGraphicsOptionHelper(GraphicsOptions::BackgroundBlue, attr); - WI_SetFlag(attr, BACKGROUND_INTENSITY); - _changedBackground = true; - break; - case DispatchTypes::GraphicsOptions::BrightBackgroundGreen: - _SetGraphicsOptionHelper(GraphicsOptions::BackgroundGreen, attr); - WI_SetFlag(attr, BACKGROUND_INTENSITY); - _changedBackground = true; - break; - case DispatchTypes::GraphicsOptions::BrightBackgroundCyan: - _SetGraphicsOptionHelper(GraphicsOptions::BackgroundCyan, attr); - WI_SetFlag(attr, BACKGROUND_INTENSITY); - _changedBackground = true; - break; - case DispatchTypes::GraphicsOptions::BrightBackgroundRed: - _SetGraphicsOptionHelper(GraphicsOptions::BackgroundRed, attr); - WI_SetFlag(attr, BACKGROUND_INTENSITY); - _changedBackground = true; - break; - case DispatchTypes::GraphicsOptions::BrightBackgroundMagenta: - _SetGraphicsOptionHelper(GraphicsOptions::BackgroundMagenta, attr); - WI_SetFlag(attr, BACKGROUND_INTENSITY); - _changedBackground = true; - break; - case DispatchTypes::GraphicsOptions::BrightBackgroundYellow: - _SetGraphicsOptionHelper(GraphicsOptions::BackgroundYellow, attr); - WI_SetFlag(attr, BACKGROUND_INTENSITY); - _changedBackground = true; - break; - case DispatchTypes::GraphicsOptions::BrightBackgroundWhite: - _SetGraphicsOptionHelper(GraphicsOptions::BackgroundWhite, attr); - WI_SetFlag(attr, BACKGROUND_INTENSITY); - _changedBackground = true; - break; - } -} - -#pragma warning(push) -#pragma warning(disable : 26497) // we do not want constexpr compilation because these always evaluate at runtime - -// Routine Description: -// Returns true if the GraphicsOption represents an extended text attribute. -// These include things such as Underlined, Italics, Blinking, etc. -// Return Value: -// - true if the opt is the indicator for an extended text attribute, false otherwise. -static constexpr bool _isExtendedTextAttribute(const DispatchTypes::GraphicsOptions opt) noexcept -{ - // TODO:GH#2916 add support for DoublyUnderlined, Faint(RGBColorOrFaint). - // These two are currently partially implemented as other things: - // * Faint is approximately the opposite of bold does, though it's much - // [more complicated]( - // https://github.com/microsoft/terminal/issues/2916#issuecomment-535860910) - // and less supported/used. - // * Doubly underlined should exist in a trinary state with Underlined - return opt == DispatchTypes::GraphicsOptions::Italics || - opt == DispatchTypes::GraphicsOptions::NotItalics || - opt == DispatchTypes::GraphicsOptions::BlinkOrXterm256Index || - opt == DispatchTypes::GraphicsOptions::Steady || - opt == DispatchTypes::GraphicsOptions::Invisible || - opt == DispatchTypes::GraphicsOptions::Visible || - opt == DispatchTypes::GraphicsOptions::CrossedOut || - opt == DispatchTypes::GraphicsOptions::NotCrossedOut; -} - -// Routine Description: -// Returns true if the GraphicsOption represents an extended color option. -// These are followed by up to 4 more values which compose the entire option. -// Return Value: -// - true if the opt is the indicator for an extended color sequence, false otherwise. -static constexpr bool _isRgbColorOption(const DispatchTypes::GraphicsOptions opt) noexcept -{ - return opt == DispatchTypes::GraphicsOptions::ForegroundExtended || - opt == DispatchTypes::GraphicsOptions::BackgroundExtended; -} - -// Routine Description: -// Returns true if the GraphicsOption represents an extended color option. -// These are followed by up to 4 more values which compose the entire option. -// Return Value: -// - true if the opt is the indicator for an extended color sequence, false otherwise. -static constexpr bool _isBoldColorOption(const DispatchTypes::GraphicsOptions opt) noexcept -{ - return opt == DispatchTypes::GraphicsOptions::BoldBright || - opt == DispatchTypes::GraphicsOptions::UnBold; -} - -// Function Description: -// - checks if this graphics option should set either the console's FG or BG to -//the default attributes. -// Return Value: -// - true if the opt sets either/or attribute to the defaults, false otherwise. -static constexpr bool _isDefaultColorOption(const DispatchTypes::GraphicsOptions opt) noexcept -{ - return opt == DispatchTypes::GraphicsOptions::Off || - opt == DispatchTypes::GraphicsOptions::ForegroundDefault || - opt == DispatchTypes::GraphicsOptions::BackgroundDefault; -} - -#pragma warning(pop) +// clang-format off +const BYTE BLUE_ATTR = 0x01; +const BYTE GREEN_ATTR = 0x02; +const BYTE RED_ATTR = 0x04; +const BYTE BRIGHT_ATTR = 0x08; +const BYTE DARK_BLACK = 0; +const BYTE DARK_RED = RED_ATTR; +const BYTE DARK_GREEN = GREEN_ATTR; +const BYTE DARK_YELLOW = RED_ATTR | GREEN_ATTR; +const BYTE DARK_BLUE = BLUE_ATTR; +const BYTE DARK_MAGENTA = RED_ATTR | BLUE_ATTR; +const BYTE DARK_CYAN = GREEN_ATTR | BLUE_ATTR; +const BYTE DARK_WHITE = RED_ATTR | GREEN_ATTR | BLUE_ATTR; +const BYTE BRIGHT_BLACK = BRIGHT_ATTR; +const BYTE BRIGHT_RED = BRIGHT_ATTR | RED_ATTR; +const BYTE BRIGHT_GREEN = BRIGHT_ATTR | GREEN_ATTR; +const BYTE BRIGHT_YELLOW = BRIGHT_ATTR | RED_ATTR | GREEN_ATTR; +const BYTE BRIGHT_BLUE = BRIGHT_ATTR | BLUE_ATTR; +const BYTE BRIGHT_MAGENTA = BRIGHT_ATTR | RED_ATTR | BLUE_ATTR; +const BYTE BRIGHT_CYAN = BRIGHT_ATTR | GREEN_ATTR | BLUE_ATTR; +const BYTE BRIGHT_WHITE = BRIGHT_ATTR | RED_ATTR | GREEN_ATTR | BLUE_ATTR; +// clang-format on // Routine Description: // - Helper to parse extended graphics options, which start with 38 (FG) or 48 (BG) @@ -333,137 +43,44 @@ static constexpr bool _isDefaultColorOption(const DispatchTypes::GraphicsOptions // Xterm index will use the param that follows to use a color from the preset 256 color xterm color table. // Arguments: // - options - An array of options that will be used to generate the RGB color -// - rgbColor - Location to place the generated RGB color into. -// - isForeground - Location to place whether or not the parsed color is for the foreground or not. -// - optionsConsumed - Location to place the number of options we consumed parsing this option. +// - attr - The attribute that will be updated with the parsed color. +// - isForeground - Whether or not the parsed color is for the foreground or not. // Return Value: -// Returns true if we successfully parsed an extended color option from the options array. -// - This corresponds to the following number of options consumed (pcOptionsConsumed): -// 1 - false, not enough options to parse. -// 2 - false, not enough options to parse. -// 3 - true, parsed an xterm index to a color -// 5 - true, parsed an RGB color. -bool AdaptDispatch::_SetRgbColorsHelper(const std::basic_string_view options, - COLORREF& rgbColor, - bool& isForeground, - size_t& optionsConsumed) +// - The number of options consumed, not including the initial 38/48. +size_t AdaptDispatch::_SetRgbColorsHelper(const std::basic_string_view options, + TextAttribute& attr, + const bool isForeground) { - bool success = false; - optionsConsumed = 1; - if (options.size() >= 2 && _isRgbColorOption(til::at(options, 0))) + size_t optionsConsumed = 0; + if (options.size() >= 1) { - optionsConsumed = 2; - const auto extendedOpt = til::at(options, 0); - const auto typeOpt = til::at(options, 1); - - if (extendedOpt == DispatchTypes::GraphicsOptions::ForegroundExtended) - { - isForeground = true; - } - else if (extendedOpt == DispatchTypes::GraphicsOptions::BackgroundExtended) - { - isForeground = false; - } - - if (typeOpt == DispatchTypes::GraphicsOptions::RGBColorOrFaint && options.size() >= 5) + optionsConsumed = 1; + const auto typeOpt = til::at(options, 0); + if (typeOpt == DispatchTypes::GraphicsOptions::RGBColorOrFaint && options.size() >= 4) { - optionsConsumed = 5; + optionsConsumed = 4; + const size_t red = til::at(options, 1); + const size_t green = til::at(options, 2); + const size_t blue = til::at(options, 3); // ensure that each value fits in a byte - unsigned int red = std::min(static_cast(til::at(options, 2)), 255u); - unsigned int green = std::min(static_cast(til::at(options, 3)), 255u); - unsigned int blue = std::min(static_cast(til::at(options, 4)), 255u); - - rgbColor = RGB(red, green, blue); - - success = _pConApi->SetConsoleRGBTextAttribute(rgbColor, isForeground); + if (red <= 255 && green <= 255 && blue <= 255) + { + const COLORREF rgbColor = RGB(red, green, blue); + attr.SetColor(rgbColor, isForeground); + } } - else if (typeOpt == DispatchTypes::GraphicsOptions::BlinkOrXterm256Index && options.size() >= 3) + else if (typeOpt == DispatchTypes::GraphicsOptions::BlinkOrXterm256Index && options.size() >= 2) { - optionsConsumed = 3; - if (til::at(options, 2) <= 255) // ensure that the provided index is on the table + optionsConsumed = 2; + const size_t tableIndex = til::at(options, 1); + COLORREF rgbColor; + if (_pConApi->PrivateGetColorTableEntry(tableIndex, rgbColor)) { - const auto tableIndex = til::at(options, 2); - - success = _pConApi->SetConsoleXtermTextAttribute(tableIndex, isForeground); + attr.SetColor(rgbColor, isForeground); } } } - return success; -} - -bool AdaptDispatch::_SetBoldColorHelper(const DispatchTypes::GraphicsOptions option) -{ - const bool bold = (option == DispatchTypes::GraphicsOptions::BoldBright); - return _pConApi->PrivateBoldText(bold); -} - -bool AdaptDispatch::_SetDefaultColorHelper(const DispatchTypes::GraphicsOptions option) -{ - const bool fg = option == GraphicsOptions::Off || option == GraphicsOptions::ForegroundDefault; - const bool bg = option == GraphicsOptions::Off || option == GraphicsOptions::BackgroundDefault; - - bool success = _pConApi->PrivateSetDefaultAttributes(fg, bg); - - if (success && fg && bg) - { - // If we're resetting both the FG & BG, also reset the meta attributes (underline) - // as well as the boldness - success = _pConApi->PrivateSetLegacyAttributes(0, false, false, true) && - _pConApi->PrivateBoldText(false) && - _pConApi->PrivateSetExtendedTextAttributes(ExtendedAttributes::Normal); - } - return success; -} - -// Method Description: -// - Sets the attributes for extended text attributes. Retrieves the current -// extended attrs from the console, modifies them according to the new -// GraphicsOption, and the sets them again. -// - Notably does _not_ handle Bold, Faint, Underline, DoublyUnderlined, or -// NoUnderline. Those should be handled in TODO:GH#2916. -// Arguments: -// - opt: the graphics option to set -// Return Value: -// - True if handled successfully. False otherwise. -bool AdaptDispatch::_SetExtendedTextAttributeHelper(const DispatchTypes::GraphicsOptions opt) -{ - ExtendedAttributes attrs{ ExtendedAttributes::Normal }; - - RETURN_BOOL_IF_FALSE(_pConApi->PrivateGetExtendedTextAttributes(attrs)); - - switch (opt) - { - case DispatchTypes::GraphicsOptions::Italics: - WI_SetFlag(attrs, ExtendedAttributes::Italics); - break; - case DispatchTypes::GraphicsOptions::NotItalics: - WI_ClearFlag(attrs, ExtendedAttributes::Italics); - break; - case DispatchTypes::GraphicsOptions::BlinkOrXterm256Index: - WI_SetFlag(attrs, ExtendedAttributes::Blinking); - break; - case DispatchTypes::GraphicsOptions::Steady: - WI_ClearFlag(attrs, ExtendedAttributes::Blinking); - break; - case DispatchTypes::GraphicsOptions::Invisible: - WI_SetFlag(attrs, ExtendedAttributes::Invisible); - break; - case DispatchTypes::GraphicsOptions::Visible: - WI_ClearFlag(attrs, ExtendedAttributes::Invisible); - break; - case DispatchTypes::GraphicsOptions::CrossedOut: - WI_SetFlag(attrs, ExtendedAttributes::CrossedOut); - break; - case DispatchTypes::GraphicsOptions::NotCrossedOut: - WI_ClearFlag(attrs, ExtendedAttributes::CrossedOut); - break; - // TODO:GH#2916 add support for the following - // case DispatchTypes::GraphicsOptions::DoublyUnderlined: - // case DispatchTypes::GraphicsOptions::RGBColorOrFaint: - // case DispatchTypes::GraphicsOptions::DoublyUnderlined: - } - - return _pConApi->PrivateSetExtendedTextAttributes(attrs); + return optionsConsumed; } // Routine Description: @@ -471,7 +88,6 @@ bool AdaptDispatch::_SetExtendedTextAttributeHelper(const DispatchTypes::Graphic // characters written into the buffer. // - Options include colors, invert, underlines, and other "font style" // type options. - // Arguments: // - options - An array of options that will be applied from 0 to N, in order, // one at a time by setting or removing flags in the font style properties. @@ -479,14 +95,8 @@ bool AdaptDispatch::_SetExtendedTextAttributeHelper(const DispatchTypes::Graphic // - True if handled successfully. False otherwise. bool AdaptDispatch::SetGraphicsRendition(const std::basic_string_view options) { - // We use the private function here to get just the default color attributes - // as a performance optimization. Calling the public - // GetConsoleScreenBufferInfoEx costs a lot of performance time/power in a - // tight loop because it has to fill the Largest Window Size by asking the - // OS and wastes time memcpying colors and other data we do not need to - // resolve this Set Graphics Rendition request. - WORD attr; - bool success = _pConApi->PrivateGetConsoleScreenBufferAttributes(attr); + TextAttribute attr; + bool success = _pConApi->PrivateGetTextAttributes(attr); if (success) { @@ -494,52 +104,166 @@ bool AdaptDispatch::SetGraphicsRendition(const std::basic_string_viewPrivateSetLegacyAttributes(attr, - _changedForeground, - _changedBackground, - _changedMetaAttrs); - - // Make sure we un-bold - if (success && opt == DispatchTypes::GraphicsOptions::Off) - { - success = _SetBoldColorHelper(opt); - } - - _changedForeground = false; - _changedBackground = false; - _changedMetaAttrs = false; + case Off: + attr.SetDefaultForeground(); + attr.SetDefaultBackground(); + attr.SetStandardErase(); + break; + case ForegroundDefault: + attr.SetDefaultForeground(); + break; + case BackgroundDefault: + attr.SetDefaultBackground(); + break; + case BoldBright: + attr.SetBold(true); + break; + case UnBold: + attr.SetBold(false); + break; + case Italics: + attr.SetItalics(true); + break; + case NotItalics: + attr.SetItalics(false); + break; + case BlinkOrXterm256Index: + attr.SetBlinking(true); + break; + case Steady: + attr.SetBlinking(false); + break; + case Invisible: + attr.SetInvisible(true); + break; + case Visible: + attr.SetInvisible(false); + break; + case CrossedOut: + attr.SetCrossedOut(true); + break; + case NotCrossedOut: + attr.SetCrossedOut(false); + break; + case Negative: + attr.SetReverseVideo(true); + break; + case Positive: + attr.SetReverseVideo(false); + break; + case Underline: + attr.SetUnderline(true); + break; + case NoUnderline: + attr.SetUnderline(false); + break; + case ForegroundBlack: + attr.SetIndexedForeground(DARK_BLACK); + break; + case ForegroundBlue: + attr.SetIndexedForeground(DARK_BLUE); + break; + case ForegroundGreen: + attr.SetIndexedForeground(DARK_GREEN); + break; + case ForegroundCyan: + attr.SetIndexedForeground(DARK_CYAN); + break; + case ForegroundRed: + attr.SetIndexedForeground(DARK_RED); + break; + case ForegroundMagenta: + attr.SetIndexedForeground(DARK_MAGENTA); + break; + case ForegroundYellow: + attr.SetIndexedForeground(DARK_YELLOW); + break; + case ForegroundWhite: + attr.SetIndexedForeground(DARK_WHITE); + break; + case BackgroundBlack: + attr.SetIndexedBackground(DARK_BLACK); + break; + case BackgroundBlue: + attr.SetIndexedBackground(DARK_BLUE); + break; + case BackgroundGreen: + attr.SetIndexedBackground(DARK_GREEN); + break; + case BackgroundCyan: + attr.SetIndexedBackground(DARK_CYAN); + break; + case BackgroundRed: + attr.SetIndexedBackground(DARK_RED); + break; + case BackgroundMagenta: + attr.SetIndexedBackground(DARK_MAGENTA); + break; + case BackgroundYellow: + attr.SetIndexedBackground(DARK_YELLOW); + break; + case BackgroundWhite: + attr.SetIndexedBackground(DARK_WHITE); + break; + case BrightForegroundBlack: + attr.SetIndexedForeground(BRIGHT_BLACK); + break; + case BrightForegroundBlue: + attr.SetIndexedForeground(BRIGHT_BLUE); + break; + case BrightForegroundGreen: + attr.SetIndexedForeground(BRIGHT_GREEN); + break; + case BrightForegroundCyan: + attr.SetIndexedForeground(BRIGHT_CYAN); + break; + case BrightForegroundRed: + attr.SetIndexedForeground(BRIGHT_RED); + break; + case BrightForegroundMagenta: + attr.SetIndexedForeground(BRIGHT_MAGENTA); + break; + case BrightForegroundYellow: + attr.SetIndexedForeground(BRIGHT_YELLOW); + break; + case BrightForegroundWhite: + attr.SetIndexedForeground(BRIGHT_WHITE); + break; + case BrightBackgroundBlack: + attr.SetIndexedBackground(BRIGHT_BLACK); + break; + case BrightBackgroundBlue: + attr.SetIndexedBackground(BRIGHT_BLUE); + break; + case BrightBackgroundGreen: + attr.SetIndexedBackground(BRIGHT_GREEN); + break; + case BrightBackgroundCyan: + attr.SetIndexedBackground(BRIGHT_CYAN); + break; + case BrightBackgroundRed: + attr.SetIndexedBackground(BRIGHT_RED); + break; + case BrightBackgroundMagenta: + attr.SetIndexedBackground(BRIGHT_MAGENTA); + break; + case BrightBackgroundYellow: + attr.SetIndexedBackground(BRIGHT_YELLOW); + break; + case BrightBackgroundWhite: + attr.SetIndexedBackground(BRIGHT_WHITE); + break; + case ForegroundExtended: + i += _SetRgbColorsHelper(options.substr(i + 1), attr, true); + break; + case BackgroundExtended: + i += _SetRgbColorsHelper(options.substr(i + 1), attr, false); + break; } } + success = _pConApi->PrivateSetTextAttributes(attr); } return success; diff --git a/src/terminal/adapter/ut_adapter/Adapter.UnitTests.vcxproj b/src/terminal/adapter/ut_adapter/Adapter.UnitTests.vcxproj index f58693e6800..553c9b275ac 100644 --- a/src/terminal/adapter/ut_adapter/Adapter.UnitTests.vcxproj +++ b/src/terminal/adapter/ut_adapter/Adapter.UnitTests.vcxproj @@ -6,7 +6,7 @@ AdapterUnitTests TerminalAdapter.UnitTests ConAdapter.Unit.Tests - DynamicLibrary + DynamicLibrary @@ -21,6 +21,9 @@ + + {0cf235bd-2da0-407e-90ee-c467e8bbc714} + {06ec74cb-9a12-429c-b551-8562ec954746} From 3c96482d1e06556b96e3088bc88e841eb74904b2 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sun, 19 Apr 2020 11:22:28 +0100 Subject: [PATCH 07/10] Update adapter tests to account for the changes to the ConGetSet API. --- .../adapter/ut_adapter/adapterTest.cpp | 345 +++++------------- 1 file changed, 91 insertions(+), 254 deletions(-) diff --git a/src/terminal/adapter/ut_adapter/adapterTest.cpp b/src/terminal/adapter/ut_adapter/adapterTest.cpp index fe412754a2c..db39e8eb131 100644 --- a/src/terminal/adapter/ut_adapter/adapterTest.cpp +++ b/src/terminal/adapter/ut_adapter/adapterTest.cpp @@ -67,7 +67,7 @@ class TestGetSet final : public ConGetSet sbiex.dwSize = _bufferSize; sbiex.srWindow = _viewport; sbiex.dwCursorPosition = _cursorPos; - sbiex.wAttributes = _attribute; + sbiex.wAttributes = _attribute.GetLegacyAttributes(); } return _getConsoleScreenBufferInfoExResult; @@ -200,17 +200,9 @@ class TestGetSet final : public ConGetSet return _privateAllowCursorBlinkingResult; } - bool SetConsoleTextAttribute(const WORD attr) override + bool SetConsoleTextAttribute(const WORD /*attr*/) override { Log::Comment(L"SetConsoleTextAttribute MOCK called..."); - - if (_setConsoleTextAttributeResult) - { - VERIFY_ARE_EQUAL(_expectedAttribute, attr); - _attribute = attr; - _usingRgbColor = false; - } - return _setConsoleTextAttributeResult; } @@ -219,86 +211,27 @@ class TestGetSet final : public ConGetSet return false; } - bool PrivateSetLegacyAttributes(const WORD attr, const bool foreground, const bool background, const bool meta) override + bool PrivateSetLegacyAttributes(const WORD /*attr*/, const bool /*foreground*/, const bool /*background*/, const bool /*meta*/) override { Log::Comment(L"PrivateSetLegacyAttributes MOCK called..."); - if (_privateSetLegacyAttributesResult) - { - VERIFY_ARE_EQUAL(_expectedForeground, foreground); - VERIFY_ARE_EQUAL(_expectedBackground, background); - VERIFY_ARE_EQUAL(_expectedMeta, meta); - if (foreground) - { - WI_UpdateFlagsInMask(_attribute, FG_ATTRS, attr); - } - if (background) - { - WI_UpdateFlagsInMask(_attribute, BG_ATTRS, attr); - } - if (meta) - { - WI_UpdateFlagsInMask(_attribute, META_ATTRS, attr); - } - - VERIFY_ARE_EQUAL(_expectedAttribute, attr); - - _expectedForeground = _expectedBackground = _expectedMeta = false; - } - return _privateSetLegacyAttributesResult; } - bool SetConsoleXtermTextAttribute(const int xtermTableEntry, const bool isForeground) override + bool SetConsoleXtermTextAttribute(const int /*xtermTableEntry*/, const bool /*isForeground*/) override { Log::Comment(L"SetConsoleXtermTextAttribute MOCK called..."); - - if (_setConsoleXtermTextAttributeResult) - { - VERIFY_ARE_EQUAL(_expectedIsForeground, isForeground); - _isForeground = isForeground; - VERIFY_ARE_EQUAL(_iExpectedXtermTableEntry, xtermTableEntry); - _xtermTableEntry = xtermTableEntry; - // if the table entry is less than 16, keep using the legacy attr - _usingRgbColor = xtermTableEntry > 16; - if (!_usingRgbColor) - { - //Convert the xterm index to the win index - const auto red = (xtermTableEntry & 0x01) > 0; - const auto green = (xtermTableEntry & 0x02) > 0; - const auto blue = (xtermTableEntry & 0x04) > 0; - const auto bright = (xtermTableEntry & 0x08) > 0; - WORD winEntry = (red ? 0x4 : 0x0) | (green ? 0x2 : 0x0) | (blue ? 0x1 : 0x0) | (bright ? 0x8 : 0x0); - _attribute = isForeground ? ((_attribute & 0xF0) | winEntry) : ((winEntry << 4) | (_attribute & 0x0F)); - } - } - return _setConsoleXtermTextAttributeResult; } - bool SetConsoleRGBTextAttribute(const COLORREF rgbColor, const bool isForeground) override + bool SetConsoleRGBTextAttribute(const COLORREF /*rgbColor*/, const bool /*isForeground*/) override { Log::Comment(L"SetConsoleRGBTextAttribute MOCK called..."); - if (_setConsoleRGBTextAttributeResult) - { - VERIFY_ARE_EQUAL(_expectedIsForeground, isForeground); - _isForeground = isForeground; - VERIFY_ARE_EQUAL(_expectedColor, rgbColor); - _rgbColor = rgbColor; - _usingRgbColor = true; - } - return _setConsoleRGBTextAttributeResult; } - bool PrivateBoldText(const bool isBold) override + bool PrivateBoldText(const bool /*isBold*/) override { Log::Comment(L"PrivateBoldText MOCK called..."); - if (_privateBoldTextResult) - { - VERIFY_ARE_EQUAL(_expectedIsBold, isBold); - _isBold = isBold; - _expectedIsBold = false; - } return !!_privateBoldTextResult; } @@ -314,16 +247,29 @@ class TestGetSet final : public ConGetSet return true; } - bool PrivateGetTextAttributes(TextAttribute& /*attrs*/) const + bool PrivateGetTextAttributes(TextAttribute& attrs) const { Log::Comment(L"PrivateGetTextAttributes MOCK called..."); - return true; + + if (_privateGetTextAttributesResult) + { + attrs = _attribute; + } + + return _privateGetTextAttributesResult; } - bool PrivateSetTextAttributes(const TextAttribute& /*attrs*/) + bool PrivateSetTextAttributes(const TextAttribute& attrs) { Log::Comment(L"PrivateSetTextAttributes MOCK called..."); - return true; + + if (_privateSetTextAttributesResult) + { + VERIFY_ARE_EQUAL(_expectedAttribute, attrs); + _attribute = attrs; + } + + return _privateSetTextAttributesResult; } bool PrivateWriteConsoleInputW(std::deque>& events, @@ -531,15 +477,9 @@ class TestGetSet final : public ConGetSet return _setCursorColorResult; } - bool PrivateGetConsoleScreenBufferAttributes(WORD& attributes) override + bool PrivateGetConsoleScreenBufferAttributes(WORD& /*attributes*/) override { Log::Comment(L"PrivateGetConsoleScreenBufferAttributes MOCK returning data..."); - - if (_privateGetConsoleScreenBufferAttributesResult) - { - attributes = _attribute; - } - return _privateGetConsoleScreenBufferAttributesResult; } @@ -585,25 +525,10 @@ class TestGetSet final : public ConGetSet return TRUE; } - bool PrivateSetDefaultAttributes(const bool foreground, - const bool background) override + bool PrivateSetDefaultAttributes(const bool /*foreground*/, + const bool /*background*/) override { Log::Comment(L"PrivateSetDefaultAttributes MOCK called..."); - if (_privateSetDefaultAttributesResult) - { - VERIFY_ARE_EQUAL(_expectedForeground, foreground); - VERIFY_ARE_EQUAL(_expectedBackground, background); - if (foreground) - { - WI_UpdateFlagsInMask(_attribute, FG_ATTRS, s_defaultFill); - } - if (background) - { - WI_UpdateFlagsInMask(_attribute, BG_ATTRS, s_defaultFill); - } - - _expectedForeground = _expectedBackground = false; - } return _privateSetDefaultAttributesResult; } @@ -613,9 +538,18 @@ class TestGetSet final : public ConGetSet return _moveToBottomResult; } - bool PrivateGetColorTableEntry(const size_t /*index*/, COLORREF& /*value*/) const noexcept override + bool PrivateGetColorTableEntry(const size_t index, COLORREF& value) const noexcept override { Log::Comment(L"PrivateGetColorTableEntry MOCK called..."); + + if (_privateGetColorTableEntryResult) + { + VERIFY_ARE_EQUAL(_expectedColorTableIndex, index); + // Simply returning the index as the color value makes it easy for + // tests to confirm that they've received the color they expected. + value = gsl::narrow_cast(index); + } + return _privateGetColorTableEntryResult; } @@ -707,6 +641,8 @@ class TestGetSet final : public ConGetSet _getConsoleCursorInfoResult = TRUE; _setConsoleCursorInfoResult = TRUE; _setConsoleTextAttributeResult = TRUE; + _privateGetTextAttributesResult = TRUE; + _privateSetTextAttributesResult = TRUE; _privateWriteConsoleInputWResult = TRUE; _privatePrependConsoleInputResult = TRUE; _privateWriteConsoleControlInputResult = TRUE; @@ -836,23 +772,11 @@ class TestGetSet final : public ConGetSet DWORD _expectedCursorSize = 0; bool _expectedCursorVisible = false; - WORD _attribute = 0; - WORD _expectedAttribute = 0; - int _xtermTableEntry = 0; - int _iExpectedXtermTableEntry = 0; - COLORREF _rgbColor = 0; - COLORREF _expectedColor = 0; - bool _isForeground = false; - bool _expectedIsForeground = false; - bool _usingRgbColor = false; - bool _expectedForeground = false; - bool _expectedBackground = false; - bool _expectedMeta = false; + TextAttribute _attribute = {}; + TextAttribute _expectedAttribute = {}; unsigned int _expectedOutputCP = 0; bool _isPty = false; bool _privateBoldTextResult = false; - bool _expectedIsBold = false; - bool _isBold = false; bool _privateShowCursorResult = false; bool _expectedShowCursor = false; @@ -862,6 +786,8 @@ class TestGetSet final : public ConGetSet bool _getConsoleCursorInfoResult = false; bool _setConsoleCursorInfoResult = false; bool _setConsoleTextAttributeResult = false; + bool _privateGetTextAttributesResult = false; + bool _privateSetTextAttributesResult = false; bool _privateWriteConsoleInputWResult = false; bool _privatePrependConsoleInputResult = false; bool _privateWriteConsoleControlInputResult = false; @@ -1289,6 +1215,9 @@ class AdapterTest _testGetSet->PrepData(CursorX::XCENTER, CursorY::YCENTER); _testGetSet->_expectedCursorPos = coordExpected; + // Attributes are restored to defaults. + _testGetSet->_expectedAttribute = {}; + VERIFY_IS_TRUE(_pDispatch.get()->CursorRestoreState(), L"By default, restore to top left corner (0,0 offset from viewport)."); Log::Comment(L"Test 2: Place cursor in center. Save. Move cursor to corner. Restore. Should come back to center."); @@ -1349,17 +1278,17 @@ class AdapterTest VERIFY_IS_TRUE(_pDispatch.get()->SetGraphicsRendition({ rgOptions, cOptions })); - Log::Comment(L"Test 2: Gracefully fail when getting buffer information fails."); + Log::Comment(L"Test 2: Gracefully fail when getting attribute data fails."); _testGetSet->PrepData(); - _testGetSet->_privateGetConsoleScreenBufferAttributesResult = FALSE; + _testGetSet->_privateGetTextAttributesResult = FALSE; VERIFY_IS_FALSE(_pDispatch.get()->SetGraphicsRendition({ rgOptions, cOptions })); Log::Comment(L"Test 3: Gracefully fail when setting attribute data fails."); _testGetSet->PrepData(); - _testGetSet->_setConsoleTextAttributeResult = FALSE; + _testGetSet->_privateSetTextAttributesResult = FALSE; // Need at least one option in order for the call to be able to fail. rgOptions[0] = (DispatchTypes::GraphicsOptions)0; cOptions = 1; @@ -1385,265 +1314,212 @@ class AdapterTest size_t cOptions = 1; rgOptions[0] = graphicsOption; - _testGetSet->_privateSetLegacyAttributesResult = TRUE; - switch (graphicsOption) { case DispatchTypes::GraphicsOptions::Off: Log::Comment(L"Testing graphics 'Off/Reset'"); _testGetSet->_attribute = (WORD)~_testGetSet->s_defaultFill; - _testGetSet->_expectedAttribute = 0; - _testGetSet->_privateSetDefaultAttributesResult = true; - _testGetSet->_expectedForeground = true; - _testGetSet->_expectedBackground = true; - _testGetSet->_expectedMeta = true; - _testGetSet->_privateBoldTextResult = true; - _testGetSet->_expectedIsBold = false; - + _testGetSet->_expectedAttribute = {}; break; case DispatchTypes::GraphicsOptions::BoldBright: Log::Comment(L"Testing graphics 'Bold/Bright'"); _testGetSet->_attribute = 0; - _testGetSet->_expectedAttribute = FOREGROUND_INTENSITY; - _testGetSet->_expectedForeground = true; - _testGetSet->_privateBoldTextResult = true; - _testGetSet->_expectedIsBold = true; + _testGetSet->_expectedAttribute = 0; + _testGetSet->_expectedAttribute.SetBold(true); break; case DispatchTypes::GraphicsOptions::Underline: Log::Comment(L"Testing graphics 'Underline'"); _testGetSet->_attribute = 0; _testGetSet->_expectedAttribute = COMMON_LVB_UNDERSCORE; - _testGetSet->_expectedMeta = true; break; case DispatchTypes::GraphicsOptions::Negative: Log::Comment(L"Testing graphics 'Negative'"); _testGetSet->_attribute = 0; _testGetSet->_expectedAttribute = COMMON_LVB_REVERSE_VIDEO; - _testGetSet->_expectedMeta = true; break; case DispatchTypes::GraphicsOptions::NoUnderline: Log::Comment(L"Testing graphics 'No Underline'"); _testGetSet->_attribute = COMMON_LVB_UNDERSCORE; _testGetSet->_expectedAttribute = 0; - _testGetSet->_expectedMeta = true; break; case DispatchTypes::GraphicsOptions::Positive: Log::Comment(L"Testing graphics 'Positive'"); _testGetSet->_attribute = COMMON_LVB_REVERSE_VIDEO; _testGetSet->_expectedAttribute = 0; - _testGetSet->_expectedMeta = true; break; case DispatchTypes::GraphicsOptions::ForegroundBlack: Log::Comment(L"Testing graphics 'Foreground Color Black'"); _testGetSet->_attribute = FOREGROUND_RED | FOREGROUND_GREEN | FOREGROUND_BLUE | FOREGROUND_INTENSITY; _testGetSet->_expectedAttribute = 0; - _testGetSet->_expectedForeground = true; break; case DispatchTypes::GraphicsOptions::ForegroundBlue: Log::Comment(L"Testing graphics 'Foreground Color Blue'"); _testGetSet->_attribute = FOREGROUND_RED | FOREGROUND_GREEN | FOREGROUND_INTENSITY; _testGetSet->_expectedAttribute = FOREGROUND_BLUE; - _testGetSet->_expectedForeground = true; break; case DispatchTypes::GraphicsOptions::ForegroundGreen: Log::Comment(L"Testing graphics 'Foreground Color Green'"); _testGetSet->_attribute = FOREGROUND_RED | FOREGROUND_BLUE | FOREGROUND_INTENSITY; _testGetSet->_expectedAttribute = FOREGROUND_GREEN; - _testGetSet->_expectedForeground = true; break; case DispatchTypes::GraphicsOptions::ForegroundCyan: Log::Comment(L"Testing graphics 'Foreground Color Cyan'"); _testGetSet->_attribute = FOREGROUND_RED | FOREGROUND_INTENSITY; _testGetSet->_expectedAttribute = FOREGROUND_BLUE | FOREGROUND_GREEN; - _testGetSet->_expectedForeground = true; break; case DispatchTypes::GraphicsOptions::ForegroundRed: Log::Comment(L"Testing graphics 'Foreground Color Red'"); _testGetSet->_attribute = FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_INTENSITY; _testGetSet->_expectedAttribute = FOREGROUND_RED; - _testGetSet->_expectedForeground = true; break; case DispatchTypes::GraphicsOptions::ForegroundMagenta: Log::Comment(L"Testing graphics 'Foreground Color Magenta'"); _testGetSet->_attribute = FOREGROUND_GREEN | FOREGROUND_INTENSITY; _testGetSet->_expectedAttribute = FOREGROUND_BLUE | FOREGROUND_RED; - _testGetSet->_expectedForeground = true; break; case DispatchTypes::GraphicsOptions::ForegroundYellow: Log::Comment(L"Testing graphics 'Foreground Color Yellow'"); _testGetSet->_attribute = FOREGROUND_BLUE | FOREGROUND_INTENSITY; _testGetSet->_expectedAttribute = FOREGROUND_GREEN | FOREGROUND_RED; - _testGetSet->_expectedForeground = true; break; case DispatchTypes::GraphicsOptions::ForegroundWhite: Log::Comment(L"Testing graphics 'Foreground Color White'"); _testGetSet->_attribute = FOREGROUND_INTENSITY; _testGetSet->_expectedAttribute = FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED; - _testGetSet->_expectedForeground = true; break; case DispatchTypes::GraphicsOptions::ForegroundDefault: Log::Comment(L"Testing graphics 'Foreground Color Default'"); - _testGetSet->_privateSetDefaultAttributesResult = true; _testGetSet->_attribute = (WORD)~_testGetSet->s_wDefaultAttribute; // set the current attribute to the opposite of default so we can ensure all relevant bits flip. // To get expected value, take what we started with and change ONLY the background series of bits to what the Default says. _testGetSet->_expectedAttribute = _testGetSet->_attribute; // expect = starting - _testGetSet->_expectedAttribute &= ~(FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED | FOREGROUND_INTENSITY); // turn off all bits related to the background - _testGetSet->_expectedAttribute |= (_testGetSet->s_defaultFill & (FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED | FOREGROUND_INTENSITY)); // reapply ONLY background bits from the default attribute. - _testGetSet->_expectedForeground = true; + _testGetSet->_expectedAttribute.SetDefaultForeground(); // set the foreground as default break; case DispatchTypes::GraphicsOptions::BackgroundBlack: Log::Comment(L"Testing graphics 'Background Color Black'"); _testGetSet->_attribute = BACKGROUND_RED | BACKGROUND_GREEN | BACKGROUND_BLUE | BACKGROUND_INTENSITY; _testGetSet->_expectedAttribute = 0; - _testGetSet->_expectedBackground = true; break; case DispatchTypes::GraphicsOptions::BackgroundBlue: Log::Comment(L"Testing graphics 'Background Color Blue'"); _testGetSet->_attribute = BACKGROUND_RED | BACKGROUND_GREEN | BACKGROUND_INTENSITY; _testGetSet->_expectedAttribute = BACKGROUND_BLUE; - _testGetSet->_expectedBackground = true; break; case DispatchTypes::GraphicsOptions::BackgroundGreen: Log::Comment(L"Testing graphics 'Background Color Green'"); _testGetSet->_attribute = BACKGROUND_RED | BACKGROUND_BLUE | BACKGROUND_INTENSITY; _testGetSet->_expectedAttribute = BACKGROUND_GREEN; - _testGetSet->_expectedBackground = true; break; case DispatchTypes::GraphicsOptions::BackgroundCyan: Log::Comment(L"Testing graphics 'Background Color Cyan'"); _testGetSet->_attribute = BACKGROUND_RED | BACKGROUND_INTENSITY; _testGetSet->_expectedAttribute = BACKGROUND_BLUE | BACKGROUND_GREEN; - _testGetSet->_expectedBackground = true; break; case DispatchTypes::GraphicsOptions::BackgroundRed: Log::Comment(L"Testing graphics 'Background Color Red'"); _testGetSet->_attribute = BACKGROUND_BLUE | BACKGROUND_GREEN | BACKGROUND_INTENSITY; _testGetSet->_expectedAttribute = BACKGROUND_RED; - _testGetSet->_expectedBackground = true; break; case DispatchTypes::GraphicsOptions::BackgroundMagenta: Log::Comment(L"Testing graphics 'Background Color Magenta'"); _testGetSet->_attribute = BACKGROUND_GREEN | BACKGROUND_INTENSITY; _testGetSet->_expectedAttribute = BACKGROUND_BLUE | BACKGROUND_RED; - _testGetSet->_expectedBackground = true; break; case DispatchTypes::GraphicsOptions::BackgroundYellow: Log::Comment(L"Testing graphics 'Background Color Yellow'"); _testGetSet->_attribute = BACKGROUND_BLUE | BACKGROUND_INTENSITY; _testGetSet->_expectedAttribute = BACKGROUND_GREEN | BACKGROUND_RED; - _testGetSet->_expectedBackground = true; break; case DispatchTypes::GraphicsOptions::BackgroundWhite: Log::Comment(L"Testing graphics 'Background Color White'"); _testGetSet->_attribute = BACKGROUND_INTENSITY; _testGetSet->_expectedAttribute = BACKGROUND_BLUE | BACKGROUND_GREEN | BACKGROUND_RED; - _testGetSet->_expectedBackground = true; break; case DispatchTypes::GraphicsOptions::BackgroundDefault: Log::Comment(L"Testing graphics 'Background Color Default'"); - _testGetSet->_privateSetDefaultAttributesResult = true; _testGetSet->_attribute = (WORD)~_testGetSet->s_wDefaultAttribute; // set the current attribute to the opposite of default so we can ensure all relevant bits flip. // To get expected value, take what we started with and change ONLY the background series of bits to what the Default says. _testGetSet->_expectedAttribute = _testGetSet->_attribute; // expect = starting - _testGetSet->_expectedAttribute &= ~(BACKGROUND_BLUE | BACKGROUND_GREEN | BACKGROUND_RED | BACKGROUND_INTENSITY); // turn off all bits related to the background - _testGetSet->_expectedAttribute |= (_testGetSet->s_defaultFill & (BACKGROUND_BLUE | BACKGROUND_GREEN | BACKGROUND_RED | BACKGROUND_INTENSITY)); // reapply ONLY background bits from the default attribute. - _testGetSet->_expectedBackground = true; + _testGetSet->_expectedAttribute.SetDefaultBackground(); // set the background as default break; case DispatchTypes::GraphicsOptions::BrightForegroundBlack: Log::Comment(L"Testing graphics 'Bright Foreground Color Black'"); _testGetSet->_attribute = FOREGROUND_RED | FOREGROUND_GREEN | FOREGROUND_BLUE; _testGetSet->_expectedAttribute = FOREGROUND_INTENSITY; - _testGetSet->_expectedForeground = true; break; case DispatchTypes::GraphicsOptions::BrightForegroundBlue: Log::Comment(L"Testing graphics 'Bright Foreground Color Blue'"); _testGetSet->_attribute = FOREGROUND_RED | FOREGROUND_GREEN; _testGetSet->_expectedAttribute = FOREGROUND_INTENSITY | FOREGROUND_BLUE; - _testGetSet->_expectedForeground = true; break; case DispatchTypes::GraphicsOptions::BrightForegroundGreen: Log::Comment(L"Testing graphics 'Bright Foreground Color Green'"); _testGetSet->_attribute = FOREGROUND_RED | FOREGROUND_BLUE; _testGetSet->_expectedAttribute = FOREGROUND_INTENSITY | FOREGROUND_GREEN; - _testGetSet->_expectedForeground = true; break; case DispatchTypes::GraphicsOptions::BrightForegroundCyan: Log::Comment(L"Testing graphics 'Bright Foreground Color Cyan'"); _testGetSet->_attribute = FOREGROUND_RED; _testGetSet->_expectedAttribute = FOREGROUND_INTENSITY | FOREGROUND_BLUE | FOREGROUND_GREEN; - _testGetSet->_expectedForeground = true; break; case DispatchTypes::GraphicsOptions::BrightForegroundRed: Log::Comment(L"Testing graphics 'Bright Foreground Color Red'"); _testGetSet->_attribute = FOREGROUND_BLUE | FOREGROUND_GREEN; _testGetSet->_expectedAttribute = FOREGROUND_INTENSITY | FOREGROUND_RED; - _testGetSet->_expectedForeground = true; break; case DispatchTypes::GraphicsOptions::BrightForegroundMagenta: Log::Comment(L"Testing graphics 'Bright Foreground Color Magenta'"); _testGetSet->_attribute = FOREGROUND_GREEN; _testGetSet->_expectedAttribute = FOREGROUND_INTENSITY | FOREGROUND_BLUE | FOREGROUND_RED; - _testGetSet->_expectedForeground = true; break; case DispatchTypes::GraphicsOptions::BrightForegroundYellow: Log::Comment(L"Testing graphics 'Bright Foreground Color Yellow'"); _testGetSet->_attribute = FOREGROUND_BLUE; _testGetSet->_expectedAttribute = FOREGROUND_INTENSITY | FOREGROUND_GREEN | FOREGROUND_RED; - _testGetSet->_expectedForeground = true; break; case DispatchTypes::GraphicsOptions::BrightForegroundWhite: Log::Comment(L"Testing graphics 'Bright Foreground Color White'"); _testGetSet->_attribute = 0; _testGetSet->_expectedAttribute = FOREGROUND_INTENSITY | FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED; - _testGetSet->_expectedForeground = true; break; case DispatchTypes::GraphicsOptions::BrightBackgroundBlack: Log::Comment(L"Testing graphics 'Bright Background Color Black'"); _testGetSet->_attribute = BACKGROUND_RED | BACKGROUND_GREEN | BACKGROUND_BLUE; _testGetSet->_expectedAttribute = BACKGROUND_INTENSITY; - _testGetSet->_expectedBackground = true; break; case DispatchTypes::GraphicsOptions::BrightBackgroundBlue: Log::Comment(L"Testing graphics 'Bright Background Color Blue'"); _testGetSet->_attribute = BACKGROUND_RED | BACKGROUND_GREEN; _testGetSet->_expectedAttribute = BACKGROUND_INTENSITY | BACKGROUND_BLUE; - _testGetSet->_expectedBackground = true; break; case DispatchTypes::GraphicsOptions::BrightBackgroundGreen: Log::Comment(L"Testing graphics 'Bright Background Color Green'"); _testGetSet->_attribute = BACKGROUND_RED | BACKGROUND_BLUE; _testGetSet->_expectedAttribute = BACKGROUND_INTENSITY | BACKGROUND_GREEN; - _testGetSet->_expectedBackground = true; break; case DispatchTypes::GraphicsOptions::BrightBackgroundCyan: Log::Comment(L"Testing graphics 'Bright Background Color Cyan'"); _testGetSet->_attribute = BACKGROUND_RED; _testGetSet->_expectedAttribute = BACKGROUND_INTENSITY | BACKGROUND_BLUE | BACKGROUND_GREEN; - _testGetSet->_expectedBackground = true; break; case DispatchTypes::GraphicsOptions::BrightBackgroundRed: Log::Comment(L"Testing graphics 'Bright Background Color Red'"); _testGetSet->_attribute = BACKGROUND_BLUE | BACKGROUND_GREEN; _testGetSet->_expectedAttribute = BACKGROUND_INTENSITY | BACKGROUND_RED; - _testGetSet->_expectedBackground = true; break; case DispatchTypes::GraphicsOptions::BrightBackgroundMagenta: Log::Comment(L"Testing graphics 'Bright Background Color Magenta'"); _testGetSet->_attribute = BACKGROUND_GREEN; _testGetSet->_expectedAttribute = BACKGROUND_INTENSITY | BACKGROUND_BLUE | BACKGROUND_RED; - _testGetSet->_expectedBackground = true; break; case DispatchTypes::GraphicsOptions::BrightBackgroundYellow: Log::Comment(L"Testing graphics 'Bright Background Color Yellow'"); _testGetSet->_attribute = BACKGROUND_BLUE; _testGetSet->_expectedAttribute = BACKGROUND_INTENSITY | BACKGROUND_GREEN | BACKGROUND_RED; - _testGetSet->_expectedBackground = true; break; case DispatchTypes::GraphicsOptions::BrightBackgroundWhite: Log::Comment(L"Testing graphics 'Bright Background Color White'"); _testGetSet->_attribute = 0; _testGetSet->_expectedAttribute = BACKGROUND_INTENSITY | BACKGROUND_BLUE | BACKGROUND_GREEN | BACKGROUND_RED; - _testGetSet->_expectedBackground = true; break; default: VERIFY_FAIL(L"Test not implemented yet!"); @@ -1659,121 +1535,89 @@ class AdapterTest _testGetSet->PrepData(); // default color from here is gray on black, FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED - _testGetSet->_privateSetLegacyAttributesResult = TRUE; - DispatchTypes::GraphicsOptions rgOptions[16]; size_t cOptions = 1; Log::Comment(L"Test 1: Basic brightness test"); Log::Comment(L"Resetting graphics options"); rgOptions[0] = DispatchTypes::GraphicsOptions::Off; - _testGetSet->_privateSetDefaultAttributesResult = true; - _testGetSet->_expectedAttribute = 0; - _testGetSet->_expectedForeground = true; - _testGetSet->_expectedBackground = true; - _testGetSet->_expectedMeta = true; - _testGetSet->_privateBoldTextResult = true; - _testGetSet->_expectedIsBold = false; + _testGetSet->_expectedAttribute = {}; VERIFY_IS_TRUE(_pDispatch.get()->SetGraphicsRendition({ rgOptions, cOptions })); Log::Comment(L"Testing graphics 'Foreground Color Blue'"); rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundBlue; - _testGetSet->_expectedAttribute = FOREGROUND_BLUE; - _testGetSet->_expectedForeground = true; + _testGetSet->_expectedAttribute.SetIndexedForeground(FOREGROUND_BLUE); VERIFY_IS_TRUE(_pDispatch.get()->SetGraphicsRendition({ rgOptions, cOptions })); Log::Comment(L"Enabling brightness"); rgOptions[0] = DispatchTypes::GraphicsOptions::BoldBright; - _testGetSet->_expectedAttribute = FOREGROUND_BLUE | FOREGROUND_INTENSITY; - _testGetSet->_expectedForeground = true; - _testGetSet->_privateBoldTextResult = true; - _testGetSet->_expectedIsBold = true; + _testGetSet->_expectedAttribute.SetBold(true); VERIFY_IS_TRUE(_pDispatch.get()->SetGraphicsRendition({ rgOptions, cOptions })); - VERIFY_IS_TRUE(_testGetSet->_isBold); + VERIFY_IS_TRUE(_testGetSet->_attribute.IsBold()); Log::Comment(L"Testing graphics 'Foreground Color Green, with brightness'"); rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundGreen; - _testGetSet->_expectedAttribute = FOREGROUND_GREEN; - _testGetSet->_expectedForeground = true; + _testGetSet->_expectedAttribute.SetIndexedForeground(FOREGROUND_GREEN); VERIFY_IS_TRUE(_pDispatch.get()->SetGraphicsRendition({ rgOptions, cOptions })); - VERIFY_IS_TRUE(WI_IsFlagSet(_testGetSet->_attribute, FOREGROUND_GREEN)); - VERIFY_IS_TRUE(_testGetSet->_isBold); + VERIFY_IS_TRUE(WI_IsFlagSet(_testGetSet->_attribute.GetLegacyAttributes(), FOREGROUND_GREEN)); + VERIFY_IS_TRUE(_testGetSet->_attribute.IsBold()); Log::Comment(L"Test 2: Disable brightness, use a bright color, next normal call remains not bright"); Log::Comment(L"Resetting graphics options"); rgOptions[0] = DispatchTypes::GraphicsOptions::Off; - _testGetSet->_privateSetDefaultAttributesResult = true; - _testGetSet->_expectedAttribute = 0; - _testGetSet->_expectedForeground = true; - _testGetSet->_expectedBackground = true; - _testGetSet->_expectedMeta = true; - _testGetSet->_privateBoldTextResult = true; - _testGetSet->_expectedIsBold = false; + _testGetSet->_expectedAttribute = {}; VERIFY_IS_TRUE(_pDispatch.get()->SetGraphicsRendition({ rgOptions, cOptions })); - VERIFY_IS_TRUE(WI_IsFlagClear(_testGetSet->_attribute, FOREGROUND_INTENSITY)); - VERIFY_IS_FALSE(_testGetSet->_isBold); + VERIFY_IS_TRUE(WI_IsFlagClear(_testGetSet->_attribute.GetLegacyAttributes(), FOREGROUND_INTENSITY)); + VERIFY_IS_FALSE(_testGetSet->_attribute.IsBold()); Log::Comment(L"Testing graphics 'Foreground Color Bright Blue'"); rgOptions[0] = DispatchTypes::GraphicsOptions::BrightForegroundBlue; - _testGetSet->_expectedAttribute = FOREGROUND_BLUE | FOREGROUND_INTENSITY; - _testGetSet->_expectedForeground = true; + _testGetSet->_expectedAttribute.SetIndexedForeground(FOREGROUND_BLUE | FOREGROUND_INTENSITY); VERIFY_IS_TRUE(_pDispatch.get()->SetGraphicsRendition({ rgOptions, cOptions })); - VERIFY_IS_FALSE(_testGetSet->_isBold); + VERIFY_IS_FALSE(_testGetSet->_attribute.IsBold()); Log::Comment(L"Testing graphics 'Foreground Color Blue', brightness of 9x series doesn't persist"); rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundBlue; - _testGetSet->_expectedAttribute = FOREGROUND_BLUE; - _testGetSet->_expectedForeground = true; + _testGetSet->_expectedAttribute.SetIndexedForeground(FOREGROUND_BLUE); VERIFY_IS_TRUE(_pDispatch.get()->SetGraphicsRendition({ rgOptions, cOptions })); - VERIFY_IS_FALSE(_testGetSet->_isBold); + VERIFY_IS_FALSE(_testGetSet->_attribute.IsBold()); Log::Comment(L"Test 3: Enable brightness, use a bright color, brightness persists to next normal call"); Log::Comment(L"Resetting graphics options"); rgOptions[0] = DispatchTypes::GraphicsOptions::Off; - _testGetSet->_privateSetDefaultAttributesResult = true; - _testGetSet->_expectedAttribute = 0; - _testGetSet->_expectedForeground = true; - _testGetSet->_expectedBackground = true; - _testGetSet->_expectedMeta = true; - _testGetSet->_privateBoldTextResult = true; - _testGetSet->_expectedIsBold = false; + _testGetSet->_expectedAttribute = {}; VERIFY_IS_TRUE(_pDispatch.get()->SetGraphicsRendition({ rgOptions, cOptions })); - VERIFY_IS_FALSE(_testGetSet->_isBold); + VERIFY_IS_FALSE(_testGetSet->_attribute.IsBold()); Log::Comment(L"Testing graphics 'Foreground Color Blue'"); rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundBlue; - _testGetSet->_expectedAttribute = FOREGROUND_BLUE; - _testGetSet->_expectedForeground = true; + _testGetSet->_expectedAttribute.SetIndexedForeground(FOREGROUND_BLUE); VERIFY_IS_TRUE(_pDispatch.get()->SetGraphicsRendition({ rgOptions, cOptions })); - VERIFY_IS_FALSE(_testGetSet->_isBold); + VERIFY_IS_FALSE(_testGetSet->_attribute.IsBold()); Log::Comment(L"Enabling brightness"); rgOptions[0] = DispatchTypes::GraphicsOptions::BoldBright; - _testGetSet->_privateBoldTextResult = true; - _testGetSet->_expectedIsBold = true; + _testGetSet->_expectedAttribute.SetBold(true); VERIFY_IS_TRUE(_pDispatch.get()->SetGraphicsRendition({ rgOptions, cOptions })); - VERIFY_IS_TRUE(_testGetSet->_isBold); + VERIFY_IS_TRUE(_testGetSet->_attribute.IsBold()); Log::Comment(L"Testing graphics 'Foreground Color Bright Blue'"); rgOptions[0] = DispatchTypes::GraphicsOptions::BrightForegroundBlue; - _testGetSet->_expectedAttribute = FOREGROUND_BLUE | FOREGROUND_INTENSITY; - _testGetSet->_expectedForeground = true; + _testGetSet->_expectedAttribute.SetIndexedForeground(FOREGROUND_BLUE | FOREGROUND_INTENSITY); VERIFY_IS_TRUE(_pDispatch.get()->SetGraphicsRendition({ rgOptions, cOptions })); - VERIFY_IS_TRUE(_testGetSet->_isBold); + VERIFY_IS_TRUE(_testGetSet->_attribute.IsBold()); Log::Comment(L"Testing graphics 'Foreground Color Blue, with brightness', brightness of 9x series doesn't affect brightness"); rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundBlue; - _testGetSet->_expectedAttribute = FOREGROUND_BLUE; - _testGetSet->_expectedForeground = true; + _testGetSet->_expectedAttribute.SetIndexedForeground(FOREGROUND_BLUE); VERIFY_IS_TRUE(_pDispatch.get()->SetGraphicsRendition({ rgOptions, cOptions })); - VERIFY_IS_TRUE(_testGetSet->_isBold); + VERIFY_IS_TRUE(_testGetSet->_attribute.IsBold()); Log::Comment(L"Testing graphics 'Foreground Color Green, with brightness'"); rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundGreen; - _testGetSet->_expectedAttribute = FOREGROUND_GREEN; - _testGetSet->_expectedForeground = true; + _testGetSet->_expectedAttribute.SetIndexedForeground(FOREGROUND_GREEN); VERIFY_IS_TRUE(_pDispatch.get()->SetGraphicsRendition({ rgOptions, cOptions })); - VERIFY_IS_TRUE(_testGetSet->_isBold); + VERIFY_IS_TRUE(_testGetSet->_attribute.IsBold()); } TEST_METHOD(DeviceStatusReportTests) @@ -2093,44 +1937,39 @@ class AdapterTest DispatchTypes::GraphicsOptions rgOptions[16]; size_t cOptions = 3; - _testGetSet->_setConsoleXtermTextAttributeResult = true; + _testGetSet->_privateGetColorTableEntryResult = true; + _testGetSet->_expectedAttribute = _testGetSet->_attribute; Log::Comment(L"Test 1: Change Foreground"); rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundExtended; rgOptions[1] = DispatchTypes::GraphicsOptions::BlinkOrXterm256Index; rgOptions[2] = (DispatchTypes::GraphicsOptions)2; // Green - _testGetSet->_expectedAttribute = FOREGROUND_GREEN; - _testGetSet->_iExpectedXtermTableEntry = 2; - _testGetSet->_expectedIsForeground = true; - _testGetSet->_usingRgbColor = false; + _testGetSet->_expectedAttribute.SetForeground(2); + _testGetSet->_expectedColorTableIndex = 2; VERIFY_IS_TRUE(_pDispatch.get()->SetGraphicsRendition({ rgOptions, cOptions })); Log::Comment(L"Test 2: Change Background"); rgOptions[0] = DispatchTypes::GraphicsOptions::BackgroundExtended; rgOptions[1] = DispatchTypes::GraphicsOptions::BlinkOrXterm256Index; rgOptions[2] = (DispatchTypes::GraphicsOptions)9; // Bright Red - _testGetSet->_expectedAttribute = FOREGROUND_GREEN | BACKGROUND_RED | BACKGROUND_INTENSITY; - _testGetSet->_iExpectedXtermTableEntry = 9; - _testGetSet->_expectedIsForeground = false; - _testGetSet->_usingRgbColor = false; + _testGetSet->_expectedAttribute.SetBackground(9); + _testGetSet->_expectedColorTableIndex = 9; VERIFY_IS_TRUE(_pDispatch.get()->SetGraphicsRendition({ rgOptions, cOptions })); Log::Comment(L"Test 3: Change Foreground to RGB color"); rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundExtended; rgOptions[1] = DispatchTypes::GraphicsOptions::BlinkOrXterm256Index; rgOptions[2] = (DispatchTypes::GraphicsOptions)42; // Arbitrary Color - _testGetSet->_iExpectedXtermTableEntry = 42; - _testGetSet->_expectedIsForeground = true; - _testGetSet->_usingRgbColor = true; + _testGetSet->_expectedColorTableIndex = 42; + _testGetSet->_expectedAttribute.SetForeground(42); VERIFY_IS_TRUE(_pDispatch.get()->SetGraphicsRendition({ rgOptions, cOptions })); Log::Comment(L"Test 4: Change Background to RGB color"); rgOptions[0] = DispatchTypes::GraphicsOptions::BackgroundExtended; rgOptions[1] = DispatchTypes::GraphicsOptions::BlinkOrXterm256Index; rgOptions[2] = (DispatchTypes::GraphicsOptions)142; // Arbitrary Color - _testGetSet->_iExpectedXtermTableEntry = 142; - _testGetSet->_expectedIsForeground = false; - _testGetSet->_usingRgbColor = true; + _testGetSet->_expectedColorTableIndex = 142; + _testGetSet->_expectedAttribute.SetBackground(142); VERIFY_IS_TRUE(_pDispatch.get()->SetGraphicsRendition({ rgOptions, cOptions })); Log::Comment(L"Test 5: Change Foreground to Legacy Attr while BG is RGB color"); @@ -2140,10 +1979,8 @@ class AdapterTest rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundExtended; rgOptions[1] = DispatchTypes::GraphicsOptions::BlinkOrXterm256Index; rgOptions[2] = (DispatchTypes::GraphicsOptions)9; // Bright Red - _testGetSet->_expectedAttribute = FOREGROUND_RED | FOREGROUND_INTENSITY | BACKGROUND_RED | BACKGROUND_INTENSITY; - _testGetSet->_iExpectedXtermTableEntry = 9; - _testGetSet->_expectedIsForeground = true; - _testGetSet->_usingRgbColor = false; + _testGetSet->_expectedAttribute.SetForeground(9); + _testGetSet->_expectedColorTableIndex = 9; VERIFY_IS_TRUE(_pDispatch.get()->SetGraphicsRendition({ rgOptions, cOptions })); } From ee517f676740456f3c1d94963b209ddd17028630 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sun, 19 Apr 2020 11:46:20 +0100 Subject: [PATCH 08/10] Remove all the APIs that aren't needed anymore. --- src/buffer/out/TextAttribute.cpp | 32 ----- src/buffer/out/TextAttribute.hpp | 8 -- src/host/getset.cpp | 127 ------------------ src/host/getset.h | 23 ---- src/host/outputStream.cpp | 118 ---------------- src/host/outputStream.hpp | 21 --- src/terminal/adapter/adaptDispatch.cpp | 8 +- src/terminal/adapter/conGetSet.hpp | 15 --- .../adapter/ut_adapter/adapterTest.cpp | 64 --------- 9 files changed, 6 insertions(+), 410 deletions(-) diff --git a/src/buffer/out/TextAttribute.cpp b/src/buffer/out/TextAttribute.cpp index 76a907713e8..aa66a3ed419 100644 --- a/src/buffer/out/TextAttribute.cpp +++ b/src/buffer/out/TextAttribute.cpp @@ -60,12 +60,6 @@ COLORREF TextAttribute::_GetRgbBackground(std::basic_string_view color return _background.GetColor(colorTable, defaultColor, false); } -void TextAttribute::SetMetaAttributes(const WORD wMeta) noexcept -{ - WI_UpdateFlagsInMask(_wAttrLegacy, META_ATTRS, wMeta); - WI_ClearAllFlags(_wAttrLegacy, COMMON_LVB_SBCSDBCS); -} - void TextAttribute::SetForeground(const COLORREF rgbForeground) noexcept { _foreground = TextColor(rgbForeground); @@ -76,27 +70,6 @@ void TextAttribute::SetBackground(const COLORREF rgbBackground) noexcept _background = TextColor(rgbBackground); } -void TextAttribute::SetLegacyAttributes(const WORD attrs, - const bool setForeground, - const bool setBackground, - const bool setMeta) noexcept -{ - if (setForeground) - { - const BYTE fgIndex = gsl::narrow_cast(attrs & FG_ATTRS); - _foreground = TextColor(fgIndex); - } - if (setBackground) - { - const BYTE bgIndex = gsl::narrow_cast(attrs & BG_ATTRS) >> 4; - _background = TextColor(bgIndex); - } - if (setMeta) - { - SetMetaAttributes(attrs); - } -} - void TextAttribute::SetIndexedForeground(const BYTE fgIndex) noexcept { _foreground = TextColor(fgIndex); @@ -236,11 +209,6 @@ ExtendedAttributes TextAttribute::GetExtendedAttributes() const noexcept return _extendedAttrs; } -void TextAttribute::SetExtendedAttributes(const ExtendedAttributes attrs) noexcept -{ - _extendedAttrs = attrs; -} - // Routine Description: // - swaps foreground and background color void TextAttribute::Invert() noexcept diff --git a/src/buffer/out/TextAttribute.hpp b/src/buffer/out/TextAttribute.hpp index 30c7a2b2738..edabdc45661 100644 --- a/src/buffer/out/TextAttribute.hpp +++ b/src/buffer/out/TextAttribute.hpp @@ -107,13 +107,6 @@ class TextAttribute final void SetLeftVerticalDisplayed(const bool isDisplayed) noexcept; void SetRightVerticalDisplayed(const bool isDisplayed) noexcept; - void SetLegacyAttributes(const WORD attrs, - const bool setForeground, - const bool setBackground, - const bool setMeta) noexcept; - - void SetMetaAttributes(const WORD wMeta) noexcept; - void Invert() noexcept; friend constexpr bool operator==(const TextAttribute& a, const TextAttribute& b) noexcept; @@ -141,7 +134,6 @@ class TextAttribute final void SetReverseVideo(bool isReversed) noexcept; ExtendedAttributes GetExtendedAttributes() const noexcept; - void SetExtendedAttributes(const ExtendedAttributes attrs) noexcept; void SetForeground(const COLORREF rgbForeground) noexcept; void SetBackground(const COLORREF rgbBackground) noexcept; diff --git a/src/host/getset.cpp b/src/host/getset.cpp index fd93962157f..b7af4741e99 100644 --- a/src/host/getset.cpp +++ b/src/host/getset.cpp @@ -919,116 +919,6 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont CATCH_RETURN(); } -void DoSrvPrivateSetLegacyAttributes(SCREEN_INFORMATION& screenInfo, - const WORD Attribute, - const bool fForeground, - const bool fBackground, - const bool fMeta) -{ - auto& buffer = screenInfo.GetActiveBuffer(); - const TextAttribute OldAttributes = buffer.GetAttributes(); - TextAttribute NewAttributes = OldAttributes; - - NewAttributes.SetLegacyAttributes(Attribute, fForeground, fBackground, fMeta); - - buffer.SetAttributes(NewAttributes); -} - -void DoSrvPrivateSetDefaultAttributes(SCREEN_INFORMATION& screenInfo, - const bool fForeground, - const bool fBackground) -{ - auto& buffer = screenInfo.GetActiveBuffer(); - TextAttribute NewAttributes = buffer.GetAttributes(); - if (fForeground) - { - NewAttributes.SetDefaultForeground(); - } - if (fBackground) - { - NewAttributes.SetDefaultBackground(); - } - buffer.SetAttributes(NewAttributes); -} - -void DoSrvPrivateSetConsoleXtermTextAttribute(SCREEN_INFORMATION& screenInfo, - const int iXtermTableEntry, - const bool fIsForeground) -{ - const CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); - auto& buffer = screenInfo.GetActiveBuffer(); - TextAttribute NewAttributes = buffer.GetAttributes(); - - COLORREF rgbColor; - if (iXtermTableEntry < COLOR_TABLE_SIZE) - { - //Convert the xterm index to the win index - WORD iWinEntry = ::XtermToWindowsIndex(iXtermTableEntry); - - rgbColor = gci.GetColorTableEntry(iWinEntry); - } - else - { - rgbColor = gci.GetColorTableEntry(iXtermTableEntry); - } - - NewAttributes.SetColor(rgbColor, fIsForeground); - - buffer.SetAttributes(NewAttributes); -} - -void DoSrvPrivateSetConsoleRGBTextAttribute(SCREEN_INFORMATION& screenInfo, - const COLORREF rgbColor, - const bool fIsForeground) -{ - auto& buffer = screenInfo.GetActiveBuffer(); - - TextAttribute NewAttributes = buffer.GetAttributes(); - NewAttributes.SetColor(rgbColor, fIsForeground); - buffer.SetAttributes(NewAttributes); -} - -void DoSrvPrivateBoldText(SCREEN_INFORMATION& screenInfo, const bool bolded) -{ - auto& buffer = screenInfo.GetActiveBuffer(); - auto attrs = buffer.GetAttributes(); - attrs.SetBold(bolded); - buffer.SetAttributes(attrs); -} - -// Method Description: -// - Retrieves the active ExtendedAttributes (italic, underline, etc.) of the -// given screen buffer. Text written to this buffer will be written with these -// attributes. -// Arguments: -// - screenInfo: The buffer to get the extended attrs from. -// Return Value: -// - the currently active ExtendedAttributes. -ExtendedAttributes DoSrvPrivateGetExtendedTextAttributes(SCREEN_INFORMATION& screenInfo) -{ - auto& buffer = screenInfo.GetActiveBuffer(); - auto attrs = buffer.GetAttributes(); - return attrs.GetExtendedAttributes(); -} - -// Method Description: -// - Sets the active ExtendedAttributes (italic, underline, etc.) of the given -// screen buffer. Text written to this buffer will be written with these -// attributes. -// Arguments: -// - screenInfo: The buffer to set the extended attrs for. -// - extendedAttrs: The new ExtendedAttributes to use -// Return Value: -// - -void DoSrvPrivateSetExtendedTextAttributes(SCREEN_INFORMATION& screenInfo, - const ExtendedAttributes extendedAttrs) -{ - auto& buffer = screenInfo.GetActiveBuffer(); - auto attrs = buffer.GetAttributes(); - attrs.SetExtendedAttributes(extendedAttrs); - buffer.SetAttributes(attrs); -} - // Routine Description: // - Sets the codepage used for translating text when calling A versions of functions affecting the output buffer. // Arguments: @@ -1651,23 +1541,6 @@ void DoSrvSetCursorColor(SCREEN_INFORMATION& screenInfo, screenInfo.GetActiveBuffer().GetTextBuffer().GetCursor().SetColor(cursorColor); } -// Routine Description: -// - A private API call to get only the default color attributes of the screen buffer. -// - This is used as a performance optimization by the VT adapter in SGR (Set Graphics Rendition) instead -// of calling for this information through the public API GetConsoleScreenBufferInfoEx which returns a lot -// of extra unnecessary data and takes a lot of extra processing time. -// Parameters -// - screenInfo - The screen buffer to retrieve default color attributes information from -// - attributes - Space that will receive color attributes data -// Return Value: -// - STATUS_SUCCESS if we succeeded or STATUS_INVALID_PARAMETER for bad params (nullptr). -[[nodiscard]] NTSTATUS DoSrvPrivateGetConsoleScreenBufferAttributes(const SCREEN_INFORMATION& screenInfo, WORD& attributes) -{ - attributes = screenInfo.GetActiveBuffer().GetAttributes().GetLegacyAttributes(); - - return STATUS_SUCCESS; -} - // Routine Description: // - A private API call for forcing the renderer to repaint the screen. If the // input screen buffer is not the active one, then just do nothing. We only diff --git a/src/host/getset.h b/src/host/getset.h index 1fad9018e80..6d1f65d2f8a 100644 --- a/src/host/getset.h +++ b/src/host/getset.h @@ -18,14 +18,6 @@ Revision History: #include "../inc/conattrs.hpp" class SCREEN_INFORMATION; -void DoSrvPrivateSetLegacyAttributes(SCREEN_INFORMATION& screenInfo, - const WORD Attribute, - const bool fForeground, - const bool fBackground, - const bool fMeta); - -void DoSrvPrivateSetDefaultAttributes(SCREEN_INFORMATION& screenInfo, const bool fForeground, const bool fBackground); - [[nodiscard]] NTSTATUS DoSrvPrivateSetCursorKeysMode(_In_ bool fApplicationMode); [[nodiscard]] NTSTATUS DoSrvPrivateSetKeypadMode(_In_ bool fApplicationMode); @@ -49,18 +41,6 @@ void DoSrvPrivateEnableButtonEventMouseMode(const bool fEnable); void DoSrvPrivateEnableAnyEventMouseMode(const bool fEnable); void DoSrvPrivateEnableAlternateScroll(const bool fEnable); -void DoSrvPrivateSetConsoleXtermTextAttribute(SCREEN_INFORMATION& screenInfo, - const int iXtermTableEntry, - const bool fIsForeground); -void DoSrvPrivateSetConsoleRGBTextAttribute(SCREEN_INFORMATION& screenInfo, - const COLORREF rgbColor, - const bool fIsForeground); - -void DoSrvPrivateBoldText(SCREEN_INFORMATION& screenInfo, const bool bolded); - -ExtendedAttributes DoSrvPrivateGetExtendedTextAttributes(SCREEN_INFORMATION& screenInfo); -void DoSrvPrivateSetExtendedTextAttributes(SCREEN_INFORMATION& screenInfo, const ExtendedAttributes attrs); - [[nodiscard]] HRESULT DoSrvPrivateEraseAll(SCREEN_INFORMATION& screenInfo); void DoSrvSetCursorStyle(SCREEN_INFORMATION& screenInfo, @@ -68,9 +48,6 @@ void DoSrvSetCursorStyle(SCREEN_INFORMATION& screenInfo, void DoSrvSetCursorColor(SCREEN_INFORMATION& screenInfo, const COLORREF cursorColor); -[[nodiscard]] NTSTATUS DoSrvPrivateGetConsoleScreenBufferAttributes(const SCREEN_INFORMATION& screenInfo, - WORD& attributes); - void DoSrvPrivateRefreshWindow(const SCREEN_INFORMATION& screenInfo); void DoSrvGetConsoleOutputCodePage(unsigned int& codepage); diff --git a/src/host/outputStream.cpp b/src/host/outputStream.cpp index c6b25064ff3..d3b40ffd120 100644 --- a/src/host/outputStream.cpp +++ b/src/host/outputStream.cpp @@ -157,112 +157,6 @@ bool ConhostInternalGetSet::SetConsoleCursorInfo(const CONSOLE_CURSOR_INFO& curs return SUCCEEDED(ServiceLocator::LocateGlobals().api.SetConsoleCursorInfoImpl(_io.GetActiveOutputBuffer(), cursorInfo.dwSize, visible)); } -// Routine Description: -// - Connects the SetConsoleTextAttribute API call directly into our Driver Message servicing call inside Conhost.exe -// Sets BOTH the FG and the BG component of the attributes. -// Arguments: -// - attr - new color/graphical attributes to apply as default within the console text buffer -// Return Value: -// - true if successful (see DoSrvSetConsoleTextAttribute). false otherwise. -bool ConhostInternalGetSet::SetConsoleTextAttribute(const WORD attr) -{ - return SUCCEEDED(ServiceLocator::LocateGlobals().api.SetConsoleTextAttributeImpl(_io.GetActiveOutputBuffer(), attr)); -} - -// Routine Description: -// - Connects the PrivateSetDefaultAttributes API call directly into our Driver Message servicing call inside Conhost.exe -// Sets the FG and/or BG to the Default attributes values. -// Arguments: -// - foreground - Set the foreground to the default attributes -// - background - Set the background to the default attributes -// Return Value: -// - true if successful (see DoSrvPrivateSetDefaultAttributes). false otherwise. -bool ConhostInternalGetSet::PrivateSetDefaultAttributes(const bool foreground, - const bool background) -{ - DoSrvPrivateSetDefaultAttributes(_io.GetActiveOutputBuffer(), foreground, background); - return true; -} - -// Routine Description: -// - Connects the PrivateSetLegacyAttributes API call directly into our Driver Message servicing call inside Conhost.exe -// Sets only the components of the attributes requested with the foreground, background, and meta flags. -// Arguments: -// - attr - new color/graphical attributes to apply as default within the console text buffer -// - foreground - The new attributes contain an update to the foreground attributes -// - background - The new attributes contain an update to the background attributes -// - meta - The new attributes contain an update to the meta attributes -// Return Value: -// - true if successful (see DoSrvVtSetLegacyAttributes). false otherwise. -bool ConhostInternalGetSet::PrivateSetLegacyAttributes(const WORD attr, - const bool foreground, - const bool background, - const bool meta) -{ - DoSrvPrivateSetLegacyAttributes(_io.GetActiveOutputBuffer(), attr, foreground, background, meta); - return true; -} - -// Routine Description: -// - Sets the current attributes of the screen buffer to use the color table entry specified by -// the xtermTableEntry. Sets either the FG or the BG component of the attributes. -// Arguments: -// - xtermTableEntry - The entry of the xterm table to use. -// - isForeground - Whether or not the color applies to the foreground. -// Return Value: -// - true if successful (see DoSrvPrivateSetConsoleXtermTextAttribute). false otherwise. -bool ConhostInternalGetSet::SetConsoleXtermTextAttribute(const int xtermTableEntry, const bool isForeground) -{ - DoSrvPrivateSetConsoleXtermTextAttribute(_io.GetActiveOutputBuffer(), xtermTableEntry, isForeground); - return true; -} - -// Routine Description: -// - Sets the current attributes of the screen buffer to use the given rgb color. -// Sets either the FG or the BG component of the attributes. -// Arguments: -// - rgbColor - The rgb color to use. -// - isForeground - Whether or not the color applies to the foreground. -// Return Value: -// - true if successful (see DoSrvPrivateSetConsoleRGBTextAttribute). false otherwise. -bool ConhostInternalGetSet::SetConsoleRGBTextAttribute(const COLORREF rgbColor, const bool isForeground) -{ - DoSrvPrivateSetConsoleRGBTextAttribute(_io.GetActiveOutputBuffer(), rgbColor, isForeground); - return true; -} - -bool ConhostInternalGetSet::PrivateBoldText(const bool bolded) -{ - DoSrvPrivateBoldText(_io.GetActiveOutputBuffer(), bolded); - return true; -} - -// Method Description: -// - Retrieves the currently active ExtendedAttributes. See also -// DoSrvPrivateGetExtendedTextAttributes -// Arguments: -// - attrs: Receives the ExtendedAttributes value. -// Return Value: -// - true if successful (see DoSrvPrivateGetExtendedTextAttributes). false otherwise. -bool ConhostInternalGetSet::PrivateGetExtendedTextAttributes(ExtendedAttributes& attrs) -{ - attrs = DoSrvPrivateGetExtendedTextAttributes(_io.GetActiveOutputBuffer()); - return true; -} - -// Method Description: -// - Sets the active ExtendedAttributes of the active screen buffer. Text -// written to this buffer will be written with these attributes. -// Arguments: -// - extendedAttrs: The new ExtendedAttributes to use -// Return Value: -// - true if successful (see DoSrvPrivateSetExtendedTextAttributes). false otherwise. -bool ConhostInternalGetSet::PrivateSetExtendedTextAttributes(const ExtendedAttributes attrs) -{ - DoSrvPrivateSetExtendedTextAttributes(_io.GetActiveOutputBuffer(), attrs); - return true; -} - // Method Description: // - Retrieves the current TextAttribute of the active screen buffer. // Arguments: @@ -602,18 +496,6 @@ bool ConhostInternalGetSet::SetCursorStyle(const CursorType style) return true; } -// Routine Description: -// - Retrieves the default color attributes information for the active screen buffer. -// - This function is used to optimize SGR calls in lieu of calling GetConsoleScreenBufferInfoEx. -// Arguments: -// - pwAttributes - Pointer to space to receive color attributes data -// Return Value: -// - true if successful. false otherwise. -bool ConhostInternalGetSet::PrivateGetConsoleScreenBufferAttributes(WORD& attributes) -{ - return NT_SUCCESS(DoSrvPrivateGetConsoleScreenBufferAttributes(_io.GetActiveOutputBuffer(), attributes)); -} - // Routine Description: // - Connects the PrivatePrependConsoleInput API call directly into our Driver Message servicing call inside Conhost.exe // Arguments: diff --git a/src/host/outputStream.hpp b/src/host/outputStream.hpp index 30686e01abd..c39047bbdf7 100644 --- a/src/host/outputStream.hpp +++ b/src/host/outputStream.hpp @@ -62,25 +62,6 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal:: bool GetConsoleCursorInfo(CONSOLE_CURSOR_INFO& cursorInfo) const override; bool SetConsoleCursorInfo(const CONSOLE_CURSOR_INFO& cursorInfo) override; - bool SetConsoleTextAttribute(const WORD attr) override; - - bool PrivateSetLegacyAttributes(const WORD attr, - const bool foreground, - const bool background, - const bool meta) override; - - bool PrivateSetDefaultAttributes(const bool foreground, - const bool background) override; - - bool SetConsoleXtermTextAttribute(const int xtermTableEntry, - const bool isForeground) override; - - bool SetConsoleRGBTextAttribute(const COLORREF rgbColor, - const bool isForeground) override; - - bool PrivateBoldText(const bool bolded) override; - bool PrivateGetExtendedTextAttributes(ExtendedAttributes& attrs) override; - bool PrivateSetExtendedTextAttributes(const ExtendedAttributes attrs) override; bool PrivateGetTextAttributes(TextAttribute& attrs) const override; bool PrivateSetTextAttributes(const TextAttribute& attrs) override; @@ -121,8 +102,6 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal:: bool PrivateEnableAlternateScroll(const bool enabled) override; bool PrivateEraseAll() override; - bool PrivateGetConsoleScreenBufferAttributes(WORD& attributes) override; - bool PrivatePrependConsoleInput(std::deque>& events, size_t& eventsWritten) override; diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index 54c208caaf1..65a6fdcf453 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -1755,8 +1755,12 @@ bool AdaptDispatch::ScreenAlignmentPattern() const auto fillLength = (csbiex.srWindow.Bottom - csbiex.srWindow.Top) * csbiex.dwSize.X; success = _pConApi->PrivateFillRegion(fillPosition, fillLength, L'E', false); // Reset the meta/extended attributes (but leave the colors unchanged). - success = success && _pConApi->PrivateSetLegacyAttributes(0, false, false, true); - success = success && _pConApi->PrivateSetExtendedTextAttributes(ExtendedAttributes::Normal); + TextAttribute attr; + if (_pConApi->PrivateGetTextAttributes(attr)) + { + attr.SetStandardErase(); + success = success && _pConApi->PrivateSetTextAttributes(attr); + } // Reset the origin mode to absolute. success = success && SetOriginMode(false); // Clear the scrolling margins. diff --git a/src/terminal/adapter/conGetSet.hpp b/src/terminal/adapter/conGetSet.hpp index 2697f34d93e..36368c67448 100644 --- a/src/terminal/adapter/conGetSet.hpp +++ b/src/terminal/adapter/conGetSet.hpp @@ -33,23 +33,9 @@ namespace Microsoft::Console::VirtualTerminal virtual bool SetConsoleScreenBufferInfoEx(const CONSOLE_SCREEN_BUFFER_INFOEX& screenBufferInfo) = 0; virtual bool SetConsoleCursorInfo(const CONSOLE_CURSOR_INFO& cursorInfo) = 0; virtual bool SetConsoleCursorPosition(const COORD position) = 0; - virtual bool SetConsoleTextAttribute(const WORD attr) = 0; virtual bool PrivateIsVtInputEnabled() const = 0; - virtual bool PrivateSetLegacyAttributes(const WORD attr, - const bool foreground, - const bool background, - const bool meta) = 0; - - virtual bool PrivateSetDefaultAttributes(const bool foreground, const bool background) = 0; - - virtual bool SetConsoleXtermTextAttribute(const int xtermTableEntry, - const bool isForeground) = 0; - virtual bool SetConsoleRGBTextAttribute(const COLORREF rgbColor, const bool isForeground) = 0; - virtual bool PrivateBoldText(const bool bolded) = 0; - virtual bool PrivateGetExtendedTextAttributes(ExtendedAttributes& attrs) = 0; - virtual bool PrivateSetExtendedTextAttributes(const ExtendedAttributes attrs) = 0; virtual bool PrivateGetTextAttributes(TextAttribute& attrs) const = 0; virtual bool PrivateSetTextAttributes(const TextAttribute& attrs) = 0; @@ -84,7 +70,6 @@ namespace Microsoft::Console::VirtualTerminal virtual bool PrivateEraseAll() = 0; virtual bool SetCursorStyle(const CursorType style) = 0; virtual bool SetCursorColor(const COLORREF color) = 0; - virtual bool PrivateGetConsoleScreenBufferAttributes(WORD& attributes) = 0; virtual bool PrivatePrependConsoleInput(std::deque>& events, size_t& eventsWritten) = 0; virtual bool PrivateWriteConsoleControlInput(const KeyEvent key) = 0; diff --git a/src/terminal/adapter/ut_adapter/adapterTest.cpp b/src/terminal/adapter/ut_adapter/adapterTest.cpp index db39e8eb131..fbcbe3e6e47 100644 --- a/src/terminal/adapter/ut_adapter/adapterTest.cpp +++ b/src/terminal/adapter/ut_adapter/adapterTest.cpp @@ -200,53 +200,11 @@ class TestGetSet final : public ConGetSet return _privateAllowCursorBlinkingResult; } - bool SetConsoleTextAttribute(const WORD /*attr*/) override - { - Log::Comment(L"SetConsoleTextAttribute MOCK called..."); - return _setConsoleTextAttributeResult; - } - bool PrivateIsVtInputEnabled() const override { return false; } - bool PrivateSetLegacyAttributes(const WORD /*attr*/, const bool /*foreground*/, const bool /*background*/, const bool /*meta*/) override - { - Log::Comment(L"PrivateSetLegacyAttributes MOCK called..."); - return _privateSetLegacyAttributesResult; - } - - bool SetConsoleXtermTextAttribute(const int /*xtermTableEntry*/, const bool /*isForeground*/) override - { - Log::Comment(L"SetConsoleXtermTextAttribute MOCK called..."); - return _setConsoleXtermTextAttributeResult; - } - - bool SetConsoleRGBTextAttribute(const COLORREF /*rgbColor*/, const bool /*isForeground*/) override - { - Log::Comment(L"SetConsoleRGBTextAttribute MOCK called..."); - return _setConsoleRGBTextAttributeResult; - } - - bool PrivateBoldText(const bool /*isBold*/) override - { - Log::Comment(L"PrivateBoldText MOCK called..."); - return !!_privateBoldTextResult; - } - - bool PrivateGetExtendedTextAttributes(ExtendedAttributes& /*attrs*/) - { - Log::Comment(L"PrivateGetExtendedTextAttributes MOCK called..."); - return true; - } - - bool PrivateSetExtendedTextAttributes(const ExtendedAttributes /*attrs*/) - { - Log::Comment(L"PrivateSetExtendedTextAttributes MOCK called..."); - return true; - } - bool PrivateGetTextAttributes(TextAttribute& attrs) const { Log::Comment(L"PrivateGetTextAttributes MOCK called..."); @@ -477,12 +435,6 @@ class TestGetSet final : public ConGetSet return _setCursorColorResult; } - bool PrivateGetConsoleScreenBufferAttributes(WORD& /*attributes*/) override - { - Log::Comment(L"PrivateGetConsoleScreenBufferAttributes MOCK returning data..."); - return _privateGetConsoleScreenBufferAttributesResult; - } - bool PrivateRefreshWindow() override { Log::Comment(L"PrivateRefreshWindow MOCK called..."); @@ -525,13 +477,6 @@ class TestGetSet final : public ConGetSet return TRUE; } - bool PrivateSetDefaultAttributes(const bool /*foreground*/, - const bool /*background*/) override - { - Log::Comment(L"PrivateSetDefaultAttributes MOCK called..."); - return _privateSetDefaultAttributesResult; - } - bool MoveToBottom() const override { Log::Comment(L"MoveToBottom MOCK called..."); @@ -640,14 +585,12 @@ class TestGetSet final : public ConGetSet _getConsoleScreenBufferInfoExResult = TRUE; _getConsoleCursorInfoResult = TRUE; _setConsoleCursorInfoResult = TRUE; - _setConsoleTextAttributeResult = TRUE; _privateGetTextAttributesResult = TRUE; _privateSetTextAttributesResult = TRUE; _privateWriteConsoleInputWResult = TRUE; _privatePrependConsoleInputResult = TRUE; _privateWriteConsoleControlInputResult = TRUE; _setConsoleWindowInfoResult = TRUE; - _privateGetConsoleScreenBufferAttributesResult = TRUE; _moveToBottomResult = true; _bufferSize.X = 100; @@ -776,7 +719,6 @@ class TestGetSet final : public ConGetSet TextAttribute _expectedAttribute = {}; unsigned int _expectedOutputCP = 0; bool _isPty = false; - bool _privateBoldTextResult = false; bool _privateShowCursorResult = false; bool _expectedShowCursor = false; @@ -785,7 +727,6 @@ class TestGetSet final : public ConGetSet bool _setConsoleCursorPositionResult = false; bool _getConsoleCursorInfoResult = false; bool _setConsoleCursorInfoResult = false; - bool _setConsoleTextAttributeResult = false; bool _privateGetTextAttributesResult = false; bool _privateSetTextAttributesResult = false; bool _privateWriteConsoleInputWResult = false; @@ -821,16 +762,11 @@ class TestGetSet final : public ConGetSet bool _privateEnableButtonEventMouseModeResult = false; bool _privateEnableAnyEventMouseModeResult = false; bool _privateEnableAlternateScrollResult = false; - bool _setConsoleXtermTextAttributeResult = false; - bool _setConsoleRGBTextAttributeResult = false; - bool _privateSetLegacyAttributesResult = false; - bool _privateGetConsoleScreenBufferAttributesResult = false; bool _setCursorStyleResult = false; CursorType _expectedCursorStyle; bool _setCursorColorResult = false; COLORREF _expectedCursorColor = 0; bool _getConsoleOutputCPResult = false; - bool _privateSetDefaultAttributesResult = false; bool _moveToBottomResult = false; bool _privateGetColorTableEntryResult = false; From 1513de7607d75f067093cee2831770c7940c2a6e Mon Sep 17 00:00:00 2001 From: James Holderness Date: Thu, 7 May 2020 13:41:23 +0100 Subject: [PATCH 09/10] Add suggested comment changes and TODO note. --- src/terminal/adapter/adaptDispatchGraphics.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/terminal/adapter/adaptDispatchGraphics.cpp b/src/terminal/adapter/adaptDispatchGraphics.cpp index a648478d136..9e565ecf3e0 100644 --- a/src/terminal/adapter/adaptDispatchGraphics.cpp +++ b/src/terminal/adapter/adaptDispatchGraphics.cpp @@ -44,7 +44,7 @@ const BYTE BRIGHT_WHITE = BRIGHT_ATTR | RED_ATTR | GREEN_ATTR | BLUE_ATTR; // Arguments: // - options - An array of options that will be used to generate the RGB color // - attr - The attribute that will be updated with the parsed color. -// - isForeground - Whether or not the parsed color is for the foreground or not. +// - isForeground - Whether or not the parsed color is for the foreground. // Return Value: // - The number of options consumed, not including the initial 38/48. size_t AdaptDispatch::_SetRgbColorsHelper(const std::basic_string_view options, @@ -76,6 +76,7 @@ size_t AdaptDispatch::_SetRgbColorsHelper(const std::basic_string_viewPrivateGetColorTableEntry(tableIndex, rgbColor)) { + // TODO GH#1223: Decouple xterm-256color indexed storage from RGB storage attr.SetColor(rgbColor, isForeground); } } From 9252ea2ad48ff35c134a47ef8e0c8aef1df5edf0 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Thu, 7 May 2020 19:02:13 +0100 Subject: [PATCH 10/10] Add bitfield(s) and href to the apis dictionary. --- .github/actions/spell-check/dictionary/apis.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/actions/spell-check/dictionary/apis.txt b/.github/actions/spell-check/dictionary/apis.txt index c8248141b74..d760b18e880 100644 --- a/.github/actions/spell-check/dictionary/apis.txt +++ b/.github/actions/spell-check/dictionary/apis.txt @@ -1,3 +1,6 @@ +bitfield +bitfields +href ICustom IMap IObject