Mir

Merge lp:~robertcarr/mir/demo-shell into lp:~mir-team/mir/trunk

Proposed by Robert Carr
Status: Merged
Approved by: Robert Carr
Approved revision: no longer in the source branch.
Merged at revision: 632
Proposed branch: lp:~robertcarr/mir/demo-shell
Merge into: lp:~mir-team/mir/trunk
Diff against target: 504 lines (+376/-8)
12 files modified
examples/CMakeLists.txt (+2/-0)
examples/demo-shell/CMakeLists.txt (+9/-0)
examples/demo-shell/application_switcher.cpp (+51/-0)
examples/demo-shell/application_switcher.h (+56/-0)
examples/demo-shell/demo_shell.cpp (+92/-0)
examples/demo-shell/fullscreen_placement_strategy.cpp (+40/-0)
examples/demo-shell/fullscreen_placement_strategy.h (+54/-0)
include/server/mir/default_server_configuration.h (+6/-1)
include/server/mir/shell/focus_controller.h (+44/-0)
include/server/mir/shell/session_manager.h (+2/-1)
src/server/default_server_configuration.cpp (+15/-3)
tests/acceptance-tests/test_focus_management_api.cpp (+5/-3)
To merge this branch: bzr merge lp:~robertcarr/mir/demo-shell
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Alan Griffiths Approve
Kevin DuBois (community) Needs Information
Alexandros Frantzis (community) Needs Fixing
Daniel van Vugt Abstain
Robert Ancell Approve
Review via email: mp+159253@code.launchpad.net

Commit message

Add demo shell

Description of the change

Add a simple demo shell as a target for improvement. Has two fun features, fullscreen all apps, and tab to switch between apps.

Currently it will crash when tabbing with --enable-input=true as the input channels will be closed twice. This is addressed (seperately) in this branch: https://code.launchpad.net/~robertcarr/mir/reflow-input-focus-selection

Might be fun to experiment with a few more small features where exploring them serves to strengthen the interfaces.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Robert Ancell (robert-ancell) wrote :

Yay, good to start to see a real shell.

review: Approve
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

The tab key only works if it's running as root. Is that right?

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

1. Tab key only works as root. Is that expected?

2. Wrong licenses:
    examples/demo-shell/application_switcher.*
    examples/demo-shell/demo_shell_placement_strategy.*

3. Lots of returns in bool me::ApplicationSwitcher::handles. I think it would be easier to understand even as a large sequence of A && B && C &&...

4. Missing const. But is that already a pre-existing problem?
    132 + bool handles(MirEvent const& event);

5. Holistically I think we have a potential problem with size and complexity. I mean, how small can you make a minimal shell? If a minimal shell requires this much code then we've made some design mistakes. The two features it has should not require hundreds of lines of code. Again, this is not a new problem introduced by this proposal but it is now most visible. I propose that we limit ourselves to one demo shell for some time while we figure out how to reduce the effort required on the shell side. It would be a bad thing if we imposed this kind of complexity on every new shell. Even if there is only one production shell.

review: Needs Fixing
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

One potential approach is to say: A shell must implement a basic (single class!) shell interface. It might not be an interface even, just a base class. And all customizations (including other classes) should stem from that initial class.

But the point is that it should be possible to make a stub/null shell with a single class. And it should compile and run.

Revision history for this message
Robert Ancell (robert-ancell) wrote :

demo_shell_placement_strategy.h doesn't seem to do anything, just seems to be a duplicate of fullscreen_placement_strategy.h

review: Needs Fixing
Revision history for this message
Robert Carr (robertcarr) wrote :

Fixed licenses, removed demo_shell_placement_strategy.h, refactored returns.

Tab key only works as root due to not having permissions on /dev/input by default. So expected I guess.

I agree we should stick to one demo shell. This one is a little verbose, it was intended in a way that would be easiest to grow (have a few more things to play around with perhaps). It's intended to use the interfaces in a way Unity might, rather than attempt to express the features as small as possible (i.e. one lambda in default server configuration).

The stub/null shell is DefaultServerConfiguration I think.

Revision history for this message
Robert Ancell (robert-ancell) :
review: Approve
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I meant...

class MyShell : public mir::Shell
{
...
}

int main()
{
    MyShell shell;
    return shell.run();
}

It should be that simple.

Any feature of a shell that might be customized (like "placement strategy") would relate to a virtual method of mir::Shell that you override. e.g. virtual std::shared_ptr<FeatureX> get_featureX();

It is dangerous to assume how "Unity might" do things. More safe is to assume that if you make everything really simple with sane defaults, then people can get learn it and use it faster. With fewer mistakes and less coupling.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Still needs GPL, not LGPL:
examples/demo-shell/fullscreen_placement_strategy.cpp

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

I like the example - but it does show some gaps in the current architecture:

234 + // TODO: Remove strange dynamic cast. Perhaps this is an indication that SessionManager provides an interface such as
235 + // "FocusController" (and configuration, the_focus_controller())

I agree - focus_next() belongs in an interface (otherwise code that uses it cannot be unit tested).

~~~~

236 + // TODO: We use this strange two phase initialization in order to
237 + // avoid a circular dependency between the_input_filters() and the_session_store(). This seems to indicate that perhaps
238 + // the InputManager should not be the InputFocusSelector

That's the_*event*_filters() right?

Doesn't the potential circular dependency in configuration indicate that we have an ownership cycle and a potential resource leak?

I think deeper thought is required.

~~~~

217 + std::initializer_list<std::shared_ptr<mi::EventFilter> const> the_event_filters() override
218 + {
219 + static std::initializer_list<std::shared_ptr<mi::EventFilter> const> filter_list = { app_switcher };
220 + return filter_list;
221 + }
222 +
223 + std::shared_ptr<me::ApplicationSwitcher> app_switcher;

Initializing a static variable with a member variable?

That means if there ever were a second instance of this class (unlikely I know) then the function would return a reference to the ApplicationSwitcher created by the first instance. (And elsewhere the new instance is used.)

review: Needs Fixing
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

There's clearly too much logic in [DefaultServerConfiguration|DemoServerConfiguration]::the_frontend_shell()

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> There's clearly too much logic in
> [DefaultServerConfiguration|DemoServerConfiguration]::the_frontend_shell()

I'm going to sort out trunk

Revision history for this message
Kevin DuBois (kdub) wrote :

56 +#include <assert.h>
needed?

Revision history for this message
Robert Carr (robertcarr) wrote :

Everything should be fixed (assert.h needed and used now, FocusController interface introduced, weird static initializer_list removed).

The only remaining problem is the strange circular dependency and the two phase initialization...I can't find a great solution here!

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Robert Carr (robertcarr) wrote :

The example as in trunk will crash with --enable-input=true when pressing any key events! This is due to somehow the input filters (the application switcher) being nullptr! Through gdb you can watch, it is constructed, the input configuration sees it as a non null ptr (as part of the event filter list), then when the event filter list it passed to the dispatcher controller somehow the member of the event filter list is nullptr! The weird static code was to work around this...

Unable to understand it as it stands.

Revision history for this message
Robert Carr (robertcarr) wrote :
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Are there dependencies I don't know of?

I find that running this crashes - and takes down a lot of the system with it.

review: Needs Information
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

My previous concerns remain. And I could nit-pick a bit, but don't want to hold this up.

I would prefer to see the crashes fixed before this lands.

review: Abstain
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :
Download full text (4.4 KiB)

mir_demo_shell still crashes for me too at rev. 602.

With no arguments (or --enable-input=true) it crashes when pressing a key:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffeb30a700 (LWP 7796)]
0x00007ffff7a0ecf7 in mir::input::EventFilterChain::handles (this=0x7a56e8, event=...)
    at /storage/work/mir.bzr/demo-shell/src/server/input/event_filter_chain.cpp:33
33 if (filter->handles(event)) return true;
(gdb) bt
#0 0x00007ffff7a0ecf7 in mir::input::EventFilterChain::handles (this=0x7a56e8, event=...)
    at /storage/work/mir.bzr/demo-shell/src/server/input/event_filter_chain.cpp:33
#1 0x00007ffff7a12940 in mir::input::android::EventFilterDispatcherPolicy::filterInputEvent (this=0x809e00, input_event=0x7fffeb309c80)
    at /storage/work/mir.bzr/demo-shell/src/server/input/android/event_filter_dispatcher_policy.cpp:59
#2 0x00007ffff7a62a61 in android::InputDispatcher::notifyKey (this=0x842ec0, args=0x7fffdc02e010)
    at /storage/work/mir.bzr/demo-shell/3rd_party/android-input/android/frameworks/base/services/input/InputDispatcher.cpp:2293
#3 0x00007ffff7ac9718 in android::NotifyKeyArgs::notify (this=0x7fffdc02e010, listener=...)
    at /storage/work/mir.bzr/demo-shell/3rd_party/android-input/android/frameworks/base/services/input/InputListener.cpp:63
#4 0x00007ffff7aca406 in android::QueuedInputListener::flush (this=0x80e690)
    at /storage/work/mir.bzr/demo-shell/3rd_party/android-input/android/frameworks/base/services/input/InputListener.cpp:174
#5 0x00007ffff7a7b6b6 in android::InputReader::loopOnce (this=0x7d5270)
    at /storage/work/mir.bzr/demo-shell/3rd_party/android-input/android/frameworks/base/services/input/InputReader.cpp:325
#6 0x00007ffff7a7d8ff in android::InputReaderThread::threadLoop (this=0x7920f0)
    at /storage/work/mir.bzr/demo-shell/3rd_party/android-input/android/frameworks/base/services/input/InputReader.cpp:856
#7 0x00007ffff7a6a485 in mir_input::Thread::run(char const*, int, unsigned long)::{lambda()#1}::operator()() const (__closure=0x792a50)
    at /storage/work/mir.bzr/demo-shell/3rd_party/android-deps/std/Thread.h:59
#8 0x00007ffff7a7a2e8 in std::_Bind_simple<mir_input::Thread::run(char const*, int, unsigned long)::{lambda()#1} ()>::_M_invoke<>(std::_Index_tuple<>) (this=0x792a50) at /usr/include/c++/4.7/functional:1598
#9 0x00007ffff7a7a235 in std::_Bind_simple<mir_input::Thread::run(char const*, int, unsigned long)::{lambda()#1} ()>::operator()() (
    this=0x792a50) at /usr/include/c++/4.7/functional:1586
#10 0x00007ffff7a7a1ce in std::thread::_Impl<std::_Bind_simple<mir_input::Thread::run(char const*, int, unsigned long)::{lambda()#1} ()> >::_M_run() (this=0x792a38) at /usr/include/c++/4.7/thread:115
#11 0x00007ffff71d09f0 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#12 0x00007ffff7428f8e in start_thread (arg=0x7fffeb30a700) at pthread_create.c:311
#13 0x00007ffff6c39e1d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:113

With --enable-input=false it crashes when starting:

#0 0x000000000042ce8f in std::__shared_ptr<mir::shell::FocusController, (__gnu_cxx::_Lock_policy)2>::operator= (this=0x8)
    at /usr/include/c++/4.7/bits/sha...

Read more...

review: Needs Fixing
Revision history for this message
Robert Carr (robertcarr) wrote :

r604 Fixes the crash but it's hard to understand why!

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Kevin DuBois (kdub) wrote :

well, I can't test (because I haven't figured out how to input a tab on android), but I'd still like to know what was going on with the crash.

l232, its sort of funny to have a comment in a demo detailing a problem we have with our dependency structures :) Is it worth holding this up to resolve it?

overall, I think it's off to a good start though

review: Needs Information
Revision history for this message
Robert Carr (robertcarr) wrote :

The crash was a segfault in EventFilterChain::handles when dereferencing null event filter. the std::initializer_list captured the shared_ptr<mi::EventFilter> in DemoServerConfiguration::the_event_filters...however this was a temporary, as the value stored on the class was of type me::ApplicationSwitcher, when the scope creating the temporary exited the initializer_lists reference became invalid. This is fixed now by maintaining the list in main.

I think the dependency situation is unfortunate but I would rather move forward (so the demo shell exists as a bed to test things like software cursor, so I can continue to refine Qt input and test things like event filtering) as there isn't much cost to refactoring demo_shell.cpp

Revision history for this message
Kevin DuBois (kdub) wrote :

> l232, its sort of funny to have a comment in a demo detailing a problem we
> have with our dependency structures :) Is it worth holding this up to resolve
> it?
why is the_event_filters returning an initializer_list anyways? :) I can see what forced the 2 step initialization in l232, maybe making a few input classes construct with either std::vector /or/ std::initializer_list could break the cycle

Revision history for this message
Robert Carr (robertcarr) wrote :

I don't think so. The cycle still exists:
To break the two phase initialization we need to initialize me::ApplicationSwitcher in the_event_filters with the_focus_controller.

This breaks on startup as DisplayServer calls the_communicator, calls the_frontend_shell, calls the_session_manager, calls the_input_target_listener, calls the_input_configuration, calls the_event_filters, which in this case reenters to the_focus_controller->the_session_manager

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> I don't think so. The cycle still exists:
> To break the two phase initialization we need to initialize
> me::ApplicationSwitcher in the_event_filters with the_focus_controller.
>
> This breaks on startup as DisplayServer calls the_communicator, calls
> the_frontend_shell, calls the_session_manager, calls
> the_input_target_listener, calls the_input_configuration, calls
> the_event_filters, which in this case reenters to
> the_focus_controller->the_session_manager

I'm still concerned that this is a symptom of an ownership cycle that is not fixed by this "workaround". Vis:

app_switcher has a shared pointer to the_focus_controller
the_focus_controller has a shared pointer to the_input_target_listener
the_input_target_listener has a shared pointer to the_dispatcher
the_dispatcher has a shared pointer to an EventFilterChain
the EventFilterChain has a shared pointer to app_switcher

With shared pointers we need ownership to be a directed, acyclic graph - otherwise there will be memory leaks.

review: Needs Fixing
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

For this reason I have been thinking about writing a mir::Ref template. It would be like a shared_ptr, but can also be told to keep track of user-relationships and detect cycles in the graph. Maybe not do cycle detection in release builds though.

Revision history for this message
Robert Carr (robertcarr) wrote :
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Looks OK, but maybe we could doc comment it?

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) :
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/CMakeLists.txt'
2--- examples/CMakeLists.txt 2013-04-16 09:08:29 +0000
3+++ examples/CMakeLists.txt 2013-04-24 05:17:25 +0000
4@@ -121,5 +121,7 @@
5 add_subdirectory(demo-inprocess-egl)
6 endif()
7
8+add_subdirectory(demo-shell)
9+
10 install(TARGETS ${DEMO_CLIENTS} RUNTIME DESTINATION bin)
11
12
13=== added directory 'examples/demo-shell'
14=== added file 'examples/demo-shell/CMakeLists.txt'
15--- examples/demo-shell/CMakeLists.txt 1970-01-01 00:00:00 +0000
16+++ examples/demo-shell/CMakeLists.txt 2013-04-24 05:17:25 +0000
17@@ -0,0 +1,9 @@
18+add_executable(mir_demo_shell
19+ demo_shell.cpp
20+ fullscreen_placement_strategy.cpp
21+ application_switcher.cpp
22+)
23+
24+target_link_libraries(mir_demo_shell
25+ mirserver
26+)
27
28=== added file 'examples/demo-shell/application_switcher.cpp'
29--- examples/demo-shell/application_switcher.cpp 1970-01-01 00:00:00 +0000
30+++ examples/demo-shell/application_switcher.cpp 2013-04-24 05:17:25 +0000
31@@ -0,0 +1,51 @@
32+/*
33+ * Copyright © 2013 Canonical Ltd.
34+ *
35+ * This program is free software: you can redistribute it and/or modify it
36+ * under the terms of the GNU General Public License version 3,
37+ * as published by the Free Software Foundation.
38+ *
39+ * This program is distributed in the hope that it will be useful,
40+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
41+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
42+ * GNU General Public License for more details.
43+ *
44+ * You should have received a copy of the GNU General Public License
45+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
46+ *
47+ * Authored by: Robert Carr <robert.carr@canonical.com>
48+ */
49+
50+#include "application_switcher.h"
51+
52+#include "mir/shell/focus_controller.h"
53+
54+#include <linux/input.h>
55+
56+#include <assert.h>
57+
58+namespace me = mir::examples;
59+namespace msh = mir::shell;
60+
61+me::ApplicationSwitcher::ApplicationSwitcher()
62+{
63+}
64+
65+void me::ApplicationSwitcher::set_focus_controller(std::shared_ptr<msh::FocusController> const& controller)
66+{
67+ focus_controller = controller;
68+}
69+
70+bool me::ApplicationSwitcher::handles(MirEvent const& event)
71+{
72+ assert(focus_controller);
73+
74+ if (event.key.type == mir_event_type_key &&
75+ event.key.action != 1 && // Not key-release
76+ event.key.scan_code == KEY_TAB) // TODO: Use keycode once we support keymapping on the server side
77+ {
78+ focus_controller->focus_next();
79+ return true;
80+ }
81+ return false;
82+}
83
84=== added file 'examples/demo-shell/application_switcher.h'
85--- examples/demo-shell/application_switcher.h 1970-01-01 00:00:00 +0000
86+++ examples/demo-shell/application_switcher.h 2013-04-24 05:17:25 +0000
87@@ -0,0 +1,56 @@
88+/*
89+ * Copyright © 2013 Canonical Ltd.
90+ *
91+ * This program is free software: you can redistribute it and/or modify it
92+ * under the terms of the GNU General Public License version 3,
93+ * as published by the Free Software Foundation.
94+ *
95+ * This program is distributed in the hope that it will be useful,
96+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
97+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
98+ * GNU General Public License for more details.
99+ *
100+ * You should have received a copy of the GNU General Public License
101+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
102+ *
103+ * Authored by: Robert Carr <robert.carr@canonical.com>
104+ */
105+
106+#ifndef MIR_EXAMPLES_APPLICATION_SWITCHER_H_
107+#define MIR_EXAMPLES_APPLICATION_SWITCHER_H_
108+
109+#include "mir/input/event_filter.h"
110+
111+#include <memory>
112+
113+namespace mir
114+{
115+namespace shell
116+{
117+class FocusController;
118+}
119+namespace examples
120+{
121+
122+class ApplicationSwitcher : public input::EventFilter
123+{
124+public:
125+ ApplicationSwitcher();
126+ ~ApplicationSwitcher() = default;
127+
128+ void set_focus_controller(std::shared_ptr<shell::FocusController> const& focus_controller);
129+
130+ bool handles(MirEvent const& event);
131+
132+protected:
133+ ApplicationSwitcher(const ApplicationSwitcher&) = delete;
134+ ApplicationSwitcher& operator=(const ApplicationSwitcher&) = delete;
135+
136+private:
137+ std::shared_ptr<shell::FocusController> focus_controller;
138+};
139+
140+}
141+} // namespace mir
142+
143+#endif // MIR_EXAMPLES_APPLICATION_SWITCHER_H_
144
145=== added file 'examples/demo-shell/demo_shell.cpp'
146--- examples/demo-shell/demo_shell.cpp 1970-01-01 00:00:00 +0000
147+++ examples/demo-shell/demo_shell.cpp 2013-04-24 05:17:25 +0000
148@@ -0,0 +1,92 @@
149+/*
150+ * Copyright © 2013 Canonical Ltd.
151+ *
152+ * This program is free software: you can redistribute it and/or modify
153+ * it under the terms of the GNU General Public License version 3 as
154+ * published by the Free Software Foundation.
155+ *
156+ * This program is distributed in the hope that it will be useful,
157+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
158+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
159+ * GNU General Public License for more details.
160+ *
161+ * You should have received a copy of the GNU General Public License
162+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
163+ *
164+ * Authored by: Robert Carr <robert.carr@canonical.com>
165+ */
166+
167+#include "application_switcher.h"
168+#include "fullscreen_placement_strategy.h"
169+
170+#include "mir/run_mir.h"
171+#include "mir/default_server_configuration.h"
172+#include "mir/shell/session_manager.h"
173+#include "mir/shell/registration_order_focus_sequence.h"
174+#include "mir/shell/single_visibility_focus_mechanism.h"
175+#include "mir/shell/session_container.h"
176+#include "mir/shell/organising_surface_factory.h"
177+#include "mir/graphics/display.h"
178+
179+#include <boost/exception/diagnostic_information.hpp>
180+#include <iostream>
181+
182+namespace me = mir::examples;
183+namespace msh = mir::shell;
184+namespace mg = mir::graphics;
185+namespace mf = mir::frontend;
186+namespace mi = mir::input;
187+
188+namespace mir
189+{
190+namespace examples
191+{
192+
193+struct DemoServerConfiguration : mir::DefaultServerConfiguration
194+{
195+ DemoServerConfiguration(int argc, char const* argv[],
196+ std::initializer_list<std::shared_ptr<mi::EventFilter> const> const& filter_list)
197+ : DefaultServerConfiguration(argc, argv),
198+ filter_list(filter_list)
199+ {
200+ }
201+
202+ std::shared_ptr<msh::PlacementStrategy> the_shell_placement_strategy()
203+ {
204+ return shell_placement_strategy(
205+ [this]
206+ {
207+ return std::make_shared<me::FullscreenPlacementStrategy>(the_display());
208+ });
209+ }
210+
211+ std::initializer_list<std::shared_ptr<mi::EventFilter> const> the_event_filters() override
212+ {
213+ return filter_list;
214+ }
215+
216+ std::initializer_list<std::shared_ptr<mi::EventFilter> const> const filter_list;
217+};
218+
219+}
220+}
221+
222+int main(int argc, char const* argv[])
223+try
224+{
225+ auto app_switcher = std::make_shared<me::ApplicationSwitcher>();
226+ me::DemoServerConfiguration config(argc, argv, {app_switcher});
227+
228+ mir::run_mir(config, [&config, &app_switcher](mir::DisplayServer&)
229+ {
230+ // We use this strange two stage initialization to avoid a circular dependency between the EventFilters
231+ // and the SessionStore
232+ app_switcher->set_focus_controller(config.the_focus_controller());
233+ });
234+ return 0;
235+}
236+catch (std::exception const& error)
237+{
238+ std::cerr << "ERROR: " << boost::diagnostic_information(error) << std::endl;
239+ return 1;
240+}
241
242=== added file 'examples/demo-shell/fullscreen_placement_strategy.cpp'
243--- examples/demo-shell/fullscreen_placement_strategy.cpp 1970-01-01 00:00:00 +0000
244+++ examples/demo-shell/fullscreen_placement_strategy.cpp 2013-04-24 05:17:25 +0000
245@@ -0,0 +1,40 @@
246+/*
247+ * Copyright © 2013 Canonical Ltd.
248+ *
249+ * This program is free software: you can redistribute it and/or modify it
250+ * under the terms of the GNU General Public License version 3,
251+ * as published by the Free Software Foundation.
252+ *
253+ * This program is distributed in the hope that it will be useful,
254+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
255+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
256+ * GNU General Public License for more details.
257+ *
258+ * You should have received a copy of the GNU General Public License
259+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
260+ *
261+ * Authored by: Robert Carr <robert.carr@canonical.com>
262+ */
263+
264+#include "fullscreen_placement_strategy.h"
265+
266+#include "mir/frontend/surface_creation_parameters.h"
267+#include "mir/graphics/viewable_area.h"
268+
269+namespace me = mir::examples;
270+namespace mf = mir::frontend;
271+namespace mg = mir::graphics;
272+
273+me::FullscreenPlacementStrategy::FullscreenPlacementStrategy(std::shared_ptr<mg::ViewableArea> const& display_area)
274+ : display_area(display_area)
275+{
276+}
277+
278+mf::SurfaceCreationParameters me::FullscreenPlacementStrategy::place(mf::SurfaceCreationParameters const& request_parameters)
279+{
280+ auto placed_parameters = request_parameters;
281+
282+ placed_parameters.size = display_area->view_area().size;
283+
284+ return placed_parameters;
285+}
286
287=== added file 'examples/demo-shell/fullscreen_placement_strategy.h'
288--- examples/demo-shell/fullscreen_placement_strategy.h 1970-01-01 00:00:00 +0000
289+++ examples/demo-shell/fullscreen_placement_strategy.h 2013-04-24 05:17:25 +0000
290@@ -0,0 +1,54 @@
291+/*
292+ * Copyright © 2013 Canonical Ltd.
293+ *
294+ * This program is free software: you can redistribute it and/or modify it
295+ * under the terms of the GNU General Public License version 3,
296+ * as published by the Free Software Foundation.
297+ *
298+ * This program is distributed in the hope that it will be useful,
299+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
300+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
301+ * GNU General Public License for more details.
302+ *
303+ * You should have received a copy of the GNU General Public License
304+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
305+ *
306+ * Authored by: Robert Carr <robert.carr@canonical.com>
307+ */
308+
309+#ifndef MIR_EXAMPLES_FULLSCREEN_PLACEMENT_STRATEGY_H_
310+#define MIR_EXAMPLES_FULLSCREEN_PLACEMENT_STRATEGY_H_
311+
312+#include "mir/shell/placement_strategy.h"
313+
314+#include <memory>
315+
316+namespace mir
317+{
318+namespace graphics
319+{
320+class ViewableArea;
321+}
322+namespace examples
323+{
324+
325+class FullscreenPlacementStrategy : public shell::PlacementStrategy
326+{
327+public:
328+ FullscreenPlacementStrategy(std::shared_ptr<graphics::ViewableArea> const& display_area);
329+ ~FullscreenPlacementStrategy() = default;
330+
331+ frontend::SurfaceCreationParameters place(frontend::SurfaceCreationParameters const& request_parameters);
332+
333+protected:
334+ FullscreenPlacementStrategy(FullscreenPlacementStrategy const&) = delete;
335+ FullscreenPlacementStrategy& operator=(FullscreenPlacementStrategy const&) = delete;
336+
337+private:
338+ std::shared_ptr<graphics::ViewableArea> const display_area;
339+};
340+
341+}
342+} // namespace mir
343+
344+#endif // MIR_EXAMPLES_FULLSCREEN_PLACEMENT_STRATEGY_H_
345
346=== modified file 'include/server/mir/default_server_configuration.h'
347--- include/server/mir/default_server_configuration.h 2013-04-23 21:20:49 +0000
348+++ include/server/mir/default_server_configuration.h 2013-04-24 05:17:25 +0000
349@@ -56,6 +56,8 @@
350 class FocusSetter;
351 class FocusSequence;
352 class PlacementStrategy;
353+class FocusController;
354+class SessionManager;
355 }
356 namespace time
357 {
358@@ -145,6 +147,8 @@
359 virtual std::shared_ptr<frontend::Shell> the_frontend_shell();
360 /** @} */
361
362+ virtual std::shared_ptr<shell::FocusController> the_focus_controller();
363+
364 /** @name shell configuration - customization
365 * configurable interfaces for modifying shell
366 * @{ */
367@@ -194,9 +198,10 @@
368 protected:
369 virtual std::shared_ptr<options::Option> the_options() const;
370 virtual std::shared_ptr<input::InputChannelFactory> the_input_channel_factory();
371+ virtual std::shared_ptr<shell::SessionManager> the_session_manager();
372
373 CachedPtr<frontend::Communicator> communicator;
374- CachedPtr<frontend::Shell> session_manager;
375+ CachedPtr<shell::SessionManager> session_manager;
376 std::shared_ptr<input::android::InputConfiguration> input_configuration;
377 CachedPtr<input::InputManager> input_manager;
378 CachedPtr<shell::InputTargetListener> input_target_listener;
379
380=== added file 'include/server/mir/shell/focus_controller.h'
381--- include/server/mir/shell/focus_controller.h 1970-01-01 00:00:00 +0000
382+++ include/server/mir/shell/focus_controller.h 2013-04-24 05:17:25 +0000
383@@ -0,0 +1,44 @@
384+/*
385+ * Copyright © 2013 Canonical Ltd.
386+ *
387+ * This program is free software: you can redistribute it and/or modify it
388+ * under the terms of the GNU General Public License version 3,
389+ * as published by the Free Software Foundation.
390+ *
391+ * This program is distributed in the hope that it will be useful,
392+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
393+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
394+ * GNU General Public License for more details.
395+ *
396+ * You should have received a copy of the GNU General Public License
397+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
398+ *
399+ * Authored by: Robert Carr <robert.carr@canonical.com>
400+ */
401+
402+#ifndef MIR_SHELL_FOCUS_CONTROLLER_H_
403+#define MIR_SHELL_FOCUS_CONTROLLER_H_
404+
405+namespace mir
406+{
407+
408+namespace shell
409+{
410+
411+class FocusController
412+{
413+public:
414+ virtual ~FocusController() {}
415+
416+ virtual void focus_next() = 0;
417+
418+protected:
419+ FocusController() = default;
420+ FocusController(FocusController const&) = delete;
421+ FocusController& operator=(FocusController const&) = delete;
422+};
423+
424+}
425+} // namespace mir
426+
427+#endif // MIR_SHELL_FOCUS_CONTROLLER_H_
428
429=== modified file 'include/server/mir/shell/session_manager.h'
430--- include/server/mir/shell/session_manager.h 2013-04-19 17:44:09 +0000
431+++ include/server/mir/shell/session_manager.h 2013-04-24 05:17:25 +0000
432@@ -21,6 +21,7 @@
433
434 #include "mir/frontend/surface_id.h"
435 #include "mir/frontend/shell.h"
436+#include "mir/shell/focus_controller.h"
437
438 #include <mutex>
439 #include <memory>
440@@ -43,7 +44,7 @@
441 class InputTargetListener;
442 class Session;
443
444-class SessionManager : public frontend::Shell
445+class SessionManager : public frontend::Shell, public shell::FocusController
446 {
447 public:
448 explicit SessionManager(std::shared_ptr<SurfaceFactory> const& surface_factory,
449
450=== modified file 'src/server/default_server_configuration.cpp'
451--- src/server/default_server_configuration.cpp 2013-04-23 21:20:49 +0000
452+++ src/server/default_server_configuration.cpp 2013-04-24 05:17:25 +0000
453@@ -337,9 +337,8 @@
454 });
455 }
456
457-
458-std::shared_ptr<mf::Shell>
459-mir::DefaultServerConfiguration::the_frontend_shell()
460+std::shared_ptr<msh::SessionManager>
461+mir::DefaultServerConfiguration::the_session_manager()
462 {
463 return session_manager(
464 [this]() -> std::shared_ptr<msh::SessionManager>
465@@ -353,6 +352,19 @@
466 });
467 }
468
469+
470+std::shared_ptr<mf::Shell>
471+mir::DefaultServerConfiguration::the_frontend_shell()
472+{
473+ return the_session_manager();
474+}
475+
476+std::shared_ptr<msh::FocusController>
477+mir::DefaultServerConfiguration::the_focus_controller()
478+{
479+ return the_session_manager();
480+}
481+
482 std::initializer_list<std::shared_ptr<mi::EventFilter> const>
483 mir::DefaultServerConfiguration::the_event_filters()
484 {
485
486=== modified file 'tests/acceptance-tests/test_focus_management_api.cpp'
487--- tests/acceptance-tests/test_focus_management_api.cpp 2013-04-18 14:10:07 +0000
488+++ tests/acceptance-tests/test_focus_management_api.cpp 2013-04-24 05:17:25 +0000
489@@ -152,11 +152,13 @@
490 {
491 struct ServerConfig : TestingServerConfiguration
492 {
493- std::shared_ptr<frontend::Shell>
494+ mir::CachedPtr<mf::Shell> frontend_shell;
495+
496+ std::shared_ptr<mf::Shell>
497 the_frontend_shell() override
498 {
499- return session_manager(
500- [this]() -> std::shared_ptr<frontend::Shell>
501+ return frontend_shell(
502+ [this]() -> std::shared_ptr<mf::Shell>
503 {
504 using namespace ::testing;
505

Subscribers

People subscribed via source and target branches