Properly fix ConPTY buffer corking (#16793)

I've found that #16079 was never properly addressed (it still randomly
occurred after even after PR #16349), which later led to the issues
described in #16769 (nushell flickering due to too many flushes).

The crux of the fix is that this brings back the `_noFlushOnEnd` flag
that was removed in PR #15991. This is then combined with a change to
the cork API: An `uncork` on `VtEngine` now only flushes if `_Flush`
got called while it was corked in the first place.

`_noFlushOnEnd` prevents us from flushing in between two "unknown"
VT sequences (like soft fonts or FTCS) which prevents them from being
corrupted. The corking prevents the remaining cases of flushing too
often. Long-term, a proper fix would be to pass through VT unmodified.

Closes #16769

(cherry picked from commit 1ede023)
Service-Card-Id: 91965217
Service-Version: 1.20

Author: lhecker

src/renderer/vt/invalidate.cpp

@@ -112,6 +112,10 @@ CATCH_RETURN();
     //      end paint to specifically handle this.
     _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,7 +94,19 @@ using namespace Microsoft::Console::Types;
         RETURN_IF_FAILED(_MoveCursor(_deferredCursorPos));
     }
 
-    _Flush();
+    // 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)
+    {
+        _Flush();
+    }
+
+    _noFlushOnEnd = false;
     return S_OK;
 }
 

src/renderer/vt/state.cpp

@@ -136,10 +136,19 @@ CATCH_RETURN();
 
 void VtEngine::_Flush() noexcept
 {
-    if (!_corked && !_buffer.empty())
+    if (_buffer.empty())
+    {
+        return;
+    }
+
+    if (!_corked)
     {
         _flushImpl();
+        return;
     }
+
+    // Defer the flush until someone calls Cork(false).
+    _flushRequested = true;
 }
 
 // _corked is often true and separating _flushImpl() out allows _flush() to be inlined.
@@ -167,7 +176,13 @@ void VtEngine::_flushImpl() noexcept
 void VtEngine::Cork(bool corked) noexcept
 {
     _corked = corked;
-    _Flush();
+
+    // Now do the deferred flush from a previous call to _Flush().
+    if (!corked && _flushRequested)
+    {
+        _flushRequested = false;
+        _flushImpl();
+    }
 }
 
 // Method Description:

src/renderer/vt/vtrenderer.hpp

@@ -138,7 +138,9 @@ namespace Microsoft::Console::Render
 
         bool _resizeQuirk{ false };
         bool _passthrough{ false };
+        bool _noFlushOnEnd{ false };
         bool _corked{ false };
+        bool _flushRequested{ false };
         std::optional<TextColor> _newBottomLineBG{ std::nullopt };
 
         [[nodiscard]] HRESULT _WriteFill(const size_t n, const char c) noexcept;