Avoid window events during ASB

src/host/inputBuffer.cpp

@@ -759,6 +759,11 @@ bool InputBuffer::_CoalesceEvent(const INPUT_RECORD& inEvent) noexcept
             return true;
         }
     }
+    else if (lastEvent.EventType == WINDOW_BUFFER_SIZE_EVENT && inEvent.EventType == WINDOW_BUFFER_SIZE_EVENT)
+    {
+        lastEvent = inEvent;
+        return true;
+    }
     else if (lastEvent.EventType == KEY_EVENT && inEvent.EventType == KEY_EVENT)
     {
         const auto& inKey = inEvent.Event.KeyEvent;

src/host/output.cpp

@@ -261,6 +261,11 @@ void ScreenBufferSizeChange(const til::size coordNewSize)
 {
     const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
 
+    if (WI_IsFlagClear(gci.pInputBuffer->InputMode, ENABLE_WINDOW_INPUT))
+    {
+        return;
+    }
+
     try
     {
         gci.pInputBuffer->Write(SynthesizeWindowBufferSizeEvent(coordNewSize));

src/host/screenInfo.cpp

@@ -702,8 +702,7 @@
     else
     {
         // Otherwise, just store the new position and go on.
-        _viewport = Viewport::FromInclusive(NewWindow);
-        Tracing::s_TraceWindowViewport(_viewport);
+        _CommitViewport(Viewport::FromInclusive(NewWindow));
     }
 
     // Update our internal virtual bottom tracker if requested. This helps keep
@@ -1115,8 +1114,7 @@
         _virtualBottom = srNewViewport.bottom;
     }
 
-    _viewport = newViewport;
-    Tracing::s_TraceWindowViewport(_viewport);
+    _CommitViewport(newViewport);
 
     auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
     if (gci.HasPendingCookedRead())
@@ -1155,27 +1153,26 @@
     const auto fResizeFromTop = prcClientNew->top != prcClientOld->top &&
                                 prcClientNew->bottom == prcClientOld->bottom;
 
-    const auto oldViewport = Viewport(_viewport);
-
     _InternalSetViewportSize(pcoordSize, fResizeFromTop, fResizeFromLeft);
+}
 
-    // MSFT 13194969, related to 12092729.
-    // If we're in virtual terminal mode, and the viewport dimensions change,
-    //      send a WindowBufferSizeEvent. If the client wants VT mode, then they
-    //      probably want the viewport resizes, not just the screen buffer
-    //      resizes. This does change the behavior of the API for v2 callers,
-    //      but only callers who've requested VT mode. In 12092729, we enabled
-    //      sending notifications from window resizes in cases where the buffer
-    //      didn't resize, so this applies the same expansion to resizes using
-    //      the window, not the API.
-    if (IsInVirtualTerminalInputMode())
-    {
-        if ((_viewport.Width() != oldViewport.Width()) ||
-            (_viewport.Height() != oldViewport.Height()))
-        {
-            ScreenBufferSizeChange(GetBufferSize().Dimensions());
-        }
+void SCREEN_INFORMATION::_CommitViewport(const Viewport& viewport)
+{
+    // VT TUI applications typically use SIGWINCH on UNIX and Windows has no equivalent
+    // for that. So, we just raise WINDOW_BUFFER_SIZE_EVENT as the closest alternative.
+    // With ASB or in ConPTY, viewport will match buffer size, and in that case we rely
+    // on InputBuffer to deduplicate events for us.
+    //
+    // Technically, this is a hack, and it resulted in regressions. Example: GH#281.
+    // But this was changed so long ago, that it's difficult to improve now.
+    // A potential ideal solution would've been the introduction of a "VIEWPORT_EVENT".
+    if (IsActiveScreenBuffer() && IsInVirtualTerminalInputMode() && viewport.Dimensions() != _viewport.Dimensions())
+    {
+        ScreenBufferSizeChange(GetBufferSize().Dimensions());
     }
+
+    _viewport = viewport;
+    Tracing::s_TraceWindowViewport(_viewport);
 }
 
 // Routine Description:
@@ -1926,11 +1923,6 @@
 
         ::SetActiveScreenBuffer(*psiNewAltBuffer);
 
-        // Kind of a hack until we have proper signal channels: If the client app wants window size events, send one for
-        // the new alt buffer's size (this is so WSL can update the TTY size when the MainSB.viewportWidth <
-        // MainSB.bufferWidth (which can happen with wrap text disabled))
-        ScreenBufferSizeChange(psiNewAltBuffer->GetBufferSize().Dimensions());
-
         // Tell the VT MouseInput handler that we're in the Alt buffer now
         gci.GetActiveInputBuffer()->GetTerminalInput().UseAlternateScreenBuffer();
     }
@@ -1954,9 +1946,6 @@
         ::SetActiveScreenBuffer(*psiMain);
         psiMain->UpdateScrollBars(); // The alt had disabled scrollbars, re-enable them
 
-        // send a _coordScreenBufferSizeChangeEvent for the new Sb viewport
-        ScreenBufferSizeChange(psiMain->GetBufferSize().Dimensions());
-
         auto psiAlt = psiMain->_psiAlternateBuffer;
         psiMain->_psiAlternateBuffer = nullptr;
 
@@ -2151,14 +2140,13 @@
     const auto x = gsl::narrow_cast<SHORT>(std::clamp(viewportRect.left, 0, coordScreenBufferSize.width - cx));
     const auto y = gsl::narrow_cast<SHORT>(std::clamp(viewportRect.top, 0, coordScreenBufferSize.height - cy));
 
-    _viewport = Viewport::FromExclusive({ x, y, x + cx, y + cy });
+    _CommitViewport(Viewport::FromExclusive({ x, y, x + cx, y + cy }));
 
     if (updateBottom)
     {
         UpdateBottom();
     }
 
-    Tracing::s_TraceWindowViewport(_viewport);
 }
 
 // Routine Description:

src/host/screenInfo.hpp

@@ -193,6 +193,7 @@ class SCREEN_INFORMATION : public ConsoleObjectHeader, public Microsoft::Console
     // Rendering / Viewport
     void _CalculateViewportSize(const til::rect* clientArea, til::size* size);
     void _AdjustViewportSize(const til::rect* clientNew, const til::rect* clientOld, const til::size* size);
+    void _CommitViewport(const Microsoft::Console::Types::Viewport& viewport);
     void _InternalSetViewportSize(const til::size* size, bool resizeFromTop, bool resizeFromLeft);
 
     // Windowing

src/host/ut_host/InputBufferTests.cpp

@@ -136,6 +136,20 @@ class InputBufferTests
         VERIFY_ARE_EQUAL(inputBuffer.GetNumberOfReadyEvents(), 3u);
     }
 
+    TEST_METHOD(InputBufferCoalescesWindowSizeEvents)
+    {
+        InputBuffer inputBuffer;
+
+        inputBuffer.Write(SynthesizeWindowBufferSizeEvent({ 80, 25 }));
+        inputBuffer.Write(SynthesizeWindowBufferSizeEvent({ 100, 40 }));
+        inputBuffer.Write(MakeKeyEvent(true, 1, L'a', 0, L'a', 0));
+        inputBuffer.Write(SynthesizeWindowBufferSizeEvent({ 120, 50 }));
+
+        VERIFY_ARE_EQUAL(3u, inputBuffer.GetNumberOfReadyEvents());
+        VERIFY_ARE_EQUAL(til::size(100, 40), til::wrap_coord_size(inputBuffer._storage.front().Event.WindowBufferSizeEvent.dwSize));
+        VERIFY_ARE_EQUAL(til::size(120, 50), til::wrap_coord_size(inputBuffer._storage.back().Event.WindowBufferSizeEvent.dwSize));
+    }
+
     TEST_METHOD(InputBufferDoesNotCoalesceBulkMouseEvents)
     {
         Log::Comment(L"The input buffer should not coalesce mouse events if more than one event is sent at a time");

src/host/ut_host/ScreenBufferTests.cpp

@@ -241,6 +241,7 @@ class ScreenBufferTests
     TEST_METHOD(UpdateVirtualBottomWhenCursorMovesBelowIt);
     TEST_METHOD(UpdateVirtualBottomWithSetConsoleCursorPosition);
     TEST_METHOD(UpdateVirtualBottomAfterInternalSetViewportSize);
+    TEST_METHOD(ViewportDimensionChangesGenerateVtWindowEvents);
     TEST_METHOD(UpdateVirtualBottomAfterResizeWithReflow);
     TEST_METHOD(DontShrinkVirtualBottomDuringResizeWithReflowAtTop);
     TEST_METHOD(DontChangeVirtualBottomWithOffscreenLinefeed);
@@ -7356,6 +7357,27 @@ void ScreenBufferTests::UpdateVirtualBottomAfterInternalSetViewportSize()
     VERIFY_ARE_EQUAL(si._virtualBottom, si.GetViewport().BottomInclusive());
 }
 
+void ScreenBufferTests::ViewportDimensionChangesGenerateVtWindowEvents()
+{
+    auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
+    gci.LockConsole();
+    auto unlock = wil::scope_exit([&] { gci.UnlockConsole(); });
+
+    const auto restoreInputMode = wil::scope_exit([&, inputMode = gci.pInputBuffer.InputMode] {
+        gci.pInputBuffer.InputMode = inputMode;
+        gci.pInputBuffer.Flush();
+    });
+    gci.pInputBuffer.InputMode = ENABLE_WINDOW_INPUT | ENABLE_VIRTUAL_TERMINAL_INPUT;
+
+    auto& si = gci.GetActiveOutputBuffer();
+    const auto dim = si.GetViewport().Dimensions();
+    si.SetViewport(Viewport::FromDimensions({ 0, 1 }, dim), false);
+    VERIFY_ARE_EQUAL(0u, gci.pInputBuffer.GetNumberOfReadyEvents());
+
+    si.SetViewport(Viewport::FromDimensions({}, dim - til::size{ 1, 1 }), false);
+    VERIFY_ARE_EQUAL(1u, gci.pInputBuffer.GetNumberOfReadyEvents());
+}
+
 void ScreenBufferTests::UpdateVirtualBottomAfterResizeWithReflow()
 {
     auto& g = ServiceLocator::LocateGlobals();

src/interactivity/win32/window.cpp

@@ -415,7 +415,6 @@ void Window::ChangeViewport(const til::inclusive_rect& NewWindow)
 
         // The new window is OK. Store it in screeninfo and refresh screen.
         ScreenInfo.SetViewport(Viewport::FromInclusive(NewWindow), false);
-        Tracing::s_TraceWindowViewport(ScreenInfo.GetViewport());
 
         if (ServiceLocator::LocateGlobals().pRender != nullptr)
         {
@@ -428,7 +427,6 @@ void Window::ChangeViewport(const til::inclusive_rect& NewWindow)
     {
         // we're iconic
         ScreenInfo.SetViewport(Viewport::FromInclusive(NewWindow), false);
-        Tracing::s_TraceWindowViewport(ScreenInfo.GetViewport());
     }
 
     ScreenInfo.UpdateScrollBars();
@@ -635,28 +633,6 @@ void Window::_UpdateWindowSize(const til::size sizeNew)
             siAttached.UpdateScrollBars();
         }
 
-        // MSFT: 12092729
-        // To fix an issue with 3rd party applications that wrap our console, notify that the screen buffer size changed
-        // when the window viewport is updated.
-        // ---
-        // - The specific scenario that this impacts is ConEmu (wrapping our console) to use Bash in WSL.
-        // - The reason this is a problem is because ConEmu has to programmatically manipulate our buffer and window size
-        //   one after another to get our dimensions to change.
-        // - The WSL layer watches our Buffer change message to know when to get the new Window size and send it into the
-        //   WSL environment. This isn't technically correct to use a Buffer message to know when Window changes, but
-        //   it's not totally their fault because we do not provide a Window changed message at all.
-        // - If our window is adjusted directly, the Buffer and Window dimensions are both updated simultaneously under lock
-        //   and WSL gets one message and updates appropriately.
-        // - If ConEmu updates it via the API, one is updated, then the other with an unlocked time interval.
-        //   The WSL layer will potentially get the Window size that hasn't been updated yet or is out of sync before the
-        //   other API call is completed which results in the application in the WSL environment thinking the window is
-        //   a different size and outputting VT sequences with an invalid assumption.
-        // - If we make it so a Window change also emits a Buffer change message, then WSL will be notified appropriately
-        //   and can pass that information into the WSL environment.
-        // - To Windows apps that weren't expecting this information, it should cause no harm because they should just receive
-        //   an additional Buffer message with the same size again and do nothing special.
-        ScreenBufferSizeChange(siAttached.GetActiveBuffer().GetBufferSize().Dimensions());
-
         _resizingWindow--;
     }