-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove Monarch/Peasant & Make UI single-threaded #18215
base: main
Are you sure you want to change the base?
Conversation
b39c5a7
to
7853853
Compare
@@ -170,7 +170,6 @@ namespace winrt::TerminalApp::implementation | |||
|
|||
const auto canDragDrop = CanDragDrop(); | |||
|
|||
_tabRow.PointerMoved({ get_weak(), &TerminalPage::_RestorePointerCursorHandler }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved into TerminalControl
as a different workaround is needed for this functionality.
{ | ||
auto weakThis{ get_weak() }; | ||
co_await wil::resume_foreground(Dispatcher()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No multi-threading = No problem. (Suppress whitespace in the diff.)
<CurrentXamlPackage Include="@(AppxPackageRegistration)" | ||
Condition="'%(AppxPackageRegistration.Architecture)'=='$(Platform)' AND | ||
$([System.String]::new('%(AppxPackageRegistration.Filename)').StartsWith('Microsoft.UI.Xaml'))" /> | ||
<CurrentXamlPackage Include="@(AppxPackageRegistration)" Condition="'%(AppxPackageRegistration.Architecture)'=='$(Platform)' AND
 $([System.String]::new('%(AppxPackageRegistration.Filename)').StartsWith('Microsoft.UI.Xaml'))" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
VS mangled this :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
86/154! It's easy to review deletions ;)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
139/154
auto isCurrentlyDark = Theme::IsSystemInDarkTheme(); | ||
if (isCurrentlyDark != _currentSystemThemeIsDark) | ||
{ | ||
_currentSystemThemeIsDark = isCurrentlyDark; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📝 DH - Guessing this moved to the Message Window; confirm
As before, a minor refactor:
it into and deduplicating its functionality with
WindowEmperor
.a single instance), I wrote single-instance code with a NT mutex
and by yeeting data across processes with
WM_COPYDATA
.way faster. The more I tried to solve them the deeper I had to dig,
because you can't just put a mutex around
CascadiaSettings
.I then tried to seeif WinUI can run multiple windows on a single
thread and, as it turns out, it can.
So, I removed the multi- from the window threading.
So, to finish it up, I had to clean up the entire eventing system
around
WindowEmperor
, cleaned up all the coroutines,and cleaned up all the callbacks.
Closes #16733
Closes #17799
Closes #18176
Closes #18191
TODO: Find other issues
Validation Steps Performed