Skip to content

Commit

Permalink
Make sure DCS strings are flushed to conpty without delay (microsoft#…
Browse files Browse the repository at this point in the history
…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 microsoft#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 microsoft#17111
  • Loading branch information
j4james authored May 6, 2024
1 parent 80d2e58 commit 9c16c5c
Show file tree
Hide file tree
Showing 10 changed files with 19 additions and 13 deletions.
10 changes: 7 additions & 3 deletions src/renderer/vt/XtermEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) :
Expand All @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion src/renderer/vt/XtermEngine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion src/renderer/vt/vtrenderer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<void(bool)> pfnLooking) noexcept;
Expand Down
2 changes: 1 addition & 1 deletion src/terminal/adapter/adaptDispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4882,7 +4882,7 @@ ITermDispatch::StringHandler AdaptDispatch::_CreatePassthroughHandler()
{
buffer += L'\\';
}
engine.ActionPassThroughString(buffer);
engine.ActionPassThroughString(buffer, true);
buffer.clear();
}
return !endOfString;
Expand Down
2 changes: 1 addition & 1 deletion src/terminal/parser/IStateMachineEngine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion src/terminal/parser/InputStateMachineEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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())
{
Expand Down
2 changes: 1 addition & 1 deletion src/terminal/parser/InputStateMachineEngine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
5 changes: 3 additions & 2 deletions src/terminal/parser/OutputStateMachineEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion src/terminal/parser/OutputStateMachineEngine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion src/terminal/parser/ut_parser/StateMachineTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 9c16c5c

Please sign in to comment.