ConPTY: Fix missing flush on console mode changes (microsoft#15991)

Previously, all unknown escape sequences would lead to an immediate call
to `VtEngine::_Flush()`. This lead to problems with nushell which uses
FTCS marks that were unknown to us. Combined with the linewise redrawing
that nushell does, Terminal would get the prompt in two separate frames,
causing a slight flickering.

microsoft#14677 fixed this by suppressing the `_Flush()` call when unknown
sequences are encountered. Unfortunately, this triggered a bug due
to our somewhat "inconsistent" architecture in conhost:
`XtermEngine::WriteTerminalW` isn't just used to flush unknown sequences
but also used directly by `InputBuffer::PassThroughWin32MouseRequest`
to write its mouse sequence directly to the ConPTY host.
`VtEngine` already contains a number of specialized member functions
like `RequestWin32Input()` to ensure that `_Flush()` is called
immediately and another member could've been added to solve this issue.
This commit now adds `RequestMouseMode` in the same vein.

But I believe we can make the system more robust in general by using
eager flushing by default (= safe), similar to how a `write()` on a
TCP socket flushes by default, and instead only selectively pause and
unpause flushing with a system similar to `TCP_CORK`.

This seems to work fairly well, as it solves:
* The original nushell bug
* The new bug
* Improves overall throughput by ~33% (due to less flushing)

In particular the last point is noteworthy, as this commit removes
the last performance bottleneck in ConPTY that isn't `VtEngine`.
Around ~95% of all CPU and wall time is spent in there now and any
improvements to `VtEngine` should yield immediately results.

Closes microsoft#15711

## Validation Steps Performed
* Clone/Run https://github.com/chrisant996/repro_enable_mouse_input
* Hold Ctrl+Alt and circle with the mouse over the viewport
* Repro.exe prints the current cursor coordinates ✅
* Run nushell
* No flickering when typing in the prompt ✅

src/host/VtIo.cpp

@@ -255,7 +255,6 @@ bool VtIo::IsUsingVt() const
         {
             g.pRender->AddRenderEngine(_pVtRenderEngine.get());
             g.getConsoleInformation().GetActiveOutputBuffer().SetTerminalConnection(_pVtRenderEngine.get());
-            g.getConsoleInformation().GetActiveInputBuffer()->SetTerminalConnection(_pVtRenderEngine.get());
 
             // Force the whole window to be put together first.
             // We don't really need the handle, we just want to leverage the setup steps.
@@ -497,6 +496,18 @@ void VtIo::EndResize()
     }
 }
 
+// The name of this method is an analogy to TCP_CORK. It instructs
+// the VT renderer to stop flushing its buffer to the output pipe.
+// Don't forget to uncork it!
+void VtIo::CorkRenderer(bool corked) const noexcept
+{
+    _pVtRenderEngine->Cork(corked);
+    if (!corked)
+    {
+        LOG_IF_FAILED(ServiceLocator::LocateGlobals().pRender->PaintFrame());
+    }
+}
+
 #ifdef UNIT_TESTING
 // Method Description:
 // - This is a test helper method. It can be used to trick VtIo into responding
@@ -547,3 +558,12 @@ bool VtIo::IsResizeQuirkEnabled() const
     }
     return S_OK;
 }
+
+[[nodiscard]] HRESULT VtIo::RequestMouseMode(bool enable) const noexcept
+{
+    if (_pVtRenderEngine)
+    {
+        return _pVtRenderEngine->RequestMouseMode(enable);
+    }
+    return S_OK;
+}

src/host/VtIo.hpp

@@ -43,13 +43,16 @@ namespace Microsoft::Console::VirtualTerminal
         void BeginResize();
         void EndResize();
 
+        void CorkRenderer(bool corked) const noexcept;
+
 #ifdef UNIT_TESTING
         void EnableConptyModeForTests(std::unique_ptr<Microsoft::Console::Render::VtEngine> vtRenderEngine);
 #endif
 
         bool IsResizeQuirkEnabled() const;
 
         [[nodiscard]] HRESULT ManuallyClearScrollback() const noexcept;
+        [[nodiscard]] HRESULT RequestMouseMode(bool enable) const noexcept;
 
         void CreatePseudoWindow();
         void SetWindowVisibility(bool showOrHide) noexcept;

src/host/_stream.cpp

@@ -334,17 +334,24 @@ try
         return CONSOLE_STATUS_WAIT;
     }
 
-    auto restoreVtQuirk{
-        wil::scope_exit([&]() { screenInfo.ResetIgnoreLegacyEquivalentVTAttributes(); })
-    };
-
+    const auto vtIo = ServiceLocator::LocateGlobals().getConsoleInformation().GetVtIo();
+    const auto restoreVtQuirk = wil::scope_exit([&]() {
+        if (requiresVtQuirk)
+        {
+            screenInfo.ResetIgnoreLegacyEquivalentVTAttributes();
+        }
+        if (vtIo->IsUsingVt())
+        {
+            vtIo->CorkRenderer(false);
+        }
+    });
     if (requiresVtQuirk)
     {
         screenInfo.SetIgnoreLegacyEquivalentVTAttributes();
     }
-    else
+    if (vtIo->IsUsingVt())
     {
-        restoreVtQuirk.release();
+        vtIo->CorkRenderer(true);
     }
 
     const std::wstring_view str{ pwchBuffer, *pcbBuffer / sizeof(WCHAR) };

src/host/getset.cpp

@@ -368,17 +368,19 @@ void ApiRoutines::GetNumberOfConsoleMouseButtonsImpl(ULONG& buttons) noexcept
             WI_ClearFlag(gci.Flags, CONSOLE_USE_PRIVATE_FLAGS);
         }
 
-        const auto newQuickEditMode{ WI_IsFlagSet(gci.Flags, CONSOLE_QUICK_EDIT_MODE) };
-
-        // Mouse input should be received when mouse mode is on and quick edit mode is off
-        // (for more information regarding the quirks of mouse mode and why/how it relates
-        //  to quick edit mode, see GH#9970)
-        const auto oldMouseMode{ !oldQuickEditMode && WI_IsFlagSet(context.InputMode, ENABLE_MOUSE_INPUT) };
-        const auto newMouseMode{ !newQuickEditMode && WI_IsFlagSet(mode, ENABLE_MOUSE_INPUT) };
-
-        if (oldMouseMode != newMouseMode)
+        if (gci.IsInVtIoMode())
         {
-            gci.GetActiveInputBuffer()->PassThroughWin32MouseRequest(newMouseMode);
+            // Mouse input should be received when mouse mode is on and quick edit mode is off
+            // (for more information regarding the quirks of mouse mode and why/how it relates
+            //  to quick edit mode, see GH#9970)
+            const auto newQuickEditMode{ WI_IsFlagSet(gci.Flags, CONSOLE_QUICK_EDIT_MODE) };
+            const auto oldMouseMode{ !oldQuickEditMode && WI_IsFlagSet(context.InputMode, ENABLE_MOUSE_INPUT) };
+            const auto newMouseMode{ !newQuickEditMode && WI_IsFlagSet(mode, ENABLE_MOUSE_INPUT) };
+
+            if (oldMouseMode != newMouseMode)
+            {
+                LOG_IF_FAILED(gci.GetVtIo()->RequestMouseMode(newMouseMode));
+            }
         }
 
         context.InputMode = mode;

src/host/inputBuffer.cpp

@@ -25,8 +25,7 @@ using namespace Microsoft::Console;
 // Return Value:
 // - A new instance of InputBuffer
 InputBuffer::InputBuffer() :
-    InputMode{ INPUT_BUFFER_DEFAULT_INPUT_MODE },
-    _pTtyConnection(nullptr)
+    InputMode{ INPUT_BUFFER_DEFAULT_INPUT_MODE }
 {
     // initialize buffer header
     fInComposition = false;
@@ -342,26 +341,6 @@ void InputBuffer::FlushAllButKeys()
     _storage.erase(newEnd, _storage.end());
 }
 
-void InputBuffer::SetTerminalConnection(_In_ Render::VtEngine* const pTtyConnection)
-{
-    this->_pTtyConnection = pTtyConnection;
-}
-
-void InputBuffer::PassThroughWin32MouseRequest(bool enable)
-{
-    if (_pTtyConnection)
-    {
-        if (enable)
-        {
-            LOG_IF_FAILED(_pTtyConnection->WriteTerminalW(L"\x1b[?1003;1006h"));
-        }
-        else
-        {
-            LOG_IF_FAILED(_pTtyConnection->WriteTerminalW(L"\x1b[?1003;1006l"));
-        }
-    }
-}
-
 // Routine Description:
 // - This routine reads from the input buffer.
 // - It can convert returned data to through the currently set Input CP, it can optionally return a wait condition

src/host/inputBuffer.hpp

@@ -63,8 +63,6 @@ class InputBuffer final : public ConsoleObjectHeader
 
     bool IsInVirtualTerminalInputMode() const;
     Microsoft::Console::VirtualTerminal::TerminalInput& GetTerminalInput();
-    void SetTerminalConnection(_In_ Microsoft::Console::Render::VtEngine* const pTtyConnection);
-    void PassThroughWin32MouseRequest(bool enable);
 
 private:
     enum class ReadingMode : uint8_t
@@ -86,7 +84,6 @@ class InputBuffer final : public ConsoleObjectHeader
     INPUT_RECORD _writePartialByteSequence{};
     bool _writePartialByteSequenceAvailable = false;
     Microsoft::Console::VirtualTerminal::TerminalInput _termInput;
-    Microsoft::Console::Render::VtEngine* _pTtyConnection;
 
     // This flag is used in _HandleTerminalInputCallback
     // If the InputBuffer leads to a _HandleTerminalInputCallback call,

src/renderer/vt/XtermEngine.cpp

@@ -555,7 +555,8 @@ CATCH_RETURN();
     {
         RETURN_IF_FAILED(_Write("\x1b[2t"));
     }
-    return _Flush();
+    _Flush();
+    return S_OK;
 }
 
 // Method Description:

src/renderer/vt/invalidate.cpp

@@ -120,12 +120,7 @@ CATCH_RETURN();
         _circled = circled;
     }
 
-    // If we flushed for any reason other than circling (i.e, a sequence that we
-    // didn't understand), we don't need to push the buffer out on EndPaint.
-    _noFlushOnEnd = !circled;
-
     _trace.TraceTriggerCircling(*pForcePaint);
-
     return S_OK;
 }
 

src/renderer/vt/paint.cpp

@@ -94,22 +94,7 @@ using namespace Microsoft::Console::Types;
         RETURN_IF_FAILED(_MoveCursor(_deferredCursorPos));
     }
 
-    // If this frame was triggered because we encountered a VT sequence which
-    // required the buffered state to get printed, we don't want to flush this
-    // frame to the pipe. That might result in us rendering half the output of a
-    // particular frame (as emitted by the client).
-    //
-    // Instead, we'll leave this frame in _buffer, and just keep appending to
-    // it as needed.
-    if (_noFlushOnEnd) [[unlikely]]
-    {
-        _noFlushOnEnd = false;
-    }
-    else
-    {
-        RETURN_IF_FAILED(_Flush());
-    }
-
+    _Flush();
     return S_OK;
 }
 

src/renderer/vt/state.cpp

@@ -46,7 +46,6 @@ VtEngine::VtEngine(_In_ wil::unique_hfile pipe,
     _circled(false),
     _firstPaint(true),
     _skipCursor(false),
-    _exitResult{ S_OK },
     _terminalOwner{ nullptr },
     _newBottomLine{ false },
     _deferredCursorPos{ INVALID_COORDS },
@@ -136,25 +135,39 @@ CATCH_RETURN();
     CATCH_RETURN();
 }
 
-[[nodiscard]] HRESULT VtEngine::_Flush() noexcept
+void VtEngine::_Flush() noexcept
+{
+    if (!_corked && !_buffer.empty())
+    {
+        _flushImpl();
+    }
+}
+
+// _corked is often true and separating _flushImpl() out allows _flush() to be inlined.
+void VtEngine::_flushImpl() noexcept
 {
     if (_hFile)
     {
-        auto fSuccess = !!WriteFile(_hFile.get(), _buffer.data(), gsl::narrow_cast<DWORD>(_buffer.size()), nullptr, nullptr);
+        const auto fSuccess = WriteFile(_hFile.get(), _buffer.data(), gsl::narrow_cast<DWORD>(_buffer.size()), nullptr, nullptr);
         _buffer.clear();
         if (!fSuccess)
         {
-            _exitResult = HRESULT_FROM_WIN32(GetLastError());
+            LOG_LAST_ERROR();
             _hFile.reset();
             if (_terminalOwner)
             {
                 _terminalOwner->CloseOutput();
             }
-            return _exitResult;
         }
     }
+}
 
-    return S_OK;
+// The name of this method is an analogy to TCP_CORK. It instructs
+// the VT renderer to stop flushing its buffer to the output pipe.
+// Don't forget to uncork it!
+void VtEngine::Cork(bool corked) noexcept
+{
+    _corked = corked;
 }
 
 // Method Description:
@@ -425,7 +438,7 @@ void VtEngine::SetTerminalOwner(Microsoft::Console::VirtualTerminal::VtIo* const
 HRESULT VtEngine::RequestCursor() noexcept
 {
     RETURN_IF_FAILED(_RequestCursor());
-    RETURN_IF_FAILED(_Flush());
+    _Flush();
     return S_OK;
 }
 
@@ -546,13 +559,20 @@ HRESULT VtEngine::RequestWin32Input() noexcept
     // in the connected terminal after passing through an RIS sequence.
     RETURN_IF_FAILED(_RequestWin32Input());
     RETURN_IF_FAILED(_RequestFocusEventMode());
-    RETURN_IF_FAILED(_Flush());
+    _Flush();
     return S_OK;
 }
 
 HRESULT VtEngine::SwitchScreenBuffer(const bool useAltBuffer) noexcept
 {
     RETURN_IF_FAILED(_SwitchScreenBuffer(useAltBuffer));
-    RETURN_IF_FAILED(_Flush());
+    _Flush();
     return S_OK;
 }
+
+HRESULT VtEngine::RequestMouseMode(const bool enable) noexcept
+{
+    const auto status = _WriteFormatted(FMT_COMPILE("\x1b[?1003;1006{}"), enable ? 'h' : 'l');
+    _Flush();
+    return status;
+}

src/renderer/vt/vtrenderer.hpp

@@ -90,6 +90,8 @@ namespace Microsoft::Console::Render
         [[nodiscard]] HRESULT RequestWin32Input() noexcept;
         [[nodiscard]] virtual HRESULT SetWindowVisibility(const bool showOrHide) noexcept = 0;
         [[nodiscard]] HRESULT SwitchScreenBuffer(const bool useAltBuffer) noexcept;
+        [[nodiscard]] HRESULT RequestMouseMode(bool enable) noexcept;
+        void Cork(bool corked) noexcept;
 
     protected:
         wil::unique_hfile _hFile;
@@ -127,7 +129,6 @@ namespace Microsoft::Console::Render
         bool _newBottomLine;
         til::point _deferredCursorPos;
 
-        HRESULT _exitResult;
         Microsoft::Console::VirtualTerminal::VtIo* _terminalOwner;
 
         Microsoft::Console::VirtualTerminal::RenderTracing _trace;
@@ -139,12 +140,13 @@ namespace Microsoft::Console::Render
 
         bool _resizeQuirk{ false };
         bool _passthrough{ false };
-        bool _noFlushOnEnd{ false };
+        bool _corked{ false };
         std::optional<TextColor> _newBottomLineBG{ std::nullopt };
 
         [[nodiscard]] HRESULT _WriteFill(const size_t n, const char c) noexcept;
         [[nodiscard]] HRESULT _Write(std::string_view const str) noexcept;
-        [[nodiscard]] HRESULT _Flush() noexcept;
+        void _Flush() noexcept;
+        void _flushImpl() noexcept;
 
         template<typename S, typename... Args>
         [[nodiscard]] HRESULT _WriteFormatted(S&& format, Args&&... args)