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

Fix cursor hiding on input #18445

Merged
merged 2 commits into from
Jan 21, 2025
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions .github/actions/spelling/expect/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1354,6 +1354,7 @@ PNMLINK
pntm
POBJECT
Podcast
POINTERUPDATE
POINTSLIST
policheck
POLYTEXTW
Expand Down
62 changes: 0 additions & 62 deletions src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,66 +47,6 @@ constexpr std::wstring_view StateCollapsed{ L"Collapsed" };
DEFINE_ENUM_FLAG_OPERATORS(winrt::Microsoft::Terminal::Control::CopyFormat);
DEFINE_ENUM_FLAG_OPERATORS(winrt::Microsoft::Terminal::Control::MouseButtonState);

// WinUI 3's UIElement.ProtectedCursor property allows someone to set the cursor on a per-element basis.
// This would allow us to hide the cursor when the TermControl has input focus and someone starts typing.
// Unfortunately, no equivalent exists for WinUI 2 so we fake it with the CoreWindow.
// There are 3 downsides:
// * SetPointerCapture() is global state and may interfere with other components.
// * You can't start dragging the cursor (for text selection) while it's still hidden.
// * The CoreWindow covers the union of all window rectangles, so the cursor is hidden even if it's outside
// the current foreground window, but still on top of another Terminal window in the background.
static void hideCursorUntilMoved()
{
static bool cursorIsHidden;
static const auto shouldVanish = []() {
BOOL shouldVanish = TRUE;
SystemParametersInfoW(SPI_GETMOUSEVANISH, 0, &shouldVanish, 0);
if (!shouldVanish)
{
return false;
}

const auto window = CoreWindow::GetForCurrentThread();
static constexpr auto releaseCapture = [](CoreWindow window, PointerEventArgs) {
if (cursorIsHidden)
{
window.ReleasePointerCapture();
}
};
static constexpr auto restoreCursor = [](CoreWindow window, PointerEventArgs) {
if (cursorIsHidden)
{
cursorIsHidden = false;
window.PointerCursor(CoreCursor{ CoreCursorType::Arrow, 0 });
}
};

winrt::Windows::Foundation::TypedEventHandler<CoreWindow, PointerEventArgs> releaseCaptureHandler{ releaseCapture };
std::ignore = window.PointerMoved(releaseCaptureHandler);
std::ignore = window.PointerPressed(releaseCaptureHandler);
std::ignore = window.PointerReleased(releaseCaptureHandler);
std::ignore = window.PointerWheelChanged(releaseCaptureHandler);
std::ignore = window.PointerCaptureLost(restoreCursor);
return true;
}();

if (shouldVanish && !cursorIsHidden)
{
try
{
const auto window = CoreWindow::GetForCurrentThread();
window.PointerCursor(nullptr);
window.SetPointerCapture();
cursorIsHidden = true;
}
catch (...)
{
// Swallow the 0x80070057 "Failed to get pointer information." exception that randomly occurs.
// Curiously, it doesn't happen during the PointerCursor() but during the SetPointerCapture() call (thanks, WinUI).
}
}
}

// InputPane::GetForCurrentView() does not reliably work for XAML islands,
// as it assumes that there's a 1:1 relationship between windows and threads.
//
Expand Down Expand Up @@ -1551,8 +1491,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
return;
}

hideCursorUntilMoved();

const auto ch = e.Character();
const auto keyStatus = e.KeyStatus();
const auto scanCode = gsl::narrow_cast<WORD>(keyStatus.ScanCode);
Expand Down
47 changes: 47 additions & 0 deletions src/cascadia/WindowsTerminal/IslandWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,48 @@ using VirtualKeyModifiers = winrt::Windows::System::VirtualKeyModifiers;
#define XAML_HOSTING_WINDOW_CLASS_NAME L"CASCADIA_HOSTING_WINDOW_CLASS"
#define IDM_SYSTEM_MENU_BEGIN 0x1000

// WinUI doesn't support the "Hide cursor on input" setting which is why all the modern Windows apps
// are broken in that respect. We want to support it though, so we implement an imitation of it here.
// We use the classic ShowCursor() API to hide it on keydown and show it on a select few messages.
// WinUI's SetPointerCapture() cannot be used for this, because it races with internal WinUI code
// calling that function and has proven itself to be very unreliable in practice.
//
// With UWP half the input stack got split off, and so most input events get rerouted through
// the CoreInput child window running in another thread (aka InputHost aka Windows.UI.Input).
// HideCursor() is called by WindowEmperor because WM_KEYDOWN is otherwise sent directly to that
// CoreInput window. Same for WM_POINTERUPDATE which we use to reliably detect cursor movement.
// WM_ACTIVATE on the other hand is only sent to each specific window and cannot be hooked by
// inspecting the MSG struct coming from GetMessage. That's why the code must be here.
// Best not think about this too much...
bool IslandWindow::IsCursorHidden() noexcept
{
return _cursorHidden;
}

void IslandWindow::HideCursor() noexcept
{
static const auto shouldVanish = []() noexcept {
BOOL shouldVanish = TRUE;
SystemParametersInfoW(SPI_GETMOUSEVANISH, 0, &shouldVanish, 0);
return shouldVanish != FALSE;
}();

if (!_cursorHidden && shouldVanish)
{
ShowCursor(FALSE);
_cursorHidden = true;
}
}

void IslandWindow::ShowCursorMaybe(const UINT message) noexcept
{
if (_cursorHidden && (message == WM_ACTIVATE || message == WM_POINTERUPDATE))
Copy link
Member

Choose a reason for hiding this comment

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

do you need one for losing focus or "switching to other window"?

Copy link
Member

Choose a reason for hiding this comment

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

should we hide the cursor only when we are active? (so that somebody sending us a WM_KEYDOWN remotely doesn't hide the cursor?)

Copy link
Member Author

@lhecker lhecker Jan 21, 2025

Choose a reason for hiding this comment

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

My assumption was that WM_ACTIVATE is a superset of WM_SETFOCUS. Is that not the case? (Not sure how to test that because I always focus a window by activating it…)

I think that hiding the cursor on an external/emulated WM_KEYDOWN is fine since the sender must've had a reason to send WM_KEYDOWN specifically and I think it should behave like any other keyboard input.

Copy link
Member

Choose a reason for hiding this comment

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

fair enough

{
_cursorHidden = false;
ShowCursor(TRUE);
}
}

IslandWindow::IslandWindow() noexcept :
_interopWindowHandle{ nullptr },
_rootGrid{ nullptr },
Expand Down Expand Up @@ -425,6 +467,11 @@ void IslandWindow::_OnGetMinMaxInfo(const WPARAM /*wParam*/, const LPARAM lParam

[[nodiscard]] LRESULT IslandWindow::MessageHandler(UINT const message, WPARAM const wparam, LPARAM const lparam) noexcept
{
if (IsCursorHidden())
{
ShowCursorMaybe(message);
}

switch (message)
{
case WM_GETMINMAXINFO:
Expand Down
8 changes: 7 additions & 1 deletion src/cascadia/WindowsTerminal/IslandWindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ class IslandWindow :
public BaseWindow<IslandWindow>
{
public:
static bool IsCursorHidden() noexcept;
static void HideCursor() noexcept;
static void ShowCursorMaybe(const UINT message) noexcept;

IslandWindow() noexcept;
virtual ~IslandWindow() override;

Expand Down Expand Up @@ -143,7 +147,7 @@ class IslandWindow :
bool _minimizeToNotificationArea{ false };

std::unordered_map<UINT, SystemMenuItemInfo> _systemMenuItems;
UINT _systemMenuNextItemId;
UINT _systemMenuNextItemId = 0;
Copy link
Member Author

Choose a reason for hiding this comment

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

This member was left uninitialized.

void _resetSystemMenu();

private:
Expand All @@ -154,4 +158,6 @@ class IslandWindow :
// though the total height will take into account the non-client area
// and the requirements of components hosted in the client area
static constexpr float minimumHeight = 0;

inline static bool _cursorHidden;
};
10 changes: 10 additions & 0 deletions src/cascadia/WindowsTerminal/WindowEmperor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,16 @@ void WindowEmperor::HandleCommandlineArgs(int nCmdShow)
_dispatchSpecialKey(msg);
continue;
}

if (msg.message == WM_KEYDOWN)
Copy link
Member

Choose a reason for hiding this comment

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

or syskeydown right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh I missed this comment. I think it should not be done on SYSKEYDOWN since that's for Alt-key combinations, which also doesn't hide the cursor in classic applications.

{
IslandWindow::HideCursor();
}
}

if (IslandWindow::IsCursorHidden())
{
IslandWindow::ShowCursorMaybe(msg.message);
}

TranslateMessage(&msg);
Expand Down
Loading