From f9318d145262444e2b671b40122fec9cab39b609 Mon Sep 17 00:00:00 2001 From: Derek Foreman Date: Mon, 11 May 2015 15:40:11 -0500 Subject: [PATCH] input: add a weston_pointer_clear_focus() helper function Valgrind has shown that in at least one place (default_grab_pointer_focus) we're testing uninitialized values coming out of weston_compositor_pick_view. This is happening when default_grab_pointer_focus is called when there is nothing on the view list, and during the first repaint when only the black surface with no input region exists. This patch adds a function to clear pointer focus and also set the sx,sy co-ordinates to a sentinel value we shouldn't compute with. Assertions are added to make sure any time pointer focus is set to NULL these values are used. weston_compositor_pick_view() now returns these values too. Now the values are always initialized, even when no view exists, and they're initialized in such a way that actually doing computation with them should fail in an obvious way, but we can compare them safely for equality. Signed-off-by: Derek Foreman Reviewed-by: Daniel Stone --- desktop-shell/exposay.c | 4 +--- desktop-shell/shell.c | 4 +--- src/compositor.c | 7 +++---- src/compositor.h | 2 ++ src/data-device.c | 3 +-- src/input.c | 34 +++++++++++++++++++++++++++++----- 6 files changed, 37 insertions(+), 17 deletions(-) diff --git a/desktop-shell/exposay.c b/desktop-shell/exposay.c index 109f8e33..08b86a3e 100644 --- a/desktop-shell/exposay.c +++ b/desktop-shell/exposay.c @@ -589,9 +589,7 @@ exposay_transition_active(struct desktop_shell *shell) if (pointer) { weston_pointer_start_grab(pointer, &shell->exposay.grab_ptr); - weston_pointer_set_focus(pointer, NULL, - pointer->x, - pointer->y); + weston_pointer_clear_focus(pointer); } wl_list_for_each(shell_output, &shell->output_list, link) { enum exposay_layout_state state; diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c index c1308c52..034d39b9 100644 --- a/desktop-shell/shell.c +++ b/desktop-shell/shell.c @@ -3218,9 +3218,7 @@ popup_grab_focus(struct weston_pointer_grab *grab) wl_resource_get_client(view->surface->resource) == client) { weston_pointer_set_focus(pointer, view, sx, sy); } else { - weston_pointer_set_focus(pointer, NULL, - wl_fixed_from_int(0), - wl_fixed_from_int(0)); + weston_pointer_clear_focus(pointer); } } diff --git a/src/compositor.c b/src/compositor.c index 00b09a67..e27f44e9 100644 --- a/src/compositor.c +++ b/src/compositor.c @@ -1716,6 +1716,8 @@ weston_compositor_pick_view(struct weston_compositor *compositor, return view; } + *vx = wl_fixed_from_int(-1000000); + *vy = wl_fixed_from_int(-1000000); return NULL; } @@ -1760,10 +1762,7 @@ weston_view_unmap(struct weston_view *view) if (keyboard && keyboard->focus == view->surface) weston_keyboard_set_focus(keyboard, NULL); if (pointer && pointer->focus == view) - weston_pointer_set_focus(pointer, - NULL, - wl_fixed_from_int(0), - wl_fixed_from_int(0)); + weston_pointer_clear_focus(pointer); if (touch && touch->focus == view) weston_touch_set_focus(touch, NULL); } diff --git a/src/compositor.h b/src/compositor.h index e1a79545..d1ca9ab7 100644 --- a/src/compositor.h +++ b/src/compositor.h @@ -374,6 +374,8 @@ weston_pointer_set_focus(struct weston_pointer *pointer, struct weston_view *view, wl_fixed_t sx, wl_fixed_t sy); void +weston_pointer_clear_focus(struct weston_pointer *pointer); +void weston_pointer_start_grab(struct weston_pointer *pointer, struct weston_pointer_grab *grab); void diff --git a/src/data-device.c b/src/data-device.c index 89ffce49..1612091a 100644 --- a/src/data-device.c +++ b/src/data-device.c @@ -591,8 +591,7 @@ weston_pointer_start_drag(struct weston_pointer *pointer, &drag->base.data_source_listener); } - weston_pointer_set_focus(pointer, NULL, - wl_fixed_from_int(0), wl_fixed_from_int(0)); + weston_pointer_clear_focus(pointer); weston_pointer_start_grab(pointer, &drag->grab); return 0; diff --git a/src/input.c b/src/input.c index 3227db46..e230c833 100644 --- a/src/input.c +++ b/src/input.c @@ -82,7 +82,7 @@ pointer_focus_view_destroyed(struct wl_listener *listener, void *data) container_of(listener, struct weston_pointer, focus_view_listener); - weston_pointer_set_focus(pointer, NULL, 0, 0); + weston_pointer_clear_focus(pointer); } static void @@ -92,7 +92,7 @@ pointer_focus_resource_destroyed(struct wl_listener *listener, void *data) container_of(listener, struct weston_pointer, focus_resource_listener); - weston_pointer_set_focus(pointer, NULL, 0, 0); + weston_pointer_clear_focus(pointer); } static void @@ -498,6 +498,9 @@ weston_pointer_create(struct weston_seat *seat) wl_signal_add(&seat->compositor->output_destroyed_signal, &pointer->output_destroy_listener); + pointer->sx = wl_fixed_from_int(-1000000); + pointer->sy = wl_fixed_from_int(-1000000); + return pointer; } @@ -628,6 +631,26 @@ seat_send_updated_caps(struct weston_seat *seat) wl_signal_emit(&seat->updated_caps_signal, seat); } + +/** Clear the pointer focus + * + * \param pointer the pointer to clear focus for. + * + * This can be used to unset pointer focus and set the co-ordinates to the + * arbitrary values we use for the no focus case. + * + * There's no requirement to use this function. For example, passing the + * results of a weston_compositor_pick_view() directly to + * weston_pointer_set_focus() will do the right thing when no view is found. + */ +WL_EXPORT void +weston_pointer_clear_focus(struct weston_pointer *pointer) +{ + weston_pointer_set_focus(pointer, NULL, + wl_fixed_from_int(-1000000), + wl_fixed_from_int(-1000000)); +} + WL_EXPORT void weston_pointer_set_focus(struct weston_pointer *pointer, struct weston_view *view, @@ -699,6 +722,9 @@ weston_pointer_set_focus(struct weston_pointer *pointer, pointer->sx = sx; pointer->sy = sy; + assert(view || sx == wl_fixed_from_int(-1000000)); + assert(view || sy == wl_fixed_from_int(-1000000)); + wl_signal_emit(&pointer->focus_signal, pointer); } @@ -2271,9 +2297,7 @@ weston_seat_release_pointer(struct weston_seat *seat) seat->pointer_device_count--; if (seat->pointer_device_count == 0) { - weston_pointer_set_focus(pointer, NULL, - wl_fixed_from_int(0), - wl_fixed_from_int(0)); + weston_pointer_clear_focus(pointer); weston_pointer_cancel_grab(pointer); if (pointer->sprite)