Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Add support for selective erase operations #14046

Merged
merged 6 commits into from
Oct 7, 2022
Merged

Add support for selective erase operations #14046

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f19599f
Add support for the DECSCA sequence.
j4james Sep 18, 2022
4587f4d
Report DECSCA state with DECRQSS.
j4james Sep 18, 2022
5b9f548
Add support for DECSED and DECSEL sequences.
j4james Sep 18, 2022
6d462be
Extend units tests to cover selective erasure.
j4james Sep 20, 2022
78ec87d
Add missing overrides.
j4james Sep 20, 2022
22b1296
Make the most of the til::rect bool operator.
j4james Sep 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions src/buffer/out/TextAttribute.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,11 @@ bool TextAttribute::IsReverseVideo() const noexcept
return WI_IsFlagSet(_attrs, CharacterAttributes::ReverseVideo);
}

bool TextAttribute::IsProtected() const noexcept
{
return WI_IsFlagSet(_attrs, CharacterAttributes::Protected);
}

void TextAttribute::SetIntense(bool isIntense) noexcept
{
WI_UpdateFlag(_attrs, CharacterAttributes::Intense, isIntense);
Expand Down Expand Up @@ -347,6 +352,11 @@ void TextAttribute::SetReverseVideo(bool isReversed) noexcept
WI_UpdateFlag(_attrs, CharacterAttributes::ReverseVideo, isReversed);
}

void TextAttribute::SetProtected(bool isProtected) noexcept
{
WI_UpdateFlag(_attrs, CharacterAttributes::Protected, isProtected);
}

// Routine Description:
// - swaps foreground and background color
void TextAttribute::Invert() noexcept
Expand All @@ -365,10 +375,11 @@ void TextAttribute::SetDefaultBackground() noexcept
}

// Method description:
// - Resets only the rendition character attributes
// - Resets only the rendition character attributes, which includes everything
// except the Protected attribute.
void TextAttribute::SetDefaultRenditionAttributes() noexcept
{
_attrs = CharacterAttributes::Normal;
_attrs &= CharacterAttributes::Protected;
Copy link
Member

Choose a reason for hiding this comment

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

Should we introduce a metaconstant for the rendition attributes? or should we just defer that until we have more non-rendition attributes... :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know. If we did, it would be everything but Protected, would seems like a pain to maintain. We could go the other way around and create a metaconstant for the "logical" attributes, but that would just be Protected which feels a bit pointless. I'd say defer it until have more non-rendition attributes, but realistically that'll probably be never.

}

// Method Description:
Expand Down
2 changes: 2 additions & 0 deletions src/buffer/out/TextAttribute.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ class TextAttribute final
bool IsDoublyUnderlined() const noexcept;
bool IsOverlined() const noexcept;
bool IsReverseVideo() const noexcept;
bool IsProtected() const noexcept;

void SetIntense(bool isIntense) noexcept;
void SetFaint(bool isFaint) noexcept;
Expand All @@ -101,6 +102,7 @@ class TextAttribute final
void SetDoublyUnderlined(bool isDoublyUnderlined) noexcept;
void SetOverlined(bool isOverlined) noexcept;
void SetReverseVideo(bool isReversed) noexcept;
void SetProtected(bool isProtected) noexcept;

constexpr CharacterAttributes GetCharacterAttributes() const noexcept
{
Expand Down
131 changes: 118 additions & 13 deletions src/host/ut_host/ScreenBufferTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ class ScreenBufferTests

TEST_METHOD(EraseScrollbackTests);
TEST_METHOD(EraseTests);
TEST_METHOD(ProtectedAttributeTests);

TEST_METHOD(ScrollUpInMargins);
TEST_METHOD(ScrollDownInMargins);
Expand Down Expand Up @@ -3634,18 +3635,24 @@ bool _ValidateLineContains(int line, T expectedContent, TextAttribute expectedAt
}

template<class T>
auto _ValidateLinesContain(int startLine, int endLine, T expectedContent, TextAttribute expectedAttr)
auto _ValidateLinesContain(int startCol, int startLine, int endLine, T expectedContent, TextAttribute expectedAttr)
{
for (auto line = startLine; line < endLine; ++line)
{
if (!_ValidateLineContains(line, expectedContent, expectedAttr))
if (!_ValidateLineContains({ startCol, line }, expectedContent, expectedAttr))
{
return false;
}
}
return true;
};

template<class T>
auto _ValidateLinesContain(int startLine, int endLine, T expectedContent, TextAttribute expectedAttr)
{
return _ValidateLinesContain(0, startLine, endLine, expectedContent, expectedAttr);
}

void ScreenBufferTests::ScrollOperations()
{
enum ScrollType : int
Expand Down Expand Up @@ -4196,18 +4203,27 @@ void ScreenBufferTests::EraseTests()
BEGIN_TEST_METHOD_PROPERTIES()
TEST_METHOD_PROPERTY(L"Data:eraseType", L"{0, 1, 2}") // corresponds to options in DispatchTypes::EraseType
TEST_METHOD_PROPERTY(L"Data:eraseScreen", L"{false, true}") // corresponds to Line (false) or Screen (true)
TEST_METHOD_PROPERTY(L"Data:selectiveErase", L"{false, true}")
END_TEST_METHOD_PROPERTIES()

int eraseTypeValue;
VERIFY_SUCCEEDED(TestData::TryGetValue(L"eraseType", eraseTypeValue));
bool eraseScreen;
VERIFY_SUCCEEDED(TestData::TryGetValue(L"eraseScreen", eraseScreen));
bool selectiveErase;
VERIFY_SUCCEEDED(TestData::TryGetValue(L"selectiveErase", selectiveErase));

const auto eraseType = gsl::narrow<DispatchTypes::EraseType>(eraseTypeValue);

std::wstringstream escapeSequence;
escapeSequence << "\x1b[";

if (selectiveErase)
{
Log::Comment(L"Erasing unprotected cells only.");
escapeSequence << "?";
}

switch (eraseType)
{
case DispatchTypes::EraseType::ToEnd:
Expand Down Expand Up @@ -4247,12 +4263,24 @@ void ScreenBufferTests::EraseTests()

// Move the viewport down a few lines, and only cover part of the buffer width.
si.SetViewport(Viewport::FromDimensions({ 5, 10 }, { bufferWidth - 10, 10 }), true);
const auto& viewport = si.GetViewport();

// Fill the entire buffer with Zs. Blue on Green.
const auto bufferChar = L'Z';
const auto bufferAttr = TextAttribute{ FOREGROUND_BLUE | BACKGROUND_GREEN };
_FillLines(0, bufferHeight, bufferChar, bufferAttr);

// For selective erasure tests, we protect the first five columns.
if (selectiveErase)
{
auto protectedAttr = bufferAttr;
protectedAttr.SetProtected(true);
for (auto line = viewport.Top(); line < viewport.BottomExclusive(); ++line)
{
_FillLine(line, L"ZZZZZ", protectedAttr);
}
}

// Set the attributes that will be used to fill the erased area.
auto fillAttr = TextAttribute{ RGB(12, 34, 56), RGB(78, 90, 12) };
fillAttr.SetCrossedOut(true);
Expand All @@ -4262,45 +4290,53 @@ void ScreenBufferTests::EraseTests()
// But note that the meta attributes are expected to be cleared.
auto expectedFillAttr = fillAttr;
expectedFillAttr.SetStandardErase();
// And with selective erasure the original attributes are maintained.
if (selectiveErase)
{
expectedFillAttr = bufferAttr;
}

// Place the cursor in the center.
const auto centerX = bufferWidth / 2;
const auto centerY = (si.GetViewport().Top() + si.GetViewport().BottomExclusive()) / 2;
const auto centerY = (viewport.Top() + viewport.BottomExclusive()) / 2;

Log::Comment(L"Set the cursor position and perform the operation.");
VERIFY_SUCCEEDED(si.SetCursorPosition({ centerX, centerY }, true));
stateMachine.ProcessString(escapeSequence.str());

// Get cursor position and viewport range.
const auto cursorPos = si.GetTextBuffer().GetCursor().GetPosition();
const auto viewportStart = si.GetViewport().Top();
const auto viewportEnd = si.GetViewport().BottomExclusive();
const auto viewportStart = viewport.Top();
const auto viewportEnd = viewport.BottomExclusive();

Log::Comment(L"Lines outside the viewport should remain unchanged.");
VERIFY_IS_TRUE(_ValidateLinesContain(0, viewportStart, bufferChar, bufferAttr));
VERIFY_IS_TRUE(_ValidateLinesContain(viewportEnd, bufferHeight, bufferChar, bufferAttr));

// With selective erasure, there's a protected range to account for at the start of the line.
const auto protectedOffset = selectiveErase ? 5 : 0;

// 1. Lines before cursor line
if (eraseScreen && eraseType != DispatchTypes::EraseType::ToEnd)
{
// For eraseScreen, if we're not erasing to the end, these rows will be cleared.
Log::Comment(L"Lines before the cursor line should be erased.");
VERIFY_IS_TRUE(_ValidateLinesContain(viewportStart, cursorPos.Y, L' ', expectedFillAttr));
VERIFY_IS_TRUE(_ValidateLinesContain(protectedOffset, viewportStart, cursorPos.Y, L' ', expectedFillAttr));
}
else
{
// Otherwise we'll be left with the original buffer content.
Log::Comment(L"Lines before the cursor line should remain unchanged.");
VERIFY_IS_TRUE(_ValidateLinesContain(viewportStart, cursorPos.Y, bufferChar, bufferAttr));
VERIFY_IS_TRUE(_ValidateLinesContain(protectedOffset, viewportStart, cursorPos.Y, bufferChar, bufferAttr));
}

// 2. Cursor Line
auto prefixPos = til::point{ 0, cursorPos.Y };
auto prefixPos = til::point{ protectedOffset, cursorPos.Y };
auto suffixPos = cursorPos;
// When erasing from the beginning, the cursor column is included in the range.
suffixPos.X += (eraseType == DispatchTypes::EraseType::FromBeginning);
size_t prefixWidth = suffixPos.X;
auto suffixWidth = bufferWidth - prefixWidth;
size_t prefixWidth = suffixPos.X - prefixPos.X;
size_t suffixWidth = bufferWidth - suffixPos.X;
if (eraseType == DispatchTypes::EraseType::ToEnd)
{
Log::Comment(L"The start of the cursor line should remain unchanged.");
Expand All @@ -4318,24 +4354,93 @@ void ScreenBufferTests::EraseTests()
if (eraseType == DispatchTypes::EraseType::All)
{
Log::Comment(L"The entire cursor line should be erased.");
VERIFY_IS_TRUE(_ValidateLineContains(cursorPos.Y, L' ', expectedFillAttr));
VERIFY_IS_TRUE(_ValidateLineContains(prefixPos, L' ', expectedFillAttr));
}

// 3. Lines after cursor line
if (eraseScreen && eraseType != DispatchTypes::EraseType::FromBeginning)
{
// For eraseScreen, if we're not erasing from the beginning, these rows will be cleared.
Log::Comment(L"Lines after the cursor line should be erased.");
VERIFY_IS_TRUE(_ValidateLinesContain(cursorPos.Y + 1, viewportEnd, L' ', expectedFillAttr));
VERIFY_IS_TRUE(_ValidateLinesContain(protectedOffset, cursorPos.Y + 1, viewportEnd, L' ', expectedFillAttr));
}
else
{
// Otherwise we'll be left with the original buffer content.
Log::Comment(L"Lines after the cursor line should remain unchanged.");
VERIFY_IS_TRUE(_ValidateLinesContain(cursorPos.Y + 1, viewportEnd, bufferChar, bufferAttr));
VERIFY_IS_TRUE(_ValidateLinesContain(protectedOffset, cursorPos.Y + 1, viewportEnd, bufferChar, bufferAttr));
}

// 4. Protected columns
if (selectiveErase)
{
Log::Comment(L"First five columns should remain unchanged.");
auto protectedAttr = bufferAttr;
protectedAttr.SetProtected(true);
for (auto line = viewport.Top(); line < viewport.BottomExclusive(); ++line)
{
VERIFY_IS_TRUE(_ValidateLineContains(line, L"ZZZZZ", protectedAttr));
}
}
}

void ScreenBufferTests::ProtectedAttributeTests()
{
auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
auto& si = gci.GetActiveOutputBuffer().GetActiveBuffer();
auto& textBuffer = si.GetTextBuffer();
auto& stateMachine = si.GetStateMachine();
WI_SetFlag(si.OutputMode, ENABLE_VIRTUAL_TERMINAL_PROCESSING);

const auto cursorPosition = textBuffer.GetCursor().GetPosition();
auto unprotectedAttribute = textBuffer.GetCurrentAttributes();
unprotectedAttribute.SetProtected(false);
auto protectedAttribute = textBuffer.GetCurrentAttributes();
protectedAttribute.SetProtected(true);

Log::Comment(L"DECSCA default should be unprotected");
textBuffer.GetCursor().SetPosition(cursorPosition);
textBuffer.SetCurrentAttributes(protectedAttribute);
stateMachine.ProcessString(L"\x1b[\"qZZZZZ");
VERIFY_IS_FALSE(textBuffer.GetCurrentAttributes().IsProtected());
VERIFY_IS_TRUE(_ValidateLineContains(cursorPosition, L"ZZZZZ", unprotectedAttribute));

Log::Comment(L"DECSCA 0 should be unprotected");
textBuffer.GetCursor().SetPosition(cursorPosition);
textBuffer.SetCurrentAttributes(protectedAttribute);
stateMachine.ProcessString(L"\x1b[0\"qZZZZZ");
VERIFY_IS_FALSE(textBuffer.GetCurrentAttributes().IsProtected());
VERIFY_IS_TRUE(_ValidateLineContains(cursorPosition, L"ZZZZZ", unprotectedAttribute));

Log::Comment(L"DECSCA 1 should be protected");
textBuffer.GetCursor().SetPosition(cursorPosition);
textBuffer.SetCurrentAttributes(unprotectedAttribute);
stateMachine.ProcessString(L"\x1b[1\"qZZZZZ");
VERIFY_IS_TRUE(textBuffer.GetCurrentAttributes().IsProtected());
VERIFY_IS_TRUE(_ValidateLineContains(cursorPosition, L"ZZZZZ", protectedAttribute));

Log::Comment(L"DECSCA 2 should be unprotected");
textBuffer.GetCursor().SetPosition(cursorPosition);
textBuffer.SetCurrentAttributes(protectedAttribute);
stateMachine.ProcessString(L"\x1b[2\"qZZZZZ");
VERIFY_IS_FALSE(textBuffer.GetCurrentAttributes().IsProtected());
VERIFY_IS_TRUE(_ValidateLineContains(cursorPosition, L"ZZZZZ", unprotectedAttribute));

Log::Comment(L"DECSCA 2;1 should be protected");
textBuffer.GetCursor().SetPosition(cursorPosition);
textBuffer.SetCurrentAttributes(unprotectedAttribute);
stateMachine.ProcessString(L"\x1b[2;1\"qZZZZZ");
VERIFY_IS_TRUE(textBuffer.GetCurrentAttributes().IsProtected());
VERIFY_IS_TRUE(_ValidateLineContains(cursorPosition, L"ZZZZZ", protectedAttribute));

Log::Comment(L"DECSCA 1;2 should be unprotected");
textBuffer.GetCursor().SetPosition(cursorPosition);
textBuffer.SetCurrentAttributes(protectedAttribute);
stateMachine.ProcessString(L"\x1b[1;2\"qZZZZZ");
VERIFY_IS_FALSE(textBuffer.GetCurrentAttributes().IsProtected());
VERIFY_IS_TRUE(_ValidateLineContains(cursorPosition, L"ZZZZZ", unprotectedAttribute));
}

void _CommonScrollingSetup()
{
// Used for testing MSFT:20204600
Expand Down
2 changes: 1 addition & 1 deletion src/inc/conattrs.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ enum class CharacterAttributes : uint16_t
TopGridline = COMMON_LVB_GRID_HORIZONTAL, // 0x400
LeftGridline = COMMON_LVB_GRID_LVERTICAL, // 0x800
RightGridline = COMMON_LVB_GRID_RVERTICAL, // 0x1000
Unused3 = 0x2000,
Protected = 0x2000,
ReverseVideo = COMMON_LVB_REVERSE_VIDEO, // 0x4000
BottomGridline = COMMON_LVB_UNDERSCORE // 0x8000
};
Expand Down
7 changes: 7 additions & 0 deletions src/terminal/adapter/DispatchTypes.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,13 @@ namespace Microsoft::Console::VirtualTerminal::DispatchTypes
BrightBackgroundWhite = 107,
};

enum LogicalAttributeOptions : VTInt
{
Default = 0,
Protected = 1,
Unprotected = 2
};

// Many of these correspond directly to SGR parameters (the GraphicsOptions enum), but
// these are distinct (notably 10 and 11, which as SGR parameters would select fonts,
// are used here to indicate that the foreground/background colors should be saved).
Expand Down
3 changes: 3 additions & 0 deletions src/terminal/adapter/ITermDispatch.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,12 @@ class Microsoft::Console::VirtualTerminal::ITermDispatch
virtual bool EraseInDisplay(const DispatchTypes::EraseType eraseType) = 0; // ED
virtual bool EraseInLine(const DispatchTypes::EraseType eraseType) = 0; // EL
virtual bool EraseCharacters(const VTInt numChars) = 0; // ECH
virtual bool SelectiveEraseInDisplay(const DispatchTypes::EraseType eraseType) = 0; // DECSED
virtual bool SelectiveEraseInLine(const DispatchTypes::EraseType eraseType) = 0; // DECSEL

virtual bool SetGraphicsRendition(const VTParameters options) = 0; // SGR
virtual bool SetLineRendition(const LineRendition rendition) = 0; // DECSWL, DECDWL, DECDHL
virtual bool SetCharacterProtectionAttribute(const VTParameters options) = 0; // DECSCA

virtual bool PushGraphicsRendition(const VTParameters options) = 0; // XTPUSHSGR
virtual bool PopGraphicsRendition() = 0; // XTPOPSGR
Expand Down
Loading