Skip to content

Commit

Permalink
Restore virtual viewport position after resize with reflow (#13087)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
j4james authored May 12, 2022
1 parent 1d3e156 commit c2f8308
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 5 deletions.
18 changes: 13 additions & 5 deletions src/host/screenInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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<short>(cursorRow + cursorDistanceFromBottom);
const auto viewportBottom = gsl::narrow_cast<short>(_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();
Expand Down
12 changes: 12 additions & 0 deletions src/host/ut_host/ScreenBufferTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit c2f8308

Please sign in to comment.