* [PATCH] ui/cocoa: Fix switched_to_fullscreen warning
@ 2022-07-02 4:43 Peter Delevoryas
2022-07-02 14:30 ` Akihiko Odaki
2022-07-12 20:15 ` Philippe Mathieu-Daudé via
0 siblings, 2 replies; 6+ messages in thread
From: Peter Delevoryas @ 2022-07-02 4:43 UTC (permalink / raw)
Cc: qemu-devel, peter.maydell, f4bug, akihiko.odaki, kraxel
I noticed this error while building QEMU on Mac OS X:
[1040/1660] Compiling Objective-C object libcommon.fa.p/ui_cocoa.m.o
../ui/cocoa.m:803:17: warning: variable 'switched_to_fullscreen' set but not used [-Wunused-but-set-variable]
static bool switched_to_fullscreen = false;
^
1 warning generated.
I think the behavior is fine if you remove "switched_to_fullscreen", I can
still switch in and out of mouse grabbed mode and fullscreen mode with this
change, and Command keycodes will only be passed to the guest if the mouse
is grabbed, which I think is the right behavior. I'm not sure why a static
piece of state was needed to handle that in the first place. Perhaps the
refactoring of the flags-state-change fixed that by toggling the Command
keycode on.
I tested this with an Ubuntu core image on macOS 12.4
wget https://cdimage.ubuntu.com/ubuntu-core/18/stable/current/ubuntu-core-18-i386.img.xz
xz -d ubuntu-core-18-i386.img.xz
qemu-system-x86_64 -drive file=ubuntu-core-18.i386.img,format=raw
Fixes: 6d73bb643aa7 ("ui/cocoa: Clear modifiers whenever possible")
Signed-off-by: Peter Delevoryas <peter@pjd.dev>
---
ui/cocoa.m | 8 --------
1 file changed, 8 deletions(-)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 84c84e98fc..13e208b037 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -800,7 +800,6 @@ - (bool) handleEventLocked:(NSEvent *)event
int buttons = 0;
int keycode = 0;
bool mouse_event = false;
- static bool switched_to_fullscreen = false;
// Location of event in virtual screen coordinates
NSPoint p = [self screenLocationOfEvent:event];
NSUInteger modifiers = [event modifierFlags];
@@ -952,13 +951,6 @@ - (bool) handleEventLocked:(NSEvent *)event
// forward command key combos to the host UI unless the mouse is grabbed
if (!isMouseGrabbed && ([event modifierFlags] & NSEventModifierFlagCommand)) {
- /*
- * Prevent the command key from being stuck down in the guest
- * when using Command-F to switch to full screen mode.
- */
- if (keycode == Q_KEY_CODE_F) {
- switched_to_fullscreen = true;
- }
return false;
}
--
2.37.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ui/cocoa: Fix switched_to_fullscreen warning
2022-07-02 4:43 [PATCH] ui/cocoa: Fix switched_to_fullscreen warning Peter Delevoryas
@ 2022-07-02 14:30 ` Akihiko Odaki
2022-07-07 0:58 ` Peter Delevoryas
2022-07-12 20:15 ` Philippe Mathieu-Daudé via
1 sibling, 1 reply; 6+ messages in thread
From: Akihiko Odaki @ 2022-07-02 14:30 UTC (permalink / raw)
To: Peter Delevoryas; +Cc: qemu-devel, peter.maydell, f4bug, kraxel
Reviewed-by: Akihiko Odaki <akihiko.odaki@gmail.com>
On 2022/07/02 13:43, Peter Delevoryas wrote:
> I noticed this error while building QEMU on Mac OS X:
>
> [1040/1660] Compiling Objective-C object libcommon.fa.p/ui_cocoa.m.o
> ../ui/cocoa.m:803:17: warning: variable 'switched_to_fullscreen' set but not used [-Wunused-but-set-variable]
> static bool switched_to_fullscreen = false;
> ^
> 1 warning generated.
>
> I think the behavior is fine if you remove "switched_to_fullscreen", I can
> still switch in and out of mouse grabbed mode and fullscreen mode with this
> change, and Command keycodes will only be passed to the guest if the mouse
> is grabbed, which I think is the right behavior. I'm not sure why a static
> piece of state was needed to handle that in the first place. Perhaps the
> refactoring of the flags-state-change fixed that by toggling the Command
> keycode on.
>
> I tested this with an Ubuntu core image on macOS 12.4
>
> wget https://cdimage.ubuntu.com/ubuntu-core/18/stable/current/ubuntu-core-18-i386.img.xz
> xz -d ubuntu-core-18-i386.img.xz
> qemu-system-x86_64 -drive file=ubuntu-core-18.i386.img,format=raw
>
> Fixes: 6d73bb643aa7 ("ui/cocoa: Clear modifiers whenever possible")
> Signed-off-by: Peter Delevoryas <peter@pjd.dev>
> ---
> ui/cocoa.m | 8 --------
> 1 file changed, 8 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 84c84e98fc..13e208b037 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -800,7 +800,6 @@ - (bool) handleEventLocked:(NSEvent *)event
> int buttons = 0;
> int keycode = 0;
> bool mouse_event = false;
> - static bool switched_to_fullscreen = false;
> // Location of event in virtual screen coordinates
> NSPoint p = [self screenLocationOfEvent:event];
> NSUInteger modifiers = [event modifierFlags];
> @@ -952,13 +951,6 @@ - (bool) handleEventLocked:(NSEvent *)event
>
> // forward command key combos to the host UI unless the mouse is grabbed
> if (!isMouseGrabbed && ([event modifierFlags] & NSEventModifierFlagCommand)) {
> - /*
> - * Prevent the command key from being stuck down in the guest
> - * when using Command-F to switch to full screen mode.
> - */
> - if (keycode == Q_KEY_CODE_F) {
> - switched_to_fullscreen = true;
> - }
> return false;
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ui/cocoa: Fix switched_to_fullscreen warning
2022-07-02 14:30 ` Akihiko Odaki
@ 2022-07-07 0:58 ` Peter Delevoryas
2022-07-12 0:05 ` Peter Delevoryas
0 siblings, 1 reply; 6+ messages in thread
From: Peter Delevoryas @ 2022-07-07 0:58 UTC (permalink / raw)
To: Akihiko Odaki; +Cc: qemu-devel, peter.maydell, f4bug, kraxel
On Sat, Jul 02, 2022 at 11:30:16PM +0900, Akihiko Odaki wrote:
> Reviewed-by: Akihiko Odaki <akihiko.odaki@gmail.com>
Just checking in on the status of this: do I need to submit a pull request?
Or will this patch be picked up in a miscellaneous pull queue eventually?
>
> On 2022/07/02 13:43, Peter Delevoryas wrote:
> > I noticed this error while building QEMU on Mac OS X:
> >
> > [1040/1660] Compiling Objective-C object libcommon.fa.p/ui_cocoa.m.o
> > ../ui/cocoa.m:803:17: warning: variable 'switched_to_fullscreen' set but not used [-Wunused-but-set-variable]
> > static bool switched_to_fullscreen = false;
> > ^
> > 1 warning generated.
> >
> > I think the behavior is fine if you remove "switched_to_fullscreen", I can
> > still switch in and out of mouse grabbed mode and fullscreen mode with this
> > change, and Command keycodes will only be passed to the guest if the mouse
> > is grabbed, which I think is the right behavior. I'm not sure why a static
> > piece of state was needed to handle that in the first place. Perhaps the
> > refactoring of the flags-state-change fixed that by toggling the Command
> > keycode on.
> >
> > I tested this with an Ubuntu core image on macOS 12.4
> >
> > wget https://cdimage.ubuntu.com/ubuntu-core/18/stable/current/ubuntu-core-18-i386.img.xz
> > xz -d ubuntu-core-18-i386.img.xz
> > qemu-system-x86_64 -drive file=ubuntu-core-18.i386.img,format=raw
> >
> > Fixes: 6d73bb643aa7 ("ui/cocoa: Clear modifiers whenever possible")
> > Signed-off-by: Peter Delevoryas <peter@pjd.dev>
> > ---
> > ui/cocoa.m | 8 --------
> > 1 file changed, 8 deletions(-)
> >
> > diff --git a/ui/cocoa.m b/ui/cocoa.m
> > index 84c84e98fc..13e208b037 100644
> > --- a/ui/cocoa.m
> > +++ b/ui/cocoa.m
> > @@ -800,7 +800,6 @@ - (bool) handleEventLocked:(NSEvent *)event
> > int buttons = 0;
> > int keycode = 0;
> > bool mouse_event = false;
> > - static bool switched_to_fullscreen = false;
> > // Location of event in virtual screen coordinates
> > NSPoint p = [self screenLocationOfEvent:event];
> > NSUInteger modifiers = [event modifierFlags];
> > @@ -952,13 +951,6 @@ - (bool) handleEventLocked:(NSEvent *)event
> > // forward command key combos to the host UI unless the mouse is grabbed
> > if (!isMouseGrabbed && ([event modifierFlags] & NSEventModifierFlagCommand)) {
> > - /*
> > - * Prevent the command key from being stuck down in the guest
> > - * when using Command-F to switch to full screen mode.
> > - */
> > - if (keycode == Q_KEY_CODE_F) {
> > - switched_to_fullscreen = true;
> > - }
> > return false;
> > }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ui/cocoa: Fix switched_to_fullscreen warning
2022-07-07 0:58 ` Peter Delevoryas
@ 2022-07-12 0:05 ` Peter Delevoryas
2022-07-12 3:56 ` Akihiko Odaki
0 siblings, 1 reply; 6+ messages in thread
From: Peter Delevoryas @ 2022-07-12 0:05 UTC (permalink / raw)
To: Akihiko Odaki; +Cc: qemu-devel, peter.maydell, f4bug, kraxel
On Wed, Jul 06, 2022 at 05:58:38PM -0700, Peter Delevoryas wrote:
> On Sat, Jul 02, 2022 at 11:30:16PM +0900, Akihiko Odaki wrote:
> > Reviewed-by: Akihiko Odaki <akihiko.odaki@gmail.com>
>
> Just checking in on the status of this: do I need to submit a pull request?
> Or will this patch be picked up in a miscellaneous pull queue eventually?
Pinging this thread again, does this change need anyone else to review it?
>
> >
> > On 2022/07/02 13:43, Peter Delevoryas wrote:
> > > I noticed this error while building QEMU on Mac OS X:
> > >
> > > [1040/1660] Compiling Objective-C object libcommon.fa.p/ui_cocoa.m.o
> > > ../ui/cocoa.m:803:17: warning: variable 'switched_to_fullscreen' set but not used [-Wunused-but-set-variable]
> > > static bool switched_to_fullscreen = false;
> > > ^
> > > 1 warning generated.
> > >
> > > I think the behavior is fine if you remove "switched_to_fullscreen", I can
> > > still switch in and out of mouse grabbed mode and fullscreen mode with this
> > > change, and Command keycodes will only be passed to the guest if the mouse
> > > is grabbed, which I think is the right behavior. I'm not sure why a static
> > > piece of state was needed to handle that in the first place. Perhaps the
> > > refactoring of the flags-state-change fixed that by toggling the Command
> > > keycode on.
> > >
> > > I tested this with an Ubuntu core image on macOS 12.4
> > >
> > > wget https://cdimage.ubuntu.com/ubuntu-core/18/stable/current/ubuntu-core-18-i386.img.xz
> > > xz -d ubuntu-core-18-i386.img.xz
> > > qemu-system-x86_64 -drive file=ubuntu-core-18.i386.img,format=raw
> > >
> > > Fixes: 6d73bb643aa7 ("ui/cocoa: Clear modifiers whenever possible")
> > > Signed-off-by: Peter Delevoryas <peter@pjd.dev>
> > > ---
> > > ui/cocoa.m | 8 --------
> > > 1 file changed, 8 deletions(-)
> > >
> > > diff --git a/ui/cocoa.m b/ui/cocoa.m
> > > index 84c84e98fc..13e208b037 100644
> > > --- a/ui/cocoa.m
> > > +++ b/ui/cocoa.m
> > > @@ -800,7 +800,6 @@ - (bool) handleEventLocked:(NSEvent *)event
> > > int buttons = 0;
> > > int keycode = 0;
> > > bool mouse_event = false;
> > > - static bool switched_to_fullscreen = false;
> > > // Location of event in virtual screen coordinates
> > > NSPoint p = [self screenLocationOfEvent:event];
> > > NSUInteger modifiers = [event modifierFlags];
> > > @@ -952,13 +951,6 @@ - (bool) handleEventLocked:(NSEvent *)event
> > > // forward command key combos to the host UI unless the mouse is grabbed
> > > if (!isMouseGrabbed && ([event modifierFlags] & NSEventModifierFlagCommand)) {
> > > - /*
> > > - * Prevent the command key from being stuck down in the guest
> > > - * when using Command-F to switch to full screen mode.
> > > - */
> > > - if (keycode == Q_KEY_CODE_F) {
> > > - switched_to_fullscreen = true;
> > > - }
> > > return false;
> > > }
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ui/cocoa: Fix switched_to_fullscreen warning
2022-07-12 0:05 ` Peter Delevoryas
@ 2022-07-12 3:56 ` Akihiko Odaki
0 siblings, 0 replies; 6+ messages in thread
From: Akihiko Odaki @ 2022-07-12 3:56 UTC (permalink / raw)
To: Peter Delevoryas; +Cc: qemu-devel, peter.maydell, f4bug, kraxel
On 2022/07/12 9:05, Peter Delevoryas wrote:
> On Wed, Jul 06, 2022 at 05:58:38PM -0700, Peter Delevoryas wrote:
>> On Sat, Jul 02, 2022 at 11:30:16PM +0900, Akihiko Odaki wrote:
>>> Reviewed-by: Akihiko Odaki <akihiko.odaki@gmail.com>
>>
>> Just checking in on the status of this: do I need to submit a pull request?
>> Or will this patch be picked up in a miscellaneous pull queue eventually?
>
> Pinging this thread again, does this change need anyone else to review it?
The patch should be picked up later. You may ping again if there is no
response after weeks.
Sorry for replying late,
Akihiko Odaki
>
>>
>>>
>>> On 2022/07/02 13:43, Peter Delevoryas wrote:
>>>> I noticed this error while building QEMU on Mac OS X:
>>>>
>>>> [1040/1660] Compiling Objective-C object libcommon.fa.p/ui_cocoa.m.o
>>>> ../ui/cocoa.m:803:17: warning: variable 'switched_to_fullscreen' set but not used [-Wunused-but-set-variable]
>>>> static bool switched_to_fullscreen = false;
>>>> ^
>>>> 1 warning generated.
>>>>
>>>> I think the behavior is fine if you remove "switched_to_fullscreen", I can
>>>> still switch in and out of mouse grabbed mode and fullscreen mode with this
>>>> change, and Command keycodes will only be passed to the guest if the mouse
>>>> is grabbed, which I think is the right behavior. I'm not sure why a static
>>>> piece of state was needed to handle that in the first place. Perhaps the
>>>> refactoring of the flags-state-change fixed that by toggling the Command
>>>> keycode on.
>>>>
>>>> I tested this with an Ubuntu core image on macOS 12.4
>>>>
>>>> wget https://cdimage.ubuntu.com/ubuntu-core/18/stable/current/ubuntu-core-18-i386.img.xz
>>>> xz -d ubuntu-core-18-i386.img.xz
>>>> qemu-system-x86_64 -drive file=ubuntu-core-18.i386.img,format=raw
>>>>
>>>> Fixes: 6d73bb643aa7 ("ui/cocoa: Clear modifiers whenever possible")
>>>> Signed-off-by: Peter Delevoryas <peter@pjd.dev>
>>>> ---
>>>> ui/cocoa.m | 8 --------
>>>> 1 file changed, 8 deletions(-)
>>>>
>>>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>>>> index 84c84e98fc..13e208b037 100644
>>>> --- a/ui/cocoa.m
>>>> +++ b/ui/cocoa.m
>>>> @@ -800,7 +800,6 @@ - (bool) handleEventLocked:(NSEvent *)event
>>>> int buttons = 0;
>>>> int keycode = 0;
>>>> bool mouse_event = false;
>>>> - static bool switched_to_fullscreen = false;
>>>> // Location of event in virtual screen coordinates
>>>> NSPoint p = [self screenLocationOfEvent:event];
>>>> NSUInteger modifiers = [event modifierFlags];
>>>> @@ -952,13 +951,6 @@ - (bool) handleEventLocked:(NSEvent *)event
>>>> // forward command key combos to the host UI unless the mouse is grabbed
>>>> if (!isMouseGrabbed && ([event modifierFlags] & NSEventModifierFlagCommand)) {
>>>> - /*
>>>> - * Prevent the command key from being stuck down in the guest
>>>> - * when using Command-F to switch to full screen mode.
>>>> - */
>>>> - if (keycode == Q_KEY_CODE_F) {
>>>> - switched_to_fullscreen = true;
>>>> - }
>>>> return false;
>>>> }
>>>
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ui/cocoa: Fix switched_to_fullscreen warning
2022-07-02 4:43 [PATCH] ui/cocoa: Fix switched_to_fullscreen warning Peter Delevoryas
2022-07-02 14:30 ` Akihiko Odaki
@ 2022-07-12 20:15 ` Philippe Mathieu-Daudé via
1 sibling, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-07-12 20:15 UTC (permalink / raw)
To: Peter Delevoryas; +Cc: qemu-devel, peter.maydell, akihiko.odaki, kraxel
On 2/7/22 06:43, Peter Delevoryas wrote:
> I noticed this error while building QEMU on Mac OS X:
>
> [1040/1660] Compiling Objective-C object libcommon.fa.p/ui_cocoa.m.o
> ../ui/cocoa.m:803:17: warning: variable 'switched_to_fullscreen' set but not used [-Wunused-but-set-variable]
> static bool switched_to_fullscreen = false;
> ^
> 1 warning generated.
>
> I think the behavior is fine if you remove "switched_to_fullscreen", I can
> still switch in and out of mouse grabbed mode and fullscreen mode with this
> change, and Command keycodes will only be passed to the guest if the mouse
> is grabbed, which I think is the right behavior. I'm not sure why a static
> piece of state was needed to handle that in the first place. Perhaps the
> refactoring of the flags-state-change fixed that by toggling the Command
> keycode on.
>
> I tested this with an Ubuntu core image on macOS 12.4
>
> wget https://cdimage.ubuntu.com/ubuntu-core/18/stable/current/ubuntu-core-18-i386.img.xz
> xz -d ubuntu-core-18-i386.img.xz
> qemu-system-x86_64 -drive file=ubuntu-core-18.i386.img,format=raw
>
> Fixes: 6d73bb643aa7 ("ui/cocoa: Clear modifiers whenever possible")
> Signed-off-by: Peter Delevoryas <peter@pjd.dev>
> ---
> ui/cocoa.m | 8 --------
> 1 file changed, 8 deletions(-)
Queued, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-07-12 20:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-02 4:43 [PATCH] ui/cocoa: Fix switched_to_fullscreen warning Peter Delevoryas
2022-07-02 14:30 ` Akihiko Odaki
2022-07-07 0:58 ` Peter Delevoryas
2022-07-12 0:05 ` Peter Delevoryas
2022-07-12 3:56 ` Akihiko Odaki
2022-07-12 20:15 ` Philippe Mathieu-Daudé via
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.