Skip to content
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

Revert the revert of "Hide the window until we're finished with initialization" #13811

Merged
merged 37 commits into from
Apr 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
103beb4
I just want to run the scratch project, I have no idea how this got s…
zadjii-msft Jul 18, 2022
c795793
Revert "I just want to run the scratch project, I have no idea how th…
zadjii-msft Jul 18, 2022
11bfc73
A BLANK SCRATCH COMMIT
zadjii-msft Jul 18, 2022
1dd6e99
prototype
zadjii-msft Jul 18, 2022
874a15b
Good-news-everyone.gif
zadjii-msft Jul 19, 2022
dc78b32
this does temporarily get the title out of the link, but then it's lose
zadjii-msft Jul 19, 2022
9a2ee6a
Cleanup, comments, etc.
zadjii-msft Jul 19, 2022
fd75c5e
I think this is all the prototyping I need
zadjii-msft Jul 21, 2022
4b717db
Merge remote-tracking branch 'origin/main' into dev/migrie/f/9458-sta…
zadjii-msft Jul 22, 2022
c0b4840
Merge remote-tracking branch 'origin/main' into dev/migrie/f/9458-sta…
zadjii-msft Jul 22, 2022
7268e96
Remove scratch work
zadjii-msft Jul 22, 2022
e333661
cleanup for review
zadjii-msft Jul 22, 2022
879a955
more comments, cleanup
zadjii-msft Jul 22, 2022
5bf3c1e
had this all before the long weekend
zadjii-msft Jul 26, 2022
01420b9
Merge remote-tracking branch 'origin/main' into dev/migrie/f/9458-sta…
zadjii-msft Jul 26, 2022
18a62bd
I think I'm gonna need to put EVERYTHING in here
zadjii-msft Jul 26, 2022
d2336e3
Merge remote-tracking branch 'origin/main' into dev/migrie/f/9458-sta…
zadjii-msft Aug 11, 2022
3ce349e
big s/o to whoever added the onecore projects to the oss repo, so we …
zadjii-msft Aug 12, 2022
5849279
Merge branch 'main' into dev/migrie/f/9458-startupInfoToTerminal
zadjii-msft Aug 19, 2022
38de95e
Code review notes
zadjii-msft Aug 19, 2022
4c08b9a
Revert "Revert "Hide the window from DWM until we're finished with in…
zadjii-msft Aug 19, 2022
a1fd241
Well this fixes it but I need to sort out the last bits of the launch…
zadjii-msft Aug 19, 2022
c34495d
yea this does it
zadjii-msft Aug 19, 2022
1fac403
until we can be sure, lets take this out
zadjii-msft Aug 22, 2022
42a5fc9
Merge remote-tracking branch 'origin/main' into dev/migrie/b/9053-att…
zadjii-msft Aug 26, 2022
b37ed82
Migrate spelling-0.0.21 changes from main
DHowett Aug 26, 2022
8c66d90
Merge remote-tracking branch 'origin/main' into dev/migrie/b/9053-att…
zadjii-msft Dec 1, 2022
6a20741
Merge branch 'dev/migrie/b/9053-attempt-2' of https://github.com/micr…
zadjii-msft Dec 1, 2022
e99af2d
Merge branch 'main' into dev/migrie/b/9053-attempt-2
zadjii-msft Jan 24, 2023
8b14a22
Merge branch 'main' into dev/migrie/b/9053-attempt-2
zadjii-msft Apr 5, 2023
6a99f25
as a point of bookmarking - this works
zadjii-msft Apr 5, 2023
431b26a
as it turns out, we don't need the invalidate. that's at least _less_…
zadjii-msft Apr 5, 2023
e67bfb3
this is cleaner
zadjii-msft Apr 5, 2023
02f5bfa
cleanup comment
zadjii-msft Apr 5, 2023
35db280
VS *shake fist*
zadjii-msft Apr 5, 2023
b7ea20c
update this comment
zadjii-msft Apr 6, 2023
5289c23
You know, I don't think we need allowsetforeground
zadjii-msft Apr 6, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 31 additions & 2 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ namespace winrt::TerminalApp::implementation
// - <none>
// Return Value:
// - <none>
void TerminalPage::_CompleteInitialization()
winrt::fire_and_forget TerminalPage::_CompleteInitialization()
{
_startupState = StartupState::Initialized;

Expand All @@ -643,10 +643,39 @@ namespace winrt::TerminalApp::implementation
if (_tabs.Size() == 0 && !(_shouldStartInboundListener || _isEmbeddingInboundListener))
{
_LastTabClosedHandlers(*this, winrt::make<LastTabClosedEventArgs>(false));
co_return;
}
else
{
_InitializedHandlers(*this, nullptr);
// GH#11561: When we start up, our window is initially just a frame
// with a transparent content area. We're gonna do all this startup
// init on the UI thread, so the UI won't actually paint till it's
// all done. This results in a few frames where the frame is
// visible, before the page paints for the first time, before any
// tabs appears, etc.
//
// To mitigate this, we're gonna wait for the UI thread to finish
// everything it's gotta do for the initial init, and _then_ fire
// our Initialized event. By waiting for everything else to finish
// (CoreDispatcherPriority::Low), we let all the tabs and panes
// actually get created. In the window layer, we're gonna cloak the
// window till this event is fired, so we don't actually see this
// frame until we're actually all ready to go.
//
// This will result in the window seemingly not loading as fast, but
// it will actually take exactly the same amount of time before it's
// usable.
//
// We also experimented with drawing a solid BG color before the
// initialization is finished. However, there are still a few frames
// after the frame is displayed before the XAML content first draws,
// so that didn't actually resolve any issues.
Dispatcher().RunAsync(CoreDispatcherPriority::Low, [weak = get_weak()]() {
if (auto self{ weak.get() })
{
self->_InitializedHandlers(*self, nullptr);
}
});
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalApp/TerminalPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ namespace winrt::TerminalApp::implementation
TYPED_EVENT(AlwaysOnTopChanged, IInspectable, IInspectable);
TYPED_EVENT(RaiseVisualBell, IInspectable, IInspectable);
TYPED_EVENT(SetTaskbarProgress, IInspectable, IInspectable);
TYPED_EVENT(Initialized, IInspectable, winrt::Windows::UI::Xaml::RoutedEventArgs);
TYPED_EVENT(Initialized, IInspectable, IInspectable);
TYPED_EVENT(IdentifyWindowsRequested, IInspectable, IInspectable);
TYPED_EVENT(RenameWindowRequested, Windows::Foundation::IInspectable, winrt::TerminalApp::RenameWindowRequestedArgs);
TYPED_EVENT(SummonWindowRequested, IInspectable, IInspectable);
Expand Down Expand Up @@ -447,7 +447,7 @@ namespace winrt::TerminalApp::implementation

void _StartInboundListener();

void _CompleteInitialization();
winrt::fire_and_forget _CompleteInitialization();

void _FocusActiveControl(IInspectable sender, IInspectable eventArgs);

Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/TerminalPage.idl
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ namespace TerminalApp
event Windows.Foundation.TypedEventHandler<Object, Object> FocusModeChanged;
event Windows.Foundation.TypedEventHandler<Object, Object> FullscreenChanged;
event Windows.Foundation.TypedEventHandler<Object, Object> AlwaysOnTopChanged;
event Windows.Foundation.TypedEventHandler<Object, Windows.UI.Xaml.RoutedEventArgs> Initialized;
event Windows.Foundation.TypedEventHandler<Object, Object> Initialized;
event Windows.Foundation.TypedEventHandler<Object, Object> SetTaskbarProgress;
event Windows.Foundation.TypedEventHandler<Object, Object> IdentifyWindowsRequested;
event Windows.Foundation.TypedEventHandler<Object, RenameWindowRequestedArgs> RenameWindowRequested;
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalApp/TerminalWindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,8 @@ namespace winrt::TerminalApp::implementation
// 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.
FORWARDED_TYPED_EVENT(Initialized, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable, _root, Initialized);

FORWARDED_TYPED_EVENT(SetTitleBarContent, winrt::Windows::Foundation::IInspectable, winrt::Windows::UI::Xaml::UIElement, _root, SetTitleBarContent);
FORWARDED_TYPED_EVENT(TitleChanged, winrt::Windows::Foundation::IInspectable, winrt::hstring, _root, TitleChanged);
FORWARDED_TYPED_EVENT(LastTabClosed, winrt::Windows::Foundation::IInspectable, winrt::TerminalApp::LastTabClosedEventArgs, _root, LastTabClosed);
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalApp/TerminalWindow.idl
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ namespace TerminalApp
// information.
void DismissDialog();

event Windows.Foundation.TypedEventHandler<Object, Object> Initialized;

event Windows.Foundation.TypedEventHandler<Object, Windows.UI.Xaml.UIElement> SetTitleBarContent;
event Windows.Foundation.TypedEventHandler<Object, String> TitleChanged;
event Windows.Foundation.TypedEventHandler<Object, LastTabClosedEventArgs> LastTabClosed;
Expand Down
69 changes: 62 additions & 7 deletions src/cascadia/WindowsTerminal/AppHost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ AppHost::AppHost(const winrt::TerminalApp::AppLogic& logic,
auto pfn = std::bind(&AppHost::_HandleCreateWindow,
this,
std::placeholders::_1,
std::placeholders::_2,
std::placeholders::_3);
std::placeholders::_2);
_window->SetCreateCallback(pfn);

// Event handlers:
Expand Down Expand Up @@ -339,6 +338,7 @@ void AppHost::Initialize()

_window->UpdateSettingsRequested({ this, &AppHost::_requestUpdateSettings });

_revokers.Initialized = _windowLogic.Initialized(winrt::auto_revoke, { this, &AppHost::_WindowInitializedHandler });
_revokers.RequestedThemeChanged = _windowLogic.RequestedThemeChanged(winrt::auto_revoke, { this, &AppHost::_UpdateTheme });
_revokers.FullscreenChanged = _windowLogic.FullscreenChanged(winrt::auto_revoke, { this, &AppHost::_FullscreenChanged });
_revokers.FocusModeChanged = _windowLogic.FocusModeChanged(winrt::auto_revoke, { this, &AppHost::_FocusModeChanged });
Expand Down Expand Up @@ -510,12 +510,31 @@ LaunchPosition AppHost::_GetWindowLaunchPosition()
return pos;
}

// Method Description:
// - Callback for when the window is first being created (during WM_CREATE).
// Stash the proposed size for later. We'll need that once we're totally
// initialized, so that we can show the window in the right position *when we
// want to show it*. If we did the _initialResizeAndRepositionWindow work now,
// it would have no effect, because the window is not yet visible.
// Arguments:
// - hwnd: The HWND of the window we're about to create.
// - proposedRect: The location and size of the window that we're about to
// create. We'll use this rect to determine which monitor the window is about
// to appear on.
void AppHost::_HandleCreateWindow(const HWND /* hwnd */, const til::rect& proposedRect)
{
// GH#11561: Hide the window until we're totally done being initialized.
// More commentary in TerminalPage::_CompleteInitialization
_initialResizeAndRepositionWindow(_window->GetHandle(), proposedRect, _launchMode);
}

// Method Description:
// - Resize the window we're about to create to the appropriate dimensions, as
// specified in the settings. This will be called during the handling of
// WM_CREATE. We'll load the settings for the app, then get the proposed size
// of the terminal from the app. Using that proposed size, we'll resize the
// window we're creating, so that it'll match the values in the settings.
// specified in the settings. This is called once the app has finished it's
// initial setup, once we have created all the tabs, panes, etc. We'll load
// the settings for the app, then get the proposed size of the terminal from
// the app. Using that proposed size, we'll resize the window we're creating,
// so that it'll match the values in the settings.
// Arguments:
// - hwnd: The HWND of the window we're about to create.
// - proposedRect: The location and size of the window that we're about to
Expand All @@ -524,7 +543,7 @@ LaunchPosition AppHost::_GetWindowLaunchPosition()
// - launchMode: A LaunchMode enum reference that indicates the launch mode
// Return Value:
// - None
void AppHost::_HandleCreateWindow(const HWND hwnd, til::rect proposedRect, LaunchMode& launchMode)
void AppHost::_initialResizeAndRepositionWindow(const HWND hwnd, til::rect proposedRect, LaunchMode& launchMode)
{
launchMode = _windowLogic.GetLaunchMode();

Expand Down Expand Up @@ -1211,6 +1230,42 @@ void AppHost::_PropertyChangedHandler(const winrt::Windows::Foundation::IInspect
}
}

winrt::fire_and_forget AppHost::_WindowInitializedHandler(const winrt::Windows::Foundation::IInspectable& /*sender*/,
const winrt::Windows::Foundation::IInspectable& /*arg*/)
{
// GH#11561: We're totally done being initialized. Resize the window to
// match the initial settings, and then call ShowWindow to finally make us
// visible.

auto nCmdShow = SW_SHOW;
if (WI_IsFlagSet(_launchMode, LaunchMode::MaximizedMode))
{
nCmdShow = SW_MAXIMIZE;
}

// For inexplicable reasons, again, hop to the BG thread, then back to the
// UI thread. This is shockingly load bearing - without this, then
// sometimes, we'll _still_ show the HWND before the XAML island actually
// paints.
co_await winrt::resume_background();
co_await wil::resume_foreground(_windowLogic.GetRoot().Dispatcher(), winrt::Windows::UI::Core::CoreDispatcherPriority::Low);
ShowWindow(_window->GetHandle(), nCmdShow);

// If we didn't start the window hidden (in one way or another), then try to
// pull ourselves to the foreground. Don't necessarily do a whole "summon",
// we don't really want to STEAL foreground if someone rightfully took it

const bool noForeground = nCmdShow == SW_SHOWMINIMIZED ||
nCmdShow == SW_SHOWNOACTIVATE ||
nCmdShow == SW_SHOWMINNOACTIVE ||
nCmdShow == SW_SHOWNA ||
nCmdShow == SW_FORCEMINIMIZE;
if (!noForeground)
{
SetForegroundWindow(_window->GetHandle());
}
}

winrt::TerminalApp::TerminalWindow AppHost::Logic()
{
return _windowLogic;
Expand Down
11 changes: 9 additions & 2 deletions src/cascadia/WindowsTerminal/AppHost.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class AppHost
winrt::com_ptr<IVirtualDesktopManager> _desktopManager{ nullptr };

bool _useNonClientArea{ false };
winrt::Microsoft::Terminal::Settings::Model::LaunchMode _launchMode{};

std::shared_ptr<ThrottledFuncTrailing<bool>> _showHideWindowThrottler;

Expand All @@ -47,7 +48,8 @@ class AppHost
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);
void _HandleCreateWindow(const HWND hwnd, const til::rect& proposedRect);

void _UpdateTitleBarContent(const winrt::Windows::Foundation::IInspectable& sender,
const winrt::Windows::UI::Xaml::UIElement& arg);
void _UpdateTheme(const winrt::Windows::Foundation::IInspectable&,
Expand All @@ -60,6 +62,9 @@ class AppHost
const winrt::Windows::Foundation::IInspectable& arg);
void _AlwaysOnTopChanged(const winrt::Windows::Foundation::IInspectable& sender,
const winrt::Windows::Foundation::IInspectable& arg);
winrt::fire_and_forget _WindowInitializedHandler(const winrt::Windows::Foundation::IInspectable& sender,
const winrt::Windows::Foundation::IInspectable& arg);

void _RaiseVisualBell(const winrt::Windows::Foundation::IInspectable& sender,
const winrt::Windows::Foundation::IInspectable& arg);
void _WindowMouseWheeled(const til::point coord, const int32_t delta);
Expand Down Expand Up @@ -116,7 +121,7 @@ class AppHost
void _PropertyChangedHandler(const winrt::Windows::Foundation::IInspectable& sender,
const winrt::Windows::UI::Xaml::Data::PropertyChangedEventArgs& args);

void _initialResizeAndRepositionWindow(const HWND hwnd, RECT proposedRect, winrt::Microsoft::Terminal::Settings::Model::LaunchMode& launchMode);
void _initialResizeAndRepositionWindow(const HWND hwnd, til::rect proposedRect, winrt::Microsoft::Terminal::Settings::Model::LaunchMode& launchMode);

void _handleMoveContent(const winrt::Windows::Foundation::IInspectable& sender,
winrt::TerminalApp::RequestMoveContentArgs args);
Expand Down Expand Up @@ -146,8 +151,10 @@ class AppHost
winrt::Microsoft::Terminal::Remoting::Peasant::SummonRequested_revoker peasantSummonRequested;
winrt::Microsoft::Terminal::Remoting::Peasant::DisplayWindowIdRequested_revoker peasantDisplayWindowIdRequested;
winrt::Microsoft::Terminal::Remoting::Peasant::QuitRequested_revoker peasantQuitRequested;

winrt::Microsoft::Terminal::Remoting::Peasant::AttachRequested_revoker AttachRequested;

winrt::TerminalApp::TerminalWindow::Initialized_revoker Initialized;
winrt::TerminalApp::TerminalWindow::CloseRequested_revoker CloseRequested;
winrt::TerminalApp::TerminalWindow::RequestedThemeChanged_revoker RequestedThemeChanged;
winrt::TerminalApp::TerminalWindow::FullscreenChanged_revoker FullscreenChanged;
Expand Down
14 changes: 4 additions & 10 deletions src/cascadia/WindowsTerminal/IslandWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ void IslandWindow::Close()
// window.
// Return Value:
// - <none>
void IslandWindow::SetCreateCallback(std::function<void(const HWND, const til::rect&, LaunchMode& launchMode)> pfn) noexcept
void IslandWindow::SetCreateCallback(std::function<void(const HWND, const til::rect&)> pfn) noexcept
{
_pfnCreateCallback = pfn;
}
Expand Down Expand Up @@ -153,19 +153,13 @@ void IslandWindow::_HandleCreateWindow(const WPARAM, const LPARAM lParam) noexce
rc.right = rc.left + pcs->cx;
rc.bottom = rc.top + pcs->cy;

auto launchMode = LaunchMode::DefaultMode;
if (_pfnCreateCallback)
{
_pfnCreateCallback(_window.get(), rc, launchMode);
_pfnCreateCallback(_window.get(), rc);
}

auto nCmdShow = SW_SHOW;
if (WI_IsFlagSet(launchMode, LaunchMode::MaximizedMode))
{
nCmdShow = SW_MAXIMIZE;
}

ShowWindow(_window.get(), nCmdShow);
// GH#11561: DO NOT call ShowWindow here. The AppHost will call ShowWindow
// once the app has completed its initialization.

UpdateWindow(_window.get());

Expand Down
5 changes: 3 additions & 2 deletions src/cascadia/WindowsTerminal/IslandWindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ class IslandWindow :

virtual void Initialize();

void SetCreateCallback(std::function<void(const HWND, const til::rect&, winrt::Microsoft::Terminal::Settings::Model::LaunchMode& launchMode)> pfn) noexcept;
void SetCreateCallback(std::function<void(const HWND, const til::rect&)> pfn) noexcept;

void SetSnapDimensionCallback(std::function<float(bool widthOrHeight, float dimension)> pfn) noexcept;

void FocusModeChanged(const bool focusMode);
Expand Down Expand Up @@ -97,7 +98,7 @@ class IslandWindow :
winrt::Windows::UI::Xaml::Controls::Grid _rootGrid;
wil::com_ptr<ITaskbarList3> _taskbar;

std::function<void(const HWND, const til::rect&, winrt::Microsoft::Terminal::Settings::Model::LaunchMode& launchMode)> _pfnCreateCallback;
std::function<void(const HWND, const til::rect&)> _pfnCreateCallback;
std::function<float(bool, float)> _pfnSnapDimensionCallback;

void _HandleCreateWindow(const WPARAM wParam, const LPARAM lParam) noexcept;
Expand Down
15 changes: 5 additions & 10 deletions src/cascadia/WindowsTerminal/WindowEmperor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,18 +169,13 @@ void WindowEmperor::_windowStartedHandlerPostXAML(const std::shared_ptr<WindowTh
sender->Logic().IsQuakeWindowChanged({ this, &WindowEmperor::_windowIsQuakeWindowChanged });
sender->UpdateSettingsRequested({ this, &WindowEmperor::_windowRequestUpdateSettings });

// Summon the window to the foreground, since we might not _currently_ be in
// DON'T Summon the window to the foreground, since we might not _currently_ be in
// the foreground, but we should act like the new window is.
//
// TODO: GH#14957 - use AllowSetForeground from the original wt.exe instead
Remoting::SummonWindowSelectionArgs args{};
args.OnCurrentDesktop(false);
args.WindowID(sender->Peasant().GetID());
args.SummonBehavior().MoveToCurrentDesktop(false);
args.SummonBehavior().ToggleVisibility(false);
args.SummonBehavior().DropdownDuration(0);
args.SummonBehavior().ToMonitor(Remoting::MonitorBehavior::InPlace);
_manager.SummonWindow(args);
// If you summon here, the resulting code will call ShowWindow(SW_SHOW) on
// the Terminal window, making it visible BEFORE the XAML island is actually
// ready to be drawn. We want to wait till the app's Initialized event
// before we make the window visible.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit worried that (1) we used to Summon from here and (2) now we aren't. Will this result in new terminal windows not coming to the front when they are the second window in the process? Will this result in us missing some activations somehow?


// Now that the window is ready to go, we can add it to our list of windows,
// because we know it will be well behaved.
Expand Down