| Message ID | 20180608200150.7032-1-programmingkidx@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | ui/cocoa.m: fix absolute mode | expand |
On 8 June 2018 at 21:01, John Arbuckle <programmingkidx@gmail.com> wrote: > Fix the cocoa front-end to correctly be able to use absolute mode. > > Signed-off-by: John Arbuckle <programmingkidx@gmail.com> This commit message is insufficiently detailed, which makes it hard to review the patch. When you send patches, please can you provide commit messages which describe what the problem is, where the current code gets things wrong and why the changes fix the issue? It is painful to have to reverse-engineer your design from your code changes. thanks -- PMM
On Jun 18, 2018, at 7:18 AM, Peter Maydell wrote: > On 8 June 2018 at 21:01, John Arbuckle <programmingkidx@gmail.com> wrote: >> Fix the cocoa front-end to correctly be able to use absolute mode. >> >> Signed-off-by: John Arbuckle <programmingkidx@gmail.com> > > This commit message is insufficiently detailed, which makes > it hard to review the patch. When you send patches, please > can you provide commit messages which describe what the > problem is, where the current code gets things wrong and > why the changes fix the issue? It is painful to have to > reverse-engineer your design from your code changes. Sorry I will correct this problem now. This should have been the commit message: When using an usb-tablet device in QEMU, the Cocoa UI uses absolute mode code to handle mouse events. The absolute mode code does not correctly determine where QEMU's window is located. Tests indicate the current code thinks the window is located in the bottom left of the screen. This makes grabbing and ungrabbing happen at the wrong place. This patch fixes this problem by making mouse grabs happen only in the window and mouse ungrabs happen only when the mouse is moved outside the window.
On 18 June 2018 at 23:24, Programmingkid <programmingkidx@gmail.com> wrote: > When using an usb-tablet device in QEMU, the Cocoa UI uses absolute >mode code to handle mouse events. The absolute mode code does not >correctly determine where QEMU's window is located. Tests indicate >the current code thinks the window is located in the bottom left >of the screen. I guess this is happening because of the note in the NSEvent locationInWindow method docs that the event can have a nil window, in which case it gives you the event location in screen coordinates: https://developer.apple.com/documentation/appkit/nsevent/1529068-locationinwindow Those docs also say that the common way to deal with this is to use the NSView convert method to get the window-relative coordinates that you want. Maybe we just need to do that? thanks -- PMM
On 8 June 2018 at 21:01, John Arbuckle <programmingkidx@gmail.com> wrote: > Fix the cocoa front-end to correctly be able to use absolute mode. > > Signed-off-by: John Arbuckle <programmingkidx@gmail.com> > case NSEventTypeMouseMoved: > if (isAbsoluteEnabled) { > - if (![self screenContainsPoint:p] || ![[self window] isKeyWindow]) { > + if ([self mouseInWindow]) { > + mouse_event = true; > + } > + > + if (![self mouseInWindow] || ![[self window] isKeyWindow]) { > if (isMouseGrabbed) { > [self ungrabMouse]; > } > } else { > - if (!isMouseGrabbed) { > + if (!isMouseGrabbed && [self mouseInWindow]) { > [self grabMouse]; > } We may well not need this part of the patch, but: this change of condition from "!isMouseGrabbed" to "!isMouseGrabbed && [self mouseInWindow]" is unnecessary. This code is inside the "else" block for the preceding "if (![self mouseInWindow] || ![[self window] isKeyWindow])", so it is not possible to reach it if "[self mouseInWindow]" returns false. thanks -- PMM
diff --git a/ui/cocoa.m b/ui/cocoa.m index 2991ed4f19..dda99ad638 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -350,9 +350,20 @@ - (BOOL) isOpaque return YES; } -- (BOOL) screenContainsPoint:(NSPoint) p +/* Returns YES if the host mouse cursor is in the QEMU window, NO otherwise */ +- (BOOL) mouseInWindow { - return (p.x > -1 && p.x < screen.width && p.y > -1 && p.y < screen.height); + NSPoint p = [NSEvent mouseLocation]; + BOOL return_value = NO; + float x, y, width, height; + x = [normalWindow frame].origin.x; + y = [normalWindow frame].origin.y; + width = [[normalWindow contentView] frame].size.width; + height = [[normalWindow contentView] frame].size.height; + if (p.x >= x && p.y >= y && p.x <= (x + width) && p.y <= (y + height)) { + return_value = YES; + } + return return_value; } - (void) hideCursor @@ -637,7 +648,6 @@ - (void) handleEvent:(NSEvent *)event int buttons = 0; int keycode = 0; bool mouse_event = false; - NSPoint p = [event locationInWindow]; switch ([event type]) { case NSEventTypeFlagsChanged: @@ -738,17 +748,22 @@ - (void) handleEvent:(NSEvent *)event break; case NSEventTypeMouseMoved: if (isAbsoluteEnabled) { - if (![self screenContainsPoint:p] || ![[self window] isKeyWindow]) { + if ([self mouseInWindow]) { + mouse_event = true; + } + + if (![self mouseInWindow] || ![[self window] isKeyWindow]) { if (isMouseGrabbed) { [self ungrabMouse]; } } else { - if (!isMouseGrabbed) { + if (!isMouseGrabbed && [self mouseInWindow]) { [self grabMouse]; } } + } else { + mouse_event = true; } - mouse_event = true; break; case NSEventTypeLeftMouseDown: if ([event modifierFlags] & NSEventModifierFlagCommand) { @@ -784,7 +799,7 @@ - (void) handleEvent:(NSEvent *)event break; case NSEventTypeLeftMouseUp: mouse_event = true; - if (!isMouseGrabbed && [self screenContainsPoint:p]) { + if (!isMouseGrabbed && [self mouseInWindow]) { if([[self window] isKeyWindow]) { [self grabMouse]; } @@ -844,7 +859,8 @@ - (void) handleEvent:(NSEvent *)event * The check on screenContainsPoint is to avoid sending out of range values for * clicks in the titlebar. */ - if ([self screenContainsPoint:p]) { + if ([self mouseInWindow]) { + NSPoint p = [event locationInWindow]; qemu_input_queue_abs(dcl->con, INPUT_AXIS_X, p.x, 0, screen.width); qemu_input_queue_abs(dcl->con, INPUT_AXIS_Y, screen.height - p.y, 0, screen.height); }
Fix the cocoa front-end to correctly be able to use absolute mode. Signed-off-by: John Arbuckle <programmingkidx@gmail.com> --- ui/cocoa.m | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-)