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

Hide the window from DWM until we're finished with initialization #12979

Merged
15 commits merged into from
May 6, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
15 changes: 12 additions & 3 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -713,12 +713,21 @@ namespace winrt::TerminalApp::implementation
// Switch to the BG thread -
co_await winrt::resume_background();

// Then enqueue the rest of this function for after the UI thread settles.
co_await wil::resume_foreground(Dispatcher(), CoreDispatcherPriority::Low);
if (auto self{ weak.get() })
{
// Then enqueue the rest of this function for after the UI thread settles.
co_await wil::resume_foreground(self->Dispatcher(), CoreDispatcherPriority::Low);
Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest just storing the dispatcher in the beginning as

const auto dispatcher = Dispatcher();

Seems like a more hassle free solution to me.
Also: Do you mind removing the winrt::apartment_context instantiation?

Finally there's the option to drop all of the coroutine stuff and write this:

Dispatcher().RunAsync(CoreDispatcherPriority::Low, [weak = get_weak()]() {
    if (auto self = weak.get())
    {
        self->_InitializedHandlers(*self, nullptr);
    }
});

Copy link
Member Author

Choose a reason for hiding this comment

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

Also: Do you mind removing the winrt::apartment_context instantiation?

frick deleted locally, forgot to save

I'd suggest just storing the dispatcher in the beginning

I thought it best to not stash an owning reference to the dispatcher, as to decrease the chances that the coroutine kept the Dispatcher alive despite the Page getting deref'ed.

}
else
{
// We don't exist anymore? Well, we probably don't need to fire
// off an Initialized event then...
co_return;
}

if (auto self{ weak.get() })
{
_InitializedHandlers(*self, nullptr);
self->_InitializedHandlers(*self, nullptr);
}
}
}
Expand Down
61 changes: 39 additions & 22 deletions src/cascadia/WindowsTerminal/AppHost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,7 @@ AppHost::AppHost() noexcept :
auto pfn = std::bind(&AppHost::_HandleCreateWindow,
this,
std::placeholders::_1,
std::placeholders::_2,
std::placeholders::_3);
std::placeholders::_2);
_window->SetCreateCallback(pfn);

_window->SetSnapDimensionCallback(std::bind(&winrt::TerminalApp::AppLogic::CalcSnappedDimension,
Expand Down Expand Up @@ -518,12 +517,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 */, RECT proposedRect)
{
// GH#11561: Hide the window until we're totally done being initialized.
// More commentary in TerminalPage::_CompleteInitialization
_proposedRect = proposedRect;
}

// 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 @@ -532,16 +550,8 @@ LaunchPosition AppHost::_GetWindowLaunchPosition()
// - launchMode: A LaunchMode enum reference that indicates the launch mode
// Return Value:
// - None
void AppHost::_HandleCreateWindow(const HWND hwnd, RECT proposedRect, LaunchMode& launchMode)
void AppHost::_initialResizeAndRepositionWindow(const HWND hwnd, RECT proposedRect, LaunchMode& launchMode)
{
// GH#11561: Hide the window until we're totally done being initialized.
// More commentary in TerminalPage::_CompleteInitialization
BOOL cloak = TRUE;
LOG_IF_FAILED(DwmSetWindowAttribute(hwnd,
DWMWA_CLOAK,
&cloak,
sizeof(cloak)));

launchMode = _logic.GetLaunchMode();

// Acquire the actual initial position
Expand Down Expand Up @@ -1560,11 +1570,18 @@ void AppHost::_CloseRequested(const winrt::Windows::Foundation::IInspectable& /*
void AppHost::_AppInitializedHandler(const winrt::Windows::Foundation::IInspectable& /*sender*/,
const winrt::Windows::Foundation::IInspectable& /*arg*/)
{
// GH#11561: We're totally done being initialized. Uncloak the window, making it visible.
// More commentary in TerminalPage::_CompleteInitialization
BOOL cloak = FALSE;
LOG_IF_FAILED(DwmSetWindowAttribute(_window->GetHandle(),
DWMWA_CLOAK,
&cloak,
sizeof(cloak)));
// 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.

LaunchMode launchMode{};
_initialResizeAndRepositionWindow(_window->GetHandle(), _proposedRect, launchMode);
Copy link
Member

Choose a reason for hiding this comment

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

Worried a bit about the resize - how often will we start up at the wrong size?

Copy link
Member Author

Choose a reason for hiding this comment

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

I never saw it?

Copy link
Member Author

Choose a reason for hiding this comment

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

lmao well this was the issue now 😑


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

ShowWindow(_window->GetHandle(), nCmdShow);
}
6 changes: 5 additions & 1 deletion src/cascadia/WindowsTerminal/AppHost.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ class AppHost

bool _shouldCreateWindow{ false };
bool _useNonClientArea{ false };
RECT _proposedRect{};

std::optional<til::throttled_func_trailing<>> _getWindowLayoutThrottler;
winrt::Windows::Foundation::IAsyncAction _SaveWindowLayouts();
Expand All @@ -39,7 +40,7 @@ class AppHost
void _HandleCommandlineArgs();
winrt::Microsoft::Terminal::Settings::Model::LaunchPosition _GetWindowLaunchPosition();

void _HandleCreateWindow(const HWND hwnd, RECT proposedRect, winrt::Microsoft::Terminal::Settings::Model::LaunchMode& launchMode);
void _HandleCreateWindow(const HWND hwnd, 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 Down Expand Up @@ -121,6 +122,9 @@ class AppHost
const winrt::Windows::Foundation::IInspectable& args);
void _HideNotificationIconRequested(const winrt::Windows::Foundation::IInspectable& sender,
const winrt::Windows::Foundation::IInspectable& args);

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

std::unique_ptr<NotificationIcon> _notificationIcon;
winrt::event_token _ReAddNotificationIconToken;
winrt::event_token _NotificationIconPressedToken;
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 @@ -112,7 +112,7 @@ void IslandWindow::Close()
// window.
// Return Value:
// - <none>
void IslandWindow::SetCreateCallback(std::function<void(const HWND, const RECT, LaunchMode& launchMode)> pfn) noexcept
void IslandWindow::SetCreateCallback(std::function<void(const HWND, const RECT)> pfn) noexcept
{
_pfnCreateCallback = pfn;
}
Expand Down Expand Up @@ -154,19 +154,13 @@ void IslandWindow::_HandleCreateWindow(const WPARAM, const LPARAM lParam) noexce
rc.right = rc.left + pcs->cx;
rc.bottom = rc.top + pcs->cy;

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

int 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 it's initialization.

UpdateWindow(_window.get());

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

virtual void Initialize();

void SetCreateCallback(std::function<void(const HWND, const RECT, winrt::Microsoft::Terminal::Settings::Model::LaunchMode& launchMode)> pfn) noexcept;
void SetCreateCallback(std::function<void(const HWND, const RECT)> pfn) noexcept;
void SetSnapDimensionCallback(std::function<float(bool widthOrHeight, float dimension)> pfn) noexcept;

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

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

void _HandleCreateWindow(const WPARAM wParam, const LPARAM lParam) noexcept;
Expand Down