Re: [PATCH v7] ui/cocoa: Use NSWindow's ability to resize
On 2023/12/20 5:06, Marek Glogowski wrote: Hi For me, the problem does not occur if you use the maximum screen resolution available. For me it is 1680x1050 - everything works fine. When I change the screen preference to a smaller screen resolution than my maximum (1440x900,1024x768 ...) the mouse starts to malfunction. The mouse pointer works strenuously and with a delay. Checked on the current version with git qemu-system-ppc AOS4 with V8 and V7 patch and the older version qemu-system-aarch64 Linux/Fedora Hi, I have just sent v9. Please see if it improves the situation. https://patchew.org/QEMU/20240213-cocoa-v9-0-d5a5e1bf0...@daynix.com/ Regards, Akihiko Odaki Wiadomość napisana przez Akihiko Odaki <mailto:akihiko.od...@daynix.com>> w dniu 19.12.2023, o godz. 13:12: On 2023/12/19 0:07, Rene Engel wrote: --- Ursprüngliche Nachricht --- Von: Akihiko Odaki <mailto:akihiko.od...@daynix.com>> Datum: 18.12.2023 08:59:41 An: Rene Engel mailto:reneenge...@emailn.de>> Betreff: Re: [PATCH v7] ui/cocoa: Use NSWindow's ability to resize On 2023/12/17 18:46, Rene Engel wrote: --- Ursprüngliche Nachricht --- Von: Akihiko Odaki <mailto:akihiko.od...@daynix.com>> Datum: 17.12.2023 07:25:52 An: Peter Maydell <mailto:peter.mayd...@linaro.org>>, Philippe Mathieu-Daudé mailto:phi...@linaro.org>>, Gerd Hoffmann mailto:kra...@redhat.com>>, Marc-André Lureau <mailto:marcandre.lur...@redhat.com>>, Marek Glogowski mailto:smark...@gmail.com>> Betreff: [PATCH v7] ui/cocoa: Use NSWindow's ability to resize Tested-by: Rene Engel <mailto:reneenge...@emailn.de>> This patch now works with the "option zoom-to-fit=on/off" thank you very much. But there is severe mouse lag within Cocoa output in full screen. You can reproduce the problem by using the mouse very slowly inside the machine where the mouse pointer no longer moves (guest). This issue only occurs with Cocoa edition SDL/GTK works without mouse lag within the machine. I can't reproduce the issue. Is it a regression caused by this change or an existing bug? I'm not sure how to reproduce it, but when I compile Qemu from master source, zoom-to-fit for cocoa output is always active without using your patch and cannot be enabled or disabled in full screen. Here the mouse speed is about the same as under the MacOs host system. When using their latest patch series v8 for cocoa I can enable and disable zoom-to-fit with "-display cocoa,zoom-to-fit=on/off -full-screen" but the mouse speed then becomes slower. I'm not sure what changes there were from Qemu Master where part of their patch must be included, unless someone else has added something similar for Qemu Master. Can you tell me your whole command line and guest operating system? Regards, Akihiko Odaki
Re: [PATCH v7] ui/cocoa: Use NSWindow's ability to resize
On 2023/12/20 5:06, Marek Glogowski wrote: Hi For me, the problem does not occur if you use the maximum screen resolution available. For me it is 1680x1050 - everything works fine. When I change the screen preference to a smaller screen resolution than my maximum (1440x900,1024x768 ...) the mouse starts to malfunction. The mouse pointer works strenuously and with a delay. Checked on the current version with git qemu-system-ppc AOS4 with V8 and V7 patch and the older version qemu-system-aarch64 Linux/Fedora Please tell me your whole command line. It is also nice if you test the combination of latest QEMU with and without the patches and Fedora.
Re: [PATCH v7] ui/cocoa: Use NSWindow's ability to resize
Hi For me, the problem does not occur if you use the maximum screen resolution available. For me it is 1680x1050 - everything works fine. When I change the screen preference to a smaller screen resolution than my maximum (1440x900,1024x768 ...) the mouse starts to malfunction. The mouse pointer works strenuously and with a delay. Checked on the current version with git qemu-system-ppc AOS4 with V8 and V7 patch and the older version qemu-system-aarch64 Linux/Fedora > Wiadomość napisana przez Akihiko Odaki w dniu > 19.12.2023, o godz. 13:12: > > On 2023/12/19 0:07, Rene Engel wrote: >> --- Ursprüngliche Nachricht --- >> Von: Akihiko Odaki >> Datum: 18.12.2023 08:59:41 >> An: Rene Engel >> Betreff: Re: [PATCH v7] ui/cocoa: Use NSWindow's ability to resize >>> On 2023/12/17 18:46, Rene Engel wrote: >>>> >>>> --- Ursprüngliche Nachricht --- >>>> Von: Akihiko Odaki >>>> Datum: 17.12.2023 07:25:52 >>>> An: Peter Maydell , Philippe Mathieu-Daudé >>> , Gerd Hoffmann , Marc-André >>> Lureau , Marek Glogowski >>> >>>> Betreff: [PATCH v7] ui/cocoa: Use NSWindow's ability to resize >>>> >>>> Tested-by: Rene Engel >>>> >>>> This patch now works with the "option zoom-to-fit=on/off" >>> thank you very much. >>>> >>>> But there is severe mouse lag within Cocoa output in full screen. You >>> can reproduce the problem by using the mouse very slowly inside the machine >>> where the mouse pointer no longer moves (guest). This issue only occurs with >>> Cocoa edition SDL/GTK works without mouse lag within the machine. >>> >>> I can't reproduce the issue. Is it a regression caused by this change or >>> >>> an existing bug? >>> >> I'm not sure how to reproduce it, but when I compile Qemu from master >> source, zoom-to-fit for cocoa output is always active without using your >> patch and cannot be enabled or disabled in full screen. Here the mouse speed >> is about the same as under the MacOs host system. >> When using their latest patch series v8 for cocoa I can enable and disable >> zoom-to-fit with "-display cocoa,zoom-to-fit=on/off -full-screen" but the >> mouse speed then becomes slower. I'm not sure what changes there were from >> Qemu Master where part of their patch must be included, unless someone else >> has added something similar for Qemu Master. > > Can you tell me your whole command line and guest operating system? > > Regards, > Akihiko Odaki
Re: [PATCH v7] ui/cocoa: Use NSWindow's ability to resize
On 2023/12/19 0:07, Rene Engel wrote: --- Ursprüngliche Nachricht --- Von: Akihiko Odaki Datum: 18.12.2023 08:59:41 An: Rene Engel Betreff: Re: [PATCH v7] ui/cocoa: Use NSWindow's ability to resize On 2023/12/17 18:46, Rene Engel wrote: --- Ursprüngliche Nachricht --- Von: Akihiko Odaki Datum: 17.12.2023 07:25:52 An: Peter Maydell , Philippe Mathieu-Daudé , Gerd Hoffmann , Marc-André Lureau , Marek Glogowski Betreff: [PATCH v7] ui/cocoa: Use NSWindow's ability to resize Tested-by: Rene Engel This patch now works with the "option zoom-to-fit=on/off" thank you very much. But there is severe mouse lag within Cocoa output in full screen. You can reproduce the problem by using the mouse very slowly inside the machine where the mouse pointer no longer moves (guest). This issue only occurs with Cocoa edition SDL/GTK works without mouse lag within the machine. I can't reproduce the issue. Is it a regression caused by this change or an existing bug? I'm not sure how to reproduce it, but when I compile Qemu from master source, zoom-to-fit for cocoa output is always active without using your patch and cannot be enabled or disabled in full screen. Here the mouse speed is about the same as under the MacOs host system. When using their latest patch series v8 for cocoa I can enable and disable zoom-to-fit with "-display cocoa,zoom-to-fit=on/off -full-screen" but the mouse speed then becomes slower. I'm not sure what changes there were from Qemu Master where part of their patch must be included, unless someone else has added something similar for Qemu Master. Can you tell me your whole command line and guest operating system? Regards, Akihiko Odaki
Re: [PATCH v7] ui/cocoa: Use NSWindow's ability to resize
On Mon, 18 Dec 2023, Akihiko Odaki wrote:
On 2023/12/17 20:39, BALATON Zoltan wrote:
On Sun, 17 Dec 2023, Akihiko Odaki wrote:
This change brings two new features:
- The window will be resizable if "Zoom To Fit" is eanbled
- The window can be made full screen by clicking full screen button
provided by the platform. (The left-top green button.)
Signed-off-by: Akihiko Odaki
Tested-by: Rene Engel
---
V5 -> V6:
Rebased.
---
Changes in v7:
- Fixed zoom-to-fit option. (Marek Glogowski)
- Link to v6:
https://lore.kernel.org/r/20231211-cocoa-v6-1-49f3be019...@daynix.com
---
ui/cocoa.m | 542
+
1 file changed, 258 insertions(+), 284 deletions(-)
Is ir possible to break this patch up into smaller ones for easier review?
E.g. separate patch moving mouse event handling out of handleEventLocked,
replacing stretch_video flag with NSWindowStyleMaskResizable and whatever
else can be done as independent steps? Not sure if that's possible or needs
the whole chnage at once but this patch seems to be too big. Some more
comments below.
I split it into three patches with v8, but most changes are still in one
patch because they depend on the change to unify the full screen window and
normal window and vice-versa.
It still helps to remove unrelated changes from the big patch so it's
more obvious what's needed for that change and thus easier to review.
[...]
@@ -513,36 +504,43 @@ - (void) drawRect:(NSRect) rect
}
}
-- (void) setContentDimensions
+- (NSSize) fixZoomedFullScreenSize:(NSSize)proposedSize
{
- COCOA_DEBUG("QemuCocoaView: setContentDimensions\n");
+ NSSize size;
- if (isFullscreen) {
- cdx = [[NSScreen mainScreen] frame].size.width /
(float)screen.width;
- cdy = [[NSScreen mainScreen] frame].size.height /
(float)screen.height;
+ size.width = (CGFloat)screen.width * proposedSize.height;
+ size.height = (CGFloat)screen.height * proposedSize.width;
One of these will be overwritten in the next if below so maybe drop this
init and do the calculation in the if legs which is then also clearer to
show that this would scale one of these with screen.width/screen.height or
the inverse of that.
This also removes stretch_video flag and the calculation to preserve aspect
ratio. Is that correct? Would it now distort the image when zooming to full
screen if guest resolution is not the same as host screen? Is that how
zoom-to-fit should work? At leest with -display sdl going to full screen
guest screen is zoomed preserving aspect ratio but maybe sdl does not have
zoom-to-fit option. I don't know how it works with other displays such as
gtk.
The purpose of this method is to fix the aspect ratio for zoom-to-fit by
shrinking width or height. It operates in the three steps:
1. Compute the values necessary either for shrinking width or height.
2. Decide which of width or height to shrink.
3. Compute the final values.
Wouldn't it be simpler to do in two steps:
1. Decide what needs to be scaled
2. Do the computation accordingly
I.e.
if (size.width < size.height) {
size.width = proposedSize.height * (CGFloat)screen.width / screen.height;
size.height = proposedSize.height;
} else {
size.width = proposedSize.width;
size.height = proposedSize.width * (CGFloat)screen.height / screen.width;
}
Seems to me more explicit than doing the scale factor calculation split in
two lines that's harder to follow. I had to think about what that does
while this shows it clearer. If you prefer shorter lines you could also
init size = proposedSize and then scale either width or height afterwards
which would still be clearer than your way I think.
Regaards,
BALATON Zoltan
Re: [PATCH v7] ui/cocoa: Use NSWindow's ability to resize
On 2023/12/17 18:46, Rene Engel wrote: --- Ursprüngliche Nachricht --- Von: Akihiko Odaki Datum: 17.12.2023 07:25:52 An: Peter Maydell , Philippe Mathieu-Daudé , Gerd Hoffmann , Marc-André Lureau , Marek Glogowski Betreff: [PATCH v7] ui/cocoa: Use NSWindow's ability to resize Tested-by: Rene Engel This patch now works with the "option zoom-to-fit=on/off" thank you very much. But there is severe mouse lag within Cocoa output in full screen. You can reproduce the problem by using the mouse very slowly inside the machine where the mouse pointer no longer moves (guest). This issue only occurs with Cocoa edition SDL/GTK works without mouse lag within the machine. I can't reproduce the issue. Is it a regression caused by this change or an existing bug?
Re: [PATCH v7] ui/cocoa: Use NSWindow's ability to resize
On 2023/12/17 20:39, BALATON Zoltan wrote:
On Sun, 17 Dec 2023, Akihiko Odaki wrote:
This change brings two new features:
- The window will be resizable if "Zoom To Fit" is eanbled
- The window can be made full screen by clicking full screen button
provided by the platform. (The left-top green button.)
Signed-off-by: Akihiko Odaki
Tested-by: Rene Engel
---
V5 -> V6:
Rebased.
---
Changes in v7:
- Fixed zoom-to-fit option. (Marek Glogowski)
- Link to v6:
https://lore.kernel.org/r/20231211-cocoa-v6-1-49f3be019...@daynix.com
---
ui/cocoa.m | 542
+
1 file changed, 258 insertions(+), 284 deletions(-)
Is ir possible to break this patch up into smaller ones for easier
review? E.g. separate patch moving mouse event handling out of
handleEventLocked, replacing stretch_video flag with
NSWindowStyleMaskResizable and whatever else can be done as independent
steps? Not sure if that's possible or needs the whole chnage at once but
this patch seems to be too big. Some more comments below.
I split it into three patches with v8, but most changes are still in one
patch because they depend on the change to unify the full screen window
and normal window and vice-versa.
diff --git a/ui/cocoa.m b/ui/cocoa.m
index cd069da6965b..31a2230f55a4 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -99,12 +99,10 @@ static void cocoa_switch(DisplayChangeListener *dcl,
static DisplayChangeListener dcl = {
.ops = &dcl_ops,
};
-static int last_buttons;
static int cursor_hide = 1;
static int left_command_key_enabled = 1;
static bool swap_opt_cmd;
-static bool stretch_video;
static NSTextField *pauseLabel;
static bool allow_events;
@@ -304,20 +302,17 @@ static void handleAnyDeviceErrors(Error * err)
*/
@interface QemuCocoaView : NSView
{
+ NSTrackingArea *trackingArea;
QEMUScreen screen;
- NSWindow *fullScreenWindow;
- float cx,cy,cw,ch,cdx,cdy;
pixman_image_t *pixman_image;
QKbdState *kbd;
BOOL isMouseGrabbed;
- BOOL isFullscreen;
BOOL isAbsoluteEnabled;
CFMachPortRef eventsTap;
}
- (void) switchSurface:(pixman_image_t *)image;
- (void) grabMouse;
- (void) ungrabMouse;
-- (void) toggleFullScreen:(id)sender;
- (void) setFullGrab:(id)sender;
- (void) handleMonitorInput:(NSEvent *)event;
- (bool) handleEvent:(NSEvent *)event;
@@ -333,8 +328,6 @@ - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled;
*/
- (BOOL) isMouseGrabbed;
- (BOOL) isAbsoluteEnabled;
-- (float) cdx;
-- (float) cdy;
- (QEMUScreen) gscreen;
- (void) raiseAllKeys;
@end
@@ -392,46 +385,43 @@ - (BOOL) isOpaque
return YES;
}
-- (BOOL) screenContainsPoint:(NSPoint) p
+- (void) removeTrackingRect
{
- return (p.x > -1 && p.x < screen.width && p.y > -1 && p.y <
screen.height);
+ if (trackingArea) {
+ [self removeTrackingArea:trackingArea];
+ [trackingArea release];
+ trackingArea = nil;
+ }
}
-/* Get location of event and convert to virtual screen coordinate */
-- (CGPoint) screenLocationOfEvent:(NSEvent *)ev
+- (void) frameUpdated
{
- NSWindow *eventWindow = [ev window];
- // XXX: Use CGRect and -convertRectFromScreen: to support macOS
10.10
- CGRect r = CGRectZero;
- r.origin = [ev locationInWindow];
- if (!eventWindow) {
- if (!isFullscreen) {
- return [[self window] convertRectFromScreen:r].origin;
- } else {
- CGPoint locationInSelfWindow = [[self window]
convertRectFromScreen:r].origin;
- CGPoint loc = [self convertPoint:locationInSelfWindow
fromView:nil];
- if (stretch_video) {
- loc.x /= cdx;
- loc.y /= cdy;
- }
- return loc;
- }
- } else if ([[self window] isEqual:eventWindow]) {
- if (!isFullscreen) {
- return r.origin;
- } else {
- CGPoint loc = [self convertPoint:r.origin fromView:nil];
- if (stretch_video) {
- loc.x /= cdx;
- loc.y /= cdy;
- }
- return loc;
- }
- } else {
- return [[self window] convertRectFromScreen:[eventWindow
convertRectToScreen:r]].origin;
+ [self removeTrackingRect];
+
+ if ([self window]) {
+ NSTrackingAreaOptions options = NSTrackingActiveInKeyWindow |
+
NSTrackingMouseEnteredAndExited |
+ NSTrackingMouseMoved;
+ trackingArea = [[NSTrackingArea alloc] initWithRect:[self frame]
+ options:options
+ owner:self
+ userInfo:nil];
+ [self addTrackingArea:trackingArea];
+ [self updateUIInfo];
}
}
+- (void) viewDidMoveToWindow
+{
+ [self resizeWindow];
+ [self frameUpdated];
+}
+
+- (void) viewWillMoveToWindow:(NSWindow *)newWindow
+{
+ [self removeTrackingRect];
+}
+
Re: [PATCH v7] ui/cocoa: Use NSWindow's ability to resize
On Sun, 17 Dec 2023, Akihiko Odaki wrote:
This change brings two new features:
- The window will be resizable if "Zoom To Fit" is eanbled
- The window can be made full screen by clicking full screen button
provided by the platform. (The left-top green button.)
Signed-off-by: Akihiko Odaki
Tested-by: Rene Engel
---
V5 -> V6:
Rebased.
---
Changes in v7:
- Fixed zoom-to-fit option. (Marek Glogowski)
- Link to v6:
https://lore.kernel.org/r/20231211-cocoa-v6-1-49f3be019...@daynix.com
---
ui/cocoa.m | 542 +
1 file changed, 258 insertions(+), 284 deletions(-)
Is ir possible to break this patch up into smaller ones for easier review?
E.g. separate patch moving mouse event handling out of handleEventLocked,
replacing stretch_video flag with NSWindowStyleMaskResizable and whatever
else can be done as independent steps? Not sure if that's possible or
needs the whole chnage at once but this patch seems to be too big. Some
more comments below.
diff --git a/ui/cocoa.m b/ui/cocoa.m
index cd069da6965b..31a2230f55a4 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -99,12 +99,10 @@ static void cocoa_switch(DisplayChangeListener *dcl,
static DisplayChangeListener dcl = {
.ops = &dcl_ops,
};
-static int last_buttons;
static int cursor_hide = 1;
static int left_command_key_enabled = 1;
static bool swap_opt_cmd;
-static bool stretch_video;
static NSTextField *pauseLabel;
static bool allow_events;
@@ -304,20 +302,17 @@ static void handleAnyDeviceErrors(Error * err)
*/
@interface QemuCocoaView : NSView
{
+NSTrackingArea *trackingArea;
QEMUScreen screen;
-NSWindow *fullScreenWindow;
-float cx,cy,cw,ch,cdx,cdy;
pixman_image_t *pixman_image;
QKbdState *kbd;
BOOL isMouseGrabbed;
-BOOL isFullscreen;
BOOL isAbsoluteEnabled;
CFMachPortRef eventsTap;
}
- (void) switchSurface:(pixman_image_t *)image;
- (void) grabMouse;
- (void) ungrabMouse;
-- (void) toggleFullScreen:(id)sender;
- (void) setFullGrab:(id)sender;
- (void) handleMonitorInput:(NSEvent *)event;
- (bool) handleEvent:(NSEvent *)event;
@@ -333,8 +328,6 @@ - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled;
*/
- (BOOL) isMouseGrabbed;
- (BOOL) isAbsoluteEnabled;
-- (float) cdx;
-- (float) cdy;
- (QEMUScreen) gscreen;
- (void) raiseAllKeys;
@end
@@ -392,46 +385,43 @@ - (BOOL) isOpaque
return YES;
}
-- (BOOL) screenContainsPoint:(NSPoint) p
+- (void) removeTrackingRect
{
-return (p.x > -1 && p.x < screen.width && p.y > -1 && p.y < screen.height);
+if (trackingArea) {
+[self removeTrackingArea:trackingArea];
+[trackingArea release];
+trackingArea = nil;
+}
}
-/* Get location of event and convert to virtual screen coordinate */
-- (CGPoint) screenLocationOfEvent:(NSEvent *)ev
+- (void) frameUpdated
{
-NSWindow *eventWindow = [ev window];
-// XXX: Use CGRect and -convertRectFromScreen: to support macOS 10.10
-CGRect r = CGRectZero;
-r.origin = [ev locationInWindow];
-if (!eventWindow) {
-if (!isFullscreen) {
-return [[self window] convertRectFromScreen:r].origin;
-} else {
-CGPoint locationInSelfWindow = [[self window]
convertRectFromScreen:r].origin;
-CGPoint loc = [self convertPoint:locationInSelfWindow
fromView:nil];
-if (stretch_video) {
-loc.x /= cdx;
-loc.y /= cdy;
-}
-return loc;
-}
-} else if ([[self window] isEqual:eventWindow]) {
-if (!isFullscreen) {
-return r.origin;
-} else {
-CGPoint loc = [self convertPoint:r.origin fromView:nil];
-if (stretch_video) {
-loc.x /= cdx;
-loc.y /= cdy;
-}
-return loc;
-}
-} else {
-return [[self window] convertRectFromScreen:[eventWindow
convertRectToScreen:r]].origin;
+[self removeTrackingRect];
+
+if ([self window]) {
+NSTrackingAreaOptions options = NSTrackingActiveInKeyWindow |
+NSTrackingMouseEnteredAndExited |
+NSTrackingMouseMoved;
+trackingArea = [[NSTrackingArea alloc] initWithRect:[self frame]
+options:options
+ owner:self
+ userInfo:nil];
+[self addTrackingArea:trackingArea];
+[self updateUIInfo];
}
}
+- (void) viewDidMoveToWindow
+{
+[self resizeWindow];
+[self frameUpdated];
+}
+
+- (void) viewWillMoveToWindow:(NSWindow *)newWindow
+{
+[self removeTrackingRect];
+}
+
- (void) hideCursor
{
if (!cursor_hide) {
@@ -494,13 +484,14 @@ - (void) drawRect:(NSRect) rect
int i;
CGImageRef clipImageRef;
CGRect clipRect;
+CGFloat d = (CGFloat)h / [self frame].size.height;
