From de65659330ba8fe7e6baa8bebb532915e96874fb Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 29 Jul 2019 15:35:59 -0500 Subject: [PATCH 1/2] Don't trigger a frame due to circling when in the middle of a resize operation This fixes #1795, and shined quite a bit of light on the whole conpty resize process. --- src/host/PtySignalInputThread.cpp | 4 +++ src/host/VtIo.cpp | 34 +++++++++++++++++++++++ src/host/VtIo.hpp | 3 ++ src/renderer/vt/invalidate.cpp | 16 ++++++++--- src/renderer/vt/state.cpp | 29 +++++++++++++++++++ src/renderer/vt/vtrenderer.hpp | 3 ++ src/terminal/adapter/InteractDispatch.cpp | 4 +++ 7 files changed, 89 insertions(+), 4 deletions(-) diff --git a/src/host/PtySignalInputThread.cpp b/src/host/PtySignalInputThread.cpp index c2218d0a794..c73af246a33 100644 --- a/src/host/PtySignalInputThread.cpp +++ b/src/host/PtySignalInputThread.cpp @@ -107,6 +107,10 @@ void PtySignalInputThread::ConnectConsole() noexcept } else { + ServiceLocator::LocateGlobals().getConsoleInformation().GetVtIo()->BeginResize(); + auto endResize = wil::scope_exit([&] { + ServiceLocator::LocateGlobals().getConsoleInformation().GetVtIo()->EndResize(); + }); if (DispatchCommon::s_ResizeWindow(*_pConApi, resizeMsg.sx, resizeMsg.sy)) { DispatchCommon::s_SuppressResizeRepaint(*_pConApi); diff --git a/src/host/VtIo.cpp b/src/host/VtIo.cpp index 3ea8d7fb386..cf7ab1ecbe5 100644 --- a/src/host/VtIo.cpp +++ b/src/host/VtIo.cpp @@ -397,3 +397,37 @@ void VtIo::_ShutdownIfNeeded() ServiceLocator::RundownAndExit(ERROR_BROKEN_PIPE); } } + +// Method Description: +// - Tell the vt renderer to begin a resize operation. During a resize +// operation, the vt renderer should _not_ request to be repainted during a +// text buffer circling event. Any callers of this method should make sure to +// call EndResize to make sure the renderer returns to normal behavior. +// See GH#1795 for context on this method. +// Arguments: +// - +// Return Value: +// - +void VtIo::BeginResize() +{ + if (_pVtRenderEngine) + { + _pVtRenderEngine->BeginResizeRequest(); + } +} + +// Method Description: +// - Tell the vt renderer to end a resize operation. +// See BeginResize for more details. +// See GH#1795 for context on this method. +// Arguments: +// - +// Return Value: +// - +void VtIo::EndResize() +{ + if (_pVtRenderEngine) + { + _pVtRenderEngine->EndResizeRequest(); + } +} diff --git a/src/host/VtIo.hpp b/src/host/VtIo.hpp index dc037b9372a..0516d9ab0ea 100644 --- a/src/host/VtIo.hpp +++ b/src/host/VtIo.hpp @@ -36,6 +36,9 @@ namespace Microsoft::Console::VirtualTerminal void CloseInput() override; void CloseOutput() override; + void BeginResize(); + void EndResize(); + private: // After CreateIoHandlers is called, these will be invalid. wil::unique_hfile _hInput; diff --git a/src/renderer/vt/invalidate.cpp b/src/renderer/vt/invalidate.cpp index 1272615baff..6d9caea7a63 100644 --- a/src/renderer/vt/invalidate.cpp +++ b/src/renderer/vt/invalidate.cpp @@ -102,11 +102,19 @@ using namespace Microsoft::Console::Render; // - S_OK [[nodiscard]] HRESULT VtEngine::InvalidateCircling(_Out_ bool* const pForcePaint) noexcept { - *pForcePaint = true; + // If we're in the middle of a resize request, don't try to immediately start a frame. + if (_inResizeRequest) + { + *pForcePaint = false; + } + else + { + *pForcePaint = true; - // Keep track of the fact that we circled, we'll need to do some work on - // end paint to specifically handle this. - _circled = true; + // Keep track of the fact that we circled, we'll need to do some work on + // end paint to specifically handle this. + _circled = true; + } return S_OK; } diff --git a/src/renderer/vt/state.cpp b/src/renderer/vt/state.cpp index d2b26bc2f5d..31cca05d4a2 100644 --- a/src/renderer/vt/state.cpp +++ b/src/renderer/vt/state.cpp @@ -54,6 +54,7 @@ VtEngine::VtEngine(_In_ wil::unique_hfile pipe, _terminalOwner{ nullptr }, _newBottomLine{ false }, _deferredCursorPos{ INVALID_COORDS }, + _inResizeRequest{ false }, _trace{} { #ifndef UNIT_TESTING @@ -417,3 +418,31 @@ HRESULT VtEngine::RequestCursor() noexcept RETURN_IF_FAILED(_Flush()); return S_OK; } + +// Method Description: +// - Tell the vt renderer to begin a resize operation. During a resize +// operation, the vt renderer should _not_ request to be repainted during a +// text buffer circling event. Any callers of this method should make sure to +// call EndResize to make sure the renderer returns to normal behavior. +// See GH#1795 for context on this method. +// Arguments: +// - +// Return Value: +// - +void VtEngine::BeginResizeRequest() +{ + _inResizeRequest = true; +} + +// Method Description: +// - Tell the vt renderer to end a resize operation. +// See BeginResize for more details. +// See GH#1795 for context on this method. +// Arguments: +// - +// Return Value: +// - +void VtEngine::EndResizeRequest() +{ + _inResizeRequest = false; +} diff --git a/src/renderer/vt/vtrenderer.hpp b/src/renderer/vt/vtrenderer.hpp index 77a9c6349c2..7ca3aa37af3 100644 --- a/src/renderer/vt/vtrenderer.hpp +++ b/src/renderer/vt/vtrenderer.hpp @@ -94,6 +94,8 @@ namespace Microsoft::Console::Render [[nodiscard]] virtual HRESULT WriteTerminalW(const std::wstring& str) noexcept = 0; void SetTerminalOwner(Microsoft::Console::ITerminalOwner* const terminalOwner); + void BeginResizeRequest(); + void EndResizeRequest(); protected: wil::unique_hfile _hFile; @@ -132,6 +134,7 @@ namespace Microsoft::Console::Render Microsoft::Console::ITerminalOwner* _terminalOwner; Microsoft::Console::VirtualTerminal::RenderTracing _trace; + bool _inResizeRequest{ false }; [[nodiscard]] HRESULT _Write(std::string_view const str) noexcept; [[nodiscard]] HRESULT _WriteFormattedString(const std::string* const pFormat, ...) noexcept; diff --git a/src/terminal/adapter/InteractDispatch.cpp b/src/terminal/adapter/InteractDispatch.cpp index afbddc0fdf1..e8c71a18423 100644 --- a/src/terminal/adapter/InteractDispatch.cpp +++ b/src/terminal/adapter/InteractDispatch.cpp @@ -114,6 +114,10 @@ bool InteractDispatch::WindowManipulation(const DispatchTypes::WindowManipulatio } break; case DispatchTypes::WindowManipulationType::ResizeWindowInCharacters: + // TODO:GH#1765 This should BeginResize and EndResize, like the signal + // pipe does. Either that, or we should introduce a better + // `ResizeConpty` function to the ConGetSet interface, that specifically + // handles a conpty resize. if (cParams == 2) { fSuccess = DispatchCommon::s_ResizeWindow(*_pConApi, rgusParams[1], rgusParams[0]); From 69ceffe67be5aa76e7e51ebfe12d8c103b9c1334 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 30 Jul 2019 09:07:26 -0500 Subject: [PATCH 2/2] Move the Begin/End to ResizeScreenBuffer, to catch more cases. --- src/host/PtySignalInputThread.cpp | 4 ---- src/host/screenInfo.cpp | 13 +++++++++++++ src/terminal/adapter/InteractDispatch.cpp | 6 ++---- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/host/PtySignalInputThread.cpp b/src/host/PtySignalInputThread.cpp index c73af246a33..c2218d0a794 100644 --- a/src/host/PtySignalInputThread.cpp +++ b/src/host/PtySignalInputThread.cpp @@ -107,10 +107,6 @@ void PtySignalInputThread::ConnectConsole() noexcept } else { - ServiceLocator::LocateGlobals().getConsoleInformation().GetVtIo()->BeginResize(); - auto endResize = wil::scope_exit([&] { - ServiceLocator::LocateGlobals().getConsoleInformation().GetVtIo()->EndResize(); - }); if (DispatchCommon::s_ResizeWindow(*_pConApi, resizeMsg.sx, resizeMsg.sy)) { DispatchCommon::s_SuppressResizeRepaint(*_pConApi); diff --git a/src/host/screenInfo.cpp b/src/host/screenInfo.cpp index c8bee162166..07233506777 100644 --- a/src/host/screenInfo.cpp +++ b/src/host/screenInfo.cpp @@ -1670,6 +1670,19 @@ bool SCREEN_INFORMATION::IsMaximizedY() const CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); NTSTATUS status = STATUS_SUCCESS; + // If we're in conpty mode, suppress any immediate painting we might do + // during the resize. + if (gci.IsInVtIoMode()) + { + gci.GetVtIo()->BeginResize(); + } + auto endResize = wil::scope_exit([&] { + if (gci.IsInVtIoMode()) + { + gci.GetVtIo()->EndResize(); + } + }); + // cancel any active selection before resizing or it will not necessarily line up with the new buffer positions Selection::Instance().ClearSelection(); diff --git a/src/terminal/adapter/InteractDispatch.cpp b/src/terminal/adapter/InteractDispatch.cpp index e8c71a18423..74c918bffe1 100644 --- a/src/terminal/adapter/InteractDispatch.cpp +++ b/src/terminal/adapter/InteractDispatch.cpp @@ -114,10 +114,8 @@ bool InteractDispatch::WindowManipulation(const DispatchTypes::WindowManipulatio } break; case DispatchTypes::WindowManipulationType::ResizeWindowInCharacters: - // TODO:GH#1765 This should BeginResize and EndResize, like the signal - // pipe does. Either that, or we should introduce a better - // `ResizeConpty` function to the ConGetSet interface, that specifically - // handles a conpty resize. + // TODO:GH#1765 We should introduce a better `ResizeConpty` function to + // the ConGetSet interface, that specifically handles a conpty resize. if (cParams == 2) { fSuccess = DispatchCommon::s_ResizeWindow(*_pConApi, rgusParams[1], rgusParams[0]);