From 3ac32af8484db806eaaf56019e1da4470b675286 Mon Sep 17 00:00:00 2001 From: Michael Kitzan Date: Thu, 9 Jan 2020 19:29:49 -0800 Subject: [PATCH] Converts Dispatcher().RunAsync to WinRT Coroutines (#4051) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary of the Pull Request This PR turns all* instances of `Dispatcher().RunAsync` to WinRT coroutines 👌. This was good coding fodder to fill my plane ride ✈️. Enjoy your holidays everyone! *With the exception of three functions whose signatures cannot be changed due to inheritance and function overriding in `TermControlAutomationPeer` [`L44`](https://github.com/microsoft/terminal/blob/master/src/cascadia/TerminalControl/TermControlAutomationPeer.cpp#L44), [`L58`](https://github.com/microsoft/terminal/blob/master/src/cascadia/TerminalControl/TermControlAutomationPeer.cpp#L58), [`L72`](https://github.com/microsoft/terminal/blob/master/src/cascadia/TerminalControl/TermControlAutomationPeer.cpp#L72). ## References ## PR Checklist * [x] Closes #3919 * [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA * [ ] Tests added/passed * [ ] Requires documentation to be updated * [x] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #3919 ## Detailed Description of the Pull Request / Additional comments My thought pattern here was to minimally disturb the existing code where possible. So where I could, I converted existing functions into coroutine using functions (like in the [core example](https://github.com/microsoft/terminal/issues/3919#issue-536598706)). For ~the most part~ all instances, I used the format where [`this` is accessed safely within a locked scope](https://github.com/microsoft/terminal/issues/3919#issuecomment-564730620). Some function signatures were changed to take objects by value instead of reference, so the coroutines don't crash when the objects are accessed past their original lifetime. The [copy](https://github.com/microsoft/terminal/blob/master/src/cascadia/TerminalApp/TerminalPage.cpp#L1132) and [paste](https://github.com/microsoft/terminal/blob/master/src/cascadia/TerminalApp/TerminalPage.cpp#L1170) event handler entry points were originally set to a high priority; however, the WinRT coroutines don't appear to support a priority scheme so this priority setting was not preserved in the translation. ## Validation Steps Performed Compiles and runs, and for every event with a clear trigger repro, I triggered it to ensure crashes weren't introduced. --- doc/STYLE.md | 1 + src/cascadia/TerminalApp/AppLogic.cpp | 40 ++-- src/cascadia/TerminalApp/AppLogic.h | 4 + src/cascadia/TerminalApp/Pane.cpp | 20 +- src/cascadia/TerminalApp/Pane.h | 1 + src/cascadia/TerminalApp/Tab.cpp | 40 ++-- src/cascadia/TerminalApp/Tab.h | 8 +- src/cascadia/TerminalApp/TerminalPage.cpp | 135 ++++++------ src/cascadia/TerminalApp/TerminalPage.h | 12 +- src/cascadia/TerminalControl/TermControl.cpp | 209 ++++++++++--------- src/cascadia/TerminalControl/TermControl.h | 9 +- 11 files changed, 259 insertions(+), 220 deletions(-) diff --git a/doc/STYLE.md b/doc/STYLE.md index 5b197dd8c..5c3488c64 100644 --- a/doc/STYLE.md +++ b/doc/STYLE.md @@ -5,3 +5,4 @@ 1. If it's brand new code or refactoring a complete class or area of the code, please follow as Modern C++ of a style as you can and reference the [C++ Core Guidelines](https://github.com/isocpp/CppCoreGuidelines) as much as you possibly can. 1. When working with any Win32 or NT API, please try to use the [Windows Implementation Library](./WIL.md) smart pointers and result handlers. 1. The use of NTSTATUS as a result code is discouraged, HRESULT or exceptions are preferred. Functions should not return a status code if they would always return a successful status code. Any function that returns a status code should be marked `noexcept` and have the `nodiscard` attribute. +1. When contributing code in `TerminalApp`, be mindful to appropriately use C++/WinRT [strong and weak references](https://docs.microsoft.com/en-us/windows/uwp/cpp-and-winrt-apis/weak-references), and have a good understanding of C++/WinRT [concurrency schemes](https://docs.microsoft.com/en-us/windows/uwp/cpp-and-winrt-apis/concurrency). diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp index 4b3712536..bb43bca3e 100644 --- a/src/cascadia/TerminalApp/AppLogic.cpp +++ b/src/cascadia/TerminalApp/AppLogic.cpp @@ -589,6 +589,30 @@ namespace winrt::TerminalApp::implementation } } + fire_and_forget AppLogic::_LoadErrorsDialogRoutine() + { + co_await winrt::resume_foreground(_root->Dispatcher()); + + const winrt::hstring titleKey = USES_RESOURCE(L"ReloadJsonParseErrorTitle"); + const winrt::hstring textKey = USES_RESOURCE(L"ReloadJsonParseErrorText"); + _ShowLoadErrorsDialog(titleKey, textKey, _settingsLoadedResult); + } + + fire_and_forget AppLogic::_ShowLoadWarningsDialogRoutine() + { + co_await winrt::resume_foreground(_root->Dispatcher()); + + _ShowLoadWarningsDialog(); + } + + fire_and_forget AppLogic::_RefreshThemeRoutine() + { + co_await winrt::resume_foreground(_root->Dispatcher()); + + // Refresh the UI theme + _ApplyTheme(_settings->GlobalSettings().GetRequestedTheme()); + } + // Method Description: // - Reloads the settings from the profile.json. void AppLogic::_ReloadSettings() @@ -602,19 +626,12 @@ namespace winrt::TerminalApp::implementation if (FAILED(_settingsLoadedResult)) { - _root->Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [this]() { - const winrt::hstring titleKey = USES_RESOURCE(L"ReloadJsonParseErrorTitle"); - const winrt::hstring textKey = USES_RESOURCE(L"ReloadJsonParseErrorText"); - _ShowLoadErrorsDialog(titleKey, textKey, _settingsLoadedResult); - }); - + _LoadErrorsDialogRoutine(); return; } else if (_settingsLoadedResult == S_FALSE) { - _root->Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [this]() { - _ShowLoadWarningsDialog(); - }); + _ShowLoadWarningsDialogRoutine(); } // Here, we successfully reloaded the settings, and created a new @@ -623,10 +640,7 @@ namespace winrt::TerminalApp::implementation // Update the settings in TerminalPage _root->SetSettings(_settings, true); - _root->Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [this]() { - // Refresh the UI theme - _ApplyTheme(_settings->GlobalSettings().GetRequestedTheme()); - }); + _RefreshThemeRoutine(); } // Method Description: diff --git a/src/cascadia/TerminalApp/AppLogic.h b/src/cascadia/TerminalApp/AppLogic.h index 44aec6eaf..c92e1d5a2 100644 --- a/src/cascadia/TerminalApp/AppLogic.h +++ b/src/cascadia/TerminalApp/AppLogic.h @@ -77,6 +77,10 @@ namespace winrt::TerminalApp::implementation void _ShowLoadErrorsDialog(const winrt::hstring& titleKey, const winrt::hstring& contentKey, HRESULT settingsLoadedResult); void _ShowLoadWarningsDialog(); + fire_and_forget _LoadErrorsDialogRoutine(); + fire_and_forget _ShowLoadWarningsDialogRoutine(); + fire_and_forget _RefreshThemeRoutine(); + void _OnLoaded(const IInspectable& sender, const Windows::UI::Xaml::RoutedEventArgs& eventArgs); [[nodiscard]] HRESULT _TryLoadSettings() noexcept; diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 2ccb21da1..aaa427891 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -734,6 +734,18 @@ void Pane::_CloseChild(const bool closeFirst) } } +winrt::fire_and_forget Pane::_CloseChildRoutine(const bool closeFirst) +{ + auto weakThis{ shared_from_this() }; + + co_await winrt::resume_foreground(_root.Dispatcher()); + + if (auto pane{ weakThis.get() }) + { + _CloseChild(closeFirst); + } +} + // Method Description: // - Adds event handlers to our children to handle their close events. // Arguments: @@ -743,15 +755,11 @@ void Pane::_CloseChild(const bool closeFirst) void Pane::_SetupChildCloseHandlers() { _firstClosedToken = _firstChild->Closed([this](auto&& /*s*/, auto&& /*e*/) { - _root.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=]() { - _CloseChild(true); - }); + _CloseChildRoutine(true); }); _secondClosedToken = _secondChild->Closed([this](auto&& /*s*/, auto&& /*e*/) { - _root.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=]() { - _CloseChild(false); - }); + _CloseChildRoutine(false); }); } diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index ec57a46dd..cbc707276 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -115,6 +115,7 @@ private: bool _NavigateFocus(const winrt::TerminalApp::Direction& direction); void _CloseChild(const bool closeFirst); + winrt::fire_and_forget _CloseChildRoutine(const bool closeFirst); void _FocusFirstChild(); void _ControlConnectionStateChangedHandler(const winrt::Microsoft::Terminal::TerminalControl::TermControl& sender, const winrt::Windows::Foundation::IInspectable& /*args*/); diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index 5c0c952d9..cea9db347 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -146,7 +146,7 @@ void Tab::_Focus() } } -void Tab::UpdateIcon(const winrt::hstring iconPath) +winrt::fire_and_forget Tab::UpdateIcon(const winrt::hstring iconPath) { // Don't reload our icon if it hasn't changed. if (iconPath == _lastIconPath) @@ -158,12 +158,12 @@ void Tab::UpdateIcon(const winrt::hstring iconPath) std::weak_ptr weakThis{ shared_from_this() }; - _tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [weakThis]() { - if (auto tab{ weakThis.lock() }) - { - tab->_tabViewItem.IconSource(GetColoredIcon(tab->_lastIconPath)); - } - }); + co_await winrt::resume_foreground(_tabViewItem.Dispatcher()); + + if (auto tab{ weakThis.lock() }) + { + _tabViewItem.IconSource(GetColoredIcon(_lastIconPath)); + } } // Method Description: @@ -185,18 +185,18 @@ winrt::hstring Tab::GetActiveTitle() const // - text: The new text string to use as the Header for our TabViewItem // Return Value: // - -void Tab::SetTabText(const winrt::hstring& text) +winrt::fire_and_forget Tab::SetTabText(const winrt::hstring text) { // Copy the hstring, so we don't capture a dead reference winrt::hstring textCopy{ text }; std::weak_ptr weakThis{ shared_from_this() }; - _tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [text = std::move(textCopy), weakThis]() { - if (auto tab{ weakThis.lock() }) - { - tab->_tabViewItem.Header(winrt::box_value(text)); - } - }); + co_await winrt::resume_foreground(_tabViewItem.Dispatcher()); + + if (auto tab{ weakThis.lock() }) + { + _tabViewItem.Header(winrt::box_value(text)); + } } // Method Description: @@ -207,13 +207,14 @@ void Tab::SetTabText(const winrt::hstring& text) // - delta: a number of lines to move the viewport relative to the current viewport. // Return Value: // - -void Tab::Scroll(const int delta) +winrt::fire_and_forget Tab::Scroll(const int delta) { auto control = GetActiveTerminalControl(); - control.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [control, delta]() { - const auto currentOffset = control.GetScrollOffset(); - control.KeyboardScrollViewport(currentOffset + delta); - }); + + co_await winrt::resume_foreground(control.Dispatcher()); + + const auto currentOffset = control.GetScrollOffset(); + control.KeyboardScrollViewport(currentOffset + delta); } // Method Description: @@ -365,6 +366,7 @@ void Tab::_AttachEventHandlersToPane(std::shared_ptr pane) pane->GotFocus([weakThis](std::shared_ptr sender) { // Do nothing if the Tab's lifetime is expired or pane isn't new. auto tab{ weakThis.lock() }; + if (tab && sender != tab->_activePane) { // Clear the active state of the entire tree, and mark only the sender as active. diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h index 48c134613..56a97f756 100644 --- a/src/cascadia/TerminalApp/Tab.h +++ b/src/cascadia/TerminalApp/Tab.h @@ -20,14 +20,14 @@ public: bool IsFocused() const noexcept; void SetFocused(const bool focused); - void Scroll(const int delta); + winrt::fire_and_forget Scroll(const int delta); bool CanSplitPane(winrt::TerminalApp::SplitState splitType); void SplitPane(winrt::TerminalApp::SplitState splitType, const GUID& profile, winrt::Microsoft::Terminal::TerminalControl::TermControl& control); - float CalcSnappedDimension(const bool widthOrHeight, const float dimension) const; + winrt::fire_and_forget UpdateIcon(const winrt::hstring iconPath); - void UpdateIcon(const winrt::hstring iconPath); + float CalcSnappedDimension(const bool widthOrHeight, const float dimension) const; void ResizeContent(const winrt::Windows::Foundation::Size& newSize); void ResizePane(const winrt::TerminalApp::Direction& direction); @@ -35,7 +35,7 @@ public: void UpdateSettings(const winrt::Microsoft::Terminal::Settings::TerminalSettings& settings, const GUID& profile); winrt::hstring GetActiveTitle() const; - void SetTabText(const winrt::hstring& text); + winrt::fire_and_forget SetTabText(const winrt::hstring text); void ClosePane(); diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 0f7891423..70e98b956 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -63,10 +63,7 @@ namespace winrt::TerminalApp::implementation _tabView = _tabRow.TabView(); _rearranging = false; - // weak_ptr to this TerminalPage object lambda capturing - auto weakThis{ get_weak() }; - - _tabView.TabDragStarting([weakThis](auto&& /*o*/, auto&& /*a*/) { + _tabView.TabDragStarting([weakThis{ get_weak() }](auto&& /*o*/, auto&& /*a*/) { if (auto page{ weakThis.get() }) { page->_rearranging = true; @@ -75,7 +72,7 @@ namespace winrt::TerminalApp::implementation } }); - _tabView.TabDragCompleted([weakThis](auto&& /*o*/, auto&& /*a*/) { + _tabView.TabDragCompleted([weakThis{ get_weak() }](auto&& /*o*/, auto&& /*a*/) { if (auto page{ weakThis.get() }) { auto& from{ page->_rearrangeFrom }; @@ -116,7 +113,7 @@ namespace winrt::TerminalApp::implementation _RegisterActionCallbacks(); //Event Bindings (Early) - _newTabButton.Click([weakThis](auto&&, auto&&) { + _newTabButton.Click([weakThis{ get_weak() }](auto&&, auto&&) { if (auto page{ weakThis.get() }) { page->_OpenNewTab(nullptr); @@ -323,9 +320,7 @@ namespace winrt::TerminalApp::implementation profileMenuItem.FontWeight(FontWeights::Bold()); } - auto weakThis{ get_weak() }; - - profileMenuItem.Click([profileIndex, weakThis](auto&&, auto&&) { + profileMenuItem.Click([profileIndex, weakThis{ get_weak() }](auto&&, auto&&) { if (auto page{ weakThis.get() }) { auto newTerminalArgs = winrt::make_self(); @@ -432,6 +427,13 @@ namespace winrt::TerminalApp::implementation TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance)); } + winrt::fire_and_forget TerminalPage::_RemoveOnCloseRoutine(Microsoft::UI::Xaml::Controls::TabViewItem tabViewItem, winrt::com_ptr page) + { + co_await winrt::resume_foreground(page->_tabView.Dispatcher()); + + page->_RemoveTabViewItem(tabViewItem); + } + // Method Description: // - Creates a new tab with the given settings. If the tab bar is not being // currently displayed, it will be shown. @@ -455,11 +457,10 @@ namespace winrt::TerminalApp::implementation // Don't capture a strong ref to the tab. If the tab is removed as this // is called, we don't really care anymore about handling the event. std::weak_ptr weakTabPtr = newTab; - auto weakThis{ get_weak() }; // When the tab's active pane changes, we'll want to lookup a new icon // for it, and possibly propogate the title up to the window. - newTab->ActivePaneChanged([weakTabPtr, weakThis]() { + newTab->ActivePaneChanged([weakTabPtr, weakThis{ get_weak() }]() { auto page{ weakThis.get() }; auto tab{ weakTabPtr.lock() }; @@ -486,15 +487,10 @@ namespace winrt::TerminalApp::implementation tabViewItem.PointerPressed({ this, &TerminalPage::_OnTabClick }); // When the tab is closed, remove it from our list of tabs. - newTab->Closed([tabViewItem, weakThis](auto&& /*s*/, auto&& /*e*/) { + newTab->Closed([tabViewItem, weakThis{ get_weak() }](auto&& /*s*/, auto&& /*e*/) { if (auto page{ weakThis.get() }) { - page->_tabView.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [tabViewItem, weakThis]() { - if (auto page{ weakThis.get() }) - { - page->_RemoveTabViewItem(tabViewItem); - } - }); + page->_RemoveOnCloseRoutine(tabViewItem, page); } }); @@ -808,9 +804,8 @@ namespace winrt::TerminalApp::implementation // Don't capture a strong ref to the tab. If the tab is removed as this // is called, we don't really care anymore about handling the event. std::weak_ptr weakTabPtr = hostingTab; - auto weakThis{ get_weak() }; - term.TitleChanged([weakTabPtr, weakThis](auto newTitle) { + term.TitleChanged([weakTabPtr, weakThis{ get_weak() }](auto newTitle) { auto page{ weakThis.get() }; auto tab{ weakTabPtr.lock() }; @@ -890,18 +885,19 @@ namespace winrt::TerminalApp::implementation return -1; } - void TerminalPage::_SetFocusedTabIndex(int tabIndex) + winrt::fire_and_forget TerminalPage::_SetFocusedTabIndex(int tabIndex) { // GH#1117: This is a workaround because _tabView.SelectedIndex(tabIndex) // sometimes set focus to an incorrect tab after removing some tabs - auto tab = _tabs.at(tabIndex); auto weakThis{ get_weak() }; - _tabView.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [tab, weakThis]() { - if (auto page{ weakThis.get() }) - { - page->_tabView.SelectedItem(tab->GetTabViewItem()); - } - }); + + co_await winrt::resume_foreground(_tabView.Dispatcher()); + + if (auto page{ weakThis.get() }) + { + auto tab = _tabs.at(tabIndex); + _tabView.SelectedItem(tab->GetTabViewItem()); + } } // Method Description: @@ -1153,37 +1149,37 @@ namespace winrt::TerminalApp::implementation // terminal control raises it's CopyToClipboard event. // Arguments: // - copiedData: the new string content to place on the clipboard. - void TerminalPage::_CopyToClipboardHandler(const IInspectable& /*sender*/, - const winrt::Microsoft::Terminal::TerminalControl::CopyToClipboardEventArgs& copiedData) + winrt::fire_and_forget TerminalPage::_CopyToClipboardHandler(const IInspectable /*sender*/, + const winrt::Microsoft::Terminal::TerminalControl::CopyToClipboardEventArgs copiedData) { - this->Dispatcher().RunAsync(CoreDispatcherPriority::High, [copiedData]() { - DataPackage dataPack = DataPackage(); - dataPack.RequestedOperation(DataPackageOperation::Copy); + co_await winrt::resume_foreground(Dispatcher(), CoreDispatcherPriority::High); - // copy text to dataPack - dataPack.SetText(copiedData.Text()); + DataPackage dataPack = DataPackage(); + dataPack.RequestedOperation(DataPackageOperation::Copy); - // copy html to dataPack - const auto htmlData = copiedData.Html(); - if (!htmlData.empty()) - { - dataPack.SetHtmlFormat(htmlData); - } + // copy text to dataPack + dataPack.SetText(copiedData.Text()); - // copy rtf data to dataPack - const auto rtfData = copiedData.Rtf(); - if (!rtfData.empty()) - { - dataPack.SetRtf(rtfData); - } + // copy html to dataPack + const auto htmlData = copiedData.Html(); + if (!htmlData.empty()) + { + dataPack.SetHtmlFormat(htmlData); + } - try - { - Clipboard::SetContent(dataPack); - Clipboard::Flush(); - } - CATCH_LOG(); - }); + // copy rtf data to dataPack + const auto rtfData = copiedData.Rtf(); + if (!rtfData.empty()) + { + dataPack.SetRtf(rtfData); + } + + try + { + Clipboard::SetContent(dataPack); + Clipboard::Flush(); + } + CATCH_LOG(); } // Method Description: @@ -1192,12 +1188,12 @@ namespace winrt::TerminalApp::implementation // data with it's PasteFromClipboard event. // Arguments: // - eventArgs: the PasteFromClipboard event sent from the TermControl - void TerminalPage::_PasteFromClipboardHandler(const IInspectable& /*sender*/, - const PasteFromClipboardEventArgs& eventArgs) + winrt::fire_and_forget TerminalPage::_PasteFromClipboardHandler(const IInspectable /*sender*/, + const PasteFromClipboardEventArgs eventArgs) { - this->Dispatcher().RunAsync(CoreDispatcherPriority::High, [eventArgs]() { - TerminalPage::PasteFromClipboard(eventArgs); - }); + co_await winrt::resume_foreground(Dispatcher(), CoreDispatcherPriority::High); + + TerminalPage::PasteFromClipboard(eventArgs); } // Function Description: @@ -1391,7 +1387,7 @@ namespace winrt::TerminalApp::implementation // This includes update the settings of all the tabs according // to their profiles, update the title and icon of each tab, and // finally create the tab flyout - void TerminalPage::_RefreshUIForSettingsReload() + winrt::fire_and_forget TerminalPage::_RefreshUIForSettingsReload() { // Re-wire the keybindings to their handlers, as we'll have created a // new AppKeyBindings object. @@ -1419,15 +1415,16 @@ namespace winrt::TerminalApp::implementation } auto weakThis{ get_weak() }; - this->Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [weakThis]() { - // repopulate the new tab button's flyout with entries for each - // profile, which might have changed - if (auto page{ weakThis.get() }) - { - page->_UpdateTabWidthMode(); - page->_CreateNewTabFlyout(); - } - }); + + co_await winrt::resume_foreground(Dispatcher()); + + // repopulate the new tab button's flyout with entries for each + // profile, which might have changed + if (auto page{ weakThis.get() }) + { + _UpdateTabWidthMode(); + _CreateNewTabFlyout(); + } } // Method Description: diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index 4e059e352..bca63fd2d 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -97,11 +97,13 @@ namespace winrt::TerminalApp::implementation winrt::Microsoft::Terminal::TerminalControl::TermControl _GetActiveControl(); int _GetFocusedTabIndex() const; - void _SetFocusedTabIndex(int tabIndex); + winrt::fire_and_forget _SetFocusedTabIndex(int tabIndex); void _CloseFocusedTab(); void _CloseFocusedPane(); void _CloseAllTabs(); + winrt::fire_and_forget _RemoveOnCloseRoutine(Microsoft::UI::Xaml::Controls::TabViewItem tabViewItem, winrt::com_ptr page); + // Todo: add more event implementations here // MSFT:20641986: Add keybindings for New Window void _Scroll(int delta); @@ -110,9 +112,9 @@ namespace winrt::TerminalApp::implementation void _ScrollPage(int delta); void _SetAcceleratorForMenuItem(Windows::UI::Xaml::Controls::MenuFlyoutItem& menuItem, const winrt::Microsoft::Terminal::Settings::KeyChord& keyChord); - void _CopyToClipboardHandler(const IInspectable& sender, const winrt::Microsoft::Terminal::TerminalControl::CopyToClipboardEventArgs& copiedData); - void _PasteFromClipboardHandler(const IInspectable& sender, - const Microsoft::Terminal::TerminalControl::PasteFromClipboardEventArgs& eventArgs); + winrt::fire_and_forget _CopyToClipboardHandler(const IInspectable sender, const winrt::Microsoft::Terminal::TerminalControl::CopyToClipboardEventArgs copiedData); + winrt::fire_and_forget _PasteFromClipboardHandler(const IInspectable sender, + const Microsoft::Terminal::TerminalControl::PasteFromClipboardEventArgs eventArgs); bool _CopyText(const bool trimTrailingWhitespace); void _PasteText(); static fire_and_forget PasteFromClipboard(winrt::Microsoft::Terminal::TerminalControl::PasteFromClipboardEventArgs eventArgs); @@ -127,7 +129,7 @@ namespace winrt::TerminalApp::implementation void _Find(); - void _RefreshUIForSettingsReload(); + winrt::fire_and_forget _RefreshUIForSettingsReload(); void _ToggleFullscreen(); diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 5a99be21d..6d5703210 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -255,45 +255,46 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // - newSettings: New settings values for the profile in this terminal. // Return Value: // - - void TermControl::UpdateSettings(Settings::IControlSettings newSettings) + winrt::fire_and_forget TermControl::UpdateSettings(Settings::IControlSettings newSettings) { _settings = newSettings; + auto weakThis{ get_weak() }; // Dispatch a call to the UI thread to apply the new settings to the // terminal. - _root.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [weakThis{ get_weak() }, this]() { - // If 'weakThis' is locked, then we can safely work with 'this' - if (auto control{ weakThis.get() }) + co_await winrt::resume_foreground(_root.Dispatcher()); + + // If 'weakThis' is locked, then we can safely work with 'this' + if (auto control{ weakThis.get() }) + { + // Update our control settings + _ApplyUISettings(); + + // Update DxEngine's SelectionBackground + _renderEngine->SetSelectionBackground(_settings.SelectionBackground()); + + // Update the terminal core with its new Core settings + _terminal->UpdateSettings(_settings); + + // Refresh our font with the renderer + _UpdateFont(); + + const auto width = _swapChainPanel.ActualWidth(); + const auto height = _swapChainPanel.ActualHeight(); + if (width != 0 && height != 0) { - // Update our control settings - _ApplyUISettings(); - - // Update DxEngine's SelectionBackground - _renderEngine->SetSelectionBackground(_settings.SelectionBackground()); - - // Update the terminal core with its new Core settings - _terminal->UpdateSettings(_settings); - - // Refresh our font with the renderer - _UpdateFont(); - - const auto width = _swapChainPanel.ActualWidth(); - const auto height = _swapChainPanel.ActualHeight(); - if (width != 0 && height != 0) - { - // If the font size changed, or the _swapchainPanel's size changed - // for any reason, we'll need to make sure to also resize the - // buffer. _DoResize will invalidate everything for us. - auto lock = _terminal->LockForWriting(); - _DoResize(width, height); - } - - // set TSF Foreground - Media::SolidColorBrush foregroundBrush{}; - foregroundBrush.Color(ColorRefToColor(_settings.DefaultForeground())); - _tsfInputControl.Foreground(foregroundBrush); + // If the font size changed, or the _swapchainPanel's size changed + // for any reason, we'll need to make sure to also resize the + // buffer. _DoResize will invalidate everything for us. + auto lock = _terminal->LockForWriting(); + _DoResize(width, height); } - }); + + // set TSF Foreground + Media::SolidColorBrush foregroundBrush{}; + foregroundBrush.Color(ColorRefToColor(_settings.DefaultForeground())); + _tsfInputControl.Foreground(foregroundBrush); + } } // Method Description: @@ -451,37 +452,38 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // - color: The background color to use as a uint32 (aka DWORD COLORREF) // Return Value: // - - void TermControl::_BackgroundColorChanged(const uint32_t color) + winrt::fire_and_forget TermControl::_BackgroundColorChanged(const uint32_t color) { - _root.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [weakThis{ get_weak() }, this, color]() { - // If 'weakThis' is locked, then we can safely work with 'this' - if (auto control{ weakThis.get() }) + auto weakThis{ get_weak() }; + + co_await winrt::resume_foreground(_root.Dispatcher()); + + if (auto control{ weakThis.get() }) + { + const auto R = GetRValue(color); + const auto G = GetGValue(color); + const auto B = GetBValue(color); + + winrt::Windows::UI::Color bgColor{}; + bgColor.R = R; + bgColor.G = G; + bgColor.B = B; + bgColor.A = 255; + + if (auto acrylic = _root.Background().try_as()) { - const auto R = GetRValue(color); - const auto G = GetGValue(color); - const auto B = GetBValue(color); - - winrt::Windows::UI::Color bgColor{}; - bgColor.R = R; - bgColor.G = G; - bgColor.B = B; - bgColor.A = 255; - - if (auto acrylic = _root.Background().try_as()) - { - acrylic.FallbackColor(bgColor); - acrylic.TintColor(bgColor); - } - else if (auto solidColor = _root.Background().try_as()) - { - solidColor.Color(bgColor); - } - - // Set the default background as transparent to prevent the - // DX layer from overwriting the background image or acrylic effect - _settings.DefaultBackground(ARGB(0, R, G, B)); + acrylic.FallbackColor(bgColor); + acrylic.TintColor(bgColor); } - }); + else if (auto solidColor = _root.Background().try_as()) + { + solidColor.Color(bgColor); + } + + // Set the default background as transparent to prevent the + // DX layer from overwriting the background image or acrylic effect + _settings.DefaultBackground(ARGB(0, R, G, B)); + } } TermControl::~TermControl() @@ -536,7 +538,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation return _connection.State(); } - void TermControl::SwapChainChanged() + winrt::fire_and_forget TermControl::SwapChainChanged() { if (!_initializedTerminal) { @@ -544,16 +546,33 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation } auto chain = _renderEngine->GetSwapChain(); + auto weakThis{ get_weak() }; - _swapChainPanel.Dispatcher().RunAsync(CoreDispatcherPriority::High, [weakThis{ get_weak() }, this, chain]() { - // If 'weakThis' is locked, then we can safely work with 'this' - if (auto control{ weakThis.get() }) - { - auto lock = _terminal->LockForWriting(); - auto nativePanel = _swapChainPanel.as(); - nativePanel->SetSwapChain(chain.Get()); - } - }); + co_await winrt::resume_foreground(_swapChainPanel.Dispatcher()); + + // If 'weakThis' is locked, then we can safely work with 'this' + if (auto control{ weakThis.get() }) + { + auto lock = _terminal->LockForWriting(); + auto nativePanel = _swapChainPanel.as(); + nativePanel->SetSwapChain(chain.Get()); + } + } + + winrt::fire_and_forget TermControl::_SwapChainRoutine() + { + auto chain = _renderEngine->GetSwapChain(); + auto weakThis{ get_weak() }; + + co_await winrt::resume_foreground(_swapChainPanel.Dispatcher()); + + if (auto control{ weakThis.get() }) + { + _terminal->LockConsole(); + auto nativePanel = _swapChainPanel.as(); + nativePanel->SetSwapChain(chain.Get()); + _terminal->UnlockConsole(); + } } bool TermControl::_InitializeTerminal() @@ -637,17 +656,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation auto inputFn = std::bind(&TermControl::_SendInputToConnection, this, std::placeholders::_1); _terminal->SetWriteInputCallback(inputFn); - auto chain = _renderEngine->GetSwapChain(); - - _swapChainPanel.Dispatcher().RunAsync(CoreDispatcherPriority::High, [weakThis{ get_weak() }, this, chain]() { - if (auto control{ weakThis.get() }) - { - _terminal->LockConsole(); - auto nativePanel = _swapChainPanel.as(); - nativePanel->SetSwapChain(chain.Get()); - _terminal->UnlockConsole(); - } - }); + _SwapChainRoutine(); // Set up the height of the ScrollViewer and the grid we're using to fake our scrolling height auto bottom = _terminal->GetViewport().BottomExclusive(); @@ -1613,9 +1622,9 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // of the buffer. // - viewHeight: the height of the viewport in rows. // - bufferSize: the length of the buffer, in rows - void TermControl::_TerminalScrollPositionChanged(const int viewTop, - const int viewHeight, - const int bufferSize) + winrt::fire_and_forget TermControl::_TerminalScrollPositionChanged(const int viewTop, + const int viewHeight, + const int bufferSize) { // Since this callback fires from non-UI thread, we might be already // closed/closing. @@ -1624,25 +1633,25 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation return; } - // Update our scrollbar - _scrollBar.Dispatcher().RunAsync(CoreDispatcherPriority::Low, [weakThis{ get_weak() }, this, viewTop, viewHeight, bufferSize]() { - // Even if we weren't closed/closing few lines above, we might be - // while waiting for this block of code to be dispatched. - // If 'weakThis' is locked, then we can safely work with 'this' - if (auto control{ weakThis.get() }) - { - if (_closing.load()) - { - return; - } - - _ScrollbarUpdater(_scrollBar, viewTop, viewHeight, bufferSize); - } - }); - // Set this value as our next expected scroll position. _lastScrollOffset = { viewTop }; _scrollPositionChangedHandlers(viewTop, viewHeight, bufferSize); + + auto weakThis{ get_weak() }; + + co_await winrt::resume_foreground(_scrollBar.Dispatcher()); + + // Even if we weren't closed/closing few lines above, we might be + // while waiting for this block of code to be dispatched. + // If 'weakThis' is locked, then we can safely work with 'this' + if (auto control{ weakThis.get() }) + { + if (!_closing.load()) + { + // Update our scrollbar + _ScrollbarUpdater(_scrollBar, viewTop, viewHeight, bufferSize); + } + } } hstring TermControl::Title() diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index 56ba48e2b..5f0eecd73 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -58,7 +58,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation TermControl(); TermControl(Settings::IControlSettings settings, TerminalConnection::ITerminalConnection connection); - void UpdateSettings(Settings::IControlSettings newSettings); + winrt::fire_and_forget UpdateSettings(Settings::IControlSettings newSettings); hstring Title(); @@ -77,7 +77,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation void AdjustFontSize(int fontSizeDelta); void ResetFontSize(); - void SwapChainChanged(); + winrt::fire_and_forget SwapChainChanged(); void CreateSearchBoxControl(); @@ -173,7 +173,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation void _Create(); void _ApplyUISettings(); void _InitializeBackgroundBrush(); - void _BackgroundColorChanged(const uint32_t color); + winrt::fire_and_forget _BackgroundColorChanged(const uint32_t color); bool _InitializeTerminal(); void _UpdateFont(const bool initialUpdate = false); void _SetFontSize(int fontSize); @@ -191,11 +191,12 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation void _SetEndSelectionPointAtCursor(Windows::Foundation::Point const& cursorPosition); void _SendInputToConnection(const std::wstring& wstr); void _SendPastedTextToConnection(const std::wstring& wstr); + winrt::fire_and_forget _SwapChainRoutine(); void _SwapChainSizeChanged(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::SizeChangedEventArgs const& e); void _SwapChainScaleChanged(Windows::UI::Xaml::Controls::SwapChainPanel const& sender, Windows::Foundation::IInspectable const& args); void _DoResize(const double newWidth, const double newHeight); void _TerminalTitleChanged(const std::wstring_view& wstr); - void _TerminalScrollPositionChanged(const int viewTop, const int viewHeight, const int bufferSize); + winrt::fire_and_forget _TerminalScrollPositionChanged(const int viewTop, const int viewHeight, const int bufferSize); void _MouseScrollHandler(const double delta, Windows::UI::Input::PointerPoint const& pointerPoint); void _MouseZoomHandler(const double delta);