From 57f637ec34c29ada03ca22a64ed3d6e9504b0678 Mon Sep 17 00:00:00 2001 From: Adrian Perez de Castro Date: Mon, 27 Nov 2023 16:22:01 +0200 Subject: [PATCH 1/3] x11: Support using xcb-keysyms as fallback for XKB Massage the parts from #303 which deal with input using xcb-keysyms to be included in the x11 platform plug-in. This provides a fallback that can be used when XKB initialization fails, typically in cases where the X server does not support XKB or has the support disabled. XKB is a X11 protocol extension after all, and it is wrong to assume that it will be always available. While at it, allow running without keyboard support, producing a warning in this case. --- platform/x11/cog-platform-x11.c | 176 ++++++++++++++++++++++++-------- platform/x11/meson.build | 31 ++++-- 2 files changed, 152 insertions(+), 55 deletions(-) diff --git a/platform/x11/cog-platform-x11.c b/platform/x11/cog-platform-x11.c index 6a5831bc..b0b31a0d 100644 --- a/platform/x11/cog-platform-x11.c +++ b/platform/x11/cog-platform-x11.c @@ -18,6 +18,11 @@ #include #include #include + +#ifdef COG_X11_USE_XCB_KEYSYMS +# include +#endif /* COG_X11_USE_XCB_KEYSYMS */ + #include #if COG_HAVE_LIBPORTAL @@ -94,6 +99,10 @@ struct CogX11Display { xkb_mod_mask_t caps_lock; } xkb; +#ifdef COG_X11_USE_XCB_KEYSYMS + xcb_key_symbols_t *xcb_keysyms; +#endif /* COG_X11_USE_XCB_KEYSYMS */ + struct { PFNEGLGETPLATFORMDISPLAYEXTPROC get_platform_display; @@ -212,35 +221,84 @@ xcb_update_xkb_modifiers(uint32_t event_state) return wpe_modifiers; } +#if COG_X11_USE_XCB_KEYSYMS +/* + * Convert XCB modifiers to WPE modifiers + * + * - wpe constants from , + * - XCB constants from + * + * Far from ideal implementation, but it avoids the need to link + * with xcb-keysyms (which is not often packaged by linux distributions) + * or requiring the X11/XKB extension (which is not always available, e.g. + * in VNC). +*/ +static uint32_t +xcb_state_to_wpe_modifiers(uint16_t xcb_state) +{ + uint32_t out = 0; + + /* SHIFT, CONTROL, ALT/META keys */ + if (xcb_state & XCB_MOD_MASK_SHIFT) + out |= wpe_input_keyboard_modifier_shift; + + if (xcb_state & XCB_MOD_MASK_CONTROL) + out |= wpe_input_keyboard_modifier_control; + + if (xcb_state & XCB_MOD_MASK_1) + out |= wpe_input_keyboard_modifier_alt; + + /* Mouse buttons */ + if (xcb_state & XCB_BUTTON_MASK_1) + out |= wpe_input_pointer_modifier_button1; + + if (xcb_state & XCB_BUTTON_MASK_2) + out |= wpe_input_pointer_modifier_button2; + + if (xcb_state & XCB_BUTTON_MASK_3) + out |= wpe_input_pointer_modifier_button3; + + if (xcb_state & XCB_BUTTON_MASK_4) + out |= wpe_input_pointer_modifier_button4; + + return out; +} +#endif /* COG_X11_USE_XCB_KEYSYMS */ + static void -xcb_handle_key_press(CogView *view, xcb_key_press_event_t *event) +key_event_fill(struct wpe_input_keyboard_event *wpe_event, xcb_key_press_event_t *event) { - uint32_t modifiers = xcb_update_xkb_modifiers(event->state); - uint32_t keysym = xkb_state_key_get_one_sym(s_display->xkb.state, event->detail); + wpe_event->time = event->time; + wpe_event->hardware_key_code = event->detail; - struct wpe_input_keyboard_event input_event = { - .time = event->time, - .key_code = keysym, - .hardware_key_code = event->detail, - .pressed = true, - .modifiers = modifiers, - }; + if (s_display->xkb.device_id >= 0 && s_display->xkb.state) { + wpe_event->modifiers = xcb_update_xkb_modifiers(event->state); + wpe_event->key_code = xkb_state_key_get_one_sym(s_display->xkb.state, event->detail); + return; + } + +#if COG_X11_USE_XCB_KEYSYMS + if (s_display->xcb_keysyms) { + wpe_event->modifiers = xcb_state_to_wpe_modifiers(event->state); + wpe_event->key_code = xcb_key_symbols_get_keysym(s_display->xcb_keysyms, event->detail, 0); + return; + } +#endif /* COG_X11_USE_XCB_KEYSYMS */ +} + +static void +xcb_handle_key_press(CogView *view, xcb_key_press_event_t *event) +{ + struct wpe_input_keyboard_event input_event = {.pressed = true}; + key_event_fill(&input_event, event); cog_view_handle_key_event(view, &input_event); } static void xcb_handle_key_release(CogView *view, xcb_key_press_event_t *event) { - uint32_t modifiers = xcb_update_xkb_modifiers(event->state); - uint32_t keysym = xkb_state_key_get_one_sym(s_display->xkb.state, event->detail); - - struct wpe_input_keyboard_event input_event = { - .time = event->time, - .key_code = keysym, - .hardware_key_code = event->detail, - .pressed = false, - .modifiers = modifiers, - }; + struct wpe_input_keyboard_event input_event = {.pressed = false}; + key_event_fill(&input_event, event); cog_view_handle_key_event(view, &input_event); } @@ -607,17 +665,12 @@ clear_xcb (void) static gboolean init_xkb (void) { - s_display->xkb.device_id = xkb_x11_get_core_keyboard_device_id (s_display->xcb.connection); - if (s_display->xkb.device_id == -1) - return FALSE; - - s_display->xkb.context = xkb_context_new (0); - if (!s_display->xkb.context) - return FALSE; - - s_display->xkb.keymap = xkb_x11_keymap_new_from_device (s_display->xkb.context, s_display->xcb.connection, s_display->xkb.device_id, XKB_KEYMAP_COMPILE_NO_FLAGS); - if (!s_display->xkb.keymap) - return FALSE; + if (((s_display->xkb.device_id = xkb_x11_get_core_keyboard_device_id(s_display->xcb.connection)) == -1) || + !(s_display->xkb.context = xkb_context_new(0)) || + !(s_display->xkb.keymap = + xkb_x11_keymap_new_from_device(s_display->xkb.context, s_display->xcb.connection, + s_display->xkb.device_id, XKB_KEYMAP_COMPILE_NO_FLAGS))) + goto no_xkb_cleanup; s_display->xkb.shift = 1 << xkb_keymap_mod_get_index(s_display->xkb.keymap, "Shift"); s_display->xkb.control = 1 << xkb_keymap_mod_get_index(s_display->xkb.keymap, "Control"); @@ -625,16 +678,48 @@ init_xkb (void) s_display->xkb.caps_lock = 1 << xkb_keymap_mod_get_index(s_display->xkb.keymap, "Lock"); s_display->xkb.num_lock = 1 << xkb_keymap_mod_get_index(s_display->xkb.keymap, "NumLock"); - s_display->xkb.state = - xkb_x11_state_new_from_device(s_display->xkb.keymap, s_display->xcb.connection, s_display->xkb.device_id); - if (!s_display->xkb.state) - return FALSE; + if (!(s_display->xkb.state = xkb_x11_state_new_from_device(s_display->xkb.keymap, s_display->xcb.connection, + s_display->xkb.device_id))) + goto no_xkb_cleanup; return TRUE; + +no_xkb_cleanup: + g_clear_pointer(&s_display->xkb.keymap, xkb_keymap_unref); + g_clear_pointer(&s_display->xkb.context, xkb_context_unref); + s_display->xkb.device_id = -1; + return FALSE; +} + +static gboolean +init_keyboard(GError **error) +{ + g_autoptr(GString) tried_impl = NULL; + + if (init_xkb()) { + g_debug("%s: Using XKB", G_STRFUNC); + return TRUE; + } + +#ifdef COG_X11_USE_XCB_KEYSYMS + tried_impl = tried_impl ? g_string_append(tried_impl, ", XCB-Keysyms") : g_string_new("XCB-Keysyms"); + + if ((s_display->xcb_keysyms = xcb_key_symbols_alloc(s_display->xcb.connection))) { + g_debug("%s: Using XCB-Keysyms", G_STRFUNC); + return TRUE; + } +#endif /* COG_X11_USE_XCB_KEYSYMS */ + + g_set_error(error, + COG_PLATFORM_WPE_ERROR, + COG_PLATFORM_WPE_ERROR_INIT, + "Could not initialize keyboard, tried %s", + tried_impl ? tried_impl->str : "no implementations"); + return FALSE; } static void -clear_xkb (void) +clear_keyboard(void) { if (s_display->xkb.state) xkb_state_unref (s_display->xkb.state); @@ -642,6 +727,10 @@ clear_xkb (void) xkb_keymap_unref (s_display->xkb.keymap); if (s_display->xkb.context) xkb_context_unref (s_display->xkb.context); + +#ifdef COG_X11_USE_XCB_KEYSYMS + g_clear_pointer(&s_display->xcb_keysyms, xcb_key_symbols_free); +#endif /* COG_X11_USE_XCB_KEYSYMS */ } static gboolean @@ -798,12 +887,9 @@ cog_x11_platform_setup(CogPlatform *platform, CogShell *shell G_GNUC_UNUSED, con return FALSE; } - if (!init_xkb ()) { - g_set_error_literal (error, - COG_PLATFORM_WPE_ERROR, - COG_PLATFORM_WPE_ERROR_INIT, - "Failed to initialize XKB"); - return FALSE; + if (!init_keyboard(error)) { + g_warning("Running without proper keyboard support: %s", (*error)->message); + g_clear_error(error); } if (!init_egl ()) { @@ -842,9 +928,9 @@ cog_x11_platform_finalize(GObject *object) { clear_glib (); cog_gl_renderer_finalize(&s_display->gl_render); - clear_egl (); - clear_xkb (); - clear_xcb (); + clear_egl(); + clear_keyboard(); + clear_xcb(); g_clear_pointer (&s_window, free); g_clear_pointer (&s_display, free); diff --git a/platform/x11/meson.build b/platform/x11/meson.build index 54909940..2d74c58a 100644 --- a/platform/x11/meson.build +++ b/platform/x11/meson.build @@ -3,19 +3,30 @@ if libportal_dep.found() x11_platform_sources += ['cog-xdp-parent-x11.c'] endif +x11_platform_c_args = platform_c_args + [ + '-DG_LOG_DOMAIN="Cog-X11"', +] +x11_platform_dependencies = [ + wpebackend_fdo_dep, + cogplatformcommon_dep, + dependency('egl'), + dependency('xcb'), + dependency('xkbcommon-x11'), + dependency('x11-xcb'), + dependency('xcb-cursor'), +] + +xcb_keysyms_dep = dependency('xcb-keysyms', required: false) +if xcb_keysyms_dep.found() + x11_platform_dependencies += [xcb_keysyms_dep] + x11_platform_c_args += ['-DCOG_X11_USE_XCB_KEYSYMS'] +endif + x11_platform_plugin = shared_module('cogplatform-x11', 'cog-platform-x11.c', x11_platform_sources, - c_args: platform_c_args + ['-DG_LOG_DOMAIN="Cog-X11"'], - dependencies: [ - wpebackend_fdo_dep, - cogplatformcommon_dep, - dependency('egl'), - dependency('xcb'), - dependency('xkbcommon-x11'), - dependency('x11-xcb'), - dependency('xcb-cursor'), - ], + c_args: x11_platform_c_args, + dependencies: x11_platform_dependencies, gnu_symbol_visibility: 'hidden', install_dir: plugin_path, install: true, From 1f400fb99eae30b7183ed45aebee2ca6c98c3959 Mon Sep 17 00:00:00 2001 From: Adrian Perez de Castro Date: Tue, 28 Nov 2023 11:43:25 +0200 Subject: [PATCH 2/3] x11: Make XKB support optional at build time Allow disabling the XKB support in the x11 platform plug-in at build time. For this, introduce a new "x11_keyboard" Meson build option that can be set to use xkb, xcb-keysyms, both, or none. Preprocessor guards are introduced where needed to skip the XKB code. --- meson_options.txt | 9 +++++++++ platform/x11/cog-platform-x11.c | 26 +++++++++++++++++++------- platform/x11/meson.build | 17 +++++++++++++---- 3 files changed, 41 insertions(+), 11 deletions(-) diff --git a/meson_options.txt b/meson_options.txt index 8ae9660f..7d24c6f5 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -52,6 +52,15 @@ option( description: 'Build content-protection support in the Wayland platform plug-in' ) +# X11 platform-specifig features +option( + 'x11_keyboard', + type: 'array', + value: ['xkb'], + choices: ['xkb', 'xcb-keysyms'], + description: 'Keyboard handler (xkb recommended)', +) + # Additional cog/cogctl options option( 'cog_appid', diff --git a/platform/x11/cog-platform-x11.c b/platform/x11/cog-platform-x11.c index b0b31a0d..ac42b6ae 100644 --- a/platform/x11/cog-platform-x11.c +++ b/platform/x11/cog-platform-x11.c @@ -23,7 +23,9 @@ # include #endif /* COG_X11_USE_XCB_KEYSYMS */ -#include +#ifdef COG_X11_USE_XKB +# include +#endif #if COG_HAVE_LIBPORTAL # include "../common/cog-file-chooser.h" @@ -87,6 +89,7 @@ struct CogX11Display { GSource *source; } xcb; +#ifdef COG_X11_USE_XKB struct { int32_t device_id; struct xkb_context *context; @@ -98,6 +101,7 @@ struct CogX11Display { xkb_mod_mask_t num_lock; xkb_mod_mask_t caps_lock; } xkb; +#endif /* COG_X11_USE_XKB */ #ifdef COG_X11_USE_XCB_KEYSYMS xcb_key_symbols_t *xcb_keysyms; @@ -194,6 +198,7 @@ xcb_paint_image (struct wpe_fdo_egl_exported_image *image) eglSwapBuffers(s_display->egl.display, s_window->egl.surface); } +#ifdef COG_X11_USE_XKB static uint32_t xcb_update_xkb_modifiers(uint32_t event_state) { @@ -220,6 +225,7 @@ xcb_update_xkb_modifiers(uint32_t event_state) xkb_state_update_mask(s_display->xkb.state, depressed_mods, 0, locked_mods, 0, 0, 0); return wpe_modifiers; } +#endif /* COG_X11_USE_XKB */ #if COG_X11_USE_XCB_KEYSYMS /* @@ -271,11 +277,13 @@ key_event_fill(struct wpe_input_keyboard_event *wpe_event, xcb_key_press_event_t wpe_event->time = event->time; wpe_event->hardware_key_code = event->detail; +#if COG_X11_USE_XKB if (s_display->xkb.device_id >= 0 && s_display->xkb.state) { wpe_event->modifiers = xcb_update_xkb_modifiers(event->state); wpe_event->key_code = xkb_state_key_get_one_sym(s_display->xkb.state, event->detail); return; } +#endif /* COG_X11_USE_XKB */ #if COG_X11_USE_XCB_KEYSYMS if (s_display->xcb_keysyms) { @@ -662,6 +670,7 @@ clear_xcb (void) XCloseDisplay (s_display->display); } +#ifdef COG_X11_USE_XKB static gboolean init_xkb (void) { @@ -690,16 +699,20 @@ init_xkb (void) s_display->xkb.device_id = -1; return FALSE; } +#endif /* COG_X11_USE_XKB */ static gboolean init_keyboard(GError **error) { g_autoptr(GString) tried_impl = NULL; +#ifdef COG_X11_USE_XKB + tried_impl = g_string_new("XKB"); if (init_xkb()) { g_debug("%s: Using XKB", G_STRFUNC); return TRUE; } +#endif /* COG_X11_USE_XKB */ #ifdef COG_X11_USE_XCB_KEYSYMS tried_impl = tried_impl ? g_string_append(tried_impl, ", XCB-Keysyms") : g_string_new("XCB-Keysyms"); @@ -721,12 +734,11 @@ init_keyboard(GError **error) static void clear_keyboard(void) { - if (s_display->xkb.state) - xkb_state_unref (s_display->xkb.state); - if (s_display->xkb.keymap) - xkb_keymap_unref (s_display->xkb.keymap); - if (s_display->xkb.context) - xkb_context_unref (s_display->xkb.context); +#ifdef COG_X11_USE_XKB + g_clear_pointer(&s_display->xkb.state, xkb_state_unref); + g_clear_pointer(&s_display->xkb.keymap, xkb_keymap_unref); + g_clear_pointer(&s_display->xkb.context, xkb_context_unref); +#endif /* COG_X11_USE_XKB */ #ifdef COG_X11_USE_XCB_KEYSYMS g_clear_pointer(&s_display->xcb_keysyms, xcb_key_symbols_free); diff --git a/platform/x11/meson.build b/platform/x11/meson.build index 2d74c58a..389eb0c9 100644 --- a/platform/x11/meson.build +++ b/platform/x11/meson.build @@ -16,12 +16,21 @@ x11_platform_dependencies = [ dependency('xcb-cursor'), ] -xcb_keysyms_dep = dependency('xcb-keysyms', required: false) -if xcb_keysyms_dep.found() - x11_platform_dependencies += [xcb_keysyms_dep] - x11_platform_c_args += ['-DCOG_X11_USE_XCB_KEYSYMS'] +x11_platform_keyboard = get_option('x11_keyboard') +if x11_platform_keyboard.length() == 0 + warning('No X11 keyboard support chosen, keyboard input will NOT work') endif +x11_platform_keyboard_dep_names = { + 'xkb': 'xkbcommon-x11', + 'xcb-keysyms': 'xcb-keysyms', +} + +foreach item : x11_platform_keyboard + x11_platform_dependencies += [dependency(x11_platform_keyboard_dep_names[item])] + x11_platform_c_args += ['-DCOG_X11_USE_@0@=1'.format(item.underscorify().to_upper())] +endforeach + x11_platform_plugin = shared_module('cogplatform-x11', 'cog-platform-x11.c', x11_platform_sources, From 891c33ca663851094274f80dca7b860cb43be3f9 Mon Sep 17 00:00:00 2001 From: Adrian Perez de Castro Date: Thu, 30 Nov 2023 14:46:20 +0200 Subject: [PATCH 3/3] meson: Add a Wrap subproject for xcb-keysyms Add a Wrap subproject definition for the xcb-keysyms dependency. It is a very simple library with a single C source file, so it is easy to make Meson build definitions for it that get added on top of the tarball release fetched by Meson if xcb-keysyms is not available. This way it is guaranteed that there will be at least some support for keyboard input if the XKB extension is not available in the X server, or not enabled at build time. --- .gitignore | 2 ++ meson.build | 2 +- platform/x11/cog-platform-x11.c | 6 ++++- .../xcb-util-keysyms-0.4.1/meson.build | 24 +++++++++++++++++++ subprojects/xcb-keysyms.wrap | 9 +++++++ 5 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 .gitignore create mode 100644 subprojects/packagefiles/xcb-util-keysyms-0.4.1/meson.build create mode 100644 subprojects/xcb-keysyms.wrap diff --git a/.gitignore b/.gitignore new file mode 100644 index 00000000..f80b8690 --- /dev/null +++ b/.gitignore @@ -0,0 +1,2 @@ +/subprojects/packagecache/ +/subprojects/xcb-util-keysyms-0.4.1/ diff --git a/meson.build b/meson.build index d89346fe..5097173e 100644 --- a/meson.build +++ b/meson.build @@ -1,5 +1,5 @@ project('cog', 'c', - meson_version: '>=0.53.2', + meson_version: '>=0.55', default_options: [ 'buildtype=debugoptimized', 'b_ndebug=if-release', diff --git a/platform/x11/cog-platform-x11.c b/platform/x11/cog-platform-x11.c index ac42b6ae..9db380db 100644 --- a/platform/x11/cog-platform-x11.c +++ b/platform/x11/cog-platform-x11.c @@ -20,7 +20,11 @@ #include #ifdef COG_X11_USE_XCB_KEYSYMS -# include +# if __has_include() +# include +# else +# include +# endif #endif /* COG_X11_USE_XCB_KEYSYMS */ #ifdef COG_X11_USE_XKB diff --git a/subprojects/packagefiles/xcb-util-keysyms-0.4.1/meson.build b/subprojects/packagefiles/xcb-util-keysyms-0.4.1/meson.build new file mode 100644 index 00000000..60469c36 --- /dev/null +++ b/subprojects/packagefiles/xcb-util-keysyms-0.4.1/meson.build @@ -0,0 +1,24 @@ +project('xcb-util-keysyms', 'c', + version: '0.4.1', + default_options: [ + ], +) + +xcb_dep = dependency('xcb', version: '>=1.4') +xcb_util_keysyms_dependencies = [xcb_dep, dependency('xproto', version: '>=7.0.8')] + +xcb_proto_version = xcb_dep.get_variable('xcbproto_version') +assert(xcb_proto_version.version_compare('>=1.6'), + 'libxcb was compiled against xcb-proto @0@, it needs to be compiled against 1.6 or newer'.format(xcb_proto_version)) + +xcb_util_keysyms_lib = static_library('xcb-util-keysyms', + 'keysyms/keysyms.c', + dependencies: xcb_util_keysyms_dependencies, + pic: true, +) + +xcb_util_keysyms_dep = declare_dependency( + link_with: xcb_util_keysyms_lib, + dependencies: xcb_util_keysyms_dependencies, + include_directories: include_directories('keysyms'), +) diff --git a/subprojects/xcb-keysyms.wrap b/subprojects/xcb-keysyms.wrap new file mode 100644 index 00000000..8b78d33f --- /dev/null +++ b/subprojects/xcb-keysyms.wrap @@ -0,0 +1,9 @@ +[wrap-file] +directory = xcb-util-keysyms-0.4.1 +source_url = https://xcb.freedesktop.org/dist/xcb-util-keysyms-0.4.1.tar.xz +source_filename = xcb-util-keysyms-0.4.1.tar.xz +source_hash = 7c260a5294412aed429df1da2f8afd3bd07b7cba3fec772fba15a613a6d5c638 +patch_directory = xcb-util-keysyms-0.4.1 + +[provide] +xcb-keysyms = xcb_util_keysyms_dep