On Thu, Mar 08, 2018 at 05:22:01PM +0100, Olivier Fourdan wrote:
> Hi,
>
> On Thu, Mar 8, 2018 at 4:27 PM, Daniel P. Berrangé <berrange@xxxxxxxxxx>
> wrote:
>
> > On Thu, Mar 08, 2018 at 04:19:03PM +0100, Olivier Fourdan wrote:
> > > When running on Xwayland, the keycode mapping property is not available,
> > > which causes unknown keycode mapping errors and the keyboard doesn't
> > > work.
> > >
> > > Use a known scancode (“XK_Page_Up”) to check against the AT scancode and
> > > use “xfree86” if it matches or assume “evdev” for anything else.
> > >
> > > Signed-off-by: Olivier Fourdan <ofourdan@xxxxxxxxxx>
> > > ---
> > > src/vncdisplaykeymap.c | 33 ++++-----------------------------
> > > 1 file changed, 4 insertions(+), 29 deletions(-)
> > >
> > > diff --git a/src/vncdisplaykeymap.c b/src/vncdisplaykeymap.c
> > > index 9ee501d..135412f 100644
> > > --- a/src/vncdisplaykeymap.c
> > > +++ b/src/vncdisplaykeymap.c
> > > @@ -162,8 +162,6 @@ const guint16 *vnc_display_keymap_gdk2xtkbd_table(GdkWindow
> > *window,
> > > {
> > > #ifdef GDK_WINDOWING_X11
> > > if (GDK_IS_X11_WINDOW(window)) {
> > > - XkbDescPtr desc;
> > > - const gchar *keycodes = NULL;
> > > GdkDisplay *dpy = gdk_window_get_display(window);
> > >
> > > /* There is no easy way to determine what X11 server
> > > @@ -175,17 +173,6 @@ const guint16 *vnc_display_keymap_gdk2xtkbd_table(GdkWindow
> > *window,
> > > */
> > >
> > > Display *xdisplay = gdk_x11_display_get_xdisplay(dpy);
> > > - desc = XkbGetMap(xdisplay,
> > > - XkbGBN_AllComponentsMask,
> > > - XkbUseCoreKbd);
> > > - if (desc) {
> > > - if (XkbGetNames(xdisplay, XkbKeycodesNameMask,
> > desc) == Success) {
> > > - keycodes = gdk_x11_get_xatom_name(desc->
> > names->keycodes);
> > > - if (!keycodes)
> > > - g_warning("could not lookup
> > keycode name");
> > > - }
> > > - XkbFreeKeyboard(desc, XkbGBN_AllComponentsMask,
> > True);
> > > - }
> > >
> > > if (check_for_xwin(dpy)) {
> > > VNC_DEBUG("Using xwin keycode mapping");
> > > @@ -195,26 +182,14 @@ const guint16 *vnc_display_keymap_gdk2xtkbd_table(GdkWindow
> > *window,
> > > VNC_DEBUG("Using xquartz keycode mapping");
> > > *maplen = G_N_ELEMENTS(keymap_xorgxquartz2xtkbd);
> > > return keymap_xorgxquartz2xtkbd;
> > > - } else if (keycodes && STRPREFIX(keycodes, "evdev")) {
> > > - VNC_DEBUG("Using evdev keycode mapping");
> > > - *maplen = G_N_ELEMENTS(keymap_xorgevdev2xtkbd);
> > > - return keymap_xorgevdev2xtkbd;
> > > - } else if (keycodes && STRPREFIX(keycodes, "xfree86")) {
> > > + } else if (XKeysymToKeycode(xdisplay, XK_Page_Up) == 0x63)
> > {
> > > VNC_DEBUG("Using xfree86 keycode mapping");
> > > *maplen = G_N_ELEMENTS(keymap_xorgkbd2xtkbd);
> > > return keymap_xorgkbd2xtkbd;
> > > } else {
> > > - g_warning("Unknown keycode mapping '%s'.\n"
> > > - "Please report to gtk-vnc-list@xxxxxxxxx
> > \n"
> > > - "including the following information:\n"
> > > - "\n"
> > > - " - Operating system\n"
> > > - " - GDK build\n"
> > > - " - X11 Server\n"
> > > - " - xprop -root\n"
> > > - " - xdpyinfo\n",
> > > - keycodes);
> > > - return NULL;
> > > + VNC_DEBUG("Using evdev keycode mapping");
> > > + *maplen = G_N_ELEMENTS(keymap_xorgevdev2xtkbd);
> > > + return keymap_xorgevdev2xtkbd;
> >
> > I don't think this is desirable logic - blindly assuming everything
> > else is evdev is not valid - that is only true of an X server running
> > on a Linux host. Thus this has the effect of hiding the important
> > error message alerting us to use of other X servers we need to handle
> > and silently giving those users incorrect keymaps.
> >
>
> This is what was suggested in downstream bug
> https://bugzilla.redhat.com/show_bug.cgi?id=1479682
>
> If I may, I would point out at
> https://bugzilla.redhat.com/show_bug.cgi?id=1479682#c32 where you wrote:
>
> “Oh, that's a nice idea, we can certainly try that approach. Just
> distinguishing AT from evdev is sufficient in this case.”
I'm happy to admit I was wrong when writing that, but I also wasn't
thinking we would delete the existing code that checks 'keycodes'
name, but rather augment it.
> Besides, the current code is not only broken, it also leaves the user with
> no working keyboard at all...
Surely we can put in a positive check for XKeysymToKeycode() value matching
that expected for evdev, and then still keep the existing fallback warning
message so we continue to detect broken platforms
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel