Mir

Merge lp:~alan-griffiths/mir/tweaks-to-raising-surfaces into lp:mir

  1. tweaks-to-raising-surfaces
  2. Merge into development-branch
Proposed by Alan Griffiths on 2015-02-24
Status: Merged
Approved by: Alan Griffiths on 2015-02-26
Approved revision: 2349
Merged at revision: 2345
Proposed branch: lp:~alan-griffiths/mir/tweaks-to-raising-surfaces
Merge into: lp:mir
Prerequisite: lp:~alan-griffiths/mir/cleanup-window-management-interfaces
Diff against target: 424 lines (+145/-15)
17 files modified
examples/server_example_basic_window_manager.h (+7/-0)
examples/server_example_canonical_window_manager.cpp (+19/-8)
examples/server_example_canonical_window_manager.h (+1/-0)
examples/server_example_generic_shell.cpp (+6/-1)
examples/server_example_generic_shell.h (+9/-0)
examples/server_example_tiling_window_manager.cpp (+1/-2)
include/server/mir/scene/surface_coordinator.h (+5/-0)
src/server/scene/surface_controller.cpp (+5/-0)
src/server/scene/surface_controller.h (+2/-0)
src/server/scene/surface_stack.cpp (+27/-1)
src/server/scene/surface_stack.h (+2/-0)
src/server/scene/surface_stack_model.h (+5/-0)
tests/include/mir_test_doubles/mock_surface_coordinator.h (+1/-0)
tests/integration-tests/test_default_shell.cpp (+9/-3)
tests/unit-tests/scene/test_application_session.cpp (+3/-0)
tests/unit-tests/scene/test_surface_controller.cpp (+1/-0)
tests/unit-tests/scene/test_surface_stack.cpp (+42/-0)
To merge this branch: bzr merge lp:~alan-griffiths/mir/tweaks-to-raising-surfaces
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve on 2015-02-26
Alexandros Frantzis (community) Approve on 2015-02-26
Andreas Pokorny (community) Approve on 2015-02-26
Robert Carr (community) 2015-02-24 Approve on 2015-02-25
Review via email: mp+250817@code.launchpad.net

Commit Message

scene, examples: enable raising multiple surfaces at a time

Description of the Change

scene, examples: enable raising multiple surfaces at a time

To post a comment you must log in.
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:2347
http://jenkins.qa.ubuntu.com/job/mir-ci/3056/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/1405
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/1404
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/1359
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1053
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1053/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1359
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1359/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/4387
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/18318

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/3056/rebuild

review: Approve (continuous-integration)
Robert Carr (robertcarr) wrote :

Cool.

review: Approve
lp:~alan-griffiths/mir/tweaks-to-raising-surfaces updated on 2015-02-26
2348. By Alan Griffiths on 2015-02-26

merge lp:mir

Andreas Pokorny (andreas-pokorny) wrote :

interesting read

+ 53 I guess the std::function cannot be replaced with auto since the right hand side depends on it.

review: Approve
Alexandros Frantzis (afrantzis) wrote :

Looks good.

Nit:
223 + std::unique_lock<decltype(guard)> ul(guard);

Could be lock_guard<>

review: Approve
lp:~alan-griffiths/mir/tweaks-to-raising-surfaces updated on 2015-02-26
2349. By Alan Griffiths on 2015-02-26

Don't use std::unique_lock where std::lock_guard is prefect

Alan Griffiths (alan-griffiths) wrote :

> interesting read
>
> + 53 I guess the std::function cannot be replaced with auto since the right
> hand side depends on it.

Yes, the type of a lambda is deduced from the whole body.

PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:2348
http://jenkins.qa.ubuntu.com/job/mir-ci/3072/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/1426
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/1425
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/1380
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1069
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1069/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1380
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1380/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/4404
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/18374

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/3072/rebuild

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'examples/server_example_basic_window_manager.h'
2--- examples/server_example_basic_window_manager.h 2015-02-23 15:04:03 +0000
3+++ examples/server_example_basic_window_manager.h 2015-02-26 10:53:47 +0000
4@@ -76,6 +76,8 @@
5
6 virtual void raise(std::weak_ptr<scene::Surface> const& surface) = 0;
7
8+ virtual void raise(SurfaceSet const& surfaces) = 0;
9+
10 virtual ~BasicWindowManagerTools() = default;
11 BasicWindowManagerTools() = default;
12 BasicWindowManagerTools(BasicWindowManagerTools const&) = delete;
13@@ -249,6 +251,11 @@
14 focus_controller->raise(surface);
15 }
16
17+ void raise(SurfaceSet const& surfaces) override
18+ {
19+ focus_controller->raise(surfaces);
20+ }
21+
22 FocusController* const focus_controller;
23 WindowManagementPolicy policy;
24
25
26=== modified file 'examples/server_example_canonical_window_manager.cpp'
27--- examples/server_example_canonical_window_manager.cpp 2015-02-23 14:19:41 +0000
28+++ examples/server_example_canonical_window_manager.cpp 2015-02-26 10:53:47 +0000
29@@ -446,14 +446,7 @@
30 case mir_surface_type_menu:
31 case mir_surface_type_inputmethod: /**< AKA "OSK" or handwriting etc. */
32 tools->set_focus_to(info_for.session.lock(), surface);
33-
34- // TODO There's currently no way to raise the active window tree while keeping
35- // TODO the order stable or consistent with spec.
36- // TODO This is definitely a frig that needs rework
37- tools->raise(surface);
38- for (auto const& child : info_for.children)
39- tools->raise(child);
40-
41+ raise_tree(surface);
42 active_surface_ = surface;
43 break;
44
45@@ -597,3 +590,21 @@
46 move_tree(child.lock(), movement);
47 }
48 }
49+
50+void me::CanonicalWindowManagerPolicy::raise_tree(std::shared_ptr<scene::Surface> const& root) const
51+{
52+ SurfaceSet surfaces;
53+ std::function<void(std::weak_ptr<scene::Surface> const& surface)> const add_children =
54+ [&,this](std::weak_ptr<scene::Surface> const& surface)
55+ {
56+ auto const& info_for = tools->info_for(surface);
57+ surfaces.insert(begin(info_for.children), end(info_for.children));
58+ for (auto const& child : info_for.children)
59+ add_children(child);
60+ };
61+
62+ surfaces.insert(root);
63+ add_children(root);
64+
65+ tools->raise(surfaces);
66+}
67
68=== modified file 'examples/server_example_canonical_window_manager.h'
69--- examples/server_example_canonical_window_manager.h 2015-02-23 14:19:41 +0000
70+++ examples/server_example_canonical_window_manager.h 2015-02-26 10:53:47 +0000
71@@ -109,6 +109,7 @@
72 bool resize(std::shared_ptr<scene::Surface> const& surface, geometry::Point cursor, geometry::Point old_cursor, geometry::Rectangle bounds);
73 bool drag(std::shared_ptr<scene::Surface> surface, geometry::Point to, geometry::Point from, geometry::Rectangle bounds);
74 void move_tree(std::shared_ptr<scene::Surface> const& root, geometry::Displacement movement) const;
75+ void raise_tree(std::shared_ptr<scene::Surface> const& root) const;
76
77 Tools* const tools;
78
79
80=== modified file 'examples/server_example_generic_shell.cpp'
81--- examples/server_example_generic_shell.cpp 2015-02-23 15:04:03 +0000
82+++ examples/server_example_generic_shell.cpp 2015-02-26 10:53:47 +0000
83@@ -145,7 +145,12 @@
84
85 void me::GenericShell::raise(std::weak_ptr<ms::Surface> const& surface)
86 {
87- return surface_coordinator->raise(surface);
88+ surface_coordinator->raise(surface);
89+}
90+
91+void me::GenericShell::raise(SurfaceSet const& surfaces)
92+{
93+ surface_coordinator->raise(surfaces);
94 }
95
96 auto me::GenericShell::focused_session() const
97
98=== modified file 'examples/server_example_generic_shell.h'
99--- examples/server_example_generic_shell.h 2015-02-23 15:04:03 +0000
100+++ examples/server_example_generic_shell.h 2015-02-26 10:53:47 +0000
101@@ -24,6 +24,8 @@
102
103 #include "mir/shell/abstract_shell.h"
104
105+#include <set>
106+
107 ///\example server_example_generic_shell.h
108 /// A generic shell that supports a window manager
109
110@@ -33,6 +35,8 @@
111
112 namespace examples
113 {
114+using SurfaceSet = std::set<std::weak_ptr<scene::Surface>, std::owner_less<std::weak_ptr<scene::Surface>>>;
115+
116 // TODO This interface keeps changes out of the Mir API (to explore the requirement)
117 class FocusController :
118 // Yes, weird - but resolves common functions (Will fix when updating core Mir API)
119@@ -52,6 +56,9 @@
120 virtual auto surface_at(geometry::Point cursor) const -> std::shared_ptr<scene::Surface> = 0;
121
122 virtual void raise(std::weak_ptr<scene::Surface> const& surface) = 0;
123+
124+ virtual void raise(SurfaceSet const& surfaces) = 0;
125+
126 private:
127 // yes clang, I mean what I said!
128 using shell::FocusController::set_focus_to;
129@@ -99,6 +106,8 @@
130
131 void raise(std::weak_ptr<scene::Surface> const& surface) override;
132
133+ void raise(SurfaceSet const& surfaces) override;
134+
135 auto focused_session() const -> std::shared_ptr<scene::Session> override;
136
137 private:
138
139=== modified file 'examples/server_example_tiling_window_manager.cpp'
140--- examples/server_example_tiling_window_manager.cpp 2015-02-24 10:10:46 +0000
141+++ examples/server_example_tiling_window_manager.cpp 2015-02-26 10:53:47 +0000
142@@ -41,8 +41,7 @@
143 {
144 }
145
146-me::TilingWindowManagerPolicy::TilingWindowManagerPolicy(
147- Tools* const tools) :
148+me::TilingWindowManagerPolicy::TilingWindowManagerPolicy(Tools* const tools) :
149 tools{tools}
150 {
151 }
152
153=== modified file 'include/server/mir/scene/surface_coordinator.h'
154--- include/server/mir/scene/surface_coordinator.h 2015-02-16 11:42:57 +0000
155+++ include/server/mir/scene/surface_coordinator.h 2015-02-26 10:53:47 +0000
156@@ -21,6 +21,7 @@
157 #define MIR_SCENE_SURFACE_COORDINATOR_H_
158
159 #include <memory>
160+#include <set>
161
162 namespace mir
163 {
164@@ -35,12 +36,16 @@
165 class SurfaceCoordinator
166 {
167 public:
168+ using SurfaceSet = std::set<std::weak_ptr<scene::Surface>, std::owner_less<std::weak_ptr<scene::Surface>>>;
169+
170 virtual std::shared_ptr<Surface> add_surface(
171 SurfaceCreationParameters const& params,
172 Session* session) = 0;
173
174 virtual void raise(std::weak_ptr<Surface> const& surface) = 0;
175
176+ virtual void raise(SurfaceSet const& surfaces) = 0;
177+
178 virtual void remove_surface(std::weak_ptr<Surface> const& surface) = 0;
179
180 virtual auto surface_at(geometry::Point) const -> std::shared_ptr<Surface> = 0;
181
182=== modified file 'src/server/scene/surface_controller.cpp'
183--- src/server/scene/surface_controller.cpp 2015-02-17 11:06:26 +0000
184+++ src/server/scene/surface_controller.cpp 2015-02-26 10:53:47 +0000
185@@ -50,6 +50,11 @@
186 surface_stack->raise(surface);
187 }
188
189+void ms::SurfaceController::raise(SurfaceSet const& surfaces)
190+{
191+ surface_stack->raise(surfaces);
192+}
193+
194 auto ms::SurfaceController::surface_at(geometry::Point cursor) const
195 -> std::shared_ptr<Surface>
196 {
197
198=== modified file 'src/server/scene/surface_controller.h'
199--- src/server/scene/surface_controller.h 2015-02-17 11:06:26 +0000
200+++ src/server/scene/surface_controller.h 2015-02-26 10:53:47 +0000
201@@ -46,6 +46,8 @@
202
203 void raise(std::weak_ptr<Surface> const& surface) override;
204
205+ void raise(SurfaceSet const& surfaces) override;
206+
207 auto surface_at(geometry::Point) const -> std::shared_ptr<Surface> override;
208
209 private:
210
211=== modified file 'src/server/scene/surface_stack.cpp'
212--- src/server/scene/surface_stack.cpp 2015-02-19 12:14:08 +0000
213+++ src/server/scene/surface_stack.cpp 2015-02-26 10:53:47 +0000
214@@ -353,6 +353,32 @@
215 BOOST_THROW_EXCEPTION(std::runtime_error("Invalid surface"));
216 }
217
218+void ms::SurfaceStack::raise(SurfaceSet const& ss)
219+{
220+ bool surfaces_reordered{false};
221+
222+ {
223+ std::lock_guard<decltype(guard)> ul(guard);
224+
225+ for (auto &layer : layers_by_depth)
226+ {
227+ auto &surfaces = layer.second;
228+
229+ auto const old_surfaces = surfaces;
230+
231+ std::stable_partition(
232+ begin(surfaces), end(surfaces),
233+ [&](std::weak_ptr<Surface> const& s) { return !ss.count(s); });
234+
235+ if (old_surfaces != surfaces)
236+ surfaces_reordered = true;
237+ }
238+ }
239+
240+ if (surfaces_reordered)
241+ observers.surfaces_reordered();
242+}
243+
244 void ms::SurfaceStack::create_rendering_tracker_for(std::shared_ptr<Surface> const& surface)
245 {
246 auto const tracker = std::make_shared<RenderingTracker>(surface);
247@@ -372,7 +398,7 @@
248
249 // Notify observer of existing surfaces
250 {
251- std::unique_lock<decltype(guard)> ul(guard);
252+ std::lock_guard<decltype(guard)> ul(guard);
253 for (auto &layer : layers_by_depth)
254 {
255 for (auto &surface : layer.second)
256
257=== modified file 'src/server/scene/surface_stack.h'
258--- src/server/scene/surface_stack.h 2015-02-16 11:28:18 +0000
259+++ src/server/scene/surface_stack.h 2015-02-26 10:53:47 +0000
260@@ -84,6 +84,8 @@
261
262 virtual void raise(std::weak_ptr<Surface> const& surface) override;
263
264+ void raise(SurfaceSet const& surfaces) override;
265+
266 void add_surface(
267 std::shared_ptr<Surface> const& surface,
268 DepthId depth,
269
270=== modified file 'src/server/scene/surface_stack_model.h'
271--- src/server/scene/surface_stack_model.h 2015-02-16 11:28:18 +0000
272+++ src/server/scene/surface_stack_model.h 2015-02-26 10:53:47 +0000
273@@ -24,6 +24,7 @@
274 #include "mir/input/input_reception_mode.h"
275
276 #include <memory>
277+#include <set>
278
279 namespace mir
280 {
281@@ -36,6 +37,8 @@
282 class SurfaceStackModel
283 {
284 public:
285+ using SurfaceSet = std::set<std::weak_ptr<Surface>, std::owner_less<std::weak_ptr<Surface>>>;
286+
287 virtual ~SurfaceStackModel() = default;
288
289 virtual void add_surface(
290@@ -47,6 +50,8 @@
291
292 virtual void raise(std::weak_ptr<Surface> const& surface) = 0;
293
294+ virtual void raise(SurfaceSet const& surfaces) = 0;
295+
296 virtual auto surface_at(geometry::Point) const -> std::shared_ptr<Surface> = 0;
297
298 protected:
299
300=== modified file 'tests/include/mir_test_doubles/mock_surface_coordinator.h'
301--- tests/include/mir_test_doubles/mock_surface_coordinator.h 2015-02-17 11:06:26 +0000
302+++ tests/include/mir_test_doubles/mock_surface_coordinator.h 2015-02-26 10:53:47 +0000
303@@ -35,6 +35,7 @@
304 struct MockSurfaceCoordinator : public scene::SurfaceCoordinator
305 {
306 MOCK_METHOD1(raise, void(std::weak_ptr<scene::Surface> const&));
307+ MOCK_METHOD1(raise, void(scene::SurfaceCoordinator::SurfaceSet const&));
308
309 MOCK_METHOD2(add_surface, std::shared_ptr<scene::Surface>(
310 scene::SurfaceCreationParameters const& params,
311
312=== modified file 'tests/integration-tests/test_default_shell.cpp'
313--- tests/integration-tests/test_default_shell.cpp 2015-02-13 06:12:34 +0000
314+++ tests/integration-tests/test_default_shell.cpp 2015-02-26 10:53:47 +0000
315@@ -88,6 +88,12 @@
316 std::make_shared<NullPlacementStrategy>(),
317 mt::fake_shared(surface_configurator)};
318 };
319+
320+inline auto AnySurface()
321+-> Matcher<std::weak_ptr<ms::Surface> const&>
322+{
323+ return Matcher<std::weak_ptr<ms::Surface> const&>(_);
324+}
325 }
326
327 TEST_F(TestDefaultShellAndFocusSelectionStrategy, cycle_focus)
328@@ -156,9 +162,9 @@
329 ON_CALL(app2, default_surface()).WillByDefault(Return(mock_surface2));
330
331 InSequence seq;
332- EXPECT_CALL(surface_coordinator, raise(_)).Times(1);
333+ EXPECT_CALL(surface_coordinator, raise(AnySurface())).Times(1);
334 EXPECT_CALL(*mock_surface1, configure(mir_surface_attrib_focus, mir_surface_focused)).Times(1);
335- EXPECT_CALL(surface_coordinator, raise(_)).Times(1);
336+ EXPECT_CALL(surface_coordinator, raise(AnySurface())).Times(1);
337 EXPECT_CALL(*mock_surface1, configure(mir_surface_attrib_focus, mir_surface_unfocused)).Times(1);
338 EXPECT_CALL(*mock_surface2, configure(mir_surface_attrib_focus, mir_surface_focused)).Times(1);
339
340@@ -187,7 +193,7 @@
341 // When we have no session.
342 EXPECT_CALL(input_targeter, focus_cleared()).Times(1);
343 }
344- EXPECT_CALL(surface_coordinator, raise(_)).Times(1);
345+ EXPECT_CALL(surface_coordinator, raise(AnySurface())).Times(1);
346
347 shell.set_focus_to(mt::fake_shared(app1));
348 shell.set_focus_to(mt::fake_shared(app1));
349
350=== modified file 'tests/unit-tests/scene/test_application_session.cpp'
351--- tests/unit-tests/scene/test_application_session.cpp 2015-02-17 11:06:26 +0000
352+++ tests/unit-tests/scene/test_application_session.cpp 2015-02-26 10:53:47 +0000
353@@ -89,6 +89,9 @@
354 void raise(std::weak_ptr<ms::Surface> const&) override
355 {
356 }
357+ void raise(SurfaceSet const&) override
358+ {
359+ }
360 std::shared_ptr<ms::Surface> add_surface(ms::SurfaceCreationParameters const&,
361 ms::Session*) override
362 {
363
364=== modified file 'tests/unit-tests/scene/test_surface_controller.cpp'
365--- tests/unit-tests/scene/test_surface_controller.cpp 2015-02-17 11:06:26 +0000
366+++ tests/unit-tests/scene/test_surface_controller.cpp 2015-02-26 10:53:47 +0000
367@@ -53,6 +53,7 @@
368 MOCK_METHOD1(remove_surface, void(std::weak_ptr<ms::Surface> const&));
369 MOCK_METHOD1(raise, void(std::weak_ptr<ms::Surface> const&));
370 MOCK_CONST_METHOD1(surface_at, std::shared_ptr<ms::Surface>(geom::Point));
371+ void raise(SurfaceSet const&) override {}
372 };
373
374 struct SurfaceController : testing::Test
375
376=== modified file 'tests/unit-tests/scene/test_surface_stack.cpp'
377--- tests/unit-tests/scene/test_surface_stack.cpp 2015-02-17 16:40:39 +0000
378+++ tests/unit-tests/scene/test_surface_stack.cpp 2015-02-26 10:53:47 +0000
379@@ -929,3 +929,45 @@
380 EXPECT_THAT(stack.surface_at(cursor_over_none).get(), IsNull());
381 }
382
383+TEST_F(SurfaceStack, raise_surfaces_to_top)
384+{
385+ stack.add_surface(stub_surface1, default_params.depth, default_params.input_mode);
386+ stack.add_surface(stub_surface2, default_params.depth, default_params.input_mode);
387+ stack.add_surface(stub_surface3, default_params.depth, default_params.input_mode);
388+
389+ NiceMock<MockSceneObserver> observer;
390+ stack.add_observer(mt::fake_shared(observer));
391+
392+ EXPECT_CALL(observer, surfaces_reordered()).Times(1);
393+
394+ stack.raise({stub_surface1, stub_surface3});
395+ EXPECT_THAT(
396+ stack.scene_elements_for(compositor_id),
397+ ElementsAre(
398+ SceneElementFor(stub_surface2),
399+ SceneElementFor(stub_surface1),
400+ SceneElementFor(stub_surface3)));
401+
402+ Mock::VerifyAndClearExpectations(&observer);
403+ EXPECT_CALL(observer, surfaces_reordered()).Times(1);
404+
405+ stack.raise({stub_surface2, stub_surface3});
406+ EXPECT_THAT(
407+ stack.scene_elements_for(compositor_id),
408+ ElementsAre(
409+ SceneElementFor(stub_surface1),
410+ SceneElementFor(stub_surface2),
411+ SceneElementFor(stub_surface3)));
412+
413+ Mock::VerifyAndClearExpectations(&observer);
414+ EXPECT_CALL(observer, surfaces_reordered()).Times(0);
415+
416+ stack.raise({stub_surface2, stub_surface1, stub_surface3});
417+ EXPECT_THAT(
418+ stack.scene_elements_for(compositor_id),
419+ ElementsAre(
420+ SceneElementFor(stub_surface1),
421+ SceneElementFor(stub_surface2),
422+ SceneElementFor(stub_surface3)));
423+}
424+

Subscribers

People subscribed via source and target branches