From 08f2e91ff53d93484ab3133470fbe60aec60fbe2 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 8 Dec 2023 20:41:09 -0800 Subject: [PATCH] apprt/gtk: use map_keycode to get the proper unshifted key for a layout Fixes #1014 There were a couple overlapping issues here: 1. To determine the "unshifted" key, we were using `keyval_to_lower`. This only works if the key is uppercase and not all layouts are uppercase for the unshifted value. 2. If we couldn't case the unicode value of a key or unshifted key to ASCII, we'd say the key was the same as the physical key. This is incorrect because the physical key is always the layout of the physical keyboard so for example with the Turkish layout on a US keyboard, you'd get US letters when a key may only correspond to non-US letters. To fix the first issue, we are using map_keycode to map the hardware keycode to all possible keyvals from it. We then use the currently active layout to find the unshifted value. To fix the scond issue, we no longer fallback to the physical key if there was IM text or non-ASCII unicode values for the key. I tested Turkish with #1014 and I tested Dvorak to make sure those basics still work. --- src/apprt/gtk/Surface.zig | 57 ++++++++++++++++++++++++++++++++++----- 1 file changed, 50 insertions(+), 7 deletions(-) diff --git a/src/apprt/gtk/Surface.zig b/src/apprt/gtk/Surface.zig index 5560b0c13..4a66320f5 100644 --- a/src/apprt/gtk/Surface.zig +++ b/src/apprt/gtk/Surface.zig @@ -1210,13 +1210,48 @@ fn keyEvent( // Get the unshifted unicode value of the keyval. This is used // by the Kitty keyboard protocol. const keyval_unicode_unshifted: u21 = unshifted: { - // Note: this can't possibly always be right, specifically in the - // case of multi-level/group keyboards. But, this works for Dvorak, - // Norwegian, and French layouts and thats what we have real users for - // right now. - const lower = c.gdk_keyval_to_lower(keyval); - const lower_unicode = c.gdk_keyval_to_unicode(lower); - break :unshifted std.math.cast(u21, lower_unicode) orelse 0; + // We need to get the currently active keyboard layout so we know + // what group to look at. + const layout = c.gdk_key_event_get_layout(@ptrCast(event)); + + // 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; + if (c.gdk_display_map_keycode( + display, + keycode, + @ptrCast(&keys), + @ptrCast(&keyvals), + &n_keys, + ) == 0) break :unshifted 0; + + defer c.g_free(keys); + defer c.g_free(keyvals); + + // debugging: + // log.debug("layout={}", .{layout}); + // for (0..@intCast(n_keys)) |i| { + // log.debug("keymap key={} codepoint={x}", .{ + // keys[i], + // c.gdk_keyval_to_unicode(keyvals[i]), + // }); + // } + + for (0..@intCast(n_keys)) |i| { + if (keys[i].group == layout and + keys[i].level == 0) + { + break :unshifted std.math.cast( + u21, + c.gdk_keyval_to_unicode(keyvals[i]), + ) orelse 0; + } + } + + break :unshifted 0; }; // We always reset our committed text when ending a keypress so that @@ -1369,6 +1404,14 @@ fn keyEvent( break :key key; } + // If we have im text then this is invalid. This means that + // the keypress generated some character that we don't know about + // in our key enum. We don't want to use the physical key because + // it can be simply wrong. For example on "Turkish Q" the "i" key + // on a US layout results in "ı" which is not the same as "i" so + // we shouldn't use the physical key. + if (self.im_len > 0 or keyval_unicode_unshifted != 0) break :key .invalid; + break :key physical_key; } else .invalid;