Mir

Merge lp:~alan-griffiths/mir/death-to-SurfaceConfigurator into lp:mir

Proposed by Alan Griffiths on 2015-03-31
Status: Merged
Approved by: Alan Griffiths on 2015-04-01
Approved revision: 2446
Merged at revision: 2448
Proposed branch: lp:~alan-griffiths/mir/death-to-SurfaceConfigurator
Merge into: lp:mir
Diff against target: 462 lines (+9/-245)
14 files modified
playground/demo-shell/demo_shell.cpp (+1/-2)
src/include/server/mir/default_server_configuration.h (+0/-3)
src/include/server/mir/scene/surface_configurator.h (+0/-54)
src/include/server/mir/shell/default_window_manager.h (+2/-4)
src/server/default_server_configuration.cpp (+0/-20)
src/server/scene/surface_allocator.h (+0/-1)
src/server/shell/default_window_manager.cpp (+3/-12)
tests/include/mir_test_doubles/mock_surface_configurator.h (+0/-43)
tests/include/mir_test_doubles/null_surface_configurator.h (+0/-48)
tests/integration-tests/test_default_window_manager.cpp (+1/-51)
tests/integration-tests/test_surface_stack_with_compositor.cpp (+0/-1)
tests/mir_test_framework/fake_event_hub_server_configuration.cpp (+1/-2)
tests/unit-tests/scene/test_surface.cpp (+0/-1)
tests/unit-tests/shell/test_default_window_manager.cpp (+1/-3)
To merge this branch: bzr merge lp:~alan-griffiths/mir/death-to-SurfaceConfigurator
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve on 2015-04-01
Daniel van Vugt 2015-03-31 Approve on 2015-04-01
Alexandros Frantzis (community) Approve on 2015-04-01
Review via email: mp+254796@code.launchpad.net

Commit message

scene: Remove the useless (and unused) SurfaceConfigurator customization point

Description of the change

scene: Remove the useless (and unused) SurfaceConfigurator customization point

To post a comment you must log in.
Daniel van Vugt (vanvugt) wrote :

Cool.

I implemented and almost proposed this a while back, but then found it was used downstream somewhere. Are all downstream users weaned off it yet?

review: Needs Information
Alexandros Frantzis (afrantzis) wrote :

Looks good. A quick grep shows a reference to the_surface_configurator in qtmir, but it's just a using declaration and the introduced name does not seem to be used, so it should be safe to just delete the declaration.

review: Approve
Alan Griffiths (alan-griffiths) wrote :

> Cool.
>
> I implemented and almost proposed this a while back, but then found it was
> used downstream somewhere. Are all downstream users weaned off it yet?

Nearly: https://code.launchpad.net/~mir-team/qtmir/compatibility-with-mir-API-changes/+merge/252589 line 161

Daniel van Vugt (vanvugt) :
review: Approve
2446. By Alan Griffiths on 2015-04-01

merge lp:mir

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'playground/demo-shell/demo_shell.cpp'
2--- playground/demo-shell/demo_shell.cpp 2015-03-31 14:05:53 +0000
3+++ playground/demo-shell/demo_shell.cpp 2015-04-01 10:12:47 +0000
4@@ -107,8 +107,7 @@
5 { return std::make_shared<shell::DefaultWindowManager>(
6 focus_controller,
7 the_placement_strategy(),
8- the_session_coordinator(),
9- the_surface_configurator()); };
10+ the_session_coordinator()); };
11 }
12
13 std::shared_ptr<msh::HostLifecycleEventListener> the_host_lifecycle_event_listener() override
14
15=== modified file 'src/include/server/mir/default_server_configuration.h'
16--- src/include/server/mir/default_server_configuration.h 2015-03-31 02:35:42 +0000
17+++ src/include/server/mir/default_server_configuration.h 2015-04-01 10:12:47 +0000
18@@ -104,7 +104,6 @@
19 class SessionCoordinator;
20 class SnapshotStrategy;
21 class SurfaceCoordinator;
22-class SurfaceConfigurator;
23 class SurfaceStackModel;
24 class SurfaceStack;
25 class SceneReport;
26@@ -283,7 +282,6 @@
27 virtual std::shared_ptr<scene::SurfaceStackModel> the_surface_stack_model();
28 virtual std::shared_ptr<scene::SurfaceFactory> the_surface_factory();
29 virtual std::shared_ptr<scene::SurfaceCoordinator>the_surface_coordinator();
30- virtual std::shared_ptr<scene::SurfaceConfigurator> the_surface_configurator();
31 /** @} */
32
33 /** @name scene configuration - dependencies
34@@ -424,7 +422,6 @@
35 CachedPtr<scene::PixelBuffer> pixel_buffer;
36 CachedPtr<scene::SnapshotStrategy> snapshot_strategy;
37 CachedPtr<shell::DisplayLayout> shell_display_layout;
38- CachedPtr<scene::SurfaceConfigurator> surface_configurator;
39 CachedPtr<compositor::DisplayBufferCompositorFactory> display_buffer_compositor_factory;
40 CachedPtr<compositor::Compositor> compositor;
41 CachedPtr<compositor::CompositorReport> compositor_report;
42
43=== removed file 'src/include/server/mir/scene/surface_configurator.h'
44--- src/include/server/mir/scene/surface_configurator.h 2015-03-24 15:18:19 +0000
45+++ src/include/server/mir/scene/surface_configurator.h 1970-01-01 00:00:00 +0000
46@@ -1,54 +0,0 @@
47-/*
48- * Copyright © 2013 Canonical Ltd.
49- *
50- * This program is free software: you can redistribute it and/or modify it
51- * under the terms of the GNU General Public License version 3,
52- * as published by the Free Software Foundation.
53- *
54- * This program is distributed in the hope that it will be useful,
55- * but WITHOUT ANY WARRANTY; without even the implied warranty of
56- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
57- * GNU General Public License for more details.
58- *
59- * You should have received a copy of the GNU General Public License
60- * along with this program. If not, see <http://www.gnu.org/licenses/>.
61- *
62- * Authored by: Robert Carr <robert.carr@canonical.com>
63- */
64-
65-#ifndef MIR_SCENE_SURFACE_CONFIGURATOR_H_
66-#define MIR_SCENE_SURFACE_CONFIGURATOR_H_
67-
68-#include "mir_toolkit/common.h"
69-
70-#include <memory>
71-
72-namespace mir
73-{
74-
75-namespace scene
76-{
77-class Surface;
78-
79-class SurfaceConfigurator
80-{
81-public:
82- virtual ~SurfaceConfigurator() = default;
83-
84- /// Returns the selected value.
85- virtual int select_attribute_value(Surface const& surface, MirSurfaceAttrib attrib,
86- int requested_value) = 0;
87-
88- virtual void attribute_set(Surface const& surface, MirSurfaceAttrib attrib,
89- int new_value) = 0;
90-
91-protected:
92- SurfaceConfigurator() = default;
93- SurfaceConfigurator(SurfaceConfigurator const&) = delete;
94- SurfaceConfigurator& operator=(SurfaceConfigurator const&) = delete;
95-};
96-
97-}
98-} // namespace mir
99-
100-#endif // MIR_SCENE_SURFACE_CONFIGURATOR_H_
101
102=== modified file 'src/include/server/mir/shell/default_window_manager.h'
103--- src/include/server/mir/shell/default_window_manager.h 2015-03-31 11:18:41 +0000
104+++ src/include/server/mir/shell/default_window_manager.h 2015-04-01 10:12:47 +0000
105@@ -23,7 +23,7 @@
106
107 namespace mir
108 {
109-namespace scene { class PlacementStrategy; class SurfaceConfigurator; class SessionCoordinator; }
110+namespace scene { class PlacementStrategy; class SessionCoordinator; }
111
112 namespace shell
113 {
114@@ -34,8 +34,7 @@
115 public:
116 explicit DefaultWindowManager(FocusController* focus_controller,
117 std::shared_ptr<scene::PlacementStrategy> const& placement_strategy,
118- std::shared_ptr<scene::SessionCoordinator> const& session_coordinator,
119- std::shared_ptr<scene::SurfaceConfigurator> const& surface_configurator);
120+ std::shared_ptr<scene::SessionCoordinator> const& session_coordinator);
121
122 void add_session(std::shared_ptr<scene::Session> const& session) override;
123
124@@ -70,7 +69,6 @@
125 FocusController* const focus_controller;
126 std::shared_ptr<scene::PlacementStrategy> const placement_strategy;
127 std::shared_ptr<scene::SessionCoordinator> const session_coordinator;
128- std::shared_ptr<scene::SurfaceConfigurator> const surface_configurator;
129 };
130 }
131 }
132
133=== modified file 'src/server/default_server_configuration.cpp'
134--- src/server/default_server_configuration.cpp 2015-01-21 07:34:50 +0000
135+++ src/server/default_server_configuration.cpp 2015-04-01 10:12:47 +0000
136@@ -29,7 +29,6 @@
137 #include "mir/options/program_option.h"
138 #include "mir/frontend/session_credentials.h"
139 #include "mir/frontend/session_authorizer.h"
140-#include "mir/scene/surface_configurator.h"
141 #include "mir/graphics/cursor.h"
142 #include "mir/scene/null_session_listener.h"
143 #include "mir/graphics/display.h"
144@@ -101,25 +100,6 @@
145 });
146 }
147
148-std::shared_ptr<ms::SurfaceConfigurator> mir::DefaultServerConfiguration::the_surface_configurator()
149-{
150- struct DefaultSurfaceConfigurator : public ms::SurfaceConfigurator
151- {
152- int select_attribute_value(ms::Surface const&, MirSurfaceAttrib, int requested_value)
153- {
154- return requested_value;
155- }
156- void attribute_set(ms::Surface const&, MirSurfaceAttrib, int)
157- {
158- }
159- };
160- return surface_configurator(
161- [this]()
162- {
163- return std::make_shared<DefaultSurfaceConfigurator>();
164- });
165-}
166-
167 std::shared_ptr<mf::SessionAuthorizer>
168 mir::DefaultServerConfiguration::the_session_authorizer()
169 {
170
171=== modified file 'src/server/scene/surface_allocator.h'
172--- src/server/scene/surface_allocator.h 2015-03-31 02:35:42 +0000
173+++ src/server/scene/surface_allocator.h 2015-04-01 10:12:47 +0000
174@@ -36,7 +36,6 @@
175 {
176 class BufferStreamFactory;
177 class SceneReport;
178-class SurfaceConfigurator;
179
180 class SurfaceAllocator : public SurfaceFactory
181 {
182
183=== modified file 'src/server/shell/default_window_manager.cpp'
184--- src/server/shell/default_window_manager.cpp 2015-03-31 11:18:41 +0000
185+++ src/server/shell/default_window_manager.cpp 2015-04-01 10:12:47 +0000
186@@ -23,7 +23,6 @@
187 #include "mir/scene/session.h"
188 #include "mir/scene/session_coordinator.h"
189 #include "mir/scene/surface.h"
190-#include "mir/scene/surface_configurator.h"
191 #include "mir/scene/surface_creation_parameters.h"
192 #include "mir/shell/focus_controller.h"
193
194@@ -34,12 +33,10 @@
195 msh::DefaultWindowManager::DefaultWindowManager(
196 FocusController* focus_controller,
197 std::shared_ptr<scene::PlacementStrategy> const& placement_strategy,
198- std::shared_ptr<scene::SessionCoordinator> const& session_coordinator,
199- std::shared_ptr<scene::SurfaceConfigurator> const& surface_configurator) :
200+ std::shared_ptr<scene::SessionCoordinator> const& session_coordinator) :
201 focus_controller{focus_controller},
202 placement_strategy{placement_strategy},
203- session_coordinator{session_coordinator},
204- surface_configurator{surface_configurator}
205+ session_coordinator{session_coordinator}
206 {
207 }
208
209@@ -138,11 +135,5 @@
210 MirSurfaceAttrib attrib,
211 int value)
212 {
213- auto const configured_value = surface_configurator->select_attribute_value(*surface, attrib, value);
214-
215- auto const result = surface->configure(attrib, configured_value);
216-
217- surface_configurator->attribute_set(*surface, attrib, result);
218-
219- return result;
220+ return surface->configure(attrib, value);
221 }
222
223=== removed file 'tests/include/mir_test_doubles/mock_surface_configurator.h'
224--- tests/include/mir_test_doubles/mock_surface_configurator.h 2015-01-21 07:34:50 +0000
225+++ tests/include/mir_test_doubles/mock_surface_configurator.h 1970-01-01 00:00:00 +0000
226@@ -1,43 +0,0 @@
227-/*
228- * Copyright © 2013 Canonical Ltd.
229- *
230- * This program is free software: you can redistribute it and/or modify it
231- * under the terms of the GNU General Public License version 3,
232- * as published by the Free Software Foundation.
233- *
234- * This program is distributed in the hope that it will be useful,
235- * but WITHOUT ANY WARRANTY; without even the implied warranty of
236- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
237- * GNU General Public License for more details.
238- *
239- * You should have received a copy of the GNU General Public License
240- * along with this program. If not, see <http://www.gnu.org/licenses/>.
241- *
242- * Authored by: Robert Carr <robert.carr@canonical.com>
243- */
244-
245-#ifndef MIR_TEST_DOUBLES_MOCK_SURFACE_CONFIGURATOR_H_
246-#define MIR_TEST_DOUBLES_MOCK_SURFACE_CONFIGURATOR_H_
247-
248-#include "mir/scene/surface_configurator.h"
249-
250-#include <gmock/gmock.h>
251-
252-namespace mir
253-{
254-namespace test
255-{
256-namespace doubles
257-{
258-
259-struct MockSurfaceConfigurator : public scene::SurfaceConfigurator
260-{
261- MOCK_METHOD3(select_attribute_value, int(scene::Surface const&, MirSurfaceAttrib, int));
262- MOCK_METHOD3(attribute_set, void(scene::Surface const&, MirSurfaceAttrib, int));
263-};
264-
265-}
266-}
267-}
268-
269-#endif /* MIR_TEST_DOUBLES_MOCK_SURFACE_CONFIGURATOR_H_ */
270
271=== removed file 'tests/include/mir_test_doubles/null_surface_configurator.h'
272--- tests/include/mir_test_doubles/null_surface_configurator.h 2015-01-21 07:34:50 +0000
273+++ tests/include/mir_test_doubles/null_surface_configurator.h 1970-01-01 00:00:00 +0000
274@@ -1,48 +0,0 @@
275-/*
276- * Copyright © 2013 Canonical Ltd.
277- *
278- * This program is free software: you can redistribute it and/or modify it
279- * under the terms of the GNU General Public License version 3,
280- * as published by the Free Software Foundation.
281- *
282- * This program is distributed in the hope that it will be useful,
283- * but WITHOUT ANY WARRANTY; without even the implied warranty of
284- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
285- * GNU General Public License for more details.
286- *
287- * You should have received a copy of the GNU General Public License
288- * along with this program. If not, see <http://www.gnu.org/licenses/>.
289- *
290- * Authored by: Robert Carr <robert.carr@canonical.com>
291- */
292-
293-#ifndef MIR_TEST_DOUBLES_NULL_SURFACE_CONFIGURATOR_H_
294-#define MIR_TEST_DOUBLES_NULL_SURFACE_CONFIGURATOR_H_
295-
296-#include "mir/scene/surface_configurator.h"
297-
298-namespace mir
299-{
300-namespace test
301-{
302-namespace doubles
303-{
304-
305-struct NullSurfaceConfigurator : public scene::SurfaceConfigurator
306-{
307- int select_attribute_value(scene::Surface const&,
308- MirSurfaceAttrib, int requested_value)
309- {
310- return requested_value;
311- }
312- void attribute_set(scene::Surface const&,
313- MirSurfaceAttrib, int)
314- {
315- }
316-};
317-
318-}
319-}
320-}
321-
322-#endif /* MIR_TEST_DOUBLES_NULL_SURFACE_CONFIGURATOR_H_ */
323
324=== modified file 'tests/integration-tests/test_default_window_manager.cpp'
325--- tests/integration-tests/test_default_window_manager.cpp 2015-03-31 11:18:41 +0000
326+++ tests/integration-tests/test_default_window_manager.cpp 2015-04-01 10:12:47 +0000
327@@ -33,7 +33,6 @@
328 #include "mir_test_doubles/null_snapshot_strategy.h"
329 #include "mir_test_doubles/null_event_sink.h"
330 #include "mir_test_doubles/null_session_event_sink.h"
331-#include "mir_test_doubles/mock_surface_configurator.h"
332 #include "mir_test_doubles/null_prompt_session_manager.h"
333 #include "mir_test_doubles/mock_input_targeter.h"
334 #include "mir_test_doubles/stub_buffer_stream_factory.h"
335@@ -82,8 +81,6 @@
336 mt::fake_shared(session_listener)
337 };
338
339- NiceMock<mtd::MockSurfaceConfigurator> surface_configurator;
340-
341 msh::AbstractShell shell{
342 mt::fake_shared(input_targeter),
343 mt::fake_shared(surface_coordinator),
344@@ -93,8 +90,7 @@
345 { return std::make_shared<msh::DefaultWindowManager>(
346 focus_controller,
347 std::make_shared<NullPlacementStrategy>(),
348- mt::fake_shared(session_manager),
349- mt::fake_shared(surface_configurator));
350+ mt::fake_shared(session_manager));
351 }};
352 };
353 }
354@@ -183,49 +179,3 @@
355
356 shell.set_focus_to(mt::fake_shared(app), mock_surface);
357 }
358-
359-TEST_F(TestDefaultWindowManager, configurator_selects_attribute_values)
360-{
361- mtd::StubSceneSession app;
362- auto const session = mt::fake_shared(app);
363- auto const surface = std::make_shared<NiceMock<mtd::MockSurface>>();
364- ON_CALL(*surface, configure(_, _)).WillByDefault(ReturnArg<1>());
365-
366- InSequence seq;
367-
368- EXPECT_CALL(surface_configurator, select_attribute_value(_, mir_surface_attrib_state, mir_surface_state_restored))
369- .WillOnce(Return(mir_surface_state_minimized));
370-
371- EXPECT_CALL(*surface, configure(mir_surface_attrib_state, mir_surface_state_minimized));
372-
373- EXPECT_CALL(surface_configurator, attribute_set(_, mir_surface_attrib_state, mir_surface_state_minimized));
374-
375- EXPECT_THAT(
376- shell.set_surface_attribute(session, surface, mir_surface_attrib_state, mir_surface_state_restored),
377- Eq(mir_surface_state_minimized));
378-}
379-
380-TEST_F(TestDefaultWindowManager, set_surface_attribute_returns_value_set_by_configurator)
381-{
382- mtd::StubSceneSession app;
383- auto const session = mt::fake_shared(app);
384- auto const surface = std::make_shared<NiceMock<mtd::MockSurface>>();
385- ON_CALL(*surface, configure(_, _)).WillByDefault(ReturnArg<1>());
386-
387- ON_CALL(surface_configurator, select_attribute_value(_, Not(mir_surface_attrib_focus), _))
388- .WillByDefault(ReturnArg<1>());
389-
390- ON_CALL(surface_configurator, select_attribute_value(_, mir_surface_attrib_focus, mir_surface_focused))
391- .WillByDefault(Return(mir_surface_unfocused));
392-
393- ON_CALL(surface_configurator, select_attribute_value(_, mir_surface_attrib_focus, Not(mir_surface_focused)))
394- .WillByDefault(Return(mir_surface_focused));
395-
396- EXPECT_THAT(
397- shell.set_surface_attribute(session, surface, mir_surface_attrib_focus, mir_surface_focused),
398- Eq(mir_surface_unfocused));
399-
400- EXPECT_THAT(
401- shell.set_surface_attribute(session, surface, mir_surface_attrib_focus, mir_surface_unfocused),
402- Eq(mir_surface_focused));
403-}
404
405=== modified file 'tests/integration-tests/test_surface_stack_with_compositor.cpp'
406--- tests/integration-tests/test_surface_stack_with_compositor.cpp 2015-03-31 02:35:42 +0000
407+++ tests/integration-tests/test_surface_stack_with_compositor.cpp 2015-04-01 10:12:47 +0000
408@@ -31,7 +31,6 @@
409 #include "mir_test_doubles/stub_display_buffer.h"
410 #include "mir_test_doubles/stub_buffer.h"
411 #include "mir_test_doubles/stub_input_sender.h"
412-#include "mir_test_doubles/null_surface_configurator.h"
413 #include "mir_test_doubles/null_display_sync_group.h"
414
415 #include <condition_variable>
416
417=== modified file 'tests/mir_test_framework/fake_event_hub_server_configuration.cpp'
418--- tests/mir_test_framework/fake_event_hub_server_configuration.cpp 2015-03-31 11:18:41 +0000
419+++ tests/mir_test_framework/fake_event_hub_server_configuration.cpp 2015-04-01 10:12:47 +0000
420@@ -69,7 +69,6 @@
421 { return std::make_shared<ms::DefaultWindowManager>(
422 focus_controller,
423 the_placement_strategy(),
424- the_session_coordinator(),
425- the_surface_configurator()); };
426+ the_session_coordinator()); };
427 }
428
429
430=== modified file 'tests/unit-tests/scene/test_surface.cpp'
431--- tests/unit-tests/scene/test_surface.cpp 2015-03-31 02:35:42 +0000
432+++ tests/unit-tests/scene/test_surface.cpp 2015-04-01 10:12:47 +0000
433@@ -23,7 +23,6 @@
434 #include "src/server/report/null_report_factory.h"
435 #include "mir/frontend/event_sink.h"
436 #include "mir/scene/surface_creation_parameters.h"
437-#include "mir/scene/surface_configurator.h"
438 #include "mir/scene/surface_event_source.h"
439 #include "mir/input/input_channel.h"
440
441
442=== modified file 'tests/unit-tests/shell/test_default_window_manager.cpp'
443--- tests/unit-tests/shell/test_default_window_manager.cpp 2015-03-31 11:18:41 +0000
444+++ tests/unit-tests/shell/test_default_window_manager.cpp 2015-04-01 10:12:47 +0000
445@@ -34,7 +34,6 @@
446 #include "mir_test_doubles/mock_session_listener.h"
447 #include "mir_test_doubles/mock_surface.h"
448 #include "mir_test_doubles/null_snapshot_strategy.h"
449-#include "mir_test_doubles/null_surface_configurator.h"
450 #include "mir_test_doubles/null_prompt_session_manager.h"
451 #include "mir_test_doubles/stub_input_targeter.h"
452 #include "mir_test_doubles/stub_buffer_stream_factory.h"
453@@ -127,8 +126,7 @@
454 { return std::make_shared<msh::DefaultWindowManager>(
455 focus_controller,
456 mt::fake_shared(placement_strategy),
457- mt::fake_shared(session_manager),
458- std::make_shared<mtd::NullSurfaceConfigurator>());
459+ mt::fake_shared(session_manager));
460 }};
461
462 void SetUp() override

Subscribers

People subscribed via source and target branches