[v2] ui/cocoa: Fix mouse association state

Commit Message

Akihiko Odaki Feb. 22, 2021, 3:07 p.m. UTC

ui/cocoa deassociates the mouse input and the mouse cursor
position only when relative movement inputs are expected. Such
inputs may let the mouse cursor leave the view and cause undesired
side effects if they are associated. On the other hand, the
problem does not occur when inputting absolute points, and the
association allows seamless cursor movement across views.

However, the synchronization of the association and the expected
input type was only done when grabbing the mouse. In reality, the
state whether the emulated input device expects absolute pointing
inputs or relative movement inputs can vary dynamically due to
USB device hot-plugging, for example.

This change adds association state updates according to input type
expectation changes. It also removes an internal flag representing
the association state because the state can now be determined with
the current input type expectation and it only adds the
complexity of the state tracking.

Signed-off-by: Akihiko Odaki <[e-mail]>
---
 ui/cocoa.m | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

Comments

Gerd Hoffmann Feb. 24, 2021, 11:27 a.m. UTC | #1

On Tue, Feb 23, 2021 at 12:07:14AM +0900, Akihiko Odaki wrote:
> ui/cocoa deassociates the mouse input and the mouse cursor
> position only when relative movement inputs are expected. Such
> inputs may let the mouse cursor leave the view and cause undesired
> side effects if they are associated. On the other hand, the
> problem does not occur when inputting absolute points, and the
> association allows seamless cursor movement across views.
> 
> However, the synchronization of the association and the expected
> input type was only done when grabbing the mouse. In reality, the
> state whether the emulated input device expects absolute pointing
> inputs or relative movement inputs can vary dynamically due to
> USB device hot-plugging, for example.
> 
> This change adds association state updates according to input type
> expectation changes. It also removes an internal flag representing
> the association state because the state can now be determined with
> the current input type expectation and it only adds the
> complexity of the state tracking.

Patch looks reasonable to me, but I'd like to see an review from
someone who knows macos better than I do for this one.

thanks,
  Gerd

Gerd Hoffmann March 11, 2021, 8:58 a.m. UTC | #2

On Tue, Feb 23, 2021 at 12:07:14AM +0900, Akihiko Odaki wrote:
> ui/cocoa deassociates the mouse input and the mouse cursor
> position only when relative movement inputs are expected. Such
> inputs may let the mouse cursor leave the view and cause undesired
> side effects if they are associated. On the other hand, the
> problem does not occur when inputting absolute points, and the
> association allows seamless cursor movement across views.
> 
> However, the synchronization of the association and the expected
> input type was only done when grabbing the mouse. In reality, the
> state whether the emulated input device expects absolute pointing
> inputs or relative movement inputs can vary dynamically due to
> USB device hot-plugging, for example.
> 
> This change adds association state updates according to input type
> expectation changes. It also removes an internal flag representing
> the association state because the state can now be determined with
> the current input type expectation and it only adds the
> complexity of the state tracking.
> 
> Signed-off-by: Akihiko Odaki <[e-mail]>

Added to usb queue.

thanks,
  Gerd

Patch

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 0ef5fdf3b7a..671af40b252 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -310,7 +310,6 @@  @interface QemuCocoaView : NSView
     BOOL isMouseGrabbed;
     BOOL isFullscreen;
     BOOL isAbsoluteEnabled;
-    BOOL isMouseDeassociated;
 }
 - (void) switchSurface:(pixman_image_t *)image;
 - (void) grabMouse;
@@ -327,14 +326,9 @@  - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled;
  * isMouseGrabbed tracks whether GUI events are directed to the guest;
  *   it controls whether special keys like Cmd get sent to the guest,
  *   and whether we capture the mouse when in non-absolute mode.
- * isMouseDeassociated tracks whether we've told MacOSX to disassociate
- *   the mouse and mouse cursor position by calling
- *   CGAssociateMouseAndMouseCursorPosition(FALSE)
- *   (which basically happens if we grab in non-absolute mode).
  */
 - (BOOL) isMouseGrabbed;
 - (BOOL) isAbsoluteEnabled;
-- (BOOL) isMouseDeassociated;
 - (float) cdx;
 - (float) cdy;
 - (QEMUScreen) gscreen;
@@ -974,10 +968,7 @@  - (void) grabMouse
             [normalWindow setTitle:@"QEMU - (Press ctrl + alt + g to release Mouse)"];
     }
     [self hideCursor];
-    if (!isAbsoluteEnabled) {
-        isMouseDeassociated = TRUE;
-        CGAssociateMouseAndMouseCursorPosition(FALSE);
-    }
+    CGAssociateMouseAndMouseCursorPosition(isAbsoluteEnabled);
     isMouseGrabbed = TRUE; // while isMouseGrabbed = TRUE, QemuCocoaApp sends all events to [cocoaView handleEvent:]
 }
 
@@ -992,17 +983,18 @@  - (void) ungrabMouse
             [normalWindow setTitle:@"QEMU"];
     }
     [self unhideCursor];
-    if (isMouseDeassociated) {
-        CGAssociateMouseAndMouseCursorPosition(TRUE);
-        isMouseDeassociated = FALSE;
-    }
+    CGAssociateMouseAndMouseCursorPosition(TRUE);
     isMouseGrabbed = FALSE;
 }
 
-- (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled {isAbsoluteEnabled = tIsAbsoluteEnabled;}
+- (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled {
+    isAbsoluteEnabled = tIsAbsoluteEnabled;
+    if (isMouseGrabbed) {
+        CGAssociateMouseAndMouseCursorPosition(isAbsoluteEnabled);
+    }
+}
 - (BOOL) isMouseGrabbed {return isMouseGrabbed;}
 - (BOOL) isAbsoluteEnabled {return isAbsoluteEnabled;}
-- (BOOL) isMouseDeassociated {return isMouseDeassociated;}
 - (float) cdx {return cdx;}
 - (float) cdy {return cdy;}
 - (QEMUScreen) gscreen {return screen;}