[PULL 0/7] ui patch queue

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 0/7] ui patch queue

Gerd Hoffmann-3
  Hi,

Here is the ui patch queue.  Switches the default
to gtk3 and sdl2 and fixes some bugs.

please pull,
  Gerd

The following changes since commit a0d4aac7467dd02e5657b79e867f067330266a24:

  Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20170605' into staging (2017-06-05 18:03:43 +0100)

are available in the git repository at:

  git://git.kraxel.org/qemu tags/pull-ui-20170608-1

for you to fetch changes up to 468949958fdfa4347a66e3cf5c8240a428532602:

  spice: don't enter opengl mode in case another UI provides opengl support (2017-06-08 08:13:46 +0200)

----------------------------------------------------------------
ui: prefer gtk3 and sdl2, various fixes.

----------------------------------------------------------------
Gerd Hoffmann (5):
      gtk: prefer gtk3 over gtk2
      sdl: prefer sdl2 over sdl1
      gtk: add deprecation warning for gtk2
      sdl: add deprecation warning for sdl1
      spice: don't enter opengl mode in case another UI provides opengl support

Ian McKellar via Qemu-devel (1):
      Improve Cocoa modifier key handling

Jonathon Jongsma (1):
      spice: Use proper enum type for kbd led state

 configure                  | 36 ++++++++++++++++++----------
 include/ui/spice-display.h |  2 ++
 ui/spice-core.c            |  1 +
 ui/spice-display.c         |  3 ++-
 ui/spice-input.c           |  2 +-
 ui/cocoa.m                 | 60 ++++++++++++++++++++++++++++++++++++----------
 6 files changed, 78 insertions(+), 26 deletions(-)

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 1/7] Improve Cocoa modifier key handling

Gerd Hoffmann-3
From: Ian McKellar via Qemu-devel <[hidden email]>

I had two problems with QEMU on macOS:
 1) Sometimes when alt-tabbing to QEMU it would act as if the 'a' key
    was pressed so I'd get 'aaaaaaaaa....'.
 2) Using Sikuli to programatically send keys to the QEMU window text
    like "foo_bar" would come out as "fooa-bar".

They looked similar and after much digging the problem turned out to be
the same. When QEMU's ui/cocoa.m received an NSFlagsChanged NSEvent it
looked at the keyCode to determine what modifier key changed. This
usually works fine but sometimes the keyCode is 0 and the app should
instead be looking at the modifierFlags bitmask. Key code 0 is the 'a'
key.

I added code that handles keyCode == 0 differently. It checks the
modifierFlags and if they differ from QEMU's idea of which modifier
keys are currently pressed it toggles those changed keys.

This fixes my problems and seems work fine.

Signed-off-by: Ian McKellar <[hidden email]>
Message-id: [hidden email]
Signed-off-by: Gerd Hoffmann <[hidden email]>
---
 ui/cocoa.m | 60 ++++++++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 48 insertions(+), 12 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 004ec2711c..1f010d3ae7 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -52,6 +52,8 @@
 /* macOS 10.12 deprecated many constants, #define the new names for older SDKs */
 #if MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_12
 #define NSEventMaskAny                  NSAnyEventMask
+#define NSEventModifierFlagCapsLock     NSAlphaShiftKeyMask
+#define NSEventModifierFlagShift        NSShiftKeyMask
 #define NSEventModifierFlagCommand      NSCommandKeyMask
 #define NSEventModifierFlagControl      NSControlKeyMask
 #define NSEventModifierFlagOption       NSAlternateKeyMask
@@ -268,7 +270,7 @@ static void handleAnyDeviceErrors(Error * err)
     NSWindow *fullScreenWindow;
     float cx,cy,cw,ch,cdx,cdy;
     CGDataProviderRef dataProviderRef;
-    int modifiers_state[256];
+    BOOL modifiers_state[256];
     BOOL isMouseGrabbed;
     BOOL isFullscreen;
     BOOL isAbsoluteEnabled;
@@ -536,18 +538,59 @@ QemuCocoaView *cocoaView;
     }
 }
 
+- (void) toggleModifier: (int)keycode {
+    // Toggle the stored state.
+    modifiers_state[keycode] = !modifiers_state[keycode];
+    // Send a keyup or keydown depending on the state.
+    qemu_input_event_send_key_qcode(dcl->con, keycode, modifiers_state[keycode]);
+}
+
+- (void) toggleStatefulModifier: (int)keycode {
+    // Toggle the stored state.
+    modifiers_state[keycode] = !modifiers_state[keycode];
+    // Generate keydown and keyup.
+    qemu_input_event_send_key_qcode(dcl->con, keycode, true);
+    qemu_input_event_send_key_qcode(dcl->con, keycode, false);
+}
+
 - (void) handleEvent:(NSEvent *)event
 {
     COCOA_DEBUG("QemuCocoaView: handleEvent\n");
 
     int buttons = 0;
-    int keycode;
+    int keycode = 0;
     bool mouse_event = false;
     NSPoint p = [event locationInWindow];
 
     switch ([event type]) {
         case NSEventTypeFlagsChanged:
-            keycode = cocoa_keycode_to_qemu([event keyCode]);
+            if ([event keyCode] == 0) {
+                // When the Cocoa keyCode is zero that means keys should be
+                // synthesized based on the values in in the eventModifiers
+                // bitmask.
+
+                if (qemu_console_is_graphic(NULL)) {
+                    NSEventModifierFlags modifiers = [event modifierFlags];
+
+                    if (!!(modifiers & NSEventModifierFlagCapsLock) != !!modifiers_state[Q_KEY_CODE_CAPS_LOCK]) {
+                        [self toggleStatefulModifier:Q_KEY_CODE_CAPS_LOCK];
+                    }
+                    if (!!(modifiers & NSEventModifierFlagShift) != !!modifiers_state[Q_KEY_CODE_SHIFT]) {
+                        [self toggleModifier:Q_KEY_CODE_SHIFT];
+                    }
+                    if (!!(modifiers & NSEventModifierFlagControl) != !!modifiers_state[Q_KEY_CODE_CTRL]) {
+                        [self toggleModifier:Q_KEY_CODE_CTRL];
+                    }
+                    if (!!(modifiers & NSEventModifierFlagOption) != !!modifiers_state[Q_KEY_CODE_ALT]) {
+                        [self toggleModifier:Q_KEY_CODE_ALT];
+                    }
+                    if (!!(modifiers & NSEventModifierFlagCommand) != !!modifiers_state[Q_KEY_CODE_META_L]) {
+                        [self toggleModifier:Q_KEY_CODE_META_L];
+                    }
+                }
+            } else {
+                keycode = cocoa_keycode_to_qemu([event keyCode]);
+            }
 
             if ((keycode == Q_KEY_CODE_META_L || keycode == Q_KEY_CODE_META_R)
                && !isMouseGrabbed) {
@@ -559,16 +602,9 @@ QemuCocoaView *cocoaView;
                 // emulate caps lock and num lock keydown and keyup
                 if (keycode == Q_KEY_CODE_CAPS_LOCK ||
                     keycode == Q_KEY_CODE_NUM_LOCK) {
-                    qemu_input_event_send_key_qcode(dcl->con, keycode, true);
-                    qemu_input_event_send_key_qcode(dcl->con, keycode, false);
+                    [self toggleStatefulModifier:keycode];
                 } else if (qemu_console_is_graphic(NULL)) {
-                    if (modifiers_state[keycode] == 0) { // keydown
-                        qemu_input_event_send_key_qcode(dcl->con, keycode, true);
-                        modifiers_state[keycode] = 1;
-                    } else { // keyup
-                        qemu_input_event_send_key_qcode(dcl->con, keycode, false);
-                        modifiers_state[keycode] = 0;
-                    }
+                  [self toggleModifier:keycode];
                 }
             }
 
--
2.9.3


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 2/7] spice: Use proper enum type for kbd led state

Gerd Hoffmann-3
In reply to this post by Gerd Hoffmann-3
From: Jonathon Jongsma <[hidden email]>

Although the Qemu and spice flags currently have the same value, it
seems more correct to pass the spice flag values to
spice_server_kbd_leds(), especially considering that this function
already makes an effort to convert between the QEMU_*_LED and
SPICE_KEYBOARD_MODIFIER_* values.

Signed-off-by: Jonathon Jongsma <[hidden email]>
Reviewed-by: Marc-André Lureau <[hidden email]>
Message-id: [hidden email]
Signed-off-by: Gerd Hoffmann <[hidden email]>
---
 ui/spice-input.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/spice-input.c b/ui/spice-input.c
index 86293dd2ce..918580239d 100644
--- a/ui/spice-input.c
+++ b/ui/spice-input.c
@@ -87,7 +87,7 @@ static void kbd_leds(void *opaque, int ledstate)
     if (ledstate & QEMU_CAPS_LOCK_LED) {
         kbd->ledstate |= SPICE_KEYBOARD_MODIFIER_FLAGS_CAPS_LOCK;
     }
-    spice_server_kbd_leds(&kbd->sin, ledstate);
+    spice_server_kbd_leds(&kbd->sin, kbd->ledstate);
 }
 
 /* mouse bits */
--
2.9.3


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 3/7] gtk: prefer gtk3 over gtk2

Gerd Hoffmann-3
In reply to this post by Gerd Hoffmann-3
In case the configure script finds both gtk2 and gtk3 installed it
still prefers gtk2 over gtk3.  Prefer gtk3 instead.

Signed-off-by: Gerd Hoffmann <[hidden email]>
Reviewed-by: Marc-André Lureau <[hidden email]>
Message-id: [hidden email]
---
 configure | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/configure b/configure
index 13e040d28c..b75b9c57c0 100755
--- a/configure
+++ b/configure
@@ -2325,14 +2325,14 @@ fi
 # GTK probe
 
 if test "$gtkabi" = ""; then
-    # The GTK ABI was not specified explicitly, so try whether 2.0 is available.
-    # Use 3.0 as a fallback if that is available.
-    if $pkg_config --exists "gtk+-2.0 >= 2.18.0"; then
-        gtkabi=2.0
-    elif $pkg_config --exists "gtk+-3.0 >= 3.0.0"; then
+    # The GTK ABI was not specified explicitly, so try whether 3.0 is available.
+    # Use 2.0 as a fallback if that is available.
+    if $pkg_config --exists "gtk+-3.0 >= 3.0.0"; then
         gtkabi=3.0
+    elif $pkg_config --exists "gtk+-2.0 >= 2.18.0"; then
+        gtkabi=2.0
     else
-        gtkabi=2.0
+        gtkabi=3.0
     fi
 fi
 
@@ -2355,7 +2355,7 @@ if test "$gtk" != "no"; then
         libs_softmmu="$gtk_libs $libs_softmmu"
         gtk="yes"
     elif test "$gtk" = "yes"; then
-        feature_not_found "gtk" "Install gtk2 or gtk3 devel"
+        feature_not_found "gtk" "Install gtk3-devel"
     else
         gtk="no"
     fi
--
2.9.3


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 4/7] sdl: prefer sdl2 over sdl1

Gerd Hoffmann-3
In reply to this post by Gerd Hoffmann-3
In case the configure script finds both SDL 1.2 and SDL 2.x installed
it still prefers SDL 1.2.  Prefer SDL 2.x instead.

Signed-off-by: Gerd Hoffmann <[hidden email]>
Reviewed-by: Marc-André Lureau <[hidden email]>
Message-id: [hidden email]
---
 configure | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/configure b/configure
index b75b9c57c0..44c773a5cd 100755
--- a/configure
+++ b/configure
@@ -2622,12 +2622,12 @@ fi
 # sdl-config even without cross prefix, and favour pkg-config over sdl-config.
 
 if test "$sdlabi" = ""; then
-    if $pkg_config --exists "sdl"; then
-        sdlabi=1.2
-    elif $pkg_config --exists "sdl2"; then
+    if $pkg_config --exists "sdl2"; then
         sdlabi=2.0
+    elif $pkg_config --exists "sdl"; then
+        sdlabi=1.2
     else
-        sdlabi=1.2
+        sdlabi=2.0
     fi
 fi
 
@@ -2654,7 +2654,7 @@ elif has ${sdl_config}; then
   sdlversion=$($sdlconfig --version)
 else
   if test "$sdl" = "yes" ; then
-    feature_not_found "sdl" "Install SDL devel"
+    feature_not_found "sdl" "Install SDL2-devel"
   fi
   sdl=no
 fi
--
2.9.3


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 5/7] gtk: add deprecation warning for gtk2

Gerd Hoffmann-3
In reply to this post by Gerd Hoffmann-3
Signed-off-by: Gerd Hoffmann <[hidden email]>
Reviewed-by: Marc-André Lureau <[hidden email]>
Message-id: [hidden email]
---
 configure | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/configure b/configure
index 44c773a5cd..41c213b378 100755
--- a/configure
+++ b/configure
@@ -5245,6 +5245,12 @@ if test "$supported_os" = "no"; then
     echo "us upstream at [hidden email]."
 fi
 
+if test "$gtkabi" = "2.0"; then
+    echo
+    echo "WARNING: Support for gtk2 will be dropped in future releases."
+    echo "WARNING: Please consider using gtk3 instead."
+fi
+
 config_host_mak="config-host.mak"
 
 echo "# Automatically generated by configure - do not modify" >config-all-disas.mak
--
2.9.3


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 6/7] sdl: add deprecation warning for sdl1

Gerd Hoffmann-3
In reply to this post by Gerd Hoffmann-3
Signed-off-by: Gerd Hoffmann <[hidden email]>
Reviewed-by: Marc-André Lureau <[hidden email]>
Message-id: [hidden email]
---
 configure | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/configure b/configure
index 41c213b378..5af180c57d 100755
--- a/configure
+++ b/configure
@@ -5251,6 +5251,12 @@ if test "$gtkabi" = "2.0"; then
     echo "WARNING: Please consider using gtk3 instead."
 fi
 
+if test "$sdlabi" = "1.2"; then
+    echo
+    echo "WARNING: Support for SDL 1.2.x will be dropped in future releases."
+    echo "WARNING: Please consider using SDL 2.x instead."
+fi
+
 config_host_mak="config-host.mak"
 
 echo "# Automatically generated by configure - do not modify" >config-all-disas.mak
--
2.9.3


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 7/7] spice: don't enter opengl mode in case another UI provides opengl support

Gerd Hoffmann-3
In reply to this post by Gerd Hoffmann-3
Signed-off-by: Gerd Hoffmann <[hidden email]>
Message-id: [hidden email]
---
 include/ui/spice-display.h | 2 ++
 ui/spice-core.c            | 1 +
 ui/spice-display.c         | 3 ++-
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/ui/spice-display.h b/include/ui/spice-display.h
index 184d4c373a..4ba9444dba 100644
--- a/include/ui/spice-display.h
+++ b/include/ui/spice-display.h
@@ -140,6 +140,8 @@ struct SimpleSpiceCursor {
     QXLCursor cursor;
 };
 
+extern bool spice_opengl;
+
 int qemu_spice_rect_is_empty(const QXLRect* r);
 void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r);
 
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 804abc5c0f..7c9ec0f0dd 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -847,6 +847,7 @@ void qemu_spice_init(void)
             exit(1);
         }
         display_opengl = 1;
+        spice_opengl = 1;
     }
 #endif
 }
diff --git a/ui/spice-display.c b/ui/spice-display.c
index b353445f58..042292cc90 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -27,6 +27,7 @@
 #include "ui/spice-display.h"
 
 static int debug = 0;
+bool spice_opengl;
 
 static void GCC_FMT_ATTR(2, 3) dprint(int level, const char *fmt, ...)
 {
@@ -1013,7 +1014,7 @@ static void qemu_spice_display_init_one(QemuConsole *con)
 
     ssd->dcl.ops = &display_listener_ops;
 #ifdef HAVE_SPICE_GL
-    if (display_opengl) {
+    if (spice_opengl) {
         ssd->dcl.ops = &display_listener_gl_ops;
         ssd->gl_unblock_bh = qemu_bh_new(qemu_spice_gl_unblock_bh, ssd);
         ssd->gl_unblock_timer = timer_new_ms(QEMU_CLOCK_REALTIME,
--
2.9.3


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PULL 0/7] ui patch queue

Peter Maydell-5
In reply to this post by Gerd Hoffmann-3
On 8 June 2017 at 08:07, Gerd Hoffmann <[hidden email]> wrote:

>   Hi,
>
> Here is the ui patch queue.  Switches the default
> to gtk3 and sdl2 and fixes some bugs.
>
> please pull,
>   Gerd
>
> The following changes since commit a0d4aac7467dd02e5657b79e867f067330266a24:
>
>   Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20170605' into staging (2017-06-05 18:03:43 +0100)
>
> are available in the git repository at:
>
>   git://git.kraxel.org/qemu tags/pull-ui-20170608-1
>
> for you to fetch changes up to 468949958fdfa4347a66e3cf5c8240a428532602:
>
>   spice: don't enter opengl mode in case another UI provides opengl support (2017-06-08 08:13:46 +0200)
>
> ----------------------------------------------------------------
> ui: prefer gtk3 and sdl2, various fixes.
>
> ----------------------------------------------------------------

This causes configure to barf warnings in my build logs on half
my build machines:

WARNING: Support for gtk2 will be dropped in future releases.
WARNING: Please consider using gtk3 instead.

I don't think it is yet possible to drop gtk2.

thanks
-- PMM

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PULL 0/7] ui patch queue

Peter Maydell-5
On 12 June 2017 at 19:00, Peter Maydell <[hidden email]> wrote:
> This causes configure to barf warnings in my build logs on half
> my build machines:
>
> WARNING: Support for gtk2 will be dropped in future releases.
> WARNING: Please consider using gtk3 instead.
>
> I don't think it is yet possible to drop gtk2.

Oh, one of them was the warning about SDL1.2.

thanks
-- PMM

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PULL 0/7] ui patch queue

Thomas Huth-3
On 12.06.2017 20:08, Peter Maydell wrote:

> On 12 June 2017 at 19:00, Peter Maydell <[hidden email]> wrote:
>> This causes configure to barf warnings in my build logs on half
>> my build machines:
>>
>> WARNING: Support for gtk2 will be dropped in future releases.
>> WARNING: Please consider using gtk3 instead.
>>
>> I don't think it is yet possible to drop gtk2.
>
> Oh, one of them was the warning about SDL1.2.

Adding the warning does not necessarily mean that we've really got to
drop support for these in the very near future, but it's a good start to
remind people that support for the old versions won't be around forever
;-) And AFAIK the old versions of these libraries are not maintained by
the upstream projects anymore, too, so at one point in time, we've
really got to deprecate support for these in QEMU, too.

So would it at least be feasible to install SDL 2 on that build
machines? (IMHO installing SDL 2 from sources is not too much of a pain,
e.g. it does not require lots of other additional libraries around)

 Thomas

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PULL 0/7] ui patch queue

Peter Maydell-5
On 13 June 2017 at 06:14, Thomas Huth <[hidden email]> wrote:

> On 12.06.2017 20:08, Peter Maydell wrote:
>> On 12 June 2017 at 19:00, Peter Maydell <[hidden email]> wrote:
>>> This causes configure to barf warnings in my build logs on half
>>> my build machines:
>>>
>>> WARNING: Support for gtk2 will be dropped in future releases.
>>> WARNING: Please consider using gtk3 instead.
>>>
>>> I don't think it is yet possible to drop gtk2.
>>
>> Oh, one of them was the warning about SDL1.2.
>
> Adding the warning does not necessarily mean that we've really got to
> drop support for these in the very near future, but it's a good start to
> remind people that support for the old versions won't be around forever
> ;-) And AFAIK the old versions of these libraries are not maintained by
> the upstream projects anymore, too, so at one point in time, we've
> really got to deprecate support for these in QEMU, too.
>
> So would it at least be feasible to install SDL 2 on that build
> machines? (IMHO installing SDL 2 from sources is not too much of a pain,
> e.g. it does not require lots of other additional libraries around)

Yes, I'll circle back and have a look at whether it's just a matter
of installing an extra -dev package. If it's necessary to build from
source then it's definitely too early to deprecate though.

(Mostly if I have a long pullreq queue than printing warning messages
in my build logs is a good way to get your pull request pushed to
the end of the todo list :-))

thanks
-- PMM

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PULL 0/7] ui patch queue

Gerd Hoffmann-3
In reply to this post by Peter Maydell-5
  Hi,

> This causes configure to barf warnings in my build logs on half
> my build machines:
>
> WARNING: Support for gtk2 will be dropped in future releases.
> WARNING: Please consider using gtk3 instead.
>
> I don't think it is yet possible to drop gtk2.

I certainly don't plan to drop it now, we need at least one or two
releases with that patch in to see what the feedback is.  But I want
start pushing people to the newer versions, so they get used more and
people will report possible regressions from the older versions.  Not
that I expect many, fedora builds distro packages with SDL2 + gtk3 for
quite a while already.

Which systems are failing?  Any chance this is just gtk3-devel missing?
gtk 3.0 was released more than five years ago, pretty much every distro
should have packages meanwhile ...

cheers,
  Gerd


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PULL 0/7] ui patch queue

Peter Maydell-5
On 13 June 2017 at 11:41, Gerd Hoffmann <[hidden email]> wrote:
> Which systems are failing?  Any chance this is just gtk3-devel missing?
> gtk 3.0 was released more than five years ago, pretty much every distro
> should have packages meanwhile ...

I've gone through and checked, and yes, it generally was just
missing dependencies. I've fixed this on most of these systems,
but for the machine in the gcc compile farm I don't have root
access, so that will take a little time for the admins to
update.

thanks
-- PMM

Loading...