From 919128d8c2a8c63e4447f29d9c3f9fb287978ef4 Mon Sep 17 00:00:00 2001 From: Carlos Garnacho Date: Thu, 18 Aug 2022 00:55:30 +0200 Subject: [PATCH 1/3] gtktext: Shuffle the places doing IM reset During text widget manipulation (inserting or deleting text via keyboard) the IM context is reset somewhat early, before the actual change took place. This makes IM lag behind in terms of surrounding text and cursor position. Shuffle these IM reset calls so that they happen after the changes, and ensure that the IM is actually reset, since that is currently toggled on a pretty narrow set of circumstances. (cherry-picked from commit 9e29739e6676bdb313aa19ce54c9da6c39cd4778) --- gtk/gtktext.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/gtk/gtktext.c b/gtk/gtktext.c index b8e15aa2d2..124e2feac2 100644 --- a/gtk/gtktext.c +++ b/gtk/gtktext.c @@ -1946,7 +1946,7 @@ gtk_text_init (GtkText *self) g_signal_connect (priv->key_controller, "key-pressed", G_CALLBACK (gtk_text_key_controller_key_pressed), self); g_signal_connect_swapped (priv->key_controller, "im-update", - G_CALLBACK (gtk_text_schedule_im_reset), self); + G_CALLBACK (gtk_im_context_reset), priv->im_context); gtk_event_controller_key_set_im_context (GTK_EVENT_CONTROLLER_KEY (priv->key_controller), priv->im_context); gtk_widget_add_controller (GTK_WIDGET (self), priv->key_controller); @@ -3808,8 +3808,6 @@ gtk_text_move_cursor (GtkText *self, GtkTextPrivate *priv = gtk_text_get_instance_private (self); int new_pos = priv->current_pos; - gtk_text_reset_im_context (self); - if (priv->current_pos != priv->selection_bound && !extend_selection) { /* If we have a current selection and aren't extending it, move to the @@ -3936,6 +3934,9 @@ gtk_text_move_cursor (GtkText *self, gtk_text_set_selection_bounds (self, new_pos, new_pos); gtk_text_pend_cursor_blink (self); + + priv->need_im_reset = TRUE; + gtk_text_reset_im_context (self); } static void @@ -3963,8 +3964,6 @@ gtk_text_delete_from_cursor (GtkText *self, int end_pos = priv->current_pos; int old_n_bytes = gtk_entry_buffer_get_bytes (get_buffer (self)); - gtk_text_reset_im_context (self); - if (!priv->editable) { gtk_widget_error_bell (GTK_WIDGET (self)); @@ -3974,6 +3973,8 @@ gtk_text_delete_from_cursor (GtkText *self, if (priv->selection_bound != priv->current_pos) { gtk_text_delete_selection (self); + gtk_text_schedule_im_reset (self); + gtk_text_reset_im_context (self); return; } @@ -4038,6 +4039,11 @@ gtk_text_delete_from_cursor (GtkText *self, if (gtk_entry_buffer_get_bytes (get_buffer (self)) == old_n_bytes) gtk_widget_error_bell (GTK_WIDGET (self)); + else + { + gtk_text_schedule_im_reset (self); + gtk_text_reset_im_context (self); + } gtk_text_pend_cursor_blink (self); } @@ -4048,8 +4054,6 @@ gtk_text_backspace (GtkText *self) GtkTextPrivate *priv = gtk_text_get_instance_private (self); int prev_pos; - gtk_text_reset_im_context (self); - if (!priv->editable) { gtk_widget_error_bell (GTK_WIDGET (self)); @@ -4059,6 +4063,8 @@ gtk_text_backspace (GtkText *self) if (priv->selection_bound != priv->current_pos) { gtk_text_delete_selection (self); + gtk_text_schedule_im_reset (self); + gtk_text_reset_im_context (self); return; } @@ -4103,6 +4109,9 @@ gtk_text_backspace (GtkText *self) { gtk_editable_delete_text (GTK_EDITABLE (self), prev_pos, priv->current_pos); } + + gtk_text_schedule_im_reset (self); + gtk_text_reset_im_context (self); } else { From 34693b0d9fddfa4df082dbee72d1cf890c86d295 Mon Sep 17 00:00:00 2001 From: Carlos Garnacho Date: Thu, 18 Aug 2022 00:58:14 +0200 Subject: [PATCH 2/3] gtktextview: Shuffle the places doing IM reset During text widget manipulation (inserting or deleting text via keyboard) the IM context is reset somewhat early, before the actual change took place. This makes IM lag behind in terms of surrounding text and cursor position. Shuffle these IM reset calls so that they happen after the changes, and ensure that the IM is actually reset, since that is currently toggled on a pretty narrow set of circumstances. Also, fix a bug during GtkEventControllerKey::im-update where the condition on cursor position editability to reset the IM context was inverted. (cherry-picked from commit 52ac71b9727b9a63773865309a970c4d97c0a75d) --- gtk/gtktextview.c | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/gtk/gtktextview.c b/gtk/gtktextview.c index 5f8bb14ffb..f7b1687581 100644 --- a/gtk/gtktextview.c +++ b/gtk/gtktextview.c @@ -5525,7 +5525,7 @@ gtk_text_view_key_controller_im_update (GtkEventControllerKey *controller, can_insert = gtk_text_iter_can_insert (&iter, priv->editable); priv->need_im_reset = TRUE; - if (!can_insert) + if (can_insert) gtk_text_view_reset_im_context (text_view); } @@ -6358,8 +6358,6 @@ gtk_text_view_move_cursor (GtkTextView *text_view, return; } - gtk_text_view_reset_im_context (text_view); - if (step == GTK_MOVEMENT_PAGES) { if (!gtk_text_view_scroll_pages (text_view, count, extend_selection)) @@ -6543,6 +6541,9 @@ gtk_text_view_move_cursor (GtkTextView *text_view, gtk_text_view_check_cursor_blink (text_view); gtk_text_view_pend_cursor_blink (text_view); + + priv->need_im_reset = TRUE; + gtk_text_view_reset_im_context (text_view); } static void @@ -6833,14 +6834,16 @@ gtk_text_view_delete_from_cursor (GtkTextView *text_view, priv = text_view->priv; - gtk_text_view_reset_im_context (text_view); - if (type == GTK_DELETE_CHARS) { /* Char delete deletes the selection, if one exists */ if (gtk_text_buffer_delete_selection (get_buffer (text_view), TRUE, priv->editable)) - return; + { + priv->need_im_reset = TRUE; + gtk_text_view_reset_im_context (text_view); + return; + } } gtk_text_buffer_get_iter_at_mark (get_buffer (text_view), &insert, @@ -6967,6 +6970,9 @@ gtk_text_view_delete_from_cursor (GtkTextView *text_view, { gtk_widget_error_bell (GTK_WIDGET (text_view)); } + + priv->need_im_reset = TRUE; + gtk_text_view_reset_im_context (text_view); } static void @@ -6977,12 +6983,14 @@ gtk_text_view_backspace (GtkTextView *text_view) priv = text_view->priv; - gtk_text_view_reset_im_context (text_view); - /* Backspace deletes the selection, if one exists */ if (gtk_text_buffer_delete_selection (get_buffer (text_view), TRUE, priv->editable)) - return; + { + priv->need_im_reset = TRUE; + gtk_text_view_reset_im_context (text_view); + return; + } gtk_text_buffer_get_iter_at_mark (get_buffer (text_view), &insert, @@ -6995,6 +7003,9 @@ gtk_text_view_backspace (GtkTextView *text_view) DV(g_print (G_STRLOC": scrolling onscreen\n")); gtk_text_view_scroll_mark_onscreen (text_view, gtk_text_buffer_get_insert (get_buffer (text_view))); + + priv->need_im_reset = TRUE; + gtk_text_view_reset_im_context (text_view); } else { From 519258c3827d524706a26b9df98c739ce5dc9ec7 Mon Sep 17 00:00:00 2001 From: Carlos Garnacho Date: Thu, 18 Aug 2022 00:58:38 +0200 Subject: [PATCH 3/3] gtkimcontextwayland: Refactor handling of client updates Currently, the wayland IM context sends zwp_text_input_v3.commit from a number of places, and some of them with partial data. In order to make client state updates "atomic" and complete, make the communication happen over an unified notify_im_change() function that happens on a narrower set of circumstances: - The GtkIMContext is reset - The GtkIMContext is just focused - The gesture to invoke the OSK is triggered - The IM context is reacting to changes coming from the compositor Notably, setting the cursor location or the surrounding text do not try to commit state on their own, and now will be flushed with the corresponding IM update or reset. But also, these requests won't be prevented from happening individually on serial mismatch, instead it will be the whole state commit which is held off. With these changes in place, all client-side updates are notified atomically to the compositor under a single .commit request. (cherry-picked from commit f66ffde68d7bd22b77b588671776020296392a6b) --- gtk/gtkimcontextwayland.c | 58 +++++++++++++++++++++++---------------- 1 file changed, 34 insertions(+), 24 deletions(-) diff --git a/gtk/gtkimcontextwayland.c b/gtk/gtkimcontextwayland.c index 069070509a..e4469b05aa 100644 --- a/gtk/gtkimcontextwayland.c +++ b/gtk/gtkimcontextwayland.c @@ -98,6 +98,11 @@ struct _GtkIMContextWayland static void gtk_im_context_wayland_focus_out (GtkIMContext *context); +static void commit_state (GtkIMContextWayland *context); +static void notify_surrounding_text (GtkIMContextWayland *context); +static void notify_cursor_location (GtkIMContextWayland *context); +static void notify_content_type (GtkIMContextWayland *context); + G_DEFINE_TYPE_WITH_CODE (GtkIMContextWayland, gtk_im_context_wayland, GTK_TYPE_IM_CONTEXT_SIMPLE, gtk_im_module_ensure_extension_point (); g_io_extension_point_implement (GTK_IM_MODULE_EXTENSION_POINT_NAME, @@ -128,7 +133,8 @@ gtk_im_context_wayland_get_global (GtkIMContextWayland *self) } static void -notify_external_change (GtkIMContextWayland *context) +notify_im_change (GtkIMContextWayland *context, + enum zwp_text_input_v3_change_cause cause) { GtkIMContextWaylandGlobal *global; gboolean result; @@ -137,9 +143,13 @@ notify_external_change (GtkIMContextWayland *context) if (global == NULL) return; - context->surrounding_change = ZWP_TEXT_INPUT_V3_CHANGE_CAUSE_OTHER; + context->surrounding_change = cause; g_signal_emit_by_name (global->current, "retrieve-surrounding", &result); + notify_surrounding_text (context); + notify_content_type (context); + notify_cursor_location (context); + commit_state (context); } static void @@ -265,17 +275,25 @@ text_input_done (void *data, uint32_t serial) { GtkIMContextWaylandGlobal *global = data; - gboolean result; + GtkIMContextWayland *context; + gboolean update_im; global->done_serial = serial; if (!global->current) return; + context = GTK_IM_CONTEXT_WAYLAND (global->current); + update_im = (context->pending_commit != NULL || + g_strcmp0 (context->pending_preedit.text, + context->current_preedit.text) != 0); + text_input_delete_surrounding_text_apply (global); text_input_commit_apply (global); - g_signal_emit_by_name (global->current, "retrieve-surrounding", &result); text_input_preedit_apply (global); + + if (update_im && global->serial == serial) + notify_im_change (context, ZWP_TEXT_INPUT_V3_CHANGE_CAUSE_INPUT_METHOD); } static void @@ -292,8 +310,6 @@ notify_surrounding_text (GtkIMContextWayland *context) global = gtk_im_context_wayland_get_global (context); if (global == NULL) return; - if (global->done_serial != global->serial) - return; len = strlen (context->surrounding.text); cursor = context->surrounding.cursor_idx; @@ -368,8 +384,6 @@ notify_cursor_location (GtkIMContextWayland *context) global = gtk_im_context_wayland_get_global (context); if (global == NULL) return; - if (global->done_serial != global->serial) - return; rect = context->cursor_rect; gtk_widget_translate_coordinates (context->widget, @@ -460,8 +474,6 @@ notify_content_type (GtkIMContextWayland *context) global = gtk_im_context_wayland_get_global (context); if (global == NULL) return; - if (global->done_serial != global->serial) - return; g_object_get (context, "input-hints", &hints, @@ -527,7 +539,6 @@ released_cb (GtkGestureClick *gesture, { GtkIMContextWaylandGlobal *global; GtkInputHints hints; - gboolean result; global = gtk_im_context_wayland_get_global (context); if (global == NULL) @@ -544,8 +555,8 @@ released_cb (GtkGestureClick *gesture, x, y)) { zwp_text_input_v3_enable (global->text_input); - g_signal_emit_by_name (global->current, "retrieve-surrounding", &result); - commit_state (context); + notify_im_change (GTK_IM_CONTEXT_WAYLAND (context), + ZWP_TEXT_INPUT_V3_CHANGE_CAUSE_OTHER); } } @@ -677,12 +688,9 @@ static void enable (GtkIMContextWayland *context_wayland, GtkIMContextWaylandGlobal *global) { - gboolean result; zwp_text_input_v3_enable (global->text_input); - g_signal_emit_by_name (global->current, "retrieve-surrounding", &result); - notify_content_type (context_wayland); - notify_cursor_location (context_wayland); - commit_state (context_wayland); + notify_im_change (context_wayland, + ZWP_TEXT_INPUT_V3_CHANGE_CAUSE_OTHER); } static void @@ -692,6 +700,12 @@ disable (GtkIMContextWayland *context_wayland, zwp_text_input_v3_disable (global->text_input); commit_state (context_wayland); + /* The commit above will still count in the .done event accounting, + * we should account for it, lest the serial gets out of sync after + * a future focus_in/enable. + */ + global->done_serial++; + /* after disable, incoming state changes won't take effect anyway */ if (context_wayland->current_preedit.text) { @@ -854,7 +868,8 @@ gtk_im_context_wayland_focus_out (GtkIMContext *context) static void gtk_im_context_wayland_reset (GtkIMContext *context) { - notify_external_change (GTK_IM_CONTEXT_WAYLAND (context)); + notify_im_change (GTK_IM_CONTEXT_WAYLAND (context), + ZWP_TEXT_INPUT_V3_CHANGE_CAUSE_OTHER); GTK_IM_CONTEXT_CLASS (gtk_im_context_wayland_parent_class)->reset (context); } @@ -889,8 +904,6 @@ gtk_im_context_wayland_set_cursor_location (GtkIMContext *context, gtk_event_controller_reset (GTK_EVENT_CONTROLLER (context_wayland->gesture)); context_wayland->cursor_rect = *rect; - notify_cursor_location (context_wayland); - commit_state (context_wayland); } static void @@ -924,9 +937,6 @@ gtk_im_context_wayland_set_surrounding (GtkIMContext *context, context_wayland->surrounding.text = g_strndup (text, len); context_wayland->surrounding.cursor_idx = cursor_index; context_wayland->surrounding.anchor_idx = selection_bound; - - notify_surrounding_text (context_wayland); - commit_state (context_wayland); } static gboolean