From c2f830843a478e77bb2703e9be2f86f9dacab4e3 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Thu, 12 May 2022 21:26:35 +0100 Subject: [PATCH] Restore virtual viewport position after resize with reflow (#13087) When the buffer is resized with a reflow, we were previously calculating the new virtual bottom based on the position of last non-space character. If the viewport was largely blank when resized, this could result in the new virtual bottom being higher than it should be. This PR attempts to address that problem by restoring the virtual bottom to a position that is the same distance from the cursor row as it was prior to the resize. This was a regression introduced in PR #12972. We still take the last non-space row into account when determining the virtual bottom, because if the content of the screen is forced to wrap, the virtual bottom will need to be lower (relative to the cursor) than it was before. We also need to check that we don't overflow the bottom of the buffer, which can occur when the viewport is at the bottom of the buffer, and the cursor position is pushed down as a result of content wrapping above it. I've manually confirmed that this fixes the problem reported in issue #13078, and I've also extended the existing `RefreshWithReflow` unit test to cover that particular scenario. Closes #13078 --- src/host/screenInfo.cpp | 18 +++++++++++++----- src/host/ut_host/ScreenBufferTests.cpp | 12 ++++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/host/screenInfo.cpp b/src/host/screenInfo.cpp index 05ddded0f12..a21086fb84e 100644 --- a/src/host/screenInfo.cpp +++ b/src/host/screenInfo.cpp @@ -1453,6 +1453,8 @@ bool SCREEN_INFORMATION::IsMaximizedY() const // Save cursor's relative height versus the viewport const auto sCursorHeightInViewportBefore = _textBuffer->GetCursor().GetPosition().Y - _viewport.Top(); + // Also save the distance to the virtual bottom so it can be restored after the resize + const auto cursorDistanceFromBottom = _virtualBottom - _textBuffer->GetCursor().GetPosition().Y; // skip any drawing updates that might occur until we swap _textBuffer with the new buffer or we exit early. newTextBuffer->GetCursor().StartDeferDrawing(); @@ -1464,14 +1466,20 @@ bool SCREEN_INFORMATION::IsMaximizedY() const if (SUCCEEDED(hr)) { - // Make sure the new virtual bottom is far enough down to include both - // the cursor row and the last non-space row. It also shouldn't be less - // than the height of the viewport, otherwise the top of the virtual - // viewport would end up negative. + // 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 = gsl::narrow_cast(cursorRow + cursorDistanceFromBottom); const auto viewportBottom = gsl::narrow_cast(_viewport.Height() - 1); - _virtualBottom = std::max({ cursorRow, lastNonSpaceRow, viewportBottom }); + _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()); // Adjust the viewport so the cursor doesn't wildly fly off up or down. const auto sCursorHeightInViewportAfter = cursorRow - _viewport.Top(); diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index 2e2d2bcb1c1..75bc3eb00c6 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -6329,6 +6329,18 @@ void ScreenBufferTests::UpdateVirtualBottomAfterResizeWithReflow() Log::Comment(L"Confirm that the virtual viewport includes the last non-space row"); const auto lastNonSpaceRow = si.GetTextBuffer().GetLastNonSpaceCharacter().Y; VERIFY_IS_GREATER_THAN_OR_EQUAL(si._virtualBottom, lastNonSpaceRow); + + Log::Comment(L"Clear the screen and note the cursor distance to the virtual bottom"); + stateMachine.ProcessString(L"\033[H\033[2J"); + const auto cursorDistanceFromBottom = si._virtualBottom - si.GetTextBuffer().GetCursor().GetPosition().Y; + VERIFY_ARE_EQUAL(si.GetViewport().Height() - 1, cursorDistanceFromBottom); + + Log::Comment(L"Stretch the viewport back to full width"); + bufferSize.X *= 2; + VERIFY_NT_SUCCESS(si.ResizeWithReflow(bufferSize)); + + Log::Comment(L"Confirm cursor distance to the virtual bottom is unchanged"); + VERIFY_ARE_EQUAL(cursorDistanceFromBottom, si._virtualBottom - si.GetTextBuffer().GetCursor().GetPosition().Y); } void ScreenBufferTests::DontShrinkVirtualBottomDuringResizeWithReflowAtTop()