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

Fix copying wrapped lines by implementing better scrolling (#5181)

Browse source 
Now that the Terminal is doing a better job of actually marking which
lines were and were not wrapped, we're not always copying lines as
"wrapped" when they should be. We're more correctly marking lines as not
wrapped, when previously we'd leave them marked wrapped.

The real problem is here in the `ScrollFrame` method - we'd manually
newline the cursor to make the terminal's viewport shift down to a new
line. If we had to scroll the viewport for a _wrapped_ line, this would
cause the Terminal to mark that line as broken, because conpty would
emit an extra `\n` that didn't actually exist.

This more correctly implements `ScrollFrame`. Now, well move where we
"thought" the cursor was, so when we get to the next `PaintBufferLine`,
if the cursor needs to newline for the next line, it'll newline, but if
we're in the middle of a wrapped line, we'll just keep printing the
wrapped line.

A couple follow up bugs were found to be caused by the same bad logic.
See #5039 and #5161 for more details on the investigations there.

## References

* #4741 RwR, which probably made this worse
* #5122, which I branched off of 
* #1245, #357 - a pair of other conpty wrapped lines bugs
* #5228 - A followup issue for this PR

## PR Checklist
* [x] Closes #5113
* [x] Closes #5180 (by fixing DECRST 25)
* [x] Closes #5039
* [x] Closes #5161 (by ensuring we only `removeSpaces` on the actual
  bottom line)
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated

## Validation Steps Performed

* Checked the cases from #1245, #357 to validate that they still work
* Added more and more tests for these scenarios, and then I added MORE
  tests
* The entire team played with this in selfhost builds
This commit is contained in:
Mike Griese 2020-04-08 19:06:25 -05:00 committed by GitHub
parent 5622f230a8
commit 6fabc4abb7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 1136 additions and 86 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
.github/actions/spell-check/patterns/patterns.txt vendored
View file

@ -5,3 +5,6 @@ https://(?:(?:www\.|)youtube\.com|youtu.be)/[-a-zA-Z0-9?&=]*
Scro\&ll
# selectionInput.cpp
:\\windows\\syste\b
TestUtils::VerifyExpectedString\(tb, L"[^"]+"
hostSm\.ProcessString\(L"[^"]+"
\b([A-Za-z])\1{3,}\b

10
.github/actions/spell-check/whitelist/alphabet.txt vendored
View file

@ -7,10 +7,10 @@ ABCDEFGHIJ
abcdefghijk
abcdefghijklmnop
ABCDEFGHIJKLMNOPQRST
ABCDEFGHIJKLMNOPQRSTQ
abcdefghijklmnopqrstuvwxyz
QQQQQ
QQQQQQQQQ
QQQQQQQQQQ
ABCDEFGHIJPQRST
ABCDEFGHIJPQRSTQ
QQQQQQQQQQABCDEFGHIJ
QQQQQQQQQQABCDEFGHIJKLMNOPQRSTQQQQQQQQQ
QQQQQQQQQQABCDEFGHIJKLMNOPQRSTQQQQQQQQQQ
@ -19,9 +19,6 @@ qrstuvwxyz
qwerty
QWERTYUIOP
qwertyuiopasdfg
TTTTTTTTTTTTTTTTTTTTTTTTTT
VVVVVVVVVVVVVVVV
yyyy
ZAAZZ
ZABBZ
ZBAZZ
@ -31,4 +28,3 @@ ZYXWVUT
ZZBBZ
ZZZBB
ZZZBZ
ZZZZZ

1
.github/actions/spell-check/whitelist/whitelist.txt vendored
View file

@ -2893,6 +2893,7 @@ ZCtrl
zd
zh
ZM
zsh
zu
zxcvbnm
zy

912
src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp
View file

@ -23,6 +23,7 @@
class InputBuffer; // This for some reason needs to be fwd-decl'd
#include "../host/inputBuffer.hpp"
#include "../host/readDataCooked.hpp"
#include "../host/output.h"
#include "test/CommonState.hpp"
#include "../cascadia/TerminalCore/Terminal.hpp"
@ -50,6 +51,9 @@ using namespace TerminalCoreUnitTests;
class TerminalCoreUnitTests::ConptyRoundtripTests final
{
// !!! DANGER: Many tests in this class expect the Terminal and Host buffers
// to be 80x32. If you change these, you'll probably inadvertently break a
// bunch of tests !!!
static const SHORT TerminalViewWidth = 80;
static const SHORT TerminalViewHeight = 32;
@ -174,6 +178,16 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final
TEST_METHOD(TestResizeHeight);
TEST_METHOD(OutputWrappedLinesAtTopOfBuffer);
TEST_METHOD(OutputWrappedLinesAtBottomOfBuffer);
TEST_METHOD(ScrollWithChangesInMiddle);
TEST_METHOD(DontWrapMoveCursorInSingleFrame);
TEST_METHOD(ClearHostTrickeryTest);
TEST_METHOD(OverstrikeAtBottomOfBuffer);
TEST_METHOD(MarginsWithStatusLine);
TEST_METHOD(OutputWrappedLineWithSpace);
TEST_METHOD(OutputWrappedLineWithSpaceAtBottomOfBuffer);
TEST_METHOD(ScrollWithMargins);
private:
@ -950,7 +964,7 @@ void ConptyRoundtripTests::PassthroughClearScrollback()
else
{
// After we hit the bottom of the viewport, the newlines come in
// separated for whatever reason.
// separated by empty writes for whatever reason.
expectedOutput.push_back("\r");
expectedOutput.push_back("\n");
expectedOutput.push_back("");
@ -1026,8 +1040,7 @@ void ConptyRoundtripTests::PassthroughHardReset()
else
{
// After we hit the bottom of the viewport, the newlines come in
// separated for whatever reason.
// separated by empty writes for whatever reason.
expectedOutput.push_back("\r");
expectedOutput.push_back("\n");
expectedOutput.push_back("");
@ -1061,6 +1074,293 @@ void ConptyRoundtripTests::PassthroughHardReset()
}
}
void ConptyRoundtripTests::OutputWrappedLinesAtTopOfBuffer()
{
Log::Comment(
L"Case 1: Write a wrapped line right at the start of the buffer, before any circling");
VERIFY_IS_NOT_NULL(_pVtRenderEngine.get());
auto& g = ServiceLocator::LocateGlobals();
auto& renderer = *g.pRender;
auto& gci = g.getConsoleInformation();
auto& si = gci.GetActiveOutputBuffer();
auto& sm = si.GetStateMachine();
auto& hostTb = si.GetTextBuffer();
auto& termTb = *term->_buffer;
_flushFirstFrame();
const auto wrappedLineLength = TerminalViewWidth + 20;
sm.ProcessString(std::wstring(wrappedLineLength, L'A'));
auto verifyBuffer = [](const TextBuffer& tb) {
// Buffer contents should look like the following: (80 wide)
// (w) means we hard wrapped the line
// (b) means the line is _not_ wrapped (it's broken, the default state.)
// cursor is on the '_'
//
// |AAAAAAAA...AAAA| (w)
// |AAAAA_ ... | (b) (There are 20 'A's on this line.)
// | ... | (b)
VERIFY_IS_TRUE(tb.GetRowByOffset(0).GetCharRow().WasWrapForced());
VERIFY_IS_FALSE(tb.GetRowByOffset(1).GetCharRow().WasWrapForced());
auto iter0 = tb.GetCellDataAt({ 0, 0 });
TestUtils::VerifySpanOfText(L"A", iter0, 0, TerminalViewWidth);
auto iter1 = tb.GetCellDataAt({ 0, 1 });
TestUtils::VerifySpanOfText(L"A", iter1, 0, 20);
auto iter2 = tb.GetCellDataAt({ 20, 1 });
TestUtils::VerifySpanOfText(L" ", iter2, 0, TerminalViewWidth - 20);
};
verifyBuffer(hostTb);
expectedOutput.push_back(std::string(TerminalViewWidth, 'A'));
expectedOutput.push_back(std::string(20, 'A'));
VERIFY_SUCCEEDED(renderer.PaintFrame());
verifyBuffer(termTb);
}
void ConptyRoundtripTests::OutputWrappedLinesAtBottomOfBuffer()
{
Log::Comment(
L"Case 2: Write a wrapped line at the end of the buffer, once the conpty started circling");
VERIFY_IS_NOT_NULL(_pVtRenderEngine.get());
auto& g = ServiceLocator::LocateGlobals();
auto& renderer = *g.pRender;
auto& gci = g.getConsoleInformation();
auto& si = gci.GetActiveOutputBuffer();
auto& hostSm = si.GetStateMachine();
auto& hostTb = si.GetTextBuffer();
auto& termTb = *term->_buffer;
_flushFirstFrame();
// First, fill the buffer with contents, so conpty starts circling
const auto hostView = si.GetViewport();
const auto end = 2 * hostView.Height();
for (auto i = 0; i < end; i++)
{
Log::Comment(NoThrowString().Format(L"Writing line %d/%d", i, end));
expectedOutput.push_back("X");
if (i < hostView.BottomInclusive())
{
expectedOutput.push_back("\r\n");
}
else
{
// After we hit the bottom of the viewport, the newlines come in
// separated by empty writes for whatever reason.
expectedOutput.push_back("\r");
expectedOutput.push_back("\n");
expectedOutput.push_back("");
}
hostSm.ProcessString(L"X\n");
VERIFY_SUCCEEDED(renderer.PaintFrame());
}
const auto wrappedLineLength = TerminalViewWidth + 20;
// The following diagrams show the buffer contents after each string emitted
// from conpty. For each of these diagrams:
// (w) means we hard wrapped the line
// (b) means the line is _not_ wrapped (it's broken, the default state.)
// cursor is on the '_'
// Initial state:
// |X | (b)
// |X | (b)
// ...
// |X | (b)
// |_ | (b)
expectedOutput.push_back(std::string(TerminalViewWidth, 'A'));
// |X | (b)
// |X | (b)
// ...
// |X | (b)
// |AAAAAAAA...AAAA|_ (w) The cursor is actually on the last A here
// TODO GH#5228 might break the "newline & repaint the wrapped char" checks here, that's okay.
expectedOutput.push_back("\r"); // This \r\n is emitted by ScrollFrame to
expectedOutput.push_back("\n"); // add a newline to the bottom of the buffer
// |X | (b)
// |X | (b)
// ...
// |X | (b)
// |AAAAAAAA...AAAA| (b)
// |_ | (b)
expectedOutput.push_back("\x1b[31;80H"); // Move the cursor BACK to the wrapped row
// |X | (b)
// |X | (b)
// ...
// |X | (b)
// |AAAAAAAA...AAAA| (b) The cursor is actually on the last A here
// | | (b)
expectedOutput.push_back(std::string(1, 'A')); // Reprint the last character of the wrapped row
// |X | (b)
// |X | (b)
// ...
// |X | (b)
// |AAAAAAAA...AAAA|_ (w) The cursor is actually on the last A here
// | | (b)
expectedOutput.push_back(std::string(20, 'A')); // Print the second line.
// |X | (b)
// |X | (b)
// ...
// |X | (b)
// |AAAAAAAA...AAAA| (w)
// |AAAAA_ | (b) There are 20 'A's on this line.
hostSm.ProcessString(std::wstring(wrappedLineLength, L'A'));
auto verifyBuffer = [](const TextBuffer& tb, const short wrappedRow) {
// Buffer contents should look like the following: (80 wide)
// (w) means we hard wrapped the line
// (b) means the line is _not_ wrapped (it's broken, the default state.)
// cursor is on the '_'
//
// |X | (b)
// |X | (b)
// ...
// |X | (b)
// |AAAAAAAA...AAAA| (w)
// |AAAAA_ ... | (b) (There are 20 'A's on this line.)
VERIFY_IS_TRUE(tb.GetRowByOffset(wrappedRow).GetCharRow().WasWrapForced());
VERIFY_IS_FALSE(tb.GetRowByOffset(wrappedRow + 1).GetCharRow().WasWrapForced());
auto iter0 = tb.GetCellDataAt({ 0, wrappedRow });
TestUtils::VerifySpanOfText(L"A", iter0, 0, TerminalViewWidth);
auto iter1 = tb.GetCellDataAt({ 0, wrappedRow + 1 });
TestUtils::VerifySpanOfText(L"A", iter1, 0, 20);
auto iter2 = tb.GetCellDataAt({ 20, wrappedRow + 1 });
TestUtils::VerifySpanOfText(L" ", iter2, 0, TerminalViewWidth - 20);
};
verifyBuffer(hostTb, hostView.BottomInclusive() - 1);
VERIFY_SUCCEEDED(renderer.PaintFrame());
verifyBuffer(termTb, term->_mutableViewport.BottomInclusive() - 1);
}
void ConptyRoundtripTests::ScrollWithChangesInMiddle()
{
Log::Comment(L"This test checks emitting a wrapped line at the bottom of the"
L" viewport while _also_ emitting other text elsewhere in the same frame. This"
L" output will cause us to scroll the viewport in one frame, but we need to"
L" make sure the wrapped line _stays_ wrapped, and the scrolled text appears in"
L" the right place.");
VERIFY_IS_NOT_NULL(_pVtRenderEngine.get());
auto& g = ServiceLocator::LocateGlobals();
auto& renderer = *g.pRender;
auto& gci = g.getConsoleInformation();
auto& si = gci.GetActiveOutputBuffer();
auto& hostSm = si.GetStateMachine();
auto& hostTb = si.GetTextBuffer();
auto& termTb = *term->_buffer;
_flushFirstFrame();
// First, fill the buffer with contents, so conpty starts circling
const auto hostView = si.GetViewport();
const auto end = 2 * hostView.Height();
for (auto i = 0; i < end; i++)
{
Log::Comment(NoThrowString().Format(L"Writing line %d/%d", i, end));
expectedOutput.push_back("X");
if (i < hostView.BottomInclusive())
{
expectedOutput.push_back("\r\n");
}
else
{
// After we hit the bottom of the viewport, the newlines come in
// separated by empty writes for whatever reason.
expectedOutput.push_back("\r");
expectedOutput.push_back("\n");
expectedOutput.push_back("");
}
hostSm.ProcessString(L"X\n");
VERIFY_SUCCEEDED(renderer.PaintFrame());
}
const auto wrappedLineLength = TerminalViewWidth + 20;
// In the Terminal, we're going to expect:
expectedOutput.push_back("\x1b[15;1H"); // Move the cursor to row 14, col 0
expectedOutput.push_back("Y"); // Print a 'Y'
expectedOutput.push_back("\x1b[32;1H"); // Move the cursor to the last row
expectedOutput.push_back(std::string(TerminalViewWidth, 'A')); // Print the first 80 'A's
// This is going to be the end of the first frame - b/c we moved the cursor
// in the middle of the frame, we're going to hide/show the cursor during
// this frame
expectedOutput.push_back("\x1b[?25h"); // hide the cursor
// On the subsequent frame:
// TODO GH#5228 might break the "newline & repaint the wrapped char" checks here, that's okay.
expectedOutput.push_back("\r"); // This \r\n is emitted by ScrollFrame to
expectedOutput.push_back("\n"); // add a newline to the bottom of the buffer
expectedOutput.push_back("\x1b[31;80H"); // Move the cursor BACK to the wrapped row
expectedOutput.push_back(std::string(1, 'A')); // Reprint the last character of the wrapped row
expectedOutput.push_back(std::string(20, 'A')); // Print the second line.
_logConpty = true;
// To the host, we'll do something very similar:
hostSm.ProcessString(L"\x1b"
L"7"); // Save cursor
hostSm.ProcessString(L"\x1b[15;1H"); // Move the cursor to row 14, col 0
hostSm.ProcessString(L"Y"); // Print a 'Y'
hostSm.ProcessString(L"\x1b"
L"8"); // Restore
hostSm.ProcessString(std::wstring(wrappedLineLength, L'A')); // Print 100 'A's
auto verifyBuffer = [](const TextBuffer& tb, const til::rectangle viewport) {
const short wrappedRow = viewport.bottom<short>() - 2;
const short start = viewport.top<short>();
for (short i = start; i < wrappedRow; i++)
{
Log::Comment(NoThrowString().Format(L"Checking row %d", i));
TestUtils::VerifyExpectedString(tb, i == start + 13 ? L"Y" : L"X", { 0, i });
}
VERIFY_IS_TRUE(tb.GetRowByOffset(wrappedRow).GetCharRow().WasWrapForced());
VERIFY_IS_FALSE(tb.GetRowByOffset(wrappedRow + 1).GetCharRow().WasWrapForced());
auto iter0 = tb.GetCellDataAt({ 0, wrappedRow });
TestUtils::VerifySpanOfText(L"A", iter0, 0, TerminalViewWidth);
auto iter1 = tb.GetCellDataAt({ 0, wrappedRow + 1 });
TestUtils::VerifySpanOfText(L"A", iter1, 0, 20);
auto iter2 = tb.GetCellDataAt({ 20, wrappedRow + 1 });
TestUtils::VerifySpanOfText(L" ", iter2, 0, TerminalViewWidth - 20);
};
Log::Comment(NoThrowString().Format(L"Checking the host buffer..."));
verifyBuffer(hostTb, hostView.ToInclusive());
Log::Comment(NoThrowString().Format(L"... Done"));
VERIFY_SUCCEEDED(renderer.PaintFrame());
Log::Comment(NoThrowString().Format(L"Checking the terminal buffer..."));
verifyBuffer(termTb, term->_mutableViewport.ToInclusive());
Log::Comment(NoThrowString().Format(L"... Done"));
}
void ConptyRoundtripTests::ScrollWithMargins()
{
auto& g = ServiceLocator::LocateGlobals();
@ -1087,6 +1387,7 @@ void ConptyRoundtripTests::ScrollWithMargins()
// The letters represent the data in the TMUX pane.
// The final *** line represents the mode line which we will
// attempt to hold in place and not scroll.
// Note that the last line will contain one '*' less than the width of the window.
Log::Comment(L"Fill host with text pattern by feeding it into VT parser.");
const auto rowsToWrite = initialTermView.Height() - 1;
@ -1103,7 +1404,7 @@ void ConptyRoundtripTests::ScrollWithMargins()
}
// For the last one, write out the asterisks for the mode line.
for (auto i = 0; i < initialTermView.Width(); ++i)
for (auto i = 0; i < initialTermView.Width() - 1; ++i)
{
hostSm.ProcessCharacter('*');
}
@ -1127,7 +1428,7 @@ void ConptyRoundtripTests::ScrollWithMargins()
}
// For the last row, verify we have an entire row of asterisks for the mode line.
const std::wstring expectedModeLine(initialTermView.Width(), L'*');
const std::wstring expectedModeLine(initialTermView.Width() - 1, L'*');
const COORD expectedPos{ 0, gsl::narrow<SHORT>(rowsToWrite) };
TestUtils::VerifyExpectedString(tb, expectedModeLine, expectedPos);
};
@ -1140,13 +1441,8 @@ void ConptyRoundtripTests::ScrollWithMargins()
expectedOutput.push_back("\r\n");
}
{
const std::string expectedString(initialTermView.Width(), '*');
const std::string expectedString(initialTermView.Width() - 1, '*');
expectedOutput.push_back(expectedString);
// Cursor gets reset into bottom right corner as we're writing all the way into that corner.
std::stringstream ss;
ss << "\x1b[" << initialTermView.Height() << ";" << initialTermView.Width() << "H";
expectedOutput.push_back(ss.str());
}
Log::Comment(L"Verify host buffer contains pattern.");
@ -1257,7 +1553,7 @@ void ConptyRoundtripTests::ScrollWithMargins()
// For the last row, verify we have an entire row of asterisks for the mode line.
{
const std::wstring expectedModeLine(initialTermView.Width(), L'*');
const std::wstring expectedModeLine(initialTermView.Width() - 1, L'*');
const COORD modeLinePos{ 0, gsl::narrow<SHORT>(rowsToWrite) };
TestUtils::VerifyExpectedString(tb, expectedModeLine, modeLinePos);
}
@ -1279,8 +1575,9 @@ void ConptyRoundtripTests::ScrollWithMargins()
expectedOutput.push_back("\r\n");
}
{
const std::string expectedString(initialTermView.Width(), '*');
expectedOutput.push_back(expectedString);
const std::string expectedString(initialTermView.Width() - 1, '*');
// There will be one extra blank space at the end of the line, to prevent delayed EOL wrapping
expectedOutput.push_back(expectedString + " ");
}
{
// Cursor gets reset into second line from bottom, left most column
@ -1302,3 +1599,590 @@ void ConptyRoundtripTests::ScrollWithMargins()
// Verify the terminal side.
verifyBufferAfter(termTb);
}
void ConptyRoundtripTests::DontWrapMoveCursorInSingleFrame()
{
// See https://github.com/microsoft/terminal/pull/5181#issuecomment-607427840
Log::Comment(L"This is a test for when a line of text exactly wrapped, but "
L"the cursor didn't end the frame at the end of line (waiting "
L"for more wrapped text). We should still move the cursor in "
L"this case.");
VERIFY_IS_NOT_NULL(_pVtRenderEngine.get());
auto& g = ServiceLocator::LocateGlobals();
auto& renderer = *g.pRender;
auto& gci = g.getConsoleInformation();
auto& si = gci.GetActiveOutputBuffer();
auto& hostSm = si.GetStateMachine();
auto& hostTb = si.GetTextBuffer();
auto& termTb = *term->_buffer;
_flushFirstFrame();
auto verifyBuffer = [](const TextBuffer& tb) {
// Simple verification: Make sure the cursor is in the correct place,
// and that it's visible. We don't care so much about the buffer
// contents in this test.
const COORD expectedCursor{ 8, 3 };
VERIFY_ARE_EQUAL(expectedCursor, tb.GetCursor().GetPosition());
VERIFY_IS_TRUE(tb.GetCursor().IsVisible());
};
hostSm.ProcessString(L"\x1b[?25l");
hostSm.ProcessString(L"\x1b[H");
hostSm.ProcessString(L"\x1b[75C");
hostSm.ProcessString(L"XXXXX");
hostSm.ProcessString(L"\x1b[4;9H");
hostSm.ProcessString(L"\x1b[?25h");
Log::Comment(L"Checking the host buffer state");
verifyBuffer(hostTb);
expectedOutput.push_back("\x1b[75C");
expectedOutput.push_back("XXXXX");
expectedOutput.push_back("\x1b[4;9H");
// We're _not_ expecting a cursor on here, because we didn't actually hide
// the cursor during the course of this frame
Log::Comment(L"Painting the frame");
VERIFY_SUCCEEDED(renderer.PaintFrame());
Log::Comment(L"Checking the terminal buffer state");
verifyBuffer(termTb);
}
void ConptyRoundtripTests::ClearHostTrickeryTest()
{
BEGIN_TEST_METHOD_PROPERTIES()
TEST_METHOD_PROPERTY(L"Data:paintEachNewline", L"{0, 1, 2}")
TEST_METHOD_PROPERTY(L"Data:cursorOnNextLine", L"{false, true}")
TEST_METHOD_PROPERTY(L"Data:paintAfterDECALN", L"{false, true}")
TEST_METHOD_PROPERTY(L"Data:changeAttributes", L"{false, true}")
TEST_METHOD_PROPERTY(L"Data:useLongSpaces", L"{false, true}")
TEST_METHOD_PROPERTY(L"Data:printTextAfterSpaces", L"{false, true}")
END_TEST_METHOD_PROPERTIES();
constexpr int PaintEveryNewline = 0;
constexpr int PaintAfterAllNewlines = 1;
constexpr int DontPaintAfterNewlines = 2;
INIT_TEST_PROPERTY(int, paintEachNewline, L"Any of: manually PaintFrame after each newline is emitted, once at the end of all newlines, or not at all");
INIT_TEST_PROPERTY(bool, cursorOnNextLine, L"Either leave the cursor on the first line, or place it on the second line of the buffer");
INIT_TEST_PROPERTY(bool, paintAfterDECALN, L"Controls whether we manually paint a frame after the DECALN sequence is emitted.");
INIT_TEST_PROPERTY(bool, changeAttributes, L"If true, change the text attributes after the 'A's and spaces");
INIT_TEST_PROPERTY(bool, useLongSpaces, L"If true, print 10 spaces instead of 5, longer than a CUF sequence.");
INIT_TEST_PROPERTY(bool, printTextAfterSpaces, L"If true, print \"ZZZZZ\" after the spaces on the first line.");
// See https://github.com/microsoft/terminal/issues/5039#issuecomment-606833841
Log::Comment(L"This is a more than comprehensive test for GH#5039. We're "
L"going to print some text to the buffer, then fill the alt-"
L"buffer with text, then switch back to the main buffer. The "
L"text from the alt buffer should not pollute the main buffer.");
// The text we're printing will look like one of the following, with the
// cursor on the _
// * cursorOnNextLine=false, useLongSpaces=false:
// AAAAA ZZZZZ_
// * cursorOnNextLine=false, useLongSpaces=true:
// AAAAA ZZZZZ_
// * cursorOnNextLine=true, useLongSpaces=false:
// AAAAA ZZZZZ
// BBBBB_
// * cursorOnNextLine=true, useLongSpaces=true:
// AAAAA ZZZZZ
// BBBBB_
//
// If printTextAfterSpaces=false, then we won't print the "ZZZZZ"
//
// The interesting case that repros the bug in GH#5039 is
// - paintEachNewline=DontPaintAfterNewlines (2)
// - cursorOnNextLine=false
// - paintAfterDECALN=<any>
// - changeAttributes=true
// - useLongSpaces=<any>
// - printTextAfterSpaces=<any>
//
// All the possible cases are left here though, to catch potential future regressions.
VERIFY_IS_NOT_NULL(_pVtRenderEngine.get());
auto& g = ServiceLocator::LocateGlobals();
auto& renderer = *g.pRender;
auto& gci = g.getConsoleInformation();
auto& si = gci.GetActiveOutputBuffer();
auto& hostSm = si.GetStateMachine();
auto& hostTb = si.GetTextBuffer();
auto& termTb = *term->_buffer;
_flushFirstFrame();
auto verifyBuffer = [&cursorOnNextLine, &useLongSpaces, &printTextAfterSpaces](const TextBuffer& tb,
const til::rectangle viewport) {
// We _would_ expect the Terminal's cursor to be on { 8, 0 }, but this
// is currently broken due to #381/#4676. So we'll use the viewport
// provided to find the actual Y position of the cursor.
const short viewTop = viewport.origin().y<short>();
const short cursorRow = viewTop + (cursorOnNextLine ? 1 : 0);
const short cursorCol = (cursorOnNextLine ? 5 :
(10 + (useLongSpaces ? 5 : 0) + (printTextAfterSpaces ? 5 : 0)));
const COORD expectedCursor{ cursorCol, cursorRow };
VERIFY_ARE_EQUAL(expectedCursor, tb.GetCursor().GetPosition());
VERIFY_IS_TRUE(tb.GetCursor().IsVisible());
auto iter = TestUtils::VerifyExpectedString(tb, L"AAAAA", { 0, viewTop });
TestUtils::VerifyExpectedString(useLongSpaces ? L" " : L" ", iter);
if (printTextAfterSpaces)
{
TestUtils::VerifyExpectedString(L"ZZZZZ", iter);
}
else
{
TestUtils::VerifyExpectedString(L" ", iter);
}
TestUtils::VerifyExpectedString(L" ", iter);
if (cursorOnNextLine)
{
TestUtils::VerifyExpectedString(tb, L"BBBBB", { 0, cursorRow });
}
};
// We're _not_ checking the conpty output during this test, only the side effects.
_checkConptyOutput = false;
gci.LockConsole(); // Lock must be taken to manipulate alt/main buffer state.
auto unlock = wil::scope_exit([&] { gci.UnlockConsole(); });
Log::Comment(L"Setting up the host buffer...");
hostSm.ProcessString(L"AAAAA");
hostSm.ProcessString(useLongSpaces ? L" " : L" ");
if (changeAttributes)
{
hostSm.ProcessString(L"\x1b[44m");
}
if (printTextAfterSpaces)
{
hostSm.ProcessString(L"ZZZZZ");
}
hostSm.ProcessString(L"\x1b[0m");
if (cursorOnNextLine)
{
hostSm.ProcessString(L"\n");
hostSm.ProcessString(L"BBBBB");
}
Log::Comment(L"Painting after the initial setup.");
VERIFY_SUCCEEDED(renderer.PaintFrame());
Log::Comment(L"Switching to the alt buffer and using DECALN to fill it with 'E's");
hostSm.ProcessString(L"\x1b[?1049h");
hostSm.ProcessString(L"\x1b#8");
if (paintAfterDECALN)
{
VERIFY_SUCCEEDED(renderer.PaintFrame());
}
for (auto i = 0; i < si.GetViewport().Height(); i++)
{
hostSm.ProcessString(L"\n");
if (paintEachNewline == PaintEveryNewline)
{
VERIFY_SUCCEEDED(renderer.PaintFrame());
}
}
if (paintEachNewline == PaintAfterAllNewlines)
{
VERIFY_SUCCEEDED(renderer.PaintFrame());
}
Log::Comment(L"Returning to the main buffer.");
hostSm.ProcessString(L"\x1b[?1049l");
Log::Comment(L"Checking the host buffer state");
verifyBuffer(hostTb, si.GetViewport().ToInclusive());
Log::Comment(L"Painting the frame");
VERIFY_SUCCEEDED(renderer.PaintFrame());
Log::Comment(L"Checking the terminal buffer state");
verifyBuffer(termTb, term->_mutableViewport.ToInclusive());
}
void ConptyRoundtripTests::OverstrikeAtBottomOfBuffer()
{
// See https://github.com/microsoft/terminal/pull/5181#issuecomment-607545241
Log::Comment(L"This test replicates the zsh menu-complete functionality. In"
L" the course of a single frame, we're going to both scroll "
L"the frame and print multiple lines of text above the bottom line.");
VERIFY_IS_NOT_NULL(_pVtRenderEngine.get());
auto& g = ServiceLocator::LocateGlobals();
auto& renderer = *g.pRender;
auto& gci = g.getConsoleInformation();
auto& si = gci.GetActiveOutputBuffer();
auto& hostSm = si.GetStateMachine();
auto& hostTb = si.GetTextBuffer();
auto& termTb = *term->_buffer;
_flushFirstFrame();
auto verifyBuffer = [](const TextBuffer& tb,
const til::rectangle viewport) {
const auto lastRow = viewport.bottom<short>() - 1;
const til::point expectedCursor{ 0, lastRow - 1 };
VERIFY_ARE_EQUAL(expectedCursor, til::point{ tb.GetCursor().GetPosition() });
VERIFY_IS_TRUE(tb.GetCursor().IsVisible());
TestUtils::VerifyExpectedString(tb, L"AAAAAAAAAA DDDDDDDDDD", til::point{ 0, lastRow - 2 });
TestUtils::VerifyExpectedString(tb, L"BBBBBBBBBB", til::point{ 0, lastRow - 1 });
TestUtils::VerifyExpectedString(tb, L"FFFFFFFFFE", til::point{ 0, lastRow });
};
_logConpty = true;
// We're _not_ checking the conpty output during this test, only the side effects.
_checkConptyOutput = false;
hostSm.ProcessString(L"\x1b#8");
hostSm.ProcessString(L"\x1b[32;1H");
hostSm.ProcessString(L"\x1b[J");
hostSm.ProcessString(L"AAAAAAAAAA");
hostSm.ProcessString(L"\x1b[K");
hostSm.ProcessString(L"\r");
hostSm.ProcessString(L"\n");
hostSm.ProcessString(L"BBBBBBBBBB");
hostSm.ProcessString(L"\x1b[K");
hostSm.ProcessString(L"\n");
hostSm.ProcessString(L"CCCCCCCCCC");
hostSm.ProcessString(L"\x1b[2A");
hostSm.ProcessString(L"\r");
hostSm.ProcessString(L"\x1b[20C");
hostSm.ProcessString(L"DDDDDDDDDD");
hostSm.ProcessString(L"\x1b[K");
hostSm.ProcessString(L"\r");
hostSm.ProcessString(L"\n");
hostSm.ProcessString(L"\x1b[1B");
hostSm.ProcessString(L"EEEEEEEEEE");
hostSm.ProcessString(L"\r");
hostSm.ProcessString(L"FFFFFFFFF");
hostSm.ProcessString(L"\r");
hostSm.ProcessString(L"\x1b[A");
hostSm.ProcessString(L"\x1b[A");
hostSm.ProcessString(L"\n");
Log::Comment(L"========== Checking the host buffer state ==========");
verifyBuffer(hostTb, si.GetViewport().ToInclusive());
Log::Comment(L"Painting the frame");
VERIFY_SUCCEEDED(renderer.PaintFrame());
Log::Comment(L"========== Checking the terminal buffer state ==========");
verifyBuffer(termTb, term->_mutableViewport.ToInclusive());
}
void ConptyRoundtripTests::MarginsWithStatusLine()
{
// See https://github.com/microsoft/terminal/issues/5161
//
// This test reproduces a case from the MSYS/cygwin (runtime < 3.1) vim.
// From what I can tell, they implement scrolling by emitting a newline at
// the bottom of the buffer (to create a new blank line), then they use
// ScrollConsoleScreenBuffer to shift the status line(s) down a line, and
// then they re-printing the status line.
Log::Comment(L"Newline, and scroll the bottom lines of the buffer down with"
L" ScrollConsoleScreenBuffer to emulate how cygwin VIM works");
VERIFY_IS_NOT_NULL(_pVtRenderEngine.get());
auto& g = ServiceLocator::LocateGlobals();
auto& renderer = *g.pRender;
auto& gci = g.getConsoleInformation();
auto& si = gci.GetActiveOutputBuffer();
auto& hostSm = si.GetStateMachine();
auto& hostTb = si.GetTextBuffer();
auto& termTb = *term->_buffer;
_flushFirstFrame();
auto verifyBuffer = [](const TextBuffer& tb,
const til::rectangle viewport) {
const auto lastRow = viewport.bottom<short>() - 1;
const til::point expectedCursor{ 1, lastRow };
VERIFY_ARE_EQUAL(expectedCursor, til::point{ tb.GetCursor().GetPosition() });
VERIFY_IS_TRUE(tb.GetCursor().IsVisible());
TestUtils::VerifyExpectedString(tb, L"EEEEEEEEEE", til::point{ 0, lastRow - 4 });
TestUtils::VerifyExpectedString(tb, L"AAAAAAAAAA", til::point{ 0, lastRow - 3 });
TestUtils::VerifyExpectedString(tb, L" ", til::point{ 0, lastRow - 2 });
TestUtils::VerifyExpectedString(tb, L"XBBBBBBBBB", til::point{ 0, lastRow - 1 });
TestUtils::VerifyExpectedString(tb, L"YCCCCCCCCC", til::point{ 0, lastRow });
};
// We're _not_ checking the conpty output during this test, only the side effects.
_checkConptyOutput = false;
// Use DECALN to fill the buffer with 'E's.
hostSm.ProcessString(L"\x1b#8");
const short originalBottom = si.GetViewport().BottomInclusive();
// Print 3 lines into the bottom of the buffer:
// AAAAAAAAAA
// BBBBBBBBBB
// CCCCCCCCCC
// In this test, the 'B' and 'C' lines represent the status lines at the
// bottom of vim, and the 'A' line is a buffer line.
hostSm.ProcessString(L"\x1b[30;1H");
hostSm.ProcessString(L"AAAAAAAAAA");
hostSm.ProcessString(L"\n");
hostSm.ProcessString(L"BBBBBBBBBB");
hostSm.ProcessString(L"\n");
hostSm.ProcessString(L"CCCCCCCCCC");
Log::Comment(L"Painting the frame");
VERIFY_SUCCEEDED(renderer.PaintFrame());
// After printing the 'C' line, the cursor is on the bottom line of the viewport.
// Emit a newline here to get a new line at the bottom of the viewport.
hostSm.ProcessString(L"\n");
const short newBottom = si.GetViewport().BottomInclusive();
{
// Emulate calling ScrollConsoleScreenBuffer to scroll the B and C lines
// down one line.
SMALL_RECT src;
src.Top = newBottom - 2;
src.Left = 0;
src.Right = si.GetViewport().Width();
src.Bottom = originalBottom;
COORD tgt = { 0, newBottom - 1 };
TextAttribute useThisAttr(0x07); // We don't terribly care about the attributes so this is arbitrary
ScrollRegion(si, src, std::nullopt, tgt, L' ', useThisAttr);
}
// Move the cursor to the location of the B line
hostSm.ProcessString(L"\x1b[31;1H");
// Print an 'X' on the 'B' line, and a 'Y' on the 'C' line.
hostSm.ProcessString(L"X");
hostSm.ProcessString(L"\n");
hostSm.ProcessString(L"Y");
Log::Comment(L"========== Checking the host buffer state ==========");
verifyBuffer(hostTb, si.GetViewport().ToInclusive());
Log::Comment(L"Painting the frame");
VERIFY_SUCCEEDED(renderer.PaintFrame());
Log::Comment(L"========== Checking the terminal buffer state ==========");
verifyBuffer(termTb, term->_mutableViewport.ToInclusive());
}
void ConptyRoundtripTests::OutputWrappedLineWithSpace()
{
// See https://github.com/microsoft/terminal/pull/5181#issuecomment-610110348
Log::Comment(L"Ensures that a buffer line in conhost that wrapped _on a "
L"space_ will still be emitted as wrapped.");
VERIFY_IS_NOT_NULL(_pVtRenderEngine.get());
auto& g = ServiceLocator::LocateGlobals();
auto& renderer = *g.pRender;
auto& gci = g.getConsoleInformation();
auto& si = gci.GetActiveOutputBuffer();
auto& sm = si.GetStateMachine();
auto& hostTb = si.GetTextBuffer();
auto& termTb = *term->_buffer;
_flushFirstFrame();
const auto firstTextLength = TerminalViewWidth - 2;
const auto spacesLength = 3;
const auto secondTextLength = 1;
sm.ProcessString(std::wstring(firstTextLength, L'A'));
sm.ProcessString(std::wstring(spacesLength, L' '));
sm.ProcessString(std::wstring(secondTextLength, L'B'));
auto verifyBuffer = [&](const TextBuffer& tb) {
// Buffer contents should look like the following: (80 wide)
// (w) means we hard wrapped the line
// (b) means the line is _not_ wrapped (it's broken, the default state.)
//
// |AAAA...AA | (w)
// | B_ ... | (b) (cursor is on the '_')
// | ... | (b)
VERIFY_IS_TRUE(tb.GetRowByOffset(0).GetCharRow().WasWrapForced());
VERIFY_IS_FALSE(tb.GetRowByOffset(1).GetCharRow().WasWrapForced());
// First row
auto iter0 = tb.GetCellDataAt({ 0, 0 });
TestUtils::VerifySpanOfText(L"A", iter0, 0, firstTextLength);
TestUtils::VerifySpanOfText(L" ", iter0, 0, 2);
// Second row
auto iter1 = tb.GetCellDataAt({ 0, 1 });
TestUtils::VerifySpanOfText(L" ", iter1, 0, 1);
auto iter2 = tb.GetCellDataAt({ 1, 1 });
TestUtils::VerifySpanOfText(L"B", iter2, 0, secondTextLength);
};
Log::Comment(L"========== Checking the host buffer state ==========");
verifyBuffer(hostTb);
std::string firstLine = std::string(firstTextLength, 'A');
firstLine += " ";
std::string secondLine{ " B" };
expectedOutput.push_back(firstLine);
expectedOutput.push_back(secondLine);
Log::Comment(L"Painting the frame");
VERIFY_SUCCEEDED(renderer.PaintFrame());
Log::Comment(L"========== Checking the terminal buffer state ==========");
verifyBuffer(termTb);
}
void ConptyRoundtripTests::OutputWrappedLineWithSpaceAtBottomOfBuffer()
{
// See https://github.com/microsoft/terminal/pull/5181#issuecomment-610110348
// This is the same test as OutputWrappedLineWithSpace, but at the bottom of
// the buffer, so we get scrolling behavior as well.
Log::Comment(L"Ensures that a buffer line in conhost that wrapped _on a "
L"space_ will still be emitted as wrapped.");
VERIFY_IS_NOT_NULL(_pVtRenderEngine.get());
auto& g = ServiceLocator::LocateGlobals();
auto& renderer = *g.pRender;
auto& gci = g.getConsoleInformation();
auto& si = gci.GetActiveOutputBuffer();
auto& sm = si.GetStateMachine();
auto& hostTb = si.GetTextBuffer();
auto& termTb = *term->_buffer;
_flushFirstFrame();
// First, fill the buffer with contents, so conpty starts circling
const auto hostView = si.GetViewport();
const auto end = 2 * hostView.Height();
for (auto i = 0; i < end; i++)
{
Log::Comment(NoThrowString().Format(L"Writing line %d/%d", i, end));
expectedOutput.push_back("X");
if (i < hostView.BottomInclusive())
{
expectedOutput.push_back("\r\n");
}
else
{
// After we hit the bottom of the viewport, the newlines come in
// separated by empty writes for whatever reason.
expectedOutput.push_back("\r");
expectedOutput.push_back("\n");
expectedOutput.push_back("");
}
sm.ProcessString(L"X\n");
VERIFY_SUCCEEDED(renderer.PaintFrame());
}
const auto firstTextLength = TerminalViewWidth - 2;
const auto spacesLength = 3;
const auto secondTextLength = 1;
std::string firstLine = std::string(firstTextLength, 'A');
firstLine += " ";
std::string secondLine{ " B" };
// The following diagrams show the buffer contents after each string emitted
// from conpty. For each of these diagrams:
// (w) means we hard wrapped the line
// (b) means the line is _not_ wrapped (it's broken, the default state.)
// cursor is on the '_'
// Initial state:
// |X | (b)
// |X | (b)
// ...
// |X | (b)
// |_ | (b)
expectedOutput.push_back(firstLine);
// |X | (b)
// |X | (b)
// ...
// |X | (b)
// |AAAAAAAA...AA _| (w) The cursor is actually on the last ' ' here
// TODO GH#5228 might break the "newline & repaint the wrapped char" checks here, that's okay.
expectedOutput.push_back("\r"); // This \r\n is emitted by ScrollFrame to
expectedOutput.push_back("\n"); // add a newline to the bottom of the buffer
// |X | (b)
// |X | (b)
// ...
// |X | (b)
// |AAAAAAAA...AA | (b)
// |_ | (b)
expectedOutput.push_back("\x1b[31;80H"); // Move the cursor BACK to the wrapped row
// |X | (b)
// |X | (b)
// ...
// |X | (b)
// |AAAAAAAA...AA _| (b) The cursor is actually on the last ' ' here
// | | (b)
expectedOutput.push_back(std::string(1, ' ')); // Reprint the last character of the wrapped row
// |X | (b)
// |X | (b)
// ...
// |X | (b)
// |AAAAAAAA...AA |_ (w) The cursor is actually on the last ' ' here
// | | (b)
expectedOutput.push_back(secondLine);
// |X | (b)
// |X | (b)
// ...
// |X | (b)
// |AAAAAAAA...AA | (w)
// | B_ | (b)
sm.ProcessString(std::wstring(firstTextLength, L'A'));
sm.ProcessString(std::wstring(spacesLength, L' '));
sm.ProcessString(std::wstring(secondTextLength, L'B'));
auto verifyBuffer = [&](const TextBuffer& tb, const til::rectangle viewport) {
// Buffer contents should look like the following: (80 wide)
// (w) means we hard wrapped the line
// (b) means the line is _not_ wrapped (it's broken, the default state.)
//
// |AAAA...AA | (w)
// | B_ ... | (b) (cursor is on the '_')
// | ... | (b)
const short wrappedRow = viewport.bottom<short>() - 2;
VERIFY_IS_TRUE(tb.GetRowByOffset(wrappedRow).GetCharRow().WasWrapForced());
VERIFY_IS_FALSE(tb.GetRowByOffset(wrappedRow + 1).GetCharRow().WasWrapForced());
// First row
auto iter0 = tb.GetCellDataAt({ 0, wrappedRow });
TestUtils::VerifySpanOfText(L"A", iter0, 0, firstTextLength);
TestUtils::VerifySpanOfText(L" ", iter0, 0, 2);
// Second row
auto iter1 = tb.GetCellDataAt({ 0, wrappedRow + 1 });
TestUtils::VerifySpanOfText(L" ", iter1, 0, 1);
auto iter2 = tb.GetCellDataAt({ 1, wrappedRow + 1 });
TestUtils::VerifySpanOfText(L"B", iter2, 0, secondTextLength);
};
Log::Comment(L"========== Checking the host buffer state ==========");
verifyBuffer(hostTb, hostView.ToInclusive());
Log::Comment(L"Painting the frame");
VERIFY_SUCCEEDED(renderer.PaintFrame());
Log::Comment(L"========== Checking the terminal buffer state ==========");
verifyBuffer(termTb, term->_mutableViewport.ToInclusive());
}

13
src/host/ut_host/ConptyOutputTests.cpp
View file

@ -27,6 +27,12 @@ using namespace Microsoft::Console::Types;
class ConptyOutputTests
{
// !!! DANGER: Many tests in this class expect the Terminal and Host buffers
// to be 80x32. If you change these, you'll probably inadvertently break a
// bunch of tests !!!
static const SHORT TerminalViewWidth = 80;
static const SHORT TerminalViewHeight = 32;
// This test class is to write some things into the PTY and then check that
// the rendering that is coming out of the VT-sequence generator is exactly
// as we expect it to be.
@ -40,7 +46,7 @@ class ConptyOutputTests
m_state->InitEvents();
m_state->PrepareGlobalFont();
m_state->PrepareGlobalScreenBuffer();
m_state->PrepareGlobalScreenBuffer(TerminalViewWidth, TerminalViewHeight, TerminalViewWidth, TerminalViewHeight);
m_state->PrepareGlobalInputBuffer();
return true;
@ -66,7 +72,7 @@ class ConptyOutputTests
gci.SetDefaultBackgroundColor(INVALID_COLOR);
gci.SetFillAttribute(0x07); // DARK_WHITE on DARK_BLACK
m_state->PrepareNewTextBufferInfo(true);
m_state->PrepareNewTextBufferInfo(true, TerminalViewWidth, TerminalViewHeight);
auto& currentBuffer = gci.GetActiveOutputBuffer();
// Make sure a test hasn't left us in the alt buffer on accident
VERIFY_IS_FALSE(currentBuffer._IsAltBuffer());
@ -347,7 +353,6 @@ void ConptyOutputTests::InvalidateUntilOneBeforeEnd()
expectedOutput.push_back("\x1b[65C");
expectedOutput.push_back("ABCDEFGHIJKLMNO");
expectedOutput.push_back("\x1b[1;80H"); // we move the cursor to the end of the line after paint
VERIFY_SUCCEEDED(renderer.PaintFrame());
@ -367,7 +372,5 @@ void ConptyOutputTests::InvalidateUntilOneBeforeEnd()
expectedOutput.push_back("\x1b[13X");
expectedOutput.push_back("\x1b[13C");
expectedOutput.push_back("\x1b[?25h"); // we turn the cursor back on for good measure
VERIFY_SUCCEEDED(renderer.PaintFrame());
}

5
src/inc/consoletaeftemplates.hpp
View file

@ -17,6 +17,11 @@ Revision History:
#pragma once
// Helper for declaring a variable to store a TEST_METHOD_PROPERTY and get it's value from the test metadata
#define INIT_TEST_PROPERTY(type, identifer, description) \
type identifer; \
VERIFY_SUCCEEDED(TestData::TryGetValue(L#identifer, identifer), description);
namespace WEX::TestExecution
{
template<>

147
src/renderer/vt/XtermEngine.cpp
View file

@ -105,7 +105,7 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe,
// by prepending a cursor off.
if (_lastCursorIsVisible)
{
_buffer.insert(0, "\x1b[25l");
_buffer.insert(0, "\x1b[?25l");
_lastCursorIsVisible = false;
}
// If the cursor was NOT previously visible, then that's fine! we don't
@ -214,7 +214,35 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe,
// _nextCursorIsVisible will still be false (from when we set it during
// StartPaint)
_nextCursorIsVisible = true;
return VtEngine::PaintCursor(options);
// If we did a delayed EOL wrap because we actually wrapped the line here,
// then don't PaintCursor. When we're at the EOL because we've wrapped, our
// internal _lastText thinks the cursor is on the cell just past the right
// of the viewport (ex { 120, 0 }). However, conhost thinks the cursor is
// actually on the last cell of the row. So it'll tell us to paint the
// cursor at { 119, 0 }. If we do that movement, then we'll break line
// wrapping.
// See GH#5113, GH#1245, GH#357
const auto nextCursorPosition = options.coordCursor;
// Only skip this paint when we think the cursor is in the cell
// immediately off the edge of the terminal, and the actual cursor is in
// the last cell of the row. We're in a deferred wrap, but the host
// thinks the cursor is actually in-frame.
// See ConptyRoundtripTests::DontWrapMoveCursorInSingleFrame
const bool cursorIsInDeferredWrap = (nextCursorPosition.X == _lastText.X - 1) && (nextCursorPosition.Y == _lastText.Y);
// If all three of these conditions are true, then:
// * cursorIsInDeferredWrap: The cursor is in a position where the line
// filled the last cell of the row, but the host tried to paint it in
// the last cell anyways
// * _delayedEolWrap && _wrappedRow.has_value(): We think we've deferred
// the wrap of a line.
// If they're all true, DON'T manually paint the cursor this frame.
if (!(cursorIsInDeferredWrap && _delayedEolWrap && _wrappedRow.has_value()))
{
return VtEngine::PaintCursor(options);
}
return S_OK;
}
// Routine Description:
@ -232,9 +260,9 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe,
[[nodiscard]] HRESULT XtermEngine::_MoveCursor(COORD const coord) noexcept
{
HRESULT hr = S_OK;
const auto originalPos = _lastText;
_trace.TraceMoveCursor(_lastText, coord);
bool performedSoftWrap = false;
if (coord.X != _lastText.X || coord.Y != _lastText.Y)
{
if (coord.X == 0 && coord.Y == 0)
@ -260,6 +288,7 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe,
if (previousLineWrapped)
{
performedSoftWrap = true;
_trace.TraceWrapped();
hr = S_OK;
}
@ -318,10 +347,7 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe,
_lastText = coord;
}
}
if (_lastText.Y != _lastViewport.ToOrigin().BottomInclusive())
{
_newBottomLine = false;
}
_deferredCursorPos = INVALID_COORDS;
_wrappedRow = std::nullopt;
@ -345,6 +371,8 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe,
[[nodiscard]] HRESULT XtermEngine::ScrollFrame() noexcept
try
{
_trace.TraceScrollFrame(_scrollDelta);
if (_scrollDelta.x() != 0)
{
// No easy way to shift left-right. Everything needs repainting.
@ -356,41 +384,92 @@ try
return S_OK;
}
const short dy = _scrollDelta.y<SHORT>();
const short dy = _scrollDelta.y<short>();
const short absDy = static_cast<short>(abs(dy));
HRESULT hr = S_OK;
// Save the old wrap state here. We're going to clear it so that
// _MoveCursor will definitely move us to the right position. We'll
// restore the state afterwards.
const auto oldWrappedRow = _wrappedRow;
const auto oldDelayedEolWrap = _delayedEolWrap;
_delayedEolWrap = false;
_wrappedRow = std::nullopt;
if (dy < 0)
{
// Instead of deleting the first line (causing everything to move up)
// move to the bottom of the buffer, and newline.
// That will cause everything to move up, by moving the viewport down.
// This will let remote conhosts scroll up to see history like normal.
const short bottom = _lastViewport.ToOrigin().BottomInclusive();
hr = _MoveCursor({ 0, bottom });
if (SUCCEEDED(hr))
{
std::string seq = std::string(absDy, '\n');
hr = _Write(seq);
// Mark that the bottom line is new, so we won't spend time with an
// ECH on it.
_newBottomLine = true;
}
// We don't need to _MoveCursor the cursor again, because it's still
// at the bottom of the viewport.
// TODO GH#5228 - We could optimize this by only doing this newline work
// when there's more invalid than just the bottom line. If only the
// bottom line is invalid, then the next thing the Renderer is going to
// tell us to do is print the new line at the bottom of the viewport,
// and _MoveCursor will automatically give us the newline we want.
// When that's implemented, we'll probably want to make sure to add a
// _lastText.Y += dy;
// statement here.
// Move the cursor to the bottom of the current viewport
const short bottom = _lastViewport.BottomInclusive();
RETURN_IF_FAILED(_MoveCursor({ 0, bottom }));
// Emit some number of newlines to create space in the buffer.
RETURN_IF_FAILED(_Write(std::string(absDy, '\n')));
}
else if (dy > 0)
{
// Move to the top of the buffer, and insert some lines of text, to
// cause the viewport contents to shift down.
hr = _MoveCursor({ 0, 0 });
if (SUCCEEDED(hr))
{
hr = _InsertLine(absDy);
}
// If we've scrolled _down_, then move the cursor to the top of the
// buffer, and insert some newlines using the InsertLines VT sequence
RETURN_IF_FAILED(_MoveCursor({ 0, 0 }));
RETURN_IF_FAILED(_InsertLine(absDy));
}
return hr;
// Restore our wrap state.
_wrappedRow = oldWrappedRow;
_delayedEolWrap = oldDelayedEolWrap;
// Shift our internal tracker of the last text position according to how
// much we've scrolled. If we manually scroll the buffer right now, by
// moving the cursor to the bottom row of the viewport and emitting a
// newline, we'll cause any wrapped lines to get broken.
//
// Instead, we'll just update our internal tracker of where the buffer
// contents are. On this frame, we'll then still move the cursor correctly
// relative to the new frame contents. To do this, we'll shift our
// coordinates we're tracking, like the row that we wrapped on and the
// position we think we left the cursor.
//
// See GH#5113
_trace.TraceLastText(_lastText);
if (_wrappedRow.has_value())
{
_wrappedRow.value() += dy;
_trace.TraceSetWrapped(_wrappedRow.value());
}
if (_delayedEolWrap && _wrappedRow.has_value())
{
// If we wrapped the last line, and we're in the middle of painting it,
// then the newline we did above just manually broke the line. What
// we're doing here is a hack: we're going to manually re-invalidate the
// last character of the wrapped row. When the PaintBufferLine calls
// come back through, we'll paint this last character again, causing us
// to get into the wrapped state once again. This is the only way to
// ensure that if a line was wrapped, and we painted the first line in
// one frame, and the second line in another frame that included other
// changes _above_ the wrapped line, that we maintain the wrap state in
// the Terminal.
const til::rectangle lastCellOfWrappedRow{
til::point{ _lastViewport.RightInclusive(), _wrappedRow.value() },
til::size{ 1, 1 }
};
_trace.TraceInvalidate(lastCellOfWrappedRow);
_invalidMap.set(lastCellOfWrappedRow);
}
// If the entire viewport was invalidated this frame, don't mark the bottom
// line as new. There are cases where this can cause visual artifacts - see
// GH#5039 and ConptyRoundtripTests::ClearHostTrickeryTest
const bool allInvalidated = _invalidMap.all();
_newBottomLine = !allInvalidated;
return S_OK;
}
CATCH_RETURN();

40
src/renderer/vt/paint.cpp
View file

@ -32,7 +32,12 @@ using namespace Microsoft::Console::Types;
_titleChanged;
_quickReturn = !somethingToDo;
_trace.TraceStartPaint(_quickReturn, _invalidMap, _lastViewport.ToInclusive(), _scrollDelta, _cursorMoved);
_trace.TraceStartPaint(_quickReturn,
_invalidMap,
_lastViewport.ToInclusive(),
_scrollDelta,
_cursorMoved,
_wrappedRow);
return _quickReturn ? S_FALSE : S_OK;
}
@ -440,8 +445,13 @@ using namespace Microsoft::Console::Types;
(!_clearedAllThisFrame);
// If we're not using erase char, but we did erase all at the start of the
// frame, don't add spaces at the end.
const bool removeSpaces = (useEraseChar || (_clearedAllThisFrame) || (_newBottomLine));
// frame, don't add spaces at the end.
//
// GH#5161: Only removeSpaces when we're in the _newBottomLine state and the
// line we're trying to print right now _actually is the bottom line_
const bool removeSpaces = useEraseChar ||
_clearedAllThisFrame ||
(_newBottomLine && coord.Y == _lastViewport.BottomInclusive());
const size_t cchActual = removeSpaces ?
(cchLine - numSpaces) :
cchLine;
@ -458,6 +468,7 @@ using namespace Microsoft::Console::Types;
// the cursor is still waiting on that character for the next character
// to follow it.
_wrappedRow = std::nullopt;
_trace.TraceClearWrapped();
}
// Move the cursor to the start of this run.
@ -467,18 +478,16 @@ using namespace Microsoft::Console::Types;
std::wstring wstr = std::wstring(unclusteredString.data(), cchActual);
RETURN_IF_FAILED(VtEngine::_WriteTerminalUtf8(wstr));
// If we've written text to the last column of the viewport, then mark
// GH#4415, GH#5181
// If the renderer told us that this was a wrapped line, then mark
// that we've wrapped this line. The next time we attempt to move the
// cursor, if we're trying to move it to the start of the next line,
// we'll remember that this line was wrapped, and not manually break the
// line.
// Don't do this if the last character we're writing is a space - The last
// char will always be a space, but if we see that, we shouldn't wrap.
const short lastWrittenChar = base::ClampAdd(_lastText.X, base::ClampSub(totalWidth, numSpaces));
if (lineWrapped &&
lastWrittenChar > _lastViewport.RightInclusive())
if (lineWrapped)
{
_wrappedRow = coord.Y;
_trace.TraceSetWrapped(coord.Y);
}
// Update our internal tracker of the cursor's position.
@ -538,7 +547,7 @@ using namespace Microsoft::Console::Types;
RETURN_IF_FAILED(_EraseLine());
}
}
else if (_newBottomLine)
else if (_newBottomLine && coord.Y == _lastViewport.BottomInclusive())
{
// If we're on a new line, then we don't need to erase the line. The
// line is already empty.
@ -546,7 +555,7 @@ using namespace Microsoft::Console::Types;
{
_deferredCursorPos = { _lastText.X + sNumSpaces, _lastText.Y };
}
else
else if (numSpaces > 0)
{
std::wstring spaces = std::wstring(numSpaces, L' ');
RETURN_IF_FAILED(VtEngine::_WriteTerminalUtf8(spaces));
@ -555,9 +564,12 @@ using namespace Microsoft::Console::Types;
}
}
// If we previously though that this was a new bottom line, it certainly
// isn't new any longer.
_newBottomLine = false;
// If we printed to the bottom line, and we previously thought that this was
// a new bottom line, it certainly isn't new any longer.
if (coord.Y == _lastViewport.BottomInclusive())
{
_newBottomLine = false;
}
return S_OK;
}

1
src/renderer/vt/state.cpp
View file

@ -36,7 +36,6 @@ VtEngine::VtEngine(_In_ wil::unique_hfile pipe,
_lastWasBold(false),
_lastViewport(initialViewport),
_invalidMap(initialViewport.Dimensions()),
_lastRealCursor({ 0 }),
_lastText({ 0 }),
_scrollDelta({ 0, 0 }),
_quickReturn(false),

83
src/renderer/vt/tracing.cpp
View file

@ -148,7 +148,8 @@ void RenderTracing::TraceStartPaint(const bool quickReturn,
const til::bitmap invalidMap,
const til::rectangle lastViewport,
const til::point scrollDelt,
const bool cursorMoved) const
const bool cursorMoved,
const std::optional<short>& wrappedRow) const
{
#ifndef UNIT_TESTING
if (TraceLoggingProviderEnabled(g_hConsoleVtRendererTraceProvider, WINEVENT_LEVEL_VERBOSE, 0))
@ -159,14 +160,29 @@ void RenderTracing::TraceStartPaint(const bool quickReturn,
const auto lastView = lastViewStr.c_str();
const auto scrollDeltaStr = scrollDelt.to_string();
const auto scrollDelta = scrollDeltaStr.c_str();
TraceLoggingWrite(g_hConsoleVtRendererTraceProvider,
"VtEngine_TraceStartPaint",
TraceLoggingBool(quickReturn),
TraceLoggingWideString(invalidated),
TraceLoggingWideString(lastView),
TraceLoggingWideString(scrollDelta),
TraceLoggingBool(cursorMoved),
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE));
if (wrappedRow.has_value())
{
TraceLoggingWrite(g_hConsoleVtRendererTraceProvider,
"VtEngine_TraceStartPaint",
TraceLoggingBool(quickReturn),
TraceLoggingWideString(invalidated),
TraceLoggingWideString(lastView),
TraceLoggingWideString(scrollDelta),
TraceLoggingBool(cursorMoved),
TraceLoggingValue(wrappedRow.value()),
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE));
}
else
{
TraceLoggingWrite(g_hConsoleVtRendererTraceProvider,
"VtEngine_TraceStartPaint",
TraceLoggingBool(quickReturn),
TraceLoggingWideString(invalidated),
TraceLoggingWideString(lastView),
TraceLoggingWideString(scrollDelta),
TraceLoggingBool(cursorMoved),
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE));
}
}
#else
UNREFERENCED_PARAMETER(quickReturn);
@ -174,6 +190,7 @@ void RenderTracing::TraceStartPaint(const bool quickReturn,
UNREFERENCED_PARAMETER(lastViewport);
UNREFERENCED_PARAMETER(scrollDelt);
UNREFERENCED_PARAMETER(cursorMoved);
UNREFERENCED_PARAMETER(wrappedRow);
#endif UNIT_TESTING
}
@ -203,6 +220,24 @@ void RenderTracing::TraceLastText(const til::point lastTextPos) const
UNREFERENCED_PARAMETER(lastTextPos);
#endif UNIT_TESTING
}
void RenderTracing::TraceScrollFrame(const til::point scrollDeltaPos) const
{
#ifndef UNIT_TESTING
if (TraceLoggingProviderEnabled(g_hConsoleVtRendererTraceProvider, WINEVENT_LEVEL_VERBOSE, 0))
{
const auto scrollDeltaStr = scrollDeltaPos.to_string();
const auto scrollDelta = scrollDeltaStr.c_str();
TraceLoggingWrite(g_hConsoleVtRendererTraceProvider,
"VtEngine_TraceScrollFrame",
TraceLoggingWideString(scrollDelta),
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE));
}
#else
UNREFERENCED_PARAMETER(scrollDeltaPos);
#endif UNIT_TESTING
}
void RenderTracing::TraceMoveCursor(const til::point lastTextPos, const til::point cursor) const
{
#ifndef UNIT_TESTING
@ -241,6 +276,36 @@ void RenderTracing::TraceWrapped() const
#endif UNIT_TESTING
}
void RenderTracing::TraceSetWrapped(const short wrappedRow) const
{
#ifndef UNIT_TESTING
if (TraceLoggingProviderEnabled(g_hConsoleVtRendererTraceProvider, WINEVENT_LEVEL_VERBOSE, 0))
{
TraceLoggingWrite(g_hConsoleVtRendererTraceProvider,
"VtEngine_TraceSetWrapped",
TraceLoggingValue(wrappedRow),
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE));
}
#else
UNREFERENCED_PARAMETER(wrappedRow);
#endif UNIT_TESTING
}
void RenderTracing::TraceClearWrapped() const
{
#ifndef UNIT_TESTING
if (TraceLoggingProviderEnabled(g_hConsoleVtRendererTraceProvider, WINEVENT_LEVEL_VERBOSE, 0))
{
const auto* const msg = "Cleared wrap state";
TraceLoggingWrite(g_hConsoleVtRendererTraceProvider,
"VtEngine_TraceClearWrapped",
TraceLoggingString(msg),
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE));
}
#else
#endif UNIT_TESTING
}
void RenderTracing::TracePaintCursor(const til::point coordCursor) const
{
#ifndef UNIT_TESTING

6
src/renderer/vt/tracing.hpp
View file

@ -29,7 +29,10 @@ namespace Microsoft::Console::VirtualTerminal
void TraceString(const std::string_view& str) const;
void TraceInvalidate(const til::rectangle view) const;
void TraceLastText(const til::point lastText) const;
void TraceScrollFrame(const til::point scrollDelta) const;
void TraceMoveCursor(const til::point lastText, const til::point cursor) const;
void TraceSetWrapped(const short wrappedRow) const;
void TraceClearWrapped() const;
void TraceWrapped() const;
void TracePaintCursor(const til::point coordCursor) const;
void TraceInvalidateAll(const til::rectangle view) const;
@ -39,7 +42,8 @@ namespace Microsoft::Console::VirtualTerminal
const til::bitmap invalidMap,
const til::rectangle lastViewport,
const til::point scrollDelta,
const bool cursorMoved) const;
const bool cursorMoved,
const std::optional<short>& wrappedRow) const;
void TraceEndPaint() const;
};
}

1
src/renderer/vt/vtrenderer.hpp
View file

@ -124,7 +124,6 @@ namespace Microsoft::Console::Render
til::bitmap _invalidMap;
COORD _lastRealCursor;
COORD _lastText;
til::point _scrollDelta;