From fc4fdbb6438aa6e305aefbd6d3a1d3465c2a9321 Mon Sep 17 00:00:00 2001 From: Chris Marchesi Date: Sat, 30 Dec 2023 19:29:23 -0800 Subject: [PATCH 1/5] apprt/gtk: ensure modifier state matches current keypress under X11 This fixes an issue in that when running under X11, when a modifier key is pressed, the modifier state will "lag" behind what should be current. This is due to how X11 sends modifiers in events, i.e. it sends the state from right before the key press, and does not include the effects of the key press itself. This is corrected by checking the X event queue directly for a pending XkbStateNotify event (we mask this on modifiers), and setting the modifiers off of that if we find one. If not, we fall back to the GDK call. --- src/apprt/gtk/App.zig | 3 +- src/apprt/gtk/Surface.zig | 26 ++++++++- src/apprt/gtk/c.zig | 2 + src/apprt/gtk/x11.zig | 120 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 148 insertions(+), 3 deletions(-) create mode 100644 src/apprt/gtk/x11.zig diff --git a/src/apprt/gtk/App.zig b/src/apprt/gtk/App.zig index f687c68a0..7142db995 100644 --- a/src/apprt/gtk/App.zig +++ b/src/apprt/gtk/App.zig @@ -28,6 +28,7 @@ const ClipboardConfirmationWindow = @import("ClipboardConfirmationWindow.zig"); const c = @import("c.zig"); const inspector = @import("inspector.zig"); const key = @import("key.zig"); +const x11 = @import("x11.zig"); const testing = std.testing; const log = std.log.scoped(.gtk); @@ -170,7 +171,7 @@ pub fn init(core_app: *CoreApp, opts: Options) !App { } const display = c.gdk_display_get_default(); - if (c.g_type_check_instance_is_a(@ptrCast(@alignCast(display)), c.gdk_x11_display_get_type()) != 0) { + if (x11.x11_is_display(display)) { // Set the X11 window class property (WM_CLASS) if are are on an X11 // display. // diff --git a/src/apprt/gtk/Surface.zig b/src/apprt/gtk/Surface.zig index 413130660..01938c1be 100644 --- a/src/apprt/gtk/Surface.zig +++ b/src/apprt/gtk/Surface.zig @@ -19,6 +19,7 @@ const Window = @import("Window.zig"); const ClipboardConfirmationWindow = @import("ClipboardConfirmationWindow.zig"); const inspector = @import("inspector.zig"); const gtk_key = @import("key.zig"); +const x11 = @import("x11.zig"); const c = @import("c.zig"); const log = std.log.scoped(.gtk_surface); @@ -253,6 +254,9 @@ im_composing: bool = false, im_buf: [128]u8 = undefined, im_len: u7 = 0, +/// Xkb state (X11 only). Will be null on Wayland. +x11_xkb: ?x11.X11Xkb = null, + pub fn create(alloc: Allocator, app: *App, opts: Options) !*Surface { var surface = try alloc.create(Surface); errdefer alloc.destroy(surface); @@ -355,6 +359,12 @@ pub fn init(self: *Surface, app: *App, opts: Options) !void { }; errdefer self.* = undefined; + // Set up Xkb if we are running under X11. + const display = c.gdk_display_get_default(); + if (x11.x11_is_display(display)) { + self.x11_xkb = try x11.X11Xkb.init(display); + } + // Set our default mouse shape try self.setMouseShape(.text); @@ -1222,6 +1232,7 @@ fn keyEvent( const self = userdataSelf(ud.?); const keyval_unicode = c.gdk_keyval_to_unicode(keyval); const event = c.gtk_event_controller_get_current_event(@ptrCast(ec_key)); + const display = c.gtk_widget_get_display(@ptrCast(self.gl_area)); // Get the unshifted unicode value of the keyval. This is used // by the Kitty keyboard protocol. @@ -1232,7 +1243,6 @@ fn keyEvent( // Get all the possible keyboard mappings for this keycode. A keycode // is the physical key pressed. - const display = c.gtk_widget_get_display(@ptrCast(self.gl_area)); var keys: [*]c.GdkKeymapKey = undefined; var keyvals: [*]c.guint = undefined; var n_keys: c_int = 0; @@ -1335,7 +1345,19 @@ fn keyEvent( _ = gtk_mods; const device = c.gdk_event_get_device(event); - var mods = translateMods(c.gdk_device_get_modifier_state(device)); + var mods = if (self.x11_xkb) |x11_xkb| init_mods: { + // Add any modifier state events from Xkb if we have them (X11 only). + if (x11_xkb.modifier_state_from_notify(display)) |xkb_mods| { + break :init_mods xkb_mods; + } else { + // Null back from the Xkb call means there was no modifier + // event to read. This likely means that the key event did not + // result in a modifier change and we can safely rely on the + // GDK state. + break :init_mods translateMods(c.gdk_device_get_modifier_state(device)); + } + } else translateMods(c.gdk_device_get_modifier_state(device)); + mods.num_lock = c.gdk_device_get_num_lock_state(device) == 1; switch (physical_key) { diff --git a/src/apprt/gtk/c.zig b/src/apprt/gtk/c.zig index d7e85f376..ffe7b1d0e 100644 --- a/src/apprt/gtk/c.zig +++ b/src/apprt/gtk/c.zig @@ -5,6 +5,8 @@ const c = @cImport({ // Add in X11-specific GDK backend which we use for specific things (e.g. // X11 window class). @cInclude("gdk/x11/gdkx.h"); + // Xkb for X11 state handling + @cInclude("X11/XKBlib.h"); }); pub usingnamespace c; diff --git a/src/apprt/gtk/x11.zig b/src/apprt/gtk/x11.zig new file mode 100644 index 000000000..a70920c79 --- /dev/null +++ b/src/apprt/gtk/x11.zig @@ -0,0 +1,120 @@ +/// Utility functions for X11 handling. +const std = @import("std"); +const c = @import("c.zig"); +const input = @import("../../input.zig"); + +const log = std.log.scoped(.gtk); + +// X11 Function types. We load these dynamically at runtime to avoid having to +// link against X11. +const XkbQueryExtensionType = *const fn (?*c.struct__XDisplay, [*c]c_int, [*c]c_int, [*c]c_int, [*c]c_int, [*c]c_int) callconv(.C) c_int; +const XkbSelectEventDetailsType = *const fn (?*c.struct__XDisplay, c_uint, c_uint, c_ulong, c_ulong) callconv(.C) c_int; +const XEventsQueuedType = *const fn (?*c.struct__XDisplay, c_int) callconv(.C) c_int; +const XPeekEventType = *const fn (?*c.struct__XDisplay, [*c]c.union__XEvent) callconv(.C) c_int; + +/// Returns true if the passed in display is an X11 display. +pub fn x11_is_display(display_: ?*c.GdkDisplay) bool { + const display = display_ orelse return false; + return (c.g_type_check_instance_is_a(@ptrCast(@alignCast(display)), c.gdk_x11_display_get_type()) != 0); +} + +pub const X11Xkb = struct { + opcode: c_int, + base_event_code: c_int, + base_error_code: c_int, + + // Dynamic functions + XkbQueryExtension: XkbQueryExtensionType = undefined, + XkbSelectEventDetails: XkbSelectEventDetailsType = undefined, + XEventsQueued: XEventsQueuedType = undefined, + XPeekEvent: XPeekEventType = undefined, + + pub fn init(display_: ?*c.GdkDisplay) !X11Xkb { + log.debug("X11: X11Xkb.init: initializing Xkb", .{}); + const display = display_ orelse { + log.err("Fatal: error initializing Xkb extension: display is null", .{}); + return error.XkbInitializationError; + }; + + const xdisplay = c.gdk_x11_display_get_xdisplay(display); + var result: X11Xkb = .{ .opcode = 0, .base_event_code = 0, .base_error_code = 0 }; + + // Load in the X11 calls we need. + log.debug("X11: X11Xkb.init: loading libX11.so dynamically", .{}); + var libX11 = try std.DynLib.open("libX11.so"); + defer libX11.close(); + result.XkbQueryExtension = libX11.lookup(XkbQueryExtensionType, "XkbQueryExtension") orelse { + log.err("Fatal: error dynamic loading libX11: missing symbol XkbQueryExtension", .{}); + return error.XkbInitializationError; + }; + result.XkbSelectEventDetails = libX11.lookup(XkbSelectEventDetailsType, "XkbSelectEventDetails") orelse { + log.err("Fatal: error dynamic loading libX11: missing symbol XkbSelectEventDetails", .{}); + return error.XkbInitializationError; + }; + result.XEventsQueued = libX11.lookup(XEventsQueuedType, "XEventsQueued") orelse { + log.err("Fatal: error dynamic loading libX11: missing symbol XEventsQueued", .{}); + return error.XkbInitializationError; + }; + result.XPeekEvent = libX11.lookup(XPeekEventType, "XPeekEvent") orelse { + log.err("Fatal: error dynamic loading libX11: missing symbol XPeekEvent", .{}); + return error.XkbInitializationError; + }; + + log.debug("X11: X11Xkb.init: running XkbQueryExtension", .{}); + var major = c.XkbMajorVersion; + var minor = c.XkbMinorVersion; + if (result.XkbQueryExtension(xdisplay, &result.opcode, &result.base_event_code, &result.base_error_code, &major, &minor) == 0) { + log.err("Fatal: error initializing Xkb extension: error executing XkbQueryExtension", .{}); + return error.XkbInitializationError; + } + + log.debug("X11: X11Xkb.init: running XkbSelectEventDetails", .{}); + if (result.XkbSelectEventDetails(xdisplay, c.XkbUseCoreKbd, c.XkbStateNotify, c.XkbModifierStateMask, c.XkbModifierStateMask) == 0) { + log.err("Fatal: error initializing Xkb extension: error executing XkbSelectEventDetails", .{}); + return error.XkbInitializationError; + } + + return result; + } + + /// Checks for an immediate pending XKB state update event, and returns the + /// keyboard state based on if it finds any. This is necessary as the + /// standard GTK X11 API (and X11 in general) does not include the current + /// key pressed in any modifier state snapshot for that event (e.g. if the + /// pressed key is a modifier, that is not necessarily reflected in the + /// modifiers). + /// + /// Returns null if there is no event. In this case, the caller should fall + /// back to the standard GDK modifier state (this likely means the key + /// event did not result in a modifier change). + pub fn modifier_state_from_notify(self: X11Xkb, display_: ?*c.GdkDisplay) ?input.Mods { + const display = display_ orelse return null; + // Shoutout to Mozilla for figuring out a clean way to do this, this is + // paraphrased from Firefox/Gecko in widget/gtk/nsGtkKeyUtils.cpp. + const xdisplay = c.gdk_x11_display_get_xdisplay(display); + if (self.XEventsQueued(xdisplay, c.QueuedAfterReading) != 0) { + var nextEvent: c.XEvent = undefined; + _ = self.XPeekEvent(xdisplay, &nextEvent); + if (nextEvent.type == self.base_event_code) { + const xkb_event: *c.XkbEvent = @ptrCast(&nextEvent); + if (xkb_event.any.xkb_type == c.XkbStateNotify) { + const xkb_state_notify_event: *c.XkbStateNotifyEvent = @ptrCast(xkb_event); + // Check the state according to XKB masks. + const lookup_mods = xkb_state_notify_event.lookup_mods; + var mods: input.Mods = .{}; + + log.debug("X11: found extra XkbStateNotify event w/lookup_mods: {b}", .{lookup_mods}); + if (lookup_mods & c.ShiftMask != 0) mods.shift = true; + if (lookup_mods & c.ControlMask != 0) mods.ctrl = true; + if (lookup_mods & c.Mod1Mask != 0) mods.alt = true; + if (lookup_mods & c.Mod2Mask != 0) mods.super = true; + if (lookup_mods & c.LockMask != 0) mods.caps_lock = true; + + return mods; + } + } + } + + return null; + } +}; From 3b7e83150f91d655cccf9f0e3bfb9566f421945c Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 31 Dec 2023 07:50:36 -0800 Subject: [PATCH 2/5] apprt/gtk: stylistic changes --- src/apprt/gtk/Surface.zig | 22 ++++---- src/apprt/gtk/x11.zig | 113 ++++++++++++++++++++++++-------------- 2 files changed, 84 insertions(+), 51 deletions(-) diff --git a/src/apprt/gtk/Surface.zig b/src/apprt/gtk/Surface.zig index 01938c1be..2ab1a0051 100644 --- a/src/apprt/gtk/Surface.zig +++ b/src/apprt/gtk/Surface.zig @@ -360,10 +360,7 @@ pub fn init(self: *Surface, app: *App, opts: Options) !void { errdefer self.* = undefined; // Set up Xkb if we are running under X11. - const display = c.gdk_display_get_default(); - if (x11.x11_is_display(display)) { - self.x11_xkb = try x11.X11Xkb.init(display); - } + self.x11_xkb = try x11.X11Xkb.init(c.gdk_display_get_default()); // Set our default mouse shape try self.setMouseShape(.text); @@ -1345,18 +1342,21 @@ fn keyEvent( _ = gtk_mods; const device = c.gdk_event_get_device(event); - var mods = if (self.x11_xkb) |x11_xkb| init_mods: { - // Add any modifier state events from Xkb if we have them (X11 only). - if (x11_xkb.modifier_state_from_notify(display)) |xkb_mods| { - break :init_mods xkb_mods; - } else { + var mods = init_mods: { + if (self.x11_xkb) |x11_xkb| { + // Add any modifier state events from Xkb if we have them (X11 only). + if (x11_xkb.modifier_state_from_notify(display)) |xkb_mods| { + break :init_mods xkb_mods; + } + // Null back from the Xkb call means there was no modifier // event to read. This likely means that the key event did not // result in a modifier change and we can safely rely on the // GDK state. - break :init_mods translateMods(c.gdk_device_get_modifier_state(device)); } - } else translateMods(c.gdk_device_get_modifier_state(device)); + + break :init_mods translateMods(c.gdk_device_get_modifier_state(device)); + }; mods.num_lock = c.gdk_device_get_num_lock_state(device) == 1; diff --git a/src/apprt/gtk/x11.zig b/src/apprt/gtk/x11.zig index a70920c79..0157af9b8 100644 --- a/src/apprt/gtk/x11.zig +++ b/src/apprt/gtk/x11.zig @@ -3,7 +3,7 @@ const std = @import("std"); const c = @import("c.zig"); const input = @import("../../input.zig"); -const log = std.log.scoped(.gtk); +const log = std.log.scoped(.gtk_x11); // X11 Function types. We load these dynamically at runtime to avoid having to // link against X11. @@ -13,9 +13,11 @@ const XEventsQueuedType = *const fn (?*c.struct__XDisplay, c_int) callconv(.C) c const XPeekEventType = *const fn (?*c.struct__XDisplay, [*c]c.union__XEvent) callconv(.C) c_int; /// Returns true if the passed in display is an X11 display. -pub fn x11_is_display(display_: ?*c.GdkDisplay) bool { - const display = display_ orelse return false; - return (c.g_type_check_instance_is_a(@ptrCast(@alignCast(display)), c.gdk_x11_display_get_type()) != 0); +pub fn x11_is_display(display: ?*c.GdkDisplay) bool { + return c.g_type_check_instance_is_a( + @ptrCast(@alignCast(display orelse return false)), + c.gdk_x11_display_get_type(), + ) != 0; } pub const X11Xkb = struct { @@ -29,47 +31,79 @@ pub const X11Xkb = struct { XEventsQueued: XEventsQueuedType = undefined, XPeekEvent: XPeekEventType = undefined, - pub fn init(display_: ?*c.GdkDisplay) !X11Xkb { - log.debug("X11: X11Xkb.init: initializing Xkb", .{}); - const display = display_ orelse { - log.err("Fatal: error initializing Xkb extension: display is null", .{}); - return error.XkbInitializationError; - }; + /// Initialize an X11Xkb struct, for the given GDK display. If the display + /// isn't backed by X then this will return null. + pub fn init(display_: ?*c.GdkDisplay) !?X11Xkb { + // Display should never be null but we just treat that as a non-X11 + // display so that the caller can just ignore it and not unwrap it. + const display = display_ orelse return null; + // If the display isn't X11, then we don't need to do anything. + if (!x11_is_display(display)) return null; + + log.debug("X11Xkb.init: initializing Xkb", .{}); const xdisplay = c.gdk_x11_display_get_xdisplay(display); var result: X11Xkb = .{ .opcode = 0, .base_event_code = 0, .base_error_code = 0 }; // Load in the X11 calls we need. - log.debug("X11: X11Xkb.init: loading libX11.so dynamically", .{}); + log.debug(" X11Xkb.init: loading libX11.so dynamically", .{}); var libX11 = try std.DynLib.open("libX11.so"); defer libX11.close(); - result.XkbQueryExtension = libX11.lookup(XkbQueryExtensionType, "XkbQueryExtension") orelse { + result.XkbQueryExtension = libX11.lookup( + XkbQueryExtensionType, + "XkbQueryExtension", + ) orelse { log.err("Fatal: error dynamic loading libX11: missing symbol XkbQueryExtension", .{}); return error.XkbInitializationError; }; - result.XkbSelectEventDetails = libX11.lookup(XkbSelectEventDetailsType, "XkbSelectEventDetails") orelse { + + result.XkbSelectEventDetails = libX11.lookup( + XkbSelectEventDetailsType, + "XkbSelectEventDetails", + ) orelse { log.err("Fatal: error dynamic loading libX11: missing symbol XkbSelectEventDetails", .{}); return error.XkbInitializationError; }; - result.XEventsQueued = libX11.lookup(XEventsQueuedType, "XEventsQueued") orelse { + + result.XEventsQueued = libX11.lookup( + XEventsQueuedType, + "XEventsQueued", + ) orelse { log.err("Fatal: error dynamic loading libX11: missing symbol XEventsQueued", .{}); return error.XkbInitializationError; }; - result.XPeekEvent = libX11.lookup(XPeekEventType, "XPeekEvent") orelse { + + result.XPeekEvent = libX11.lookup( + XPeekEventType, + "XPeekEvent", + ) orelse { log.err("Fatal: error dynamic loading libX11: missing symbol XPeekEvent", .{}); return error.XkbInitializationError; }; - log.debug("X11: X11Xkb.init: running XkbQueryExtension", .{}); + log.debug("X11Xkb.init: running XkbQueryExtension", .{}); var major = c.XkbMajorVersion; var minor = c.XkbMinorVersion; - if (result.XkbQueryExtension(xdisplay, &result.opcode, &result.base_event_code, &result.base_error_code, &major, &minor) == 0) { + if (result.XkbQueryExtension( + xdisplay, + &result.opcode, + &result.base_event_code, + &result.base_error_code, + &major, + &minor, + ) == 0) { log.err("Fatal: error initializing Xkb extension: error executing XkbQueryExtension", .{}); return error.XkbInitializationError; } - log.debug("X11: X11Xkb.init: running XkbSelectEventDetails", .{}); - if (result.XkbSelectEventDetails(xdisplay, c.XkbUseCoreKbd, c.XkbStateNotify, c.XkbModifierStateMask, c.XkbModifierStateMask) == 0) { + log.debug("X11Xkb.init: running XkbSelectEventDetails", .{}); + if (result.XkbSelectEventDetails( + xdisplay, + c.XkbUseCoreKbd, + c.XkbStateNotify, + c.XkbModifierStateMask, + c.XkbModifierStateMask, + ) == 0) { log.err("Fatal: error initializing Xkb extension: error executing XkbSelectEventDetails", .{}); return error.XkbInitializationError; } @@ -89,32 +123,31 @@ pub const X11Xkb = struct { /// event did not result in a modifier change). pub fn modifier_state_from_notify(self: X11Xkb, display_: ?*c.GdkDisplay) ?input.Mods { const display = display_ orelse return null; + // Shoutout to Mozilla for figuring out a clean way to do this, this is // paraphrased from Firefox/Gecko in widget/gtk/nsGtkKeyUtils.cpp. const xdisplay = c.gdk_x11_display_get_xdisplay(display); - if (self.XEventsQueued(xdisplay, c.QueuedAfterReading) != 0) { - var nextEvent: c.XEvent = undefined; - _ = self.XPeekEvent(xdisplay, &nextEvent); - if (nextEvent.type == self.base_event_code) { - const xkb_event: *c.XkbEvent = @ptrCast(&nextEvent); - if (xkb_event.any.xkb_type == c.XkbStateNotify) { - const xkb_state_notify_event: *c.XkbStateNotifyEvent = @ptrCast(xkb_event); - // Check the state according to XKB masks. - const lookup_mods = xkb_state_notify_event.lookup_mods; - var mods: input.Mods = .{}; + if (self.XEventsQueued(xdisplay, c.QueuedAfterReading) == 0) return null; - log.debug("X11: found extra XkbStateNotify event w/lookup_mods: {b}", .{lookup_mods}); - if (lookup_mods & c.ShiftMask != 0) mods.shift = true; - if (lookup_mods & c.ControlMask != 0) mods.ctrl = true; - if (lookup_mods & c.Mod1Mask != 0) mods.alt = true; - if (lookup_mods & c.Mod2Mask != 0) mods.super = true; - if (lookup_mods & c.LockMask != 0) mods.caps_lock = true; + var nextEvent: c.XEvent = undefined; + _ = self.XPeekEvent(xdisplay, &nextEvent); + if (nextEvent.type != self.base_event_code) return null; - return mods; - } - } - } + const xkb_event: *c.XkbEvent = @ptrCast(&nextEvent); + if (xkb_event.any.xkb_type != c.XkbStateNotify) return null; - return null; + const xkb_state_notify_event: *c.XkbStateNotifyEvent = @ptrCast(xkb_event); + // Check the state according to XKB masks. + const lookup_mods = xkb_state_notify_event.lookup_mods; + var mods: input.Mods = .{}; + + log.debug("X11: found extra XkbStateNotify event w/lookup_mods: {b}", .{lookup_mods}); + if (lookup_mods & c.ShiftMask != 0) mods.shift = true; + if (lookup_mods & c.ControlMask != 0) mods.ctrl = true; + if (lookup_mods & c.Mod1Mask != 0) mods.alt = true; + if (lookup_mods & c.Mod2Mask != 0) mods.super = true; + if (lookup_mods & c.LockMask != 0) mods.caps_lock = true; + + return mods; } }; From a3ce446fee9c7f177f60fccdcae021234e9262fe Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 31 Dec 2023 08:00:43 -0800 Subject: [PATCH 3/5] apprt/gtk: use some comptime to load X11 functions --- src/apprt/gtk/x11.zig | 101 ++++++++++++++++++++---------------------- 1 file changed, 48 insertions(+), 53 deletions(-) diff --git a/src/apprt/gtk/x11.zig b/src/apprt/gtk/x11.zig index 0157af9b8..c9fd270b0 100644 --- a/src/apprt/gtk/x11.zig +++ b/src/apprt/gtk/x11.zig @@ -5,13 +5,6 @@ const input = @import("../../input.zig"); const log = std.log.scoped(.gtk_x11); -// X11 Function types. We load these dynamically at runtime to avoid having to -// link against X11. -const XkbQueryExtensionType = *const fn (?*c.struct__XDisplay, [*c]c_int, [*c]c_int, [*c]c_int, [*c]c_int, [*c]c_int) callconv(.C) c_int; -const XkbSelectEventDetailsType = *const fn (?*c.struct__XDisplay, c_uint, c_uint, c_ulong, c_ulong) callconv(.C) c_int; -const XEventsQueuedType = *const fn (?*c.struct__XDisplay, c_int) callconv(.C) c_int; -const XPeekEventType = *const fn (?*c.struct__XDisplay, [*c]c.union__XEvent) callconv(.C) c_int; - /// Returns true if the passed in display is an X11 display. pub fn x11_is_display(display: ?*c.GdkDisplay) bool { return c.g_type_check_instance_is_a( @@ -24,12 +17,7 @@ pub const X11Xkb = struct { opcode: c_int, base_event_code: c_int, base_error_code: c_int, - - // Dynamic functions - XkbQueryExtension: XkbQueryExtensionType = undefined, - XkbSelectEventDetails: XkbSelectEventDetailsType = undefined, - XEventsQueued: XEventsQueuedType = undefined, - XPeekEvent: XPeekEventType = undefined, + funcs: Funcs, /// Initialize an X11Xkb struct, for the given GDK display. If the display /// isn't backed by X then this will return null. @@ -43,48 +31,17 @@ pub const X11Xkb = struct { log.debug("X11Xkb.init: initializing Xkb", .{}); const xdisplay = c.gdk_x11_display_get_xdisplay(display); - var result: X11Xkb = .{ .opcode = 0, .base_event_code = 0, .base_error_code = 0 }; - - // Load in the X11 calls we need. - log.debug(" X11Xkb.init: loading libX11.so dynamically", .{}); - var libX11 = try std.DynLib.open("libX11.so"); - defer libX11.close(); - result.XkbQueryExtension = libX11.lookup( - XkbQueryExtensionType, - "XkbQueryExtension", - ) orelse { - log.err("Fatal: error dynamic loading libX11: missing symbol XkbQueryExtension", .{}); - return error.XkbInitializationError; - }; - - result.XkbSelectEventDetails = libX11.lookup( - XkbSelectEventDetailsType, - "XkbSelectEventDetails", - ) orelse { - log.err("Fatal: error dynamic loading libX11: missing symbol XkbSelectEventDetails", .{}); - return error.XkbInitializationError; - }; - - result.XEventsQueued = libX11.lookup( - XEventsQueuedType, - "XEventsQueued", - ) orelse { - log.err("Fatal: error dynamic loading libX11: missing symbol XEventsQueued", .{}); - return error.XkbInitializationError; - }; - - result.XPeekEvent = libX11.lookup( - XPeekEventType, - "XPeekEvent", - ) orelse { - log.err("Fatal: error dynamic loading libX11: missing symbol XPeekEvent", .{}); - return error.XkbInitializationError; + var result: X11Xkb = .{ + .opcode = 0, + .base_event_code = 0, + .base_error_code = 0, + .funcs = try Funcs.init(), }; log.debug("X11Xkb.init: running XkbQueryExtension", .{}); var major = c.XkbMajorVersion; var minor = c.XkbMinorVersion; - if (result.XkbQueryExtension( + if (result.funcs.XkbQueryExtension( xdisplay, &result.opcode, &result.base_event_code, @@ -97,7 +54,7 @@ pub const X11Xkb = struct { } log.debug("X11Xkb.init: running XkbSelectEventDetails", .{}); - if (result.XkbSelectEventDetails( + if (result.funcs.XkbSelectEventDetails( xdisplay, c.XkbUseCoreKbd, c.XkbStateNotify, @@ -127,10 +84,10 @@ pub const X11Xkb = struct { // Shoutout to Mozilla for figuring out a clean way to do this, this is // paraphrased from Firefox/Gecko in widget/gtk/nsGtkKeyUtils.cpp. const xdisplay = c.gdk_x11_display_get_xdisplay(display); - if (self.XEventsQueued(xdisplay, c.QueuedAfterReading) == 0) return null; + if (self.funcs.XEventsQueued(xdisplay, c.QueuedAfterReading) == 0) return null; var nextEvent: c.XEvent = undefined; - _ = self.XPeekEvent(xdisplay, &nextEvent); + _ = self.funcs.XPeekEvent(xdisplay, &nextEvent); if (nextEvent.type != self.base_event_code) return null; const xkb_event: *c.XkbEvent = @ptrCast(&nextEvent); @@ -151,3 +108,41 @@ pub const X11Xkb = struct { return mods; } }; + +/// The functions that we load dynamically from libX11.so. +const Funcs = struct { + XkbQueryExtension: XkbQueryExtensionType, + XkbSelectEventDetails: XkbSelectEventDetailsType, + XEventsQueued: XEventsQueuedType, + XPeekEvent: XPeekEventType, + + // X11 Function types. We load these dynamically at runtime to avoid having to + // link against X11. + const XkbQueryExtensionType = *const fn (?*c.struct__XDisplay, [*c]c_int, [*c]c_int, [*c]c_int, [*c]c_int, [*c]c_int) callconv(.C) c_int; + const XkbSelectEventDetailsType = *const fn (?*c.struct__XDisplay, c_uint, c_uint, c_ulong, c_ulong) callconv(.C) c_int; + const XEventsQueuedType = *const fn (?*c.struct__XDisplay, c_int) callconv(.C) c_int; + const XPeekEventType = *const fn (?*c.struct__XDisplay, [*c]c.union__XEvent) callconv(.C) c_int; + + pub fn init() !Funcs { + var libX11 = try std.DynLib.open("libX11.so"); + defer libX11.close(); + + var result: Funcs = undefined; + inline for (@typeInfo(Funcs).Struct.fields) |field| { + const name = comptime name: { + const null_term = field.name ++ .{0}; + break :name null_term[0..field.name.len :0]; + }; + + @field(result, field.name) = libX11.lookup( + field.type, + name, + ) orelse { + log.err(" error dynamic loading libX11: missing symbol {s}", .{field.name}); + return error.XkbInitializationError; + }; + } + + return result; + } +}; From d2355546061b321767b6bdb6d1929d91c0041476 Mon Sep 17 00:00:00 2001 From: Chris Marchesi Date: Sun, 31 Dec 2023 12:10:25 -0800 Subject: [PATCH 4/5] apprt/gtk: Move Xkb state to App, remove un-needed fields, style changes --- src/apprt/gtk/App.zig | 16 +++++++++++++++- src/apprt/gtk/Surface.zig | 25 ++++++++----------------- src/apprt/gtk/x11.zig | 14 ++++++-------- 3 files changed, 29 insertions(+), 26 deletions(-) diff --git a/src/apprt/gtk/App.zig b/src/apprt/gtk/App.zig index 7142db995..e9b377ec7 100644 --- a/src/apprt/gtk/App.zig +++ b/src/apprt/gtk/App.zig @@ -56,6 +56,9 @@ clipboard_confirmation_window: ?*ClipboardConfirmationWindow = null, /// This is set to false when the main loop should exit. running: bool = true, +/// Xkb state (X11 only). Will be null on Wayland. +x11_xkb: ?x11.X11Xkb = null, + pub fn init(core_app: *CoreApp, opts: Options) !App { _ = opts; @@ -170,8 +173,9 @@ pub fn init(core_app: *CoreApp, opts: Options) !App { return error.GtkApplicationRegisterFailed; } + var x11_xkb: ?x11.X11Xkb = null; const display = c.gdk_display_get_default(); - if (x11.x11_is_display(display)) { + if (x11.is_display(display)) { // Set the X11 window class property (WM_CLASS) if are are on an X11 // display. // @@ -197,6 +201,9 @@ pub fn init(core_app: *CoreApp, opts: Options) !App { "ghostty"; c.g_set_prgname(prgname); c.gdk_x11_display_set_program_class(display, app_id); + + // Set up Xkb + x11_xkb = try x11.X11Xkb.init(c.gdk_display_get_default()); } // This just calls the "activate" signal but its part of the normal @@ -210,6 +217,7 @@ pub fn init(core_app: *CoreApp, opts: Options) !App { .config = config, .ctx = ctx, .cursor_none = cursor_none, + .x11_xkb = x11_xkb, // If we are NOT the primary instance, then we never want to run. // This means that another instance of the GTK app is running and // our "activate" call above will open a window. @@ -574,3 +582,9 @@ test "isValidAppId" { try testing.expect(!isValidAppId("")); try testing.expect(!isValidAppId("foo" ** 86)); } + +/// Loads keyboard state from Xkb if there is an event pending and Xkb is +/// loaded (X11 only). Returns null otherwise. +pub fn modifier_state_from_xkb(self: *App, display_: ?*c.GdkDisplay) ?input.Mods { + return (self.x11_xkb orelse return null).modifier_state_from_notify(display_); +} diff --git a/src/apprt/gtk/Surface.zig b/src/apprt/gtk/Surface.zig index 2ab1a0051..a0f2091bb 100644 --- a/src/apprt/gtk/Surface.zig +++ b/src/apprt/gtk/Surface.zig @@ -19,7 +19,6 @@ const Window = @import("Window.zig"); const ClipboardConfirmationWindow = @import("ClipboardConfirmationWindow.zig"); const inspector = @import("inspector.zig"); const gtk_key = @import("key.zig"); -const x11 = @import("x11.zig"); const c = @import("c.zig"); const log = std.log.scoped(.gtk_surface); @@ -254,9 +253,6 @@ im_composing: bool = false, im_buf: [128]u8 = undefined, im_len: u7 = 0, -/// Xkb state (X11 only). Will be null on Wayland. -x11_xkb: ?x11.X11Xkb = null, - pub fn create(alloc: Allocator, app: *App, opts: Options) !*Surface { var surface = try alloc.create(Surface); errdefer alloc.destroy(surface); @@ -359,9 +355,6 @@ pub fn init(self: *Surface, app: *App, opts: Options) !void { }; errdefer self.* = undefined; - // Set up Xkb if we are running under X11. - self.x11_xkb = try x11.X11Xkb.init(c.gdk_display_get_default()); - // Set our default mouse shape try self.setMouseShape(.text); @@ -1343,18 +1336,16 @@ fn keyEvent( const device = c.gdk_event_get_device(event); var mods = init_mods: { - if (self.x11_xkb) |x11_xkb| { - // Add any modifier state events from Xkb if we have them (X11 only). - if (x11_xkb.modifier_state_from_notify(display)) |xkb_mods| { - break :init_mods xkb_mods; - } - - // Null back from the Xkb call means there was no modifier - // event to read. This likely means that the key event did not - // result in a modifier change and we can safely rely on the - // GDK state. + // Add any modifier state events from Xkb if we have them (X11 only). + if (self.app.modifier_state_from_xkb(display)) |xkb_mods| { + break :init_mods xkb_mods; } + // Null back from the Xkb call means there was no modifier + // event to read. This likely means that the key event did not + // result in a modifier change and we can safely rely on the + // GDK state. + break :init_mods translateMods(c.gdk_device_get_modifier_state(device)); }; diff --git a/src/apprt/gtk/x11.zig b/src/apprt/gtk/x11.zig index c9fd270b0..a188df8bb 100644 --- a/src/apprt/gtk/x11.zig +++ b/src/apprt/gtk/x11.zig @@ -6,7 +6,7 @@ const input = @import("../../input.zig"); const log = std.log.scoped(.gtk_x11); /// Returns true if the passed in display is an X11 display. -pub fn x11_is_display(display: ?*c.GdkDisplay) bool { +pub fn is_display(display: ?*c.GdkDisplay) bool { return c.g_type_check_instance_is_a( @ptrCast(@alignCast(display orelse return false)), c.gdk_x11_display_get_type(), @@ -14,9 +14,7 @@ pub fn x11_is_display(display: ?*c.GdkDisplay) bool { } pub const X11Xkb = struct { - opcode: c_int, base_event_code: c_int, - base_error_code: c_int, funcs: Funcs, /// Initialize an X11Xkb struct, for the given GDK display. If the display @@ -27,25 +25,25 @@ pub const X11Xkb = struct { const display = display_ orelse return null; // If the display isn't X11, then we don't need to do anything. - if (!x11_is_display(display)) return null; + if (!is_display(display)) return null; log.debug("X11Xkb.init: initializing Xkb", .{}); const xdisplay = c.gdk_x11_display_get_xdisplay(display); var result: X11Xkb = .{ - .opcode = 0, .base_event_code = 0, - .base_error_code = 0, .funcs = try Funcs.init(), }; log.debug("X11Xkb.init: running XkbQueryExtension", .{}); + var opcode: c_int = 0; + var base_error_code: c_int = 0; var major = c.XkbMajorVersion; var minor = c.XkbMinorVersion; if (result.funcs.XkbQueryExtension( xdisplay, - &result.opcode, + &opcode, &result.base_event_code, - &result.base_error_code, + &base_error_code, &major, &minor, ) == 0) { From 732063375a924190e4a42bba3c1b4d57761adbd7 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 1 Jan 2024 08:34:19 -0800 Subject: [PATCH 5/5] apprt/gtk: stylistic changes --- src/apprt/gtk/App.zig | 19 ++++++++++++------- src/apprt/gtk/Surface.zig | 21 +++++++-------------- src/apprt/gtk/x11.zig | 18 ++++++++---------- 3 files changed, 27 insertions(+), 31 deletions(-) diff --git a/src/apprt/gtk/App.zig b/src/apprt/gtk/App.zig index e9b377ec7..3d30d9ba5 100644 --- a/src/apprt/gtk/App.zig +++ b/src/apprt/gtk/App.zig @@ -57,7 +57,7 @@ clipboard_confirmation_window: ?*ClipboardConfirmationWindow = null, running: bool = true, /// Xkb state (X11 only). Will be null on Wayland. -x11_xkb: ?x11.X11Xkb = null, +x11_xkb: ?x11.Xkb = null, pub fn init(core_app: *CoreApp, opts: Options) !App { _ = opts; @@ -173,9 +173,13 @@ pub fn init(core_app: *CoreApp, opts: Options) !App { return error.GtkApplicationRegisterFailed; } - var x11_xkb: ?x11.X11Xkb = null; - const display = c.gdk_display_get_default(); - if (x11.is_display(display)) { + // Perform all X11 initialization. This ultimately returns the X11 + // keyboard state but the block does more than that (i.e. setting up + // WM_CLASS). + const x11_xkb: ?x11.Xkb = x11_xkb: { + const display = c.gdk_display_get_default(); + if (!x11.is_display(display)) break :x11_xkb null; + // Set the X11 window class property (WM_CLASS) if are are on an X11 // display. // @@ -203,8 +207,8 @@ pub fn init(core_app: *CoreApp, opts: Options) !App { c.gdk_x11_display_set_program_class(display, app_id); // Set up Xkb - x11_xkb = try x11.X11Xkb.init(c.gdk_display_get_default()); - } + break :x11_xkb try x11.Xkb.init(display); + }; // This just calls the "activate" signal but its part of the normal // startup routine so we just call it: @@ -586,5 +590,6 @@ test "isValidAppId" { /// Loads keyboard state from Xkb if there is an event pending and Xkb is /// loaded (X11 only). Returns null otherwise. pub fn modifier_state_from_xkb(self: *App, display_: ?*c.GdkDisplay) ?input.Mods { - return (self.x11_xkb orelse return null).modifier_state_from_notify(display_); + const x11_xkb = self.x11_xkb orelse return null; + return x11_xkb.modifier_state_from_notify(display_); } diff --git a/src/apprt/gtk/Surface.zig b/src/apprt/gtk/Surface.zig index a0f2091bb..37c5155f0 100644 --- a/src/apprt/gtk/Surface.zig +++ b/src/apprt/gtk/Surface.zig @@ -1333,22 +1333,15 @@ fn keyEvent( // was presssed (i.e. left control) const mods = mods: { _ = gtk_mods; - const device = c.gdk_event_get_device(event); - var mods = init_mods: { - // Add any modifier state events from Xkb if we have them (X11 only). - if (self.app.modifier_state_from_xkb(display)) |xkb_mods| { - break :init_mods xkb_mods; - } - - // Null back from the Xkb call means there was no modifier - // event to read. This likely means that the key event did not - // result in a modifier change and we can safely rely on the - // GDK state. - - break :init_mods translateMods(c.gdk_device_get_modifier_state(device)); - }; + // Add any modifier state events from Xkb if we have them (X11 only). + // Null back from the Xkb call means there was no modifier + // event to read. This likely means that the key event did not + // result in a modifier change and we can safely rely on the + // GDK state. + var mods = self.app.modifier_state_from_xkb(display) orelse + translateMods(c.gdk_device_get_modifier_state(device)); mods.num_lock = c.gdk_device_get_num_lock_state(device) == 1; switch (physical_key) { diff --git a/src/apprt/gtk/x11.zig b/src/apprt/gtk/x11.zig index a188df8bb..8965a72e0 100644 --- a/src/apprt/gtk/x11.zig +++ b/src/apprt/gtk/x11.zig @@ -13,13 +13,13 @@ pub fn is_display(display: ?*c.GdkDisplay) bool { ) != 0; } -pub const X11Xkb = struct { +pub const Xkb = struct { base_event_code: c_int, funcs: Funcs, - /// Initialize an X11Xkb struct, for the given GDK display. If the display + /// Initialize an Xkb struct, for the given GDK display. If the display /// isn't backed by X then this will return null. - pub fn init(display_: ?*c.GdkDisplay) !?X11Xkb { + pub fn init(display_: ?*c.GdkDisplay) !?Xkb { // Display should never be null but we just treat that as a non-X11 // display so that the caller can just ignore it and not unwrap it. const display = display_ orelse return null; @@ -27,14 +27,14 @@ pub const X11Xkb = struct { // If the display isn't X11, then we don't need to do anything. if (!is_display(display)) return null; - log.debug("X11Xkb.init: initializing Xkb", .{}); + log.debug("Xkb.init: initializing Xkb", .{}); const xdisplay = c.gdk_x11_display_get_xdisplay(display); - var result: X11Xkb = .{ + var result: Xkb = .{ .base_event_code = 0, .funcs = try Funcs.init(), }; - log.debug("X11Xkb.init: running XkbQueryExtension", .{}); + log.debug("Xkb.init: running XkbQueryExtension", .{}); var opcode: c_int = 0; var base_error_code: c_int = 0; var major = c.XkbMajorVersion; @@ -51,7 +51,7 @@ pub const X11Xkb = struct { return error.XkbInitializationError; } - log.debug("X11Xkb.init: running XkbSelectEventDetails", .{}); + log.debug("Xkb.init: running XkbSelectEventDetails", .{}); if (result.funcs.XkbSelectEventDetails( xdisplay, c.XkbUseCoreKbd, @@ -76,7 +76,7 @@ pub const X11Xkb = struct { /// Returns null if there is no event. In this case, the caller should fall /// back to the standard GDK modifier state (this likely means the key /// event did not result in a modifier change). - pub fn modifier_state_from_notify(self: X11Xkb, display_: ?*c.GdkDisplay) ?input.Mods { + pub fn modifier_state_from_notify(self: Xkb, display_: ?*c.GdkDisplay) ?input.Mods { const display = display_ orelse return null; // Shoutout to Mozilla for figuring out a clean way to do this, this is @@ -114,8 +114,6 @@ const Funcs = struct { XEventsQueued: XEventsQueuedType, XPeekEvent: XPeekEventType, - // X11 Function types. We load these dynamically at runtime to avoid having to - // link against X11. const XkbQueryExtensionType = *const fn (?*c.struct__XDisplay, [*c]c_int, [*c]c_int, [*c]c_int, [*c]c_int, [*c]c_int) callconv(.C) c_int; const XkbSelectEventDetailsType = *const fn (?*c.struct__XDisplay, c_uint, c_uint, c_ulong, c_ulong) callconv(.C) c_int; const XEventsQueuedType = *const fn (?*c.struct__XDisplay, c_int) callconv(.C) c_int;