Merge lp://qastaging/~bregma/geis/lp-742555 into lp://qastaging/geis

Proposed by Stephen M. Webb
Status: Merged
Merged at revision: 129
Proposed branch: lp://qastaging/~bregma/geis/lp-742555
Merge into: lp://qastaging/geis
Diff against target: 499 lines (+229/-56)
9 files modified
ChangeLog (+21/-0)
include/geis/geis.h (+10/-0)
libutouch-geis/backend/xcb/geis_xcb_backend.c (+10/-7)
libutouch-geis/backend/xcb/grail_gestures.c (+102/-49)
libutouch-geis/geis_v1.c (+5/-0)
testsuite/geis1/Makefile.am (+1/-0)
testsuite/geis1/check_geis1_api.c (+2/-0)
testsuite/geis1/check_gesture_types.c (+77/-0)
testsuite/geistest/geistest.c (+1/-0)
To merge this branch: bzr merge lp://qastaging/~bregma/geis/lp-742555
Reviewer Review Type Date Requested Status
Chase Douglas (community) Approve
Review via email: mp+54920@code.qastaging.launchpad.net

Description of the change

Adds through support for TOUCH gesture types for direct touch begin/end handling.

To post a comment you must log in.
Revision history for this message
Chase Douglas (chasedouglas) wrote :

I'd prefer to move to C99 and use dynamic array sizes than use alloca, just cause it seems more conventional.

The s_grail_class_map array has a bunch of changes to unaffected gesture types. It appears there's a comma added near the end of each line. I think the comma is superfluous and should be removed so there's no change.

grail_mask_clear could just use memset, which would make the code more readable.

Why does grail_mask_setbit only concern itself with the lower 5 bits of each uint32_t?

Although grail exposes velocity attributes for touch gestures, I don't want to expose the attributes through GEIS. I propose masking them out.

Looks good otherwise!

review: Needs Fixing
130. By Stephen M. Webb

* Replaced alloca() calls with C99 dynamic arrays.
* Used memset instead of a for-loop for array initialization.
* Removed extraneous commas from s_grail_class_map.

131. By Stephen M. Webb

Properly initialized the grail gesture bitmask in all cases.

Revision history for this message
Stephen M. Webb (bregma) wrote :

New revision pushed up.

(1) alloca() replaced by C99 dynamic arrays.
(b) used memset() instead of a for-loop to initialize the array.
(3) removed extraneous commas.
(5) initialized all instances of mask_len.

grail_mask_setbit() only concerns itself with the lower 2^5 bits of each uint32_t because there are only 2^5 bits in a uint32_t.

132. By Stephen M. Webb

Cleared touch count mask.

133. By Stephen M. Webb

Changed the list of properties received from grail for the Touch gesture.

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

The code looks good now, and geistest is behaving as I expect :).

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches