Resizing while alt buffer is active, then disabling it, blows away renderer

[DHowett]

1. WSL, in a small window
2. vim -u NONE
3. Maximize the window
4. Esc :q Enter
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:

terminal/src/cascadia/TerminalCore/Terminal.cpp

Lines 252 to 256 in 9edf55d

	const auto oldDimensions = _GetMutableViewport().Dimensions();
	if (viewportSize == oldDimensions)
	{
	return S_FALSE;
	}

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:

terminal/src/cascadia/TerminalCore/TerminalApi.cpp

Lines 42 to 47 in 9edf55d

	void Terminal::SetViewportPosition(const til::point position)
	{
	const auto dimensions = _GetMutableViewport().Dimensions();
	_mutableViewport = Viewport::FromDimensions(position.to_win32_coord(), dimensions);
	Terminal::_NotifyScrollEvent();
	}

Potentially regressed in #13024 (/cc @j4james)

[j4james]

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:

terminal/src/cascadia/TerminalCore/TerminalApi.cpp

Lines 301 to 303 in 6b936d9

	// Move the viewport, adjust the scroll bar if needed, and restore the old cursor position
	_mutableViewport = Viewport::FromExclusive(newWin);
	Terminal::_NotifyScrollEvent();

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.

[j4james]

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.

[j4james]

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.

[DHowett]

That makes sense to me. The viewport in the alt buffer is always the entire buffer, right?

[j4james]

Yeah, if you look at the _GetMutableViewport method, it's just returning a Viewport object with the alt buffer dimensions, so it's always at 0,0.

terminal/src/cascadia/TerminalCore/Terminal.cpp

Lines 970 to 977 in f4e0d9f

	Viewport Terminal::_GetMutableViewport() const noexcept
	{
	// GH#3493: if we're in the alt buffer, then it's possible that the mutable
	// viewport's size hasn't been updated yet. In that case, use the
	// temporarily stashed _altBufferSize instead.
	return _inAltBuffer() ? Viewport::FromDimensions(_altBufferSize.to_win32_coord()) :
	_mutableViewport;
	}

[ghost]

🎉This issue was addressed in #13039, which has now been successfully released as Windows Terminal Preview v1.14.143.:tada:

Handy links:

• Release Notes
• Store Download