From b5c8c854cc2d0000a09672112dc20e6a98639713 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 13 Jan 2020 11:23:42 -0600 Subject: [PATCH 1/5] let's first move reflowing to the text buffer --- src/buffer/out/textBuffer.cpp | 212 ++++++++++++++++++++++++++++++++++ src/buffer/out/textBuffer.hpp | 2 + src/host/screenInfo.cpp | 211 +-------------------------------- 3 files changed, 218 insertions(+), 207 deletions(-) diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index a4eaa73140..c5829dc16f 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -1541,3 +1541,215 @@ std::string TextBuffer::GenRTF(const TextAndColor& rows, const int fontHeightPoi return {}; } } + +HRESULT TextBuffer::ReflowBuffer(TextBuffer& oldBuffer, TextBuffer& newBuffer) +{ + Cursor& oldCursor = oldBuffer.GetCursor(); + Cursor& newCursor = newBuffer.GetCursor(); + // skip any drawing updates that might occur as we manipulate the new buffer + oldCursor.StartDeferDrawing(); + newCursor.StartDeferDrawing(); + + // We need to save the old cursor position so that we can + // place the new cursor back on the equivalent character in + // the new buffer. + COORD cOldCursorPos = oldCursor.GetPosition(); + COORD cOldLastChar = oldBuffer.GetLastNonSpaceCharacter(); + + short const cOldRowsTotal = cOldLastChar.Y + 1; + short const cOldColsTotal = oldBuffer.GetSize().Width(); + + COORD cNewCursorPos = { 0 }; + bool fFoundCursorPos = false; + + HRESULT hr = S_OK; + // Loop through all the rows of the old buffer and reprint them into the new buffer + for (short iOldRow = 0; iOldRow < cOldRowsTotal; iOldRow++) + { + // Fetch the row and its "right" which is the last printable character. + const ROW& row = oldBuffer.GetRowByOffset(iOldRow); + const CharRow& charRow = row.GetCharRow(); + short iRight = static_cast(charRow.MeasureRight()); + + // There is a special case here. If the row has a "wrap" + // flag on it, but the right isn't equal to the width (one + // index past the final valid index in the row) then there + // were a bunch trailing of spaces in the row. + // (But the measuring functions for each row Left/Right do + // not count spaces as "displayable" so they're not + // included.) + // As such, adjust the "right" to be the width of the row + // to capture all these spaces + if (charRow.WasWrapForced()) + { + iRight = cOldColsTotal; + + // And a combined special case. + // If we wrapped off the end of the row by adding a + // piece of padding because of a double byte LEADING + // character, then remove one from the "right" to + // leave this padding out of the copy process. + if (charRow.WasDoubleBytePadded()) + { + iRight--; + } + } + + // Loop through every character in the current row (up to + // the "right" boundary, which is one past the final valid + // character) + for (short iOldCol = 0; iOldCol < iRight; iOldCol++) + { + if (iOldCol == cOldCursorPos.X && iOldRow == cOldCursorPos.Y) + { + cNewCursorPos = newCursor.GetPosition(); + fFoundCursorPos = true; + } + + try + { + // TODO: MSFT: 19446208 - this should just use an iterator and the inserter... + const auto glyph = row.GetCharRow().GlyphAt(iOldCol); + const auto dbcsAttr = row.GetCharRow().DbcsAttrAt(iOldCol); + const auto textAttr = row.GetAttrRow().GetAttrByColumn(iOldCol); + + if (!newBuffer.InsertCharacter(glyph, dbcsAttr, textAttr)) + { + hr = E_OUTOFMEMORY; + break; + } + } + CATCH_RETURN(); + } + if (SUCCEEDED(hr)) + { + // If we didn't have a full row to copy, insert a new + // line into the new buffer. + // Only do so if we were not forced to wrap. If we did + // force a word wrap, then the existing line break was + // only because we ran out of space. + if (iRight < cOldColsTotal && !charRow.WasWrapForced()) + { + if (iRight == cOldCursorPos.X && iOldRow == cOldCursorPos.Y) + { + cNewCursorPos = newCursor.GetPosition(); + fFoundCursorPos = true; + } + // Only do this if it's not the final line in the buffer. + // On the final line, we want the cursor to sit + // where it is done printing for the cursor + // adjustment to follow. + if (iOldRow < cOldRowsTotal - 1) + { + hr = newBuffer.NewlineCursor() ? hr : E_OUTOFMEMORY; + } + else + { + // If we are on the final line of the buffer, we have one more check. + // We got into this code path because we are at the right most column of a row in the old buffer + // that had a hard return (no wrap was forced). + // However, as we're inserting, the old row might have just barely fit into the new buffer and + // caused a new soft return (wrap was forced) putting the cursor at x=0 on the line just below. + // We need to preserve the memory of the hard return at this point by inserting one additional + // hard newline, otherwise we've lost that information. + // We only do this when the cursor has just barely poured over onto the next line so the hard return + // isn't covered by the soft one. + // e.g. + // The old line was: + // |aaaaaaaaaaaaaaaaaaa | with no wrap which means there was a newline after that final a. + // The cursor was here ^ + // And the new line will be: + // |aaaaaaaaaaaaaaaaaaa| and show a wrap at the end + // | | + // ^ and the cursor is now there. + // If we leave it like this, we've lost the newline information. + // So we insert one more newline so a continued reflow of this buffer by resizing larger will + // continue to look as the original output intended with the newline data. + // After this fix, it looks like this: + // |aaaaaaaaaaaaaaaaaaa| no wrap at the end (preserved hard newline) + // | | + // ^ and the cursor is now here. + const COORD coordNewCursor = newCursor.GetPosition(); + if (coordNewCursor.X == 0 && coordNewCursor.Y > 0) + { + if (newBuffer.GetRowByOffset(coordNewCursor.Y - 1).GetCharRow().WasWrapForced()) + { + hr = newBuffer.NewlineCursor() ? hr : E_OUTOFMEMORY; + } + } + } + } + } + } + if (SUCCEEDED(hr)) + { + // Finish copying remaining parameters from the old text buffer to the new one + newBuffer.CopyProperties(oldBuffer); + + // If we found where to put the cursor while placing characters into the buffer, + // just put the cursor there. Otherwise we have to advance manually. + if (fFoundCursorPos) + { + newCursor.SetPosition(cNewCursorPos); + } + else + { + // Advance the cursor to the same offset as before + // get the number of newlines and spaces between the old end of text and the old cursor, + // then advance that many newlines and chars + int iNewlines = cOldCursorPos.Y - cOldLastChar.Y; + int iIncrements = cOldCursorPos.X - cOldLastChar.X; + const COORD cNewLastChar = newBuffer.GetLastNonSpaceCharacter(); + + // If the last row of the new buffer wrapped, there's going to be one less newline needed, + // because the cursor is already on the next line + if (newBuffer.GetRowByOffset(cNewLastChar.Y).GetCharRow().WasWrapForced()) + { + iNewlines = std::max(iNewlines - 1, 0); + } + else + { + // if this buffer didn't wrap, but the old one DID, then the d(columns) of the + // old buffer will be one more than in this buffer, so new need one LESS. + if (oldBuffer.GetRowByOffset(cOldLastChar.Y).GetCharRow().WasWrapForced()) + { + iNewlines = std::max(iNewlines - 1, 0); + } + } + + for (int r = 0; r < iNewlines; r++) + { + if (!newBuffer.NewlineCursor()) + { + hr = E_OUTOFMEMORY; + break; + } + } + if (SUCCEEDED(hr)) + { + for (int c = 0; c < iIncrements - 1; c++) + { + if (!newBuffer.IncrementCursor()) + { + hr = E_OUTOFMEMORY; + break; + } + } + } + } + } + + if (SUCCEEDED(hr)) + { + // Save old cursor size before we delete it + ULONG const ulSize = oldCursor.GetSize(); + + // Set size back to real size as it will be taking over the rendering duties. + newCursor.SetSize(ulSize); + } + + newCursor.EndDeferDrawing(); + oldCursor.EndDeferDrawing(); + + return hr; +} diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index f77f879a82..f8b23ac1c2 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -158,6 +158,8 @@ public: const std::wstring_view fontFaceName, const COLORREF backgroundColor); + static HRESULT ReflowBuffer(TextBuffer& oldBuffer, TextBuffer& newBuffer); + private: std::deque _storage; Cursor _cursor; diff --git a/src/host/screenInfo.cpp b/src/host/screenInfo.cpp index f6d7e4407a..34e5c522b4 100644 --- a/src/host/screenInfo.cpp +++ b/src/host/screenInfo.cpp @@ -1425,224 +1425,21 @@ bool SCREEN_INFORMATION::IsMaximizedY() const // Save cursor's relative height versus the viewport SHORT const sCursorHeightInViewportBefore = _textBuffer->GetCursor().GetPosition().Y - _viewport.Top(); - Cursor& oldCursor = _textBuffer->GetCursor(); - Cursor& newCursor = newTextBuffer->GetCursor(); - // skip any drawing updates that might occur as we manipulate the new buffer - oldCursor.StartDeferDrawing(); - newCursor.StartDeferDrawing(); + HRESULT hr = TextBuffer::ReflowBuffer(*_textBuffer.get(), *newTextBuffer.get()); - // We need to save the old cursor position so that we can - // place the new cursor back on the equivalent character in - // the new buffer. - COORD cOldCursorPos = oldCursor.GetPosition(); - COORD cOldLastChar = _textBuffer->GetLastNonSpaceCharacter(); - - short const cOldRowsTotal = cOldLastChar.Y + 1; - short const cOldColsTotal = GetBufferSize().Width(); - - COORD cNewCursorPos = { 0 }; - bool fFoundCursorPos = false; - - NTSTATUS status = STATUS_SUCCESS; - // Loop through all the rows of the old buffer and reprint them into the new buffer - for (short iOldRow = 0; iOldRow < cOldRowsTotal; iOldRow++) - { - // Fetch the row and its "right" which is the last printable character. - const ROW& Row = _textBuffer->GetRowByOffset(iOldRow); - const CharRow& charRow = Row.GetCharRow(); - short iRight = static_cast(charRow.MeasureRight()); - - // There is a special case here. If the row has a "wrap" - // flag on it, but the right isn't equal to the width (one - // index past the final valid index in the row) then there - // were a bunch trailing of spaces in the row. - // (But the measuring functions for each row Left/Right do - // not count spaces as "displayable" so they're not - // included.) - // As such, adjust the "right" to be the width of the row - // to capture all these spaces - if (charRow.WasWrapForced()) - { - iRight = cOldColsTotal; - - // And a combined special case. - // If we wrapped off the end of the row by adding a - // piece of padding because of a double byte LEADING - // character, then remove one from the "right" to - // leave this padding out of the copy process. - if (charRow.WasDoubleBytePadded()) - { - iRight--; - } - } - - // Loop through every character in the current row (up to - // the "right" boundary, which is one past the final valid - // character) - for (short iOldCol = 0; iOldCol < iRight; iOldCol++) - { - if (iOldCol == cOldCursorPos.X && iOldRow == cOldCursorPos.Y) - { - cNewCursorPos = newCursor.GetPosition(); - fFoundCursorPos = true; - } - - try - { - // TODO: MSFT: 19446208 - this should just use an iterator and the inserter... - const auto glyph = Row.GetCharRow().GlyphAt(iOldCol); - const auto dbcsAttr = Row.GetCharRow().DbcsAttrAt(iOldCol); - const auto textAttr = Row.GetAttrRow().GetAttrByColumn(iOldCol); - - if (!newTextBuffer->InsertCharacter(glyph, dbcsAttr, textAttr)) - { - status = STATUS_NO_MEMORY; - break; - } - } - catch (...) - { - return NTSTATUS_FROM_HRESULT(wil::ResultFromCaughtException()); - } - } - if (NT_SUCCESS(status)) - { - // If we didn't have a full row to copy, insert a new - // line into the new buffer. - // Only do so if we were not forced to wrap. If we did - // force a word wrap, then the existing line break was - // only because we ran out of space. - if (iRight < cOldColsTotal && !charRow.WasWrapForced()) - { - if (iRight == cOldCursorPos.X && iOldRow == cOldCursorPos.Y) - { - cNewCursorPos = newCursor.GetPosition(); - fFoundCursorPos = true; - } - // Only do this if it's not the final line in the buffer. - // On the final line, we want the cursor to sit - // where it is done printing for the cursor - // adjustment to follow. - if (iOldRow < cOldRowsTotal - 1) - { - status = newTextBuffer->NewlineCursor() ? status : STATUS_NO_MEMORY; - } - else - { - // If we are on the final line of the buffer, we have one more check. - // We got into this code path because we are at the right most column of a row in the old buffer - // that had a hard return (no wrap was forced). - // However, as we're inserting, the old row might have just barely fit into the new buffer and - // caused a new soft return (wrap was forced) putting the cursor at x=0 on the line just below. - // We need to preserve the memory of the hard return at this point by inserting one additional - // hard newline, otherwise we've lost that information. - // We only do this when the cursor has just barely poured over onto the next line so the hard return - // isn't covered by the soft one. - // e.g. - // The old line was: - // |aaaaaaaaaaaaaaaaaaa | with no wrap which means there was a newline after that final a. - // The cursor was here ^ - // And the new line will be: - // |aaaaaaaaaaaaaaaaaaa| and show a wrap at the end - // | | - // ^ and the cursor is now there. - // If we leave it like this, we've lost the newline information. - // So we insert one more newline so a continued reflow of this buffer by resizing larger will - // continue to look as the original output intended with the newline data. - // After this fix, it looks like this: - // |aaaaaaaaaaaaaaaaaaa| no wrap at the end (preserved hard newline) - // | | - // ^ and the cursor is now here. - const COORD coordNewCursor = newCursor.GetPosition(); - if (coordNewCursor.X == 0 && coordNewCursor.Y > 0) - { - if (newTextBuffer->GetRowByOffset(coordNewCursor.Y - 1).GetCharRow().WasWrapForced()) - { - status = newTextBuffer->NewlineCursor() ? status : STATUS_NO_MEMORY; - } - } - } - } - } - } - if (NT_SUCCESS(status)) - { - // Finish copying remaining parameters from the old text buffer to the new one - newTextBuffer->CopyProperties(*_textBuffer); - - // If we found where to put the cursor while placing characters into the buffer, - // just put the cursor there. Otherwise we have to advance manually. - if (fFoundCursorPos) - { - newCursor.SetPosition(cNewCursorPos); - } - else - { - // Advance the cursor to the same offset as before - // get the number of newlines and spaces between the old end of text and the old cursor, - // then advance that many newlines and chars - int iNewlines = cOldCursorPos.Y - cOldLastChar.Y; - int iIncrements = cOldCursorPos.X - cOldLastChar.X; - const COORD cNewLastChar = newTextBuffer->GetLastNonSpaceCharacter(); - - // If the last row of the new buffer wrapped, there's going to be one less newline needed, - // because the cursor is already on the next line - if (newTextBuffer->GetRowByOffset(cNewLastChar.Y).GetCharRow().WasWrapForced()) - { - iNewlines = std::max(iNewlines - 1, 0); - } - else - { - // if this buffer didn't wrap, but the old one DID, then the d(columns) of the - // old buffer will be one more than in this buffer, so new need one LESS. - if (_textBuffer->GetRowByOffset(cOldLastChar.Y).GetCharRow().WasWrapForced()) - { - iNewlines = std::max(iNewlines - 1, 0); - } - } - - for (int r = 0; r < iNewlines; r++) - { - if (!newTextBuffer->NewlineCursor()) - { - status = STATUS_NO_MEMORY; - break; - } - } - if (NT_SUCCESS(status)) - { - for (int c = 0; c < iIncrements - 1; c++) - { - if (!newTextBuffer->IncrementCursor()) - { - status = STATUS_NO_MEMORY; - break; - } - } - } - } - } - - if (NT_SUCCESS(status)) + if (SUCCEEDED(hr)) { + Cursor& newCursor = newTextBuffer->GetCursor(); // Adjust the viewport so the cursor doesn't wildly fly off up or down. SHORT const sCursorHeightInViewportAfter = newCursor.GetPosition().Y - _viewport.Top(); COORD coordCursorHeightDiff = { 0 }; coordCursorHeightDiff.Y = sCursorHeightInViewportAfter - sCursorHeightInViewportBefore; LOG_IF_FAILED(SetViewportOrigin(false, coordCursorHeightDiff, true)); - // Save old cursor size before we delete it - ULONG const ulSize = oldCursor.GetSize(); - _textBuffer.swap(newTextBuffer); - - // Set size back to real size as it will be taking over the rendering duties. - newCursor.SetSize(ulSize); - newCursor.EndDeferDrawing(); } - oldCursor.EndDeferDrawing(); - return status; + return NTSTATUS_FROM_HRESULT(hr); } // From 9aec69467cf72c536fca001abc4e82222012307e Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 13 Jan 2020 11:34:50 -0600 Subject: [PATCH 2/5] add a doc comment because I'm not a barbarian --- src/buffer/out/textBuffer.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index c5829dc16f..cce094ceea 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -1542,6 +1542,16 @@ std::string TextBuffer::GenRTF(const TextAndColor& rows, const int fontHeightPoi } } +// Function Description: +// - Reflow the contents from the old buffer into the new buffer. The new buffer +// can have different dimensions than the old buffer. If it does, then this +// function will attempt to maintain the logical contents of the old buffer, +// by continuing wrapped lines onto the next line in the new buffer. +// Arguments: +// - oldBuffer - the text buffer to copy the contents FROM +// - newBuffer - the text buffer to copy the contents TO +// Return Value: +// - S_OK if we successfully copied the contents to the new buffer, otherwise an appropriate HRESULT. HRESULT TextBuffer::ReflowBuffer(TextBuffer& oldBuffer, TextBuffer& newBuffer) { Cursor& oldCursor = oldBuffer.GetCursor(); From 1a2654d291e530a189c376702d6f47ecc05c84bc Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 13 Jan 2020 17:07:43 -0600 Subject: [PATCH 3/5] Try to wrap the line properly with conpty This confusingly doesn't always work --- src/renderer/vt/XtermEngine.cpp | 13 +++++++++++-- src/renderer/vt/paint.cpp | 15 +++++++++++++++ src/renderer/vt/vtrenderer.hpp | 2 ++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/renderer/vt/XtermEngine.cpp b/src/renderer/vt/XtermEngine.cpp index 2c1d8a08ed..556381c566 100644 --- a/src/renderer/vt/XtermEngine.cpp +++ b/src/renderer/vt/XtermEngine.cpp @@ -19,7 +19,7 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, _ColorTable(ColorTable), _cColorTable(cColorTable), _fUseAsciiOnly(fUseAsciiOnly), - _previousLineWrapped(false), + // _previousLineWrapped(false), _usingUnderLine(false), _needToDisableCursor(false), _lastCursorIsVisible(false), @@ -248,7 +248,13 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, // If the previous line wrapped, then the cursor is already at this // position, we just don't know it yet. Don't emit anything. - if (_previousLineWrapped) + bool previousLineWrapped = false; + if (_wrappedRow.has_value()) + { + previousLineWrapped = coord.Y == _wrappedRow.value() + 1; + } + + if (previousLineWrapped) { hr = S_OK; } @@ -298,6 +304,9 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, _newBottomLine = false; } _deferredCursorPos = INVALID_COORDS; + + _wrappedRow = std::nullopt; + return hr; } diff --git a/src/renderer/vt/paint.cpp b/src/renderer/vt/paint.cpp index 920dc86aeb..02b3589596 100644 --- a/src/renderer/vt/paint.cpp +++ b/src/renderer/vt/paint.cpp @@ -449,6 +449,21 @@ 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 + // 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 is 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. + + // TODO: This seems to be off by one char. Resizing cmd.exe, the '.' at the + // end of the initial message sometimes gets cut off weirdly. + if ((_lastText.X + (totalWidth - numSpaces)) > _lastViewport.RightInclusive()) + { + _wrappedRow = coord.Y; + } + // Update our internal tracker of the cursor's position. // See MSFT:20266233 // If the cursor is at the rightmost column of the terminal, and we write a diff --git a/src/renderer/vt/vtrenderer.hpp b/src/renderer/vt/vtrenderer.hpp index 5adf1cd1d9..3b180e5e7c 100644 --- a/src/renderer/vt/vtrenderer.hpp +++ b/src/renderer/vt/vtrenderer.hpp @@ -136,6 +136,8 @@ namespace Microsoft::Console::Render Microsoft::Console::VirtualTerminal::RenderTracing _trace; bool _inResizeRequest{ false }; + std::optional _wrappedRow{ std::nullopt }; + [[nodiscard]] HRESULT _Write(std::string_view const str) noexcept; [[nodiscard]] HRESULT _WriteFormattedString(const std::string* const pFormat, ...) noexcept; [[nodiscard]] HRESULT _Flush() noexcept; From aae6ce60a491036f5917042efe6a85fee76ea67a Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 14 Jan 2020 16:07:22 -0600 Subject: [PATCH 4/5] This works, fixing the ECH at end of wrapped line bug The trick was that when a line that was exactly filled was followed by an empty line, we'd ECH when the cursor is virtually on the last char of the wrapped line. That was wrong. For a scenario like: |ABCDEF| | | We'd incorrectly render that as ["ABCDEF", "^[[K", "\r\n"]. That ECH in the middle there would erase the 'F', because the cursor was virtually still on the 'F' (because it had deferred wrapping to the next char). So, when we're about to ECH following a wrapped line, reset the _wrappedRow first, to make sure we correctly \r\n first. --- src/renderer/vt/XtermEngine.cpp | 3 +++ src/renderer/vt/paint.cpp | 14 +++++++++++++- src/renderer/vt/tracing.cpp | 27 +++++++++++++++++++++++++++ src/renderer/vt/tracing.hpp | 2 ++ 4 files changed, 45 insertions(+), 1 deletion(-) diff --git a/src/renderer/vt/XtermEngine.cpp b/src/renderer/vt/XtermEngine.cpp index 556381c566..f652ceb3d5 100644 --- a/src/renderer/vt/XtermEngine.cpp +++ b/src/renderer/vt/XtermEngine.cpp @@ -235,6 +235,8 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, { HRESULT hr = S_OK; + _trace.TraceMoveCursor(coord); + if (coord.X != _lastText.X || coord.Y != _lastText.Y) { if (coord.X == 0 && coord.Y == 0) @@ -256,6 +258,7 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, if (previousLineWrapped) { + _trace.TraceWrapped(); hr = S_OK; } else diff --git a/src/renderer/vt/paint.cpp b/src/renderer/vt/paint.cpp index 02b3589596..6edf254bd9 100644 --- a/src/renderer/vt/paint.cpp +++ b/src/renderer/vt/paint.cpp @@ -376,7 +376,9 @@ using namespace Microsoft::Console::Types; return S_OK; } - RETURN_IF_FAILED(_MoveCursor(coord)); + // auto lastPaintWrapped = _wrappedRow; + + // RETURN_IF_FAILED(_MoveCursor(coord)); std::wstring unclusteredString; unclusteredString.reserve(clusters.size()); @@ -445,6 +447,16 @@ using namespace Microsoft::Console::Types; (totalWidth - numSpaces) : totalWidth; + if (removeSpaces && _wrappedRow.has_value() /* && columnsActual == 0*/) + { + // If the previous row _exactly_ filled the line, and we're about to + // clear _the entire line_, then we actually do want to move the + // cursor down. + // RETURN_IF_FAILED(_MoveCursor(coord)); + _wrappedRow = std::nullopt; + } + RETURN_IF_FAILED(_MoveCursor(coord)); + // Write the actual text string std::wstring wstr = std::wstring(unclusteredString.data(), cchActual); RETURN_IF_FAILED(VtEngine::_WriteTerminalUtf8(wstr)); diff --git a/src/renderer/vt/tracing.cpp b/src/renderer/vt/tracing.cpp index 3c5f4e0aa6..04c8d54c8b 100644 --- a/src/renderer/vt/tracing.cpp +++ b/src/renderer/vt/tracing.cpp @@ -210,3 +210,30 @@ void RenderTracing::TraceLastText(const COORD lastTextPos) const UNREFERENCED_PARAMETER(lastTextPos); #endif UNIT_TESTING } + +void RenderTracing::TraceMoveCursor(const COORD pos) const +{ +#ifndef UNIT_TESTING + const auto lastTextStr = _CoordToString(pos); + const auto cursor = lastTextStr.c_str(); + TraceLoggingWrite(g_hConsoleVtRendererTraceProvider, + "VtEngine_TraceMoveCursor", + TraceLoggingString(cursor), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE)); +#else + UNREFERENCED_PARAMETER(pos); +#endif UNIT_TESTING +} + +void RenderTracing::TraceWrapped() const +{ +#ifndef UNIT_TESTING + const auto* const msg = "Wrapped instead of \\r\\n"; + TraceLoggingWrite(g_hConsoleVtRendererTraceProvider, + "VtEngine_TraceWrapped", + TraceLoggingString(msg), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE)); +#else + UNREFERENCED_PARAMETER(pos); +#endif UNIT_TESTING +} diff --git a/src/renderer/vt/tracing.hpp b/src/renderer/vt/tracing.hpp index 8d923adc81..ef00bea81b 100644 --- a/src/renderer/vt/tracing.hpp +++ b/src/renderer/vt/tracing.hpp @@ -29,6 +29,8 @@ namespace Microsoft::Console::VirtualTerminal void TraceString(const std::string_view& str) const; void TraceInvalidate(const Microsoft::Console::Types::Viewport view) const; void TraceLastText(const COORD lastText) const; + void TraceMoveCursor(const COORD pos) const; + void TraceWrapped() const; void TraceInvalidateAll(const Microsoft::Console::Types::Viewport view) const; void TraceTriggerCircling(const bool newFrame) const; void TraceStartPaint(const bool quickReturn, From 416be4656febb708193ed7ecc36914c6e98c1333 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 14 Jan 2020 16:46:33 -0600 Subject: [PATCH 5/5] This is some cleanup, almost ready for PR but I need to write tests which are blocked on #4213 --- src/renderer/vt/paint.cpp | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/renderer/vt/paint.cpp b/src/renderer/vt/paint.cpp index 6edf254bd9..bda79f6197 100644 --- a/src/renderer/vt/paint.cpp +++ b/src/renderer/vt/paint.cpp @@ -376,10 +376,6 @@ using namespace Microsoft::Console::Types; return S_OK; } - // auto lastPaintWrapped = _wrappedRow; - - // RETURN_IF_FAILED(_MoveCursor(coord)); - std::wstring unclusteredString; unclusteredString.reserve(clusters.size()); short totalWidth = 0; @@ -447,14 +443,19 @@ using namespace Microsoft::Console::Types; (totalWidth - numSpaces) : totalWidth; - if (removeSpaces && _wrappedRow.has_value() /* && columnsActual == 0*/) + if (cchActual == 0) { - // If the previous row _exactly_ filled the line, and we're about to - // clear _the entire line_, then we actually do want to move the - // cursor down. - // RETURN_IF_FAILED(_MoveCursor(coord)); + // If the previous row wrapped, but this line is empty, then we actually + // do want to move the cursor down. Otherwise, we'll possibly end up + // accidentally erasing the last character from the previous line, as + // the cursor is still waiting on that character for the next character + // to follow it. _wrappedRow = std::nullopt; + // TODO:: Write a test that emulates ~/vttests/reflow-120.py + // TODO:: Write a test that emulates ~/vttests/reflow-advanced.py } + + // Move the cursor to the start of this run. RETURN_IF_FAILED(_MoveCursor(coord)); // Write the actual text string @@ -468,9 +469,6 @@ using namespace Microsoft::Console::Types; // line. // Don't do this is 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. - - // TODO: This seems to be off by one char. Resizing cmd.exe, the '.' at the - // end of the initial message sometimes gets cut off weirdly. if ((_lastText.X + (totalWidth - numSpaces)) > _lastViewport.RightInclusive()) { _wrappedRow = coord.Y;