Merge lp:~dandrader/grail/lp1026962 into lp:grail

[Daniel d'Andrada]

Fixes bug 1023397.

[Chase Douglas (chasedouglas)]

Looks good to me :).

review: Approve

=== modified file 'src/gesture.cpp'
--- src/gesture.cpp	2012-05-09 17:18:29 +0000
+++ src/gesture.cpp	2012-07-23 14:05:24 +0000
@@ -104,31 +104,24 @@
  * Update a gesture with the passed in frame event and list of modified touches
  */
 void Gesture::Update(UFEvent frame_event, TouchSet& modified_touches) {
-  if (IsPhysicallyEnded()) {
-    /* The new event may have given us ownership of all the touches, in which
-     * case we need to send all gesture slices to the client */
-    if (IsActive())
-      FlushSlices();
-
-    /* If the gesture ended, no need to update state */
-    return;
-  }
-
-  UFFrame frame;
-  UFStatus status = frame_event_get_property(frame_event, UFEventPropertyFrame,
-                                             &frame);
-  if (status != UFStatusSuccess) {
-    LOG(Warn) << "failed to get frame from event\n";
+
+  if (ended_) {
+    /* Now the only thing that can happen is receiving ownership */
+    if (!owned_ && !not_owned_) {
+      CheckOwned();
+      if (IsActive())
+        FlushSlices();
+    }
     return;
   }
 
   TouchSet ended_touches;
   bool touch_found = false;
   for (UFTouchId touch_id : modified_touches) {
-    const auto& it = current_touches_.find(touch_id);
+    const auto& it = all_touches_.find(touch_id);
 
     /* If the touch is not part of this gesture, skip it */
-    if (it != current_touches_.end())
+    if (it != all_touches_.end())
       touch_found = true;
     else
       continue;
@@ -158,15 +151,15 @@
     slice = new UGSlice(*this, frame_event, current_touches_,
                         recognized_);
     last_slice_ = SharedUGSlice(slice);
-  } else {
+    slices_.push(last_slice_);
+  } else if (!IsPhysicallyEnded()) {
     UGSlice* slice;
     /* Create gesture update or end slice */
     slice = new UGSlice(last_slice_, *this, frame_event, current_touches_);
     last_slice_ = SharedUGSlice(slice);
+    slices_.push(last_slice_);
   }
 
-  slices_.push(last_slice_);
-
   for (UFTouchId touch_id : ended_touches)
     current_touches_.erase(touch_id);
 

=== modified file 'test/regular/Makefile.am'
--- test/regular/Makefile.am	2012-07-11 13:22:47 +0000
+++ test/regular/Makefile.am	2012-07-23 14:05:24 +0000
@@ -27,6 +27,7 @@
 	grail-fixture.h \
 	intermittent_3touch.cpp \
 	one-touch-gest-finished.cpp \
+	pending-end.cpp \
 	still-gest-finishes.cpp \
 	subscription.cpp
 

=== modified file 'test/regular/grail-fixture.cpp'
--- test/regular/grail-fixture.cpp	2012-07-11 13:22:17 +0000
+++ test/regular/grail-fixture.cpp	2012-07-23 14:05:24 +0000
@@ -56,6 +56,7 @@
   device_ptr->direct = 1;
   device_ptr->axes.push_back(axis_x_struct);
   device_ptr->axes.push_back(axis_y_struct);
+  /* pixels/m */
   device_ptr->window_resolution_x = 3764.70;
   device_ptr->window_resolution_y = 3764.70;
 }
@@ -144,6 +145,227 @@
   return subscription;
 }
 
+void GrailTest::BeginTouch(int touch_id)
+{
+  BeginTouchWindowCoords(touch_id, touch_id * 10.0f, 0.0f);
+}
+
+void GrailTest::BeginTouchWindowCoords(int touch_id, float window_x, float window_y)
+{
+  struct UFTouch_ touch_struct;
+  touch_struct.id = touch_id;
+  touch_struct.state = UFTouchStateBegin;
+  touch_struct.time = time;
+  touch_struct.start_time = touch_struct.time;
+  touch_struct.window_x = window_x;
+  touch_struct.window_y = window_y;
+  touch_struct.owned = 0;
+  touch_struct.pending_end = 0;
+
+  touches.push_back(touch_struct);
+
+  SendFrameEvent(time, fake_window_id, touches);
+
+  // leave it in a neutral state for next uses of touches vector
+  touches[touches.size()-1].state = UFTouchStateUpdate;
+}
+
+void GrailTest::GiveTouchOwnership(int touch_id)
+{
+  auto iterator = FindTouch(touch_id);
+  iterator->state = UFTouchStateUpdate;
+  iterator->owned = 1; // ownership will trigger delivery of grail events.
+  iterator->time = time;
+
+  SendFrameEvent(time, fake_window_id, touches);
+}
+
+void GrailTest::SetTouchWindowCoords(int touch_id, float window_x, float window_y)
+{
+  auto iterator = FindTouch(touch_id);
+  iterator->window_x = window_x;
+  iterator->window_y = window_y;
+}
+
+void GrailTest::UpdateTouches()
+{
+  for (struct UFTouch_ &touch : touches)
+  {
+    touch.time = time;
+  }
+  SendFrameEvent(time, fake_window_id, touches);
+}
+
+void GrailTest::EndTouch(int touch_id)
+{
+  auto iterator = FindTouch(touch_id);
+  iterator->state = UFTouchStateEnd;
+  iterator->time = time;
+
+  SendFrameEvent(time, fake_window_id, touches);
+
+  touches.erase(iterator);
+}
+
+std::vector<struct UFTouch_>::iterator GrailTest::FindTouch(unsigned int touch_id)
+{
+  std::vector<struct UFTouch_>::iterator it = touches.begin();
+
+  while (it != touches.end())
+  {
+    if (it->id == touch_id)
+      return it;
+    ++it;
+  }
+
+  return it;
+}
+
+void GrailTest::ProcessGrailEvents()
+{
+  UGEvent grail_event;
+  UGStatus get_event_status;
+
+  do
+  {
+    get_event_status = grail_get_event(grail_handle, &grail_event);
+    if (get_event_status != UGStatusSuccess)
+      continue;
+
+    switch (grail_event_get_type(grail_event))
+    {
+      case UGEventTypeSlice:
+        {
+          UGStatus status;
+          UGSlice slice;
+          status = grail_event_get_property(grail_event, UGEventPropertySlice, &slice);
+          ASSERT_EQ(UGStatusSuccess, status);
+          ProcessSlice(slice);
+        }
+        break;
+      default:
+        FAIL(); // we are only expecting slice events
+    }
+
+    grail_event_unref(grail_event);
+  }
+  while (get_event_status == UGStatusSuccess);
+
+  ASSERT_EQ(UGStatusErrorNoEvent, get_event_status);
+}
+
+void GrailTest::ProcessSlice(UGSlice slice)
+{
+  switch (grail_slice_get_state(slice))
+  {
+    case UGGestureStateBegin:
+      AddNewGesture(slice);
+      break;
+    case UGGestureStateUpdate:
+      UpdateGesture(slice);
+      break;
+    case UGGestureStateEnd:
+      EndGesture(slice);
+      break;
+    default:
+      FAIL();
+  }
+}
+
+GrailGesture *GrailTest::GestureWithId(unsigned int id)
+{
+  GrailGesture *gesture = nullptr;
+
+  for (GrailGesture &other_gesture : grail_gestures)
+  {
+    if (id == other_gesture.id)
+    {
+      gesture = &other_gesture;
+      break;
+    }
+  }
+
+  return gesture;
+}
+
+void GrailTest::AddNewGesture(UGSlice slice)
+{
+  unsigned int gesture_id;
+  UGStatus status =  grail_slice_get_property(slice, UGSlicePropertyId, &gesture_id);
+  ASSERT_EQ(UGStatusSuccess, status);
+
+  // there should be no gesture with this id
+  ASSERT_EQ(nullptr, GestureWithId(gesture_id));
+
+  GrailGesture gesture;
+  gesture.id = gesture_id;
+
+  unsigned int num_touches;
+  status =  grail_slice_get_property(slice, UGSlicePropertyNumTouches, &num_touches);
+  ASSERT_EQ(UGStatusSuccess, status);
+  for (unsigned int i = 0; i < num_touches; ++i)
+  {
+    UFTouchId touch_id;
+    status = grail_slice_get_touch_id(slice, i, &touch_id);
+    ASSERT_EQ(UGStatusSuccess, status);
+    gesture.touches.insert(touch_id);
+  }
+
+  gesture.construction_finished = grail_slice_get_construction_finished(slice);
+  gesture.state = UGGestureStateBegin;
+
+  grail_gestures.push_back(gesture);
+}
+
+void GrailTest::UpdateGesture(UGSlice slice)
+{
+  GrailGesture *gesture = nullptr;
+  unsigned int gesture_id;
+
+  UGStatus status = grail_slice_get_property(slice, UGSlicePropertyId, &gesture_id);
+  ASSERT_EQ(UGStatusSuccess, status);
+
+  gesture = GestureWithId(gesture_id);
+
+  // the gesture must already exist
+  ASSERT_NE(nullptr, gesture);
+
+  if (grail_slice_get_construction_finished(slice))
+  {
+    gesture->construction_finished = true;
+  }
+  else
+  {
+    // can't go back from true to false
+    ASSERT_FALSE(gesture->construction_finished);
+  }
+
+  ASSERT_TRUE(gesture->state == UGGestureStateBegin
+           || gesture->state == UGGestureStateUpdate);
+  gesture->state = UGGestureStateUpdate;
+}
+
+void GrailTest::EndGesture(UGSlice slice)
+{
+  GrailGesture *gesture = nullptr;
+  unsigned int gesture_id;
+
+  UGStatus status = grail_slice_get_property(slice, UGSlicePropertyId, &gesture_id);
+  ASSERT_EQ(UGStatusSuccess, status);
+
+  gesture = GestureWithId(gesture_id);
+
+  // the gesture must already exist
+  ASSERT_NE(nullptr, gesture);
+
+  gesture->construction_finished = grail_slice_get_construction_finished(slice);
+  ASSERT_TRUE(gesture->construction_finished);
+
+  ASSERT_TRUE(gesture->state == UGGestureStateBegin
+           || gesture->state == UGGestureStateUpdate);
+  gesture->state = UGGestureStateEnd;
+}
+
 void GrailTest::PrintPendingGestures()
 {
   UGEvent grail_event;

=== modified file 'test/regular/grail-fixture.h'
--- test/regular/grail-fixture.h	2012-07-11 13:22:17 +0000
+++ test/regular/grail-fixture.h	2012-07-23 14:05:24 +0000
@@ -5,6 +5,21 @@
 #include <utouch/grail.h>
 #include "frame_mock.h"
 
+#include <list>
+#include <set>
+
+class GrailGesture
+{
+  public:
+  bool HasTouch(UFTouchId touch_id) const
+  {
+    return touches.find(touch_id) != touches.end();
+  }
+  unsigned int id;
+  std::set<UFTouchId> touches;
+  bool construction_finished;
+  UGGestureState state;
+};
 
 class GrailTest : public ::testing::Test
 {
@@ -38,6 +53,21 @@
       std::vector<struct UFTouch_> &touches);
   void SendDeviceAddedEvent(uint64_t time);
 
+  void BeginTouch(int touch_id);
+  void BeginTouchWindowCoords(int touch_id, float window_x, float window_y);
+  void GiveTouchOwnership(int touch_id);
+  void SetTouchWindowCoords(int touch_id, float window_x, float window_y);
+  /* Updates the time of all touches in the "touches" vector and sends that vector
+     in a frame event to grail */
+  void UpdateTouches();
+  void EndTouch(int touch_id);
+  std::vector<struct UFTouch_>::iterator FindTouch(unsigned int touch_id);
+
+  /* Fetches and processes all pending grail events, updating grail_gestures
+     list accordingly. */
+  void ProcessGrailEvents();
+  GrailGesture *GestureWithId(unsigned int id);
+
   /* Helper function for debugging purposes.
      Prints all gestures in grail's queue.
    */
@@ -46,6 +76,22 @@
 
   UGHandle grail_handle;
   std::shared_ptr<struct UFDevice_> device_ptr;
+
+  uint64_t time;
+  UFWindowId fake_window_id;
+  std::vector<struct UFTouch_> touches;
+
+  /*
+    A representation of the currently active gestures according to the grail events
+    fetched by ProcessGrailEvents()
+   */
+  std::list<GrailGesture> grail_gestures;
+
+  private:
+  void ProcessSlice(UGSlice slice);
+  void AddNewGesture(UGSlice slice);
+  void UpdateGesture(UGSlice slice);
+  void EndGesture(UGSlice slice);
 };
 
 #endif // GRAIL_FIXTURE

=== modified file 'test/regular/intermittent_3touch.cpp'
--- test/regular/intermittent_3touch.cpp	2012-07-11 13:22:47 +0000
+++ test/regular/intermittent_3touch.cpp	2012-07-23 14:05:24 +0000
@@ -1,234 +1,9 @@
 #include "grail-fixture.h"
 
-#include <list>
-#include <set>
-
-class GrailGesture
-{
-  public:
-  bool HasTouch(UFTouchId touch_id) const
-  {
-    return touches.find(touch_id) != touches.end();
-  }
-  unsigned int id;
-  std::set<UFTouchId> touches;
-  bool construction_finished;
-};
-
 class Intermittent3TouchTest : public GrailTest
 {
-  protected:
-
-  void BeginTouch(int touch_id);
-  void GiveTouchOwnership(int touch_id);
-  void EndTouch(int touch_id);
-  std::vector<struct UFTouch_>::iterator FindTouch(unsigned int touch_id);
-  void ProcessGrailEvents();
-  void ProcessSlice(UGSlice slice);
-  void AddNewGesture(UGSlice slice);
-  void UpdateGesture(UGSlice slice);
-  void EndGesture(UGSlice slice);
-  GrailGesture *GestureWithId(unsigned int id);
-
-  uint64_t time;
-  UFWindowId fake_window_id;
-
-  std::vector<struct UFTouch_> touches;
-
-  std::list<GrailGesture> grail_gestures;
 };
 
-void Intermittent3TouchTest::BeginTouch(int touch_id)
-{
-  struct UFTouch_ touch_struct;
-  touch_struct.id = touch_id;
-  touch_struct.state = UFTouchStateBegin;
-  touch_struct.time = time;
-  touch_struct.start_time = touch_struct.time;
-  touch_struct.owned = 0;
-  touch_struct.pending_end = 0;
-
-  touches.push_back(touch_struct);
-
-  SendFrameEvent(time, fake_window_id, touches);
-}
-
-void Intermittent3TouchTest::GiveTouchOwnership(int touch_id)
-{
-  auto iterator = FindTouch(touch_id);
-  iterator->state = UFTouchStateUpdate;
-  iterator->owned = 1; // ownership will trigger delivery of grail events.
-  iterator->time = time;
-
-  SendFrameEvent(time, fake_window_id, touches);
-}
-
-void Intermittent3TouchTest::EndTouch(int touch_id)
-{
-  auto iterator = FindTouch(touch_id);
-  iterator->state = UFTouchStateEnd;
-  iterator->time = time;
-
-  SendFrameEvent(time, fake_window_id, touches);
-
-  touches.erase(iterator);
-}
-
-std::vector<struct UFTouch_>::iterator Intermittent3TouchTest::FindTouch(unsigned int touch_id)
-{
-  std::vector<struct UFTouch_>::iterator it = touches.begin();
-
-  while (it != touches.end())
-  {
-    if (it->id == touch_id)
-      return it;
-    ++it;
-  }
-
-  return it;
-}
-
-void Intermittent3TouchTest::ProcessGrailEvents()
-{
-  UGEvent grail_event;
-  UGStatus get_event_status;
-
-  do
-  {
-    get_event_status = grail_get_event(grail_handle, &grail_event);
-    if (get_event_status != UGStatusSuccess)
-      continue;
-
-    switch (grail_event_get_type(grail_event))
-    {
-      case UGEventTypeSlice:
-        {
-          UGStatus status;
-          UGSlice slice;
-          status = grail_event_get_property(grail_event, UGEventPropertySlice, &slice);
-          ASSERT_EQ(UGStatusSuccess, status);
-          ProcessSlice(slice);
-        }
-        break;
-      default:
-        FAIL(); // we are only expecting slice events
-    }
-
-    grail_event_unref(grail_event);
-  }
-  while (get_event_status == UGStatusSuccess);
-
-  ASSERT_EQ(UGStatusErrorNoEvent, get_event_status);
-}
-
-void Intermittent3TouchTest::ProcessSlice(UGSlice slice)
-{
-  switch (grail_slice_get_state(slice))
-  {
-    case UGGestureStateBegin:
-      AddNewGesture(slice);
-      break;
-    case UGGestureStateUpdate:
-      UpdateGesture(slice);
-      break;
-    case UGGestureStateEnd:
-      EndGesture(slice);
-      break;
-    default:
-      FAIL();
-  }
-}
-
-GrailGesture *Intermittent3TouchTest::GestureWithId(unsigned int id)
-{
-  GrailGesture *gesture = nullptr;
-
-  for (auto other_gesture : grail_gestures)
-  {
-    if (id == other_gesture.id)
-    {
-      gesture = &other_gesture;
-      break;
-    }
-  }
-
-  return gesture;
-}
-
-void Intermittent3TouchTest::AddNewGesture(UGSlice slice)
-{
-  unsigned int gesture_id;
-  UGStatus status =  grail_slice_get_property(slice, UGSlicePropertyId, &gesture_id);
-  ASSERT_EQ(UGStatusSuccess, status);
-
-  // there should be no gesture with this id
-  ASSERT_EQ(nullptr, GestureWithId(gesture_id));
-
-  GrailGesture gesture;
-  gesture.id = gesture_id;
-
-  unsigned int num_touches;
-  status =  grail_slice_get_property(slice, UGSlicePropertyNumTouches, &num_touches);
-  ASSERT_EQ(UGStatusSuccess, status);
-  for (unsigned int i = 0; i < num_touches; ++i)
-  {
-    UFTouchId touch_id;
-    status = grail_slice_get_touch_id(slice, i, &touch_id);
-    ASSERT_EQ(UGStatusSuccess, status);
-    gesture.touches.insert(touch_id);
-  }
-
-  gesture.construction_finished = grail_slice_get_construction_finished(slice);
-
-  grail_gestures.push_back(gesture);
-}
-
-void Intermittent3TouchTest::UpdateGesture(UGSlice slice)
-{
-  GrailGesture *gesture = nullptr;
-  unsigned int gesture_id;
-
-  UGStatus status = grail_slice_get_property(slice, UGSlicePropertyId, &gesture_id);
-  ASSERT_EQ(UGStatusSuccess, status);
-
-  gesture = GestureWithId(gesture_id);
-
-  // the gesture must already exist
-  ASSERT_NE(nullptr, gesture);
-
-  if (grail_slice_get_construction_finished(slice))
-  {
-    gesture->construction_finished = true;
-  }
-  else
-  {
-    // can't go back from true to false
-    ASSERT_FALSE(gesture->construction_finished);
-  }
-}
-
-void Intermittent3TouchTest::EndGesture(UGSlice slice)
-{
-  unsigned int gesture_id;
-  bool found = false;
-
-  UGStatus status = grail_slice_get_property(slice, UGSlicePropertyId, &gesture_id);
-  ASSERT_EQ(UGStatusSuccess, status);
-
-  for (auto it = grail_gestures.begin(); !found && it != grail_gestures.end(); ++it)
-  {
-    if (gesture_id == it->id)
-    {
-      found = true;
-      grail_gestures.erase(it);
-      break;
-    }
-  }
-
-  // the gesture must already exist
-  ASSERT_TRUE(found);
-}
-
 /*
   Simulate the following situation:
      The user lays 3 fingers over a touchscreen or trackpad
@@ -276,7 +51,6 @@
   */
   ASSERT_EQ(1, grail_gestures.size());
   ASSERT_EQ(3, grail_gestures.front().touches.size());
-  int first_gesture_id = grail_gestures.front().id;
 
   time = 20531961;
   EndTouch(18);
@@ -296,13 +70,13 @@
     gesture(id=1, touches={19,20,21}) begins
     gesture(id=2, touches={19,20,21}) begins
   */
-  ASSERT_EQ(nullptr, GestureWithId(first_gesture_id));
-  ASSERT_EQ(1, grail_gestures.size());
-  const GrailGesture &gesture = grail_gestures.front();
-  ASSERT_EQ(3, gesture.touches.size());
-  ASSERT_TRUE(gesture.HasTouch(19));
-  ASSERT_TRUE(gesture.HasTouch(20));
-  ASSERT_TRUE(gesture.HasTouch(21));
+  ASSERT_EQ(UGGestureStateEnd, GestureWithId(0)->state);
+  ASSERT_EQ(2, grail_gestures.size());
+  GrailGesture *gesture = GestureWithId(1);
+  ASSERT_EQ(3, gesture->touches.size());
+  ASSERT_TRUE(gesture->HasTouch(19));
+  ASSERT_TRUE(gesture->HasTouch(20));
+  ASSERT_TRUE(gesture->HasTouch(21));
 
   grail_subscription_delete(subscription);
 }

=== added file 'test/regular/pending-end.cpp'
--- test/regular/pending-end.cpp	1970-01-01 00:00:00 +0000
+++ test/regular/pending-end.cpp	2012-07-23 14:05:24 +0000
@@ -0,0 +1,78 @@
+#include "grail-fixture.h"
+
+/*
+  Checks that once a touch whose end is pending gets finally owned its slices
+  are sent to the client.
+
+  Regression test for https://bugs.launchpad.net/utouch-grail/+bug/1026962
+ */
+TEST_F(GrailTest, PendingEndImpedesGestures)
+{
+  UGStatus status;
+  struct UFHandle_ frame_handle_struct;
+  fake_window_id = 321;
+
+  status = grail_new(&frame_handle_struct, &grail_handle);
+  ASSERT_EQ(UGStatusSuccess, status);
+
+  SendDeviceAddedEvent(time);
+
+  UGSubscription sub_3touch =
+    CreateSubscription(3, UGGestureTypeTouch | UGGestureTypeDrag | UGGestureTypePinch,
+                       device_ptr.get(), fake_window_id);
+  ASSERT_NE(nullptr, sub_3touch);
+
+  time = 13688369;
+  BeginTouchWindowCoords(1 /* touch id */, 30.0f /* x */, 0.0f /* y */);
+
+  time = 13688369;
+  BeginTouchWindowCoords(2, 40.0f, 0.0f);
+  time = 13688372;
+  GiveTouchOwnership(2);
+
+  time = 13688369;
+  BeginTouchWindowCoords(3, 50.0f, 0.0f);
+  time = 13688373;
+  GiveTouchOwnership(3);
+
+  time = 13688430;
+  grail_update_time(grail_handle, time);
+
+  /* Perform a 4 fingers drag. */
+  for (int i=1; i<=4; ++i)
+  {
+    time += 100;
+    SetTouchWindowCoords(1, 30.0f, i*10.0f);
+    SetTouchWindowCoords(2, 40.0f, i*10.0f);
+    SetTouchWindowCoords(3, 50.0f, i*10.0f);
+    UpdateTouches();
+  }
+
+  time = 13688877;
+  FindTouch(1)->pending_end = 1;
+  UpdateTouches();
+
+  ProcessGrailEvents();
+  // no gesture slices yet since not all touches are owned
+  ASSERT_EQ(0, grail_gestures.size());
+
+  time = 13688878;
+  GiveTouchOwnership(1);
+
+  EndTouch(1);
+
+  ProcessGrailEvents();
+  // Gesture slices should have been sent to the client by now
+  ASSERT_EQ(1, grail_gestures.size());
+  // Gesture must have ended since it cannot continue with only two touches.
+  ASSERT_EQ(UGGestureStateEnd, grail_gestures.front().state);
+
+  EndTouch(2);
+
+  time = 13688880;
+  EndTouch(3);
+
+  ProcessGrailEvents();
+
+  grail_subscription_delete(sub_3touch);
+}