Repository: microsoft/terminal (language: cpp)

Issue:
Another case of the virtual bottom being miscalculated
### Windows Terminal version

Commit ec726e7ba70b760f98eb1e1ea1d71124536783c0

### Windows build number

10.0.19041.1415

### Other Software

[vttest](https://www.invisible-island.net/vttest/)

### Steps to reproduce

1. Open a bash shell in conhost.
2. Immediately run vttest.
3. Select test 4 (Test of double-sized characters).
4. Press enter twice until it gets to the 132-column test.


### Expected Behavior

It should display a 132-column version of the test output with examples of double width and double height content.

### Actual Behavior

The page appears mostly blank, but if you scroll back in the buffer, you can see the test content has rendered on the page above (partially overwriting the previous test). This doesn't seem to happen (at least not consistently) if you've already scrolled down in the buffer when you start the test.

I believe this is another victim of PR #12972. And it looks like the source of the problem is again in the `ResizeWithReflow` code, which picks a position for the virtual bottom that isn't far enough down. I think this is because the majority of the page is blank at the time of the resize.


Discussion / hints:
I should add that I have an idea for fixing this: Essentially what we'd need to do is save the distance between the cursor row and the virtual bottom *prior* to resizing, and then add that distance to the new cursor row *after* resizing, and that should give us a good estimate of where the new virtual bottom ought to be. We'd still probably also need to take into account the other boundaries that we're currently checking, like the last non-space row, and the viewport height.

If that works, I'll try and put together a PR with the fix on Thursday.
(FWIW: I don't necessarily think you do, but _if you do_ feel bad about introducing more virtual bottom bugs . . . just think about all the ones we already had that you helped to enumerate _and fix(!)_. In true Windows Console fashion, and in the fashion of every other project everywhere, you can't make an omelet without breaking a few ~lions~ eggs. I'm eternally grateful for you breaking those eggs.)

Fix patch:
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<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();
diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp
index 2bb59a62c77..6b6dd4ccd2c 100644
--- a/src/host/ut_host/ScreenBufferTests.cpp
+++ b/src/host/ut_host/ScreenBufferTests.cpp
@@ -6291,6 +6291,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()


Test patch: