[PATCH weston v2] input: add a weston_pointer_clear_focus() helper function

Derek Foreman derekf at osg.samsung.com
Mon May 11 13:40:11 PDT 2015


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 <derekf at osg.samsung.com>
---

A rebase and an extended log message.


 desktop-shell/exposay.c |  3 +--
 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(+), 16 deletions(-)

diff --git a/desktop-shell/exposay.c b/desktop-shell/exposay.c
index 4b65cbd..5d77893 100644
--- a/desktop-shell/exposay.c
+++ b/desktop-shell/exposay.c
@@ -572,8 +572,7 @@ exposay_transition_active(struct desktop_shell *shell)
 	shell->exposay.grab_ptr.interface = &exposay_ptr_grab;
 	weston_pointer_start_grab(seat->pointer,
 	                          &shell->exposay.grab_ptr);
-	weston_pointer_set_focus(seat->pointer, NULL,
-			         seat->pointer->x, seat->pointer->y);
+	weston_pointer_clear_focus(seat->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 c05a5c7..65198d5 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -3165,9 +3165,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 8f02b4d..f108e5c 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -1770,6 +1770,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;
 }
 
@@ -1809,10 +1811,7 @@ weston_view_unmap(struct weston_view *view)
 		if (seat->keyboard && seat->keyboard->focus == view->surface)
 			weston_keyboard_set_focus(seat->keyboard, NULL);
 		if (seat->pointer && seat->pointer->focus == view)
-			weston_pointer_set_focus(seat->pointer,
-						 NULL,
-						 wl_fixed_from_int(0),
-						 wl_fixed_from_int(0));
+			weston_pointer_clear_focus(seat->pointer);
 		if (seat->touch && seat->touch->focus == view)
 			weston_touch_set_focus(seat->touch, NULL);
 	}
diff --git a/src/compositor.h b/src/compositor.h
index 867f8c4..ed35153 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -379,6 +379,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 3e7baf9..e1c8ad7 100644
--- a/src/data-device.c
+++ b/src/data-device.c
@@ -587,8 +587,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 501a4b6..4a6b017 100644
--- a/src/input.c
+++ b/src/input.c
@@ -78,7 +78,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
@@ -88,7 +88,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
@@ -490,6 +490,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;
 }
 
@@ -620,6 +623,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,
@@ -691,6 +714,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);
 }
 
@@ -2232,9 +2258,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)
-- 
2.1.4



More information about the wayland-devel mailing list