Merge lp:~dandrader/unity/lp969554 into lp:unity

[Daniel d'Andrada]

Fix pinch gesture.

Fixes bug #969554

Now with unit tests.

[Thomi Richards (thomir-deactivatedaccount)]

Hi,

How can we test this? We need either a unit test, or an autopilot test, or, as a final resort, a manual test. From the looks of it, GestureEngine is going to be tricky to write a unit test for (it contains a lot of XLib calls, and is tightly coupled with CompScreen), but perhaps we can write an autopilot test? We'd need to work out how to simulate multi-touch using python & XLib.

...If that doesn't work, we need a manual test. Please consult the README and TEST_TEMPLATE files in manual-tests/ before writing one.

Thanks

review: Needs Fixing

[Daniel d'Andrada (dandrader)]

> How can we test this? We need either a unit test, or an autopilot test, or, as
> a final resort, a manual test.[...]

It now has a unit test, but it's built on top of the commits from lp:~dandrader/unity/lp940612 (merge proposal https://code.launchpad.net/~dandrader/unity/lp940612/+merge/99956)

[Thomi Richards (thomir-deactivatedaccount)]

Looks good to me.

review: Approve

[Unity Merger (unity-merger)]

The prerequisite lp:~dandrader/unity/lp940612 has not yet been merged into lp:unity.

[Unity Merger (unity-merger)]

No proposals found for merge of lp:~dandrader/unity/lp940612 into lp:unity.

=== modified file 'plugins/unityshell/src/GestureEngine.cpp'
--- plugins/unityshell/src/GestureEngine.cpp	2012-04-04 17:09:30 +0000
+++ plugins/unityshell/src/GestureEngine.cpp	2012-04-04 17:09:31 +0000
@@ -244,7 +244,6 @@
       return;
 
     _pinch_id = data->id;
-    _pinch_start_radius = data->radius;
 
     if (_pinch_grab)
       _screen->removeGrab(_pinch_grab, NULL);
@@ -257,17 +256,14 @@
   if (data->id != _pinch_id)
     return;
 
-  float delta_radius = data->radius - _pinch_start_radius;
-  if (delta_radius > 110.0f)
+  if (data->radius > 1.25)
   {
     _pinch_window->maximize(MAXIMIZE_STATE);
-    _pinch_start_radius = data->radius;
     EndDrag();
   }
-  else if (delta_radius < -110.0f)
+  else if (data->radius < 0.8)
   {
     _pinch_window->maximize(0);
-    _pinch_start_radius = data->radius;
     EndDrag();
   }
 }

=== modified file 'plugins/unityshell/src/GestureEngine.h'
--- plugins/unityshell/src/GestureEngine.h	2012-04-04 17:09:30 +0000
+++ plugins/unityshell/src/GestureEngine.h	2012-04-04 17:09:31 +0000
@@ -64,6 +64,5 @@
   int _pinch_id;
   int _touch_id;
 
-  float _pinch_start_radius;
   Cursor _fleur_cursor;
 };

=== modified file 'tests/test-gesture-engine/compiz_mock/core/window.h'
--- tests/test-gesture-engine/compiz_mock/core/window.h	2012-04-04 17:09:30 +0000
+++ tests/test-gesture-engine/compiz_mock/core/window.h	2012-04-04 17:09:31 +0000
@@ -26,7 +26,7 @@
 
 class CompWindowMock {
 public:
-  CompWindowMock() : _moved(false) {}
+  CompWindowMock() : _moved(false), _maximize_count(0), _maximize_state(0) {}
 
   int x() const {return _geometry.x();}
   int y() const {return _geometry.y();}
@@ -41,7 +41,7 @@
 
   unsigned int actions () {return _actions;}
 
-  void maximize(int state) {}
+  void maximize(int state) {++_maximize_count; _maximize_state = state;}
 
   /* OBS: I wonder why it returns a reference */
   unsigned int &state() {return _state;}
@@ -61,6 +61,9 @@
   bool _moved;
   int _movement_x;
   int _movement_y;
+
+  int _maximize_count;
+  int _maximize_state;
 };
 
 #endif

=== modified file 'tests/test-gesture-engine/test_gesture_engine.cpp'
--- tests/test-gesture-engine/test_gesture_engine.cpp	2012-04-04 17:09:30 +0000
+++ tests/test-gesture-engine/test_gesture_engine.cpp	2012-04-04 17:09:31 +0000
@@ -36,6 +36,39 @@
     GenerateWindows();
   }
 
+  void PerformPinch(GestureEngine &gesture_engine, float peak_radius) {
+    CompWindowMock *middle_window = screen_mock->_client_list_stacking[1];
+
+    GeisAdapterMock::GeisTouchData touch_data;
+    touch_data.id = 1;
+    touch_data.touches = 3;
+    touch_data.window = 123;
+    touch_data.focus_x = 100; /* hits the middle window */
+    touch_data.focus_y = 100;
+    gesture_engine.OnTouchStart(&touch_data);
+
+    GeisAdapterMock::GeisPinchData pinch_data;
+    pinch_data.id = 1;
+    pinch_data.touches = 3;
+    pinch_data.window = 123;
+    pinch_data.focus_x = 100; /* hits the middle window */
+    pinch_data.focus_y = 100;
+    pinch_data.radius = 1.0;
+    gesture_engine.OnPinchStart(&pinch_data);
+
+    touch_data.focus_x += 10;
+    touch_data.focus_y += 20;
+    gesture_engine.OnTouchUpdate(&touch_data);
+
+    pinch_data.focus_x += 10;
+    pinch_data.focus_y += 20;
+    pinch_data.radius = peak_radius;
+    gesture_engine.OnPinchUpdate(&pinch_data);
+
+    gesture_engine.OnTouchFinish(&touch_data);
+    gesture_engine.OnPinchFinish(&pinch_data);
+  }
+
  private:
   void GenerateWindows() {
     /* remove windows from previous test */
@@ -155,6 +188,38 @@
   ASSERT_FALSE(middle_window->_moved);
 }
 
+TEST_F(GestureEngineTest, ThreeFingersPinchMaximizesWindow)
+{
+  GestureEngine gesture_engine(screen_mock);
+  CompWindowMock *middle_window = screen_mock->_client_list_stacking[1];
+
+  PerformPinch(gesture_engine, 2.0);
+
+  ASSERT_EQ(1, middle_window->_maximize_count);
+  ASSERT_EQ(MAXIMIZE_STATE, middle_window->_maximize_state);
+}
+
+TEST_F(GestureEngineTest, ThreeFingersPinchRestoresWindow)
+{
+  GestureEngine gesture_engine(screen_mock);
+  CompWindowMock *middle_window = screen_mock->_client_list_stacking[1];
+
+  PerformPinch(gesture_engine, 0.3);
+
+  ASSERT_EQ(1, middle_window->_maximize_count);
+  ASSERT_EQ(0, middle_window->_maximize_state);
+}
+
+TEST_F(GestureEngineTest, MinimalThreeFingersPinchDoesNothing)
+{
+  GestureEngine gesture_engine(screen_mock);
+  CompWindowMock *middle_window = screen_mock->_client_list_stacking[1];
+
+  PerformPinch(gesture_engine, 1.1);
+
+  ASSERT_EQ(0, middle_window->_maximize_count);
+}
+
 int main(int argc, char** argv)
 {
   ::testing::InitGoogleTest(&argc, argv);