Re: [PATCH] ui/cocoa: Use qemu_add_mouse_change_notifier

[Phil Dennis-Jordan <lists@philjordan.eu>]

This looks fine to me. I've tested it briefly with a graphical Linux
guest and some tracing in the notifyMouseModeChange on a macOS 13
host. When I hot-unplug the usb-tablet I get an absolute -> relative
notification; everything works in relative mode after hot-adding a USB
mouse. Hot-unplugging and replugging the tablet yields a relative ->
absolute notification, and pointer input continues to behave
correctly.

Reviewed-by: Phil Dennis-Jordan <phil@philjordan.eu>
Tested-by: Phil Dennis-Jordan <phil@philjordan.eu>

On Fri, 22 Mar 2024 at 11:55, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> This eliminates the polling in cocoa_refresh and implements the
> propagation of the mouse mode change from absolute to relative.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>  ui/cocoa.m | 62 ++++++++++++++++++++++++++++++++++++++------------------------
>  1 file changed, 38 insertions(+), 24 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 810751cf2644..b53920b8814c 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -310,6 +310,14 @@ @interface QemuCocoaView : NSView
>      NSTrackingArea *trackingArea;
>      QEMUScreen screen;
>      pixman_image_t *pixman_image;
> +    /* The state surrounding mouse grabbing is potentially confusing.
> +     * isAbsoluteEnabled tracks qemu_input_is_absolute() [ie "is the emulated
> +     *   pointing device an absolute-position one?"], but is only updated on
> +     *   next refresh.
> +     * 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.
> +     */
>      BOOL isMouseGrabbed;
>      BOOL isAbsoluteEnabled;
>      CFMachPortRef eventsTap;
> @@ -321,17 +329,8 @@ - (void) setFullGrab:(id)sender;
>  - (void) handleMonitorInput:(NSEvent *)event;
>  - (bool) handleEvent:(NSEvent *)event;
>  - (bool) handleEventLocked:(NSEvent *)event;
> -- (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled;
> -/* The state surrounding mouse grabbing is potentially confusing.
> - * isAbsoluteEnabled tracks qemu_input_is_absolute() [ie "is the emulated
> - *   pointing device an absolute-position one?"], but is only updated on
> - *   next refresh.
> - * 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.
> - */
> +- (void) notifyMouseModeChange;
>  - (BOOL) isMouseGrabbed;
> -- (BOOL) isAbsoluteEnabled;
>  - (QEMUScreen) gscreen;
>  - (void) raiseAllKeys;
>  @end
> @@ -437,6 +436,7 @@ - (void) selectConsoleLocked:(unsigned int)index
>      qkbd_state_switch_console(kbd, con);
>      dcl.con = con;
>      register_displaychangelistener(&dcl);
> +    [self notifyMouseModeChange];
>      [self updateUIInfo];
>  }
>
> @@ -1103,14 +1103,26 @@ - (void) ungrabMouse
>      [self raiseAllButtons];
>  }
>
> -- (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled {
> +- (void) notifyMouseModeChange {
> +    bool tIsAbsoluteEnabled = bool_with_bql(^{
> +        return qemu_input_is_absolute(dcl.con);
> +    });
> +
> +    if (tIsAbsoluteEnabled == isAbsoluteEnabled) {
> +        return;
> +    }
> +
>      isAbsoluteEnabled = tIsAbsoluteEnabled;
> +
>      if (isMouseGrabbed) {
> -        CGAssociateMouseAndMouseCursorPosition(isAbsoluteEnabled);
> +        if (isAbsoluteEnabled) {
> +            [self ungrabMouse];
> +        } else {
> +            CGAssociateMouseAndMouseCursorPosition(isAbsoluteEnabled);
> +        }
>      }
>  }
>  - (BOOL) isMouseGrabbed {return isMouseGrabbed;}
> -- (BOOL) isAbsoluteEnabled {return isAbsoluteEnabled;}
>  - (QEMUScreen) gscreen {return screen;}
>
>  /*
> @@ -1784,6 +1796,17 @@ static void addRemovableDevicesMenuItems(void)
>      qapi_free_BlockInfoList(pointerToFree);
>  }
>
> +static void cocoa_mouse_mode_change_notify(Notifier *notifier, void *data)
> +{
> +    dispatch_async(dispatch_get_main_queue(), ^{
> +        [cocoaView notifyMouseModeChange];
> +    });
> +}
> +
> +static Notifier mouse_mode_change_notifier = {
> +    .notify = cocoa_mouse_mode_change_notify
> +};
> +
>  @interface QemuCocoaPasteboardTypeOwner : NSObject<NSPasteboardTypeOwner>
>  @end
>
> @@ -1968,17 +1991,6 @@ static void cocoa_refresh(DisplayChangeListener *dcl)
>      COCOA_DEBUG("qemu_cocoa: cocoa_refresh\n");
>      graphic_hw_update(dcl->con);
>
> -    if (qemu_input_is_absolute(dcl->con)) {
> -        dispatch_async(dispatch_get_main_queue(), ^{
> -            if (![cocoaView isAbsoluteEnabled]) {
> -                if ([cocoaView isMouseGrabbed]) {
> -                    [cocoaView ungrabMouse];
> -                }
> -            }
> -            [cocoaView setAbsoluteEnabled:YES];
> -        });
> -    }
> -
>      if (cbchangecount != [[NSPasteboard generalPasteboard] changeCount]) {
>          qemu_clipboard_info_unref(cbinfo);
>          cbinfo = qemu_clipboard_info_new(&cbpeer, QEMU_CLIPBOARD_SELECTION_CLIPBOARD);
> @@ -2055,6 +2067,8 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
>
>      // register vga output callbacks
>      register_displaychangelistener(&dcl);
> +    qemu_add_mouse_mode_change_notifier(&mouse_mode_change_notifier);
> +    [cocoaView notifyMouseModeChange];
>      [cocoaView updateUIInfo];
>
>      qemu_event_init(&cbevent, false);
>
> ---
> base-commit: fea445e8fe9acea4f775a832815ee22bdf2b0222
> change-id: 20240322-mouse-bbc0fa90e8dc
>
> Best regards,
> --
> Akihiko Odaki <akihiko.odaki@daynix.com>
>
>