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

Proposed by Daniel d'Andrada
Status: Superseded
Proposed branch: lp:~dandrader/unity/lp940612
Merge into: lp:unity
Diff against target: 232 lines (+45/-61)
4 files modified
plugins/unityshell/src/GeisAdapter.cpp (+10/-10)
plugins/unityshell/src/GeisAdapter.h (+10/-10)
plugins/unityshell/src/GestureEngine.cpp (+24/-40)
plugins/unityshell/src/GestureEngine.h (+1/-1)
To merge this branch: bzr merge lp:~dandrader/unity/lp940612
Reviewer Review Type Date Requested Status
Tim Penhey (community) Needs Fixing
Review via email: mp+99726@code.launchpad.net

This proposal has been superseded by a proposal from 2012-03-29.

Description of the change

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

Please keep the variable type and variable name separate:
  const CompWindowVector &client_list_stacking
should be
  CompWindowVector const& client_list_stacking
(or
  const CompWindowVector& client_list_stacking
   - but my preference is for the first)

> if (client_list_stacking.size() == 0)

can just be:

  if (client_list_stacking.empty())

> int pos_x = (int) fpos_x;

Please don't use C style casts. It isn't needed here (mostly
certain about that).

The do/while loop misses the first element of the vector if there
is more than one element.

A traditional for loop is more idiomatic and understandable.
Also, can remove the window and result temporaries from outside the loop.
And... if using the iterators, you don't need the empty check first
as the initial check makes sure of that too.

for (auto iter = client_list_stacking.rbegin(),
          end = client_list_stacking.rend();
     iter != end; ++iter)
{
    CompWindow* window = *iter;

    if (pos_x >= window->x() && pos_x <= (window->width() + window->x())
        &&
        pos_y >= window->y() && pos_y <= (window->height() + window->y()))
      return window;
}

return nullptr;

review: Needs Fixing

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/unityshell/src/GeisAdapter.cpp'
2--- plugins/unityshell/src/GeisAdapter.cpp 2011-12-22 17:52:55 +0000
3+++ plugins/unityshell/src/GeisAdapter.cpp 2012-03-29 14:28:18 +0000
4@@ -237,9 +237,9 @@
5 else if (g_str_equal(attr.name, GEIS_GESTURE_ATTRIBUTE_TIMESTAMP))
6 result->timestamp = attr.integer_val;
7 else if (g_str_equal(attr.name, GEIS_GESTURE_ATTRIBUTE_FOCUS_X))
8- result->focus_x = attr.integer_val;
9+ result->focus_x = attr.float_val;
10 else if (g_str_equal(attr.name, GEIS_GESTURE_ATTRIBUTE_FOCUS_Y))
11- result->focus_y = attr.integer_val;
12+ result->focus_y = attr.float_val;
13 else if (g_str_equal (attr.name, GEIS_GESTURE_ATTRIBUTE_TOUCHES))
14 result->touches = attr.integer_val;
15 else if (g_str_equal(attr.name, GEIS_GESTURE_ATTRIBUTE_GESTURE_NAME))
16@@ -289,9 +289,9 @@
17 else if (g_str_equal(attr.name, GEIS_GESTURE_ATTRIBUTE_TIMESTAMP))
18 result->timestamp = attr.integer_val;
19 else if (g_str_equal(attr.name, GEIS_GESTURE_ATTRIBUTE_FOCUS_X))
20- result->focus_x = attr.integer_val;
21+ result->focus_x = attr.float_val;
22 else if (g_str_equal(attr.name, GEIS_GESTURE_ATTRIBUTE_FOCUS_Y))
23- result->focus_y = attr.integer_val;
24+ result->focus_y = attr.float_val;
25 else if (g_str_equal(attr.name, GEIS_GESTURE_ATTRIBUTE_TOUCHES))
26 result->touches = attr.integer_val;
27 else if (g_str_equal(attr.name, GEIS_GESTURE_ATTRIBUTE_BOUNDINGBOX_X1))
28@@ -322,9 +322,9 @@
29 else if (g_str_equal(attr.name, GEIS_GESTURE_ATTRIBUTE_TIMESTAMP))
30 result->timestamp = attr.integer_val;
31 else if (g_str_equal(attr.name, GEIS_GESTURE_ATTRIBUTE_FOCUS_X))
32- result->focus_x = attr.integer_val;
33+ result->focus_x = attr.float_val;
34 else if (g_str_equal(attr.name, GEIS_GESTURE_ATTRIBUTE_FOCUS_Y))
35- result->focus_y = attr.integer_val;
36+ result->focus_y = attr.float_val;
37 else if (g_str_equal(attr.name, GEIS_GESTURE_ATTRIBUTE_TOUCHES))
38 result->touches = attr.integer_val;
39 else if (g_str_equal(attr.name, GEIS_GESTURE_ATTRIBUTE_POSITION_X))
40@@ -367,9 +367,9 @@
41 else if (g_str_equal(attr.name, GEIS_GESTURE_ATTRIBUTE_TIMESTAMP))
42 result->timestamp = attr.integer_val;
43 else if (g_str_equal(attr.name, GEIS_GESTURE_ATTRIBUTE_FOCUS_X))
44- result->focus_x = attr.integer_val;
45+ result->focus_x = attr.float_val;
46 else if (g_str_equal(attr.name, GEIS_GESTURE_ATTRIBUTE_FOCUS_Y))
47- result->focus_y = attr.integer_val;
48+ result->focus_y = attr.float_val;
49 else if (g_str_equal(attr.name, GEIS_GESTURE_ATTRIBUTE_TOUCHES))
50 result->touches = attr.integer_val;
51 else if (g_str_equal(attr.name, GEIS_GESTURE_ATTRIBUTE_RADIUS))
52@@ -406,9 +406,9 @@
53 else if (g_str_equal(attr.name, GEIS_GESTURE_ATTRIBUTE_TIMESTAMP))
54 result->timestamp = attr.integer_val;
55 else if (g_str_equal(attr.name, GEIS_GESTURE_ATTRIBUTE_FOCUS_X))
56- result->focus_x = attr.integer_val;
57+ result->focus_x = attr.float_val;
58 else if (g_str_equal(attr.name, GEIS_GESTURE_ATTRIBUTE_FOCUS_Y))
59- result->focus_y = attr.integer_val;
60+ result->focus_y = attr.float_val;
61 else if (g_str_equal(attr.name, GEIS_GESTURE_ATTRIBUTE_TOUCHES))
62 result->touches = attr.integer_val;
63 else if (g_str_equal(attr.name, GEIS_GESTURE_ATTRIBUTE_ANGLE))
64
65=== modified file 'plugins/unityshell/src/GeisAdapter.h'
66--- plugins/unityshell/src/GeisAdapter.h 2011-08-29 23:36:21 +0000
67+++ plugins/unityshell/src/GeisAdapter.h 2012-03-29 14:28:18 +0000
68@@ -41,8 +41,8 @@
69 Window window;
70 int touches;
71 int timestamp;
72- int focus_x;
73- int focus_y;
74+ float focus_x;
75+ float focus_y;
76 int tap_length_ms;
77 float position_x;
78 float position_y;
79@@ -59,8 +59,8 @@
80 Window window;
81 int touches;
82 int timestamp;
83- int focus_x;
84- int focus_y;
85+ float focus_x;
86+ float focus_y;
87 float delta_x;
88 float delta_y;
89 float velocity_x;
90@@ -80,8 +80,8 @@
91 Window window;
92 int touches;
93 int timestamp;
94- int focus_x;
95- int focus_y;
96+ float focus_x;
97+ float focus_y;
98 float angle;
99 float angle_delta;
100 float angle_velocity;
101@@ -98,8 +98,8 @@
102 Window window;
103 int touches;
104 int timestamp;
105- int focus_x;
106- int focus_y;
107+ float focus_x;
108+ float focus_y;
109 float radius;
110 float radius_delta;
111 float radius_velocity;
112@@ -116,8 +116,8 @@
113 Window window;
114 int touches;
115 int timestamp;
116- int focus_x;
117- int focus_y;
118+ float focus_x;
119+ float focus_y;
120 float bound_x1;
121 float bound_y1;
122 float bound_x2;
123
124=== modified file 'plugins/unityshell/src/GestureEngine.cpp'
125--- plugins/unityshell/src/GestureEngine.cpp 2012-03-14 06:24:18 +0000
126+++ plugins/unityshell/src/GestureEngine.cpp 2012-03-29 14:28:18 +0000
127@@ -75,44 +75,28 @@
128 }
129 }
130
131-CompWindow*
132-GestureEngine::FindCompWindow(Window window)
133+CompWindow* GestureEngine::FindCompWindowAtPos(float fpos_x, float fpos_y)
134 {
135- CompWindow* result = _screen->findTopLevelWindow(window);
136-
137- while (!result)
138- {
139- Window parent, root;
140- Window* children = NULL;
141- unsigned int nchildren;
142- Status status;
143-
144- status = XQueryTree(_screen->dpy(), window, &root, &parent, &children, &nchildren);
145- if (status == 0)
146- break;
147-
148- if (children)
149- XFree(children);
150-
151- // parent will be zero when the window passed to this method is already the
152- // root one.
153- if (parent == root || parent == 0)
154- break;
155-
156- window = parent;
157- result = _screen->findTopLevelWindow(window);
158- }
159-
160- if (result)
161- {
162- if (!(result->type() & (CompWindowTypeUtilMask |
163- CompWindowTypeNormalMask |
164- CompWindowTypeDialogMask |
165- CompWindowTypeModalDialogMask)))
166- result = 0;
167- }
168-
169- return result;
170+ const CompWindowVector& client_list_stacking = _screen->clientList(true);
171+ if (client_list_stacking.empty())
172+ return nullptr;
173+
174+ int pos_x = fpos_x;
175+ int pos_y = fpos_y;
176+
177+ for (auto iter = client_list_stacking.rbegin(),
178+ end = client_list_stacking.rend();
179+ iter != end; ++iter)
180+ {
181+ CompWindow* window = *iter;
182+
183+ if (pos_x >= window->x() && pos_x <= (window->width() + window->x())
184+ &&
185+ pos_y >= window->y() && pos_y <= (window->height() + window->y()))
186+ return window;
187+ }
188+
189+ return nullptr;
190 }
191
192 void
193@@ -120,7 +104,7 @@
194 {
195 if (data->touches == 3)
196 {
197- _drag_window = FindCompWindow(data->window);
198+ _drag_window = FindCompWindowAtPos(data->focus_x, data->focus_y);
199
200
201 if (!_drag_window)
202@@ -222,7 +206,7 @@
203 {
204 if (data->touches == 3 && data->window != 0)
205 {
206- CompWindow* result = FindCompWindow(data->window);
207+ CompWindow* result = FindCompWindowAtPos(data->focus_x, data->focus_y);
208
209 if (result)
210 {
211@@ -256,7 +240,7 @@
212 {
213 if (data->touches == 3)
214 {
215- _pinch_window = FindCompWindow(data->window);
216+ _pinch_window = FindCompWindowAtPos(data->focus_x, data->focus_y);
217
218 if (!_pinch_window)
219 return;
220
221=== modified file 'plugins/unityshell/src/GestureEngine.h'
222--- plugins/unityshell/src/GestureEngine.h 2011-09-28 14:31:35 +0000
223+++ plugins/unityshell/src/GestureEngine.h 2012-03-29 14:28:18 +0000
224@@ -52,7 +52,7 @@
225
226 void EndDrag();
227 private:
228- CompWindow* FindCompWindow(Window window);
229+ CompWindow* FindCompWindowAtPos(float pos_x, float pos_y);
230
231 CompScreen* _screen;
232 CompWindow* _drag_window;