Merge lp:~jpakkane/libgrip/type_detection_refactoring into lp:libgrip

Proposed by Jussi Pakkanen
Status: Merged
Merged at revision: 55
Proposed branch: lp:~jpakkane/libgrip/type_detection_refactoring
Merge into: lp:libgrip
Diff against target: 86 lines (+31/-17)
1 file modified
src/gripgesturemanager.c (+31/-17)
To merge this branch: bzr merge lp:~jpakkane/libgrip/type_detection_refactoring
Reviewer Review Type Date Requested Status
Chase Douglas (community) Approve
Stephen M. Webb (community) Approve
Review via email: mp+70719@code.launchpad.net

Description of the change

There are two different locations where input attributes are converted to types. This patch consolidates them into a single function.

It is currently static but we should expose this function in API. Since gesture subscription is done with input types, for symmetry they should be easily and directly available. In fact it is worth asking if the direct/independent attributes should be exposed to libgrip users at all. I don't think that's something they really care about. Should libgrip ever get other backends, they might even not provide those attributes.

Depending on feedback I can add the API exposition here or create a new branch.

To post a comment you must log in.
Revision history for this message
Stephen M. Webb (bregma) wrote :

I have no problem with this patch per se. I would hold off on making the function public until we've had a review of the use stories for the API itself.

review: Approve
Revision history for this message
Chase Douglas (chasedouglas) wrote :

Looks good to me too.

I can't think of any reason not to expose the API publicly, so I'm +1 on that. However, I'll defer judgement to Stephen. To give a second take, in utouch-qml I only expose the type of device in the same sense as grip_get_device_type, so it would mirror what is done there.

All that said, we're up against the feature freeze deadline and this is just a convenience addition. I think we should leave it aside for now if we aren't absolutely sure about it rather than rush it in.

review: Approve
Revision history for this message
Jussi Pakkanen (jpakkane) wrote :

The main use story can be found in our touch patch for Evince. It goes through several hoops (setting up different callback functions that call other things etc) which essentially reduces to one if statement in the main callback function. That if just negates the drag amounts if the gesture came from a touchpad. Having this function would make all the extra stuff just go away.

But I'll merge this now and start a new branch for exposing.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/gripgesturemanager.c'
2--- src/gripgesturemanager.c 2011-08-04 01:04:47 +0000
3+++ src/gripgesturemanager.c 2011-08-08 11:11:21 +0000
4@@ -419,6 +419,22 @@
5 return input_device;
6 }
7
8+static
9+GripDeviceType
10+grip_get_device_type (GripInputDevice *input_device) {
11+ gboolean is_direct = grip_input_device_is_direct (input_device);
12+ gboolean is_independent = grip_input_device_is_independent (input_device);
13+ if (is_direct && !is_independent)
14+ return GRIP_DEVICE_TOUCHSCREEN;
15+ if (!is_direct && !is_independent)
16+ return GRIP_DEVICE_TOUCHPAD;
17+ if (!is_direct && is_independent)
18+ return GRIP_DEVICE_INDEPENDENT;
19+
20+ g_critical("Unknown touch device type.");
21+ return GRIP_DEVICE_TOUCHSCREEN;
22+}
23+
24 /*
25 * registration_for_input_device:
26 * @registrations: A collection of #GripGestureRegistration
27@@ -433,18 +449,16 @@
28 registration_for_input_device (struct Registrations *registrations,
29 GripInputDevice *input_device)
30 {
31- gboolean is_direct = grip_input_device_is_direct (input_device);
32- gboolean is_independent = grip_input_device_is_independent (input_device);
33-
34- if (is_direct && !is_independent)
35+ GripDeviceType device_type = grip_get_device_type(input_device);
36+ if (device_type == GRIP_DEVICE_TOUCHSCREEN)
37 {
38 return registrations->touchscreen;
39 }
40- else if (!is_direct && !is_independent)
41+ else if (GRIP_DEVICE_TOUCHPAD)
42 {
43 return registrations->touchpad;
44 }
45- else if (!is_direct && is_independent)
46+ else if (GRIP_DEVICE_INDEPENDENT)
47 {
48 return registrations->independent;
49 }
50@@ -1010,6 +1024,7 @@
51 return g_object_new (GRIP_TYPE_GESTURE_MANAGER, NULL);
52 }
53
54+
55 static void
56 grip_devices_for_type (GripDeviceType type, GArray *selection,
57 GripDevices *devices)
58@@ -1020,18 +1035,17 @@
59 {
60 GripInputDevice *input_device = g_ptr_array_index (devices, i);
61 GeisInputDeviceId id = grip_input_device_get_id (input_device);
62- gboolean is_direct = grip_input_device_is_direct (input_device);
63- gboolean is_independent = grip_input_device_is_independent (input_device);
64+ GripDeviceType device_type= grip_get_device_type(input_device);
65
66- if (type & GRIP_DEVICE_TOUCHSCREEN && is_direct && !is_independent)
67- {
68- g_array_append_val (selection, id);
69- }
70- if (type & GRIP_DEVICE_TOUCHPAD && !is_direct && !is_independent)
71- {
72- g_array_append_val (selection, id);
73- }
74- if (type & GRIP_DEVICE_INDEPENDENT && !is_direct && is_independent)
75+ if ((type & GRIP_DEVICE_TOUCHSCREEN) && device_type == GRIP_DEVICE_TOUCHSCREEN)
76+ {
77+ g_array_append_val (selection, id);
78+ }
79+ if ((type & GRIP_DEVICE_TOUCHPAD) && device_type == GRIP_DEVICE_TOUCHPAD)
80+ {
81+ g_array_append_val (selection, id);
82+ }
83+ if ((type & GRIP_DEVICE_INDEPENDENT) && device_type == GRIP_DEVICE_INDEPENDENT)
84 {
85 g_array_append_val (selection, id);
86 }

Subscribers

People subscribed via source and target branches