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

Make the window name _quake special #9785

Merged
11 commits merged into from
Apr 26, 2021
6 changes: 6 additions & 0 deletions .github/actions/spelling/expect/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1025,14 +1025,20 @@ hsl
hstr
hstring
hsv
HTBOTTOMLEFT
HTBOTTOMRIGHT
HTCAPTION
HTCLIENT
HTLEFT
htm
HTMAXBUTTON
HTMINBUTTON
html
HTMLTo
HTRIGHT
HTTOP
HTTOPLEFT
HTTOPRIGHT
hu
hungapp
HVP
Expand Down
6 changes: 5 additions & 1 deletion src/cascadia/TerminalApp/AppLogic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,10 @@ namespace winrt::TerminalApp::implementation
// launched _fullscreen_, toggle fullscreen mode. This will make sure
// that the window size is _first_ set up as something sensible, so
// leaving fullscreen returns to a reasonable size.
//
// We know at the start, that the root TerminalPage definitely isn't
// in focus nor fullscreen mode. So "Toggle" here will always work
// to "enable".
const auto launchMode = this->GetLaunchMode();
if (IsQuakeWindow())
{
Expand Down Expand Up @@ -1424,7 +1428,7 @@ namespace winrt::TerminalApp::implementation
}
}

bool AppLogic::IsQuakeWindow() const
bool AppLogic::IsQuakeWindow() const noexcept
{
return _root->IsQuakeWindow();
}
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/AppLogic.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ namespace winrt::TerminalApp::implementation
void WindowName(const winrt::hstring& name);
uint64_t WindowId();
void WindowId(const uint64_t& id);
bool IsQuakeWindow() const;
bool IsQuakeWindow() const noexcept;

Windows::Foundation::Size GetLaunchDimensions(uint32_t dpi);
bool CenterOnLaunch();
Expand Down
23 changes: 14 additions & 9 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ namespace winrt
using IInspectable = Windows::Foundation::IInspectable;
}

static constexpr std::wstring_view QuakeWindowName{ L"_quake" };

namespace winrt::TerminalApp::implementation
{
TerminalPage::TerminalPage() :
Expand Down Expand Up @@ -2054,9 +2056,12 @@ namespace winrt::TerminalApp::implementation

void TerminalPage::_SetFocusMode(const bool inFocusMode)
{
if (inFocusMode != FocusMode())
// If we're the quake window, we must always be in focus mode.
// Prevent leaving focus mode here.
const bool newInFocusMode = inFocusMode || IsQuakeWindow();
if (newInFocusMode != FocusMode())
{
_isInFocusMode = inFocusMode || IsQuakeWindow();
_isInFocusMode = newInFocusMode;
_UpdateTabView();
_FocusModeChangedHandlers(*this, nullptr);
}
Expand Down Expand Up @@ -2268,7 +2273,7 @@ namespace winrt::TerminalApp::implementation

bool TerminalPage::FocusMode() const
{
return _isInFocusMode || IsQuakeWindow();
return _isInFocusMode;
}

bool TerminalPage::Fullscreen() const
Expand Down Expand Up @@ -2613,10 +2618,10 @@ namespace winrt::TerminalApp::implementation
// If we're entering quake mode, or leaving it
if (IsQuakeWindow() != oldIsQuakeMode)
{
// If we're entering QM from ~FM, then this will enter FM
// If we're entering QM from FM, then this will do nothing
// If we're leaving QM (we're already in FM), then this will do nothing
_SetFocusMode(_isInFocusMode);
// If we're entering Quake Mode from ~Focus Mode, then this will enter Focus Mode
// If we're entering Quake Mode from Focus Mode, then this will do nothing
// If we're leaving Quake Mode (we're already in Focus Mode), then this will do nothing
_SetFocusMode(true);
_IsQuakeWindowChangedHandlers(*this, nullptr);
}
}
Expand Down Expand Up @@ -2755,8 +2760,8 @@ namespace winrt::TerminalApp::implementation
}
}

bool TerminalPage::IsQuakeWindow() const
bool TerminalPage::IsQuakeWindow() const noexcept
{
return WindowName() == L"_quake";
return WindowName() == QuakeWindowName;
}
}
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/TerminalPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ namespace winrt::TerminalApp::implementation
void WindowId(const uint64_t& value);
winrt::hstring WindowIdForDisplay() const noexcept;
winrt::hstring WindowNameForDisplay() const noexcept;
bool IsQuakeWindow() const;
bool IsQuakeWindow() const noexcept;

WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler);

Expand Down
25 changes: 13 additions & 12 deletions src/cascadia/WindowsTerminal/AppHost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ AppHost::AppHost() noexcept :
_window = std::make_unique<IslandWindow>();
}

_window->IsQuakeWindow(_logic.IsQuakeWindow());
// Update our own internal state tracking if we're in quake mode or not.
_IsQuakeWindowChanged(nullptr, nullptr);

// Tell the window to callback to us when it's about to handle a WM_CREATE
auto pfn = std::bind(&AppHost::_HandleCreateWindow,
Expand Down Expand Up @@ -244,6 +245,7 @@ void AppHost::Initialize()
_logic.FullscreenChanged({ this, &AppHost::_FullscreenChanged });
_logic.FocusModeChanged({ this, &AppHost::_FocusModeChanged });
_logic.AlwaysOnTopChanged({ this, &AppHost::_AlwaysOnTopChanged });
_logic.RaiseVisualBell({ this, &AppHost::_RaiseVisualBell });

_logic.Create();

Expand Down Expand Up @@ -399,14 +401,6 @@ void AppHost::_HandleCreateWindow(const HWND hwnd, RECT proposedRect, LaunchMode

til::point origin{ ::base::saturated_cast<ptrdiff_t>(proposedRect.left),
::base::saturated_cast<ptrdiff_t>(proposedRect.top) };
if (centerOnLaunch)
{
// Move our proposed location into the center of that specific monitor.
origin = til::point{
nearestMonitorInfo.rcWork.left + ((desktopDimensions.width() / 2) - (dimensions.width() / 2)),
nearestMonitorInfo.rcWork.top + ((desktopDimensions.height() / 2) - (dimensions.height() / 2))
};
}

if (_logic.IsQuakeWindow())
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
{
Expand All @@ -416,18 +410,25 @@ void AppHost::_HandleCreateWindow(const HWND hwnd, RECT proposedRect, LaunchMode
const til::size availableSpace = desktopDimensions + nonClientSize;

origin = til::point{
nearestMonitorInfo.rcWork.left - (nonClientSize.width() / 2),
nearestMonitorInfo.rcWork.top
::base::saturated_cast<ptrdiff_t>(nearestMonitorInfo.rcWork.left - (nonClientSize.width() / 2)),
Copy link
Member

Choose a reason for hiding this comment

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

I feel like this is weird. We're doing the math unprotected from over/underflow first then casting the result into ptrdiff_t.

Shouldn't we be doing a saturated operation on these things or casting them into the math wrappers first and then let it do the saturated subtract?

The same goes for some of the other saturated casts below. Though I'm not totally clear on what the source types are here that requires us to do the cast to begin with.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea you know, I felt weird writing that. I thinkthe types are LONGs originally. Maybe point is more forgiving than size, which is why I was getting weird "can't use an initializer list" errors.

lemme back that out and spend more than 30 seconds on it

::base::saturated_cast<ptrdiff_t>(nearestMonitorInfo.rcWork.top)
};
dimensions = til::size{
availableSpace.width(),
availableSpace.height() / 2
};
launchMode = LaunchMode::FocusMode;
}
else if (centerOnLaunch)
{
// Move our proposed location into the center of that specific monitor.
origin = til::point{
::base::saturated_cast<ptrdiff_t>(nearestMonitorInfo.rcWork.left + ((desktopDimensions.width() / 2) - (dimensions.width() / 2))),
::base::saturated_cast<ptrdiff_t>(nearestMonitorInfo.rcWork.top + ((desktopDimensions.height() / 2) - (dimensions.height() / 2)))
};
}

const til::rectangle newRect{ origin, dimensions };
// const auto newPos = Viewport::FromDimensions(origin, dimensions);
bool succeeded = SetWindowPos(hwnd,
nullptr,
newRect.left<int>(),
Expand Down
32 changes: 20 additions & 12 deletions src/cascadia/WindowsTerminal/IslandWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ LRESULT IslandWindow::_OnSizing(const WPARAM wParam, const LPARAM lParam)
{
// If we haven't been given the callback that would adjust the dimension,
// then we can't do anything, so just bail out.
return FALSE;
return false;
}

LPRECT winRect = reinterpret_cast<LPRECT>(lParam);
Expand All @@ -182,8 +182,9 @@ LRESULT IslandWindow::_OnSizing(const WPARAM wParam, const LPARAM lParam)
// optional parameter so give two UINTs.
UINT dpix = USER_DEFAULT_SCREEN_DPI;
UINT dpiy = USER_DEFAULT_SCREEN_DPI;
// If this fails, we'll use the default of 96.
GetDpiForMonitor(hmon, MDT_EFFECTIVE_DPI, &dpix, &dpiy);
// If this fails, we'll use the default of 96. I think it can only fail for
// bad parameters, which we won't have, so no big deal.
LOG_IF_FAILED(GetDpiForMonitor(hmon, MDT_EFFECTIVE_DPI, &dpix, &dpiy));

const auto widthScale = base::ClampedNumeric<float>(dpix) / USER_DEFAULT_SCREEN_DPI;
const long minWidthScaled = minimumWidth * widthScale;
Expand All @@ -196,7 +197,7 @@ LRESULT IslandWindow::_OnSizing(const WPARAM wParam, const LPARAM lParam)
auto clientHeight = winRect->bottom - winRect->top - nonClientSize.cy;

// If we're the quake window, prevent resizing on all sides except the
// bottom. This also applies to resising with the Alt+Space menu
// bottom. This also applies to resizing with the Alt+Space menu
if (IsQuakeWindow() && wParam != WMSZ_BOTTOM)
{
// Stuff our current window size into the lParam, and return true. This
Expand Down Expand Up @@ -254,7 +255,7 @@ LRESULT IslandWindow::_OnSizing(const WPARAM wParam, const LPARAM lParam)
break;
}

return TRUE;
return true;
}

// Method Description:
Expand Down Expand Up @@ -929,7 +930,8 @@ void IslandWindow::IsQuakeWindow(bool isQuakeWindow) noexcept
if (_isQuakeWindow != isQuakeWindow)
{
_isQuakeWindow = isQuakeWindow;
if (IsQuakeWindow())
// Don't enter quake mode if we don't have an HWND yet
if (IsQuakeWindow() && _window)
{
_enterQuakeMode();
}
Expand All @@ -938,29 +940,35 @@ void IslandWindow::IsQuakeWindow(bool isQuakeWindow) noexcept

void IslandWindow::_enterQuakeMode()
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
{
if (!_window)
{
return;
}

RECT windowRect = GetWindowRect();
HMONITOR hmon = MonitorFromRect(&windowRect, MONITOR_DEFAULTTONEAREST);
MONITORINFO nearestMonitorInfo;

UINT dpix = USER_DEFAULT_SCREEN_DPI;
UINT dpiy = USER_DEFAULT_SCREEN_DPI;
// If this fails, we'll use the default of 96.
GetDpiForMonitor(hmon, MDT_EFFECTIVE_DPI, &dpix, &dpiy);
// If this fails, we'll use the default of 96. I think it can only fail for
// bad parameters, which we won't have, so no big deal.
LOG_IF_FAILED(GetDpiForMonitor(hmon, MDT_EFFECTIVE_DPI, &dpix, &dpiy));

nearestMonitorInfo.cbSize = sizeof(MONITORINFO);
// Get monitor dimensions:
GetMonitorInfo(hmon, &nearestMonitorInfo);
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
const til::size desktopDimensions{ gsl::narrow<short>(nearestMonitorInfo.rcWork.right - nearestMonitorInfo.rcWork.left),
gsl::narrow<short>(nearestMonitorInfo.rcWork.bottom - nearestMonitorInfo.rcWork.top) };
const til::size desktopDimensions{ ::base::saturated_cast<ptrdiff_t>(nearestMonitorInfo.rcWork.right - nearestMonitorInfo.rcWork.left),
::base::saturated_cast<ptrdiff_t>(nearestMonitorInfo.rcWork.bottom - nearestMonitorInfo.rcWork.top) };

// If we just use rcWork by itself, we'll fail to account for the invisible
// space reserved for the resize handles. So retrieve that size here.
const til::size ncSize{ GetTotalNonClientExclusiveSize(dpix) };
const til::size availableSpace = desktopDimensions + ncSize;

const til::point origin{
::base::saturated_cast<short>(nearestMonitorInfo.rcWork.left - (ncSize.width() / 2)),
::base::saturated_cast<short>(nearestMonitorInfo.rcWork.top)
::base::saturated_cast<ptrdiff_t>(nearestMonitorInfo.rcWork.left - (ncSize.width() / 2)),
::base::saturated_cast<ptrdiff_t>(nearestMonitorInfo.rcWork.top)
};
const til::size dimensions{
availableSpace.width(),
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ int NonClientIslandWindow::_GetResizeHandleHeight() const noexcept
const auto originalRet = DefWindowProc(_window.get(), WM_NCHITTEST, 0, lParam);
if (originalRet != HTCLIENT)
{
// If we're the quake window, supress resizing on any side except the
// If we're the quake window, suppress resizing on any side except the
// bottom. I don't believe that this actually works on the top. That's
// handled below.
if (IsQuakeWindow())
Expand Down