Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ConPTY: Fix missing flush on console mode changes #15991

Merged
merged 1 commit into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion src/host/VtIo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
3 changes: 3 additions & 0 deletions src/host/VtIo.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
19 changes: 13 additions & 6 deletions src/host/_stream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) };
Expand Down
22 changes: 12 additions & 10 deletions src/host/getset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
23 changes: 1 addition & 22 deletions src/host/inputBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions src/host/inputBuffer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion src/renderer/vt/XtermEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,8 @@ CATCH_RETURN();
{
RETURN_IF_FAILED(_Write("\x1b[2t"));
}
return _Flush();
_Flush();
return S_OK;
}

// Method Description:
Expand Down
5 changes: 0 additions & 5 deletions src/renderer/vt/invalidate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
17 changes: 1 addition & 16 deletions src/renderer/vt/paint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
38 changes: 29 additions & 9 deletions src/renderer/vt/state.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}
8 changes: 5 additions & 3 deletions src/renderer/vt/vtrenderer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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)
Expand Down
Loading