Skip to content

Commit

Permalink
apprt/gtk: fundamentally rework input method handling (ghostty-org#5280)
Browse files Browse the repository at this point in the history 
Fixes ghostty-org#4332

This commit fundamentally reworks the input method handling in the GTK
apprt, making it work properly (as reported in the linked issue) on both
Wayland and X11. This was tested with both a Gnome desktop on Wayland
and i3 on X11 with fcitx and mozc.

The main changes are:

- Both key press and release events must be forwarded to the input
method.

- Input method callbacks such as preedit and commit must be expected
outside of keypress events to handle on-screen keyboards and
non-keyboard input devices.

- Input methods should always commit when told to. Previously, we would
only commit when a keypress event was given. This is incorrect. For
example, it didn't work with input method changes outside the app which
should result in committed text (as can be seen with "official" Gnome
apps like Notes or Console).

The key input handling also now generally does less so I think input
latency should be positively affected by this change. I didn't measure.
  • Loading branch information
@mitchellh
mitchellh authored Jan 21, 2025
2 parents 5cb2fa6 + 52936b9 commit 6265adf
Showing 1 changed file with 132 additions and 121 deletions.
253 changes: 132 additions & 121 deletions src/apprt/gtk/Surface.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -368,10 +368,9 @@ cursor_pos: apprt.CursorPos,
inspector: ?*inspector.Inspector = null,

/// Key input states. See gtkKeyPressed for detailed descriptions.
in_keypress: bool = false,
in_keyevent: bool = false,
im_context: *c.GtkIMContext,
im_composing: bool = false,
im_commit_buffered: bool = false,
im_buf: [128]u8 = undefined,
im_len: u7 = 0,

Expand Down Expand Up @@ -1604,30 +1603,36 @@ fn gtkKeyReleased(
)) 1 else 0;
}

/// Key press event. This is where we do ALL of our key handling,
/// translation to keyboard layouts, dead key handling, etc. Key handling
/// is complicated so this comment will explain what's going on.
/// Key press event (press or release).
///
/// At a high level, we want to construct an `input.KeyEvent` and
/// pass that to `keyCallback`. At a low level, this is more complicated
/// than it appears because we need to construct all of this information
/// and its not given to us.
///
/// For press events, we run the keypress through the input method context
/// in order to determine if we're in a dead key state, completed unicode
/// char, etc. This all happens through various callbacks: preedit, commit,
/// etc. These inspect "in_keypress" if they have to and set some instance
/// state.
/// For all events, we run the GdkEvent through the input method context.
/// This allows the input method to capture the event and trigger
/// callbacks such as preedit, commit, etc.
///
/// We then take all of the information in order to determine if we have
/// There are a couple important aspects to the prior paragraph: we must
/// send ALL events through the input method context. This is because
/// input methods use both key press and key release events to determine
/// the state of the input method. For example, fcitx uses key release
/// events on modifiers (i.e. ctrl+shift) to switch the input method.
///
/// We set some state to note we're in a key event (self.in_keyevent)
/// because some of the input method callbacks change behavior based on
/// this state. For example, we don't want to send character events
/// like "a" via the input "commit" event if we're actively processing
/// a keypress because we'd lose access to the keycode information.
/// However, a "commit" event may still happen outside of a keypress
/// event from e.g. a tablet or on-screen keyboard.
///
/// Finally, we take all of the information in order to determine if we have
/// a unicode character or if we have to map the keyval to a code to
/// get the underlying logical key, etc.
///
/// Finally, we can emit the keyCallback.
///
/// Note we ALSO have an IMContext attached directly to the widget
/// which can emit preedit and commit callbacks. But, if we're not
/// in a keypress, we let those automatically work.
/// Then we can emit the keyCallback.
pub fn keyEvent(
self: *Surface,
action: input.Action,
Expand All @@ -1636,26 +1641,15 @@ pub fn keyEvent(
keycode: c.guint,
gtk_mods: c.GdkModifierType,
) bool {
// log.warn("GTKIM: keyEvent action={}", .{action});
const event = c.gtk_event_controller_get_current_event(
@ptrCast(ec_key),
) orelse return false;

const keyval_unicode = c.gdk_keyval_to_unicode(keyval);

// Get the unshifted unicode value of the keyval. This is used
// by the Kitty keyboard protocol.
const keyval_unicode_unshifted: u21 = gtk_key.keyvalUnicodeUnshifted(
@ptrCast(self.gl_area),
event,
keycode,
);

// We always reset our committed text when ending a keypress so that
// future keypresses don't think we have a commit event.
defer self.im_len = 0;

// We only want to send the event through the IM context if we're a press
if (action == .press or action == .repeat) {
// The block below is all related to input method handling. See the function
// comment for some high level details and then the comments within
// the block for more specifics.
{
// This can trigger an input method so we need to notify the im context
// where the cursor is so it can render the dropdowns in the correct
// place.
Expand All @@ -1667,41 +1661,77 @@ pub fn keyEvent(
.height = 1,
});

// We mark that we're in a keypress event. We use this in our
// IM commit callback to determine if we need to send a char callback
// to the core surface or not.
self.in_keypress = true;
defer self.in_keypress = false;

// Pass the event through the IM controller to handle dead key states.
// Filter is true if the event was handled by the IM controller.
const im_handled = c.gtk_im_context_filter_keypress(self.im_context, event) != 0;
// log.warn("im_handled={} im_len={} im_composing={}", .{ im_handled, self.im_len, self.im_composing });

// If this is a dead key, then we're composing a character and
// we need to set our proper preedit state.
if (self.im_composing) preedit: {
const text = self.im_buf[0..self.im_len];
self.core_surface.preeditCallback(text) catch |err| {
log.err("error in preedit callback err={}", .{err});
break :preedit;
};

// If we're composing then we don't want to send the key
// event to the core surface so we always return immediately.
// Pass the event through the IM controller. This will return true
// if the input method handled the event.
//
// Confusingly, not all events handled by the input method result
// in this returning true so we have to maintain some local state to
// find those and in one case we simply lose information.
//
// - If we change the input method via keypress while we have preedit
// text, the input method will commit the pending text but will not
// mark it as handled. We use the `was_composing` variable to detect
// this case.
//
// - If we switch input methods (i.e. via ctrl+shift with fcitx),
// the input method will handle the key release event but will not
// mark it as handled. I don't know any way to detect this case so
// it will result in a key event being sent to the key callback.
// For Kitty text encoding, this will result in modifiers being
// triggered despite being technically consumed. At the time of
// writing, both Kitty and Alacritty have the same behavior. I
// know of no way to fix this.
const was_composing = self.im_composing;
const im_handled = filter: {
// We note that we're in a keypress because we want some logic to
// depend on this. For example, we don't want to send character events
// like "a" via the input "commit" event if we're actively processing
// a keypress because we'd lose access to the keycode information.
self.in_keyevent = true;
defer self.in_keyevent = false;
break :filter c.gtk_im_context_filter_keypress(
self.im_context,
event,
) != 0;
};
// log.warn("GTKIM: im_handled={} im_len={} im_composing={}", .{
// im_handled,
// self.im_len,
// self.im_composing,
// });

if (self.im_composing) {
// If we're composing and the input method handled this event then
// we don't continue processing it. Any preedit changes or any of that
// would've been handled by the preedit events.
if (im_handled) return true;
} else {
// If we aren't composing, then we set our preedit to
// empty no matter what.
self.core_surface.preeditCallback(null) catch {};

// If the IM handled this and we have no text, then we just
// return because this probably just changed the input method
// or something.
if (im_handled and self.im_len == 0) return true;
} else if (was_composing) {
// If we were composing and now we're not it means that we committed
// the text. We also don't want to encode a key event for this.
return true;
}

// At this point, for the sake of explanation of internal state:
// it is possible that im_len > 0 and im_composing == false. This
// means that we received a commit event from the input method that
// we want associated with the key event. This is common: its how
// basic character translation for simple inputs like "a" work.
}

// We always reset the length of the im buffer. There's only one scenario
// we reach this point with im_len > 0 and that's if we received a commit
// event from the input method. We don't want to keep that state around
// since we've handled it here.
defer self.im_len = 0;

// Get the keyvals for this event.
const keyval_unicode = c.gdk_keyval_to_unicode(keyval);
const keyval_unicode_unshifted: u21 = gtk_key.keyvalUnicodeUnshifted(
@ptrCast(self.gl_area),
event,
keycode,
);

// We want to get the physical unmapped key to process physical keybinds.
// (These are keybinds explicitly marked as requesting physical mapping).
const physical_key = keycode: for (input.keycodes.entries) |entry| {
Expand Down Expand Up @@ -1834,12 +1864,11 @@ fn gtkInputPreeditStart(
_: *c.GtkIMContext,
ud: ?*anyopaque,
) callconv(.C) void {
//log.debug("preedit start", .{});
// log.warn("GTKIM: preedit start", .{});
const self = userdataSelf(ud.?);
if (!self.in_keypress) return;

// Mark that we are now composing a string with a dead key state.
// We'll record the string in the preedit-changed callback.
// Start our composing state for the input method and reset our
// input buffer to empty.
self.im_composing = true;
self.im_len = 0;
}
Expand All @@ -1848,93 +1877,75 @@ fn gtkInputPreeditChanged(
ctx: *c.GtkIMContext,
ud: ?*anyopaque,
) callconv(.C) void {
// log.warn("GTKIM: preedit change", .{});
const self = userdataSelf(ud.?);

// If there's buffered character, send the characters directly to the surface.
if (self.im_composing and self.im_commit_buffered) {
defer self.im_commit_buffered = false;
defer self.im_len = 0;
_ = self.core_surface.keyCallback(.{
.action = .press,
.key = .invalid,
.physical_key = .invalid,
.mods = .{},
.consumed_mods = .{},
.composing = false,
.utf8 = self.im_buf[0..self.im_len],
}) catch |err| {
log.err("error in key callback err={}", .{err});
return;
};
}

if (!self.in_keypress) return;

// Get our pre-edit string that we'll use to show the user.
var buf: [*c]u8 = undefined;
_ = c.gtk_im_context_get_preedit_string(ctx, &buf, null, null);
defer c.g_free(buf);
const str = std.mem.sliceTo(buf, 0);

// If our string becomes empty we ignore this. This can happen after
// a commit event when the preedit is being cleared and we don't want
// to set im_len to zero. This is safe because preeditstart always sets
// im_len to zero.
if (str.len == 0) return;

// Copy the preedit string into the im_buf. This is safe because
// commit will always overwrite this.
self.im_len = @intCast(@min(self.im_buf.len, str.len));
@memcpy(self.im_buf[0..self.im_len], str);
// Update our preedit state in Ghostty core
self.core_surface.preeditCallback(str) catch |err| {
log.err("error in preedit callback err={}", .{err});
};
}

fn gtkInputPreeditEnd(
_: *c.GtkIMContext,
ud: ?*anyopaque,
) callconv(.C) void {
//log.debug("preedit end", .{});
// log.warn("GTKIM: preedit end", .{});
const self = userdataSelf(ud.?);
if (!self.in_keypress) return;

// End our composing state for GTK, allowing us to commit the text.
self.im_composing = false;

// End our preedit state in Ghostty core
self.core_surface.preeditCallback(null) catch |err| {
log.err("error in preedit callback err={}", .{err});
};
}

fn gtkInputCommit(
_: *c.GtkIMContext,
bytes: [*:0]u8,
ud: ?*anyopaque,
) callconv(.C) void {
// log.warn("GTKIM: input commit", .{});
const self = userdataSelf(ud.?);
const str = std.mem.sliceTo(bytes, 0);

// If we're in a key event, then we want to buffer the commit so
// that we can send the proper keycallback followed by the char
// callback.
if (self.in_keypress) {
if (str.len <= self.im_buf.len) {
@memcpy(self.im_buf[0..str.len], str);
self.im_len = @intCast(str.len);

// If composing is done and character should be committed,
// It should be committed in preedit callback.
if (self.im_composing) {
self.im_commit_buffered = true;
}

// log.debug("input commit len={}", .{self.im_len});
} else {
// If we're in a keyEvent (i.e. a keyboard event) and we're not composing,
// then this is just a normal key press resulting in UTF-8 text. We
// want the keyEvent to handle this so that the UTF-8 text can be associated
// with a keyboard event.
if (!self.im_composing and self.in_keyevent) {
if (str.len > self.im_buf.len) {
log.warn("not enough buffer space for input method commit", .{});
return;
}

// Copy our committed text to the buffer
@memcpy(self.im_buf[0..str.len], str);
self.im_len = @intCast(str.len);

// log.debug("input commit len={}", .{self.im_len});
return;
}

// This prevents staying in composing state after commit even though
// input method has changed.
// If we reach this point from above it means we're composing OR
// not in a keypress. In either case, we want to commit the text
// given to us because that's what GTK is asking us to do. If we're
// not in a keypress it means that this commit came via a non-keyboard
// event (i.e. on-screen keyboard, tablet of some kind, etc.).

// Committing ends composing state
self.im_composing = false;

// We're not in a keypress, so this was sent from an on-screen emoji
// keyboard or something like that. Send the characters directly to
// the surface.
// Send the text to the core surface, associated with no key (an
// invalid key, which should produce no PTY encoding).
_ = self.core_surface.keyCallback(.{
.action = .press,
.key = .invalid,
Expand All @@ -1944,7 +1955,7 @@ fn gtkInputCommit(
.composing = false,
.utf8 = str,
}) catch |err| {
log.err("error in key callback err={}", .{err});
log.warn("error in key callback err={}", .{err});
return;
};
}
Expand Down

0 comments on commit 6265adf

Please sign in to comment.