From 7b0aca444f33cb6b71d445daa08f3d1917c287b2 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 31 Mar 2023 14:37:17 -0500 Subject: [PATCH] Enable tearing out tabs to create new windows (#14935) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _This is the last one 🎉_ ## Summary _In the final chapter of our tale, we present a PR of great significance. It grants the power to tear tabs from their windows and create a new window where they may be dropped, one not necessarily of the Terminal sort. The dimensions of the original window are transferred to this new abode, and its placement on the screen is determined by the user's placement of the tab._ _This is the last main chapter of the tear-out saga, and the dawning of the new age._ Closes #5000 Related to #1256 ## Detailed description We're really leaning on the existing `RequestNewWindow` event that the monarch already had - honestly, most of that was so simple that it could have just been in the parent PRs. We just need to add new support for passing in a content blob of json, and making sure the Terminal always uses that over commandline args. Easy enough. There's a bit of wackiness here in adjusting the positioning just right so that the new window appears in the right place, but it feels... pretty good all things considered. --- src/cascadia/Remoting/Monarch.cpp | 24 +-- src/cascadia/Remoting/Monarch.h | 12 +- src/cascadia/Remoting/Monarch.idl | 5 +- src/cascadia/Remoting/WindowManager.cpp | 5 +- src/cascadia/Remoting/WindowManager.h | 2 +- src/cascadia/Remoting/WindowManager.idl | 2 +- src/cascadia/TerminalApp/TabManagement.cpp | 4 +- src/cascadia/TerminalApp/TerminalPage.cpp | 108 ++++++++++--- src/cascadia/TerminalApp/TerminalPage.h | 18 ++- src/cascadia/TerminalApp/TerminalPage.idl | 2 +- src/cascadia/TerminalApp/TerminalWindow.cpp | 142 +++++++++++++++--- src/cascadia/TerminalApp/TerminalWindow.h | 7 + src/cascadia/TerminalApp/TerminalWindow.idl | 2 +- src/cascadia/TerminalControl/TermControl.cpp | 18 --- .../UnitTests_Remoting/RemotingTests.cpp | 2 +- src/cascadia/WindowsTerminal/AppHost.cpp | 88 +++++++++-- src/cascadia/WindowsTerminal/AppHost.h | 4 +- .../WindowsTerminal/WindowEmperor.cpp | 43 ++++-- src/cascadia/WindowsTerminal/WindowEmperor.h | 3 +- 19 files changed, 380 insertions(+), 111 deletions(-) diff --git a/src/cascadia/Remoting/Monarch.cpp b/src/cascadia/Remoting/Monarch.cpp index a62493ecb4..3466e84d00 100644 --- a/src/cascadia/Remoting/Monarch.cpp +++ b/src/cascadia/Remoting/Monarch.cpp @@ -1058,7 +1058,8 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation void Monarch::RequestMoveContent(winrt::hstring window, winrt::hstring content, - uint32_t tabIndex) + uint32_t tabIndex, + const Windows::Foundation::IReference& windowBounds) { TraceLoggingWrite(g_hRemotingProvider, "Monarch_MoveContent_Requested", @@ -1102,11 +1103,14 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), TraceLoggingKeyword(TIL_KEYWORD_TRACE)); - // TODO GH#5000 - // // In the case where window couldn't be found, then create a window - // for that name / ID. Do this as a part of tear-out (different than - // drag/drop) + // for that name / ID. + // + // Don't let the window literally be named "-1", because that's silly + auto request = winrt::make_self(window == L"-1" ? L"" : window, + content, + windowBounds); + _RequestNewWindowHandlers(*this, *request); } } @@ -1132,16 +1136,14 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation } else { + // We couldn't find the peasant that started the drag. Well that + // sure is weird, but that would indicate that the sender closed + // after starting the drag. No matter. We can just do nothing. + TraceLoggingWrite(g_hRemotingProvider, "Monarch_SendContent_NoWindow", TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), TraceLoggingKeyword(TIL_KEYWORD_TRACE)); - - // TODO GH#5000 - // - // In the case where window couldn't be found, then create a window - // for that name / ID. Do this as a part of tear-out (different than - // drag/drop) } } } diff --git a/src/cascadia/Remoting/Monarch.h b/src/cascadia/Remoting/Monarch.h index 54c31e98e2..ef311fad4e 100644 --- a/src/cascadia/Remoting/Monarch.h +++ b/src/cascadia/Remoting/Monarch.h @@ -48,12 +48,22 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation _args{ command.Commandline() }, _CurrentDirectory{ command.CurrentDirectory() } {}; + WindowRequestedArgs(const winrt::hstring& window, const winrt::hstring& content, const Windows::Foundation::IReference& bounds) : + _Id{ 0u }, + _WindowName{ window }, + _args{}, + _CurrentDirectory{}, + _Content{ content }, + _InitialBounds{ bounds } {}; + void Commandline(const winrt::array_view& value) { _args = { value.begin(), value.end() }; }; winrt::com_array Commandline() { return winrt::com_array{ _args.begin(), _args.end() }; } WINRT_PROPERTY(uint64_t, Id); WINRT_PROPERTY(winrt::hstring, WindowName); WINRT_PROPERTY(winrt::hstring, CurrentDirectory); + WINRT_PROPERTY(winrt::hstring, Content); + WINRT_PROPERTY(Windows::Foundation::IReference, InitialBounds); private: winrt::com_array _args; @@ -81,7 +91,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation Windows::Foundation::Collections::IVectorView GetPeasantInfos(); Windows::Foundation::Collections::IVector GetAllWindowLayouts(); - void RequestMoveContent(winrt::hstring window, winrt::hstring content, uint32_t tabIndex); + void RequestMoveContent(winrt::hstring window, winrt::hstring content, uint32_t tabIndex, const Windows::Foundation::IReference& windowBounds); void RequestSendContent(const Remoting::RequestReceiveContentArgs& args); TYPED_EVENT(FindTargetWindowRequested, winrt::Windows::Foundation::IInspectable, winrt::Microsoft::Terminal::Remoting::FindTargetWindowArgs); diff --git a/src/cascadia/Remoting/Monarch.idl b/src/cascadia/Remoting/Monarch.idl index 44ebf7b0f1..bc60fbe2c7 100644 --- a/src/cascadia/Remoting/Monarch.idl +++ b/src/cascadia/Remoting/Monarch.idl @@ -26,6 +26,9 @@ namespace Microsoft.Terminal.Remoting String[] Commandline { get; }; String CurrentDirectory { get; }; + + String Content { get; }; + Windows.Foundation.IReference InitialBounds { get; }; } [default_interface] runtimeclass SummonWindowSelectionArgs { @@ -69,7 +72,7 @@ namespace Microsoft.Terminal.Remoting Windows.Foundation.Collections.IVectorView GetPeasantInfos { get; }; Windows.Foundation.Collections.IVector GetAllWindowLayouts(); - void RequestMoveContent(String window, String content, UInt32 tabIndex); + void RequestMoveContent(String window, String content, UInt32 tabIndex, Windows.Foundation.IReference bounds); void RequestSendContent(RequestReceiveContentArgs args); event Windows.Foundation.TypedEventHandler FindTargetWindowRequested; diff --git a/src/cascadia/Remoting/WindowManager.cpp b/src/cascadia/Remoting/WindowManager.cpp index afa8403790..6331e3e9eb 100644 --- a/src/cascadia/Remoting/WindowManager.cpp +++ b/src/cascadia/Remoting/WindowManager.cpp @@ -416,10 +416,11 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation winrt::fire_and_forget WindowManager::RequestMoveContent(winrt::hstring window, winrt::hstring content, - uint32_t tabIndex) + uint32_t tabIndex, + Windows::Foundation::IReference windowBounds) { co_await winrt::resume_background(); - _monarch.RequestMoveContent(window, content, tabIndex); + _monarch.RequestMoveContent(window, content, tabIndex, windowBounds); } winrt::fire_and_forget WindowManager::RequestSendContent(Remoting::RequestReceiveContentArgs args) diff --git a/src/cascadia/Remoting/WindowManager.h b/src/cascadia/Remoting/WindowManager.h index e42d6b4ba2..015ab9f347 100644 --- a/src/cascadia/Remoting/WindowManager.h +++ b/src/cascadia/Remoting/WindowManager.h @@ -44,7 +44,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation Windows::Foundation::Collections::IVector GetAllWindowLayouts(); bool DoesQuakeWindowExist(); - winrt::fire_and_forget RequestMoveContent(winrt::hstring window, winrt::hstring content, uint32_t tabIndex); + winrt::fire_and_forget RequestMoveContent(winrt::hstring window, winrt::hstring content, uint32_t tabIndex, Windows::Foundation::IReference windowBounds); winrt::fire_and_forget RequestSendContent(Remoting::RequestReceiveContentArgs args); TYPED_EVENT(FindTargetWindowRequested, winrt::Windows::Foundation::IInspectable, winrt::Microsoft::Terminal::Remoting::FindTargetWindowArgs); diff --git a/src/cascadia/Remoting/WindowManager.idl b/src/cascadia/Remoting/WindowManager.idl index ca9d78a5d1..5dd72b7f19 100644 --- a/src/cascadia/Remoting/WindowManager.idl +++ b/src/cascadia/Remoting/WindowManager.idl @@ -26,7 +26,7 @@ namespace Microsoft.Terminal.Remoting Boolean DoesQuakeWindowExist(); - void RequestMoveContent(String window, String content, UInt32 tabIndex); + void RequestMoveContent(String window, String content, UInt32 tabIndex, Windows.Foundation.IReference bounds); void RequestSendContent(RequestReceiveContentArgs args); event Windows.Foundation.TypedEventHandler FindTargetWindowRequested; diff --git a/src/cascadia/TerminalApp/TabManagement.cpp b/src/cascadia/TerminalApp/TabManagement.cpp index 560a531650..457cf34c1c 100644 --- a/src/cascadia/TerminalApp/TabManagement.cpp +++ b/src/cascadia/TerminalApp/TabManagement.cpp @@ -512,9 +512,9 @@ namespace winrt::TerminalApp::implementation _mruTabs.RemoveAt(mruIndex); } - if (_stashedDraggedTab && *_stashedDraggedTab == tab) + if (_stashed.draggedTab && *_stashed.draggedTab == tab) { - _stashedDraggedTab = nullptr; + _stashed.draggedTab = nullptr; } _tabs.RemoveAt(tabIndex); diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index efe73161ec..9b0abe8904 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -250,6 +250,7 @@ namespace winrt::TerminalApp::implementation _tabView.TabDragStarting({ this, &TerminalPage::_onTabDragStarting }); _tabView.TabStripDragOver({ this, &TerminalPage::_onTabStripDragOver }); _tabView.TabStripDrop({ this, &TerminalPage::_onTabStripDrop }); + _tabView.TabDroppedOutside({ this, &TerminalPage::_onTabDroppedOutside }); _CreateNewTabFlyout(); @@ -1985,13 +1986,18 @@ namespace winrt::TerminalApp::implementation // and should be expected to be empty after this call. void TerminalPage::_MoveContent(std::vector&& actions, const winrt::hstring& windowName, - const uint32_t tabIndex) + const uint32_t tabIndex, + const std::optional& dragPoint) { const auto winRtActions{ winrt::single_threaded_vector(std::move(actions)) }; const auto str{ ActionAndArgs::Serialize(winRtActions) }; const auto request = winrt::make_self(windowName, str, tabIndex); + if (dragPoint.has_value()) + { + request->WindowPosition(dragPoint->to_winrt_point()); + } _RequestMoveContentHandlers(*this, *request); } @@ -2026,6 +2032,11 @@ namespace winrt::TerminalApp::implementation return true; } + uint32_t TerminalPage::NumberOfTabs() const + { + return _tabs.Size(); + } + // Method Description: // - Called when it is determined that an existing tab or pane should be // attached to our window. content represents a blob of JSON describing @@ -2036,11 +2047,9 @@ namespace winrt::TerminalApp::implementation // reattach instead of create new content, so this method simply needs to // parse the JSON and pump it into our action handler. Almost the same as // doing something like `wt -w 0 nt`. - winrt::fire_and_forget TerminalPage::AttachContent(winrt::hstring content, + winrt::fire_and_forget TerminalPage::AttachContent(IVector args, uint32_t tabIndex) { - auto args = ActionAndArgs::Deserialize(content); - if (args == nullptr || args.Size() == 0) { @@ -2064,18 +2073,6 @@ namespace winrt::TerminalApp::implementation { _SelectTab(tabIndex); } - else - { - if (firstIsSplitPane) - { - // Create the equivalent NewTab action. - const auto newAction = Settings::Model::ActionAndArgs{ Settings::Model::ShortcutAction::NewTab, - Settings::Model::NewTabArgs(firstAction.Args() ? - firstAction.Args().try_as().TerminalArgs() : - nullptr) }; - args.SetAt(0, newAction); - } - } for (const auto& action : args) { @@ -4611,7 +4608,23 @@ namespace winrt::TerminalApp::implementation { // First: stash the tab we started dragging. // We're going to be asked for this. - _stashedDraggedTab = tabImpl; + _stashed.draggedTab = tabImpl; + + // Stash the offset from where we started the drag to the + // tab's origin. We'll use that offset in the future to help + // position the dropped window. + + // First, the position of the pointer, from the CoreWindow + const til::point pointerPosition{ til::math::rounding, CoreWindow::GetForCurrentThread().PointerPosition() }; + // Next, the position of the tab itself: + const til::point tabPosition{ til::math::rounding, eventTab.TransformToVisual(nullptr).TransformPoint({ 0, 0 }) }; + // Now, we need to add the origin of our CoreWindow to the tab + // position. + const auto& coreWindowBounds{ CoreWindow::GetForCurrentThread().Bounds() }; + const til::point windowOrigin{ til::math::rounding, coreWindowBounds.X, coreWindowBounds.Y }; + const auto realTabPosition = windowOrigin + tabPosition; + // Subtract the two to get the offset. + _stashed.dragOffset = til::point{ pointerPosition - realTabPosition }; // Into the DataPackage, let's stash our own window ID. const auto id{ _WindowProperties.WindowId() }; @@ -4630,7 +4643,8 @@ namespace winrt::TerminalApp::implementation // to ask us to send our content to them. // * We'll get a TabDroppedOutside to indicate that this tab was // dropped _not_ on a TabView. - // * This we can't handle yet, and is the last point of TODO GH#5000 + // * This will be handled by _onTabDroppedOutside, which will + // raise a MoveContent (to a new window) event. } } @@ -4736,7 +4750,7 @@ namespace winrt::TerminalApp::implementation { co_return; } - if (!_stashedDraggedTab) + if (!_stashed.draggedTab) { co_return; } @@ -4747,12 +4761,58 @@ namespace winrt::TerminalApp::implementation if (const auto& page{ weakThis.get() }) { // `this` is safe to use in here. - auto startupActions = _stashedDraggedTab->BuildStartupActions(true); - _DetachTabFromWindow(_stashedDraggedTab); - _MoveContent(std::move(startupActions), winrt::hstring{ fmt::format(L"{}", args.TargetWindow()) }, args.TabIndex()); - // _RemoveTab will make sure to null out the _stashedDraggedTab - _RemoveTab(*_stashedDraggedTab); + + _sendDraggedTabToWindow(winrt::hstring{ fmt::format(L"{}", args.TargetWindow()) }, + args.TabIndex(), + std::nullopt); } } + winrt::fire_and_forget TerminalPage::_onTabDroppedOutside(winrt::IInspectable sender, + winrt::MUX::Controls::TabViewTabDroppedOutsideEventArgs e) + { + // Get the current pointer point from the CoreWindow + const auto& pointerPoint{ CoreWindow::GetForCurrentThread().PointerPosition() }; + + // This is called when a tab FROM OUR WINDOW was dropped outside the + // tabview. We already know which tab was being dragged. We'll just + // invoke a moveTab action with the target window being -1. That will + // force the creation of a new window. + + if (!_stashed.draggedTab) + { + co_return; + } + + // must do the work of adding/removing tabs on the UI thread. + auto weakThis{ get_weak() }; + co_await wil::resume_foreground(Dispatcher(), CoreDispatcherPriority::Normal); + if (const auto& page{ weakThis.get() }) + { + // `this` is safe to use in here. + + // We need to convert the pointer point to a point that we can use + // to position the new window. We'll use the drag offset from before + // so that the tab in the new window is positioned so that it's + // basically still directly under the cursor. + + // -1 is the magic number for "new window" + // 0 as the tab index, because we don't care. It's making a new window. It'll be the only tab. + const til::point adjusted = til::point{ til::math::rounding, pointerPoint } - _stashed.dragOffset; + _sendDraggedTabToWindow(winrt::hstring{ L"-1" }, 0, adjusted); + } + } + + void TerminalPage::_sendDraggedTabToWindow(const winrt::hstring& windowId, + const uint32_t tabIndex, + std::optional dragPoint) + { + auto startupActions = _stashed.draggedTab->BuildStartupActions(true); + _DetachTabFromWindow(_stashed.draggedTab); + + _MoveContent(std::move(startupActions), windowId, tabIndex, dragPoint); + // _RemoveTab will make sure to null out the _stashed.draggedTab + _RemoveTab(*_stashed.draggedTab); + } + } diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index 46cf9c11c8..efc68f5f6b 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -67,6 +67,7 @@ namespace winrt::TerminalApp::implementation WINRT_PROPERTY(winrt::hstring, Window); WINRT_PROPERTY(winrt::hstring, Content); WINRT_PROPERTY(uint32_t, TabIndex); + WINRT_PROPERTY(Windows::Foundation::IReference, WindowPosition); public: RequestMoveContentArgs(const winrt::hstring window, const winrt::hstring content, uint32_t tabIndex) : @@ -162,9 +163,11 @@ namespace winrt::TerminalApp::implementation bool OnDirectKeyEvent(const uint32_t vkey, const uint8_t scanCode, const bool down); - winrt::fire_and_forget AttachContent(winrt::hstring content, uint32_t tabIndex); + winrt::fire_and_forget AttachContent(Windows::Foundation::Collections::IVector args, uint32_t tabIndex); winrt::fire_and_forget SendContentToOther(winrt::TerminalApp::RequestReceiveContentArgs args); + uint32_t NumberOfTabs() const; + WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler); // -------------------------------- WinRT Events --------------------------------- @@ -263,7 +266,11 @@ namespace winrt::TerminalApp::implementation TerminalApp::ContentManager _manager{ nullptr }; - winrt::com_ptr _stashedDraggedTab{ nullptr }; + struct StashedDragData + { + winrt::com_ptr draggedTab{ nullptr }; + til::point dragOffset{ 0, 0 }; + } _stashed; winrt::Microsoft::Terminal::TerminalConnection::ConptyConnection::NewConnection_revoker _newConnectionRevoker; @@ -503,10 +510,15 @@ namespace winrt::TerminalApp::implementation void _onTabDragStarting(const winrt::Microsoft::UI::Xaml::Controls::TabView& sender, const winrt::Microsoft::UI::Xaml::Controls::TabViewTabDragStartingEventArgs& e); void _onTabStripDragOver(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::DragEventArgs& e); winrt::fire_and_forget _onTabStripDrop(winrt::Windows::Foundation::IInspectable sender, winrt::Windows::UI::Xaml::DragEventArgs e); + winrt::fire_and_forget _onTabDroppedOutside(winrt::Windows::Foundation::IInspectable sender, winrt::Microsoft::UI::Xaml::Controls::TabViewTabDroppedOutsideEventArgs e); void _DetachPaneFromWindow(std::shared_ptr pane); void _DetachTabFromWindow(const winrt::com_ptr& terminalTab); - void _MoveContent(std::vector&& actions, const winrt::hstring& windowName, const uint32_t tabIndex); + void _MoveContent(std::vector&& actions, + const winrt::hstring& windowName, + const uint32_t tabIndex, + const std::optional& dragPoint = std::nullopt); + void _sendDraggedTabToWindow(const winrt::hstring& windowId, const uint32_t tabIndex, std::optional dragPoint); void _ContextMenuOpened(const IInspectable& sender, const IInspectable& args); void _SelectionMenuOpened(const IInspectable& sender, const IInspectable& args); diff --git a/src/cascadia/TerminalApp/TerminalPage.idl b/src/cascadia/TerminalApp/TerminalPage.idl index a23203046d..5c8a151b90 100644 --- a/src/cascadia/TerminalApp/TerminalPage.idl +++ b/src/cascadia/TerminalApp/TerminalPage.idl @@ -29,6 +29,7 @@ namespace TerminalApp String Window { get; }; String Content { get; }; UInt32 TabIndex { get; }; + Windows.Foundation.IReference WindowPosition { get; }; }; [default_interface] runtimeclass RequestReceiveContentArgs { RequestReceiveContentArgs(UInt64 src, UInt64 tgt, UInt32 tabIndex); @@ -77,7 +78,6 @@ namespace TerminalApp String KeyboardServiceDisabledText { get; }; TaskbarState TaskbarState{ get; }; - void AttachContent(String content, UInt32 tabIndex); Windows.UI.Xaml.Media.Brush TitlebarBrush { get; }; void WindowActivated(Boolean activated); diff --git a/src/cascadia/TerminalApp/TerminalWindow.cpp b/src/cascadia/TerminalApp/TerminalWindow.cpp index 3db175daec..dd984c6c12 100644 --- a/src/cascadia/TerminalApp/TerminalWindow.cpp +++ b/src/cascadia/TerminalApp/TerminalWindow.cpp @@ -23,6 +23,7 @@ using namespace winrt::Windows::System; using namespace winrt::Microsoft::Terminal; using namespace winrt::Microsoft::Terminal::Control; using namespace winrt::Microsoft::Terminal::Settings::Model; +using namespace winrt::Windows::Foundation::Collections; using namespace ::TerminalApp; namespace winrt @@ -144,23 +145,33 @@ namespace winrt::TerminalApp::implementation _root = winrt::make_self(*_WindowProperties, _manager); _dialog = ContentDialog{}; - // Pass commandline args into the TerminalPage. If we were supposed to - // load from a persisted layout, do that instead. - - // layout will only ever be non-null if there were >0 tabs persisted in - // .TabLayout(). We can re-evaluate that as a part of TODO: GH#12633 - if (const auto& layout = LoadPersistedLayout()) + // Pass in information about the initial state of the window. + // * If we were supposed to start from serialized "content", do that, + // * If we were supposed to load from a persisted layout, do that + // instead. + // * if we have commandline arguments, Pass commandline args into the + // TerminalPage. + if (!_initialContentArgs.empty()) { - std::vector actions; - for (const auto& a : layout.TabLayout()) - { - actions.emplace_back(a); - } - _root->SetStartupActions(actions); + _root->SetStartupActions(_initialContentArgs); } else { - _root->SetStartupActions(_appArgs.GetStartupActions()); + // layout will only ever be non-null if there were >0 tabs persisted in + // .TabLayout(). We can re-evaluate that as a part of TODO: GH#12633 + if (const auto& layout = LoadPersistedLayout()) + { + std::vector actions; + for (const auto& a : layout.TabLayout()) + { + actions.emplace_back(a); + } + _root->SetStartupActions(actions); + } + else + { + _root->SetStartupActions(_appArgs.GetStartupActions()); + } } // Check if we were started as a COM server for inbound connections of console sessions @@ -176,6 +187,7 @@ namespace winrt::TerminalApp::implementation return _root->Initialize(hwnd); } + // Method Description: // - Called around the codebase to discover if this is a UWP where we need to turn off specific settings. // Arguments: @@ -598,6 +610,17 @@ namespace winrt::TerminalApp::implementation commandlineSize.height); } + if (_contentBounds) + { + // If we've been created as a torn-out window, then we'll need to + // use that size instead. _contentBounds is in raw pixels. Huzzah! + // Just return that. + return { + _contentBounds.Value().Width, + _contentBounds.Value().Height + }; + } + // GH#2061 - If the global setting "Always show tab bar" is // set or if "Show tabs in title bar" is set, then we'll need to add // the height of the tab bar here. @@ -645,6 +668,11 @@ namespace winrt::TerminalApp::implementation // - LaunchMode enum that indicates the launch mode LaunchMode TerminalWindow::GetLaunchMode() { + if (_contentBounds) + { + return LaunchMode::DefaultMode; + } + // GH#4620/#5801 - If the user passed --maximized or --fullscreen on the // commandline, then use that to override the value from the settings. const auto valueFromSettings = _settings.GlobalSettings().LaunchMode(); @@ -683,12 +711,24 @@ namespace winrt::TerminalApp::implementation } } - // Commandline args trump everything else + // Commandline args trump everything except for content bounds (tear-out) if (_appArgs.GetPosition().has_value()) { initialPosition = _appArgs.GetPosition().value(); } + if (_contentBounds) + { + // If the user has specified a contentBounds, then we should use + // that to determine the initial position of the window. This is + // used when the user is dragging a tab out of the window, to create + // a new window. + // + // contentBounds is in screen pixels, but that's okay! we want to + // return screen pixels out of here. Nailed it. + const til::rect bounds = { til::math::rounding, _contentBounds.Value() }; + initialPosition = { bounds.left, bounds.top }; + } return { initialPosition.X ? initialPosition.X.Value() : defaultInitialX, initialPosition.Y ? initialPosition.Y.Value() : defaultInitialY @@ -697,8 +737,11 @@ namespace winrt::TerminalApp::implementation bool TerminalWindow::CenterOnLaunch() { - // If the position has been specified on the commandline, don't center on launch - return _settings.GlobalSettings().CenterOnLaunch() && !_appArgs.GetPosition().has_value(); + // If + // * the position has been specified on the commandline, + // * We're opening the window as a part of tear out (and _contentBounds were set) + // then don't center on launch + return !_contentBounds && _settings.GlobalSettings().CenterOnLaunch() && !_appArgs.GetPosition().has_value(); } // Method Description: @@ -1008,6 +1051,19 @@ namespace winrt::TerminalApp::implementation return result; } + void TerminalWindow::SetStartupContent(const winrt::hstring& content, + const Windows::Foundation::IReference& bounds) + { + _contentBounds = bounds; + + const auto& args = _contentStringToActions(content, true); + + for (const auto& action : args) + { + _initialContentArgs.push_back(action); + } + } + // Method Description: // - Parse the provided commandline arguments into actions, and try to // perform them immediately. @@ -1175,11 +1231,63 @@ namespace winrt::TerminalApp::implementation _WindowProperties->WindowId(id); } + // Method Description: + // - Deserialize this string of content into a list of actions to perform. + // If replaceFirstWithNewTab is true and the first serialized action is a + // `splitPane` action, we'll attempt to replace that action with the + // equivalent `newTab` action. + IVector TerminalWindow::_contentStringToActions(const winrt::hstring& content, + const bool replaceFirstWithNewTab) + { + try + { + const auto& args = ActionAndArgs::Deserialize(content); + if (args == nullptr || + args.Size() == 0) + { + return args; + } + + const auto& firstAction = args.GetAt(0); + const bool firstIsSplitPane{ firstAction.Action() == ShortcutAction::SplitPane }; + if (replaceFirstWithNewTab && + firstIsSplitPane) + { + // Create the equivalent NewTab action. + const auto newAction = Settings::Model::ActionAndArgs{ Settings::Model::ShortcutAction::NewTab, + Settings::Model::NewTabArgs(firstAction.Args() ? + firstAction.Args().try_as().TerminalArgs() : + nullptr) }; + args.SetAt(0, newAction); + } + + return args; + } + catch (...) + { + LOG_CAUGHT_EXCEPTION(); + } + + return nullptr; + } + void TerminalWindow::AttachContent(winrt::hstring content, uint32_t tabIndex) { if (_root) { - _root->AttachContent(content, tabIndex); + // `splitPane` allows the user to specify which tab to split. In that + // case, split specifically the requested pane. + // + // If there's not enough tabs, then just turn this pane into a new tab. + // + // If the first action is `newTab`, the index is always going to be 0, + // so don't do anything in that case. + + const bool replaceFirstWithNewTab = tabIndex >= _root->NumberOfTabs(); + + const auto& args = _contentStringToActions(content, replaceFirstWithNewTab); + + _root->AttachContent(args, tabIndex); } } void TerminalWindow::SendContentToOther(winrt::TerminalApp::RequestReceiveContentArgs args) diff --git a/src/cascadia/TerminalApp/TerminalWindow.h b/src/cascadia/TerminalApp/TerminalWindow.h index eda5c9e84a..adb2c9b663 100644 --- a/src/cascadia/TerminalApp/TerminalWindow.h +++ b/src/cascadia/TerminalApp/TerminalWindow.h @@ -74,6 +74,7 @@ namespace winrt::TerminalApp::implementation bool HasCommandlineArguments() const noexcept; int32_t SetStartupCommandline(array_view actions); + void SetStartupContent(const winrt::hstring& content, const Windows::Foundation::IReference& contentBounds); int32_t ExecuteCommandline(array_view actions, const winrt::hstring& cwd); void SetSettingsStartupArgs(const std::vector& actions); winrt::hstring ParseCommandlineMessage(); @@ -171,6 +172,7 @@ namespace winrt::TerminalApp::implementation ::TerminalApp::AppCommandlineArgs _appArgs; bool _gotSettingsStartupActions{ false }; std::vector _settingsStartupArgs{}; + Windows::Foundation::IReference _contentBounds{ nullptr }; winrt::com_ptr _WindowProperties{ nullptr }; @@ -181,6 +183,7 @@ namespace winrt::TerminalApp::implementation TerminalApp::SettingsLoadEventArgs _initialLoadResult{ nullptr }; TerminalApp::ContentManager _manager{ nullptr }; + std::vector _initialContentArgs; void _ShowLoadErrorsDialog(const winrt::hstring& titleKey, const winrt::hstring& contentKey, @@ -193,6 +196,10 @@ namespace winrt::TerminalApp::implementation void _RefreshThemeRoutine(); void _OnLoaded(const IInspectable& sender, const Windows::UI::Xaml::RoutedEventArgs& eventArgs); void _OpenSettingsUI(); + + winrt::Windows::Foundation::Collections::IVector _contentStringToActions(const winrt::hstring& content, + const bool replaceFirstWithNewTab); + // These are events that are handled by the TerminalPage, but are // exposed through the AppLogic. This macro is used to forward the event // directly to them. diff --git a/src/cascadia/TerminalApp/TerminalWindow.idl b/src/cascadia/TerminalApp/TerminalWindow.idl index 770289895e..825afa44c4 100644 --- a/src/cascadia/TerminalApp/TerminalWindow.idl +++ b/src/cascadia/TerminalApp/TerminalWindow.idl @@ -54,7 +54,7 @@ namespace TerminalApp Boolean HasCommandlineArguments(); Int32 SetStartupCommandline(String[] commands); - + void SetStartupContent(String json, Windows.Foundation.IReference bounds); Int32 ExecuteCommandline(String[] commands, String cwd); String ParseCommandlineMessage { get; }; Boolean ShouldExitEarly { get; }; diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index fcca688c2c..028971bd21 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -1258,12 +1258,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation return true; } - // TODO: GH#5000 - // The Core owning the keybindings is weird. That's for sure. In the - // future, we may want to pass the keybindings into the control - // separately, so the control can have a pointer to an in-proc - // Keybindings object, rather than routing through the ControlCore. - // (see GH#5000) auto bindings = _core.Settings().KeyBindings(); if (!bindings) { @@ -2877,18 +2871,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation IControlSettings TermControl::Settings() const { - // TODO: GH#5000 - // We still need this in a couple places: - // - Pane.cpp uses this for parsing out the StartingTitle, Commandline, - // etc for Pane::GetTerminalArgsForPane. - // - TerminalTab::_CreateToolTipTitle uses the ProfileName for the - // tooltip for the tab. - // - // These both happen on the UI thread right now. In the future, when we - // have to hop across the process boundary to get at the core settings, - // it may make sense to cache these values inside the TermControl - // itself, so it can do the hop once when it's first setup, rather than - // when it's needed by the UI thread. return _core.Settings(); } diff --git a/src/cascadia/UnitTests_Remoting/RemotingTests.cpp b/src/cascadia/UnitTests_Remoting/RemotingTests.cpp index 058a6c0793..0599aa980a 100644 --- a/src/cascadia/UnitTests_Remoting/RemotingTests.cpp +++ b/src/cascadia/UnitTests_Remoting/RemotingTests.cpp @@ -118,7 +118,7 @@ namespace RemotingUnitTests bool DoesQuakeWindowExist() DIE; winrt::Windows::Foundation::Collections::IVectorView GetPeasantInfos() DIE; winrt::Windows::Foundation::Collections::IVector GetAllWindowLayouts() DIE; - void RequestMoveContent(winrt::hstring, winrt::hstring, uint32_t) DIE; + void RequestMoveContent(winrt::hstring, winrt::hstring, uint32_t, winrt::Windows::Foundation::IReference) DIE; void RequestSendContent(Remoting::RequestReceiveContentArgs) DIE; TYPED_EVENT(FindTargetWindowRequested, winrt::Windows::Foundation::IInspectable, Remoting::FindTargetWindowArgs); diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp index b4958ece43..3776017f25 100644 --- a/src/cascadia/WindowsTerminal/AppHost.cpp +++ b/src/cascadia/WindowsTerminal/AppHost.cpp @@ -38,7 +38,7 @@ AppHost::AppHost(const winrt::TerminalApp::AppLogic& logic, _windowLogic{ nullptr }, _window{ nullptr } { - _HandleCommandlineArgs(); + _HandleCommandlineArgs(args); // _HandleCommandlineArgs will create a _windowLogic _useNonClientArea = _windowLogic.GetShowTabsInTitlebar(); @@ -157,8 +157,6 @@ void AppHost::s_DisplayMessageBox(const winrt::TerminalApp::ParseCommandlineResu // the WindowManager, to ask if we should become a window process. // - If we should create a window, then pass the arguments to the app logic for // processing. -// - If we shouldn't become a window, set _shouldCreateWindow to false and exit -// immediately. // - If the logic determined there's an error while processing that commandline, // display a message box to the user with the text of the error, and exit. // * We display a message box because we're a Win32 application (not a @@ -169,7 +167,7 @@ void AppHost::s_DisplayMessageBox(const winrt::TerminalApp::ParseCommandlineResu // - // Return Value: // - -void AppHost::_HandleCommandlineArgs() +void AppHost::_HandleCommandlineArgs(const Remoting::WindowRequestedArgs& windowArgs) { // We did want to make a window, so let's instantiate it here. // We don't have XAML yet, but we do have other stuff. @@ -178,7 +176,12 @@ void AppHost::_HandleCommandlineArgs() if (_peasant) { const auto& args{ _peasant.InitialArgs() }; - if (args) + + if (!windowArgs.Content().empty()) + { + _windowLogic.SetStartupContent(windowArgs.Content(), windowArgs.InitialBounds()); + } + else if (args) { const auto result = _windowLogic.SetStartupCommandline(args.Commandline()); const auto message = _windowLogic.ParseCommandlineMessage(); @@ -202,7 +205,6 @@ void AppHost::_HandleCommandlineArgs() // then we're going to exit immediately. if (_windowLogic.ShouldImmediatelyHandoffToElevated()) { - _shouldCreateWindow = false; _windowLogic.HandoffToElevated(); return; } @@ -796,11 +798,6 @@ void AppHost::_WindowMouseWheeled(const til::point coord, const int32_t delta) } } -bool AppHost::HasWindow() -{ - return _shouldCreateWindow; -} - // Method Description: // - Event handler for the Peasant::ExecuteCommandlineRequested event. Take the // provided commandline args, and attempt to parse them and perform the @@ -1221,10 +1218,77 @@ winrt::TerminalApp::TerminalWindow AppHost::Logic() return _windowLogic; } +// Method Description: +// - Raised from Page -> us -> manager -> monarch +// - Called when the user attempts to move a tab or pane to another window. +// `args` will contain info about the structure of the content being moved, +// and where it should go. +// - If the WindowPosition is filled in, then the user was dragging a tab out of +// this window and dropping it in empty space, indicating it should create a +// new window. In that case, we'll make some adjustments using that info and +// our own window info, so that the new window will be created in the right +// place and with the same size. void AppHost::_handleMoveContent(const winrt::Windows::Foundation::IInspectable& /*sender*/, winrt::TerminalApp::RequestMoveContentArgs args) { - _windowManager.RequestMoveContent(args.Window(), args.Content(), args.TabIndex()); + winrt::Windows::Foundation::Rect rect{}; + winrt::Windows::Foundation::IReference windowBoundsReference{ nullptr }; + + if (args.WindowPosition() && _window) + { + // The WindowPosition is in DIPs. We need to convert it to pixels. + const til::point dragPositionInDips{ til::math::rounding, args.WindowPosition().Value() }; + const auto scale = _window->GetCurrentDpiScale(); + + til::point dragPositionInPixels{ + til::math::rounding, + dragPositionInDips.x * scale, + dragPositionInDips.y * scale, + }; + + // Fortunately, the window position is already in pixels. + til::rect windowBoundsInPixels{ _window->GetWindowRect() }; + til::size windowSize{ windowBoundsInPixels.size() }; + + const auto dpi = _window->GetCurrentDpi(); + const auto nonClientFrame = _window->GetNonClientFrame(dpi); + + // If this window is maximized, you don't _really_ want the new window + // showing up at the same size (the size of a maximized window). You + // want it to just make a normal-sized window. This logic was taken out + // of AppHost::_HandleCreateWindow. + if (IsZoomed(_window->GetHandle())) + { + const auto initialSize = _windowLogic.GetLaunchDimensions(dpi); + + const auto islandWidth = Utils::ClampToShortMax(static_cast(ceil(initialSize.Width)), 1); + const auto islandHeight = Utils::ClampToShortMax(static_cast(ceil(initialSize.Height)), 1); + + // Get the size of a window we'd need to host that client rect. This will + // add the titlebar space. + const til::size nonClientSize{ _window->GetTotalNonClientExclusiveSize(dpi) }; + + const auto adjustedWidth = islandWidth + nonClientSize.width; + const auto adjustedHeight = islandHeight + nonClientSize.height; + + windowSize = til::size{ Utils::ClampToShortMax(adjustedWidth, 1), + Utils::ClampToShortMax(adjustedHeight, 1) }; + } + + // Adjust for the non-client bounds + dragPositionInPixels.x -= nonClientFrame.left; + dragPositionInPixels.y -= nonClientFrame.top; + windowSize = windowSize - nonClientFrame.size(); + + // Use the drag event as the new position, and the size of the actual window. + rect = winrt::Windows::Foundation::Rect{ static_cast(dragPositionInPixels.x), + static_cast(dragPositionInPixels.y), + static_cast(windowSize.width), + static_cast(windowSize.height) }; + windowBoundsReference = rect; + } + + _windowManager.RequestMoveContent(args.Window(), args.Content(), args.TabIndex(), windowBoundsReference); } void AppHost::_handleAttach(const winrt::Windows::Foundation::IInspectable& /*sender*/, diff --git a/src/cascadia/WindowsTerminal/AppHost.h b/src/cascadia/WindowsTerminal/AppHost.h index 3521d984b6..dc2c3da91c 100644 --- a/src/cascadia/WindowsTerminal/AppHost.h +++ b/src/cascadia/WindowsTerminal/AppHost.h @@ -21,7 +21,6 @@ public: bool OnDirectKeyEvent(const uint32_t vkey, const uint8_t scanCode, const bool down); void SetTaskbarProgress(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::Foundation::IInspectable& args); - bool HasWindow(); winrt::TerminalApp::TerminalWindow Logic(); static void s_DisplayMessageBox(const winrt::TerminalApp::ParseCommandlineResult& message); @@ -39,14 +38,13 @@ private: winrt::com_ptr _desktopManager{ nullptr }; - bool _shouldCreateWindow{ false }; bool _useNonClientArea{ false }; std::shared_ptr> _showHideWindowThrottler; void _preInit(); - void _HandleCommandlineArgs(); + void _HandleCommandlineArgs(const winrt::Microsoft::Terminal::Remoting::WindowRequestedArgs& args); winrt::Microsoft::Terminal::Settings::Model::LaunchPosition _GetWindowLaunchPosition(); void _HandleCreateWindow(const HWND hwnd, til::rect proposedRect, winrt::Microsoft::Terminal::Settings::Model::LaunchMode& launchMode); diff --git a/src/cascadia/WindowsTerminal/WindowEmperor.cpp b/src/cascadia/WindowsTerminal/WindowEmperor.cpp index 75fe386d6c..3343004aa0 100644 --- a/src/cascadia/WindowsTerminal/WindowEmperor.cpp +++ b/src/cascadia/WindowsTerminal/WindowEmperor.cpp @@ -114,20 +114,41 @@ void WindowEmperor::_createNewWindowThread(const Remoting::WindowRequestedArgs& auto window{ std::make_shared(_app.Logic(), args, _manager, peasant) }; std::weak_ptr weakThis{ weak_from_this() }; + // Increment our count of window instances _now_, immediately. We're + // starting a window now, we shouldn't exit (due to having 0 windows) till + // this window has a chance to actually start. + // * We can't just put the window immediately into _windows right now, + // because there are multiple async places where we iterate over all + // _windows assuming that they have initialized and we can call methods + // that might hit the TerminalPage. + // * If we don't somehow track this window now, before it has been actually + // started, there's a possible race. As an example, it would be possible + // to drag a tab out of the single window, which would create a new + // window, but have the original window exit before the new window has + // started, causing the app to exit. + // Hence: increment the number of total windows now. + _windowThreadInstances.fetch_add(1, std::memory_order_relaxed); + std::thread t([weakThis, window]() { - window->CreateHost(); - - if (auto self{ weakThis.lock() }) + try { - self->_windowStartedHandler(window); - } + const auto cleanup = wil::scope_exit([&]() { + if (auto self{ weakThis.lock() }) + { + self->_windowExitedHandler(window->Peasant().GetID()); + } + }); - window->RunMessagePump(); + window->CreateHost(); - if (auto self{ weakThis.lock() }) - { - self->_windowExitedHandler(window->Peasant().GetID()); + if (auto self{ weakThis.lock() }) + { + self->_windowStartedHandlerPostXAML(window); + } + + window->RunMessagePump(); } + CATCH_LOG() }); LOG_IF_FAILED(SetThreadDescription(t.native_handle(), L"Window Thread")); @@ -140,7 +161,7 @@ void WindowEmperor::_createNewWindowThread(const Remoting::WindowRequestedArgs& // Q: Why isn't adding these callbacks just a part of _createNewWindowThread? // A: Until the thread actually starts, the AppHost (and its Logic()) haven't // been ctor'd or initialized, so trying to add callbacks immediately will A/V -void WindowEmperor::_windowStartedHandler(const std::shared_ptr& sender) +void WindowEmperor::_windowStartedHandlerPostXAML(const std::shared_ptr& sender) { // Add a callback to the window's logic to let us know when the window's // quake mode state changes. We'll use this to check if we need to add @@ -181,7 +202,7 @@ void WindowEmperor::_windowExitedHandler(uint64_t senderID) return w->Peasant().GetID() == senderID; }); - if (lockedWindows->size() == 0) + if (_windowThreadInstances.fetch_sub(1, std::memory_order_relaxed) == 1) { _close(); } diff --git a/src/cascadia/WindowsTerminal/WindowEmperor.h b/src/cascadia/WindowsTerminal/WindowEmperor.h index 9ed30a41ce..73e3f1ebbe 100644 --- a/src/cascadia/WindowsTerminal/WindowEmperor.h +++ b/src/cascadia/WindowsTerminal/WindowEmperor.h @@ -41,6 +41,7 @@ private: winrt::Microsoft::Terminal::Remoting::WindowManager _manager; til::shared_mutex>> _windows; + std::atomic _windowThreadInstances; std::optional> _getWindowLayoutThrottler; @@ -51,7 +52,7 @@ private: std::unique_ptr _notificationIcon; - void _windowStartedHandler(const std::shared_ptr& sender); + void _windowStartedHandlerPostXAML(const std::shared_ptr& sender); void _windowExitedHandler(uint64_t senderID); void _becomeMonarch();