From 9c16c5ca82943c2c466fb16a093a559d53a2fb0c Mon Sep 17 00:00:00 2001 From: James Holderness Date: Mon, 6 May 2024 21:03:33 +0100 Subject: [PATCH] Make sure DCS strings are flushed to conpty without delay (#17195) When the `DCS` passthrough code was first implemented, it relied on the `ActionPassThroughString` method flushing the given string immediately. However, that has since stopped being the case, so `DCS` operations end up being delayed until the entire sequence has been parsed. This PR fixes the issue by introducing a `flush` parameter to force an immediate flush on the `ActionPassThroughString` method, as well as the `XtermEngine::WriteTerminalW` method that it calls. ## Validation Steps Performed I've confirmed that the test case in issue #17111 now updates the color table as soon as each color entry is parsed, instead of delaying the updates until the end of the sequence. Closes #17111 --- src/renderer/vt/XtermEngine.cpp | 10 +++++++--- src/renderer/vt/XtermEngine.hpp | 2 +- src/renderer/vt/vtrenderer.hpp | 2 +- src/terminal/adapter/adaptDispatch.cpp | 2 +- src/terminal/parser/IStateMachineEngine.hpp | 2 +- src/terminal/parser/InputStateMachineEngine.cpp | 3 ++- src/terminal/parser/InputStateMachineEngine.hpp | 2 +- src/terminal/parser/OutputStateMachineEngine.cpp | 5 +++-- src/terminal/parser/OutputStateMachineEngine.hpp | 2 +- src/terminal/parser/ut_parser/StateMachineTest.cpp | 2 +- 10 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/renderer/vt/XtermEngine.cpp b/src/renderer/vt/XtermEngine.cpp index b6d0c58a862..9c489aa29cc 100644 --- a/src/renderer/vt/XtermEngine.cpp +++ b/src/renderer/vt/XtermEngine.cpp @@ -520,9 +520,10 @@ CATCH_RETURN(); // proper utf-8 string, depending on our mode. // Arguments: // - wstr - wstring of text to be written +// - flush - set to true if the string should be flushed immediately // Return Value: // - S_OK or suitable HRESULT error from either conversion or writing pipe. -[[nodiscard]] HRESULT XtermEngine::WriteTerminalW(const std::wstring_view wstr) noexcept +[[nodiscard]] HRESULT XtermEngine::WriteTerminalW(const std::wstring_view wstr, const bool flush) noexcept { RETURN_IF_FAILED(_fUseAsciiOnly ? VtEngine::_WriteTerminalAscii(wstr) : @@ -535,8 +536,11 @@ CATCH_RETURN(); // cause flickering (where we've buffered some state but not the whole // "frame" as specified by the app). We'll just immediately buffer this // sequence, and flush it when the render thread comes around to paint the - // frame normally. - + // frame normally, unless a flush has been explicitly requested. + if (flush) + { + _flushImpl(); + } return S_OK; } diff --git a/src/renderer/vt/XtermEngine.hpp b/src/renderer/vt/XtermEngine.hpp index 6f1b480b61e..7449fb3e38b 100644 --- a/src/renderer/vt/XtermEngine.hpp +++ b/src/renderer/vt/XtermEngine.hpp @@ -51,7 +51,7 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT InvalidateScroll(const til::point* const pcoordDelta) noexcept override; - [[nodiscard]] HRESULT WriteTerminalW(const std::wstring_view str) noexcept override; + [[nodiscard]] HRESULT WriteTerminalW(const std::wstring_view str, const bool flush) noexcept override; [[nodiscard]] HRESULT SetWindowVisibility(const bool showOrHide) noexcept override; diff --git a/src/renderer/vt/vtrenderer.hpp b/src/renderer/vt/vtrenderer.hpp index b5c487fc1c0..26fc67cb138 100644 --- a/src/renderer/vt/vtrenderer.hpp +++ b/src/renderer/vt/vtrenderer.hpp @@ -78,7 +78,7 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT RequestCursor() noexcept; [[nodiscard]] HRESULT InheritCursor(const til::point coordCursor) noexcept; [[nodiscard]] HRESULT WriteTerminalUtf8(const std::string_view str) noexcept; - [[nodiscard]] virtual HRESULT WriteTerminalW(const std::wstring_view str) noexcept = 0; + [[nodiscard]] virtual HRESULT WriteTerminalW(const std::wstring_view str, const bool flush = false) noexcept = 0; void SetTerminalOwner(Microsoft::Console::VirtualTerminal::VtIo* const terminalOwner); void SetResizeQuirk(const bool resizeQuirk); void SetLookingForDSRCallback(std::function pfnLooking) noexcept; diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index bf038dd75ee..02c22db8eec 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -4882,7 +4882,7 @@ ITermDispatch::StringHandler AdaptDispatch::_CreatePassthroughHandler() { buffer += L'\\'; } - engine.ActionPassThroughString(buffer); + engine.ActionPassThroughString(buffer, true); buffer.clear(); } return !endOfString; diff --git a/src/terminal/parser/IStateMachineEngine.hpp b/src/terminal/parser/IStateMachineEngine.hpp index 1eefc5be2a5..d1d838ccc5e 100644 --- a/src/terminal/parser/IStateMachineEngine.hpp +++ b/src/terminal/parser/IStateMachineEngine.hpp @@ -35,7 +35,7 @@ namespace Microsoft::Console::VirtualTerminal virtual bool ActionPrint(const wchar_t wch) = 0; virtual bool ActionPrintString(const std::wstring_view string) = 0; - virtual bool ActionPassThroughString(const std::wstring_view string) = 0; + virtual bool ActionPassThroughString(const std::wstring_view string, const bool flush = false) = 0; virtual bool ActionEscDispatch(const VTID id) = 0; virtual bool ActionVt52EscDispatch(const VTID id, const VTParameters parameters) = 0; diff --git a/src/terminal/parser/InputStateMachineEngine.cpp b/src/terminal/parser/InputStateMachineEngine.cpp index df7a8c2cfb7..4a25d056f49 100644 --- a/src/terminal/parser/InputStateMachineEngine.cpp +++ b/src/terminal/parser/InputStateMachineEngine.cpp @@ -276,9 +276,10 @@ bool InputStateMachineEngine::ActionPrintString(const std::wstring_view string) // string of characters given. // Arguments: // - string - string to dispatch. +// - flush - not applicable to the input state machine. // Return Value: // - true iff we successfully dispatched the sequence. -bool InputStateMachineEngine::ActionPassThroughString(const std::wstring_view string) +bool InputStateMachineEngine::ActionPassThroughString(const std::wstring_view string, const bool /*flush*/) { if (_pDispatch->IsVtInputEnabled()) { diff --git a/src/terminal/parser/InputStateMachineEngine.hpp b/src/terminal/parser/InputStateMachineEngine.hpp index 6b90c729492..4b17669aedc 100644 --- a/src/terminal/parser/InputStateMachineEngine.hpp +++ b/src/terminal/parser/InputStateMachineEngine.hpp @@ -142,7 +142,7 @@ namespace Microsoft::Console::VirtualTerminal bool ActionPrintString(const std::wstring_view string) override; - bool ActionPassThroughString(const std::wstring_view string) override; + bool ActionPassThroughString(const std::wstring_view string, const bool flush) override; bool ActionEscDispatch(const VTID id) override; diff --git a/src/terminal/parser/OutputStateMachineEngine.cpp b/src/terminal/parser/OutputStateMachineEngine.cpp index 34d40fc850d..f4b3fcbf677 100644 --- a/src/terminal/parser/OutputStateMachineEngine.cpp +++ b/src/terminal/parser/OutputStateMachineEngine.cpp @@ -180,14 +180,15 @@ bool OutputStateMachineEngine::ActionPrintString(const std::wstring_view string) // we don't know what to do with it) // Arguments: // - string - string to dispatch. +// - flush - set to true if the string should be flushed immediately. // Return Value: // - true iff we successfully dispatched the sequence. -bool OutputStateMachineEngine::ActionPassThroughString(const std::wstring_view string) +bool OutputStateMachineEngine::ActionPassThroughString(const std::wstring_view string, const bool flush) { auto success = true; if (_pTtyConnection != nullptr) { - const auto hr = _pTtyConnection->WriteTerminalW(string); + const auto hr = _pTtyConnection->WriteTerminalW(string, flush); LOG_IF_FAILED(hr); success = SUCCEEDED(hr); } diff --git a/src/terminal/parser/OutputStateMachineEngine.hpp b/src/terminal/parser/OutputStateMachineEngine.hpp index 1ff22ab5a99..64fbf81503e 100644 --- a/src/terminal/parser/OutputStateMachineEngine.hpp +++ b/src/terminal/parser/OutputStateMachineEngine.hpp @@ -38,7 +38,7 @@ namespace Microsoft::Console::VirtualTerminal bool ActionPrintString(const std::wstring_view string) override; - bool ActionPassThroughString(const std::wstring_view string) override; + bool ActionPassThroughString(const std::wstring_view string, const bool flush) override; bool ActionEscDispatch(const VTID id) override; diff --git a/src/terminal/parser/ut_parser/StateMachineTest.cpp b/src/terminal/parser/ut_parser/StateMachineTest.cpp index 0bf36050e95..f910a72c1a4 100644 --- a/src/terminal/parser/ut_parser/StateMachineTest.cpp +++ b/src/terminal/parser/ut_parser/StateMachineTest.cpp @@ -59,7 +59,7 @@ class Microsoft::Console::VirtualTerminal::TestStateMachineEngine : public IStat return true; }; - bool ActionPassThroughString(const std::wstring_view string) override + bool ActionPassThroughString(const std::wstring_view string, const bool /*flush*/) override { passedThrough += string; return true;