[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH] ui/cocoa.m: fix sending mouse event to guest
Date: Sun, 3 Apr 2016 13:21:43 +0100

On 2 April 2016 at 18:53, Programmingkid <address@hidden> wrote:
> On Apr 2, 2016, at 1:35 PM, Peter Maydell wrote:
>> On 2 April 2016 at 18:25, Programmingkid <address@hidden> wrote:
>>> On Apr 2, 2016, at 1:07 PM, Peter Maydell wrote:
>>>> On 2 April 2016 at 17:56, Programmingkid <address@hidden> wrote:
>>>>> The mouse down event should not be sent to the guest if the mouse down 
>>>>> event
>>>>> causes an activation of QEMU. This patch prevents activation clicks from 
>>>>> going
>>>>> to the guest.
>>>>> Signed-off-by: John Arbuckle <address@hidden>
>>>>> ---
>>>>> ui/cocoa.m | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>>>>> index 60a7c07..07d9c86 100644
>>>>> --- a/ui/cocoa.m
>>>>> +++ b/ui/cocoa.m
>>>>> @@ -698,7 +698,7 @@ QemuCocoaView *cocoaView;
>>>>>         * 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]) &&
>>>>>            (last_buttons != buttons)) {
>>>>>            static uint32_t bmap[INPUT_BUTTON__MAX] = {
>>>>>                [INPUT_BUTTON_LEFT]       = MOUSE_EVENT_LBUTTON,
>>>>> --
>>>>> 2.7.2
>>>> I'm afraid I don't really understand why you think this
>>>> should change. On the face of it the current code looks right:
>>>> we pass through the mouse button if:
>>>>  (1) we've got the mouse grab
>>>> or (2) our window has the focus, even if it's not grabbed
>>>> I would expect the "activation click" to be "we don't have
>>>> the mouse grab, and we don't have focus either (some other
>>>> app is foreground)".
>>> When QEMU's main window is in the background and the user clicks on it,
>>> the NSLeftMouseUp case in the handleEvent: method will set the
>>> isMouseGrabbed variable to true. This means the
>>> "if ((isMouseGrabbed || [[self window] isKeyWindow]) &&
>>> (last_buttons != buttons))" code will always be true for a left
>>> mouse button click. The mouse event will be sent to the guest
>>> when it shouldn't be.
>> OK, that sounds like a bug, but this doesn't look like the
>> right way to fix it, because with your change we won't
>> pass through the click if this is a click on the window
>> when it's not in the background.
> I think logically this is the best solution. If the window is in
> the foreground and doesn't have the mouse grabbed, the user can't
> move the mouse pointer in the guest. This could cause stuff to be
> clicked that might cause undesirable effects.

So why do we now have a different condition for "don't pass
through buttons" and "don't pass through mouse events" ?

The process for making changes should be something like:
 * identify the model we are trying to use to decide whether
   to pass events through to the guest or not
 * if this model is wrong (ie there is a design flaw),
   identify what the changes to the design need to be
   (possibly looking at other OSX VM UIs and how we handle
   similar cases in our other UI frontends, to identify what
   might be reasonable behaviour), so that we have a coherent
   design which we can use now and in the future to fix the code
 * work out where the code is diverging from the design
   (either because the code is buggy or because we made a
   change to the design in the previous step)
 * change the code
 * make sure that comments match the changed code/design

I don't have strong feelings about what the right design for
handling mouse events should be, but I do think we should
have one, and this patch seems to be making a local change
rather than considering the larger picture.

-- PMM

reply via email to

[Prev in Thread] Current Thread [Next in Thread]