From 8b0d6fe2b246a710aa6d21c90fbc99bf6692c821 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Fri, 1 Mar 2024 16:51:00 +0100 Subject: [PATCH] Properly fix ConPTY buffer corking --- src/renderer/vt/invalidate.cpp | 4 ++++ src/renderer/vt/paint.cpp | 14 +++++++++++++- src/renderer/vt/state.cpp | 19 +++++++++++++++++-- src/renderer/vt/vtrenderer.hpp | 2 ++ 4 files changed, 36 insertions(+), 3 deletions(-) diff --git a/src/renderer/vt/invalidate.cpp b/src/renderer/vt/invalidate.cpp index 8f80d3eb8ea..489a6b50bc1 100644 --- a/src/renderer/vt/invalidate.cpp +++ b/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; } diff --git a/src/renderer/vt/paint.cpp b/src/renderer/vt/paint.cpp index d8e3e66686b..1d87bb420d6 100644 --- a/src/renderer/vt/paint.cpp +++ b/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; } diff --git a/src/renderer/vt/state.cpp b/src/renderer/vt/state.cpp index 5bb6b7d694d..1223d0704c2 100644 --- a/src/renderer/vt/state.cpp +++ b/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: diff --git a/src/renderer/vt/vtrenderer.hpp b/src/renderer/vt/vtrenderer.hpp index 7a974850afc..66bf4c863a5 100644 --- a/src/renderer/vt/vtrenderer.hpp +++ b/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 _newBottomLineBG{ std::nullopt }; [[nodiscard]] HRESULT _WriteFill(const size_t n, const char c) noexcept;