diff --git a/src/cascadia/Remoting/Monarch.cpp b/src/cascadia/Remoting/Monarch.cpp index a62493ecb4a..3466e84d009 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 54c31e98e27..ef311fad4e1 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 44ebf7b0f1c..bc60fbe2c71 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 afa8403790e..6331e3e9eb8 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 e42d6b4ba27..015ab9f3472 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 ca9d78a5d1a..5dd72b7f19a 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 560a531650b..457cf34c1ca 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 efe73161ec0..9b0abe8904a 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,36 @@ namespace winrt::TerminalApp::implementation { co_return; } - if (!_stashedDraggedTab) + 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. + + _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; } @@ -4747,12 +4790,29 @@ 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); + + // 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 46cf9c11c8c..efc68f5f6b2 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 a23203046dd..5c8a151b909 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 3db175daec8..dd984c6c126 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 eda5c9e84ab..adb2c9b6638 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 770289895e2..825afa44c4a 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 fcca688c2c4..028971bd210 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 058a6c0793c..0599aa980af 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 b4958ece432..3776017f25e 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 3521d984b6a..dc2c3da91cc 100644 --- a/src/cascadia/WindowsTerminal/AppHost.h +++ b/src/cascadia/WindowsTerminal/AppHost.h @@ -21,7 +21,6 @@ class AppHost 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 @@ class AppHost 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 75fe386d6c6..3343004aa0b 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() }; - std::thread t([weakThis, window]() { - window->CreateHost(); + // 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); - if (auto self{ weakThis.lock() }) + std::thread t([weakThis, window]() { + 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 9ed30a41cea..73e3f1ebbe1 100644 --- a/src/cascadia/WindowsTerminal/WindowEmperor.h +++ b/src/cascadia/WindowsTerminal/WindowEmperor.h @@ -41,6 +41,7 @@ class WindowEmperor : public std::enable_shared_from_this winrt::Microsoft::Terminal::Remoting::WindowManager _manager; til::shared_mutex>> _windows; + std::atomic _windowThreadInstances; std::optional> _getWindowLayoutThrottler; @@ -51,7 +52,7 @@ class WindowEmperor : public std::enable_shared_from_this 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();