Merge lp:~chasedouglas/grail/fix-hold-reject into lp:grail

[Chase Douglas]

Ensure touches are rejected after all possible gestures are exhausted rather than when the touch ends.

[Thomas Voß (thomas-voss)]

Is there a reason why touch-deletion code is duplicated for both recognizers? Could you move the code up the inheritance hierarchy?

Apart from that: Looks good to me.

review: Needs Information

[Chase Douglas (chasedouglas)]

Yes, it could probably be moved up for the most part. The main reason it's not already is that the atomic recognizer has to maintain an extra list of touches: new_touches_.

There are multiple ways of skinning this cat:

* Create one big virtual function in recognizer.cpp, and have atomic-recognizer override it as it is currently written.

* Create virtual functions for each touch state.

* Something else?

I haven't really taken the time to think much about it. It would be an unnecessary code change, and at this point in the cycle we need to minimize such changes.

[lp:~chasedouglas/grail/fix-hold-reject]

226. By Chase Douglas on 2012-04-03

Delete subscription in test fixture TearDown() method

The subscription is "owned" by the test fixture, so it should be deleted
by it, not by the test itself.

[Stephen M. Webb (bregma)]

Looks simple enough, should break the touch hold at the right time.

review: Approve

=== modified file 'src/v3/atomic-recognizer.cpp'
--- src/v3/atomic-recognizer.cpp	2012-03-27 15:48:25 +0000
+++ src/v3/atomic-recognizer.cpp	2012-04-03 18:13:20 +0000
@@ -191,10 +191,18 @@
       case UFTouchStateEnd: {
         auto it = all_touches_.find(frame_touch_get_id(touch));
         if (it != all_touches_.end()) {
-          const SharedTouch& grail_touch = it->second;
+          if (it->second.expired()) {
+            ERASE_TOUCH(it->first, all_touches_);
+            break;
+          }
+
+          const SharedTouch& grail_touch = it->second.lock();
 
           grail_touch->Update(touch);
 
+          if (grail_touch->ended())
+            ERASE_TOUCH(grail_touch->id(), all_touches_);
+
           if (grail_touch->pending_end() || grail_touch->ended()) {
             ERASE_TOUCH(grail_touch->id(), new_touches_);
             ERASE_TOUCH(grail_touch->id(), free_touches_);

=== modified file 'src/v3/atomic-recognizer.h'
--- src/v3/atomic-recognizer.h	2012-03-27 15:46:44 +0000
+++ src/v3/atomic-recognizer.h	2012-04-03 18:13:20 +0000
@@ -43,6 +43,8 @@
   void FindGesturesToAccept(uint64_t event_time);
   virtual uint64_t NextTimeout();
 
+  std::map<UFTouchId, std::weak_ptr<Touch>> all_touches_;
+
   /* Touches that have begun but not yet been matched against subscriptions
      (for the creation of new gestures) or used to update existing gestures. */
   TouchMap new_touches_;

=== modified file 'src/v3/recognizer.cpp'
--- src/v3/recognizer.cpp	2012-03-26 21:54:35 +0000
+++ src/v3/recognizer.cpp	2012-04-03 18:13:20 +0000
@@ -217,15 +217,6 @@
       RejectGesture(gesture);
     }
   }
-
-  /* Delete all ended touches */
-  for (auto it = all_touches_.begin(); it != all_touches_.end(); ) {
-    const auto& pair = *it++;
-    const SharedTouch& touch = pair.second;
-
-    if (touch->ended())
-      all_touches_.erase(touch->id());
-  }
 }
 
 /**

=== modified file 'src/v3/recognizer.h'
--- src/v3/recognizer.h	2012-03-26 21:54:35 +0000
+++ src/v3/recognizer.h	2012-04-03 18:13:20 +0000
@@ -99,7 +99,6 @@
   std::set<SharedGesture> unaccepted_gestures_;
   std::set<SharedGesture> accepted_gestures_;
   TouchMap free_touches_;
-  TouchMap all_touches_;
 
   unsigned int num_subscriptions_;
 

=== modified file 'src/v3/regular-recognizer.cpp'
--- src/v3/regular-recognizer.cpp	2012-03-27 15:52:30 +0000
+++ src/v3/regular-recognizer.cpp	2012-04-03 18:13:20 +0000
@@ -128,10 +128,18 @@
       case UFTouchStateEnd: {
         auto it = all_touches_.find(frame_touch_get_id(touch));
         if (it != all_touches_.end()) {
-          const SharedTouch& grail_touch = it->second;
+          if (it->second.expired()) {
+            ERASE_TOUCH(it->first, all_touches_);
+            break;
+          }
+
+          const SharedTouch& grail_touch = it->second.lock();
 
           grail_touch->Update(touch);
 
+          if (grail_touch->ended())
+            ERASE_TOUCH(grail_touch->id(), all_touches_);
+
           if (grail_touch->pending_end() || grail_touch->ended())
             ERASE_TOUCH(grail_touch->id(), free_touches_);
         }

=== modified file 'src/v3/regular-recognizer.h'
--- src/v3/regular-recognizer.h	2012-03-27 15:46:44 +0000
+++ src/v3/regular-recognizer.h	2012-04-03 18:13:20 +0000
@@ -44,6 +44,8 @@
   void MatchThreeTouchGestures(const SharedTouch& touch);
   void MatchFourTouchGestures(const SharedTouch& touch);
   void MatchFiveTouchGestures(const SharedTouch& touch);
+
+  std::map<UFTouchId, std::weak_ptr<Touch>> all_touches_;
 };
 
 } // namespace grail