Mir

Merge lp:~alan-griffiths/mir/fix-1447886 into lp:mir

Proposed by Alan Griffiths on 2016-04-28
Status: Merged
Approved by: Alan Griffiths on 2016-04-29
Approved revision: 3483
Merged at revision: 3488
Proposed branch: lp:~alan-griffiths/mir/fix-1447886
Merge into: lp:mir
Diff against target: 264 lines (+98/-38)
4 files modified
examples/server_example_canonical_window_manager.cpp (+45/-19)
examples/server_example_canonical_window_manager.h (+4/-0)
src/include/server/mir/shell/canonical_window_manager.h (+4/-0)
src/server/shell/canonical_window_manager.cpp (+45/-19)
To merge this branch: bzr merge lp:~alan-griffiths/mir/fix-1447886
Reviewer Review Type Date Requested Status
Daniel van Vugt Approve on 2016-04-29
Cemil Azizoglu (community) 2016-04-28 Approve on 2016-04-28
Mir CI Bot continuous-integration Approve on 2016-04-28
Review via email: mp+293223@code.launchpad.net

Commit message

Update the resize logic in CanonicalWindowManagerPolicy following the discussion on lp:1447886

Description of the change

Update the resize logic in CanonicalWindowManagerPolicy following the discussion on lp:1447886

It now looks at the gesture as a whole and only does position checking at the start of the resize gesture. Thereafter the gesture continues until the button is released, regardless of whether the cursor is still inside the surface/titlebar.

To post a comment you must log in.
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3483
https://mir-jenkins.ubuntu.com/job/mir-ci/910/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/956
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/997
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/988
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/988
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/966
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/966/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/966
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/966/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/966
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/966/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/966
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/966/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/966
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/966/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/910/rebuild

review: Approve (continuous-integration)
Cemil Azizoglu (cemil-azizoglu) wrote :

lgtm

review: Approve
Daniel van Vugt (vanvugt) wrote :

Feels like an improvement. I don't want to be a blocker on this for too long, but things worth noting:

New bug: Dragging a corner past the window's minimum size and back again results in the cursor losing its original position within the window. The cursor keeping the same relative position within the window for the duration of the resize feels most professional and least frustrating. Just try it in Compiz/Unity7 to get a feel for what should happen. Admittedly mir_proving_server has the same bug right now.

Old bug: I just noticed you're dividing the window into quarters instead of thirds. So grabbing a window from the middle of an edge incorrectly resizes two axes instead of one. See Compiz/Unity or mir_proving_server for examples of what should happen. This one would represent a regression for mir_proving_server, but also isn't the bug being fixed here. Just a recommended prerequisite before abolishing DefaultWindowManager.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'examples/server_example_canonical_window_manager.cpp'
2--- examples/server_example_canonical_window_manager.cpp 2016-04-15 03:41:28 +0000
3+++ examples/server_example_canonical_window_manager.cpp 2016-04-28 09:10:37 +0000
4@@ -90,7 +90,7 @@
5
6 void me::CanonicalWindowManagerPolicyCopy::resize(Point cursor)
7 {
8- select_active_surface(tools->surface_at(old_cursor));
9+ if (!resizing) select_active_surface(tools->surface_at(old_cursor));
10 resize(active_surface(), cursor, old_cursor, display_area);
11 }
12
13@@ -733,6 +733,7 @@
14 mir_pointer_event_axis_value(event, mir_pointer_axis_y)};
15
16 bool consumes_event = false;
17+ bool resize_event = false;
18
19 if (action == mir_pointer_action_button_down)
20 {
21@@ -750,6 +751,7 @@
22 if (mir_pointer_event_button_state(event, mir_pointer_button_tertiary))
23 {
24 resize(cursor);
25+ resize_event = active_surface_.lock().get();
26 consumes_event = true;
27 }
28 }
29@@ -768,6 +770,7 @@
30 }
31 }
32
33+ resizing = resize_event;
34 old_cursor = cursor;
35 return consumes_event;
36 }
37@@ -852,40 +855,63 @@
38
39 bool me::CanonicalWindowManagerPolicyCopy::resize(std::shared_ptr<ms::Surface> const& surface, Point cursor, Point old_cursor, Rectangle bounds)
40 {
41- if (!surface || !surface->input_area_contains(old_cursor))
42+ if (!surface)
43 return false;
44
45+ auto const& surface_info = tools->info_for(surface);
46+
47 auto const top_left = surface->top_left();
48 Rectangle const old_pos{top_left, surface->size()};
49
50- auto anchor = top_left;
51-
52- for (auto const& corner : {
53- old_pos.top_right(),
54- old_pos.bottom_left(),
55- old_pos.bottom_right()})
56+ if (!resizing)
57 {
58- if ((old_cursor - anchor).length_squared() <
59- (old_cursor - corner).length_squared())
60+ auto anchor = old_pos.bottom_right();
61+
62+ for (auto const& corner : {
63+ old_pos.top_right(),
64+ old_pos.bottom_left(),
65+ top_left})
66 {
67- anchor = corner;
68+ if ((old_cursor - anchor).length_squared() <
69+ (old_cursor - corner).length_squared())
70+ {
71+ anchor = corner;
72+ }
73 }
74+
75+ left_resize = anchor.x != top_left.x;
76+ top_resize = anchor.y != top_left.y;
77 }
78
79- bool const left_resize = anchor.x != top_left.x;
80- bool const top_resize = anchor.y != top_left.y;
81 int const x_sign = left_resize? -1 : 1;
82 int const y_sign = top_resize? -1 : 1;
83
84- auto const delta = cursor-old_cursor;
85-
86- Size new_size{old_pos.size.width + x_sign*delta.dx, old_pos.size.height + y_sign*delta.dy};
87-
88+ auto delta = cursor-old_cursor;
89+
90+ auto new_width = old_pos.size.width + x_sign * delta.dx;
91+ auto new_height = old_pos.size.height + y_sign * delta.dy;
92+
93+ auto const min_width = std::max(surface_info.min_width, Width{5});
94+ auto const min_height = std::max(surface_info.min_height, Height{5});
95+
96+ if (new_width < min_width)
97+ {
98+ new_width = min_width;
99+ if (delta.dx > DeltaX{0})
100+ delta.dx = DeltaX{0};
101+ }
102+
103+ if (new_height < min_height)
104+ {
105+ new_height = min_height;
106+ if (delta.dy > DeltaY{0})
107+ delta.dy = DeltaY{0};
108+ }
109+
110+ Size new_size{new_width, new_height};
111 Point new_pos = top_left + left_resize*delta.dx + top_resize*delta.dy;
112
113
114- auto const& surface_info = tools->info_for(surface);
115-
116 surface_info.constrain_resize(surface, new_pos, new_size, left_resize, top_resize, bounds);
117
118 apply_resize(surface, surface_info.titlebar, new_pos, new_size);
119
120=== modified file 'examples/server_example_canonical_window_manager.h'
121--- examples/server_example_canonical_window_manager.h 2016-04-15 03:41:28 +0000
122+++ examples/server_example_canonical_window_manager.h 2016-04-28 09:10:37 +0000
123@@ -125,6 +125,10 @@
124 using FullscreenSurfaces = std::set<std::weak_ptr<scene::Surface>, std::owner_less<std::weak_ptr<scene::Surface>>>;
125
126 FullscreenSurfaces fullscreen_surfaces;
127+
128+ bool resizing = false;
129+ bool left_resize = false;
130+ bool top_resize = false;
131 };
132 }
133 }
134
135=== modified file 'src/include/server/mir/shell/canonical_window_manager.h'
136--- src/include/server/mir/shell/canonical_window_manager.h 2016-03-23 06:39:56 +0000
137+++ src/include/server/mir/shell/canonical_window_manager.h 2016-04-28 09:10:37 +0000
138@@ -110,6 +110,10 @@
139 using FullscreenSurfaces = std::set<std::weak_ptr<scene::Surface>, std::owner_less<std::weak_ptr<scene::Surface>>>;
140
141 FullscreenSurfaces fullscreen_surfaces;
142+
143+ bool resizing = false;
144+ bool left_resize = false;
145+ bool top_resize = false;
146 };
147
148 using CanonicalWindowManager = WindowManagerConstructor<CanonicalWindowManagerPolicy>;
149
150=== modified file 'src/server/shell/canonical_window_manager.cpp'
151--- src/server/shell/canonical_window_manager.cpp 2016-04-05 11:59:12 +0000
152+++ src/server/shell/canonical_window_manager.cpp 2016-04-28 09:10:37 +0000
153@@ -113,7 +113,7 @@
154
155 void msh::CanonicalWindowManagerPolicy::resize(Point cursor)
156 {
157- select_active_surface(tools->surface_at(old_cursor));
158+ if (!resizing) select_active_surface(tools->surface_at(old_cursor));
159 resize(active_surface(), cursor, old_cursor, display_area);
160 old_cursor = cursor;
161 }
162@@ -703,6 +703,7 @@
163 mir_pointer_event_axis_value(event, mir_pointer_axis_y)};
164
165 bool consumes_event = false;
166+ bool resize_event = false;
167
168 if (action == mir_pointer_action_button_down)
169 {
170@@ -720,10 +721,12 @@
171 if (mir_pointer_event_button_state(event, mir_pointer_button_tertiary))
172 {
173 resize(cursor);
174+ resize_event = active_surface_.lock().get();
175 consumes_event = true;
176 }
177 }
178
179+ resizing = resize_event;
180 old_cursor = cursor;
181 return consumes_event;
182 }
183@@ -786,40 +789,63 @@
184
185 bool msh::CanonicalWindowManagerPolicy::resize(std::shared_ptr<ms::Surface> const& surface, Point cursor, Point old_cursor, Rectangle bounds)
186 {
187- if (!surface || !surface->input_area_contains(old_cursor))
188+ if (!surface)
189 return false;
190
191+ auto const& surface_info = tools->info_for(surface);
192+
193 auto const top_left = surface->top_left();
194 Rectangle const old_pos{top_left, surface->size()};
195
196- auto anchor = top_left;
197-
198- for (auto const& corner : {
199- old_pos.top_right(),
200- old_pos.bottom_left(),
201- old_pos.bottom_right()})
202+ if (!resizing)
203 {
204- if ((old_cursor - anchor).length_squared() <
205- (old_cursor - corner).length_squared())
206+ auto anchor = old_pos.bottom_right();
207+
208+ for (auto const& corner : {
209+ old_pos.top_right(),
210+ old_pos.bottom_left(),
211+ top_left})
212 {
213- anchor = corner;
214+ if ((old_cursor - anchor).length_squared() <
215+ (old_cursor - corner).length_squared())
216+ {
217+ anchor = corner;
218+ }
219 }
220+
221+ left_resize = anchor.x != top_left.x;
222+ top_resize = anchor.y != top_left.y;
223 }
224
225- bool const left_resize = anchor.x != top_left.x;
226- bool const top_resize = anchor.y != top_left.y;
227 int const x_sign = left_resize? -1 : 1;
228 int const y_sign = top_resize? -1 : 1;
229
230- auto const delta = cursor-old_cursor;
231-
232- Size new_size{old_pos.size.width + x_sign*delta.dx, old_pos.size.height + y_sign*delta.dy};
233-
234+ auto delta = cursor-old_cursor;
235+
236+ auto new_width = old_pos.size.width + x_sign * delta.dx;
237+ auto new_height = old_pos.size.height + y_sign * delta.dy;
238+
239+ auto const min_width = std::max(surface_info.min_width, Width{5});
240+ auto const min_height = std::max(surface_info.min_height, Height{5});
241+
242+ if (new_width < min_width)
243+ {
244+ new_width = min_width;
245+ if (delta.dx > DeltaX{0})
246+ delta.dx = DeltaX{0};
247+ }
248+
249+ if (new_height < min_height)
250+ {
251+ new_height = min_height;
252+ if (delta.dy > DeltaY{0})
253+ delta.dy = DeltaY{0};
254+ }
255+
256+ Size new_size{new_width, new_height};
257 Point new_pos = top_left + left_resize*delta.dx + top_resize*delta.dy;
258
259
260- auto const& surface_info = tools->info_for(surface);
261-
262 surface_info.constrain_resize(surface, new_pos, new_size, left_resize, top_resize, bounds);
263
264 apply_resize(surface, new_pos, new_size);

Subscribers

People subscribed via source and target branches