From 31e904a7ca4ba8d3e1a73c5feec65d967e005460 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 28 Mar 2023 15:48:40 -0500 Subject: [PATCH] There's no reason that this should have entirely broken moving panes. That doesn't make sense. --- src/cascadia/TerminalControl/ControlCore.cpp | 23 ++++++++++--- src/cascadia/TerminalControl/ControlCore.h | 2 +- src/cascadia/TerminalControl/TermControl.cpp | 36 +++++++------------- src/cascadia/TerminalControl/TermControl.h | 2 +- 4 files changed, 33 insertions(+), 30 deletions(-) diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 8b0e90a69f2..0b58ffd730a 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -1555,19 +1555,32 @@ namespace winrt::Microsoft::Terminal::Control::implementation _RendererWarningHandlers(*this, winrt::make(hr)); } - void ControlCore::_renderEngineSwapChainChanged(const HANDLE handle) + winrt::fire_and_forget ControlCore::_renderEngineSwapChainChanged(const HANDLE sourceHandle) { - // `handle` is a weak ref to a HANDLE that's ultimately owned by the + // `sourceHandle` is a weak ref to a HANDLE that's ultimately owned by the // render engine's own unique_handle. We'll add another ref to it here. // This will make sure that we always have a valid HANDLE to give to // callers of our own SwapChainHandle method, even if the renderer is // currently in the process of discarding this value and creating a new // one. Callers should have already set up the SwapChainChanged // callback, so this all works out. - _lastSwapChainHandle.attach(handle); - // Now bubble the event up to the control. - _SwapChainChangedHandlers(*this, winrt::box_value(reinterpret_cast(handle))); + winrt::handle duplicatedHandle; + const auto processHandle = GetCurrentProcess(); + THROW_IF_WIN32_BOOL_FALSE(DuplicateHandle(processHandle, sourceHandle, processHandle, duplicatedHandle.put(), 0, FALSE, DUPLICATE_SAME_ACCESS)); + + const auto weakThis{ get_weak() }; + + co_await wil::resume_foreground(_dispatcher); + + if (auto core{ weakThis.get() }) + { + // `this` is safe to use now + + _lastSwapChainHandle = (std::move(duplicatedHandle)); + // Now bubble the event up to the control. + _SwapChainChangedHandlers(*this, winrt::box_value(reinterpret_cast(_lastSwapChainHandle.get()))); + } } void ControlCore::_rendererBackgroundColorChanged() diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h index fa263b65888..8f7e40e6242 100644 --- a/src/cascadia/TerminalControl/ControlCore.h +++ b/src/cascadia/TerminalControl/ControlCore.h @@ -328,7 +328,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation #pragma region RendererCallbacks void _rendererWarning(const HRESULT hr); - void _renderEngineSwapChainChanged(const HANDLE handle); + winrt::fire_and_forget _renderEngineSwapChainChanged(const HANDLE handle); void _rendererBackgroundColorChanged(); void _rendererTabColorChanged(); #pragma endregion diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index c0e84977943..841b72a9cd4 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -866,28 +866,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation return _core.ConnectionState(); } - winrt::fire_and_forget TermControl::RenderEngineSwapChainChanged(IInspectable /*sender*/, IInspectable args) + void TermControl::RenderEngineSwapChainChanged(IInspectable /*sender*/, IInspectable args) { - // This event comes in on the render thread, not the UI thread. - - const auto weakThis{ get_weak() }; - - winrt::handle handle; - - // Add a ref to the handle passed to us, so that the HANDLE will remain - // valid to the other side of the co_await. - handle.attach(reinterpret_cast(winrt::unbox_value(args))); - - co_await wil::resume_foreground(Dispatcher()); - - if (auto control{ weakThis.get() }) - { - _AttachDxgiSwapChainToXaml(handle.get()); - } - // Detach from the handle. If you don't do this, we'll CloseHandle() on - // the handle when `handle` goes out of scope, resulting in the swap - // chain being closed. - handle.detach(); + // This event comes in on the UI thread + HANDLE h = reinterpret_cast(winrt::unbox_value(args)); + _AttachDxgiSwapChainToXaml(h); } // Method Description: @@ -1818,7 +1801,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation // GH#5421: Enable the UiaEngine before checking for the SearchBox // That way, new selections are notified to automation clients. // The _uiaEngine lives in _interactivity, so call into there to enable it. - _interactivity.GotFocus(); + + if (_interactivity) + { + _interactivity.GotFocus(); + } // If the searchbox is focused, we don't want TSFInputControl to think // it has focus so it doesn't intercept IME input. We also don't want the @@ -1873,7 +1860,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation // This will disable the accessibility notifications, because the // UiaEngine lives in ControlInteractivity - _interactivity.LostFocus(); + if (_interactivity) + { + _interactivity.LostFocus(); + } if (TSFInputControl() != nullptr) { diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index 56767936fb2..f06a50f3373 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -95,7 +95,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation void ToggleShaderEffects(); - winrt::fire_and_forget RenderEngineSwapChainChanged(IInspectable sender, IInspectable args); + void RenderEngineSwapChainChanged(IInspectable sender, IInspectable args); void _AttachDxgiSwapChainToXaml(HANDLE swapChainHandle); winrt::fire_and_forget _RendererEnteredErrorState(IInspectable sender, IInspectable args);