Merge lp:~gerboland/qtmir/qmirserver-hides-mirserver into lp:qtmir
- qmirserver-hides-mirserver
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Daniel d'Andrada |
Approved revision: | 340 |
Merged at revision: | 344 |
Proposed branch: | lp:~gerboland/qtmir/qmirserver-hides-mirserver |
Merge into: | lp:qtmir |
Diff against target: |
750 lines (+279/-192) 14 files modified
demos/qml-demo-shell/qml-demo-shell.qml (+15/-0) src/platforms/mirserver/CMakeLists.txt (+1/-0) src/platforms/mirserver/display.cpp (+1/-6) src/platforms/mirserver/display.h (+7/-9) src/platforms/mirserver/miropenglcontext.cpp (+2/-3) src/platforms/mirserver/miropenglcontext.h (+0/-1) src/platforms/mirserver/mirserverintegration.cpp (+24/-35) src/platforms/mirserver/mirserverintegration.h (+3/-4) src/platforms/mirserver/nativeinterface.cpp (+13/-10) src/platforms/mirserver/nativeinterface.h (+3/-3) src/platforms/mirserver/qmirserver.cpp (+69/-72) src/platforms/mirserver/qmirserver.h (+21/-49) src/platforms/mirserver/qmirserver_p.cpp (+53/-0) src/platforms/mirserver/qmirserver_p.h (+67/-0) |
To merge this branch: | bzr merge lp:~gerboland/qtmir/qmirserver-hides-mirserver |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Daniel d'Andrada (community) | Approve | ||
PS Jenkins bot (community) | continuous-integration | Needs Fixing | |
Review via email: mp+258038@code.launchpad.net |
Commit message
[qpa] refactor QMirServer to clean up its API, and fix strange thread design.
This does the following:
- QMirServer has a much cleaner API - using d-pointer to encourage ABI stability
- move quit decision point to MirServerIntegr
- the Mir server is run from a class inheriting a QThread, simplifies thread control significantly
Description of the change
PS Jenkins bot (ps-jenkins) wrote : | # |
Daniel d'Andrada (dandrader) wrote : | # |
If you're thinking about making QMirServer public API, could you please add some documentation to src/platforms/
Daniel d'Andrada (dandrader) wrote : | # |
So MirServer::stop() is thread safe or is it a hack?
"""
void MirServerWorker
{
server->stop();
}
"""
Daniel d'Andrada (dandrader) wrote : | # |
> So MirServer::stop() is thread safe or is it a hack?
>
>
> """
> void MirServerWorker
> {
> server->stop();
> }
> """
Please ignore me. Now I see that MirServerWorker
Daniel d'Andrada (dandrader) wrote : | # |
In src/platforms/
"""
Q_SLOT void serverStopped();
"""
Are you sure you want this slot in the public API?
And, by the way, it's named like a signal. It should be onServerStopped() or, even better, quit().
Daniel d'Andrada (dandrader) wrote : | # |
in src/platforms/
"""
connect(
"""
I think this line deserves a comment explaining why this has to be a direct connection, which will cause QMirServer:
trunk has a terse comment about it.
Which makes me think maybe you should say from which thread QMirServer:
Daniel d'Andrada (dandrader) wrote : | # |
In src/platforms/
"""
bool run();
Q_SLOT bool stop();
"""
Again, thinking about having it as public API: you should probably document these methods, answering questions like: Is it a blocking call or does it return immediately?
What's the user expected to do with those return values? I don't see you using them for anything.
"""
connect(
"""
"""
if (!d->serverWork
{
qCritical() << "ERROR: QMirServer - Mir failed to start";
"""
Isn't QMirServer going out of its territory when it modifies QCoreApplication directly like that?
I would expect the user of QMirServer to do this instead, like:
"""
if (!qmirServer-
QCoreApplic
}
"""
Daniel d'Andrada (dandrader) wrote : | # |
In src/platforms/
"""
-Display:
- : QObject(parent)
- , m_mirServer(server)
+Display:
{
- std::shared_
+ std::shared_
"""
I think you could go further and pass the display config already in the constructor instead of the whole server since that's the only thing Display is interested in after all.
-------
In src/platforms/
"""
MirOpenGLConte
- : m_mirServer(server)
#if GL_DEBUG
- , m_logger(new QOpenGLDebugLog
+ : m_logger(new QOpenGLDebugLog
#endif
{
- std::shared_
+ std::shared_
"""
Likewise here.
Gerry Boland (gerboland) wrote : | # |
> If you're thinking about making QMirServer public API, could you please add
> some documentation to src/platforms/
> does, what's its main purpose.
It's far from ready yet, but that is the plan. I'll start writing documentation when we've the API in a good condition, which will be another while yet.
Gerry Boland (gerboland) wrote : | # |
> in src/platforms/
>
> """
> connect(
> &QMirServer:
> """
>
> I think this line deserves a comment explaining why this has to be a direct
> connection, which will cause QMirServer:
> within MirServerWorker's thread.
>
> trunk has a terse comment about it.
Fair point. The line above it also creates a direct connection, and that line has a comment pointing to the explanation. But yeah, that connection is easily missed, will fix
Gerry Boland (gerboland) wrote : | # |
> In src/platforms/
>
> """
> bool run();
> Q_SLOT bool stop();
> """
>
> Again, thinking about having it as public API: you should probably document
> these methods, answering questions like: Is it a blocking call or does it
> return immediately?
>
> What's the user expected to do with those return values? I don't see you using
> them for anything.
I was planning to document things later. But I can start now if you'd prefer.
> """
> connect(
> d->serverWorker, &MirServerWorke
> see FIXME in MirServerWorker
> """
>
> """
> if (!d->serverWork
> {
> qCritical() << "ERROR: QMirServer - Mir failed to start";
> QCoreApplicatio
> """
>
> Isn't QMirServer going out of its territory when it modifies QCoreApplication
> directly like that?
> I would expect the user of QMirServer to do this instead, like:
>
> """
> if (!qmirServer-
> QCoreApplicatio
> }
> """
You're totally right, this is also the direction I wanted to go. I was hesitant to do so, as I feared it along with all the other changes would make quite a large MR. So your suggestion is to be the next step: have the QMirServer user listen for mir or qt shutdown, and react accordingly. Should I just do that now?
Gerry Boland (gerboland) wrote : | # |
> In src/platforms/
>
> """
> -Display:
> - : QObject(parent)
> - , m_mirServer(server)
> +Display:
> {
> - std::shared_
> m_mirServer-
> + std::shared_
> server-
> """
>
> I think you could go further and pass the display config already in the
> constructor instead of the whole server since that's the only thing Display is
> interested in after all.
>
> -------
>
> In src/platforms/
>
> """
> MirOpenGLContex
> const QSurfaceFormat &format)
> - : m_mirServer(server)
> #if GL_DEBUG
> - , m_logger(new QOpenGLDebugLog
> + : m_logger(new QOpenGLDebugLog
> #endif
> {
> - std::shared_
> m_mirServer-
> + std::shared_
> """
>
> Likewise here.
I had a reason for not doing that at the time, but I can't recall it now. Will do
Daniel d'Andrada (dandrader) wrote : | # |
> > In src/platforms/
> >
> > """
> > bool run();
> > Q_SLOT bool stop();
> > """
> >
> > Again, thinking about having it as public API: you should probably document
> > these methods, answering questions like: Is it a blocking call or does it
> > return immediately?
> >
> > What's the user expected to do with those return values? I don't see you
> using
> > them for anything.
>
> I was planning to document things later. But I can start now if you'd prefer.
>
>
>
> > """
> > connect(
> > d->serverWorker, &MirServerWorke
> //
> > see FIXME in MirServerWorker
> > """
> >
> > """
> > if (!d->serverWork
> > {
> > qCritical() << "ERROR: QMirServer - Mir failed to start";
> > QCoreApplicatio
> > """
> >
> > Isn't QMirServer going out of its territory when it modifies
> QCoreApplication
> > directly like that?
> > I would expect the user of QMirServer to do this instead, like:
> >
> > """
> > if (!qmirServer-
> > QCoreApplicatio
> > }
> > """
>
>
> You're totally right, this is also the direction I wanted to go. I was
> hesitant to do so, as I feared it along with all the other changes would make
> quite a large MR. So your suggestion is to be the next step: have the
> QMirServer user listen for mir or qt shutdown, and react accordingly. Should I
> just do that now?
I can live with this being an intermediate step but it should leave the code in a consistent state. Eg: If you don't use the return values for anything then don't return them in the first place (have a void return type). Also it would be good to add TODOs for the next step, like over those QCoreApplication calls inside QMirServer.
Daniel d'Andrada (dandrader) wrote : | # |
The commit message line is a bit too long. That first line should shorter, followed by a longer description below if needed
- 332. By Gerry Boland
-
Trying to fix shutdown, incomplete
- 333. By Gerry Boland
-
Fix shutdown issues by inheriting QThread directly - also get to avoid having qt event loop for mir
- 334. By Gerry Boland
-
Clean up shutdown codepath further. run/stop return void
- 335. By Gerry Boland
-
Unneeded return statements
- 336. By Gerry Boland
-
Remove old comment, it not the case any more
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:336
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
- 337. By Gerry Boland
-
Merge qtmir trunk
Gerry Boland (gerboland) wrote : | # |
> > In src/platforms/
> >
> > """
> > -Display:
> > - : QObject(parent)
> > - , m_mirServer(server)
> > +Display:
> > {
> > - std::shared_
> > m_mirServer-
> > + std::shared_
> > server-
> > """
> >
> > I think you could go further and pass the display config already in the
> > constructor instead of the whole server since that's the only thing Display
> is
> > interested in after all.
> >
> > -------
> >
> > In src/platforms/
> >
> > """
> > MirOpenGLContex
> > const QSurfaceFormat &format)
> > - : m_mirServer(server)
> > #if GL_DEBUG
> > - , m_logger(new QOpenGLDebugLog
> > + : m_logger(new QOpenGLDebugLog
> > #endif
> > {
> > - std::shared_
> > m_mirServer-
> > + std::shared_
> server-
> > """
> >
> > Likewise here.
>
>
> I had a reason for not doing that at the time, but I can't recall it now. Will
> do
I thought of the reason. It would require me to include loads of Mir internal headers in MirServerIntegr
As the multimonitor work will refactor those classes quite a bit, I decided to just keep this approach knowing that I'll improve it later.
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:337
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
- 338. By Gerry Boland
-
Merge trunk
- 339. By Gerry Boland
-
Remove unneeded header
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:339
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
- 340. By Gerry Boland
-
Only pass mg::DisplayConf
iguration to qtmir::Display, as that is all it needs
Gerry Boland (gerboland) wrote : | # |
Ready for a second review pass. Main change is in the thread design which simplifies event passing greatly. API improved too, QMirServer no longer quits the process, owner does instead
Daniel d'Andrada (dandrader) wrote : | # |
"""
bool QMirServer::start()
{
[...]
Q_EMIT started();
[...]
}
"""
Is this signal being used for anything?
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:340
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
Gerry Boland (gerboland) wrote : | # |
> """
> bool QMirServer::start()
> {
> [...]
> Q_EMIT started();
> [...]
> }
> """
>
> Is this signal being used for anything?
Nope, added to API for symmetry (the "stopped" signal is necessary)
Daniel d'Andrada (dandrader) wrote : | # |
Code looks ok, but didn't test on the device to see if broke anything.
Daniel d'Andrada (dandrader) wrote : | # |
* Did you perform an exploratory manual test run of the code change and any related functionality?
Yes.
* Did CI run pass? If not, please explain why.
No, because Jenkins runs on wily and wily doesn't have mir 0.13 yet.
Preview Diff
1 | === modified file 'demos/qml-demo-shell/qml-demo-shell.qml' |
2 | --- demos/qml-demo-shell/qml-demo-shell.qml 2015-01-23 10:23:07 +0000 |
3 | +++ demos/qml-demo-shell/qml-demo-shell.qml 2015-05-27 12:12:52 +0000 |
4 | @@ -110,6 +110,21 @@ |
5 | y: point.y |
6 | } |
7 | |
8 | + Rectangle { |
9 | + width: 60 |
10 | + height: 40 |
11 | + color: "red" |
12 | + anchors { right: parent.right; bottom: parent.bottom } |
13 | + Text { |
14 | + anchors.centerIn: parent |
15 | + text: "Quit" |
16 | + } |
17 | + MouseArea { |
18 | + anchors.fill: parent |
19 | + onClicked: Qt.quit() |
20 | + } |
21 | + } |
22 | + |
23 | Connections { |
24 | target: SurfaceManager |
25 | onSurfaceCreated: { |
26 | |
27 | === modified file 'src/platforms/mirserver/CMakeLists.txt' |
28 | --- src/platforms/mirserver/CMakeLists.txt 2015-05-13 09:40:03 +0000 |
29 | +++ src/platforms/mirserver/CMakeLists.txt 2015-05-27 12:12:52 +0000 |
30 | @@ -44,6 +44,7 @@ |
31 | qteventfeeder.cpp |
32 | plugin.cpp |
33 | qmirserver.cpp |
34 | + qmirserver_p.cpp |
35 | sessionauthorizer.cpp |
36 | sessionlistener.cpp |
37 | surfaceobserver.cpp |
38 | |
39 | === modified file 'src/platforms/mirserver/display.cpp' |
40 | --- src/platforms/mirserver/display.cpp 2014-12-01 11:10:54 +0000 |
41 | +++ src/platforms/mirserver/display.cpp 2015-05-27 12:12:52 +0000 |
42 | @@ -23,18 +23,13 @@ |
43 | |
44 | #include <mir/graphics/display.h> |
45 | #include <mir/graphics/display_configuration.h> |
46 | -#include <QDebug> |
47 | |
48 | namespace mg = mir::graphics; |
49 | |
50 | // TODO: Listen for display changes and update the list accordingly |
51 | |
52 | -Display::Display(const QSharedPointer<MirServer> &server, QObject *parent) |
53 | - : QObject(parent) |
54 | - , m_mirServer(server) |
55 | +Display::Display(const std::shared_ptr<mir::graphics::DisplayConfiguration> &displayConfig) |
56 | { |
57 | - std::shared_ptr<mir::graphics::DisplayConfiguration> displayConfig = m_mirServer->the_display()->configuration(); |
58 | - |
59 | displayConfig->for_each_output([this](mg::DisplayConfigurationOutput const& output) { |
60 | if (output.used) { |
61 | auto screen = new Screen(output); |
62 | |
63 | === modified file 'src/platforms/mirserver/display.h' |
64 | --- src/platforms/mirserver/display.h 2014-12-01 11:10:54 +0000 |
65 | +++ src/platforms/mirserver/display.h 2015-05-27 12:12:52 +0000 |
66 | @@ -19,23 +19,21 @@ |
67 | #ifndef DISPLAY_H |
68 | #define DISPLAY_H |
69 | |
70 | -#include <QObject> |
71 | #include <qpa/qplatformscreen.h> |
72 | - |
73 | -class MirServer; |
74 | - |
75 | -class Display : public QObject |
76 | +#include <memory> |
77 | + |
78 | +namespace mir { namespace graphics { class DisplayConfiguration; }} |
79 | + |
80 | +class Display |
81 | { |
82 | - Q_OBJECT |
83 | public: |
84 | - Display(const QSharedPointer<MirServer> &server, QObject *parent = 0); |
85 | - ~Display(); |
86 | + Display(const std::shared_ptr<mir::graphics::DisplayConfiguration> &displayConfig); |
87 | + virtual ~Display(); |
88 | |
89 | QList<QPlatformScreen *> screens() const { return m_screens; } |
90 | |
91 | private: |
92 | QList<QPlatformScreen *> m_screens; |
93 | - const QSharedPointer<MirServer> m_mirServer; |
94 | }; |
95 | |
96 | #endif // DISPLAY_H |
97 | |
98 | === modified file 'src/platforms/mirserver/miropenglcontext.cpp' |
99 | --- src/platforms/mirserver/miropenglcontext.cpp 2015-01-12 17:54:55 +0000 |
100 | +++ src/platforms/mirserver/miropenglcontext.cpp 2015-05-27 12:12:52 +0000 |
101 | @@ -36,12 +36,11 @@ |
102 | // (i.e. individual display output buffers) to use as a common base context. |
103 | |
104 | MirOpenGLContext::MirOpenGLContext(const QSharedPointer<MirServer> &server, const QSurfaceFormat &format) |
105 | - : m_mirServer(server) |
106 | #if GL_DEBUG |
107 | - , m_logger(new QOpenGLDebugLogger(this)) |
108 | + : m_logger(new QOpenGLDebugLogger(this)) |
109 | #endif |
110 | { |
111 | - std::shared_ptr<mir::graphics::Display> display = m_mirServer->the_display(); |
112 | + std::shared_ptr<mir::graphics::Display> display = server->the_display(); |
113 | |
114 | // create a temporary GL context to fetch the EGL display and config, so Qt can determine the surface format |
115 | std::unique_ptr<mir::graphics::GLContext> mirContext = display->create_gl_context(); |
116 | |
117 | === modified file 'src/platforms/mirserver/miropenglcontext.h' |
118 | --- src/platforms/mirserver/miropenglcontext.h 2014-12-01 11:10:54 +0000 |
119 | +++ src/platforms/mirserver/miropenglcontext.h 2015-05-27 12:12:52 +0000 |
120 | @@ -52,7 +52,6 @@ |
121 | #endif |
122 | |
123 | private: |
124 | - const QSharedPointer<MirServer> m_mirServer; |
125 | QSurfaceFormat m_format; |
126 | #if GL_DEBUG |
127 | QOpenGLDebugLogger *m_logger; |
128 | |
129 | === modified file 'src/platforms/mirserver/mirserverintegration.cpp' |
130 | --- src/platforms/mirserver/mirserverintegration.cpp 2015-04-01 10:48:28 +0000 |
131 | +++ src/platforms/mirserver/mirserverintegration.cpp 2015-05-27 12:12:52 +0000 |
132 | @@ -1,5 +1,5 @@ |
133 | /* |
134 | - * Copyright (C) 2013-2014 Canonical, Ltd. |
135 | + * Copyright (C) 2013-2015 Canonical, Ltd. |
136 | * |
137 | * This program is free software: you can redistribute it and/or modify it under |
138 | * the terms of the GNU Lesser General Public License version 3, as published by |
139 | @@ -30,7 +30,6 @@ |
140 | #include <qpa/qwindowsysteminterface.h> |
141 | |
142 | #include <QCoreApplication> |
143 | -#include <QStringList> |
144 | #include <QOpenGLContext> |
145 | |
146 | #if QT_VERSION < QT_VERSION_CHECK(5, 2, 0) |
147 | @@ -41,10 +40,7 @@ |
148 | |
149 | // Mir |
150 | #include <mir/graphics/display.h> |
151 | -#include <mir/graphics/display_buffer.h> |
152 | - |
153 | -// std |
154 | -#include <csignal> |
155 | +#include <mir/graphics/display_configuration.h> |
156 | |
157 | // local |
158 | #include "clipboard.h" |
159 | @@ -66,36 +62,25 @@ |
160 | #if QT_VERSION < QT_VERSION_CHECK(5, 2, 0) |
161 | , m_eventDispatcher(createUnixEventDispatcher()) |
162 | #endif |
163 | + , m_mirServer(new QMirServer(QCoreApplication::arguments())) |
164 | , m_display(nullptr) |
165 | - , m_qmirServer(nullptr) |
166 | , m_nativeInterface(nullptr) |
167 | , m_clipboard(new Clipboard) |
168 | { |
169 | - // Start Mir server only once Qt has initialized its event dispatcher, see initialize() |
170 | - |
171 | - QStringList args = QCoreApplication::arguments(); |
172 | - // convert arguments back into argc-argv form that Mir wants |
173 | - char **argv; |
174 | - argv = new char*[args.size() + 1]; |
175 | - for (int i = 0; i < args.size(); i++) { |
176 | - argv[i] = new char[strlen(args.at(i).toStdString().c_str())+1]; |
177 | - memcpy(argv[i], args.at(i).toStdString().c_str(), strlen(args.at(i).toStdString().c_str())+1); |
178 | - } |
179 | - argv[args.size()] = '\0'; |
180 | - |
181 | // For access to sensors, qtmir uses qtubuntu-sensors. qtubuntu-sensors reads the |
182 | // UBUNTU_PLATFORM_API_BACKEND variable to decide if to load a valid sensor backend or not. |
183 | // For it to function we need to ensure a valid backend has been specified |
184 | if (qEnvironmentVariableIsEmpty("UBUNTU_PLATFORM_API_BACKEND")) { |
185 | - if (qgetenv("DESKTOP_SESSION").contains("mir")) { |
186 | + if (qgetenv("DESKTOP_SESSION").contains("mir") || !qEnvironmentVariableIsSet("ANDROID_DATA")) { |
187 | qputenv("UBUNTU_PLATFORM_API_BACKEND", "desktop_mirclient"); |
188 | } else { |
189 | qputenv("UBUNTU_PLATFORM_API_BACKEND", "touch_mirclient"); |
190 | } |
191 | } |
192 | |
193 | - m_mirServer = QSharedPointer<MirServer>( |
194 | - new MirServer(args.length(), const_cast<const char**>(argv))); |
195 | + // If Mir shuts down, quit. |
196 | + QObject::connect(m_mirServer.data(), &QMirServer::stopped, |
197 | + QCoreApplication::instance(), &QCoreApplication::quit); |
198 | |
199 | #if QT_VERSION < QT_VERSION_CHECK(5, 2, 0) |
200 | QGuiApplicationPrivate::instance()->setEventDispatcher(eventDispatcher_); |
201 | @@ -109,7 +94,6 @@ |
202 | { |
203 | delete m_nativeInterface; |
204 | delete m_display; |
205 | - delete m_qmirServer; |
206 | } |
207 | |
208 | bool MirServerIntegration::hasCapability(QPlatformIntegration::Capability cap) const |
209 | @@ -135,18 +119,21 @@ |
210 | |
211 | DisplayWindow* displayWindow = nullptr; |
212 | |
213 | + auto const mirServer = m_mirServer->mirServer().lock(); |
214 | mg::DisplayBuffer* first_buffer{nullptr}; |
215 | mg::DisplaySyncGroup* first_group{nullptr}; |
216 | - m_mirServer->the_display()->for_each_display_sync_group([&](mg::DisplaySyncGroup &group) { |
217 | - if (!first_group) { |
218 | - first_group = &group; |
219 | - } |
220 | - group.for_each_display_buffer([&](mg::DisplayBuffer &buffer) { |
221 | - if (!first_buffer) { |
222 | - first_buffer = &buffer; |
223 | + if (mirServer) { |
224 | + mirServer->the_display()->for_each_display_sync_group([&](mg::DisplaySyncGroup &group) { |
225 | + if (!first_group) { |
226 | + first_group = &group; |
227 | } |
228 | + group.for_each_display_buffer([&](mg::DisplayBuffer &buffer) { |
229 | + if (!first_buffer) { |
230 | + first_buffer = &buffer; |
231 | + } |
232 | + }); |
233 | }); |
234 | - }); |
235 | + } |
236 | |
237 | // FIXME(gerry) this will go very bad for >1 display buffer |
238 | if (first_group && first_buffer) |
239 | @@ -168,7 +155,7 @@ |
240 | QPlatformOpenGLContext *MirServerIntegration::createPlatformOpenGLContext(QOpenGLContext *context) const |
241 | { |
242 | qDebug() << "createPlatformOpenGLContext" << context; |
243 | - return new MirOpenGLContext(m_mirServer, context->format()); |
244 | + return new MirOpenGLContext(m_mirServer->mirServer(), context->format()); |
245 | } |
246 | |
247 | #if QT_VERSION >= QT_VERSION_CHECK(5, 2, 0) |
248 | @@ -181,10 +168,12 @@ |
249 | void MirServerIntegration::initialize() |
250 | { |
251 | // Creates instance of and start the Mir server in a separate thread |
252 | - m_qmirServer = new QMirServer(m_mirServer); |
253 | + if (!m_mirServer->start()) { |
254 | + exit(2); |
255 | + } |
256 | |
257 | - m_display = new Display(m_mirServer); |
258 | - m_nativeInterface = new NativeInterface(m_mirServer); |
259 | + m_display = new Display(m_mirServer->mirServer().data()->the_display()->configuration()); |
260 | + m_nativeInterface = new NativeInterface(m_mirServer->mirServer()); |
261 | |
262 | for (QPlatformScreen *screen : m_display->screens()) |
263 | screenAdded(screen); |
264 | |
265 | === modified file 'src/platforms/mirserver/mirserverintegration.h' |
266 | --- src/platforms/mirserver/mirserverintegration.h 2014-12-11 15:27:02 +0000 |
267 | +++ src/platforms/mirserver/mirserverintegration.h 2015-05-27 12:12:52 +0000 |
268 | @@ -1,5 +1,5 @@ |
269 | /* |
270 | - * Copyright (C) 2013-2014 Canonical, Ltd. |
271 | + * Copyright (C) 2013-2015 Canonical, Ltd. |
272 | * |
273 | * This program is free software: you can redistribute it and/or modify it under |
274 | * the terms of the GNU Lesser General Public License version 3, as published by |
275 | @@ -69,8 +69,6 @@ |
276 | QPlatformNativeInterface *nativeInterface() const override; |
277 | |
278 | private: |
279 | - QSharedPointer<MirServer> m_mirServer; |
280 | - |
281 | QScopedPointer<QPlatformAccessibility> m_accessibility; |
282 | QScopedPointer<QPlatformFontDatabase> m_fontDb; |
283 | QScopedPointer<QPlatformServices> m_services; |
284 | @@ -78,8 +76,9 @@ |
285 | QScopedPointer<QAbstractEventDispatcher> m_eventDispatcher; |
286 | #endif |
287 | |
288 | + QScopedPointer<QMirServer> m_mirServer; |
289 | + |
290 | Display *m_display; |
291 | - QMirServer *m_qmirServer; |
292 | NativeInterface *m_nativeInterface; |
293 | QPlatformInputContext* m_inputContext; |
294 | QScopedPointer<qtmir::Clipboard> m_clipboard; |
295 | |
296 | === modified file 'src/platforms/mirserver/nativeinterface.cpp' |
297 | --- src/platforms/mirserver/nativeinterface.cpp 2015-01-28 14:25:36 +0000 |
298 | +++ src/platforms/mirserver/nativeinterface.cpp 2015-05-27 12:12:52 +0000 |
299 | @@ -1,5 +1,5 @@ |
300 | /* |
301 | - * Copyright (C) 2013 Canonical, Ltd. |
302 | + * Copyright (C) 2013-2015 Canonical, Ltd. |
303 | * |
304 | * This program is free software: you can redistribute it and/or modify it under |
305 | * the terms of the GNU Lesser General Public License version 3, as published by |
306 | @@ -18,7 +18,7 @@ |
307 | |
308 | #include "nativeinterface.h" |
309 | |
310 | -NativeInterface::NativeInterface(const QSharedPointer<MirServer> &server) |
311 | +NativeInterface::NativeInterface(const QWeakPointer<MirServer> &server) |
312 | : m_mirServer(server) |
313 | { |
314 | } |
315 | @@ -27,14 +27,17 @@ |
316 | { |
317 | void *result = nullptr; |
318 | |
319 | - if (resource == "SessionAuthorizer") |
320 | - result = m_mirServer->sessionAuthorizer(); |
321 | - else if (resource == "Shell") |
322 | - result = m_mirServer->shell(); |
323 | - else if (resource == "SessionListener") |
324 | - result = m_mirServer->sessionListener(); |
325 | - else if (resource == "PromptSessionListener") |
326 | - result = m_mirServer->promptSessionListener(); |
327 | + auto const server = m_mirServer.lock(); |
328 | |
329 | + if (server) { |
330 | + if (resource == "SessionAuthorizer") |
331 | + result = server->sessionAuthorizer(); |
332 | + else if (resource == "Shell") |
333 | + result = server->shell(); |
334 | + else if (resource == "SessionListener") |
335 | + result = server->sessionListener(); |
336 | + else if (resource == "PromptSessionListener") |
337 | + result = server->promptSessionListener(); |
338 | + } |
339 | return result; |
340 | } |
341 | |
342 | === modified file 'src/platforms/mirserver/nativeinterface.h' |
343 | --- src/platforms/mirserver/nativeinterface.h 2014-12-01 11:05:01 +0000 |
344 | +++ src/platforms/mirserver/nativeinterface.h 2015-05-27 12:12:52 +0000 |
345 | @@ -1,5 +1,5 @@ |
346 | /* |
347 | - * Copyright (C) 2013 Canonical, Ltd. |
348 | + * Copyright (C) 2013-2015 Canonical, Ltd. |
349 | * |
350 | * This program is free software: you can redistribute it and/or modify it under |
351 | * the terms of the GNU Lesser General Public License version 3, as published by |
352 | @@ -29,11 +29,11 @@ |
353 | class NativeInterface : public QPlatformNativeInterface |
354 | { |
355 | public: |
356 | - NativeInterface(const QSharedPointer<MirServer> &); |
357 | + NativeInterface(const QWeakPointer<MirServer> &); |
358 | |
359 | virtual void *nativeResourceForIntegration(const QByteArray &resource); |
360 | |
361 | - QSharedPointer<MirServer> m_mirServer; |
362 | + QWeakPointer<MirServer> m_mirServer; |
363 | }; |
364 | |
365 | #endif // NATIVEINTEGRATION_H |
366 | |
367 | === modified file 'src/platforms/mirserver/qmirserver.cpp' |
368 | --- src/platforms/mirserver/qmirserver.cpp 2015-04-27 15:59:42 +0000 |
369 | +++ src/platforms/mirserver/qmirserver.cpp 2015-05-27 12:12:52 +0000 |
370 | @@ -1,5 +1,5 @@ |
371 | /* |
372 | - * Copyright (C) 2013 Canonical, Ltd. |
373 | + * Copyright (C) 2013-2015 Canonical, Ltd. |
374 | * |
375 | * This program is free software: you can redistribute it and/or modify it under |
376 | * the terms of the GNU Lesser General Public License version 3, as published by |
377 | @@ -19,80 +19,77 @@ |
378 | #include <QCoreApplication> |
379 | #include <QDebug> |
380 | |
381 | -#include <mir/main_loop.h> |
382 | - |
383 | // local |
384 | +#include "mirserver.h" |
385 | #include "qmirserver.h" |
386 | - |
387 | - |
388 | -void MirServerWorker::run() |
389 | -{ |
390 | - auto const main_loop = server->the_main_loop(); |
391 | - // By enqueuing the notification code in the main loop, we are |
392 | - // ensuring that the server has really and fully started before |
393 | - // leaving wait_for_startup(). |
394 | - main_loop->enqueue( |
395 | - this, |
396 | - [&] |
397 | - { |
398 | - std::lock_guard<std::mutex> lock(mutex); |
399 | - mir_running = true; |
400 | - started_cv.notify_one(); |
401 | - }); |
402 | - |
403 | - server->run(); |
404 | - Q_EMIT stopped(); |
405 | -} |
406 | - |
407 | -bool MirServerWorker::wait_for_mir_startup() |
408 | -{ |
409 | - std::unique_lock<decltype(mutex)> lock(mutex); |
410 | - started_cv.wait_for(lock, std::chrono::seconds{10}, [&]{ return mir_running; }); |
411 | - return mir_running; |
412 | -} |
413 | - |
414 | -QMirServer::QMirServer(const QSharedPointer<MirServer> &server, QObject *parent) |
415 | +#include "qmirserver_p.h" |
416 | + |
417 | + |
418 | +QMirServer::QMirServer(const QStringList &arguments, QObject *parent) |
419 | : QObject(parent) |
420 | - , m_mirServer(new MirServerWorker(server)) |
421 | -{ |
422 | - m_mirServer->moveToThread(&m_mirThread); |
423 | - |
424 | - connect(this, &QMirServer::run, m_mirServer, &MirServerWorker::run); |
425 | - connect(this, &QMirServer::stop, m_mirServer, &MirServerWorker::stop); |
426 | - |
427 | - connect(QCoreApplication::instance(), &QCoreApplication::aboutToQuit, this, &QMirServer::shutDownMirServer); |
428 | - connect(m_mirServer, &MirServerWorker::stopped, this, &QMirServer::shutDownQApplication, Qt::DirectConnection); // since no event loop |
429 | - |
430 | - m_mirThread.start(QThread::TimeCriticalPriority); |
431 | - Q_EMIT run(); |
432 | - |
433 | - if (!m_mirServer->wait_for_mir_startup()) |
434 | + , d_ptr(new QMirServerPrivate()) |
435 | +{ |
436 | + Q_D(QMirServer); |
437 | + |
438 | + // convert arguments back into argc-argv form that Mir wants |
439 | + int argc = arguments.size(); |
440 | + char **argv = new char*[argc + 1]; |
441 | + for (int i = 0; i < argc; i++) { |
442 | + argv[i] = new char[strlen(arguments.at(i).toStdString().c_str())+1]; |
443 | + memcpy(argv[i], arguments.at(i).toStdString().c_str(), strlen(arguments.at(i).toStdString().c_str())+1); |
444 | + } |
445 | + argv[argc] = '\0'; |
446 | + |
447 | + d->server = QSharedPointer<MirServer>(new MirServer(argc, const_cast<const char**>(argv))); |
448 | + |
449 | + d->serverThread = new MirServerThread(d->server); |
450 | + |
451 | + connect(d->serverThread, &MirServerThread::stopped, this, &QMirServer::stopped); |
452 | +} |
453 | + |
454 | +QMirServer::~QMirServer() |
455 | +{ |
456 | + stop(); |
457 | +} |
458 | + |
459 | +bool QMirServer::start() |
460 | +{ |
461 | + Q_D(QMirServer); |
462 | + |
463 | + d->serverThread->start(QThread::TimeCriticalPriority); |
464 | + |
465 | + if (!d->serverThread->waitForMirStartup()) |
466 | { |
467 | qCritical() << "ERROR: QMirServer - Mir failed to start"; |
468 | - exit(2); |
469 | - } |
470 | -} |
471 | - |
472 | -QMirServer::~QMirServer() |
473 | -{ |
474 | - shutDownMirServer(); |
475 | -} |
476 | - |
477 | -void QMirServer::shutDownMirServer() |
478 | -{ |
479 | - if (m_mirThread.isRunning()) { |
480 | - m_mirServer->stop(); |
481 | - m_mirThread.wait(); |
482 | - } |
483 | -} |
484 | - |
485 | -void QMirServer::shutDownQApplication() |
486 | -{ |
487 | - if (m_mirThread.isRunning()) |
488 | - m_mirThread.quit(); |
489 | - |
490 | - // if unexpected mir server stop, better quit the QApplication |
491 | - if (!QCoreApplication::closingDown()) { |
492 | - QCoreApplication::quit(); |
493 | - } |
494 | + return false; |
495 | + } |
496 | + |
497 | + Q_EMIT started(); |
498 | + return true; |
499 | +} |
500 | + |
501 | +void QMirServer::stop() |
502 | +{ |
503 | + Q_D(QMirServer); |
504 | + |
505 | + if (d->serverThread->isRunning()) { |
506 | + d->serverThread->stop(); |
507 | + if (!d->serverThread->wait(10000)) { |
508 | + // do something to indicate fail during shutdown |
509 | + qCritical() << "ERROR: QMirServer - Mir failed to shut down correctly, terminating it"; |
510 | + d->serverThread->terminate(); |
511 | + } |
512 | + } |
513 | +} |
514 | + |
515 | +bool QMirServer::isRunning() const |
516 | +{ |
517 | + Q_D(const QMirServer); |
518 | + return d->serverThread->isRunning(); |
519 | +} |
520 | + |
521 | +QWeakPointer<MirServer> QMirServer::mirServer() const |
522 | +{ |
523 | + Q_D(const QMirServer); |
524 | + return d->server.toWeakRef(); |
525 | } |
526 | |
527 | === modified file 'src/platforms/mirserver/qmirserver.h' |
528 | --- src/platforms/mirserver/qmirserver.h 2014-12-02 14:55:17 +0000 |
529 | +++ src/platforms/mirserver/qmirserver.h 2015-05-27 12:12:52 +0000 |
530 | @@ -1,5 +1,5 @@ |
531 | /* |
532 | - * Copyright (C) 2013 Canonical, Ltd. |
533 | + * Copyright (C) 2013-2015 Canonical, Ltd. |
534 | * |
535 | * This program is free software: you can redistribute it and/or modify it under |
536 | * the terms of the GNU Lesser General Public License version 3, as published by |
537 | @@ -19,63 +19,35 @@ |
538 | |
539 | // Qt |
540 | #include <QObject> |
541 | -#include <QThread> |
542 | -#include <QSharedPointer> |
543 | - |
544 | -// local |
545 | -#include "mirserver.h" |
546 | - |
547 | -#include <condition_variable> |
548 | -#include <mutex> |
549 | - |
550 | -// Wrap mir::Server with QObject, so it can be controlled via QThread |
551 | -class MirServerWorker : public QObject |
552 | +#include <QWeakPointer> |
553 | + |
554 | +class QMirServerPrivate; |
555 | +class MirServer; |
556 | + |
557 | +class QMirServer: public QObject |
558 | { |
559 | Q_OBJECT |
560 | |
561 | public: |
562 | - MirServerWorker(const QSharedPointer<MirServer> &server) |
563 | - : server(server) |
564 | - {} |
565 | - |
566 | - bool wait_for_mir_startup(); |
567 | + QMirServer(const QStringList &arguments, QObject* parent=0); |
568 | + virtual ~QMirServer(); |
569 | + |
570 | + bool start(); |
571 | + Q_SLOT void stop(); |
572 | + bool isRunning() const; |
573 | + |
574 | + QWeakPointer<MirServer> mirServer() const; |
575 | |
576 | Q_SIGNALS: |
577 | + void started(); |
578 | void stopped(); |
579 | |
580 | -public Q_SLOTS: |
581 | - void run(); |
582 | - void stop() { server->stop(); } |
583 | - |
584 | -private: |
585 | - std::mutex mutex; |
586 | - std::condition_variable started_cv; |
587 | - bool mir_running{false}; |
588 | - |
589 | - const QSharedPointer<MirServer> server; |
590 | -}; |
591 | - |
592 | - |
593 | -class QMirServer: public QObject |
594 | -{ |
595 | - Q_OBJECT |
596 | - |
597 | -public: |
598 | - QMirServer(const QSharedPointer<MirServer> &config, QObject* parent=0); |
599 | - ~QMirServer(); |
600 | - |
601 | -Q_SIGNALS: |
602 | - void run(); |
603 | - void stop(); |
604 | - |
605 | -protected Q_SLOTS: |
606 | - void shutDownMirServer(); |
607 | - void shutDownQApplication(); |
608 | - |
609 | -private: |
610 | - QThread m_mirThread; |
611 | - MirServerWorker *m_mirServer; |
612 | +protected: |
613 | + QMirServerPrivate * const d_ptr; |
614 | + |
615 | +private: |
616 | Q_DISABLE_COPY(QMirServer) |
617 | + Q_DECLARE_PRIVATE(QMirServer) |
618 | }; |
619 | |
620 | #endif // QMIRSERVER_H |
621 | |
622 | === added file 'src/platforms/mirserver/qmirserver_p.cpp' |
623 | --- src/platforms/mirserver/qmirserver_p.cpp 1970-01-01 00:00:00 +0000 |
624 | +++ src/platforms/mirserver/qmirserver_p.cpp 2015-05-27 12:12:52 +0000 |
625 | @@ -0,0 +1,53 @@ |
626 | +/* |
627 | + * Copyright (C) 2015 Canonical, Ltd. |
628 | + * |
629 | + * This program is free software: you can redistribute it and/or modify it under |
630 | + * the terms of the GNU Lesser General Public License version 3, as published by |
631 | + * the Free Software Foundation. |
632 | + * |
633 | + * This program is distributed in the hope that it will be useful, but WITHOUT |
634 | + * ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, |
635 | + * SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU |
636 | + * Lesser General Public License for more details. |
637 | + * |
638 | + * You should have received a copy of the GNU Lesser General Public License |
639 | + * along with this program. If not, see <http://www.gnu.org/licenses/>. |
640 | + */ |
641 | + |
642 | +// Mir |
643 | +#include <mir/main_loop.h> |
644 | + |
645 | +// local |
646 | +#include "qmirserver_p.h" |
647 | + |
648 | + |
649 | +void MirServerThread::run() |
650 | +{ |
651 | + auto const main_loop = server->the_main_loop(); |
652 | + // By enqueuing the notification code in the main loop, we are |
653 | + // ensuring that the server has really and fully started before |
654 | + // leaving wait_for_startup(). |
655 | + main_loop->enqueue( |
656 | + this, |
657 | + [&] |
658 | + { |
659 | + std::lock_guard<std::mutex> lock(mutex); |
660 | + mir_running = true; |
661 | + started_cv.notify_one(); |
662 | + }); |
663 | + |
664 | + server->run(); // blocks until Mir server stopped |
665 | + Q_EMIT stopped(); |
666 | +} |
667 | + |
668 | +void MirServerThread::stop() |
669 | +{ |
670 | + server->stop(); |
671 | +} |
672 | + |
673 | +bool MirServerThread::waitForMirStartup() |
674 | +{ |
675 | + std::unique_lock<decltype(mutex)> lock(mutex); |
676 | + started_cv.wait_for(lock, std::chrono::seconds{10}, [&]{ return mir_running; }); |
677 | + return mir_running; |
678 | +} |
679 | |
680 | === added file 'src/platforms/mirserver/qmirserver_p.h' |
681 | --- src/platforms/mirserver/qmirserver_p.h 1970-01-01 00:00:00 +0000 |
682 | +++ src/platforms/mirserver/qmirserver_p.h 2015-05-27 12:12:52 +0000 |
683 | @@ -0,0 +1,67 @@ |
684 | +/* |
685 | + * Copyright (C) 2015 Canonical, Ltd. |
686 | + * |
687 | + * This program is free software: you can redistribute it and/or modify it under |
688 | + * the terms of the GNU Lesser General Public License version 3, as published by |
689 | + * the Free Software Foundation. |
690 | + * |
691 | + * This program is distributed in the hope that it will be useful, but WITHOUT |
692 | + * ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, |
693 | + * SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU |
694 | + * Lesser General Public License for more details. |
695 | + * |
696 | + * You should have received a copy of the GNU Lesser General Public License |
697 | + * along with this program. If not, see <http://www.gnu.org/licenses/>. |
698 | + */ |
699 | + |
700 | +#ifndef QMIRSERVER_P_H |
701 | +#define QMIRSERVER_P_H |
702 | + |
703 | +// Qt |
704 | +#include <QThread> |
705 | +#include <QSharedPointer> |
706 | + |
707 | +// std |
708 | +#include <condition_variable> |
709 | +#include <mutex> |
710 | + |
711 | +// local |
712 | +#include "mirserver.h" |
713 | + |
714 | +class QMirServer; |
715 | +class MirServerThread; |
716 | + |
717 | +struct QMirServerPrivate |
718 | +{ |
719 | + QSharedPointer<MirServer> server; |
720 | + MirServerThread *serverThread; |
721 | +}; |
722 | + |
723 | + |
724 | +class MirServerThread : public QThread |
725 | +{ |
726 | + Q_OBJECT |
727 | + |
728 | +public: |
729 | + MirServerThread(const QSharedPointer<MirServer> &server) |
730 | + : server(server) |
731 | + {} |
732 | + |
733 | + bool waitForMirStartup(); |
734 | + |
735 | +Q_SIGNALS: |
736 | + void stopped(); |
737 | + |
738 | +public Q_SLOTS: |
739 | + void run() override; |
740 | + void stop(); |
741 | + |
742 | +private: |
743 | + std::mutex mutex; |
744 | + std::condition_variable started_cv; |
745 | + bool mir_running{false}; |
746 | + |
747 | + const QSharedPointer<MirServer> server; |
748 | +}; |
749 | + |
750 | +#endif // QMIRSERVER_P_H |
PASSED: Continuous integration, rev:331 jenkins. qa.ubuntu. com/job/ qtmir-ci/ 258/ jenkins. qa.ubuntu. com/job/ qtmir-vivid- amd64-ci/ 109 jenkins. qa.ubuntu. com/job/ qtmir-vivid- armhf-ci/ 109 jenkins. qa.ubuntu. com/job/ qtmir-vivid- armhf-ci/ 109/artifact/ work/output/ *zip*/output. zip
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/qtmir- ci/258/ rebuild
http://