Re: [PATCH v2 1/1] ui/cocoa: add zoom-to-fit display option
> On 27 Oct 2023, at 16:24, Akihiko Odaki wrote:
>
> On 2023/10/28 0:09, carwynel...@gmail.com wrote:
>> From: Carwyn Ellis
>> Provides a display option, zoom-to-fit, that enables scaling of the
>> display when full-screen mode is enabled.
>> Also ensures that the corresponding menu item is marked as enabled when
>> the option is set to on.
>> Signed-off-by: Carwyn Ellis
>> ---
>> qapi/ui.json | 8 ++--
>> ui/cocoa.m | 35 ---
>> 2 files changed, 26 insertions(+), 17 deletions(-)
>> diff --git a/qapi/ui.json b/qapi/ui.json
>> index 006616aa77..fd12791ff9 100644
>> --- a/qapi/ui.json
>> +++ b/qapi/ui.json
>> @@ -1409,13 +1409,17 @@
>> # codes match their position on non-Mac keyboards and you can use
>> # Meta/Super and Alt where you expect them. (default: off)
>> #
>> -# Since: 7.0
>> +# @zoom-to-fit: Scale display to fit when full-screen enabled.
>> +# Defaults to "off".
>> +#
>> +# Since: 8.2
>
> I don't think this new option will affect only when full-screen enabled, but
> probably it will affect also in a windowed mode. Perhaps you can just copy
> the description for DisplayGTK except the statement regarding virtio-gpu.
Ok.
>
> Also don't replace "Since: 7.0". It denotes the version that introduced the
> structure, not an individual member.
Ok, I’ll flip it back to 7.0.
>
>> ##
>> { 'struct': 'DisplayCocoa',
>>'data': {
>>'*left-command-key': 'bool',
>>'*full-grab': 'bool',
>> - '*swap-opt-cmd': 'bool'
>> + '*swap-opt-cmd': 'bool',
>> + '*zoom-to-fit': 'bool'
>>} }
>>##
>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>> index d95276013c..903adb85a1 100644
>> --- a/ui/cocoa.m
>> +++ b/ui/cocoa.m
>> @@ -104,7 +104,7 @@ static void cocoa_switch(DisplayChangeListener *dcl,
>> static int left_command_key_enabled = 1;
>> static bool swap_opt_cmd;
>> -static bool stretch_video;
>> +static bool stretch_video = false;
>
> You don't need to assign false here. C initializes it as false by default.
Ahh of course.
Thanks again for getting back to me so quickly! :)
Re: [PATCH 1/1] ui/cocoa: add full-screen-scaling display option
> On 26 Oct 2023, at 03:51, Akihiko Odaki wrote:
>
> On 2023/10/25 23:04, carwynel...@gmail.com wrote:
>> From: Carwyn Ellis
>> Provides a display option, full-screen-scaling, that enables scaling of
>> the display when full-screen mode is enabled.
>> Also ensures that the corresponding menu item is marked as enabled when
>> the option is set to on.
>
> Hi,
>
> Thank you for your contribution.
>
> Please drop qemu-triv...@nongnu.org. This change is not that trivial.
> Instead add "Graphics maintainers" listed in MAINTAINERS file to CC.
>
> Please also add Signed-off-by tag. See docs/devel/submitting-a-patch.rst to
> know what the tag means.
Hi,
No problem at all. Thanks for getting back to me so quickly! :)
I guess I missed this on the email body so I’ll make sure this is present when
I resubmit.
Thanks
>
>> ---
>> qapi/ui.json | 6 +-
>> ui/cocoa.m | 33 -
>> 2 files changed, 25 insertions(+), 14 deletions(-)
>> diff --git a/qapi/ui.json b/qapi/ui.json
>> index 006616aa77..9035b230ce 100644
>> --- a/qapi/ui.json
>> +++ b/qapi/ui.json
>> @@ -1409,13 +1409,17 @@
>> # codes match their position on non-Mac keyboards and you can use
>> # Meta/Super and Alt where you expect them. (default: off)
>> #
>> +# @full-screen-scaling: Scale display to fit when full-screen enabled.
>> +# Defaults to "off".
>> +#
>
> I think it's better to name zoom-to-fit to align with DisplayGTK.
> It should also have "(Since 8.2)".
I did look for an existing param but missed this one. Thanks!
>
>> # Since: 7.0
>> ##
>> { 'struct': 'DisplayCocoa',
>>'data': {
>>'*left-command-key': 'bool',
>>'*full-grab': 'bool',
>> - '*swap-opt-cmd': 'bool'
>> + '*swap-opt-cmd': 'bool',
>> + '*full-screen-scaling': 'bool'
>>} }
>>##
>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>> index d95276013c..7ddc4de174 100644
>> --- a/ui/cocoa.m
>> +++ b/ui/cocoa.m
>> @@ -1671,7 +1671,9 @@ static void create_initial_menus(void)
>> // View menu
>> menu = [[NSMenu alloc] initWithTitle:@"View"];
>> [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"Enter Fullscreen"
>> action:@selector(doToggleFullScreen:) keyEquivalent:@"f"] autorelease]]; //
>> Fullscreen
>> -[menu addItem: [[[NSMenuItem alloc] initWithTitle:@"Zoom To Fit"
>> action:@selector(zoomToFit:) keyEquivalent:@""] autorelease]];
>> +menuItem = [[[NSMenuItem alloc] initWithTitle:@"Zoom To Fit"
>> action:@selector(zoomToFit:) keyEquivalent:@""] autorelease];
>> +[menuItem setState: (stretch_video) ? NSControlStateValueOn :
>> NSControlStateValueOff];
>
> nit: You don't need parentheses around strech_video.
No problem - will change this.
>
>> +[menu addItem: menuItem];
>> menuItem = [[[NSMenuItem alloc] initWithTitle:@"View" action:nil
>> keyEquivalent:@""] autorelease];
>> [menuItem setSubmenu:menu];
>> [[NSApp mainMenu] addItem:menuItem];
>> @@ -2041,18 +2043,6 @@ static void cocoa_display_init(DisplayState *ds,
>> DisplayOptions *opts)
>>[QemuApplication sharedApplication];
>> -create_initial_menus();
>> -
>> -/*
>> - * Create the menu entries which depend on QEMU state (for consoles
>> - * and removable devices). These make calls back into QEMU functions,
>> - * which is OK because at this point we know that the second thread
>> - * holds the iothread lock and is synchronously waiting for us to
>> - * finish.
>> - */
>> -add_console_menu_entries();
>> -addRemovableDevicesMenuItems();
>> -
>> // Create an Application controller
>> QemuCocoaAppController *controller = [[QemuCocoaAppController alloc]
>> init];
>
> QemuCocoaAppController also has code to initialize stretch_video; it's not OK
> to have code to initialize a same variable in two different places.
Ok, I’ll take a look and tidy this up.
>
>> [NSApp setDelegate:controller];
>> @@ -2062,6 +2052,7 @@ static void cocoa_display_init(DisplayState *ds,
>> DisplayOptions *opts)
>> [NSApp activateIgnoringOtherApps: YES];
>> [controller toggleFullScreen: nil];
>> }
>> +
>
> Don't add a blank line here. It's irrelevant from this change.
Ok.
I’ll aim to get this resubmitted as a V2 later today with the appropriate CCs.
Thanks again,
Carwyn
Re: [PATCH v3 2/2] hw/display/vmware_vga: do not discard screen updates
Awesome! Thanks! > On 22 Apr 2022, at 11:40, Gerd Hoffmann wrote: > > On Sun, Apr 10, 2022 at 05:49:30PM +0100, Carwyn Ellis wrote: >> ping >> >> https://patchew.org/QEMU/20220206183956.10694-1-carwynel...@gmail.com/20220206183956.10694-3-carwynel...@gmail.com/ >> >> Originally submitted as one of two patches, the first patch to use trace >> events has been merged, however the patch that fixes garbled output hasn’t >> been reviewed yet. > > Hmm, slipped through for some reason, IIRC I cherry-picked the trace > events patch from v2 series and probably simply missed v3. Queued now. > > thanks, > Gerd >
[PATCH v2 0/1] ui/cocoa: show/hide menu in fullscreen on mouse
Minor change to make fullscreen mode in the Cocoa UI a little more convenient. The menu bar is now made visible when the mouse is released (ungrabbed) making it accessible without having to leave fullscreen mode. Grabbing the mouse hides the menu. Incorporates changes in response to feedback from Akihiko Odaki. Carwyn Ellis (1): ui/cocoa: show/hide menu in fullscreen on mouse ungrab/grab ui/cocoa.m | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) -- 2.35.1
[PATCH v2 1/1] ui/cocoa: show/hide menu in fullscreen on mouse ungrab/grab
The menu bar is only accessible when the Cocoa UI is windowed. In order
to allow the menu bar to be accessible in fullscreen mode, this change
makes the menu visible when the mouse is ungrabbed.
When the mouse is grabbed the menu is hidden again.
Incorporates changes in response to review feedback from Akihiko Odaki.
Signed-off-by: Carwyn Ellis
---
ui/cocoa.m | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index c4e5468f9e..ea2cd4ece0 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -656,13 +656,11 @@ - (void) toggleFullScreen:(id)sender
[fullScreenWindow close];
[normalWindow setContentView: self];
[normalWindow makeKeyAndOrderFront: self];
-[NSMenu setMenuBarVisible:YES];
} else { // switch from desktop to fullscreen
isFullscreen = TRUE;
[normalWindow orderOut: nil]; /* Hide the window */
[self grabMouse];
[self setContentDimensions];
-[NSMenu setMenuBarVisible:NO];
fullScreenWindow = [[NSWindow alloc] initWithContentRect:[[NSScreen
mainScreen] frame]
styleMask:NSWindowStyleMaskBorderless
backing:NSBackingStoreBuffered
@@ -1141,7 +1139,9 @@ - (void) grabMouse
{
COCOA_DEBUG("QemuCocoaView: grabMouse\n");
-if (!isFullscreen) {
+if (isFullscreen) {
+[NSMenu setMenuBarVisible: FALSE];
+} else {
if (qemu_name)
[normalWindow setTitle:[NSString stringWithFormat:@"QEMU %s -
(Press ctrl + alt + g to release Mouse)", qemu_name]];
else
@@ -1156,7 +1156,9 @@ - (void) ungrabMouse
{
COCOA_DEBUG("QemuCocoaView: ungrabMouse\n");
-if (!isFullscreen) {
+if (isFullscreen) {
+[NSMenu setMenuBarVisible: TRUE];
+} else {
if (qemu_name)
[normalWindow setTitle:[NSString stringWithFormat:@"QEMU %s",
qemu_name]];
else
--
2.35.1
Re: [PATCH 1/1] ui/cocoa: show/hide menu in fullscreen on mouse ungrab/grab
Thanks, taking a look now and will push up another patch once I’ve tested the
changes.
Regards
Carwyn
> On 18 Feb 2022, at 18:42, Akihiko Odaki wrote:
>
> On 2022/01/03 20:45, Carwyn Ellis wrote:
>> The menu bar is only accessible when the Cocoa UI is windowed. In order
>> to allow the menu bar to be accessible in fullscreen mode, this change
>> makes the menu visible when the mouse is ungrabbed.
>> When the mouse is grabbed the menu is hidden again.
>> Signed-off-by: Carwyn Ellis
>> ---
>> ui/cocoa.m | 8 ++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>> index 69745c483b..42dcf47da4 100644
>> --- a/ui/cocoa.m
>> +++ b/ui/cocoa.m
>> @@ -1037,7 +1037,9 @@ QemuCocoaView *cocoaView;
>> {
>> COCOA_DEBUG("QemuCocoaView: grabMouse\n");
>> -if (!isFullscreen) {
>> +if (isFullscreen) {
>> +[NSMenu setMenuBarVisible: FALSE];
>> +} else {
>> if (qemu_name)
>> [normalWindow setTitle:[NSString stringWithFormat:@"QEMU %s -
>> (Press ctrl + alt + g to release Mouse)", qemu_name]];
>> else
>> @@ -1052,7 +1054,9 @@ QemuCocoaView *cocoaView;
>> {
>> COCOA_DEBUG("QemuCocoaView: ungrabMouse\n");
>> -if (!isFullscreen) {
>> +if (isFullscreen) {
>> +[NSMenu setMenuBarVisible: TRUE];
>> +} else {
>> if (qemu_name)
>> [normalWindow setTitle:[NSString stringWithFormat:@"QEMU %s",
>> qemu_name]];
>> else
>
> [QemuCocoaView -toggleFullscreen:] also has the calls to [NSMenu
> setMenuBarVisible:], which should be removed.
>
> Regards,
> Akihiko Odaki
Re: [PATCH v3 2/2] hw/display/vmware_vga: do not discard screen updates
ping
https://patchew.org/QEMU/20220206183956.10694-1-carwynel...@gmail.com/20220206183956.10694-3-carwynel...@gmail.com/
Originally submitted as one of two patches, the first patch to use trace events
has been merged, however the patch that fixes garbled output hasn’t been
reviewed yet.
Do let me know if you think there’s anything else that needs changing here and
I’ll resubmit if so. FWIW I’ve been using this fix for a couple of months now
without any issues.
Thanks
Carwyn
> On 6 Feb 2022, at 18:39, Carwyn Ellis wrote:
>
> In certain circumstances, typically when there is lots changing on the
> screen, updates will be discarded resulting in garbled output.
>
> This change simplifies the traversal of the display update FIFO queue
> when applying updates. We just track the queue length and iterate up to
> the end of the queue.
>
> Additionally when adding updates to the queue, if the buffer reaches
> capacity we force a flush before accepting further events.
>
> Signed-off-by: Carwyn Ellis
> ---
> hw/display/trace-events | 1 +
> hw/display/vmware_vga.c | 41 +++--
> 2 files changed, 24 insertions(+), 18 deletions(-)
>
> diff --git a/hw/display/trace-events b/hw/display/trace-events
> index 91efc88f04..0c0ffcbe42 100644
> --- a/hw/display/trace-events
> +++ b/hw/display/trace-events
> @@ -24,6 +24,7 @@ vmware_setmode(uint32_t w, uint32_t h, uint32_t bpp) "%dx%d
> @ %d bpp"
> vmware_verify_rect_less_than_zero(const char *name, const char *param, int x)
> "%s: %s was < 0 (%d)"
> vmware_verify_rect_greater_than_bound(const char *name, const char *param,
> int bound, int x) "%s: %s was > %d (%d)"
> vmware_verify_rect_surface_bound_exceeded(const char *name, const char
> *component, int bound, const char *param1, int value1, const char *param2,
> int value2) "%s: %s > %d (%s: %d, %s: %d)"
> +vmware_update_rect_delayed_flush(void) "display update FIFO full - forcing
> flush"
>
> # virtio-gpu-base.c
> virtio_gpu_features(bool virgl) "virgl %d"
> diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
> index 0cc43a1f15..8a3c3cb8f0 100644
> --- a/hw/display/vmware_vga.c
> +++ b/hw/display/vmware_vga.c
> @@ -80,7 +80,7 @@ struct vmsvga_state_s {
> struct vmsvga_rect_s {
> int x, y, w, h;
> } redraw_fifo[REDRAW_FIFO_LEN];
> -int redraw_fifo_first, redraw_fifo_last;
> +int redraw_fifo_last;
> };
>
> #define TYPE_VMWARE_SVGA "vmware-svga"
> @@ -380,33 +380,39 @@ static inline void vmsvga_update_rect(struct
> vmsvga_state_s *s,
> dpy_gfx_update(s->vga.con, x, y, w, h);
> }
>
> -static inline void vmsvga_update_rect_delayed(struct vmsvga_state_s *s,
> -int x, int y, int w, int h)
> -{
> -struct vmsvga_rect_s *rect = >redraw_fifo[s->redraw_fifo_last++];
> -
> -s->redraw_fifo_last &= REDRAW_FIFO_LEN - 1;
> -rect->x = x;
> -rect->y = y;
> -rect->w = w;
> -rect->h = h;
> -}
> -
> static inline void vmsvga_update_rect_flush(struct vmsvga_state_s *s)
> {
> struct vmsvga_rect_s *rect;
>
> if (s->invalidated) {
> -s->redraw_fifo_first = s->redraw_fifo_last;
> +s->redraw_fifo_last = 0;
> return;
> }
> /* Overlapping region updates can be optimised out here - if someone
> * knows a smart algorithm to do that, please share. */
> -while (s->redraw_fifo_first != s->redraw_fifo_last) {
> -rect = >redraw_fifo[s->redraw_fifo_first++];
> -s->redraw_fifo_first &= REDRAW_FIFO_LEN - 1;
> +for (int i = 0; i < s->redraw_fifo_last; i++) {
> +rect = >redraw_fifo[i];
> vmsvga_update_rect(s, rect->x, rect->y, rect->w, rect->h);
> }
> +
> +s->redraw_fifo_last = 0;
> +}
> +
> +static inline void vmsvga_update_rect_delayed(struct vmsvga_state_s *s,
> +int x, int y, int w, int h)
> +{
> +
> +if (s->redraw_fifo_last >= REDRAW_FIFO_LEN) {
> +trace_vmware_update_rect_delayed_flush();
> +vmsvga_update_rect_flush(s);
> +}
> +
> +struct vmsvga_rect_s *rect = >redraw_fifo[s->redraw_fifo_last++];
> +
> +rect->x = x;
> +rect->y = y;
> +rect->w = w;
> +rect->h = h;
> }
>
> #ifdef HW_RECT_ACCEL
> @@ -1159,7 +1165,6 @@ static void vmsvga_reset(DeviceState *dev)
> s->config = 0;
> s->svgaid = SVGA_ID;
> s->cursor.on = 0;
> -s->redraw_fifo_first = 0;
> s->redraw_fifo_last = 0;
> s->syncing = 0;
>
> --
> 2.35.1
>
[PATCH v3 0/2] use trace events and fix garbled output
This patchset supersedes earlier submissions and incorporates feedback from Laurent Vivier, Gerd Hoffmann and Philippe Mathieu-Daudé. There are two patches addressing the following in the vmware vga display code - use of fprintf to log debug output to STDERR This has been replaced with trace events. - garbled display due to lost display updates This prevents an issue that can cause garbled display output when a high number of screen updates are being requested. The queue is now flushed when it reaches capacity. The code traversing the queue when updates are being applied to the display has also been simplified, since we always start the traversal at the beginning of the queue to ensure that all updates are applied. Carwyn Ellis (2): hw/display/vmware_vga: replace fprintf calls with trace events hw/display/vmware_vga: do not discard screen updates hw/display/trace-events | 4 +++ hw/display/vmware_vga.c | 71 - 2 files changed, 45 insertions(+), 30 deletions(-) -- 2.35.1
[PATCH v3 1/2] hw/display/vmware_vga: replace fprintf calls with trace events
Debug output was always being sent to STDERR.
This has been replaced with trace events.
Signed-off-by: Carwyn Ellis
---
hw/display/trace-events | 3 +++
hw/display/vmware_vga.c | 30 ++
2 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/hw/display/trace-events b/hw/display/trace-events
index 4a687d1b8e..91efc88f04 100644
--- a/hw/display/trace-events
+++ b/hw/display/trace-events
@@ -21,6 +21,9 @@ vmware_palette_write(uint32_t index, uint32_t value) "index
%d, value 0x%x"
vmware_scratch_read(uint32_t index, uint32_t value) "index %d, value 0x%x"
vmware_scratch_write(uint32_t index, uint32_t value) "index %d, value 0x%x"
vmware_setmode(uint32_t w, uint32_t h, uint32_t bpp) "%dx%d @ %d bpp"
+vmware_verify_rect_less_than_zero(const char *name, const char *param, int x)
"%s: %s was < 0 (%d)"
+vmware_verify_rect_greater_than_bound(const char *name, const char *param, int
bound, int x) "%s: %s was > %d (%d)"
+vmware_verify_rect_surface_bound_exceeded(const char *name, const char
*component, int bound, const char *param1, int value1, const char *param2, int
value2) "%s: %s > %d (%s: %d, %s: %d)"
# virtio-gpu-base.c
virtio_gpu_features(bool virgl) "virgl %d"
diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index e2969a6c81..0cc43a1f15 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -297,46 +297,52 @@ static inline bool vmsvga_verify_rect(DisplaySurface
*surface,
int x, int y, int w, int h)
{
if (x < 0) {
-fprintf(stderr, "%s: x was < 0 (%d)\n", name, x);
+trace_vmware_verify_rect_less_than_zero(name, "x", x);
return false;
}
if (x > SVGA_MAX_WIDTH) {
-fprintf(stderr, "%s: x was > %d (%d)\n", name, SVGA_MAX_WIDTH, x);
+trace_vmware_verify_rect_greater_than_bound(name, "x", SVGA_MAX_WIDTH,
+x);
return false;
}
if (w < 0) {
-fprintf(stderr, "%s: w was < 0 (%d)\n", name, w);
+trace_vmware_verify_rect_less_than_zero(name, "w", w);
return false;
}
if (w > SVGA_MAX_WIDTH) {
-fprintf(stderr, "%s: w was > %d (%d)\n", name, SVGA_MAX_WIDTH, w);
+trace_vmware_verify_rect_greater_than_bound(name, "w", SVGA_MAX_WIDTH,
+w);
return false;
}
if (x + w > surface_width(surface)) {
-fprintf(stderr, "%s: width was > %d (x: %d, w: %d)\n",
-name, surface_width(surface), x, w);
+trace_vmware_verify_rect_surface_bound_exceeded(name, "width",
+surface_width(surface),
+"x", x, "w", w);
return false;
}
if (y < 0) {
-fprintf(stderr, "%s: y was < 0 (%d)\n", name, y);
+trace_vmware_verify_rect_less_than_zero(name, "y", y);
return false;
}
if (y > SVGA_MAX_HEIGHT) {
-fprintf(stderr, "%s: y was > %d (%d)\n", name, SVGA_MAX_HEIGHT, y);
+trace_vmware_verify_rect_greater_than_bound(name, "y", SVGA_MAX_HEIGHT,
+y);
return false;
}
if (h < 0) {
-fprintf(stderr, "%s: h was < 0 (%d)\n", name, h);
+trace_vmware_verify_rect_less_than_zero(name, "h", h);
return false;
}
if (h > SVGA_MAX_HEIGHT) {
-fprintf(stderr, "%s: h was > %d (%d)\n", name, SVGA_MAX_HEIGHT, h);
+trace_vmware_verify_rect_greater_than_bound(name, "y", SVGA_MAX_HEIGHT,
+y);
return false;
}
if (y + h > surface_height(surface)) {
-fprintf(stderr, "%s: update height > %d (y: %d, h: %d)\n",
-name, surface_height(surface), y, h);
+trace_vmware_verify_rect_surface_bound_exceeded(name, "height",
+
surface_height(surface),
+"y", y, "h", h);
return false;
}
--
2.35.1
[PATCH v3 2/2] hw/display/vmware_vga: do not discard screen updates
In certain circumstances, typically when there is lots changing on the
screen, updates will be discarded resulting in garbled output.
This change simplifies the traversal of the display update FIFO queue
when applying updates. We just track the queue length and iterate up to
the end of the queue.
Additionally when adding updates to the queue, if the buffer reaches
capacity we force a flush before accepting further events.
Signed-off-by: Carwyn Ellis
---
hw/display/trace-events | 1 +
hw/display/vmware_vga.c | 41 +++--
2 files changed, 24 insertions(+), 18 deletions(-)
diff --git a/hw/display/trace-events b/hw/display/trace-events
index 91efc88f04..0c0ffcbe42 100644
--- a/hw/display/trace-events
+++ b/hw/display/trace-events
@@ -24,6 +24,7 @@ vmware_setmode(uint32_t w, uint32_t h, uint32_t bpp) "%dx%d @
%d bpp"
vmware_verify_rect_less_than_zero(const char *name, const char *param, int x)
"%s: %s was < 0 (%d)"
vmware_verify_rect_greater_than_bound(const char *name, const char *param, int
bound, int x) "%s: %s was > %d (%d)"
vmware_verify_rect_surface_bound_exceeded(const char *name, const char
*component, int bound, const char *param1, int value1, const char *param2, int
value2) "%s: %s > %d (%s: %d, %s: %d)"
+vmware_update_rect_delayed_flush(void) "display update FIFO full - forcing
flush"
# virtio-gpu-base.c
virtio_gpu_features(bool virgl) "virgl %d"
diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index 0cc43a1f15..8a3c3cb8f0 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -80,7 +80,7 @@ struct vmsvga_state_s {
struct vmsvga_rect_s {
int x, y, w, h;
} redraw_fifo[REDRAW_FIFO_LEN];
-int redraw_fifo_first, redraw_fifo_last;
+int redraw_fifo_last;
};
#define TYPE_VMWARE_SVGA "vmware-svga"
@@ -380,33 +380,39 @@ static inline void vmsvga_update_rect(struct
vmsvga_state_s *s,
dpy_gfx_update(s->vga.con, x, y, w, h);
}
-static inline void vmsvga_update_rect_delayed(struct vmsvga_state_s *s,
-int x, int y, int w, int h)
-{
-struct vmsvga_rect_s *rect = >redraw_fifo[s->redraw_fifo_last++];
-
-s->redraw_fifo_last &= REDRAW_FIFO_LEN - 1;
-rect->x = x;
-rect->y = y;
-rect->w = w;
-rect->h = h;
-}
-
static inline void vmsvga_update_rect_flush(struct vmsvga_state_s *s)
{
struct vmsvga_rect_s *rect;
if (s->invalidated) {
-s->redraw_fifo_first = s->redraw_fifo_last;
+s->redraw_fifo_last = 0;
return;
}
/* Overlapping region updates can be optimised out here - if someone
* knows a smart algorithm to do that, please share. */
-while (s->redraw_fifo_first != s->redraw_fifo_last) {
-rect = >redraw_fifo[s->redraw_fifo_first++];
-s->redraw_fifo_first &= REDRAW_FIFO_LEN - 1;
+for (int i = 0; i < s->redraw_fifo_last; i++) {
+rect = >redraw_fifo[i];
vmsvga_update_rect(s, rect->x, rect->y, rect->w, rect->h);
}
+
+s->redraw_fifo_last = 0;
+}
+
+static inline void vmsvga_update_rect_delayed(struct vmsvga_state_s *s,
+int x, int y, int w, int h)
+{
+
+if (s->redraw_fifo_last >= REDRAW_FIFO_LEN) {
+trace_vmware_update_rect_delayed_flush();
+vmsvga_update_rect_flush(s);
+}
+
+struct vmsvga_rect_s *rect = >redraw_fifo[s->redraw_fifo_last++];
+
+rect->x = x;
+rect->y = y;
+rect->w = w;
+rect->h = h;
}
#ifdef HW_RECT_ACCEL
@@ -1159,7 +1165,6 @@ static void vmsvga_reset(DeviceState *dev)
s->config = 0;
s->svgaid = SVGA_ID;
s->cursor.on = 0;
-s->redraw_fifo_first = 0;
s->redraw_fifo_last = 0;
s->syncing = 0;
--
2.35.1
[PATCH 2/2] hw/display/vmware_vga: do not discard screen updates
In certain circumstances, typically when there is lots changing on the
screen, updates will be discarded resulting in garbled output.
This change simplifies the traversal of the display update FIFO queue
when applying updates. We just track the queue length and iterate up to
the end of the queue.
Additionanlly when adding updates to the queue, if the buffer reaches
capacity we force a flush before accepting further events.
Signed-off-by: Carwyn Ellis
---
hw/display/trace-events | 1 +
hw/display/vmware_vga.c | 41 +++--
2 files changed, 24 insertions(+), 18 deletions(-)
diff --git a/hw/display/trace-events b/hw/display/trace-events
index e1a0d2a88a..5e3cdb3fa3 100644
--- a/hw/display/trace-events
+++ b/hw/display/trace-events
@@ -24,6 +24,7 @@ vmware_setmode(uint32_t w, uint32_t h, uint32_t bpp) "%dx%d @
%d bpp"
vmware_verify_rect_less_than_zero(const char *name, const char *param, int x)
"%s: %s was < 0 (%d)"
vmware_verify_rect_greater_than_bound(const char *name, const char *param, int
bound, int x) "%s: %s was > %d (%d)"
vmware_verify_rect_surface_bound_exceeded(const char *name, const char
*component, int bound, const char *param1, int value1, const char *param2, int
value2) "%s: %s > %d (%s: %d, %s, %d)"
+vmware_update_rect_delayed_flush(void) "display update FIFO full - forcing
flush"
# virtio-gpu-base.c
virtio_gpu_features(bool virgl) "virgl %d"
diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index 0d32a605a0..e6943005e3 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -80,7 +80,7 @@ struct vmsvga_state_s {
struct vmsvga_rect_s {
int x, y, w, h;
} redraw_fifo[REDRAW_FIFO_LEN];
-int redraw_fifo_first, redraw_fifo_last;
+int redraw_fifo_last;
};
#define TYPE_VMWARE_SVGA "vmware-svga"
@@ -372,33 +372,39 @@ static inline void vmsvga_update_rect(struct
vmsvga_state_s *s,
dpy_gfx_update(s->vga.con, x, y, w, h);
}
-static inline void vmsvga_update_rect_delayed(struct vmsvga_state_s *s,
-int x, int y, int w, int h)
-{
-struct vmsvga_rect_s *rect = >redraw_fifo[s->redraw_fifo_last++];
-
-s->redraw_fifo_last &= REDRAW_FIFO_LEN - 1;
-rect->x = x;
-rect->y = y;
-rect->w = w;
-rect->h = h;
-}
-
static inline void vmsvga_update_rect_flush(struct vmsvga_state_s *s)
{
struct vmsvga_rect_s *rect;
if (s->invalidated) {
-s->redraw_fifo_first = s->redraw_fifo_last;
+s->redraw_fifo_last = 0;
return;
}
/* Overlapping region updates can be optimised out here - if someone
* knows a smart algorithm to do that, please share. */
-while (s->redraw_fifo_first != s->redraw_fifo_last) {
-rect = >redraw_fifo[s->redraw_fifo_first++];
-s->redraw_fifo_first &= REDRAW_FIFO_LEN - 1;
+for (int i = 0; i < s->redraw_fifo_last; i++) {
+rect = >redraw_fifo[i];
vmsvga_update_rect(s, rect->x, rect->y, rect->w, rect->h);
}
+
+s->redraw_fifo_last = 0;
+}
+
+static inline void vmsvga_update_rect_delayed(struct vmsvga_state_s *s,
+int x, int y, int w, int h)
+{
+
+if (s->redraw_fifo_last >= REDRAW_FIFO_LEN) {
+trace_vmware_update_rect_delayed_flush();
+vmsvga_update_rect_flush(s);
+}
+
+struct vmsvga_rect_s *rect = >redraw_fifo[s->redraw_fifo_last++];
+
+rect->x = x;
+rect->y = y;
+rect->w = w;
+rect->h = h;
}
#ifdef HW_RECT_ACCEL
@@ -1151,7 +1157,6 @@ static void vmsvga_reset(DeviceState *dev)
s->config = 0;
s->svgaid = SVGA_ID;
s->cursor.on = 0;
-s->redraw_fifo_first = 0;
s->redraw_fifo_last = 0;
s->syncing = 0;
--
2.34.1
[PATCH 0/2] use trace events and fix garbled output
This patchset supersedes the earlier submission and incorporates feedback from Laurent Vivier and Gerd Hoffmann. There are two patches addressing the following in the vmware vga display code - use of fprintf to log debug output to STDERR This has been replaced with trace events. - garbled display due to lost display updates This prevents an issue that can cause garbled display output when a high number of screen updates are being requested. The queue is now flushed when it reaches capacity. The code traversing the queue when updates are being applied to the display has also been simplified, since we always start the traversal at the beginning of the queue to ensure that all updates are applied. Carwyn Ellis (2): hw/display/vmware_vga: replace fprintf calls with trace events hw/display/vmware_vga: do not discard screen updates hw/display/trace-events | 4 +++ hw/display/vmware_vga.c | 63 + 2 files changed, 37 insertions(+), 30 deletions(-) -- 2.34.1
[PATCH 1/2] hw/display/vmware_vga: replace fprintf calls with trace events
Debug output was always being sent to STDERR.
This has been replaced with trace events.
Signed-off-by: Carwyn Ellis
---
hw/display/trace-events | 3 +++
hw/display/vmware_vga.c | 22 ++
2 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/hw/display/trace-events b/hw/display/trace-events
index 3a7a2c957f..e1a0d2a88a 100644
--- a/hw/display/trace-events
+++ b/hw/display/trace-events
@@ -21,6 +21,9 @@ vmware_palette_write(uint32_t index, uint32_t value) "index
%d, value 0x%x"
vmware_scratch_read(uint32_t index, uint32_t value) "index %d, value 0x%x"
vmware_scratch_write(uint32_t index, uint32_t value) "index %d, value 0x%x"
vmware_setmode(uint32_t w, uint32_t h, uint32_t bpp) "%dx%d @ %d bpp"
+vmware_verify_rect_less_than_zero(const char *name, const char *param, int x)
"%s: %s was < 0 (%d)"
+vmware_verify_rect_greater_than_bound(const char *name, const char *param, int
bound, int x) "%s: %s was > %d (%d)"
+vmware_verify_rect_surface_bound_exceeded(const char *name, const char
*component, int bound, const char *param1, int value1, const char *param2, int
value2) "%s: %s > %d (%s: %d, %s, %d)"
# virtio-gpu-base.c
virtio_gpu_features(bool virgl) "virgl %d"
diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index e2969a6c81..0d32a605a0 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -297,46 +297,44 @@ static inline bool vmsvga_verify_rect(DisplaySurface
*surface,
int x, int y, int w, int h)
{
if (x < 0) {
-fprintf(stderr, "%s: x was < 0 (%d)\n", name, x);
+trace_vmware_verify_rect_less_than_zero(name, "x", x);
return false;
}
if (x > SVGA_MAX_WIDTH) {
-fprintf(stderr, "%s: x was > %d (%d)\n", name, SVGA_MAX_WIDTH, x);
+trace_vmware_verify_rect_greater_than_bound(name, "x", SVGA_MAX_WIDTH,
x);
return false;
}
if (w < 0) {
-fprintf(stderr, "%s: w was < 0 (%d)\n", name, w);
+trace_vmware_verify_rect_less_than_zero(name, "w", w);
return false;
}
if (w > SVGA_MAX_WIDTH) {
-fprintf(stderr, "%s: w was > %d (%d)\n", name, SVGA_MAX_WIDTH, w);
+trace_vmware_verify_rect_greater_than_bound(name, "w", SVGA_MAX_WIDTH,
w);
return false;
}
if (x + w > surface_width(surface)) {
-fprintf(stderr, "%s: width was > %d (x: %d, w: %d)\n",
-name, surface_width(surface), x, w);
+trace_vmware_verify_rect_surface_bound_exceeded(name, "width",
surface_width(surface), "x", x, "w", w);
return false;
}
if (y < 0) {
-fprintf(stderr, "%s: y was < 0 (%d)\n", name, y);
+trace_vmware_verify_rect_less_than_zero(name, "y", y);
return false;
}
if (y > SVGA_MAX_HEIGHT) {
-fprintf(stderr, "%s: y was > %d (%d)\n", name, SVGA_MAX_HEIGHT, y);
+trace_vmware_verify_rect_greater_than_bound(name, "y",
SVGA_MAX_HEIGHT, y);
return false;
}
if (h < 0) {
-fprintf(stderr, "%s: h was < 0 (%d)\n", name, h);
+trace_vmware_verify_rect_less_than_zero(name, "h", h);
return false;
}
if (h > SVGA_MAX_HEIGHT) {
-fprintf(stderr, "%s: h was > %d (%d)\n", name, SVGA_MAX_HEIGHT, h);
+trace_vmware_verify_rect_greater_than_bound(name, "y",
SVGA_MAX_HEIGHT, y);
return false;
}
if (y + h > surface_height(surface)) {
-fprintf(stderr, "%s: update height > %d (y: %d, h: %d)\n",
-name, surface_height(surface), y, h);
+trace_vmware_verify_rect_surface_bound_exceeded(name, "height",
surface_height(surface), "y", y, "h", h);
return false;
}
--
2.34.1
Re: [PATCH 2/2] hw/display/vmware_vga: do not discard screen updates
> On 4 Jan 2022, at 12:23, Gerd Hoffmann wrote:
>
> Hi,
>
>> This change firstly increases the screen update FIFO size to ensure it's
>> large enough to accomodate all updates deferred in a given screen
>> refresh cycle.
>
> How do you know it's large enough?
>
>> @@ -385,7 +385,14 @@ static inline void vmsvga_update_rect_delayed(struct
>> vmsvga_state_s *s,
>> {
>> struct vmsvga_rect_s *rect = >redraw_fifo[s->redraw_fifo_last++];
>>
>> -s->redraw_fifo_last &= REDRAW_FIFO_LEN - 1;
>> +if (s->redraw_fifo_last >= REDRAW_FIFO_LEN) {
>> +VMWARE_VGA_DEBUG("%s: Discarding updates - FIFO length %d
>> exceeded\n",
>> +"vmsvga_update_rect_delayed",
>> +REDRAW_FIFO_LEN
>
> Hmm, apparently you don't know ;)
>
> How about just calling vmsvga_update_rect_flush()
> when the fifo is (almost) full?
Yeah will give that a shot. Wasn’t sure how it’d play so did the simplest thing
possible.
>
> Which guest do you use btw? I'm kind-of surprised this is still being
> used even though it hasn't seen any development (beside fixing a bug now
> and then) for a decade or so and the feature gap to recent vmware is
> huge ...
>
This is an old vmware vm that rarely gets used. Figured I’d see if I could get
it working over the holidays after making the move off an intel mac to m1, and
noticed the issue with the output. In this case using the already configured
vmware drivers was the least worst option.
> take care,
> Gerd
>
Cheers
Carwyn
Re: [PATCH 1/2] hw/display/vmware_vga: only show debug output if DEBUG enabled
Ok cool.
Thanks for the info!
> On 4 Jan 2022, at 09:27, Laurent Vivier wrote:
>
> Le 04/01/2022 à 10:20, Carwyn Ellis a écrit :
>> Hey,
>> Thanks for getting back to me.
>> Yeah will take a look and update when I have a mo.
>
> It's really easy to do, see below for an example:
>
> ...
>>>> @@ -297,45 +303,45 @@ static inline bool
>>>> vmsvga_verify_rect(DisplaySurface *surface,
>>>>int x, int y, int w, int h)
>>>> {
>>>> if (x < 0) {
>>>> -fprintf(stderr, "%s: x was < 0 (%d)\n", name, x);
>>>> +VMWARE_VGA_DEBUG("%s: x was < 0 (%d)\n", name, x);
>
> replace it by:
>
>trace_vmsvga_verify_rect_check_neg(name, x);
>
> and in hw/display/trace-events you add:
>
>vmsvga_verify_rect_check_neg(const char *name, int x) "%s: x was < 0 (%d)"
>
> Thanks,
> Laurent
Re: [PATCH 1/2] hw/display/vmware_vga: only show debug output if DEBUG enabled
Hey,
Thanks for getting back to me.
Yeah will take a look and update when I have a mo.
Cheers
Carwyn
> On 4 Jan 2022, at 09:18, Laurent Vivier wrote:
>
> Le 04/01/2022 à 10:11, Carwyn Ellis a écrit :
>> Debug output was always being sent to STDERR. This has been replaced by
>> a define that will only show this output when DEBUG is set to true.
>> Signed-off-by: Carwyn Ellis
>> ---
>> hw/display/vmware_vga.c | 26 --
>> 1 file changed, 16 insertions(+), 10 deletions(-)
>> diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
>> index e2969a6c81..8080e085d1 100644
>> --- a/hw/display/vmware_vga.c
>> +++ b/hw/display/vmware_vga.c
>> @@ -43,6 +43,12 @@
>>/* See http://vmware-svga.sf.net/ for some documentation on VMWare SVGA */
>> +#ifdef DEBUG
>> +#define VMWARE_VGA_DEBUG(...) { (void) fprintf(stdout, __VA_ARGS__); }
>> +#else
>> +#define VMWARE_VGA_DEBUG(...) ((void) 0)
>> +#endif
>> +
>
> Could you replace this macro by adding some trace-events instead.
>
> See https://qemu-project.gitlab.io/qemu/devel/tracing.html#using-trace-events
>
> Thanks,
> Laurent
>
>> struct vmsvga_state_s {
>> VGACommonState vga;
>> @@ -297,45 +303,45 @@ static inline bool vmsvga_verify_rect(DisplaySurface
>> *surface,
>>int x, int y, int w, int h)
>> {
>> if (x < 0) {
>> -fprintf(stderr, "%s: x was < 0 (%d)\n", name, x);
>> +VMWARE_VGA_DEBUG("%s: x was < 0 (%d)\n", name, x);
>> return false;
>> }
>> if (x > SVGA_MAX_WIDTH) {
>> -fprintf(stderr, "%s: x was > %d (%d)\n", name, SVGA_MAX_WIDTH, x);
>> +VMWARE_VGA_DEBUG("%s: x was > %d (%d)\n", name, SVGA_MAX_WIDTH, x);
>> return false;
>> }
>> if (w < 0) {
>> -fprintf(stderr, "%s: w was < 0 (%d)\n", name, w);
>> +VMWARE_VGA_DEBUG("%s: w was < 0 (%d)\n", name, w);
>> return false;
>> }
>> if (w > SVGA_MAX_WIDTH) {
>> -fprintf(stderr, "%s: w was > %d (%d)\n", name, SVGA_MAX_WIDTH, w);
>> +VMWARE_VGA_DEBUG("%s: w was > %d (%d)\n", name, SVGA_MAX_WIDTH, w);
>> return false;
>> }
>> if (x + w > surface_width(surface)) {
>> -fprintf(stderr, "%s: width was > %d (x: %d, w: %d)\n",
>> +VMWARE_VGA_DEBUG("%s: width was > %d (x: %d, w: %d)\n",
>> name, surface_width(surface), x, w);
>> return false;
>> }
>>if (y < 0) {
>> -fprintf(stderr, "%s: y was < 0 (%d)\n", name, y);
>> +VMWARE_VGA_DEBUG("%s: y was < 0 (%d)\n", name, y);
>> return false;
>> }
>> if (y > SVGA_MAX_HEIGHT) {
>> -fprintf(stderr, "%s: y was > %d (%d)\n", name, SVGA_MAX_HEIGHT, y);
>> +VMWARE_VGA_DEBUG("%s: y was > %d (%d)\n", name, SVGA_MAX_HEIGHT, y);
>> return false;
>> }
>> if (h < 0) {
>> -fprintf(stderr, "%s: h was < 0 (%d)\n", name, h);
>> +VMWARE_VGA_DEBUG("%s: h was < 0 (%d)\n", name, h);
>> return false;
>> }
>> if (h > SVGA_MAX_HEIGHT) {
>> -fprintf(stderr, "%s: h was > %d (%d)\n", name, SVGA_MAX_HEIGHT, h);
>> +VMWARE_VGA_DEBUG("%s: h was > %d (%d)\n", name, SVGA_MAX_HEIGHT, h);
>> return false;
>> }
>> if (y + h > surface_height(surface)) {
>> -fprintf(stderr, "%s: update height > %d (y: %d, h: %d)\n",
>> +VMWARE_VGA_DEBUG("%s: update height > %d (y: %d, h: %d)\n",
>> name, surface_height(surface), y, h);
>> return false;
>> }
>
[PATCH 1/2] hw/display/vmware_vga: only show debug output if DEBUG enabled
Debug output was always being sent to STDERR. This has been replaced by
a define that will only show this output when DEBUG is set to true.
Signed-off-by: Carwyn Ellis
---
hw/display/vmware_vga.c | 26 --
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index e2969a6c81..8080e085d1 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -43,6 +43,12 @@
/* See http://vmware-svga.sf.net/ for some documentation on VMWare SVGA */
+#ifdef DEBUG
+#define VMWARE_VGA_DEBUG(...) { (void) fprintf(stdout, __VA_ARGS__); }
+#else
+#define VMWARE_VGA_DEBUG(...) ((void) 0)
+#endif
+
struct vmsvga_state_s {
VGACommonState vga;
@@ -297,45 +303,45 @@ static inline bool vmsvga_verify_rect(DisplaySurface
*surface,
int x, int y, int w, int h)
{
if (x < 0) {
-fprintf(stderr, "%s: x was < 0 (%d)\n", name, x);
+VMWARE_VGA_DEBUG("%s: x was < 0 (%d)\n", name, x);
return false;
}
if (x > SVGA_MAX_WIDTH) {
-fprintf(stderr, "%s: x was > %d (%d)\n", name, SVGA_MAX_WIDTH, x);
+VMWARE_VGA_DEBUG("%s: x was > %d (%d)\n", name, SVGA_MAX_WIDTH, x);
return false;
}
if (w < 0) {
-fprintf(stderr, "%s: w was < 0 (%d)\n", name, w);
+VMWARE_VGA_DEBUG("%s: w was < 0 (%d)\n", name, w);
return false;
}
if (w > SVGA_MAX_WIDTH) {
-fprintf(stderr, "%s: w was > %d (%d)\n", name, SVGA_MAX_WIDTH, w);
+VMWARE_VGA_DEBUG("%s: w was > %d (%d)\n", name, SVGA_MAX_WIDTH, w);
return false;
}
if (x + w > surface_width(surface)) {
-fprintf(stderr, "%s: width was > %d (x: %d, w: %d)\n",
+VMWARE_VGA_DEBUG("%s: width was > %d (x: %d, w: %d)\n",
name, surface_width(surface), x, w);
return false;
}
if (y < 0) {
-fprintf(stderr, "%s: y was < 0 (%d)\n", name, y);
+VMWARE_VGA_DEBUG("%s: y was < 0 (%d)\n", name, y);
return false;
}
if (y > SVGA_MAX_HEIGHT) {
-fprintf(stderr, "%s: y was > %d (%d)\n", name, SVGA_MAX_HEIGHT, y);
+VMWARE_VGA_DEBUG("%s: y was > %d (%d)\n", name, SVGA_MAX_HEIGHT, y);
return false;
}
if (h < 0) {
-fprintf(stderr, "%s: h was < 0 (%d)\n", name, h);
+VMWARE_VGA_DEBUG("%s: h was < 0 (%d)\n", name, h);
return false;
}
if (h > SVGA_MAX_HEIGHT) {
-fprintf(stderr, "%s: h was > %d (%d)\n", name, SVGA_MAX_HEIGHT, h);
+VMWARE_VGA_DEBUG("%s: h was > %d (%d)\n", name, SVGA_MAX_HEIGHT, h);
return false;
}
if (y + h > surface_height(surface)) {
-fprintf(stderr, "%s: update height > %d (y: %d, h: %d)\n",
+VMWARE_VGA_DEBUG("%s: update height > %d (y: %d, h: %d)\n",
name, surface_height(surface), y, h);
return false;
}
--
2.34.1
[PATCH 2/2] hw/display/vmware_vga: do not discard screen updates
In certain circumstances, typically when there is lots changing on the
screen, updates will be discarded resulting in garbled output.
This change firstly increases the screen update FIFO size to ensure it's
large enough to accomodate all updates deferred in a given screen
refresh cycle.
When updating the screen all updates are applied to ensure the display
output is rendered correctly.
Signed-off-by: Carwyn Ellis
---
hw/display/vmware_vga.c | 24 +---
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index 8080e085d1..28556f39c6 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -82,7 +82,7 @@ struct vmsvga_state_s {
uint32_t fifo_next;
uint32_t fifo_stop;
-#define REDRAW_FIFO_LEN 512
+#define REDRAW_FIFO_LEN 8192
struct vmsvga_rect_s {
int x, y, w, h;
} redraw_fifo[REDRAW_FIFO_LEN];
@@ -385,7 +385,14 @@ static inline void vmsvga_update_rect_delayed(struct
vmsvga_state_s *s,
{
struct vmsvga_rect_s *rect = >redraw_fifo[s->redraw_fifo_last++];
-s->redraw_fifo_last &= REDRAW_FIFO_LEN - 1;
+if (s->redraw_fifo_last >= REDRAW_FIFO_LEN) {
+VMWARE_VGA_DEBUG("%s: Discarding updates - FIFO length %d exceeded\n",
+"vmsvga_update_rect_delayed",
+REDRAW_FIFO_LEN
+);
+s->redraw_fifo_last = REDRAW_FIFO_LEN - 1;
+}
+
rect->x = x;
rect->y = y;
rect->w = w;
@@ -402,11 +409,13 @@ static inline void vmsvga_update_rect_flush(struct
vmsvga_state_s *s)
}
/* Overlapping region updates can be optimised out here - if someone
* knows a smart algorithm to do that, please share. */
-while (s->redraw_fifo_first != s->redraw_fifo_last) {
-rect = >redraw_fifo[s->redraw_fifo_first++];
-s->redraw_fifo_first &= REDRAW_FIFO_LEN - 1;
+for (int i = 0; i < s->redraw_fifo_last; i++) {
+rect = >redraw_fifo[i];
vmsvga_update_rect(s, rect->x, rect->y, rect->w, rect->h);
}
+
+s->redraw_fifo_first = 0;
+s->redraw_fifo_last = 0;
}
#ifdef HW_RECT_ACCEL
@@ -607,13 +616,14 @@ static inline uint32_t vmsvga_fifo_read(struct
vmsvga_state_s *s)
static void vmsvga_fifo_run(struct vmsvga_state_s *s)
{
uint32_t cmd, colour;
-int args, len, maxloop = 1024;
+int args, len = 1024;
int x, y, dx, dy, width, height;
struct vmsvga_cursor_definition_s cursor;
uint32_t cmd_start;
len = vmsvga_fifo_length(s);
-while (len > 0 && --maxloop > 0) {
+
+while (len > 0) {
/* May need to go back to the start of the command if incomplete */
cmd_start = s->fifo_stop;
--
2.34.1
[PATCH 0/2] hw/display/vmware_vga: supress debug output and fix
Two patches addressing the following in the vmware vga display code - only show debug output if DEBUG is explicitly enabled - do not discard display updates This prevents an issue that can cause garbled display output when a high number of screen updates are being requested. The FIFO queue size has been increased and all display update events are now processed ensuring correct display output even during periods of high activity. Carwyn Ellis (2): hw/display/vmware_vga: only show debug output if DEBUG enabled hw/display/vmware_vga: do not discard screen updates hw/display/vmware_vga.c | 50 +++-- 1 file changed, 33 insertions(+), 17 deletions(-) -- 2.34.1
[PATCH 1/1] ui/cocoa: show/hide menu in fullscreen on mouse ungrab/grab
The menu bar is only accessible when the Cocoa UI is windowed. In order
to allow the menu bar to be accessible in fullscreen mode, this change
makes the menu visible when the mouse is ungrabbed.
When the mouse is grabbed the menu is hidden again.
Signed-off-by: Carwyn Ellis
---
ui/cocoa.m | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 69745c483b..42dcf47da4 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1037,7 +1037,9 @@ QemuCocoaView *cocoaView;
{
COCOA_DEBUG("QemuCocoaView: grabMouse\n");
-if (!isFullscreen) {
+if (isFullscreen) {
+[NSMenu setMenuBarVisible: FALSE];
+} else {
if (qemu_name)
[normalWindow setTitle:[NSString stringWithFormat:@"QEMU %s -
(Press ctrl + alt + g to release Mouse)", qemu_name]];
else
@@ -1052,7 +1054,9 @@ QemuCocoaView *cocoaView;
{
COCOA_DEBUG("QemuCocoaView: ungrabMouse\n");
-if (!isFullscreen) {
+if (isFullscreen) {
+[NSMenu setMenuBarVisible: TRUE];
+} else {
if (qemu_name)
[normalWindow setTitle:[NSString stringWithFormat:@"QEMU %s",
qemu_name]];
else
--
2.34.1
[PATCH 0/1] Show/hide the menu bar in fullscreen on mouse
Minor change to make fullscreen mode in the Cocoa UI a little more convenient. The menu bar is now made visible when the mouse is released (ungrabbed) making it accessible without having to leave fullscreen mode. Grabbing the mouse hides the menu. Carwyn Ellis (1): ui/cocoa: show/hide menu in fullscreen on mouse ungrab/grab ui/cocoa.m | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) -- 2.34.1
[PATCH 1/2] ui/cocoa: add option to disable left-command forwarding to guest
When switching between guest and host on a Mac using command-tab the
command key is sent to the guest which can trigger functionality in the
guest OS. Specifying left-command-key=off disables forwarding this key
to the guest. Defaults to enabled.
Also updated the cocoa display documentation to reference the new
left-command-key option along with the existing show-cursor option.
Signed-off-by: Carwyn Ellis
---
qapi/ui.json| 17 +
qemu-options.hx | 12
ui/cocoa.m | 8 +++-
3 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/qapi/ui.json b/qapi/ui.json
index 2b4371da37..764480e145 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1107,6 +1107,22 @@
'data': { '*grab-on-hover' : 'bool',
'*zoom-to-fit' : 'bool' } }
+##
+# @DisplayCocoa:
+#
+# Cocoa display options.
+#
+# @left-command-key: Enable/disable forwarding of left command key to
+#guest. Allows command-tab window switching on the
+#host without sending this key to the guest when
+#"off". Defaults to "on"
+#
+# Since: 6.2.50
+#
+##
+{ 'struct' : 'DisplayCocoa',
+ 'data': { '*left-command-key' : 'bool' } }
+
##
# @DisplayEGLHeadless:
#
@@ -1254,6 +1270,7 @@
'discriminator' : 'type',
'data': {
'gtk': { 'type': 'DisplayGTK', 'if': 'CONFIG_GTK' },
+ 'cocoa': { 'type': 'DisplayCocoa', 'if': 'CONFIG_COCOA' },
'curses': { 'type': 'DisplayCurses', 'if': 'CONFIG_CURSES' },
'egl-headless': { 'type': 'DisplayEGLHeadless',
'if': { 'all': ['CONFIG_OPENGL', 'CONFIG_GBM'] } },
diff --git a/qemu-options.hx b/qemu-options.hx
index fd1f8135fb..6fa9c38c83 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1912,6 +1912,9 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
#if defined(CONFIG_DBUS_DISPLAY)
"-display dbus[,addr=]\n"
" [,gl=on|core|es|off][,rendernode=]\n"
+#endif
+#if defined(CONFIG_COCOA)
+"-display cocoa[,show-cursor=on|off][,left-command-key=on|off]\n"
#endif
"-display none\n"
"select display backend type\n"
@@ -1999,6 +2002,15 @@ SRST
``charset=CP850`` for IBM CP850 encoding. The default is
``CP437``.
+``cocoa``
+Display video output in a Cocoa window. Mac only. This interface
+provides drop-down menus and other UI elements to configure and
+control the VM during runtime. Valid parameters are:
+
+``show-cursor=on|off`` : Force showing the mouse cursor
+
+``left-command-key=on|off`` : Disable forwarding left command key to
host
+
``egl-headless[,rendernode=]``
Offload all OpenGL operations to a local DRI device. For any
graphical display, this display needs to be paired with either
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 69745c483b..01045d6698 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -95,6 +95,7 @@ static DisplayChangeListener dcl = {
};
static int last_buttons;
static int cursor_hide = 1;
+static int left_command_key_enabled = 1;
static int gArgc;
static char **gArgv;
@@ -834,7 +835,8 @@ QemuCocoaView *cocoaView;
/* Don't pass command key changes to guest unless mouse is
grabbed */
case kVK_Command:
if (isMouseGrabbed &&
-!!(modifiers & NSEventModifierFlagCommand)) {
+!!(modifiers & NSEventModifierFlagCommand) &&
+left_command_key_enabled) {
[self toggleKey:Q_KEY_CODE_META_L];
}
break;
@@ -2054,6 +2056,10 @@ static void cocoa_display_init(DisplayState *ds,
DisplayOptions *opts)
cursor_hide = 0;
}
+if (opts->u.cocoa.has_left_command_key && !opts->u.cocoa.left_command_key)
{
+left_command_key_enabled = 0;
+}
+
// register vga output callbacks
register_displaychangelistener();
--
2.34.1
[PATCH 2/2] ui/cocoa: release mouse when user switches away from QEMU window
This resolves an issue where using command-tab to switch between QEMU
and other windows on the host can leave the mouse pointer visible.
By releasing the mouse when the user switches away, the user must left
click on the QEMU window when switching back in order to hide the
pointer and return control to the guest.
This appraoch ensures that the calls to NSCursor hide and unhide are
always balanced and thus work correctly when invoked.
Signed-off-by: Carwyn Ellis
---
ui/cocoa.m | 2 ++
1 file changed, 2 insertions(+)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 01045d6698..3f7af4a8fa 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1243,6 +1243,7 @@ QemuCocoaView *cocoaView;
- (void) applicationWillResignActive: (NSNotification *)aNotification
{
COCOA_DEBUG("QemuCocoaAppController: applicationWillResignActive\n");
+[cocoaView ungrabMouse];
[cocoaView raiseAllKeys];
}
@@ -2052,6 +2053,7 @@ static void cocoa_display_init(DisplayState *ds,
DisplayOptions *opts)
[(QemuCocoaAppController *)[[NSApplication sharedApplication]
delegate] toggleFullScreen: nil];
});
}
+
if (opts->has_show_cursor && opts->show_cursor) {
cursor_hide = 0;
}
--
2.34.1
[PATCH 0/2] ui/cocoa: Add option to disable left command and hide cursor on click
Supersedes earlier submissions and splits the patch into two separate patches covering - addition of left-command-key option to disable forwarding this key to the guest - fix for persistent mouse cursor when switching from and back to the QEMU window Having made the switch to an M1 Mac I needed to switch from VMware back to QEMU in order to run some intel guests. This patch addresses a couple of niggles with the cocoa UI, namely: - Using command-tab to switch between the guest OS and MacOS sends the command keypress to the guest which can be annoying e.g. on a windows guest this may trigger the start menu - Switching between the guest and MacOS sometimes leaves the MacOS mouse cursor visible with no way to hide it without switching windows again I've made the following changes - Added a new cocoa display option left-command-key which can be used to disable the left command key in the guest. Default is on. - Added a call to ungrabMouse in the applicationWillResignActive method which frees the mouse and unhides the cursor when switching away from the QEMU window. When switching back the user must left-click in to grab the mouse and hide the cursor again. After testing several different approaches this was the only way I could find to reliably hide the cursor every time the user returns to QEMU after switching to another app on the host machine. - Updated the command line docs to reference the show-cursor option which is also respected by the cocoa UI code. Carwyn Ellis (2): ui/cocoa: add option to disable left-command forwarding to guest ui/cocoa: release mouse when user switches away from QEMU window qapi/ui.json| 17 + qemu-options.hx | 12 ui/cocoa.m | 10 +- 3 files changed, 38 insertions(+), 1 deletion(-) -- 2.34.1
Re: [PATCH 1/1] ui/cocoa: Add option to disable left command and hide cursor on click
> On 31 Dec 2021, at 17:56, Alexander Orzechowski
> wrote:
>
>
> On 12/31/21 12:42, Carwyn Ellis wrote:
>> When switching between guest and host on a Mac using command-tab the
>> command key is sent to the guest which can trigger functionality in the
>> guest OS. Specifying left-command-key=off disables forwarding this key
>> to the guest. Defaults to enabled.
>>
>> When switching between guest and host on a Mac with a fullscreen guest
>> the host pointer will occasionally persist despite the ui code
>> requesting that it be hidden. Added cursor hide calls on left and right
>> mouse click to hide the cursor when the mouse is clicked.
>>
>> Also updated the cocoa display documentation to reference the new
>> left-command-key option along with the existing show-cursor option.
>> ---
>> qapi/ui.json| 17 +
>> qemu-options.hx | 12
>> ui/cocoa.m | 33 +
>> 3 files changed, 54 insertions(+), 8 deletions(-)
>>
>> diff --git a/qapi/ui.json b/qapi/ui.json
>> index 2b4371da37..764480e145 100644
>> --- a/qapi/ui.json
>> +++ b/qapi/ui.json
>> @@ -1107,6 +1107,22 @@
>>'data': { '*grab-on-hover' : 'bool',
>> '*zoom-to-fit' : 'bool' } }
>> +##
>> +# @DisplayCocoa:
>> +#
>> +# Cocoa display options.
>> +#
>> +# @left-command-key: Enable/disable forwarding of left command key to
>> +#guest. Allows command-tab window switching on the
>> +#host without sending this key to the guest when
>> +#"off". Defaults to "on"
>> +#
>> +# Since: 6.2.50
>> +#
>> +##
>> +{ 'struct' : 'DisplayCocoa',
>> + 'data': { '*left-command-key' : 'bool' } }
>> +
>> ##
>> # @DisplayEGLHeadless:
>> #
>> @@ -1254,6 +1270,7 @@
>>'discriminator' : 'type',
>>'data': {
>>'gtk': { 'type': 'DisplayGTK', 'if': 'CONFIG_GTK' },
>> + 'cocoa': { 'type': 'DisplayCocoa', 'if': 'CONFIG_COCOA' },
>>'curses': { 'type': 'DisplayCurses', 'if': 'CONFIG_CURSES' },
>>'egl-headless': { 'type': 'DisplayEGLHeadless',
>> 'if': { 'all': ['CONFIG_OPENGL', 'CONFIG_GBM'] } },
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 7d47510947..5214457676 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -1869,6 +1869,9 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
>> #if defined(CONFIG_DBUS_DISPLAY)
>> "-display dbus[,addr=]\n"
>> " [,gl=on|core|es|off][,rendernode=]\n"
>> +#endif
>> +#if defined(CONFIG_COCOA)
>> +"-display cocoa[,show-cursor=on|off][,left-command-key=on|off]\n"
>> #endif
>> "-display none\n"
>> "select display backend type\n"
>> @@ -1956,6 +1959,15 @@ SRST
>> ``charset=CP850`` for IBM CP850 encoding. The default is
>> ``CP437``.
>> +``cocoa``
>> +Display video output in a Cocoa window. Mac only. This interface
>> +provides drop-down menus and other UI elements to configure and
>> +control the VM during runtime. Valid parameters are:
>> +
>> +``show-cursor=on|off`` : Force showing the mouse cursor
>> +
>> +``left-command-key=on|off`` : Disable forwarding left command key
>> to host
>> +
>> ``egl-headless[,rendernode=]``
>> Offload all OpenGL operations to a local DRI device. For any
>> graphical display, this display needs to be paired with either
>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>> index 69745c483b..10e492538a 100644
>> --- a/ui/cocoa.m
>> +++ b/ui/cocoa.m
>> @@ -95,6 +95,8 @@ static DisplayChangeListener dcl = {
>> };
>> static int last_buttons;
>> static int cursor_hide = 1;
>> +static bool cursor_visible = 1;
>> +static int left_command_key_enabled = 1;
>>static int gArgc;
>> static char **gArgv;
>> @@ -411,18 +413,18 @@ QemuCocoaView *cocoaView;
>>- (void) hideCursor
>> {
>> -if (!cursor_hide) {
>> -return;
>> +if (cursor_hide && cursor_visible) {
>> +cursor_visible = 0;
>> +[NSCursor hide];
>> }
>> -[NSCursor hide];
>> }
>>- (void) unhideCursor
>> {
>> -if (!cursor_hide) {
>> -return;
>> +if (curso
Re: [PATCH 1/1] ui/cocoa: Add option to disable left command and hide cursor on click
Ok cool, I’ll separate this out into 2 patches for the fullscreen and
left-command changes respectively.
Will aim to get this out at some point tomorrow.
Cheers
Carwyn
> On 31 Dec 2021, at 17:49, Alexander Orzechowski
> wrote:
>
>
> On 12/31/21 12:42, Carwyn Ellis wrote:
>> When switching between guest and host on a Mac using command-tab the
>> command key is sent to the guest which can trigger functionality in the
>> guest OS. Specifying left-command-key=off disables forwarding this key
>> to the guest. Defaults to enabled.
>>
>> When switching between guest and host on a Mac with a fullscreen guest
>> the host pointer will occasionally persist despite the ui code
>> requesting that it be hidden. Added cursor hide calls on left and right
>> mouse click to hide the cursor when the mouse is clicked.
>>
>> Also updated the cocoa display documentation to reference the new
>> left-command-key option along with the existing show-cursor option.
>> ---
>> qapi/ui.json| 17 +
>> qemu-options.hx | 12
>> ui/cocoa.m | 33 +
>> 3 files changed, 54 insertions(+), 8 deletions(-)
>>
>> diff --git a/qapi/ui.json b/qapi/ui.json
>> index 2b4371da37..764480e145 100644
>> --- a/qapi/ui.json
>> +++ b/qapi/ui.json
>> @@ -1107,6 +1107,22 @@
>>'data': { '*grab-on-hover' : 'bool',
>> '*zoom-to-fit' : 'bool' } }
>> +##
>> +# @DisplayCocoa:
>> +#
>> +# Cocoa display options.
>> +#
>> +# @left-command-key: Enable/disable forwarding of left command key to
>> +#guest. Allows command-tab window switching on the
>> +#host without sending this key to the guest when
>> +#"off". Defaults to "on"
>> +#
>> +# Since: 6.2.50
>> +#
>> +##
>> +{ 'struct' : 'DisplayCocoa',
>> + 'data': { '*left-command-key' : 'bool' } }
>> +
>> ##
>> # @DisplayEGLHeadless:
>> #
>> @@ -1254,6 +1270,7 @@
>>'discriminator' : 'type',
>>'data': {
>>'gtk': { 'type': 'DisplayGTK', 'if': 'CONFIG_GTK' },
>> + 'cocoa': { 'type': 'DisplayCocoa', 'if': 'CONFIG_COCOA' },
>>'curses': { 'type': 'DisplayCurses', 'if': 'CONFIG_CURSES' },
>>'egl-headless': { 'type': 'DisplayEGLHeadless',
>> 'if': { 'all': ['CONFIG_OPENGL', 'CONFIG_GBM'] } },
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 7d47510947..5214457676 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -1869,6 +1869,9 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
>> #if defined(CONFIG_DBUS_DISPLAY)
>> "-display dbus[,addr=]\n"
>> " [,gl=on|core|es|off][,rendernode=]\n"
>> +#endif
>> +#if defined(CONFIG_COCOA)
>> +"-display cocoa[,show-cursor=on|off][,left-command-key=on|off]\n"
>> #endif
>> "-display none\n"
>> "select display backend type\n"
>> @@ -1956,6 +1959,15 @@ SRST
>> ``charset=CP850`` for IBM CP850 encoding. The default is
>> ``CP437``.
>> +``cocoa``
>> +Display video output in a Cocoa window. Mac only. This interface
>> +provides drop-down menus and other UI elements to configure and
>> +control the VM during runtime. Valid parameters are:
>> +
>> +``show-cursor=on|off`` : Force showing the mouse cursor
>> +
>> +``left-command-key=on|off`` : Disable forwarding left command key
>> to host
>> +
>> ``egl-headless[,rendernode=]``
>> Offload all OpenGL operations to a local DRI device. For any
>> graphical display, this display needs to be paired with either
>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>> index 69745c483b..10e492538a 100644
>> --- a/ui/cocoa.m
>> +++ b/ui/cocoa.m
>> @@ -95,6 +95,8 @@ static DisplayChangeListener dcl = {
>> };
>> static int last_buttons;
>> static int cursor_hide = 1;
>> +static bool cursor_visible = 1;
>> +static int left_command_key_enabled = 1;
>>static int gArgc;
>> static char **gArgv;
>> @@ -411,18 +413,18 @@ QemuCocoaView *cocoaView;
>>- (void) hideCursor
>> {
>> -if (!cursor_hide) {
>> -return;
>> +if (cursor_hide && cursor_visible) {
>> +cursor_visible = 0;
>> +[NSCursor hide];
>>
[PATCH 1/1] ui/cocoa: Add option to disable left command and hide cursor on click
When switching between guest and host on a Mac using command-tab the
command key is sent to the guest which can trigger functionality in the
guest OS. Specifying left-command-key=off disables forwarding this key
to the guest. Defaults to enabled.
When switching between guest and host on a Mac with a fullscreen guest
the host pointer will occasionally persist despite the ui code
requesting that it be hidden. Added cursor hide calls on left and right
mouse click to hide the cursor when the mouse is clicked.
Also updated the cocoa display documentation to reference the new
left-command-key option along with the existing show-cursor option.
---
qapi/ui.json| 17 +
qemu-options.hx | 12
ui/cocoa.m | 33 +
3 files changed, 54 insertions(+), 8 deletions(-)
diff --git a/qapi/ui.json b/qapi/ui.json
index 2b4371da37..764480e145 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1107,6 +1107,22 @@
'data': { '*grab-on-hover' : 'bool',
'*zoom-to-fit' : 'bool' } }
+##
+# @DisplayCocoa:
+#
+# Cocoa display options.
+#
+# @left-command-key: Enable/disable forwarding of left command key to
+#guest. Allows command-tab window switching on the
+#host without sending this key to the guest when
+#"off". Defaults to "on"
+#
+# Since: 6.2.50
+#
+##
+{ 'struct' : 'DisplayCocoa',
+ 'data': { '*left-command-key' : 'bool' } }
+
##
# @DisplayEGLHeadless:
#
@@ -1254,6 +1270,7 @@
'discriminator' : 'type',
'data': {
'gtk': { 'type': 'DisplayGTK', 'if': 'CONFIG_GTK' },
+ 'cocoa': { 'type': 'DisplayCocoa', 'if': 'CONFIG_COCOA' },
'curses': { 'type': 'DisplayCurses', 'if': 'CONFIG_CURSES' },
'egl-headless': { 'type': 'DisplayEGLHeadless',
'if': { 'all': ['CONFIG_OPENGL', 'CONFIG_GBM'] } },
diff --git a/qemu-options.hx b/qemu-options.hx
index 7d47510947..5214457676 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1869,6 +1869,9 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
#if defined(CONFIG_DBUS_DISPLAY)
"-display dbus[,addr=]\n"
" [,gl=on|core|es|off][,rendernode=]\n"
+#endif
+#if defined(CONFIG_COCOA)
+"-display cocoa[,show-cursor=on|off][,left-command-key=on|off]\n"
#endif
"-display none\n"
"select display backend type\n"
@@ -1956,6 +1959,15 @@ SRST
``charset=CP850`` for IBM CP850 encoding. The default is
``CP437``.
+``cocoa``
+Display video output in a Cocoa window. Mac only. This interface
+provides drop-down menus and other UI elements to configure and
+control the VM during runtime. Valid parameters are:
+
+``show-cursor=on|off`` : Force showing the mouse cursor
+
+``left-command-key=on|off`` : Disable forwarding left command key to
host
+
``egl-headless[,rendernode=]``
Offload all OpenGL operations to a local DRI device. For any
graphical display, this display needs to be paired with either
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 69745c483b..10e492538a 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -95,6 +95,8 @@ static DisplayChangeListener dcl = {
};
static int last_buttons;
static int cursor_hide = 1;
+static bool cursor_visible = 1;
+static int left_command_key_enabled = 1;
static int gArgc;
static char **gArgv;
@@ -411,18 +413,18 @@ QemuCocoaView *cocoaView;
- (void) hideCursor
{
-if (!cursor_hide) {
-return;
+if (cursor_hide && cursor_visible) {
+cursor_visible = 0;
+[NSCursor hide];
}
-[NSCursor hide];
}
- (void) unhideCursor
{
-if (!cursor_hide) {
-return;
+if (cursor_hide && !cursor_visible) {
+cursor_visible = 1;
+[NSCursor unhide];
}
-[NSCursor unhide];
}
- (void) drawRect:(NSRect) rect
@@ -831,10 +833,14 @@ QemuCocoaView *cocoaView;
}
break;
-/* Don't pass command key changes to guest unless mouse is
grabbed */
+/*
+Don't pass command key changes to guest unless mouse is
grabbed
+or the key is explicitly disabled using the
left-command-key option
+*/
case kVK_Command:
if (isMouseGrabbed &&
-!!(modifiers & NSEventModifierFlagCommand)) {
+!!(modifiers & NSEventModifierFlagCommand) &&
+left_command_key_enabled) {
[self toggleKey:Q_KEY_CODE_META_L];
}
break;
@@ -923,10 +929,16 @@ QemuCocoaView *cocoaView;
case NSEventTypeLeftMouseDown:
buttons |= MOUSE_EVENT_LBUTTON;
mouse_event = true;
+if (isFullscreen) {
+[self hideCursor];
+}
break;
case
[PATCH 0/1] ui/cocoa: Add option to disable left command and hide cursor on click
Apologies for all the spam on what should be a simple change. Still getting the hang of all of this. :/ Please disregard my earlier submissions. After further testing I realised that the calls to cursor hide/unhide weren't balanced which broke the hide/unhide behaviour. I've added an additional static flag to track the cursor state so the cursor state is only updated where this would change the existing cursor state, ensuring the calls are now balanced. Having made the switch to an M1 Mac I needed to switch from VMware back to QEMU in order to run some intel guests. This patch addresses a couple of niggles with the cocoa UI, namely: - Using command-tab to switch between the guest OS and MacOS sends the command keypress to the guest which can be annoying e.g. on a windows guest this may trigger the start menu - Switching between the guest and MacOS sometimes leaves the MacOS mouse cursor visible with no way to hide it without switching windows again To address these issues I've made the following changes - Added a new cocoa display option left-command-key which can be used to disable the left command key in the guest. Default is on. - Added a call to hideCursor on left and right mouse clicks so if the cursor is visible after switching back to the guest a mouse click will hide the cursor again. - Also updated the command line docs to reference the show-cursor option which is also respected by the cocoa UI code. Carwyn Ellis (1): ui/cocoa: Add option to disable left command and hide cursor on click qapi/ui.json| 17 + qemu-options.hx | 12 ui/cocoa.m | 33 + 3 files changed, 54 insertions(+), 8 deletions(-) -- 2.34.1
[PATCH 0/1] ui/cocoa: Add option to disable left command and hide cursor on click
Having made the switch to an M1 Mac I needed to switch from VMware back to QEMU in order to run some intel guests. This patch addresses a couple of niggles with the cocoa UI, namely: - Using command-tab to switch between the guest OS and MacOS sends the command keypress to the guest which can be annoying e.g. on a windows guest this may trigger the start menu - Switching between the guest and MacOS sometimes leaves the MacOS mouse cursor visible with no way to hide it without switching windows again To address these issues I've made the following changes - Added a new cocoa display option left-command-key which can be used to disable the left command key in the guest. Default is on. - Added a call to hideCursor on left and right mouse clicks so if the cursor is visible after switching back to the guest a mouse click will hide the cursor again. - Also updated the command line docs to reference the show-cursor option which is also respected by the cocoa UI code. Carwyn Ellis (1): ui/cocoa: Add option to disable left command and hide cursor on click qapi/ui.json| 17 + qemu-options.hx | 12 ui/cocoa.m | 16 ++-- 3 files changed, 43 insertions(+), 2 deletions(-) -- 2.34.1
[PATCH 1/1] ui/cocoa: Add option to disable left command and hide cursor on click
When switching between guest and host on a Mac using command-tab the
command key is sent to the guest which can trigger functionality in the
guest OS. Specifying left-command-key=off disables forwarding this key
to the guest. Defaults to enabled.
When switching between guest and host on a Mac with a fullscreen guest
the host pointer will occasionally persist despite the ui code
requesting that it be hidden. Added cursor hide calls on left and right
mouse click to hide the cursor when the mouse is clicked.
Also updated the cocoa display documentation to reference the new
left-command-key option along with the existing show-cursor option.
---
qapi/ui.json| 17 +
qemu-options.hx | 12
ui/cocoa.m | 16 ++--
3 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/qapi/ui.json b/qapi/ui.json
index 2b4371da37..dd1569ed4b 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1107,6 +1107,22 @@
'data': { '*grab-on-hover' : 'bool',
'*zoom-to-fit' : 'bool' } }
+##
+# @DisplayCocoa:
+#
+# Cocoa display options.
+#
+# @left-command-key: Enable/disable forwarding of left command key to
+#guest. Allows command-tab window switching on the
+#host without sending this key to the guest when
+#"off". Defaults to "on"
+#
+# Since: 6.2.50
+#
+##
+{ 'struct' : 'DisplayCocoa',
+ 'data': { '*left-command-key' : 'bool' } }
+
##
# @DisplayEGLHeadless:
#
@@ -1254,6 +1270,7 @@
'discriminator' : 'type',
'data': {
'gtk': { 'type': 'DisplayGTK', 'if': 'CONFIG_GTK' },
+ 'cocoa': { 'type': 'DisplayCocoa', 'if': 'CONFIG_COCOA' },
'curses': { 'type': 'DisplayCurses', 'if': 'CONFIG_CURSES' },
'egl-headless': { 'type': 'DisplayEGLHeadless',
'if': { 'all': ['CONFIG_OPENGL', 'CONFIG_GBM'] } },
diff --git a/qemu-options.hx b/qemu-options.hx
index 7d47510947..5214457676 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1869,6 +1869,9 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
#if defined(CONFIG_DBUS_DISPLAY)
"-display dbus[,addr=]\n"
" [,gl=on|core|es|off][,rendernode=]\n"
+#endif
+#if defined(CONFIG_COCOA)
+"-display cocoa[,show-cursor=on|off][,left-command-key=on|off]\n"
#endif
"-display none\n"
"select display backend type\n"
@@ -1956,6 +1959,15 @@ SRST
``charset=CP850`` for IBM CP850 encoding. The default is
``CP437``.
+``cocoa``
+Display video output in a Cocoa window. Mac only. This interface
+provides drop-down menus and other UI elements to configure and
+control the VM during runtime. Valid parameters are:
+
+``show-cursor=on|off`` : Force showing the mouse cursor
+
+``left-command-key=on|off`` : Disable forwarding left command key to
host
+
``egl-headless[,rendernode=]``
Offload all OpenGL operations to a local DRI device. For any
graphical display, this display needs to be paired with either
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 69745c483b..105b742831 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -95,6 +95,7 @@ static DisplayChangeListener dcl = {
};
static int last_buttons;
static int cursor_hide = 1;
+static int left_command_key_enabled = 1;
static int gArgc;
static char **gArgv;
@@ -831,10 +832,14 @@ QemuCocoaView *cocoaView;
}
break;
-/* Don't pass command key changes to guest unless mouse is
grabbed */
+/*
+Don't pass command key changes to guest unless mouse is
grabbed
+or the key is explicitly disabled using the
left-command-key option
+*/
case kVK_Command:
if (isMouseGrabbed &&
-!!(modifiers & NSEventModifierFlagCommand)) {
+!!(modifiers & NSEventModifierFlagCommand) &&
+left_command_key_enabled) {
[self toggleKey:Q_KEY_CODE_META_L];
}
break;
@@ -923,10 +928,12 @@ QemuCocoaView *cocoaView;
case NSEventTypeLeftMouseDown:
buttons |= MOUSE_EVENT_LBUTTON;
mouse_event = true;
+[self hideCursor];
break;
case NSEventTypeRightMouseDown:
buttons |= MOUSE_EVENT_RBUTTON;
mouse_event = true;
+[self hideCursor];
break;
case NSEventTypeOtherMouseDown:
buttons |= MOUSE_EVENT_MBUTTON;
@@ -2050,10 +2057,15 @@ static void cocoa_display_init(DisplayState *ds,
DisplayOptions *opts)
[(QemuCocoaAppController *)[[NSApplication sharedApplication]
delegate] toggleFullScreen: nil];
});
}
+
if (opts->has_show_cursor && opts->show_cursor) {
cursor_hide = 0;
}
+if
