From 1d69b1b505682f5df4bc276321af06d1404377e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCllner?= Date: Sat, 8 Feb 2025 01:00:02 +0100 Subject: [PATCH 1/4] clutter: Prepare for `get_key_focus()` returning NULL While the existing `get_key_focus()` methods looks like a getter of the `key-focus` property and is detected as such by gobject introspection, it behaves differently in that it returns the stage if no explicit focus has been set. This is about to change, so adjust the couple of cases that rely on the fallback to the stage. Part-of: --- clutter/clutter/clutter-accessibility.c | 2 ++ clutter/clutter/clutter-stage.c | 6 +++++- src/tests/clutter/interactive/test-events.c | 2 ++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/clutter/clutter/clutter-accessibility.c b/clutter/clutter/clutter-accessibility.c index 8b803b2d751..8727d7e1db9 100644 --- a/clutter/clutter/clutter-accessibility.c +++ b/clutter/clutter/clutter-accessibility.c @@ -230,6 +230,8 @@ check_key_visibility (ClutterStage *stage) ClutterActor *focus; focus = clutter_stage_get_key_focus (stage); + if (focus == NULL) + focus = CLUTTER_ACTOR (stage); accessible = clutter_actor_get_accessible (focus); g_return_val_if_fail (accessible != NULL, 0); diff --git a/clutter/clutter/clutter-stage.c b/clutter/clutter/clutter-stage.c index fa6d8a5157c..636beede599 100644 --- a/clutter/clutter/clutter-stage.c +++ b/clutter/clutter/clutter-stage.c @@ -4178,10 +4178,13 @@ clutter_stage_get_event_actor (ClutterStage *stage, { ClutterInputDevice *device; ClutterEventSequence *sequence; + ClutterStagePrivate *priv; g_return_val_if_fail (CLUTTER_IS_STAGE (stage), NULL); g_return_val_if_fail (event != NULL, NULL); + priv = clutter_stage_get_instance_private (stage); + switch (clutter_event_type (event)) { case CLUTTER_KEY_PRESS: @@ -4193,7 +4196,8 @@ clutter_stage_get_event_actor (ClutterStage *stage, case CLUTTER_IM_COMMIT: case CLUTTER_IM_DELETE: case CLUTTER_IM_PREEDIT: - return clutter_stage_get_key_focus (stage); + return priv->key_focused_actor ? + priv->key_focused_actor : CLUTTER_ACTOR (stage); case CLUTTER_MOTION: case CLUTTER_ENTER: case CLUTTER_LEAVE: diff --git a/src/tests/clutter/interactive/test-events.c b/src/tests/clutter/interactive/test-events.c index 752d423c045..c17dd84115e 100644 --- a/src/tests/clutter/interactive/test-events.c +++ b/src/tests/clutter/interactive/test-events.c @@ -206,6 +206,8 @@ input_cb (ClutterActor *actor, event_type == CLUTTER_KEY_RELEASE) { source_actor = clutter_stage_get_key_focus (CLUTTER_STAGE (stage)); + if (source_actor == NULL) + source_actor = stage; } else { -- GitLab From e49edebf92aa26ac5bf60b1dd5c483ef10038191 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCllner?= Date: Sat, 8 Feb 2025 00:00:45 +0100 Subject: [PATCH 2/4] clutter/stage: Turn `get_key_focus()` into a proper getter The method currently returns the stage itself when the property is NULL. This has become particularly problematic as the method is detected as getter by gobject introspection, and gjs now optimizes property accesses by calling the getter method instead. Address this by turning the method into a genuine getter without falling back to the stage. Part-of: --- clutter/clutter/clutter-stage.c | 7 ++----- src/backends/meta-stage.c | 6 ------ src/core/events.c | 2 +- src/x11/meta-x11-display.c | 2 +- 4 files changed, 4 insertions(+), 13 deletions(-) diff --git a/clutter/clutter/clutter-stage.c b/clutter/clutter/clutter-stage.c index 636beede599..a09dab3ba76 100644 --- a/clutter/clutter/clutter-stage.c +++ b/clutter/clutter/clutter-stage.c @@ -2171,7 +2171,7 @@ clutter_stage_set_key_focus (ClutterStage *stage, * * Retrieves the actor that is currently under key focus. * - * Return value: (transfer none): the actor with key focus, or the stage + * Return value: (transfer none) (nullable): the actor with key focus */ ClutterActor * clutter_stage_get_key_focus (ClutterStage *stage) @@ -2181,10 +2181,7 @@ clutter_stage_get_key_focus (ClutterStage *stage) g_return_val_if_fail (CLUTTER_IS_STAGE (stage), NULL); priv = clutter_stage_get_instance_private (stage); - if (priv->key_focused_actor) - return priv->key_focused_actor; - - return CLUTTER_ACTOR (stage); + return priv->key_focused_actor; } /*** Perspective boxed type ******/ diff --git a/src/backends/meta-stage.c b/src/backends/meta-stage.c index f3823e3c7cb..ac436e8f6dd 100644 --- a/src/backends/meta-stage.c +++ b/src/backends/meta-stage.c @@ -307,12 +307,6 @@ key_focus_actor_changed (ClutterStage *stage, { ClutterActor *key_focus = clutter_stage_get_key_focus (stage); - /* If there's no explicit key focus, clutter_stage_get_key_focus() - * returns the stage. - */ - if (key_focus == CLUTTER_ACTOR (stage)) - key_focus = NULL; - clutter_stage_set_active (stage, key_focus != NULL); } diff --git a/src/core/events.c b/src/core/events.c index ba2dca734e0..9bf4a0001f3 100644 --- a/src/core/events.c +++ b/src/core/events.c @@ -77,7 +77,7 @@ stage_has_key_focus (MetaDisplay *display) { ClutterStage *stage = stage_from_display (display); - return clutter_stage_get_key_focus (stage) == CLUTTER_ACTOR (stage); + return clutter_stage_get_key_focus (stage) == NULL; } static gboolean diff --git a/src/x11/meta-x11-display.c b/src/x11/meta-x11-display.c index 7f3499ea6e0..0b372e3dfa0 100644 --- a/src/x11/meta-x11-display.c +++ b/src/x11/meta-x11-display.c @@ -1301,7 +1301,7 @@ stage_has_focus_actor (MetaX11Display *x11_display) key_focus = clutter_stage_get_key_focus (stage); - return key_focus != CLUTTER_ACTOR (stage); + return key_focus != NULL; } static void -- GitLab From 897cc303cb75e442e577aeb70aca60b90e50d67b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCllner?= Date: Mon, 10 Feb 2025 20:46:53 +0100 Subject: [PATCH 3/4] tests/shell: Do not set key-focus to stage Moving the key focus to the stage should be done by unsetting the focus rather than setting it to the stage itself. `clutter_stage_set_key_focus()` already "normalizes" the stage to NULL internally, so this does not change the actual behavior of the code. Part-of: --- src/tests/meta-test-shell.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/meta-test-shell.c b/src/tests/meta-test-shell.c index b06cf1f269e..83a169652f5 100644 --- a/src/tests/meta-test-shell.c +++ b/src/tests/meta-test-shell.c @@ -326,7 +326,7 @@ on_overlay_key (MetaDisplay *display, { test_shell->overview.grab = clutter_stage_grab (stage, CLUTTER_ACTOR (stage)); test_shell->overview.prev_focus = clutter_stage_get_key_focus (stage); - clutter_stage_set_key_focus (stage, CLUTTER_ACTOR (stage)); + clutter_stage_set_key_focus (stage, NULL); } else { -- GitLab From 33761eb2afd165c7ccd48c3f4a385e598775d366 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCllner?= Date: Mon, 10 Feb 2025 20:56:50 +0100 Subject: [PATCH 4/4] clutter/stage: Warn when setting stage key focus to stage While the `get_key_focus()` method returned the stage itself when no explicit focus was set, it made sense for the corresponding setter to accept the stage as synonym for NULL. But now that the getter always returns the property value, it makes more sense to expect NULL to unset the key focus. Keep the current behavior of normalizing the key focus to NULL in that case to minimize breakage, but print a warning to use NULL instead. Part-of: --- clutter/clutter/clutter-stage.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/clutter/clutter/clutter-stage.c b/clutter/clutter/clutter-stage.c index a09dab3ba76..074f5fe4d6b 100644 --- a/clutter/clutter/clutter-stage.c +++ b/clutter/clutter/clutter-stage.c @@ -2110,7 +2110,11 @@ clutter_stage_set_key_focus (ClutterStage *stage, /* normalize the key focus. NULL == stage */ if (actor == CLUTTER_ACTOR (stage)) - actor = NULL; + { + g_warning ("Stage key focus was set to stage itself, " + "unsetting focus instead"); + actor = NULL; + } /* avoid emitting signals and notifications if we're setting the same * actor as the key focus -- GitLab