Re: [Qemu-devel] [PATCH 1/2] Fix absolute input device grabbing issues on Mojave

[Peter Maydell]

On Fri, 15 Mar 2019 at 09:37, Chen Zhang <address@hidden> wrote:
>
>
> Signed-off-by: Chen Zhang <address@hidden>

Hi; thanks for this patch. I think a commit message for
the patch would be useful, containing the rationale for
why we make the change. I know you put that in the cover letter,
but that won't get into the git commit history.

If you know under what circumstances the event's window
pointer is NULL, that would also be useful to record.

> ---
>  ui/cocoa.m | 29 +++++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 420b2411c1..5d0a6599d9 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -405,6 +405,24 @@ QemuCocoaView *cocoaView;
>      return (p.x > -1 && p.x < screen.width && p.y > -1 && p.y < 
> screen.height);
>  }
>
> +/* Get location of event and convert to virtual screen coordinate */
> +- (CGPoint) screenLocationOfEvent:(NSEvent *)ev
> +{
> +    NSWindow *eventWindow = [ev window];
> +    if (!eventWindow) {
> +       return [self.window convertPointFromScreen:[ev locationInWindow]];
> +    } else if ([self.window isEqual:eventWindow]) {
> +        return [ev locationInWindow];
> +    } else {
> +        return [self.window convertPointFromScreen:[eventWindow 
> convertPointToScreen:[ev locationInWindow]]];
> +    }
> +}
> +
> +- (BOOL) screenContainsPointOfEvent:(NSEvent *)ev
> +{
> +    return [self screenContainsPoint:[self screenLocationOfEvent:ev]];
> +}

It looks like we pretty much always call
[self screenLocationOfEvent: event] after we call
[self screenContainsPoint:p], which will do the conversion
all over again. So I think it would be clearer to have
a method which did "return an NSPoint which is the location
of this event in the QEMU display window", and then just
call that where we currently do
 NSPoint p = [event locationInWindow];
Then we wouldn't need to change any of the following code or
the screenContainsPoint calls.

thanks
-- PMM