On Fri, 15 Mar 2019 at 09:37, Chen Zhang <tgfb...@me.com> wrote: > > > Signed-off-by: Chen Zhang <tgfb...@me.com>
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