Skip to content

Commit

Permalink
Revert the revert of "Hide the window until we're finished with initi…
Browse files Browse the repository at this point in the history
…alization" (#13811)

This is a revert of the revert of #12979. We mainly reverted that PR
because of an issue where restored windows would grow/shrink slightly on
external displays. It was too close to the ship date for that release,
so we backed it out wholesale (in #13098). I think I've found the real
root of the problem, and fixed it here.

The money diff here from the original PR:
4c08b9a...c34495d.
Basically, I had put the part where we actually handle the creation of
the window into `_AppInitializedHandler`, when we should have left the
window to be created in `_HandleCreateWindow` We create it there,
_hidden_, and then should only _show_ it in `_AppInitializedHandler`.

I'm _NOT_ incorporating the change for #9053. I reverted that bit in
1fac403. I am too worried about that messing with the phwnd that I
wanted to get that reviewed and committed atomically, separately.

* fixes  #11561
* tested manually
* I work here
  • Loading branch information
zadjii-msft authored Apr 6, 2023
1 parent 6f8ef58 commit 083fc64
Show file tree
Hide file tree
Showing 10 changed files with 121 additions and 36 deletions.
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.

// 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

0 comments on commit 083fc64

Please sign in to comment.