feat: Enable 'MouseKeys' on screen setup and never restore old settings

Previously we were toggling the 'MouseKeys' Windows accessibility feature on and off, but this was causing many bugs; cursor show delay while Windows brings up the mouse stack(?) and often forever lost mouse cursor when toggling off the 'MouseKeys' feature. After a few days of hacking away at this, it seems safest to just leave it on.

src/lib/platform/MSWindowsDesks.cpp

@@ -452,12 +452,11 @@ void MSWindowsDesks::deskMouseRelativeMove(int32_t dx, int32_t dy) const
   }
 }
 
-// the system shows the mouse cursor when an internal display count
+/*!
-// is >= 0.  this count is maintained per application but there's
+ * Wraps the `ShowCursor` function and calls it repeatedly until the cursor visibility is at
-// apparently a system wide count added to the application's count.
+ * the desired state. Windows maintains an internal counter for cursor visibility, and only
-// this system count is 0 if there's a mouse attached to the system
+ * shows or hides the cursor when it reaches a certain threshold.
-// and -1 otherwise.  the mouse keys accessibility feature can modify
+ */
-// this system count by making the system appear to have a mouse.
 void setCursorVisibility(bool visible)
 {
   LOG_DEBUG("%s cursor", visible ? "showing" : "hiding");

src/lib/platform/MSWindowsScreen.cpp

@@ -176,6 +176,7 @@ MSWindowsScreen::getWindowInstance()
 void MSWindowsScreen::enable()
 {
   LOG_DEBUG("enabling %s screen", m_isPrimary ? "primary" : "secondary");
+  m_isEnabled = true;
 
   assert(m_isOnScreen == m_isPrimary);
 
@@ -203,6 +204,7 @@ void MSWindowsScreen::enable()
 void MSWindowsScreen::disable()
 {
   LOG_DEBUG("disabling %s screen", m_isPrimary ? "primary" : "secondary");
+  m_isEnabled = false;
 
   // stop tracking the active desk
   m_desks->disable();
@@ -231,7 +233,6 @@ void MSWindowsScreen::disable()
   }
 
   m_isOnScreen = m_isPrimary;
-  setupMouseKeys();
 }
 
 void MSWindowsScreen::enter()
@@ -316,7 +317,6 @@ void MSWindowsScreen::leave()
 
   // now off screen
   m_isOnScreen = false;
-  setupMouseKeys();
 }
 
 bool MSWindowsScreen::setClipboard(ClipboardID, const IClipboard *src)
@@ -983,12 +983,22 @@ bool MSWindowsScreen::onEvent(HWND, UINT msg, WPARAM wParam, LPARAM lParam, LRES
     *result = TRUE;
     return true;
 
-  case WM_DEVICECHANGE:
+  case WM_DEVICECHANGE: {
+    // re-run mouse keys setup in case a mouse was plugged in or unplugged; i.e. if a mouse was
+    // unplugged from the client, make sure the mouse cursor is still visible.
+    // the device change event happens for every discreet hardware change, so if you're using a
+    // usb switcher, this generates many device change events. it would be nice to log here but
+    // the log would be too noisy.
     setupMouseKeys();
-    break;
+  } break;
 
   case WM_SETTINGCHANGE:
+    // sometimes fired when the mouse keys setting is changed, but doesn't seem very reliable.
+    // these events may arrive at any time (e.g. when the program is shutting down) if the message
+    // loop stops being processed for any reason. this may be a bug or something out of our control.
+    // forcing mouse keys on may help in scenarios where mouse keys are being turned off by another app.
     if (wParam == SPI_SETMOUSEKEYS) {
+      LOG_DEBUG("mouse keys setting was changed");
       setupMouseKeys();
     }
     break;
@@ -1586,47 +1596,78 @@ void MSWindowsScreen::updateKeysCB(void *)
 
 void MSWindowsScreen::setupMouseKeys()
 {
-  // check for mouse
+  // we only need to enable the mouse keys feature when on a secondary screen.
-  m_hasMouse = (GetSystemMetrics(SM_MOUSEPRESENT) != 0);
+  // this tricks windows into showing the mouse cursor when there is no real mouse.
-
+  if (m_isPrimary) {
-  // decide if we should show the mouse
+    // silent return to avoid noise.
-  bool showMouse = (!m_hasMouse && !m_isPrimary && m_isOnScreen);
+    return;
-
-  // show/hide the mouse
-  if (showMouse != m_showingMouse) {
-    if (showMouse) {
-      m_oldMouseKeys.cbSize = sizeof(m_oldMouseKeys);
-      m_gotOldMouseKeys = (SystemParametersInfo(SPI_GETMOUSEKEYS, m_oldMouseKeys.cbSize, &m_oldMouseKeys, 0) != 0);
-      if (m_gotOldMouseKeys) {
-        m_mouseKeys = m_oldMouseKeys;
-        m_showingMouse = true;
-        updateMouseKeys();
-      }
-    } else {
-      if (m_gotOldMouseKeys) {
-        SystemParametersInfo(SPI_SETMOUSEKEYS, m_oldMouseKeys.cbSize, &m_oldMouseKeys, SPIF_SENDCHANGE);
-        m_showingMouse = false;
-      }
-    }
   }
+
+  // this is the case when there is some kind of a mouse (real or simulated by mouse keys).
+  m_hasMouse = (GetSystemMetrics(SM_MOUSEPRESENT) != 0);
+  if (m_hasMouse) {
+    // silent return to avoid noise.
+    return;
+  }
+
+  // prevents mouse keys being configured again when the program is shutting down since this function
+  // is called based on system events such as system setting changes or hardware changes which
+  // can occur at any time.
+  if (!m_isEnabled) {
+    LOG_DEBUG("mouse keys setup skipped, screen is not enabled");
+    return;
+  }
+
+  m_mouseKeys.cbSize = sizeof(m_mouseKeys);
+  m_gotMouseKeys = (SystemParametersInfo(SPI_GETMOUSEKEYS, m_mouseKeys.cbSize, &m_mouseKeys, 0) != 0);
+  if (!m_gotMouseKeys) {
+    LOG_ERR("unable to get old mouse keys settings, error: %d", GetLastError());
+    return;
+  }
+
+  updateMouseKeys();
 }
 
 void MSWindowsScreen::updateMouseKeys()
 {
-  DWORD oldFlags = m_mouseKeys.dwFlags;
+  if (m_hasMouse || !m_gotMouseKeys || m_isPrimary) {
-
+    // silent return to avoid noise.
-  // turn on MouseKeys
+    return;
-  m_mouseKeys.dwFlags = MKF_AVAILABLE | MKF_MOUSEKEYSON;
-
-  // make sure MouseKeys is active in whatever state the NumLock is
-  // not currently in.
-  if ((m_keyState->getActiveModifiers() & KeyModifierNumLock) != 0) {
-    m_mouseKeys.dwFlags |= MKF_REPLACENUMBERS;
   }
 
-  // update MouseKeys
+  DWORD oldFlags = m_mouseKeys.dwFlags;
-  if (oldFlags != m_mouseKeys.dwFlags) {
+
-    SystemParametersInfo(SPI_SETMOUSEKEYS, m_mouseKeys.cbSize, &m_mouseKeys, SPIF_SENDCHANGE);
+  // turn on the windows mouse keys accessibility feature.
+  // this is referred to as 'MouseKeys' in the docs.
+  // makes the mouse cursor visible if there is no real mouse.
+  //
+  // historically, we would only set the `MKF_REPLACENUMBERS` flag when num lock is on.
+  // however, this was a strange hidden feature that the user will most likely not expect;
+  // it's probably more sensible to always set the `MKF_REPLACENUMBERS` flag, so that when the
+  // mouse keys feature is left on after the program exits, the num pad on a local keyboard still
+  // types numbers instead of moving the mouse cursor around (which would surprise most users).
+  //
+  // by default, windows 11 shows the mouse keys status in the system tray, but turning this on
+  // might actually cause confusion for users who are not familiar with the mouse keys feature.
+  m_mouseKeys.dwFlags = MKF_AVAILABLE | MKF_MOUSEKEYSON | MKF_REPLACENUMBERS;
+
+  // only update the mouse keys settings if different to avoid noise.
+  if (oldFlags == m_mouseKeys.dwFlags) {
+    // silent return to avoid noise.
+    return;
+  }
+
+  // we used to restore the old mouse keys settings but toggling the mouse keys feature on and off
+  // causes the mouse cursor to be come stuck in an invisible state even when there is a real mouse.
+  // we may want to reintroduce it (restore old mouse keys flags) as a user option in the future,
+  // e.g. for users who use periodically use their windows client directly and use the numpad for cursor keys.
+  LOG_INFO("enabling mouse keys for cursor visibility");
+  LOG_DEBUG("setting mouse keys flags: 0x%08x", m_mouseKeys.dwFlags);
+  const auto ok = SystemParametersInfo(SPI_SETMOUSEKEYS, m_mouseKeys.cbSize, &m_mouseKeys, SPIF_SENDCHANGE);
+  if (!ok) {
+    LOG_ERR("failed to set mouse keys, error: %d", GetLastError());
+  } else {
+    LOG_DEBUG1("mouse keys enabled successfully");
   }
 }
 

src/lib/platform/MSWindowsScreen.h

@@ -209,14 +209,16 @@ private: // HACK
   // job to update the key state
   void updateKeysCB(void *);
 
-  // determine whether the mouse is hidden by the system and force
+  // determine whether the mouse is hidden by the system.
-  // it to be displayed if user has entered this secondary screen.
+  // if true and on secondary screen, enable mouse keys to show the cursor.
+  // we were previously restoring the old mouse key settings when not needed, but this was causing
+  // issues where the mouse cursor becomes permanently hidden, even if there is a real mouse
+  // attached to the system; this could be a windows bug, but losing your mouse is a nightmare
+  // so we shouldn't risk doing that.
   void setupMouseKeys();
 
-  // setupMouseKeys uses MouseKeys to show the cursor.  since we
+  // enables the mouse keys accessibility feature to to ensure the
-  // don't actually want MouseKeys behavior we have to make sure
+  // mouse cursor can be shown.
-  // it applies when NumLock is in whatever state it's not in now.
-  // this method does that.
   void updateMouseKeys();
 
   // our window proc
@@ -258,6 +260,9 @@ private:
   // true if mouse has entered the screen
   bool m_isOnScreen;
 
+  // true if the screen is enabled
+  bool m_isEnabled = false;
+
   // our resources
   ATOM m_class = 0;
 
@@ -317,22 +322,13 @@ private:
   // map of button state
   bool m_buttons[1 + kButtonExtra0 + 1];
 
-  // the system shows the mouse cursor when an internal display count
+  // m_hasMouse is true if there's a mouse attached to the system or
-  // is >= 0.  this count is maintained per application but there's
+  // mouse keys is simulating one.  we track this so we can force the
-  // apparently a system wide count added to the application's count.
-  // this system count is 0 if there's a mouse attached to the system
-  // and -1 otherwise.  the MouseKeys accessibility feature can modify
-  // this system count by making the system appear to have a mouse.
-  //
-  // m_hasMouse is true iff there's a mouse attached to the system or
-  // MouseKeys is simulating one.  we track this so we can force the
   // cursor to be displayed when the user has entered this screen.
-  // m_showingMouse is true when we're doing that.
   bool m_hasMouse;
-  bool m_showingMouse = false;
+
-  bool m_gotOldMouseKeys;
+  bool m_gotMouseKeys = false;
   MOUSEKEYS m_mouseKeys;
-  MOUSEKEYS m_oldMouseKeys;
 
   MSWindowsHook m_hook;