Re: [Qemu-devel] [PATCH v2] ui/cocoa.m: fix sending mouse event to guest

[Programmingkid]

On Jan 6, 2017, at 11:19 AM, Peter Maydell wrote:

> On 6 January 2017 at 15:44, Programmingkid <address@hidden> wrote:
>> 
>> On Jan 6, 2017, at 8:48 AM, Peter Maydell wrote:
>> 
>>> On 1 January 2017 at 21:31, Programmingkid <address@hidden> wrote:
>>>> The mouse down event should not be sent to the guest if the mouse down 
>>>> event
>>>> causes QEMU to move the foreground from the background. This patch prevents
>>>> these activation clicks from going to the guest.
> 
>>>>    if (mouse_event) {
>>>>        /* Don't send button events to the guest unless we've got a
>>>> -         * mouse grab or window focus. If we have neither then this event
>>>> -         * is the user clicking on the background window to activate and
>>>> -         * bring us to the front, which will be done by the sendEvent
>>>> -         * call below. We definitely don't want to pass that click through
>>>> -         * to the guest.
>>>> +         * mouse grab, window focus, or in fullscreen mode. If we have 
>>>> neither
>>>> +         * then this event is the user clicking on the background window 
>>>> to
>>>> +         * activate and bring us to the front, which will be done by the
>>>> +         * sendEvent call below. We definitely don't want to pass that 
>>>> click
>>>> +         * through to the guest.
>>>>         */
>>>> -        if ((isMouseGrabbed || [[self window] isKeyWindow]) &&
>>>> +        if (isMouseGrabbed && ([[self window] isKeyWindow] || 
>>>> isFullscreen) &&
>>>>            (last_buttons != buttons)) {
>>>>            static uint32_t bmap[INPUT_BUTTON__MAX] = {
>>>>                [INPUT_BUTTON_LEFT]       = MOUSE_EVENT_LBUTTON,
>>> 
>>> Comment doesn't match code. The comment says "mouse grab, full screen
>>> or focus", but the code is doing "mouse grab and (full screen or focus)".
>>> 
>>> Which is correct?
>> 
>> 
>> This sounds more accurate:
>> 
>> (mouse grab and main window is the front window) or QEMU is in fullscreen.
> 
> That's not what your code does either.

Yes, you are right. Didn't see the parentheses. This is what it does:
mouse is grabbed and either QEMU window is front window or QEMU is in 
fullscreen.

The click that causes QEMU to grab the mouse should not be sent to the guest 
because the
host mouse could be on one part of the screen, and the guest mouse could be in 
another
part of the screen the user isn't aware of yet.

> What is the actual situation which we currently get wrong and need
> to get right?

When the user clicks on QEMU's window to bring QEMU to the front, that 
activation
click should not be sent to the guest. I have had a lot of problems without this
patch. When the activation click was wrongly sent to the guest, I'm had stuff 
happen like volumes unmount because the guest mouse cursor was over an eject 
button.

> Your commit message says "if the event causes QEMU
> to move to the foreground", but my expectation was that if we
> weren't in the foreground then we would not have the mouse grabbed,
> and in any case your patch adds a check on isFullscreen but your
> commit message doesn't say anything about full-screen or not being
> relevant to the problem.

I know the code works correctly, it is just the commit message that needs 
tweaking.