Repository: microsoft/terminal (language: cpp)

Issue:
Resizing while alt buffer is active, then disabling it, blows away renderer
1. WSL, in a small window
2. `vim -u NONE`
3. Maximize the window
4. <kbd>Esc</kbd> `:q` <kbd>Enter</kbd>
5. Observe the window get "stuck" on vim

It looks like we're failing to render a dirty region that is outside of the new, smaller main buffer.

It appears as though Terminal is not resizing the main buffer, as it believes it is already the correct size.

That check is happening here:

https://github.com/microsoft/terminal/blob/9edf55de7564912cb8e19ef3a3f88b689be08af7/src/cascadia/TerminalCore/Terminal.cpp#L252-L256

At the time of failure, `_GetMutableViewport` returns `_mutableViewport`, since we are no longer in the alt buffer.

Setting a write breakpoint on `_mutableViewport` to determine who is setting it, it's coming from `EraseAll` -> `SetViewportPosition`:

https://github.com/microsoft/terminal/blob/9edf55de7564912cb8e19ef3a3f88b689be08af7/src/cascadia/TerminalCore/TerminalApi.cpp#L42-L47

Potentially regressed in #13024 (/cc @j4james)


Discussion / hints:
Yeah, it's definitely regressed in #13024. I remember thinking at the time that the assignment to `_mutableViewport` looked suspicious, but I got that from the original "EraseAll" implementation here:

https://github.com/microsoft/terminal/blob/6b936d9a74a5d677387f3edc9a68e6b33d566606/src/cascadia/TerminalCore/TerminalApi.cpp#L301-L303

So I'm not sure why that worked before. There must be something else different between the conhost and terminal architectures. I'll do some digging.
OK, I see now why it worked before. The terminal "EraseAll" implementation had an early exit check for `_inAltBuffer()` which meant the viewport wouldn't be set at all. But I think that's one of those things that only worked in Terminal because conpty would redraw the whole screen afterwards anyway. It's not actually clearing the screen in that case, so we can't do that in the current implementation.
But now that I think about it we can just make the `Terminal::SetViewportPosition` method a no-op when in the alt buffer - that should fix it.
That makes sense to me. The viewport in the alt buffer is always the entire buffer, right?

Fix patch:
diff --git a/src/cascadia/TerminalCore/TerminalApi.cpp b/src/cascadia/TerminalCore/TerminalApi.cpp
index c8af8f07b2b..f888cfa67f9 100644
--- a/src/cascadia/TerminalCore/TerminalApi.cpp
+++ b/src/cascadia/TerminalCore/TerminalApi.cpp
@@ -41,9 +41,13 @@ til::rect Terminal::GetViewport() const
 
 void Terminal::SetViewportPosition(const til::point position)
 {
-    const auto dimensions = _GetMutableViewport().Dimensions();
-    _mutableViewport = Viewport::FromDimensions(position.to_win32_coord(), dimensions);
-    Terminal::_NotifyScrollEvent();
+    // The viewport is fixed at 0,0 for the alt buffer, so this is a no-op.
+    if (!_inAltBuffer())
+    {
+        const auto dimensions = _GetMutableViewport().Dimensions();
+        _mutableViewport = Viewport::FromDimensions(position.to_win32_coord(), dimensions);
+        Terminal::_NotifyScrollEvent();
+    }
 }
 
 void Terminal::SetTextAttributes(const TextAttribute& attrs)


Test patch: