From 18dae6dae827e341d4a3c0311c4f3bfc9c55dfca Mon Sep 17 00:00:00 2001 From: Dustin Howett Date: Mon, 25 Sep 2023 13:40:13 -0500 Subject: [PATCH 1/4] version: bump to 1.20 on main --- custom.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/custom.props b/custom.props index 6027b31193d..328d7042eb2 100644 --- a/custom.props +++ b/custom.props @@ -5,7 +5,7 @@ true 2023 1 - 19 + 20 Windows Terminal From e0fc3bcd0a68e8be0c599b388818ca20ebe1cddb Mon Sep 17 00:00:00 2001 From: "Dustin L. Howett" Date: Mon, 25 Sep 2023 19:24:16 -0500 Subject: [PATCH 2/4] About: check PackageManager for updates in addition to Store (#16012) With us adding a .appinstaller distribution of Canary, the Store services update checker has beome insufficient to determine whether there are package updates. App Installer supports us checking for updates by using PackageManager and the Package interfaces. We'll use those instead of the Store services interface, and bail out early if the App Installer gives us an answer. --- src/cascadia/TerminalApp/AboutDialog.cpp | 52 ++++++++++++++++++++---- src/cascadia/TerminalApp/pch.h | 1 + 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/src/cascadia/TerminalApp/AboutDialog.cpp b/src/cascadia/TerminalApp/AboutDialog.cpp index 764f7b6dcaf..c443b73e09f 100644 --- a/src/cascadia/TerminalApp/AboutDialog.cpp +++ b/src/cascadia/TerminalApp/AboutDialog.cpp @@ -86,16 +86,54 @@ namespace winrt::TerminalApp::implementation co_await wil::resume_foreground(strongThis->Dispatcher()); UpdatesAvailable(true); #else // release build, likely has a store context - if (auto storeContext{ winrt::Windows::Services::Store::StoreContext::GetDefault() }) + bool packageManagerAnswered{ false }; + + try + { + if (auto currentPackage{ winrt::Windows::ApplicationModel::Package::Current() }) + { + // We need to look up our package in the Package Manager; we cannot use Current + winrt::Windows::Management::Deployment::PackageManager pm; + if (auto lookedUpPackage{ pm.FindPackageForUser(winrt::hstring{}, currentPackage.Id().FullName()) }) + { + using winrt::Windows::ApplicationModel::PackageUpdateAvailability; + auto availabilityResult = co_await lookedUpPackage.CheckUpdateAvailabilityAsync(); + co_await wil::resume_foreground(strongThis->Dispatcher()); + auto availability = availabilityResult.Availability(); + switch (availability) + { + case PackageUpdateAvailability::Available: + case PackageUpdateAvailability::Required: + case PackageUpdateAvailability::NoUpdates: + UpdatesAvailable(availability != PackageUpdateAvailability::NoUpdates); + packageManagerAnswered = true; + break; + case PackageUpdateAvailability::Error: + case PackageUpdateAvailability::Unknown: + default: + // Do not set packageManagerAnswered, which will trigger the store check. + break; + } + } + } + } + catch (...) + { + } // Do nothing on failure + + if (!packageManagerAnswered) { - const auto updates = co_await storeContext.GetAppAndOptionalStorePackageUpdatesAsync(); - co_await wil::resume_foreground(strongThis->Dispatcher()); - if (updates) + if (auto storeContext{ winrt::Windows::Services::Store::StoreContext::GetDefault() }) { - const auto numUpdates = updates.Size(); - if (numUpdates > 0) + const auto updates = co_await storeContext.GetAppAndOptionalStorePackageUpdatesAsync(); + co_await wil::resume_foreground(strongThis->Dispatcher()); + if (updates) { - UpdatesAvailable(true); + const auto numUpdates = updates.Size(); + if (numUpdates > 0) + { + UpdatesAvailable(true); + } } } } diff --git a/src/cascadia/TerminalApp/pch.h b/src/cascadia/TerminalApp/pch.h index 0065e5278fc..01811ad62f6 100644 --- a/src/cascadia/TerminalApp/pch.h +++ b/src/cascadia/TerminalApp/pch.h @@ -49,6 +49,7 @@ #include #include #include +#include #include #include From c7f30a86d7d48585bdd72493283fa10c22c54664 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 26 Sep 2023 02:24:29 +0200 Subject: [PATCH 3/4] Fix the prompt sometimes not being erased properly (#15880) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A carriage return (enter key) will increase the _distanceEnd by up to viewport-width many columns, since it increases the Y distance between the start and end by 1 (it's a newline after all). This will make _flushBuffer() think that the new _buffer is way longer than the old one and so _erase() ends up not erasing the tail end of the prompt, even if the new prompt is actually shorter. This commit fixes the issue by separating the newline printing out from the regular text printing loops. ## Validation Steps Performed * Run cmd.exe * Write "echo hello" and press Enter * Write "foobar foo bar" (don't press Enter) * Press F7, select "echo hello" and press Enter * Previous prompt says "echo hello" ✅ --- src/host/readDataCooked.cpp | 149 +++++++++++++++++++++--------------- src/host/readDataCooked.hpp | 22 ++++-- 2 files changed, 103 insertions(+), 68 deletions(-) diff --git a/src/host/readDataCooked.cpp b/src/host/readDataCooked.cpp index a5859d6766f..d05502dc0d0 100644 --- a/src/host/readDataCooked.cpp +++ b/src/host/readDataCooked.cpp @@ -204,18 +204,19 @@ bool COOKED_READ_DATA::Read(const bool isUnicode, size_t& numBytes, ULONG& contr { controlKeyState = 0; - const auto done = _readCharInputLoop(); + _readCharInputLoop(); // NOTE: Don't call _flushBuffer in a wil::scope_exit/defer. // It may throw and throwing during an ongoing exception is a bad idea. _flushBuffer(); - if (done) + if (_state == State::Accumulating) { - _handlePostCharInputLoop(isUnicode, numBytes, controlKeyState); + return false; } - return done; + _handlePostCharInputLoop(isUnicode, numBytes, controlKeyState); + return true; } // Printing wide glyphs at the end of a row results in a forced line wrap and a padding whitespace to be inserted. @@ -308,17 +309,10 @@ size_t COOKED_READ_DATA::_wordNext(const std::wstring_view& chars, size_t positi return position; } -const std::wstring_view& COOKED_READ_DATA::_newlineSuffix() const noexcept -{ - static constexpr std::wstring_view cr{ L"\r" }; - static constexpr std::wstring_view crlf{ L"\r\n" }; - return WI_IsFlagSet(_pInputBuffer->InputMode, ENABLE_PROCESSED_INPUT) ? crlf : cr; -} - // Reads text off of the InputBuffer and dispatches it to the current popup or otherwise into the _buffer contents. -bool COOKED_READ_DATA::_readCharInputLoop() +void COOKED_READ_DATA::_readCharInputLoop() { - for (;;) + while (_state == State::Accumulating) { const auto hasPopup = !_popups.empty(); auto charOrVkey = UNICODE_NULL; @@ -331,7 +325,7 @@ bool COOKED_READ_DATA::_readCharInputLoop() const auto status = GetChar(_pInputBuffer, &charOrVkey, true, pCommandLineEditingKeys, pPopupKeys, &modifiers); if (status == CONSOLE_STATUS_WAIT) { - return false; + break; } THROW_IF_NTSTATUS_FAILED(status); @@ -339,10 +333,7 @@ bool COOKED_READ_DATA::_readCharInputLoop() { const auto wch = static_cast(popupKeys ? 0 : charOrVkey); const auto vkey = static_cast(popupKeys ? charOrVkey : 0); - if (_popupHandleInput(wch, vkey, modifiers)) - { - return true; - } + _popupHandleInput(wch, vkey, modifiers); } else { @@ -350,16 +341,16 @@ bool COOKED_READ_DATA::_readCharInputLoop() { _handleVkey(charOrVkey, modifiers); } - else if (_handleChar(charOrVkey, modifiers)) + else { - return true; + _handleChar(charOrVkey, modifiers); } } } } // Handles character input for _readCharInputLoop() when no popups exist. -bool COOKED_READ_DATA::_handleChar(wchar_t wch, const DWORD modifiers) +void COOKED_READ_DATA::_handleChar(wchar_t wch, const DWORD modifiers) { // All paths in this function modify the buffer. @@ -376,17 +367,19 @@ bool COOKED_READ_DATA::_handleChar(wchar_t wch, const DWORD modifiers) _bufferCursor++; _controlKeyState = modifiers; - return true; + _transitionState(State::DoneWithWakeupMask); + return; } switch (wch) { case UNICODE_CARRIAGERETURN: { - _buffer.append(_newlineSuffix()); + // NOTE: Don't append newlines to the buffer just yet! See _handlePostCharInputLoop for more information. _bufferCursor = _buffer.size(); _markAsDirty(); - return true; + _transitionState(State::DoneWithCarriageReturn); + return; } case EXTKEY_ERASE_PREV_WORD: // Ctrl+Backspace case UNICODE_BACKSPACE: @@ -415,7 +408,7 @@ bool COOKED_READ_DATA::_handleChar(wchar_t wch, const DWORD modifiers) LOG_IF_FAILED(pConsoleWindow->SignalUia(UIA_Text_TextChangedEventId)); } } - return false; + return; } // If processed mode is disabled, control characters like backspace are treated like any other character. break; @@ -437,7 +430,6 @@ bool COOKED_READ_DATA::_handleChar(wchar_t wch, const DWORD modifiers) _bufferCursor++; _markAsDirty(); - return false; } // Handles non-character input for _readCharInputLoop() when no popups exist. @@ -656,15 +648,34 @@ void COOKED_READ_DATA::_handlePostCharInputLoop(const bool isUnicode, size_t& nu std::wstring_view input{ _buffer }; size_t lineCount = 1; - if (WI_IsFlagSet(_pInputBuffer->InputMode, ENABLE_ECHO_INPUT)) + if (_state == State::DoneWithCarriageReturn) { - // The last characters in line-read are a \r or \r\n unless _ctrlWakeupMask was used. - // Neither History nor s_MatchAndCopyAlias want to know about them. - const auto& suffix = _newlineSuffix(); - if (input.ends_with(suffix)) - { - input.remove_suffix(suffix.size()); + static constexpr std::wstring_view cr{ L"\r" }; + static constexpr std::wstring_view crlf{ L"\r\n" }; + const auto newlineSuffix = WI_IsFlagSet(_pInputBuffer->InputMode, ENABLE_PROCESSED_INPUT) ? crlf : cr; + std::wstring alias; + // Here's why we can't easily use _flushBuffer() to handle newlines: + // + // A carriage return (enter key) will increase the _distanceEnd by up to viewport-width many columns, + // since it increases the Y distance between the start and end by 1 (it's a newline after all). + // This will make _flushBuffer() think that the new _buffer is way longer than the old one and so + // _erase() ends up not erasing the tail end of the prompt, even if the new prompt is actually shorter. + // + // If you were to break this (remove this code and then append \r\n in _handleChar()) + // you can reproduce the issue easily if you do this: + // * Run cmd.exe + // * Write "echo hello" and press Enter + // * Write "foobar foo bar" (don't press Enter) + // * Press F7, select "echo hello" and press Enter + // + // It'll print "hello" but the previous prompt will say "echo hello bar" because the _distanceEnd + // ended up being well over 14 leading it to believe that "bar" got overwritten during WriteCharsLegacy(). + + WriteCharsLegacy(_screenInfo, newlineSuffix, true, nullptr); + + if (WI_IsFlagSet(_pInputBuffer->InputMode, ENABLE_ECHO_INPUT)) + { if (_history) { auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); @@ -672,24 +683,27 @@ void COOKED_READ_DATA::_handlePostCharInputLoop(const bool isUnicode, size_t& nu } Tracing::s_TraceCookedRead(_processHandle, input); + alias = Alias::s_MatchAndCopyAlias(input, _exeName, lineCount); + } - const auto alias = Alias::s_MatchAndCopyAlias(input, _exeName, lineCount); - if (!alias.empty()) - { - _buffer = alias; - } + if (!alias.empty()) + { + _buffer = std::move(alias); + } + else + { + _buffer.append(newlineSuffix); + } - // NOTE: Even if there's no alias we should restore the trailing \r\n that we removed above. - input = std::wstring_view{ _buffer }; + input = std::wstring_view{ _buffer }; - // doskey aliases may result in multiple lines of output (for instance `doskey test=echo foo$Techo bar$Techo baz`). - // We need to emit them as multiple cooked reads as well, so that each read completes at a \r\n. - if (lineCount > 1) - { - // ProcessAliases() is supposed to end each line with \r\n. If it doesn't we might as well fail-fast. - const auto firstLineEnd = input.find(UNICODE_LINEFEED) + 1; - input = input.substr(0, std::min(input.size(), firstLineEnd)); - } + // doskey aliases may result in multiple lines of output (for instance `doskey test=echo foo$Techo bar$Techo baz`). + // We need to emit them as multiple cooked reads as well, so that each read completes at a \r\n. + if (lineCount > 1) + { + // ProcessAliases() is supposed to end each line with \r\n. If it doesn't we might as well fail-fast. + const auto firstLineEnd = input.find(UNICODE_LINEFEED) + 1; + input = input.substr(0, std::min(input.size(), firstLineEnd)); } } @@ -722,6 +736,12 @@ void COOKED_READ_DATA::_handlePostCharInputLoop(const bool isUnicode, size_t& nu controlKeyState = _controlKeyState; } +void COOKED_READ_DATA::_transitionState(State state) noexcept +{ + assert(_state == State::Accumulating); + _state = state; +} + // Signals to _flushBuffer() that the contents of _buffer are stale and need to be redrawn. // ALL _buffer and _bufferCursor changes must be flagged with _markAsDirty(). // @@ -733,6 +753,7 @@ void COOKED_READ_DATA::_markAsDirty() } // Draws the contents of _buffer onto the screen. +// NOTE: Don't call _flushBuffer() after appending newlines to the buffer! See _handlePostCharInputLoop for more information. void COOKED_READ_DATA::_flushBuffer() { // _flushBuffer() is called often and is a good place to assert() that our _bufferCursor is still in bounds. @@ -1043,12 +1064,12 @@ void COOKED_READ_DATA::_popupsDone() _screenInfo.GetTextBuffer().GetCursor().SetIsPopupShown(false); } -bool COOKED_READ_DATA::_popupHandleInput(wchar_t wch, uint16_t vkey, DWORD modifiers) +void COOKED_READ_DATA::_popupHandleInput(wchar_t wch, uint16_t vkey, DWORD modifiers) { if (_popups.empty()) { assert(false); // Don't call this function. - return false; + return; } auto& popup = _popups.back(); @@ -1056,17 +1077,18 @@ bool COOKED_READ_DATA::_popupHandleInput(wchar_t wch, uint16_t vkey, DWORD modif { case PopupKind::CopyToChar: _popupHandleCopyToCharInput(popup, wch, vkey, modifiers); - return false; + break; case PopupKind::CopyFromChar: _popupHandleCopyFromCharInput(popup, wch, vkey, modifiers); - return false; + break; case PopupKind::CommandNumber: _popupHandleCommandNumberInput(popup, wch, vkey, modifiers); - return false; + break; case PopupKind::CommandList: - return _popupHandleCommandListInput(popup, wch, vkey, modifiers); + _popupHandleCommandListInput(popup, wch, vkey, modifiers); + break; default: - return false; + break; } } @@ -1166,7 +1188,7 @@ void COOKED_READ_DATA::_popupHandleCommandNumberInput(Popup& popup, const wchar_ } } -bool COOKED_READ_DATA::_popupHandleCommandListInput(Popup& popup, const wchar_t wch, const uint16_t vkey, const DWORD modifiers) +void COOKED_READ_DATA::_popupHandleCommandListInput(Popup& popup, const wchar_t wch, const uint16_t vkey, const DWORD modifiers) { auto& cl = popup.commandList; @@ -1174,30 +1196,31 @@ bool COOKED_READ_DATA::_popupHandleCommandListInput(Popup& popup, const wchar_t { _buffer.assign(_history->RetrieveNth(cl.selected)); _popupsDone(); - return _handleChar(UNICODE_CARRIAGERETURN, modifiers); + _handleChar(UNICODE_CARRIAGERETURN, modifiers); + return; } switch (vkey) { case VK_ESCAPE: _popupsDone(); - return false; + return; case VK_F9: _popupPush(PopupKind::CommandNumber); - return false; + return; case VK_DELETE: _history->Remove(cl.selected); if (_history->GetNumberOfCommands() <= 0) { _popupsDone(); - return false; + return; } break; case VK_LEFT: case VK_RIGHT: _replaceBuffer(_history->RetrieveNth(cl.selected)); _popupsDone(); - return false; + return; case VK_UP: if (WI_IsFlagSet(modifiers, SHIFT_PRESSED)) { @@ -1230,11 +1253,11 @@ bool COOKED_READ_DATA::_popupHandleCommandListInput(Popup& popup, const wchar_t cl.selected += popup.contentRect.height(); break; default: - return false; + return; } _popupDrawCommandList(popup); - return false; + return; } void COOKED_READ_DATA::_popupDrawPrompt(const Popup& popup, const UINT id) const diff --git a/src/host/readDataCooked.hpp b/src/host/readDataCooked.hpp index 8f1e5849e81..803724b6f52 100644 --- a/src/host/readDataCooked.hpp +++ b/src/host/readDataCooked.hpp @@ -41,6 +41,13 @@ class COOKED_READ_DATA final : public ReadData private: static constexpr uint8_t CommandNumberMaxInputLength = 5; + enum class State : uint8_t + { + Accumulating = 0, + DoneWithWakeupMask, + DoneWithCarriageReturn, + }; + enum class PopupKind { // Copies text from the previous command between the current cursor position and the first instance @@ -106,11 +113,11 @@ class COOKED_READ_DATA final : public ReadData static size_t _wordPrev(const std::wstring_view& chars, size_t position); static size_t _wordNext(const std::wstring_view& chars, size_t position); - const std::wstring_view& _newlineSuffix() const noexcept; - bool _readCharInputLoop(); - bool _handleChar(wchar_t wch, DWORD modifiers); + void _readCharInputLoop(); + void _handleChar(wchar_t wch, DWORD modifiers); void _handleVkey(uint16_t vkey, DWORD modifiers); void _handlePostCharInputLoop(bool isUnicode, size_t& numBytes, ULONG& controlKeyState); + void _transitionState(State state) noexcept; void _markAsDirty(); void _flushBuffer(); void _erase(ptrdiff_t distance) const; @@ -124,8 +131,8 @@ class COOKED_READ_DATA final : public ReadData void _popupHandleCopyToCharInput(Popup& popup, wchar_t wch, uint16_t vkey, DWORD modifiers); void _popupHandleCopyFromCharInput(Popup& popup, wchar_t wch, uint16_t vkey, DWORD modifiers); void _popupHandleCommandNumberInput(Popup& popup, wchar_t wch, uint16_t vkey, DWORD modifiers); - bool _popupHandleCommandListInput(Popup& popup, wchar_t wch, uint16_t vkey, DWORD modifiers); - bool _popupHandleInput(wchar_t wch, uint16_t vkey, DWORD keyState); + void _popupHandleCommandListInput(Popup& popup, wchar_t wch, uint16_t vkey, DWORD modifiers); + void _popupHandleInput(wchar_t wch, uint16_t vkey, DWORD keyState); void _popupDrawPrompt(const Popup& popup, UINT id) const; void _popupDrawCommandList(Popup& popup) const; @@ -140,10 +147,15 @@ class COOKED_READ_DATA final : public ReadData std::wstring _buffer; size_t _bufferCursor = 0; + // _distanceCursor is the distance between the start of the prompt and the + // current cursor location in columns (including wide glyph padding columns). ptrdiff_t _distanceCursor = 0; + // _distanceEnd is the distance between the start of the prompt and its last + // glyph at the end in columns (including wide glyph padding columns). ptrdiff_t _distanceEnd = 0; bool _bufferDirty = false; bool _insertMode = false; + State _state = State::Accumulating; std::vector _popups; }; From 74748394c17c168843b511dd837268445e5dfd6c Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 26 Sep 2023 02:28:51 +0200 Subject: [PATCH 4/4] Reimplement TextBuffer::Reflow (#15701) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Subjectively speaking, this commit makes 3 improvements: * Most importantly, it now would work with arbitrary Unicode text. (No more `IsGlyphFullWidth` or DBCS handling during reflow.) * Due to the simpler implementation it hopefully makes review of future changes and maintenance simpler. (~3x less LOC.) * It improves perf. by 1-2 orders of magnitude. (At 120x9001 with a full buffer I get 60ms -> 2ms.) Unfortunately, I'm not confident that the new code replicates the old code exactly, because I failed to understand it. During development I simply tried to match its behavior with what I think reflow should do. Closes #797 Closes #3088 Closes #4968 Closes #6546 Closes #6901 Closes #15964 Closes MSFT:19446208 Related to #5800 and #8000 ## Validation Steps Performed * Unit tests ✅ * Feature tests ✅ * Reflow with a scrollback ✅ * Reflowing the cursor cell causes a forced line-wrap ✅ (Even at the end of the buffer. ✅) * `color 8f` and reflowing retains the background color ✅ * Enter alt buffer, Resize window, Exit alt buffer ✅ --- .github/actions/spelling/expect/expect.txt | 8 +- src/buffer/out/Row.cpp | 42 +- src/buffer/out/Row.hpp | 1 + src/buffer/out/textBuffer.cpp | 470 ++++++++---------- src/buffer/out/textBuffer.hpp | 9 +- src/buffer/out/ut_textbuffer/ReflowTests.cpp | 223 ++++----- src/cascadia/TerminalCore/Terminal.cpp | 148 ++---- src/cascadia/TerminalCore/TerminalApi.cpp | 50 +- src/host/VtIo.cpp | 34 -- src/host/VtIo.hpp | 3 - src/host/screenInfo.cpp | 94 ++-- src/host/ut_host/ApiRoutinesTests.cpp | 2 +- src/host/ut_host/ScreenBufferTests.cpp | 4 +- src/host/ut_host/TextBufferTests.cpp | 8 +- src/inc/til/rle.h | 12 +- src/renderer/vt/invalidate.cpp | 16 +- src/renderer/vt/state.cpp | 29 -- src/renderer/vt/vtrenderer.hpp | 1 - .../adapter/ut_adapter/adapterTest.cpp | 10 +- src/types/UiaTextRangeBase.cpp | 2 +- 20 files changed, 464 insertions(+), 702 deletions(-) diff --git a/.github/actions/spelling/expect/expect.txt b/.github/actions/spelling/expect/expect.txt index af587b42b67..7505061fab2 100644 --- a/.github/actions/spelling/expect/expect.txt +++ b/.github/actions/spelling/expect/expect.txt @@ -749,7 +749,6 @@ gfx GGI GHIJK GHIJKL -GHIJKLM gitfilters gitmodules gle @@ -1000,7 +999,6 @@ listptrsize lld llx LMENU -LMNOP lnk lnkd lnkfile @@ -1223,7 +1221,6 @@ nonspace NOOWNERZORDER Nop NOPAINT -NOPQRST noprofile NOREDRAW NOREMOVE @@ -1498,7 +1495,6 @@ pwsz pythonw Qaabbcc qos -QRSTU QUERYOPEN QUESTIONMARK quickedit @@ -1861,7 +1857,6 @@ testname TESTNULL testpass testpasses -testtimeout TEXCOORD texel TExpected @@ -2019,7 +2014,6 @@ utext UText UTEXT utr -UVWX UVWXY UVWXYZ uwa @@ -2078,7 +2072,6 @@ VTRGBTo vtseq vtterm vttest -VWX waitable WANSUNG WANTARROWS @@ -2274,6 +2267,7 @@ YCast YCENTER YCount YDPI +YLimit YOffset YSubstantial YVIRTUALSCREEN diff --git a/src/buffer/out/Row.cpp b/src/buffer/out/Row.cpp index 6c1c136e947..4036b991d81 100644 --- a/src/buffer/out/Row.cpp +++ b/src/buffer/out/Row.cpp @@ -152,11 +152,15 @@ til::CoordType CharToColumnMapper::GetTrailingColumnAt(ptrdiff_t offset) noexcep return col; } +// If given a pointer inside the ROW's text buffer, this function will return the corresponding column. +// This function in particular returns the glyph's first column. til::CoordType CharToColumnMapper::GetLeadingColumnAt(const wchar_t* str) noexcept { return GetLeadingColumnAt(str - _chars); } +// If given a pointer inside the ROW's text buffer, this function will return the corresponding column. +// This function in particular returns the glyph's last column (this matters for wide glyphs). til::CoordType CharToColumnMapper::GetTrailingColumnAt(const wchar_t* str) noexcept { return GetTrailingColumnAt(str - _chars); @@ -364,11 +368,16 @@ void ROW::TransferAttributes(const til::small_rle& a void ROW::CopyFrom(const ROW& source) { - RowCopyTextFromState state{ .source = source }; - CopyTextFrom(state); - TransferAttributes(source.Attributes(), _columnCount); _lineRendition = source._lineRendition; _wrapForced = source._wrapForced; + + RowCopyTextFromState state{ + .source = source, + .sourceColumnLimit = source.GetReadableColumnCount(), + }; + CopyTextFrom(state); + + TransferAttributes(source.Attributes(), _columnCount); } // Returns the previous possible cursor position, preceding the given column. @@ -382,7 +391,17 @@ til::CoordType ROW::NavigateToPrevious(til::CoordType column) const noexcept // Returns the row width if column is beyond the width of the row. til::CoordType ROW::NavigateToNext(til::CoordType column) const noexcept { - return _adjustForward(_clampedColumn(column + 1)); + return _adjustForward(_clampedColumnInclusive(column + 1)); +} + +// Returns the starting column of the glyph at the given column. +// In other words, if you have 3 wide glyphs +// AA BB CC +// 01 23 45 <-- column +// then `AdjustToGlyphStart(3)` returns 2. +til::CoordType ROW::AdjustToGlyphStart(til::CoordType column) const noexcept +{ + return _adjustBackward(_clampedColumn(column)); } // Routine Description: @@ -719,11 +738,12 @@ try if (sourceColBeg < sourceColLimit) { charOffsets = source._charOffsets.subspan(sourceColBeg, static_cast(sourceColLimit) - sourceColBeg + 1); - const auto charsOffset = charOffsets.front() & CharOffsetsMask; + const auto beg = size_t{ charOffsets.front() } & CharOffsetsMask; + const auto end = size_t{ charOffsets.back() } & CharOffsetsMask; // We _are_ using span. But C++ decided that string_view and span aren't convertible. // _chars is a std::span for performance and because it refers to raw, shared memory. #pragma warning(suppress : 26481) // Don't use pointer arithmetic. Use span instead (bounds.1). - chars = { source._chars.data() + charsOffset, source._chars.size() - charsOffset }; + chars = { source._chars.data() + beg, end - beg }; } WriteHelper h{ *this, state.columnBegin, state.columnLimit, chars }; @@ -939,6 +959,16 @@ til::CoordType ROW::MeasureLeft() const noexcept til::CoordType ROW::MeasureRight() const noexcept { + if (_wrapForced) + { + auto width = _columnCount; + if (_doubleBytePadded) + { + width--; + } + return width; + } + const auto text = GetText(); const auto beg = text.begin(); const auto end = text.end(); diff --git a/src/buffer/out/Row.hpp b/src/buffer/out/Row.hpp index 586aa12b95b..29f7fe18b8c 100644 --- a/src/buffer/out/Row.hpp +++ b/src/buffer/out/Row.hpp @@ -136,6 +136,7 @@ class ROW final til::CoordType NavigateToPrevious(til::CoordType column) const noexcept; til::CoordType NavigateToNext(til::CoordType column) const noexcept; + til::CoordType AdjustToGlyphStart(til::CoordType column) const noexcept; void ClearCell(til::CoordType column); OutputCellIterator WriteCells(OutputCellIterator it, til::CoordType columnBegin, std::optional wrap = std::nullopt, std::optional limitRight = std::nullopt); diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 84336eaf03e..26c8152ea8e 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -806,9 +806,9 @@ void TextBuffer::IncrementCircularBuffer(const TextAttribute& fillAttributes) // - The viewport //Return value: // - Coordinate position (relative to the text buffer) -til::point TextBuffer::GetLastNonSpaceCharacter(std::optional viewOptional) const +til::point TextBuffer::GetLastNonSpaceCharacter(const Viewport* viewOptional) const { - const auto viewport = viewOptional.has_value() ? viewOptional.value() : GetSize(); + const auto viewport = viewOptional ? *viewOptional : GetSize(); til::point coordEndOfText; // Search the given viewport by starting at the bottom. @@ -1070,46 +1070,40 @@ void TextBuffer::Reset() noexcept // - newSize - new size of screen. // Return Value: // - Success if successful. Invalid parameter if screen buffer size is unexpected. No memory if allocation failed. -[[nodiscard]] NTSTATUS TextBuffer::ResizeTraditional(til::size newSize) noexcept +void TextBuffer::ResizeTraditional(til::size newSize) { // Guard against resizing the text buffer to 0 columns/rows, which would break being able to insert text. newSize.width = std::max(newSize.width, 1); newSize.height = std::max(newSize.height, 1); - try - { - TextBuffer newBuffer{ newSize, _currentAttributes, 0, false, _renderer }; - const auto cursorRow = GetCursor().GetPosition().y; - const auto copyableRows = std::min(_height, newSize.height); - til::CoordType srcRow = 0; - til::CoordType dstRow = 0; - - if (cursorRow >= newSize.height) - { - srcRow = cursorRow - newSize.height + 1; - } + TextBuffer newBuffer{ newSize, _currentAttributes, 0, false, _renderer }; + const auto cursorRow = GetCursor().GetPosition().y; + const auto copyableRows = std::min(_height, newSize.height); + til::CoordType srcRow = 0; + til::CoordType dstRow = 0; - for (; dstRow < copyableRows; ++dstRow, ++srcRow) - { - newBuffer.GetMutableRowByOffset(dstRow).CopyFrom(GetRowByOffset(srcRow)); - } - - // NOTE: Keep this in sync with _reserve(). - _buffer = std::move(newBuffer._buffer); - _bufferEnd = newBuffer._bufferEnd; - _commitWatermark = newBuffer._commitWatermark; - _initialAttributes = newBuffer._initialAttributes; - _bufferRowStride = newBuffer._bufferRowStride; - _bufferOffsetChars = newBuffer._bufferOffsetChars; - _bufferOffsetCharOffsets = newBuffer._bufferOffsetCharOffsets; - _width = newBuffer._width; - _height = newBuffer._height; + if (cursorRow >= newSize.height) + { + srcRow = cursorRow - newSize.height + 1; + } - _SetFirstRowIndex(0); + for (; dstRow < copyableRows; ++dstRow, ++srcRow) + { + newBuffer.GetMutableRowByOffset(dstRow).CopyFrom(GetRowByOffset(srcRow)); } - CATCH_RETURN(); - return S_OK; + // NOTE: Keep this in sync with _reserve(). + _buffer = std::move(newBuffer._buffer); + _bufferEnd = newBuffer._bufferEnd; + _commitWatermark = newBuffer._commitWatermark; + _initialAttributes = newBuffer._initialAttributes; + _bufferRowStride = newBuffer._bufferRowStride; + _bufferOffsetChars = newBuffer._bufferOffsetChars; + _bufferOffsetCharOffsets = newBuffer._bufferOffsetCharOffsets; + _width = newBuffer._width; + _height = newBuffer._height; + + _SetFirstRowIndex(0); } void TextBuffer::SetAsActiveBuffer(const bool isActiveBuffer) noexcept @@ -2412,204 +2406,181 @@ void TextBuffer::_AppendRTFText(std::ostringstream& contentBuilder, const std::w // the new buffer. The rows's new value is placed back into this parameter. // Return Value: // - S_OK if we successfully copied the contents to the new buffer, otherwise an appropriate HRESULT. -HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer, - TextBuffer& newBuffer, - const std::optional lastCharacterViewport, - std::optional> positionInfo) -try +void TextBuffer::Reflow(TextBuffer& oldBuffer, TextBuffer& newBuffer, const Viewport* lastCharacterViewport, PositionInformation* positionInfo) { const auto& oldCursor = oldBuffer.GetCursor(); auto& newCursor = newBuffer.GetCursor(); - // 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. - const auto cOldCursorPos = oldCursor.GetPosition(); - const auto cOldLastChar = oldBuffer.GetLastNonSpaceCharacter(lastCharacterViewport); - - const auto cOldRowsTotal = cOldLastChar.y + 1; - - til::point cNewCursorPos; - auto fFoundCursorPos = false; - auto foundOldMutable = false; - auto foundOldVisible = false; - // Loop through all the rows of the old buffer and reprint them into the new buffer - til::CoordType iOldRow = 0; - for (; iOldRow < cOldRowsTotal; iOldRow++) - { - // Fetch the row and its "right" which is the last printable character. - const auto& row = oldBuffer.GetRowByOffset(iOldRow); - const auto cOldColsTotal = oldBuffer.GetLineWidth(iOldRow); - auto iRight = row.MeasureRight(); - - // If we're starting a new row, try and preserve the line rendition - // from the row in the original buffer. - const auto newBufferPos = newCursor.GetPosition(); - if (newBufferPos.x == 0) - { - auto& newRow = newBuffer.GetMutableRowByOffset(newBufferPos.y); - newRow.SetLineRendition(row.GetLineRendition()); - } + til::point oldCursorPos = oldCursor.GetPosition(); + til::point newCursorPos; + + // BODGY: We use oldCursorPos in two critical places below: + // * To compute an oldHeight that includes at a minimum the cursor row + // * For REFLOW_JANK_CURSOR_WRAP (see comment below) + // Both of these would break the reflow algorithm, but the latter of the two in particular + // would cause the main copy loop below to deadlock. In other words, these two lines + // protect this function against yet-unknown bugs in other parts of the code base. + oldCursorPos.x = std::clamp(oldCursorPos.x, 0, oldBuffer._width - 1); + oldCursorPos.y = std::clamp(oldCursorPos.y, 0, oldBuffer._height - 1); + + const auto lastRowWithText = oldBuffer.GetLastNonSpaceCharacter(lastCharacterViewport).y; + + auto mutableViewportTop = positionInfo ? positionInfo->mutableViewportTop : til::CoordTypeMax; + auto visibleViewportTop = positionInfo ? positionInfo->visibleViewportTop : til::CoordTypeMax; + + til::CoordType oldY = 0; + til::CoordType newY = 0; + til::CoordType newX = 0; + til::CoordType newWidth = newBuffer.GetSize().Width(); + til::CoordType newYLimit = til::CoordTypeMax; + + const auto oldHeight = std::max(lastRowWithText, oldCursorPos.y) + 1; + const auto newHeight = newBuffer.GetSize().Height(); + const auto newWidthU16 = gsl::narrow_cast(newWidth); - // 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 (row.WasWrapForced()) + // Copy oldBuffer into newBuffer until oldBuffer has been fully consumed. + for (; oldY < oldHeight && newY < newYLimit; ++oldY) + { + const auto& oldRow = oldBuffer.GetRowByOffset(oldY); + + // A pair of double height rows should optimally wrap as a union (i.e. after wrapping there should be 4 lines). + // But for this initial implementation I chose the alternative approach: Just truncate them. + if (oldRow.GetLineRendition() != LineRendition::SingleWidth) { - 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 (row.WasDoubleBytePadded()) + // Since rows with a non-standard line rendition should be truncated it's important + // that we pretend as if the previous row ended in a newline, even if it didn't. + // This is what this if does: It newlines. + if (newX) { - iRight--; + newX = 0; + newY++; } - } - // Loop through every character in the current row (up to - // the "right" boundary, which is one past the final valid - // character) - til::CoordType iOldCol = 0; - const auto copyRight = iRight; - for (; iOldCol < copyRight; iOldCol++) - { - if (iOldCol == cOldCursorPos.x && iOldRow == cOldCursorPos.y) + auto& newRow = newBuffer.GetMutableRowByOffset(newY); + + // See the comment marked with "REFLOW_RESET". + if (newY >= newHeight) { - cNewCursorPos = newCursor.GetPosition(); - fFoundCursorPos = true; + newRow.Reset(newBuffer._initialAttributes); } - // TODO: MSFT: 19446208 - this should just use an iterator and the inserter... - const auto glyph = row.GlyphAt(iOldCol); - const auto dbcsAttr = row.DbcsAttrAt(iOldCol); - const auto textAttr = row.GetAttrByColumn(iOldCol); + newRow.CopyFrom(oldRow); + newRow.SetWrapForced(false); - newBuffer.InsertCharacter(glyph, dbcsAttr, textAttr); + if (oldY == oldCursorPos.y) + { + newCursorPos = { newRow.AdjustToGlyphStart(oldCursorPos.x), newY }; + } + if (oldY >= mutableViewportTop) + { + positionInfo->mutableViewportTop = newY; + mutableViewportTop = til::CoordTypeMax; + } + if (oldY >= visibleViewportTop) + { + positionInfo->visibleViewportTop = newY; + visibleViewportTop = til::CoordTypeMax; + } + + newY++; + continue; } - // GH#32: Copy the attributes from the rest of the row into this new buffer. - // From where we are in the old buffer, to the end of the row, copy the - // remaining attributes. - // - if the old buffer is smaller than the new buffer, then just copy - // what we have, as it was. We already copied all _text_ with colors, - // but it's possible for someone to just put some color into the - // buffer to the right of that without any text (as just spaces). The - // buffer looks weird to the user when we resize and it starts losing - // those colors, so we need to copy them over too... as long as there - // is space. The last attr in the row will be extended to the end of - // the row in the new buffer. - // - if the old buffer is WIDER, than we might have wrapped onto a new - // line. Use the cursor's position's Y so that we know where the new - // row is, and start writing at the cursor position. Again, the attr - // in the last column of the old row will be extended to the end of the - // row that the text was flowed onto. - // - if the text in the old buffer didn't actually fill the whole - // line in the new buffer, then we didn't wrap. That's fine. just - // copy attributes from the old row till the end of the new row, and - // move on. - const auto newRowY = newCursor.GetPosition().y; - auto& newRow = newBuffer.GetMutableRowByOffset(newRowY); - auto newAttrColumn = newCursor.GetPosition().x; - const auto newWidth = newBuffer.GetLineWidth(newRowY); - // Stop when we get to the end of the buffer width, or the new position - // for inserting an attr would be past the right of the new buffer. - for (auto copyAttrCol = iOldCol; - copyAttrCol < cOldColsTotal && newAttrColumn < newWidth; - copyAttrCol++, newAttrColumn++) + // Rows don't store any information for what column the last written character is in. + // We simply truncate all trailing whitespace in this implementation. + auto oldRowLimit = oldRow.MeasureRight(); + if (oldY == oldCursorPos.y) { - // TODO: MSFT: 19446208 - this should just use an iterator and the inserter... - const auto textAttr = row.GetAttrByColumn(copyAttrCol); - newRow.SetAttrToEnd(newAttrColumn, textAttr); + // REFLOW_JANK_CURSOR_WRAP: + // Pretending as if there's always at least whitespace in front of the cursor has the benefit that + // * the cursor retains its distance from any preceding text. + // * when a client application starts writing on this new, empty line, + // enlarging the buffer unwraps the text onto the preceding line. + oldRowLimit = std::max(oldRowLimit, oldCursorPos.x + 1); } - // If we found the old row that the caller was interested in, set the - // out value of that parameter to the cursor's current Y position (the - // new location of the _end_ of that row in the buffer). - if (positionInfo.has_value()) + til::CoordType oldX = 0; + + // Copy oldRow into newBuffer until oldRow has been fully consumed. + // We use a do-while loop to ensure that line wrapping occurs and + // that attributes are copied over even for seemingly empty rows. + do { - if (!foundOldMutable) + // This if condition handles line wrapping. + // Only if we write past the last column we should wrap and as such this if + // condition is in front of the text insertion code instead of behind it. + // A SetWrapForced of false implies an explicit newline, which is the default. + if (newX >= newWidth) { - if (iOldRow >= positionInfo.value().get().mutableViewportTop) - { - positionInfo.value().get().mutableViewportTop = newCursor.GetPosition().y; - foundOldMutable = true; - } + newBuffer.GetMutableRowByOffset(newY).SetWrapForced(true); + newX = 0; + newY++; } - if (!foundOldVisible) + // REFLOW_RESET: + // If we shrink the buffer vertically, for instance from 100 rows to 90 rows, we will write 10 rows in the + // new buffer twice. We need to reset them before copying text, or otherwise we'll see the previous contents. + // We don't need to be smart about this. Reset() is fast and shrinking doesn't occur often. + if (newY >= newHeight && newX == 0) { - if (iOldRow >= positionInfo.value().get().visibleViewportTop) + // We need to ensure not to overwrite the row the cursor is on. + if (newY >= newYLimit) { - positionInfo.value().get().visibleViewportTop = newCursor.GetPosition().y; - foundOldVisible = true; + break; } + newBuffer.GetMutableRowByOffset(newY).Reset(newBuffer._initialAttributes); } - } - // 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 && !row.WasWrapForced()) - { - if (!fFoundCursorPos && (iRight == cOldCursorPos.x && iOldRow == cOldCursorPos.y)) + auto& newRow = newBuffer.GetMutableRowByOffset(newY); + + RowCopyTextFromState state{ + .source = oldRow, + .columnBegin = newX, + .columnLimit = til::CoordTypeMax, + .sourceColumnBegin = oldX, + .sourceColumnLimit = oldRowLimit, + }; + newRow.CopyTextFrom(state); + + const auto& oldAttr = oldRow.Attributes(); + auto& newAttr = newRow.Attributes(); + const auto attributes = oldAttr.slice(gsl::narrow_cast(oldX), oldAttr.size()); + newAttr.replace(gsl::narrow_cast(newX), newAttr.size(), attributes); + newAttr.resize_trailing_extent(newWidthU16); + + if (oldY == oldCursorPos.y && oldCursorPos.x >= oldX) { - cNewCursorPos = newCursor.GetPosition(); - fFoundCursorPos = true; + // In theory AdjustToGlyphStart ensures we don't put the cursor on a trailing wide glyph. + // In practice I don't think that this can possibly happen. Better safe than sorry. + newCursorPos = { newRow.AdjustToGlyphStart(oldCursorPos.x - oldX + newX), newY }; + // If there's so much text past the old cursor position that it doesn't fit into new buffer, + // then the new cursor position will be "lost", because it's overwritten by unrelated text. + // We have two choices how can handle this: + // * If the new cursor is at an y < 0, just put the cursor at (0,0) + // * Stop writing into the new buffer before we overwrite the new cursor position + // This implements the second option. There's no fundamental reason why this is better. + newYLimit = newY + newHeight; } - // 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) + if (oldY >= mutableViewportTop) { - newBuffer.NewlineCursor(); + positionInfo->mutableViewportTop = newY; + mutableViewportTop = til::CoordTypeMax; } - else + if (oldY >= visibleViewportTop) { - // 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 auto coordNewCursor = newCursor.GetPosition(); - if (coordNewCursor.x == 0 && coordNewCursor.y > 0) - { - if (newBuffer.GetRowByOffset(coordNewCursor.y - 1).WasWrapForced()) - { - newBuffer.NewlineCursor(); - } - } + positionInfo->visibleViewportTop = newY; + visibleViewportTop = til::CoordTypeMax; } + + oldX = state.sourceColumnEnd; + newX = state.columnEnd; + } while (oldX < oldRowLimit); + + // If the row had an explicit newline we also need to newline. :) + if (!oldRow.WasWrapForced()) + { + newX = 0; + newY++; } } @@ -2617,85 +2588,40 @@ try // printable character. This is to fix the `color 2f` scenario, where you // change the buffer colors then resize and everything below the last // printable char gets reset. See GH #12567 - auto newRowY = newCursor.GetPosition().y + 1; - const auto newHeight = newBuffer.GetSize().Height(); - const auto oldHeight = oldBuffer._estimateOffsetOfLastCommittedRow() + 1; - for (; - iOldRow < oldHeight && newRowY < newHeight; - iOldRow++) + const auto initializedRowsEnd = oldBuffer._estimateOffsetOfLastCommittedRow() + 1; + for (; oldY < initializedRowsEnd && newY < newHeight; oldY++, newY++) { - const auto& row = oldBuffer.GetRowByOffset(iOldRow); - - // Optimization: Since all these rows are below the last printable char, - // we can reasonably assume that they are filled with just spaces. - // That's convenient, we can just copy the attr row from the old buffer - // into the new one, and resize the row to match. We'll rely on the - // behavior of ATTR_ROW::Resize to trim down when narrower, or extend - // the last attr when wider. - auto& newRow = newBuffer.GetMutableRowByOffset(newRowY); - const auto newWidth = newBuffer.GetLineWidth(newRowY); - newRow.TransferAttributes(row.Attributes(), newWidth); - - newRowY++; + auto& oldRow = oldBuffer.GetRowByOffset(oldY); + auto& newRow = newBuffer.GetMutableRowByOffset(newY); + auto& newAttr = newRow.Attributes(); + newAttr = oldRow.Attributes(); + newAttr.resize_trailing_extent(newWidthU16); } - // Finish copying remaining parameters from the old text buffer to the new one - newBuffer.CopyProperties(oldBuffer); - newBuffer.CopyHyperlinkMaps(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) + // Since we didn't use IncrementCircularBuffer() we need to compute the proper + // _firstRow offset now, in a way that replicates IncrementCircularBuffer(). + // We need to do the same for newCursorPos.y for basically the same reason. + if (newY > newHeight) { - newCursor.SetPosition(cNewCursorPos); + newBuffer._firstRow = newY % newHeight; + // _firstRow maps from API coordinates that always start at 0,0 in the top left corner of the + // terminal's scrollback, to the underlying buffer Y coordinate via `(y + _firstRow) % height`. + // Here, we need to un-map the `newCursorPos.y` from the underlying Y coordinate to the API coordinate + // and so we do `(y - _firstRow) % height`, but we add `+ newHeight` to avoid getting negative results. + newCursorPos.y = (newCursorPos.y - newBuffer._firstRow + newHeight) % newHeight; } - 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 - auto iNewlines = cOldCursorPos.y - cOldLastChar.y; - const auto iIncrements = cOldCursorPos.x - cOldLastChar.x; - const auto 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).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).WasWrapForced()) - { - iNewlines = std::max(iNewlines - 1, 0); - } - } - - for (auto r = 0; r < iNewlines; r++) - { - newBuffer.NewlineCursor(); - } - for (auto c = 0; c < iIncrements - 1; c++) - { - newBuffer.IncrementCursor(); - } - } - - // Save old cursor size before we delete it - const auto ulSize = oldCursor.GetSize(); + newBuffer.CopyProperties(oldBuffer); + newBuffer.CopyHyperlinkMaps(oldBuffer); - // Set size back to real size as it will be taking over the rendering duties. - newCursor.SetSize(ulSize); + assert(newCursorPos.x >= 0 && newCursorPos.x < newWidth); + assert(newCursorPos.y >= 0 && newCursorPos.y < newHeight); + newCursor.SetSize(oldCursor.GetSize()); + newCursor.SetPosition(newCursorPos); newBuffer._marks = oldBuffer._marks; newBuffer._trimMarksOutsideBuffer(); - - return S_OK; } -CATCH_RETURN() // Method Description: // - Adds or updates a hyperlink in our hyperlink table @@ -2916,14 +2842,10 @@ void TextBuffer::AddMark(const ScrollMark& m) void TextBuffer::_trimMarksOutsideBuffer() { - const auto height = GetSize().Height(); - _marks.erase(std::remove_if(_marks.begin(), - _marks.end(), - [height](const auto& m) { - return (m.start.y < 0) || - (m.start.y >= height); - }), - _marks.end()); + const til::CoordType height = _height; + std::erase_if(_marks, [height](const auto& m) { + return (m.start.y < 0) || (m.start.y >= height); + }); } std::wstring_view TextBuffer::CurrentCommand() const diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index 360c74aab8c..a9d2e1714b6 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -163,7 +163,7 @@ class TextBuffer final // Scroll needs access to this to quickly rotate around the buffer. void IncrementCircularBuffer(const TextAttribute& fillAttributes = {}); - til::point GetLastNonSpaceCharacter(std::optional viewOptional = std::nullopt) const; + til::point GetLastNonSpaceCharacter(const Microsoft::Console::Types::Viewport* viewOptional = nullptr) const; Cursor& GetCursor() noexcept; const Cursor& GetCursor() const noexcept; @@ -194,7 +194,7 @@ class TextBuffer final void Reset() noexcept; - [[nodiscard]] HRESULT ResizeTraditional(const til::size newSize) noexcept; + void ResizeTraditional(const til::size newSize); void SetAsActiveBuffer(const bool isActiveBuffer) noexcept; bool IsActiveBuffer() const noexcept; @@ -262,10 +262,7 @@ class TextBuffer final til::CoordType visibleViewportTop{ 0 }; }; - static HRESULT Reflow(TextBuffer& oldBuffer, - TextBuffer& newBuffer, - const std::optional lastCharacterViewport, - std::optional> positionInfo); + static void Reflow(TextBuffer& oldBuffer, TextBuffer& newBuffer, const Microsoft::Console::Types::Viewport* lastCharacterViewport = nullptr, PositionInformation* positionInfo = nullptr); std::vector SearchText(const std::wstring_view& needle, bool caseInsensitive) const; std::vector SearchText(const std::wstring_view& needle, bool caseInsensitive, til::CoordType rowBeg, til::CoordType rowEnd) const; diff --git a/src/buffer/out/ut_textbuffer/ReflowTests.cpp b/src/buffer/out/ut_textbuffer/ReflowTests.cpp index 3c99a9fb583..d1fad4c407f 100644 --- a/src/buffer/out/ut_textbuffer/ReflowTests.cpp +++ b/src/buffer/out/ut_textbuffer/ReflowTests.cpp @@ -46,8 +46,6 @@ namespace std::vector buffers; }; - static constexpr auto true_due_to_exact_wrap_bug{ true }; - static const TestCase testCases[] = { TestCase{ L"No reflow required", @@ -61,7 +59,7 @@ namespace { L"EFG ", false }, { L" ", false }, }, - { 0, 1 } // cursor on $ + { 0, 1 }, // cursor on $ }, TestBuffer{ { 5, 5 }, // reduce width by 1 @@ -72,7 +70,7 @@ namespace { L"EFG ", false }, { L" ", false }, }, - { 0, 1 } // cursor on $ + { 0, 1 }, // cursor on $ }, TestBuffer{ { 4, 5 }, @@ -83,7 +81,7 @@ namespace { L"EFG ", false }, { L" ", false }, }, - { 0, 1 } // cursor on $ + { 0, 1 }, // cursor on $ }, }, }, @@ -99,40 +97,40 @@ namespace { L" ", false }, { L" ", false }, }, - { 0, 1 } // cursor on $ + { 0, 1 }, // cursor on $ }, TestBuffer{ { 5, 5 }, // reduce width by 1 { { L"ABCDE", true }, - { L"F$ ", false }, // [BUG] EXACT WRAP. $ should be alone on next line. - { L" ", false }, + { L"F ", false }, + { L"$ ", false }, { L" ", false }, { L" ", false }, }, - { 1, 1 } // cursor on $ + { 0, 2 }, // cursor on $ }, TestBuffer{ { 6, 5 }, // grow width back to original { - { L"ABCDEF", true_due_to_exact_wrap_bug }, + { L"ABCDEF", false }, { L"$ ", false }, { L" ", false }, { L" ", false }, { L" ", false }, }, - { 0, 1 } // cursor on $ + { 0, 1 }, // cursor on $ }, TestBuffer{ { 7, 5 }, // grow width wider than original { - { L"ABCDEF$", true_due_to_exact_wrap_bug }, // EXACT WRAP BUG: $ should be alone on next line - { L" ", false }, + { L"ABCDEF ", false }, + { L"$ ", false }, { L" ", false }, { L" ", false }, { L" ", false }, }, - { 6, 0 } // cursor on $ + { 0, 1 }, // cursor on $ }, }, }, @@ -148,7 +146,7 @@ namespace { L" ", false }, { L" ", false }, }, - { 1, 1 } // cursor on $ + { 1, 1 }, // cursor on $ }, TestBuffer{ { 5, 5 }, // reduce width by 1 @@ -159,7 +157,7 @@ namespace { L" ", false }, { L" ", false }, }, - { 2, 1 } // cursor on $ + { 2, 1 }, // cursor on $ }, TestBuffer{ { 6, 5 }, // grow width back to original @@ -170,7 +168,7 @@ namespace { L" ", false }, { L" ", false }, }, - { 1, 1 } // cursor on $ + { 1, 1 }, // cursor on $ }, TestBuffer{ { 7, 5 }, // grow width wider than original @@ -181,7 +179,7 @@ namespace { L" ", false }, { L" ", false }, }, - { 0, 1 } // cursor on $ + { 0, 1 }, // cursor on $ }, }, }, @@ -197,29 +195,29 @@ namespace { L"EFG ", false }, { L" ", false }, }, - { 0, 1 } // cursor on $ + { 0, 1 }, // cursor on $ }, TestBuffer{ { 7, 5 }, // reduce width by 1 { { L"AB $", true }, - { L" CD", true_due_to_exact_wrap_bug }, - { L" ", false }, + { L" CD", false }, // CD ends with a newline -> .wrap = false { L"EFG ", false }, { L" ", false }, + { L" ", false }, }, - { 6, 0 } // cursor on $ + { 6, 0 }, // cursor on $ }, TestBuffer{ { 8, 5 }, { { L"AB $ ", true }, - { L" CD ", false }, // Goes to false because we hit the end of ..CD - { L"EFG ", false }, // [BUG] EFG moves up due to exact wrap bug above + { L" CD ", false }, + { L"EFG ", false }, { L" ", false }, { L" ", false }, }, - { 6, 0 } // cursor on $ + { 6, 0 }, // cursor on $ }, }, }, @@ -236,19 +234,19 @@ namespace { L" ", false }, { L" ", false }, }, - { 2, 1 } // cursor on $ + { 2, 1 }, // cursor on $ }, TestBuffer{ { 5, 5 }, // reduce width by 1 { //--012345-- { L"カタ ", true }, // KA TA [FORCED SPACER] - { L"カナ$", true_due_to_exact_wrap_bug }, // KA NA + { L"カナ$", false }, // KA NA { L" ", false }, { L" ", false }, { L" ", false }, }, - { 4, 1 } // cursor on $ + { 4, 1 }, // cursor on $ }, TestBuffer{ { 6, 5 }, // grow width back to original @@ -260,7 +258,7 @@ namespace { L" ", false }, { L" ", false }, }, - { 2, 1 } // cursor on $ + { 2, 1 }, // cursor on $ }, TestBuffer{ { 7, 5 }, // grow width wider than original (by one; no visible change!) @@ -272,7 +270,7 @@ namespace { L" ", false }, { L" ", false }, }, - { 2, 1 } // cursor on $ + { 2, 1 }, // cursor on $ }, TestBuffer{ { 8, 5 }, // grow width enough to fit second DBCS @@ -284,7 +282,7 @@ namespace { L" ", false }, { L" ", false }, }, - { 0, 1 } // cursor on $ + { 0, 1 }, // cursor on $ }, }, }, @@ -300,41 +298,29 @@ namespace { L"MNOPQR", false }, { L"STUVWX", false }, }, - { 0, 1 } // cursor on $ + { 0, 1 }, // cursor on $ }, TestBuffer{ { 5, 5 }, // reduce width by 1 { - { L"F$ ", false }, - { L"GHIJK", true }, // [BUG] We should see GHIJK\n L\n MNOPQ\n R\n - { L"LMNOP", true }, // The wrapping here is irregular - { L"QRSTU", true }, - { L"VWX ", false }, + { L"$ ", false }, + { L"GHIJK", true }, + { L"L ", false }, + { L"MNOPQ", true }, + { L"R ", false }, }, - { 1, 1 } // [BUG] cursor moves to 1,1 instead of sticking with the $ + { 0, 0 }, }, TestBuffer{ { 6, 5 }, // going back to 6,5, the data lost has been destroyed { - //{ L"F$ ", false }, // [BUG] this line is erroneously destroyed too! - { L"GHIJKL", true }, - { L"MNOPQR", true }, - { L"STUVWX", true }, + { L"$ ", false }, + { L"GHIJKL", false }, + { L"MNOPQR", false }, + { L" ", false }, { L" ", false }, - { L" ", false }, // [BUG] this line is added - }, - { 1, 1 }, // [BUG] cursor does not follow [H], it sticks at 1,1 - }, - TestBuffer{ - { 7, 5 }, // a number of errors are carried forward from the previous buffer - { - { L"GHIJKLM", true }, - { L"NOPQRST", true }, - { L"UVWX ", false }, // [BUG] This line loses wrap for some reason - { L" ", false }, - { L" ", false }, }, - { 0, 1 }, // [BUG] The cursor moves to 0, 1 now, sticking with the [N] from before + { 0, 0 }, }, }, }, @@ -353,18 +339,18 @@ namespace { L" ", false }, { L" ", false }, }, - { 1, 1 } // cursor *after* $ + { 1, 1 }, // cursor *after* $ }, TestBuffer{ { 5, 5 }, // reduce width by 1 { { L"ABCDE", true }, - { L"F$ ", false }, // [BUG] EXACT WRAP. $ should be alone on next line. - { L" ", false }, + { L"F ", false }, + { L"$ ", false }, { L" ", false }, { L" ", false }, }, - { 2, 1 } // cursor follows space after $ to next line + { 1, 2 }, // cursor follows space after $ to next line }, }, }, @@ -380,7 +366,7 @@ namespace { L"STUVWX", true }, { L"YZ0 $ ", false }, }, - { 5, 4 } // cursor *after* $ + { 5, 4 }, // cursor *after* $ }, TestBuffer{ { 5, 5 }, // reduce width by 1 @@ -391,7 +377,7 @@ namespace { L"UVWXY", true }, { L"Z0 $ ", false }, }, - { 4, 4 } // cursor follows space after $ to newly introduced bottom line + { 4, 4 }, // cursor follows space after $ to newly introduced bottom line }, }, }, @@ -402,40 +388,36 @@ namespace { 6, 5 }, { { L"ABCDEF", false }, - // v cursor { L"$ ", false }, - // ^ cursor { L" ", false }, { L" ", false }, { L" ", false }, }, - { 5, 1 } // cursor in space far after $ + { 5, 1 }, // The cursor is 5 columns to the right of the $ (last column). }, TestBuffer{ { 5, 5 }, // reduce width by 1 { { L"ABCDE", true }, - { L"F$ ", true }, // [BUG] This line is marked wrapped, and I do not know why - // v cursor - { L" ", false }, - // ^ cursor + { L"F ", false }, + // The reflow implementation marks a wrapped cursor as a forced row-wrap (= the row is padded with whitespace), so that when + // the buffer is enlarged again, we restore the original cursor position correctly. That's why it says .cursor={5,1} below. + { L"$ ", true }, { L" ", false }, { L" ", false }, }, - { 1, 2 } // cursor stays same linear distance from $ + { 0, 3 }, // $ is now at 0,2 and the cursor used to be 5 columns to the right. -> 0,3 }, TestBuffer{ { 6, 5 }, // grow back to original size { - { L"ABCDEF", true_due_to_exact_wrap_bug }, - // v cursor [BUG] cursor does not retain linear distance from $ + { L"ABCDEF", false }, { L"$ ", false }, - // ^ cursor { L" ", false }, { L" ", false }, { L" ", false }, }, - { 4, 1 } // cursor stays same linear distance from $ + { 5, 1 }, }, }, }, @@ -446,39 +428,37 @@ namespace { 6, 5 }, { { L"ABCDEF", false }, - // v cursor { L"$ ", false }, - // ^ cursor { L"BLAH ", false }, { L"BLAH ", false }, { L" ", false }, }, - { 5, 1 } // cursor in space far after $ + { 5, 1 }, // The cursor is 5 columns to the right of the $ (last column). }, TestBuffer{ { 5, 5 }, // reduce width by 1 { - { L"ABCDE", true }, - { L"F$ ", false }, - { L"BLAH ", false }, - { L"BLAH ", true }, // [BUG] this line wraps, no idea why - // v cursor [BUG] cursor erroneously moved to end of all content + { L"F ", false }, + // The reflow implementation pads the row with the cursor with whitespace. + // Search for "REFLOW_JANK_CURSOR_WRAP" to find the corresponding code. + { L"$ ", true }, { L" ", false }, - // ^ cursor + { L"BLAH ", false }, + { L"BLAH ", false }, }, - { 0, 4 } }, + { 0, 2 }, + }, TestBuffer{ { 6, 5 }, // grow back to original size { - { L"ABCDEF", true }, + { L"F ", false }, { L"$ ", false }, { L"BLAH ", false }, - // v cursor [BUG] cursor is pulled up to previous line because it was marked wrapped { L"BLAH ", false }, - // ^ cursor { L" ", false }, }, - { 5, 3 } }, + { 5, 1 }, + }, }, }, TestCase{ @@ -489,27 +469,24 @@ namespace { 6, 5 }, { { L"ABCDEF", false }, - // v cursor { L"$ ", false }, - // ^ cursor { L" ", false }, { L" ", false }, { L" ", false }, }, - { 5, 1 } // cursor in space far after $ + { 5, 1 }, // The cursor is 5 columns to the right of the $ (last column). }, TestBuffer{ { 2, 5 }, // reduce width aggressively { { L"CD", true }, - { L"EF", true }, + { L"EF", false }, { L"$ ", true }, { L" ", true }, - // v cursor { L" ", false }, - // ^ cursor }, - { 1, 4 } }, + { 1, 4 }, + }, }, }, TestCase{ @@ -519,27 +496,24 @@ namespace { 6, 5 }, { { L"ABCDEF", true }, - // v cursor { L"$ ", true }, - // ^ cursor { L" ", true }, { L" ", true }, { L" ", true }, }, - { 5, 1 } // cursor in space far after $ + { 5, 1 }, // cursor in space far after $ }, TestBuffer{ { 2, 5 }, // reduce width aggressively { - { L"EF", true }, - { L"$ ", true }, { L" ", true }, { L" ", true }, - // v cursor [BUG] cursor does not maintain linear distance from $ - { L" ", false }, - // ^ cursor + { L" ", true }, + { L" ", true }, + { L" ", true }, }, - { 1, 4 } }, + { 1, 0 }, + }, }, }, TestCase{ @@ -549,14 +523,12 @@ namespace { 6, 5 }, { { L"ABCDEF", true }, - // v cursor { L"$ ", true }, - // ^ cursor { L" ", true }, { L" ", true }, { L" Q", true }, }, - { 5, 1 } // cursor in space far after $ + { 5, 1 }, // cursor in space far after $ }, TestBuffer{ { 2, 5 }, // reduce width aggressively @@ -564,12 +536,11 @@ namespace { L" ", true }, { L" ", true }, { L" ", true }, - { L" Q", true }, - // v cursor [BUG] cursor jumps to end of world - { L" ", false }, // POTENTIAL [BUG] a whole new blank line is added for the cursor - // ^ cursor + { L" ", true }, + { L" ", true }, }, - { 1, 4 } }, + { 1, 0 }, + }, }, }, TestCase{ @@ -579,27 +550,24 @@ namespace { 6, 5 }, { { L"ABCDEF", false }, - // v cursor { L"$ ", false }, - // ^ cursor { L" ", false }, { L" ", true }, { L" Q", true }, }, - { 5, 1 } // cursor in space far after $ + { 5, 1 }, // cursor in space far after $ }, TestBuffer{ { 2, 5 }, // reduce width aggressively { + { L" ", false }, + { L" ", false }, { L" ", true }, { L" ", true }, { L" ", true }, - { L" Q", true }, - // v cursor [BUG] cursor jumps to different place than fully wrapped case - { L" ", false }, - // ^ cursor }, - { 0, 4 } }, + { 1, 0 }, + }, }, }, TestCase{ @@ -614,24 +582,21 @@ namespace { L"$ ", false }, { L" Q", true }, { L" ", true }, - // v cursor { L" ", true }, - // ^ cursor }, - { 5, 4 } // cursor at end of buffer + { 5, 4 }, // cursor at end of buffer }, TestBuffer{ { 2, 5 }, // reduce width aggressively { + { L" ", false }, + { L" ", true }, + { L" ", true }, { L" ", true }, { L" ", true }, - { L" Q", true }, - { L" ", false }, - // v cursor [BUG] cursor loses linear distance from Q; is this important? - { L" ", false }, - // ^ cursor }, - { 0, 4 } }, + { 1, 0 }, + }, }, }, }; @@ -761,7 +726,7 @@ class ReflowTests static std::unique_ptr _textBufferByReflowingTextBuffer(TextBuffer& originalBuffer, const til::size newSize) { auto buffer = std::make_unique(newSize, TextAttribute{ 0x7 }, 0, false, renderer); - TextBuffer::Reflow(originalBuffer, *buffer, std::nullopt, std::nullopt); + TextBuffer::Reflow(originalBuffer, *buffer); return buffer; } diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index b2a99833587..fe2417b1f97 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -229,6 +229,7 @@ std::wstring_view Terminal::GetWorkingDirectory() noexcept // nothing to do (the viewportSize is the same as our current size), or an // appropriate HRESULT for failing to resize. [[nodiscard]] HRESULT Terminal::UserResize(const til::size viewportSize) noexcept +try { const auto oldDimensions = _GetMutableViewport().Dimensions(); if (viewportSize == oldDimensions) @@ -236,97 +237,59 @@ std::wstring_view Terminal::GetWorkingDirectory() noexcept return S_FALSE; } - // Shortcut: if we're in the alt buffer, just resize the - // alt buffer and put off resizing the main buffer till we switch back. Fortunately, this is easy. We don't need to - // worry about the viewport and scrollback at all! The alt buffer never has - // any scrollback, so we just need to resize it and presto, we're done. if (_inAltBuffer()) { - // stash this resize for the future. + // _deferredResize will indicate to UseMainScreenBuffer() that it needs to reflow the main buffer. + // Deferring the reflow of the main buffer has the benefit that it avoids destroying the state + // of the text buffer any more than necessary. For ConPTY in particular a reflow is destructive, + // because it "forgets" text that wraps beyond the top of its viewport when shrinking it. _deferredResize = viewportSize; - _altBuffer->GetCursor().StartDeferDrawing(); - // we're capturing `this` here because when we exit, we want to EndDefer on the (newly created) active buffer. - auto endDefer = wil::scope_exit([this]() noexcept { _altBuffer->GetCursor().EndDeferDrawing(); }); - - // GH#3494: We don't need to reflow the alt buffer. Apps that use the - // alt buffer will redraw themselves. This prevents graphical artifacts. - // - // This is consistent with VTE - RETURN_IF_FAILED(_altBuffer->ResizeTraditional(viewportSize)); + // GH#3494: We don't need to reflow the alt buffer. Apps that use the alt buffer will + // redraw themselves. This prevents graphical artifacts and is consistent with VTE. + _altBuffer->ResizeTraditional(viewportSize); - // Since the _mutableViewport is no longer the size of the actual - // viewport, then update our _altBufferSize tracker we're using to help - // us out here. _altBufferSize = viewportSize; + _altBuffer->TriggerRedrawAll(); return S_OK; } - const auto dx = viewportSize.width - oldDimensions.width; - const auto newBufferHeight = std::clamp(viewportSize.height + _scrollbackLines, 0, SHRT_MAX); - - til::size bufferSize{ viewportSize.width, newBufferHeight }; - - // This will be used to determine where the viewport should be in the new buffer. - const auto oldViewportTop = _mutableViewport.Top(); - auto newViewportTop = oldViewportTop; - auto newVisibleTop = _VisibleStartIndex(); + const auto newBufferHeight = std::clamp(viewportSize.height + _scrollbackLines, 1, SHRT_MAX); + const til::size bufferSize{ viewportSize.width, newBufferHeight }; // If the original buffer had _no_ scroll offset, then we should be at the // bottom in the new buffer as well. Track that case now. const auto originalOffsetWasZero = _scrollOffset == 0; - // skip any drawing updates that might occur until we swap _buffer with the new buffer or if we exit early. - _mainBuffer->GetCursor().StartDeferDrawing(); - // we're capturing `this` here because when we exit, we want to EndDefer on the (newly created) active buffer. - auto endDefer = wil::scope_exit([this]() noexcept { _mainBuffer->GetCursor().EndDeferDrawing(); }); + // GH#3848 - We'll initialize the new buffer with the default attributes, + // but after the resize, we'll want to make sure that the new buffer's + // current attributes (the ones used for printing new text) match the + // old buffer's. + auto newTextBuffer = std::make_unique(bufferSize, + TextAttribute{}, + 0, + _mainBuffer->IsActiveBuffer(), + _mainBuffer->GetRenderer()); + + // Build a PositionInformation to track the position of both the top of + // the mutable viewport and the top of the visible viewport in the new + // buffer. + // * the new value of mutableViewportTop will be used to figure out + // where we should place the mutable viewport in the new buffer. This + // requires a bit of trickiness to remain consistent with conpty's + // buffer (as seen below). + // * the new value of visibleViewportTop will be used to calculate the + // new scrollOffset in the new buffer, so that the visible lines on + // the screen remain roughly the same. + TextBuffer::PositionInformation positionInfo{ + .mutableViewportTop = _mutableViewport.Top(), + .visibleViewportTop = _VisibleStartIndex(), + }; - // First allocate a new text buffer to take the place of the current one. - std::unique_ptr newTextBuffer; - try - { - // GH#3848 - Stash away the current attributes the old text buffer is - // using. We'll initialize the new buffer with the default attributes, - // but after the resize, we'll want to make sure that the new buffer's - // current attributes (the ones used for printing new text) match the - // old buffer's. - const auto& oldBufferAttributes = _mainBuffer->GetCurrentAttributes(); - newTextBuffer = std::make_unique(bufferSize, - TextAttribute{}, - 0, // temporarily set size to 0 so it won't render. - _mainBuffer->IsActiveBuffer(), - _mainBuffer->GetRenderer()); - - // start defer drawing on the new buffer - newTextBuffer->GetCursor().StartDeferDrawing(); - - // Build a PositionInformation to track the position of both the top of - // the mutable viewport and the top of the visible viewport in the new - // buffer. - // * the new value of mutableViewportTop will be used to figure out - // where we should place the mutable viewport in the new buffer. This - // requires a bit of trickiness to remain consistent with conpty's - // buffer (as seen below). - // * the new value of visibleViewportTop will be used to calculate the - // new scrollOffset in the new buffer, so that the visible lines on - // the screen remain roughly the same. - TextBuffer::PositionInformation oldRows{ 0 }; - oldRows.mutableViewportTop = oldViewportTop; - oldRows.visibleViewportTop = newVisibleTop; - - const std::optional oldViewStart{ oldViewportTop }; - RETURN_IF_FAILED(TextBuffer::Reflow(*_mainBuffer.get(), - *newTextBuffer.get(), - _mutableViewport, - { oldRows })); - - newViewportTop = oldRows.mutableViewportTop; - newVisibleTop = oldRows.visibleViewportTop; - - // Restore the active text attributes - newTextBuffer->SetCurrentAttributes(oldBufferAttributes); - } - CATCH_RETURN(); + TextBuffer::Reflow(*_mainBuffer.get(), *newTextBuffer.get(), &_mutableViewport, &positionInfo); + + // Restore the active text attributes + newTextBuffer->SetCurrentAttributes(_mainBuffer->GetCurrentAttributes()); // Conpty resizes a little oddly - if the height decreased, and there were // blank lines at the bottom, those lines will get trimmed. If there's not @@ -369,7 +332,7 @@ std::wstring_view Terminal::GetWorkingDirectory() noexcept const auto maxRow = std::max(newLastChar.y, newCursorPos.y); const auto proposedTopFromLastLine = maxRow - viewportSize.height + 1; - const auto proposedTopFromScrollback = newViewportTop; + const auto proposedTopFromScrollback = positionInfo.mutableViewportTop; auto proposedTop = std::max(proposedTopFromLastLine, proposedTopFromScrollback); @@ -392,17 +355,13 @@ std::wstring_view Terminal::GetWorkingDirectory() noexcept const auto proposedViewFromTop = Viewport::FromDimensions({ 0, proposedTopFromScrollback }, viewportSize); if (maxRow < proposedViewFromTop.BottomInclusive()) { - if (dx < 0 && proposedTop > 0) + if (viewportSize.width < oldDimensions.width && proposedTop > 0) { - try + const auto& row = newTextBuffer->GetRowByOffset(proposedTop - 1); + if (row.WasWrapForced()) { - const auto& row = newTextBuffer->GetRowByOffset(::base::ClampSub(proposedTop, 1)); - if (row.WasWrapForced()) - { - proposedTop--; - } + proposedTop--; } - CATCH_LOG(); } } } @@ -435,7 +394,7 @@ std::wstring_view Terminal::GetWorkingDirectory() noexcept // GH#3494: Maintain scrollbar position during resize // Make sure that we don't scroll past the mutableViewport at the bottom of the buffer - newVisibleTop = std::min(newVisibleTop, _mutableViewport.Top()); + auto newVisibleTop = std::min(positionInfo.visibleViewportTop, _mutableViewport.Top()); // Make sure we don't scroll past the top of the scrollback newVisibleTop = std::max(newVisibleTop, 0); @@ -443,16 +402,11 @@ std::wstring_view Terminal::GetWorkingDirectory() noexcept // before, and shouldn't be now either. _scrollOffset = originalOffsetWasZero ? 0 : static_cast(::base::ClampSub(_mutableViewport.Top(), newVisibleTop)); - // GH#5029 - make sure to InvalidateAll here, so that we'll paint the entire visible viewport. - try - { - _activeBuffer().TriggerRedrawAll(); - _NotifyScrollEvent(); - } - CATCH_LOG(); - + _mainBuffer->TriggerRedrawAll(); + _NotifyScrollEvent(); return S_OK; } +CATCH_RETURN() void Terminal::Write(std::wstring_view stringView) { @@ -1073,14 +1027,12 @@ const TerminalInput& Terminal::_getTerminalInput() const noexcept // _VisibleStartIndex is the first visible line of the buffer int Terminal::_VisibleStartIndex() const noexcept { - return _inAltBuffer() ? ViewStartIndex() : - std::max(0, ViewStartIndex() - _scrollOffset); + return _inAltBuffer() ? 0 : std::max(0, _mutableViewport.Top() - _scrollOffset); } int Terminal::_VisibleEndIndex() const noexcept { - return _inAltBuffer() ? ViewEndIndex() : - std::max(0, ViewEndIndex() - _scrollOffset); + return _inAltBuffer() ? _altBufferSize.height - 1 : std::max(0, _mutableViewport.BottomInclusive() - _scrollOffset); } Viewport Terminal::_GetVisibleViewport() const noexcept diff --git a/src/cascadia/TerminalCore/TerminalApi.cpp b/src/cascadia/TerminalCore/TerminalApi.cpp index 0f4b1407031..efdb9f8ee77 100644 --- a/src/cascadia/TerminalCore/TerminalApi.cpp +++ b/src/cascadia/TerminalCore/TerminalApi.cpp @@ -248,33 +248,19 @@ void Terminal::UseAlternateScreenBuffer(const TextAttribute& attrs) } void Terminal::UseMainScreenBuffer() { - // Short-circuit: do nothing. - if (!_inAltBuffer()) + // To make UserResize() work as if we're back in the main buffer, we first need to unset + // _altBuffer, which is used throughout this class as an indicator via _inAltBuffer(). + // + // We delay destroying the alt buffer instance to get a valid altBuffer->GetCursor() reference below. + const auto altBuffer = std::exchange(_altBuffer, nullptr); + if (!altBuffer) { return; } ClearSelection(); - // Copy our cursor state back to the main buffer's cursor - { - // Update the alt buffer's cursor style, visibility, and position to match our own. - const auto& myCursor = _altBuffer->GetCursor(); - auto& tgtCursor = _mainBuffer->GetCursor(); - tgtCursor.SetStyle(myCursor.GetSize(), myCursor.GetType()); - tgtCursor.SetIsVisible(myCursor.IsVisible()); - tgtCursor.SetBlinkingAllowed(myCursor.IsBlinkingAllowed()); - - // The new position should match the viewport-relative position of the main buffer. - // This is the equal and opposite effect of what we did in UseAlternateScreenBuffer - auto tgtCursorPos = myCursor.GetPosition(); - tgtCursorPos.y += _mutableViewport.Top(); - tgtCursor.SetPosition(tgtCursorPos); - } - _mainBuffer->SetAsActiveBuffer(true); - // destroy the alt buffer - _altBuffer = nullptr; if (_deferredResize.has_value()) { @@ -282,6 +268,24 @@ void Terminal::UseMainScreenBuffer() _deferredResize = std::nullopt; } + // After exiting the alt buffer, the main buffer should adopt the current cursor position and style. + // This is the equal and opposite effect of what we did in UseAlternateScreenBuffer and matches xterm. + // + // We have to do this after the call to UserResize() to ensure that the TextBuffer sizes match up. + // Otherwise the cursor position may be temporarily out of bounds and some code may choke on that. + { + const auto& altCursor = altBuffer->GetCursor(); + auto& mainCursor = _mainBuffer->GetCursor(); + + mainCursor.SetStyle(altCursor.GetSize(), altCursor.GetType()); + mainCursor.SetIsVisible(altCursor.IsVisible()); + mainCursor.SetBlinkingAllowed(altCursor.IsBlinkingAllowed()); + + auto tgtCursorPos = altCursor.GetPosition(); + tgtCursorPos.y += _mutableViewport.Top(); + mainCursor.SetPosition(tgtCursorPos); + } + // update all the hyperlinks on the screen _updateUrlDetection(); @@ -293,11 +297,7 @@ void Terminal::UseMainScreenBuffer() _NotifyScrollEvent(); // redraw the screen - try - { - _activeBuffer().TriggerRedrawAll(); - } - CATCH_LOG(); + _activeBuffer().TriggerRedrawAll(); } // NOTE: This is the version of AddMark that comes from VT diff --git a/src/host/VtIo.cpp b/src/host/VtIo.cpp index cb31fa407e8..dd8b635efd9 100644 --- a/src/host/VtIo.cpp +++ b/src/host/VtIo.cpp @@ -462,40 +462,6 @@ void VtIo::SendCloseEvent() } } -// Method Description: -// - Tell the vt renderer to begin a resize operation. During a resize -// operation, the vt renderer should _not_ request to be repainted during a -// text buffer circling event. Any callers of this method should make sure to -// call EndResize to make sure the renderer returns to normal behavior. -// See GH#1795 for context on this method. -// Arguments: -// - -// Return Value: -// - -void VtIo::BeginResize() -{ - if (_pVtRenderEngine) - { - _pVtRenderEngine->BeginResizeRequest(); - } -} - -// Method Description: -// - Tell the vt renderer to end a resize operation. -// See BeginResize for more details. -// See GH#1795 for context on this method. -// Arguments: -// - -// Return Value: -// - -void VtIo::EndResize() -{ - if (_pVtRenderEngine) - { - _pVtRenderEngine->EndResizeRequest(); - } -} - // The name of this method is an analogy to TCP_CORK. It instructs // the VT renderer to stop flushing its buffer to the output pipe. // Don't forget to uncork it! diff --git a/src/host/VtIo.hpp b/src/host/VtIo.hpp index 3e23350d153..2ecc0d51754 100644 --- a/src/host/VtIo.hpp +++ b/src/host/VtIo.hpp @@ -40,9 +40,6 @@ namespace Microsoft::Console::VirtualTerminal void CloseInput(); void CloseOutput(); - void BeginResize(); - void EndResize(); - void CorkRenderer(bool corked) const noexcept; #ifdef UNIT_TESTING diff --git a/src/host/screenInfo.cpp b/src/host/screenInfo.cpp index 58ac0812d52..d02bbaf3d67 100644 --- a/src/host/screenInfo.cpp +++ b/src/host/screenInfo.cpp @@ -1387,6 +1387,7 @@ bool SCREEN_INFORMATION::IsMaximizedY() const // Return Value: // - Success if successful. Invalid parameter if screen buffer size is unexpected. No memory if allocation failed. [[nodiscard]] NTSTATUS SCREEN_INFORMATION::ResizeWithReflow(const til::size coordNewScreenSize) +try { if ((USHORT)coordNewScreenSize.width >= SHORT_MAX || (USHORT)coordNewScreenSize.height >= SHORT_MAX) { @@ -1394,26 +1395,14 @@ bool SCREEN_INFORMATION::IsMaximizedY() const return STATUS_INVALID_PARAMETER; } - // First allocate a new text buffer to take the place of the current one. - std::unique_ptr newTextBuffer; - - // GH#3848 - Stash away the current attributes the old text buffer is using. - // We'll initialize the new buffer with the default attributes, but after - // the resize, we'll want to make sure that the new buffer's current + // GH#3848 - We'll initialize the new buffer with the default attributes, + // but after the resize, we'll want to make sure that the new buffer's current // attributes (the ones used for printing new text) match the old buffer's. - const auto oldPrimaryAttributes = _textBuffer->GetCurrentAttributes(); - try - { - newTextBuffer = std::make_unique(coordNewScreenSize, - TextAttribute{}, - 0, // temporarily set size to 0 so it won't render. - _textBuffer->IsActiveBuffer(), - _textBuffer->GetRenderer()); - } - catch (...) - { - return NTSTATUS_FROM_HRESULT(wil::ResultFromCaughtException()); - } + auto newTextBuffer = std::make_unique(coordNewScreenSize, + TextAttribute{}, + 0, // temporarily set size to 0 so it won't render. + _textBuffer->IsActiveBuffer(), + _textBuffer->GetRenderer()); // Save cursor's relative height versus the viewport const auto sCursorHeightInViewportBefore = _textBuffer->GetCursor().GetPosition().y - _viewport.Top(); @@ -1426,38 +1415,35 @@ bool SCREEN_INFORMATION::IsMaximizedY() const // we're capturing _textBuffer by reference here because when we exit, we want to EndDefer on the current active buffer. auto endDefer = wil::scope_exit([&]() noexcept { _textBuffer->GetCursor().EndDeferDrawing(); }); - auto hr = TextBuffer::Reflow(*_textBuffer.get(), *newTextBuffer.get(), std::nullopt, std::nullopt); + TextBuffer::Reflow(*_textBuffer.get(), *newTextBuffer.get()); - if (SUCCEEDED(hr)) - { - // Since the reflow doesn't preserve the virtual bottom, we try and - // estimate where it ought to be by making it the same distance from - // the cursor row as it was before the resize. However, we also need - // to make sure it is far enough down to include the last non-space - // row, and it shouldn't be less than the height of the viewport, - // otherwise the top of the virtual viewport would end up negative. - const auto cursorRow = newTextBuffer->GetCursor().GetPosition().y; - const auto lastNonSpaceRow = newTextBuffer->GetLastNonSpaceCharacter().y; - const auto estimatedBottom = cursorRow + cursorDistanceFromBottom; - const auto viewportBottom = _viewport.Height() - 1; - _virtualBottom = std::max({ lastNonSpaceRow, estimatedBottom, viewportBottom }); + // Since the reflow doesn't preserve the virtual bottom, we try and + // estimate where it ought to be by making it the same distance from + // the cursor row as it was before the resize. However, we also need + // to make sure it is far enough down to include the last non-space + // row, and it shouldn't be less than the height of the viewport, + // otherwise the top of the virtual viewport would end up negative. + const auto cursorRow = newTextBuffer->GetCursor().GetPosition().y; + const auto lastNonSpaceRow = newTextBuffer->GetLastNonSpaceCharacter().y; + const auto estimatedBottom = cursorRow + cursorDistanceFromBottom; + const auto viewportBottom = _viewport.Height() - 1; + _virtualBottom = std::max({ lastNonSpaceRow, estimatedBottom, viewportBottom }); - // We can't let it extend past the bottom of the buffer either. - _virtualBottom = std::min(_virtualBottom, newTextBuffer->GetSize().BottomInclusive()); + // We can't let it extend past the bottom of the buffer either. + _virtualBottom = std::min(_virtualBottom, newTextBuffer->GetSize().BottomInclusive()); - // Adjust the viewport so the cursor doesn't wildly fly off up or down. - const auto sCursorHeightInViewportAfter = cursorRow - _viewport.Top(); - til::point coordCursorHeightDiff; - coordCursorHeightDiff.y = sCursorHeightInViewportAfter - sCursorHeightInViewportBefore; - LOG_IF_FAILED(SetViewportOrigin(false, coordCursorHeightDiff, false)); + // Adjust the viewport so the cursor doesn't wildly fly off up or down. + const auto sCursorHeightInViewportAfter = cursorRow - _viewport.Top(); + til::point coordCursorHeightDiff; + coordCursorHeightDiff.y = sCursorHeightInViewportAfter - sCursorHeightInViewportBefore; + LOG_IF_FAILED(SetViewportOrigin(false, coordCursorHeightDiff, false)); - newTextBuffer->SetCurrentAttributes(oldPrimaryAttributes); + newTextBuffer->SetCurrentAttributes(_textBuffer->GetCurrentAttributes()); - _textBuffer.swap(newTextBuffer); - } - - return NTSTATUS_FROM_HRESULT(hr); + _textBuffer = std::move(newTextBuffer); + return STATUS_SUCCESS; } +NT_CATCH_RETURN() // // Routine Description: @@ -1467,11 +1453,14 @@ bool SCREEN_INFORMATION::IsMaximizedY() const // Return Value: // - Success if successful. Invalid parameter if screen buffer size is unexpected. No memory if allocation failed. [[nodiscard]] NTSTATUS SCREEN_INFORMATION::ResizeTraditional(const til::size coordNewScreenSize) +try { _textBuffer->GetCursor().StartDeferDrawing(); auto endDefer = wil::scope_exit([&]() noexcept { _textBuffer->GetCursor().EndDeferDrawing(); }); - return NTSTATUS_FROM_HRESULT(_textBuffer->ResizeTraditional(coordNewScreenSize)); + _textBuffer->ResizeTraditional(coordNewScreenSize); + return STATUS_SUCCESS; } +NT_CATCH_RETURN() // // Routine Description: @@ -1493,19 +1482,6 @@ bool SCREEN_INFORMATION::IsMaximizedY() const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); auto status = STATUS_SUCCESS; - // If we're in conpty mode, suppress any immediate painting we might do - // during the resize. - if (gci.IsInVtIoMode()) - { - gci.GetVtIo()->BeginResize(); - } - auto endResize = wil::scope_exit([&] { - if (gci.IsInVtIoMode()) - { - gci.GetVtIo()->EndResize(); - } - }); - // cancel any active selection before resizing or it will not necessarily line up with the new buffer positions Selection::Instance().ClearSelection(); diff --git a/src/host/ut_host/ApiRoutinesTests.cpp b/src/host/ut_host/ApiRoutinesTests.cpp index 90d3fd1044b..611eb55f14c 100644 --- a/src/host/ut_host/ApiRoutinesTests.cpp +++ b/src/host/ut_host/ApiRoutinesTests.cpp @@ -607,7 +607,7 @@ class ApiRoutinesTests auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); auto& si = gci.GetActiveOutputBuffer(); - VERIFY_SUCCEEDED(si.GetTextBuffer().ResizeTraditional({ 5, 5 }), L"Make the buffer small so this doesn't take forever."); + si.GetTextBuffer().ResizeTraditional({ 5, 5 }); // Tests are run both with and without the DECSTBM margins set. This should not alter // the results, since ScrollConsoleScreenBuffer should not be affected by VT margins. diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index 334d770d0b8..b7ea13e6dc6 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -2307,7 +2307,7 @@ void ScreenBufferTests::GetWordBoundary() // Make the buffer as big as our test text. const til::size newBufferSize = { gsl::narrow(length), 10 }; - VERIFY_SUCCEEDED(si.GetTextBuffer().ResizeTraditional(newBufferSize)); + si.GetTextBuffer().ResizeTraditional(newBufferSize); const OutputCellIterator it(text, si.GetAttributes()); si.Write(it, { 0, 0 }); @@ -2383,7 +2383,7 @@ void ScreenBufferTests::GetWordBoundaryTrimZeros(const bool on) // Make the buffer as big as our test text. const til::size newBufferSize = { gsl::narrow(length), 10 }; - VERIFY_SUCCEEDED(si.GetTextBuffer().ResizeTraditional(newBufferSize)); + si.GetTextBuffer().ResizeTraditional(newBufferSize); const OutputCellIterator it(text, si.GetAttributes()); si.Write(it, { 0, 0 }); diff --git a/src/host/ut_host/TextBufferTests.cpp b/src/host/ut_host/TextBufferTests.cpp index d52dfc5ae69..d1c7395635f 100644 --- a/src/host/ut_host/TextBufferTests.cpp +++ b/src/host/ut_host/TextBufferTests.cpp @@ -1755,7 +1755,7 @@ void TextBufferTests::ResizeTraditional() auto expectedSpace = UNICODE_SPACE; std::wstring_view expectedSpaceView(&expectedSpace, 1); - VERIFY_SUCCEEDED(buffer.ResizeTraditional(newSize)); + buffer.ResizeTraditional(newSize); Log::Comment(L"Verify every cell in the X dimension is still the same as when filled and the new Y row is just empty default cells."); { @@ -1821,7 +1821,7 @@ void TextBufferTests::ResizeTraditionalRotationPreservesHighUnicode() _buffer->_SetFirstRowIndex(pos.y); // Perform resize to rotate the rows around - VERIFY_NT_SUCCESS(_buffer->ResizeTraditional(bufferSize)); + _buffer->ResizeTraditional(bufferSize); // Retrieve the text at the old and new positions. const auto shouldBeEmptyText = *_buffer->GetTextDataAt(pos); @@ -1893,7 +1893,7 @@ void TextBufferTests::ResizeTraditionalHighUnicodeRowRemoval() // Perform resize to trim off the row of the buffer that included the emoji til::size trimmedBufferSize{ bufferSize.width, bufferSize.height - 1 }; - VERIFY_NT_SUCCESS(_buffer->ResizeTraditional(trimmedBufferSize)); + _buffer->ResizeTraditional(trimmedBufferSize); } // This tests that columns removed from the buffer while resizing traditionally will also drop the high unicode @@ -1923,7 +1923,7 @@ void TextBufferTests::ResizeTraditionalHighUnicodeColumnRemoval() // Perform resize to trim off the column of the buffer that included the emoji til::size trimmedBufferSize{ bufferSize.width - 1, bufferSize.height }; - VERIFY_NT_SUCCESS(_buffer->ResizeTraditional(trimmedBufferSize)); + _buffer->ResizeTraditional(trimmedBufferSize); } void TextBufferTests::TestBurrito() diff --git a/src/inc/til/rle.h b/src/inc/til/rle.h index d94c3a744e6..34b6c929a0b 100644 --- a/src/inc/til/rle.h +++ b/src/inc/til/rle.h @@ -426,7 +426,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" // Returns the range [start_index, end_index) as a new vector. // It works just like std::string::substr(), but with absolute indices. - [[nodiscard]] basic_rle slice(size_type start_index, size_type end_index) const noexcept + [[nodiscard]] basic_rle slice(size_type start_index, size_type end_index) const { if (end_index > _total_length) { @@ -446,14 +446,14 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" // --> It's safe to subtract 1 from end_index rle_scanner scanner(_runs.begin(), _runs.end()); - auto [begin_run, start_run_pos] = scanner.scan(start_index); - auto [end_run, end_run_pos] = scanner.scan(end_index - 1); + const auto [begin_run, start_run_pos] = scanner.scan(start_index); + const auto [end_run, end_run_pos] = scanner.scan(end_index - 1); container slice{ begin_run, end_run + 1 }; slice.back().length = end_run_pos + 1; slice.front().length -= start_run_pos; - return { std::move(slice), static_cast(end_index - start_index) }; + return { std::move(slice), gsl::narrow_cast(end_index - start_index) }; } // Replace the range [start_index, end_index) with the given value. @@ -463,7 +463,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" { _check_indices(start_index, end_index); - const rle_type replacement{ value, static_cast(end_index - start_index) }; + const rle_type replacement{ value, gsl::narrow_cast(end_index - start_index) }; _replace_unchecked(start_index, end_index, { &replacement, 1 }); } @@ -651,7 +651,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" size_type total = 0; }; - basic_rle(container&& runs, size_type size) : + basic_rle(container&& runs, size_type size) noexcept : _runs(std::forward(runs)), _total_length(size) { diff --git a/src/renderer/vt/invalidate.cpp b/src/renderer/vt/invalidate.cpp index 8969b04a39d..7ef59bc03b0 100644 --- a/src/renderer/vt/invalidate.cpp +++ b/src/renderer/vt/invalidate.cpp @@ -106,19 +106,11 @@ CATCH_RETURN(); // - S_OK [[nodiscard]] HRESULT VtEngine::InvalidateFlush(_In_ const bool circled, _Out_ bool* const pForcePaint) noexcept { - // If we're in the middle of a resize request, don't try to immediately start a frame. - if (_inResizeRequest) - { - *pForcePaint = false; - } - else - { - *pForcePaint = true; + *pForcePaint = true; - // Keep track of the fact that we circled, we'll need to do some work on - // end paint to specifically handle this. - _circled = circled; - } + // Keep track of the fact that we circled, we'll need to do some work on + // end paint to specifically handle this. + _circled = circled; _trace.TraceTriggerCircling(*pForcePaint); return S_OK; diff --git a/src/renderer/vt/state.cpp b/src/renderer/vt/state.cpp index b360b7b93a6..8408ee3aeb6 100644 --- a/src/renderer/vt/state.cpp +++ b/src/renderer/vt/state.cpp @@ -49,7 +49,6 @@ VtEngine::VtEngine(_In_ wil::unique_hfile pipe, _terminalOwner{ nullptr }, _newBottomLine{ false }, _deferredCursorPos{ INVALID_COORDS }, - _inResizeRequest{ false }, _trace{}, _bufferLine{}, _buffer{}, @@ -459,34 +458,6 @@ HRESULT VtEngine::RequestCursor() noexcept return S_OK; } -// Method Description: -// - Tell the vt renderer to begin a resize operation. During a resize -// operation, the vt renderer should _not_ request to be repainted during a -// text buffer circling event. Any callers of this method should make sure to -// call EndResize to make sure the renderer returns to normal behavior. -// See GH#1795 for context on this method. -// Arguments: -// - -// Return Value: -// - -void VtEngine::BeginResizeRequest() -{ - _inResizeRequest = true; -} - -// Method Description: -// - Tell the vt renderer to end a resize operation. -// See BeginResize for more details. -// See GH#1795 for context on this method. -// Arguments: -// - -// Return Value: -// - -void VtEngine::EndResizeRequest() -{ - _inResizeRequest = false; -} - // Method Description: // - Configure the renderer for the resize quirk. This changes the behavior of // conpty to _not_ InvalidateAll the entire viewport on a resize operation. diff --git a/src/renderer/vt/vtrenderer.hpp b/src/renderer/vt/vtrenderer.hpp index 208918407da..15de6e0e760 100644 --- a/src/renderer/vt/vtrenderer.hpp +++ b/src/renderer/vt/vtrenderer.hpp @@ -132,7 +132,6 @@ namespace Microsoft::Console::Render Microsoft::Console::VirtualTerminal::VtIo* _terminalOwner; Microsoft::Console::VirtualTerminal::RenderTracing _trace; - bool _inResizeRequest{ false }; std::optional _wrappedRow{ std::nullopt }; diff --git a/src/terminal/adapter/ut_adapter/adapterTest.cpp b/src/terminal/adapter/ut_adapter/adapterTest.cpp index 3c2e1ba4d76..c3192462052 100644 --- a/src/terminal/adapter/ut_adapter/adapterTest.cpp +++ b/src/terminal/adapter/ut_adapter/adapterTest.cpp @@ -2106,26 +2106,26 @@ class AdapterTest auto& stateMachine = *_testGetSet->_stateMachine; Log::Comment(L"Default tabs stops in 80-column mode"); - VERIFY_SUCCEEDED(textBuffer.ResizeTraditional({ 80, 600 })); + textBuffer.ResizeTraditional({ 80, 600 }); _pDispatch->RequestPresentationStateReport(DispatchTypes::PresentationReportFormat::TabulationStopReport); _testGetSet->ValidateInputEvent(L"\033P2$u9/17/25/33/41/49/57/65/73\033\\"); Log::Comment(L"Default tabs stops in 132-column mode"); - VERIFY_SUCCEEDED(textBuffer.ResizeTraditional({ 132, 600 })); + textBuffer.ResizeTraditional({ 132, 600 }); _pDispatch->RequestPresentationStateReport(DispatchTypes::PresentationReportFormat::TabulationStopReport); _testGetSet->ValidateInputEvent(L"\033P2$u9/17/25/33/41/49/57/65/73/81/89/97/105/113/121/129\033\\"); Log::Comment(L"Custom tab stops in 80 columns"); - VERIFY_SUCCEEDED(textBuffer.ResizeTraditional({ 80, 600 })); + textBuffer.ResizeTraditional({ 80, 600 }); _testGetSet->_stateMachine->ProcessString(L"\033P2$t30/60/120/240\033\\"); _pDispatch->RequestPresentationStateReport(DispatchTypes::PresentationReportFormat::TabulationStopReport); _testGetSet->ValidateInputEvent(L"\033P2$u30/60\033\\"); Log::Comment(L"After expanding width to 132 columns"); - VERIFY_SUCCEEDED(textBuffer.ResizeTraditional({ 132, 600 })); + textBuffer.ResizeTraditional({ 132, 600 }); _pDispatch->RequestPresentationStateReport(DispatchTypes::PresentationReportFormat::TabulationStopReport); _testGetSet->ValidateInputEvent(L"\033P2$u30/60/120\033\\"); - VERIFY_SUCCEEDED(textBuffer.ResizeTraditional({ 80, 600 })); + textBuffer.ResizeTraditional({ 80, 600 }); Log::Comment(L"Out of order tab stops"); stateMachine.ProcessString(L"\033P2$t44/22/66\033\\"); diff --git a/src/types/UiaTextRangeBase.cpp b/src/types/UiaTextRangeBase.cpp index 1f56c04b535..af5ebb431ea 100644 --- a/src/types/UiaTextRangeBase.cpp +++ b/src/types/UiaTextRangeBase.cpp @@ -1335,7 +1335,7 @@ til::point UiaTextRangeBase::_getDocumentEnd() const { const auto optimizedBufferSize{ _getOptimizedBufferSize() }; const auto& buffer{ _pData->GetTextBuffer() }; - const auto lastCharPos{ buffer.GetLastNonSpaceCharacter(optimizedBufferSize) }; + const auto lastCharPos{ buffer.GetLastNonSpaceCharacter(&optimizedBufferSize) }; const auto cursorPos{ buffer.GetCursor().GetPosition() }; return { optimizedBufferSize.Left(), std::max(lastCharPos.y, cursorPos.y) + 1 }; }