After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 679797 - Missing enter/leave events generation for touch events
Missing enter/leave events generation for touch events
Status: RESOLVED FIXED
Product: clutter
Classification: Platform
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: clutter-maint
clutter-maint
Depends on:
Blocks:
 
 
Reported: 2012-07-12 13:24 UTC by Lionel Landwerlin
Modified: 2012-07-17 20:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch v1 (25.14 KB, patch)
2012-07-12 13:25 UTC, Lionel Landwerlin
reviewed Details | Review
patch v2 (31.44 KB, patch)
2012-07-17 15:57 UTC, Lionel Landwerlin
accepted-commit_now Details | Review

Description Lionel Landwerlin 2012-07-12 13:24:33 UTC
Currently Clutter does not generate enter and leave events for touch devices. This means that toolkits like Mx or St can't get an accurate hover/active states for buttons widgets for example (typical case is touching a button and moving you finger away from that button, the buttons retains its pressed state).

The following patch adds enter/leave events generation for touch devices. This complicates a bit the ClutterInputDevice management because we now need to track multiple pointer/touch points positions.

The clutter_input_device_get_coords() has been added to the API to retrieve the position of a touch point if a sequence argument is given, otherwise it returns the pointer's position. It could mean the deprecation of 
clutter_input_device_get_device_coords(). It's up to someone else to decide.

Also the ClutterInputDevice's management of pointer/touch tracking could be reworked a bit to make it less complicated.
Comment 1 Lionel Landwerlin 2012-07-12 13:25:23 UTC
Created attachment 218625 [details] [review]
patch v1
Comment 2 Lionel Landwerlin 2012-07-13 16:50:25 UTC
Reading Tomeu's blog post (http://blog.tomeuvizoso.net/2012/07/multi-touch-in-webkit-clutter.html) about touch events support in webkit makes me realize that the crossing event structure in Clutter should probably have a ClutterEventSequence added to it.
Comment 3 Emmanuele Bassi (:ebassi) 2012-07-16 16:21:24 UTC
(In reply to comment #2)
> Reading Tomeu's blog post
> (http://blog.tomeuvizoso.net/2012/07/multi-touch-in-webkit-clutter.html) about
> touch events support in webkit makes me realize that the crossing event
> structure in Clutter should probably have a ClutterEventSequence added to it.

strictly speaking, we don't need to do that - we can store the EventSequence in the private data structure.

on the other hand, if we added fields to the event structures I'd like to see a series of static assertions to ensure that the ABI is maintained - at least for 1.x.
Comment 4 Emmanuele Bassi (:ebassi) 2012-07-16 16:36:38 UTC
Review of attachment 218625 [details] [review]:

::: clutter/clutter-input-device.c
@@ +386,3 @@
   self->current_state = self->previous_state = 0;
+
+  self->touch_sequences_info = g_hash_table_new_full (g_direct_hash,

you can use NULL, NULL instead of g_direct_*.

@@ +425,3 @@
+      if (info == NULL)
+        {
+          info = g_new0 (ClutterTouchInfo, 1);

I'd use g_slice_new0() instead.

@@ +811,3 @@
+ * Since: 1.12
+ */
+void

if @sequence is not valid any more, the (x, y) values are unset, but there's no notification to the caller.

this function should return a boolean for that.

@@ +812,3 @@
+ */
+void
+clutter_input_device_get_coords (ClutterInputDevice   *device,

given that it's new API, I'd use ClutterPoint instead of a decomposed (x, y) pair.

::: clutter/clutter-input-device.h
@@ +56,3 @@
                                                                  gint                *y);
+CLUTTER_AVAILABLE_IN_1_12
+void                    clutter_input_device_get_coords        (ClutterInputDevice   *device,

either this function supercedes clutter_input_device_get_device_coords(), in which case the latter should be deprecated, or its name should be get_touch_coords() to ensure that it's used for touch events.
Comment 5 Lionel Landwerlin 2012-07-17 15:57:16 UTC
Created attachment 219030 [details] [review]
patch v2

After review I moved the clutter_input_device_get_device_coords() to deprecated API.
Comment 6 Emmanuele Bassi (:ebassi) 2012-07-17 20:12:52 UTC
Review of attachment 219030 [details] [review]:

looks generally good to me, with two minor things that can be corrected when you commit the patch.

::: clutter/deprecated/clutter-input-device-deprecated.c
@@ +27,3 @@
+                                        gint               *y)
+{
+  g_return_if_fail (CLUTTER_IS_INPUT_DEVICE (device));

you could simply reimplement get_device_coords() as get_coords(device, NULL, x, y).

::: clutter/deprecated/clutter-input-device.h
@@ +32,3 @@
+G_BEGIN_DECLS
+
+CLUTTER_DEPRECATED_IN_1_12

this should be:

CLUTTER_DEPRECATED_IN_1_12_FOR(clutter_input_device_get_coords)
Comment 7 Lionel Landwerlin 2012-07-17 20:56:18 UTC
Pushed to master with last modifications.