* [PATCH] ui/cocoa: Set UI information
@ 2021-06-16 14:19 Akihiko Odaki
2021-06-23 12:43 ` Gerd Hoffmann
2022-02-04 16:19 ` Peter Maydell
0 siblings, 2 replies; 9+ messages in thread
From: Akihiko Odaki @ 2021-06-16 14:19 UTC (permalink / raw)
Cc: Peter Maydell, qemu-devel, Akihiko Odaki, Gerd Hoffmann
Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
---
ui/cocoa.m | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 995301502be..8b83f91723a 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -540,6 +540,43 @@ - (void) setContentDimensions
}
}
+- (void) updateUIInfo
+{
+ NSSize frameSize;
+ QemuUIInfo info;
+
+ if (!qemu_console_is_graphic(dcl.con)) {
+ return;
+ }
+
+ if ([self window]) {
+ NSDictionary *description = [[[self window] screen] deviceDescription];
+ CGDirectDisplayID display = [[description objectForKey:@"NSScreenNumber"] unsignedIntValue];
+ NSSize screenSize = [[[self window] screen] frame].size;
+ CGSize screenPhysicalSize = CGDisplayScreenSize(display);
+
+ frameSize = isFullscreen ? screenSize : [self frame].size;
+ info.width_mm = frameSize.width / screenSize.width * screenPhysicalSize.width;
+ info.height_mm = frameSize.height / screenSize.height * screenPhysicalSize.height;
+ } else {
+ frameSize = [self frame].size;
+ info.width_mm = 0;
+ info.height_mm = 0;
+ }
+
+ info.xoff = 0;
+ info.yoff = 0;
+ info.width = frameSize.width;
+ info.height = frameSize.height;
+
+ dpy_set_ui_info(dcl.con, &info);
+}
+
+- (void)viewDidMoveToWindow
+{
+ [self updateUIInfo];
+}
+
- (void) switchSurface:(pixman_image_t *)image
{
COCOA_DEBUG("QemuCocoaView: switchSurface\n");
@@ -1258,6 +1295,16 @@ - (NSApplicationTerminateReply)applicationShouldTerminate:
return [self verifyQuit];
}
+- (void)windowDidChangeScreen:(NSNotification *)notification
+{
+ [cocoaView updateUIInfo];
+}
+
+- (void)windowDidResize:(NSNotification *)notification
+{
+ [cocoaView updateUIInfo];
+}
+
/* Called when the user clicks on a window's close button */
- (BOOL)windowShouldClose:(id)sender
{
@@ -1936,6 +1983,8 @@ static void cocoa_switch(DisplayChangeListener *dcl,
COCOA_DEBUG("qemu_cocoa: cocoa_switch\n");
+ [cocoaView updateUIInfo];
+
// The DisplaySurface will be freed as soon as this callback returns.
// We take a reference to the underlying pixman image here so it does
// not disappear from under our feet; the switchSurface method will
--
2.30.1 (Apple Git-130)
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] ui/cocoa: Set UI information
2021-06-16 14:19 [PATCH] ui/cocoa: Set UI information Akihiko Odaki
@ 2021-06-23 12:43 ` Gerd Hoffmann
2022-02-04 16:19 ` Peter Maydell
1 sibling, 0 replies; 9+ messages in thread
From: Gerd Hoffmann @ 2021-06-23 12:43 UTC (permalink / raw)
To: Akihiko Odaki; +Cc: Peter Maydell, qemu-devel
On Wed, Jun 16, 2021 at 11:19:10PM +0900, Akihiko Odaki wrote:
> Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
> ---
> ui/cocoa.m | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 995301502be..8b83f91723a 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -540,6 +540,43 @@ - (void) setContentDimensions
> }
> }
Added to UI queue.
thanks,
Gerd
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ui/cocoa: Set UI information
2021-06-16 14:19 [PATCH] ui/cocoa: Set UI information Akihiko Odaki
2021-06-23 12:43 ` Gerd Hoffmann
@ 2022-02-04 16:19 ` Peter Maydell
2022-02-05 2:06 ` Akihiko Odaki
2022-02-14 10:27 ` Gerd Hoffmann
1 sibling, 2 replies; 9+ messages in thread
From: Peter Maydell @ 2022-02-04 16:19 UTC (permalink / raw)
To: Akihiko Odaki; +Cc: qemu-devel, Gerd Hoffmann
On Wed, 16 Jun 2021 at 15:19, Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
> ---
> ui/cocoa.m | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
Hi; I was looking at the cocoa.m code to see about maybe deleting the
unnecessary autorelease pools, and I ran into some code I was a bit
unsure about that was added in this patch.
In particular I'm wondering about the interactions across threads here.
> +- (void) updateUIInfo
> +{
> + NSSize frameSize;
> + QemuUIInfo info;
> +
> + if (!qemu_console_is_graphic(dcl.con)) {
> + return;
> + }
> +
> + if ([self window]) {
> + NSDictionary *description = [[[self window] screen] deviceDescription];
> + CGDirectDisplayID display = [[description objectForKey:@"NSScreenNumber"] unsignedIntValue];
> + NSSize screenSize = [[[self window] screen] frame].size;
> + CGSize screenPhysicalSize = CGDisplayScreenSize(display);
> +
> + frameSize = isFullscreen ? screenSize : [self frame].size;
> + info.width_mm = frameSize.width / screenSize.width * screenPhysicalSize.width;
> + info.height_mm = frameSize.height / screenSize.height * screenPhysicalSize.height;
> + } else {
> + frameSize = [self frame].size;
> + info.width_mm = 0;
> + info.height_mm = 0;
> + }
> +
> + info.xoff = 0;
> + info.yoff = 0;
> + info.width = frameSize.width;
> + info.height = frameSize.height;
> +
> + dpy_set_ui_info(dcl.con, &info);
This function makes some cocoa calls, and it also calls a QEMU
UI layer function, dpy_set_ui_info().
> +- (void)windowDidChangeScreen:(NSNotification *)notification
> +{
> + [cocoaView updateUIInfo];
We call it from the cocoa UI thread in callbacks like this.
> /* Called when the user clicks on a window's close button */
> - (BOOL)windowShouldClose:(id)sender
> {
> @@ -1936,6 +1983,8 @@ static void cocoa_switch(DisplayChangeListener *dcl,
>
> COCOA_DEBUG("qemu_cocoa: cocoa_switch\n");
>
> + [cocoaView updateUIInfo];
We also call it from the QEMU thread, when the QEMU thread calls
this cocoa_switch callback function.
(1) A question for Akihiko:
Are all the cocoa calls we make in updateUIInfo safe to
make from a non-UI thread? Would it be preferable for this
call in cocoa_switch() to be moved inside the dispatch_async block?
(Moving it would mean that I wouldn't have to think about whether
any of the code in it needs to have an autorelease pool :-))
(2) A question for Gerd:
Is it safe to call dpy_set_ui_info() from a non-QEMU-main-thread?
It doesn't appear to do any locking that would protect against
multiple threads calling it simultaneously.
thanks
-- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ui/cocoa: Set UI information
2022-02-04 16:19 ` Peter Maydell
@ 2022-02-05 2:06 ` Akihiko Odaki
2022-02-08 18:07 ` Peter Maydell
2022-02-14 10:27 ` Gerd Hoffmann
1 sibling, 1 reply; 9+ messages in thread
From: Akihiko Odaki @ 2022-02-05 2:06 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu Developers, Gerd Hoffmann
On Sat, Feb 5, 2022 at 1:19 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Wed, 16 Jun 2021 at 15:19, Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
> >
> > Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
> > ---
> > ui/cocoa.m | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 49 insertions(+)
>
> Hi; I was looking at the cocoa.m code to see about maybe deleting the
> unnecessary autorelease pools, and I ran into some code I was a bit
> unsure about that was added in this patch.
>
> In particular I'm wondering about the interactions across threads here.
>
> > +- (void) updateUIInfo
> > +{
> > + NSSize frameSize;
> > + QemuUIInfo info;
> > +
> > + if (!qemu_console_is_graphic(dcl.con)) {
> > + return;
> > + }
> > +
> > + if ([self window]) {
> > + NSDictionary *description = [[[self window] screen] deviceDescription];
> > + CGDirectDisplayID display = [[description objectForKey:@"NSScreenNumber"] unsignedIntValue];
> > + NSSize screenSize = [[[self window] screen] frame].size;
> > + CGSize screenPhysicalSize = CGDisplayScreenSize(display);
> > +
> > + frameSize = isFullscreen ? screenSize : [self frame].size;
> > + info.width_mm = frameSize.width / screenSize.width * screenPhysicalSize.width;
> > + info.height_mm = frameSize.height / screenSize.height * screenPhysicalSize.height;
> > + } else {
> > + frameSize = [self frame].size;
> > + info.width_mm = 0;
> > + info.height_mm = 0;
> > + }
> > +
> > + info.xoff = 0;
> > + info.yoff = 0;
> > + info.width = frameSize.width;
> > + info.height = frameSize.height;
> > +
> > + dpy_set_ui_info(dcl.con, &info);
>
> This function makes some cocoa calls, and it also calls a QEMU
> UI layer function, dpy_set_ui_info().
>
> > +- (void)windowDidChangeScreen:(NSNotification *)notification
> > +{
> > + [cocoaView updateUIInfo];
>
> We call it from the cocoa UI thread in callbacks like this.
>
> > /* Called when the user clicks on a window's close button */
> > - (BOOL)windowShouldClose:(id)sender
> > {
> > @@ -1936,6 +1983,8 @@ static void cocoa_switch(DisplayChangeListener *dcl,
> >
> > COCOA_DEBUG("qemu_cocoa: cocoa_switch\n");
> >
> > + [cocoaView updateUIInfo];
>
> We also call it from the QEMU thread, when the QEMU thread calls
> this cocoa_switch callback function.
>
> (1) A question for Akihiko:
> Are all the cocoa calls we make in updateUIInfo safe to
> make from a non-UI thread? Would it be preferable for this
> call in cocoa_switch() to be moved inside the dispatch_async block?
> (Moving it would mean that I wouldn't have to think about whether
> any of the code in it needs to have an autorelease pool :-))
It is not safe. Apparently I totally forgot about threads when I wrote this.
It should be moved in the dispatch_async block as you suggest. Should
I write a patch, or will you write one before you delete autorelease
pools?
Regards,
Akihiko Odaki
>
> (2) A question for Gerd:
> Is it safe to call dpy_set_ui_info() from a non-QEMU-main-thread?
> It doesn't appear to do any locking that would protect against
> multiple threads calling it simultaneously.
>
> thanks
> -- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ui/cocoa: Set UI information
2022-02-05 2:06 ` Akihiko Odaki
@ 2022-02-08 18:07 ` Peter Maydell
2022-02-09 11:32 ` Akihiko Odaki
0 siblings, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2022-02-08 18:07 UTC (permalink / raw)
To: Akihiko Odaki; +Cc: qemu Developers, Gerd Hoffmann
On Sat, 5 Feb 2022 at 02:06, Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
>
> On Sat, Feb 5, 2022 at 1:19 AM Peter Maydell <peter.maydell@linaro.org> wrote
> > (1) A question for Akihiko:
> > Are all the cocoa calls we make in updateUIInfo safe to
> > make from a non-UI thread? Would it be preferable for this
> > call in cocoa_switch() to be moved inside the dispatch_async block?
> > (Moving it would mean that I wouldn't have to think about whether
> > any of the code in it needs to have an autorelease pool :-))
>
> It is not safe. Apparently I totally forgot about threads when I wrote this.
>
> It should be moved in the dispatch_async block as you suggest. Should
> I write a patch, or will you write one before you delete autorelease
> pools?
I'll write a patchset. If you have time to test it when I send it out
that would be great.
Incidentally, I think the answer to my other question
> > Is it safe to call dpy_set_ui_info() from a non-QEMU-main-thread?
is "no, and so the body of updateUIInfo should be enclosed in a
with_iothread_lock block".
-- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ui/cocoa: Set UI information
2022-02-08 18:07 ` Peter Maydell
@ 2022-02-09 11:32 ` Akihiko Odaki
0 siblings, 0 replies; 9+ messages in thread
From: Akihiko Odaki @ 2022-02-09 11:32 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu Developers, Gerd Hoffmann
On Wed, Feb 9, 2022 at 3:07 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Sat, 5 Feb 2022 at 02:06, Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
> >
> > On Sat, Feb 5, 2022 at 1:19 AM Peter Maydell <peter.maydell@linaro.org> wrote
>
> > > (1) A question for Akihiko:
> > > Are all the cocoa calls we make in updateUIInfo safe to
> > > make from a non-UI thread? Would it be preferable for this
> > > call in cocoa_switch() to be moved inside the dispatch_async block?
> > > (Moving it would mean that I wouldn't have to think about whether
> > > any of the code in it needs to have an autorelease pool :-))
> >
> > It is not safe. Apparently I totally forgot about threads when I wrote this.
> >
> > It should be moved in the dispatch_async block as you suggest. Should
> > I write a patch, or will you write one before you delete autorelease
> > pools?
>
> I'll write a patchset. If you have time to test it when I send it out
> that would be great.
Thanks, I will test the patchset soon after I receive it.
>
> Incidentally, I think the answer to my other question
> > > Is it safe to call dpy_set_ui_info() from a non-QEMU-main-thread?
>
> is "no, and so the body of updateUIInfo should be enclosed in a
> with_iothread_lock block".
>
> -- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ui/cocoa: Set UI information
2022-02-04 16:19 ` Peter Maydell
2022-02-05 2:06 ` Akihiko Odaki
@ 2022-02-14 10:27 ` Gerd Hoffmann
2022-02-14 10:43 ` Peter Maydell
1 sibling, 1 reply; 9+ messages in thread
From: Gerd Hoffmann @ 2022-02-14 10:27 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, Akihiko Odaki
> (2) A question for Gerd:
> Is it safe to call dpy_set_ui_info() from a non-QEMU-main-thread?
No.
take care,
Gerd
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ui/cocoa: Set UI information
2022-02-14 10:27 ` Gerd Hoffmann
@ 2022-02-14 10:43 ` Peter Maydell
2022-02-14 11:51 ` Gerd Hoffmann
0 siblings, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2022-02-14 10:43 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel, Akihiko Odaki
On Mon, 14 Feb 2022 at 10:27, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> > (2) A question for Gerd:
> > Is it safe to call dpy_set_ui_info() from a non-QEMU-main-thread?
>
> No.
Is it OK to do so if the other thread is holding the iothread lock?
(This is how we do a lot of the other "need to call a QEMU function"
work from the cocoa UI thread.)
thanks
-- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ui/cocoa: Set UI information
2022-02-14 10:43 ` Peter Maydell
@ 2022-02-14 11:51 ` Gerd Hoffmann
0 siblings, 0 replies; 9+ messages in thread
From: Gerd Hoffmann @ 2022-02-14 11:51 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, Akihiko Odaki
On Mon, Feb 14, 2022 at 10:43:32AM +0000, Peter Maydell wrote:
> On Mon, 14 Feb 2022 at 10:27, Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > > (2) A question for Gerd:
> > > Is it safe to call dpy_set_ui_info() from a non-QEMU-main-thread?
> >
> > No.
>
> Is it OK to do so if the other thread is holding the iothread lock?
> (This is how we do a lot of the other "need to call a QEMU function"
> work from the cocoa UI thread.)
That should work.
take care,
Gerd
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-02-14 11:53 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-16 14:19 [PATCH] ui/cocoa: Set UI information Akihiko Odaki
2021-06-23 12:43 ` Gerd Hoffmann
2022-02-04 16:19 ` Peter Maydell
2022-02-05 2:06 ` Akihiko Odaki
2022-02-08 18:07 ` Peter Maydell
2022-02-09 11:32 ` Akihiko Odaki
2022-02-14 10:27 ` Gerd Hoffmann
2022-02-14 10:43 ` Peter Maydell
2022-02-14 11:51 ` Gerd Hoffmann
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.