QtMir

Merge lp:~dandrader/qtmir/initialSurfaceGeom into lp:qtmir

  1. initialSurfaceGeom
  2. Merge into trunk
Proposed by Daniel d'Andrada on 2016-01-19
Status: Merged
Approved by: Nick Dedekind on 2016-01-26
Approved revision: no longer in the source branch.
Merged at revision: 443
Proposed branch: lp:~dandrader/qtmir/initialSurfaceGeom
Merge into: lp:qtmir
Prerequisite: lp:~dandrader/qtmir/removeUselessClass
Diff against target: 256 lines (+84/-16)
9 files modified
src/modules/Unity/Application/application.cpp (+16/-0)
src/modules/Unity/Application/application.h (+3/-0)
src/modules/Unity/Application/application_manager.cpp (+24/-1)
src/modules/Unity/Application/application_manager.h (+2/-0)
src/platforms/mirserver/mirserver.cpp (+8/-1)
src/platforms/mirserver/mirserver.h (+3/-1)
src/platforms/mirserver/mirwindowmanager.cpp (+20/-10)
src/platforms/mirserver/mirwindowmanager.h (+7/-2)
tests/mirserver/WindowManager/window_manager.cpp (+1/-1)
To merge this branch: bzr merge lp:~dandrader/qtmir/initialSurfaceGeom
Reviewer Review Type Date Requested Status
Michał Sawicz Abstain on 2016-01-27
Nick Dedekind (community) 2016-01-19 Approve on 2016-01-26
Unity8 CI Bot continuous-integration Needs Fixing on 2016-01-22
PS Jenkins bot continuous-integration Needs Fixing on 2016-01-22
Review via email: mp+283227@code.launchpad.net

Commit Message

Let shell decide the initial surface size

Description of the Change

* Are there any related MPs required for this MP to build/function as expected? Please list.
https://code.launchpad.net/~dandrader/unity-api/initialSurfaceGeom/+merge/283222
https://code.launchpad.net/~dandrader/unity8/initialSurfaceGeom/+merge/283223

* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

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

FAILED: Continuous integration, rev:436
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-1-ci/22/
Executed test runs:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-1-ci/22/rebuild

review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Unity8 CI Bot (unity8-ci-bot) wrote :

FAILED: Continuous integration, rev:437
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-1-ci/26/
Executed test runs:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-1-ci/26/rebuild

review: Needs Fixing (continuous-integration)
Nick Dedekind (nick-dedekind) wrote :

Really don't like the way we have to get the window manager pointer, but mir doesnt seem to offer it up except from the configuration, which i dont think we have access to either...

Perhaps caching and returning the pointer from the MirServer would be neater?
http://pastebin.ubuntu.com/14600571/

Otherwise its Tested and working.

review: Needs Information
Daniel d'Andrada (dandrader) wrote :

Ok, thanks for the patch.

On 22/01/2016 14:32, Nick Dedekind wrote:
> Review: Needs Information
>
> Really don't like the way we have to get the window manager pointer, but mir doesnt seem to offer it up except from the configuration, which i dont think we have access to either...
>
> Perhaps caching and returning the pointer from the MirServer would be neater?
> http://pastebin.ubuntu.com/14600571/
>
> Otherwise its Tested and working.
>
> Diff comments:
>
>> === modified file 'src/platforms/mirserver/mirwindowmanager.cpp'
>> --- src/platforms/mirserver/mirwindowmanager.cpp 2015-12-02 12:27:45 +0000
>> +++ src/platforms/mirserver/mirwindowmanager.cpp 2016-01-20 20:00:49 +0000
>> @@ -174,9 +184,19 @@
>> }
>> }
>>
>> -std::unique_ptr<MirWindowManager> MirWindowManager::create(
>> +std::shared_ptr<MirWindowManager> MirWindowManager::create(
>> mir::shell::FocusController* /*focus_controller*/,
>> const std::shared_ptr<mir::shell::DisplayLayout> &displayLayout)
>> {
>> - return std::make_unique<MirWindowManagerImpl>(displayLayout);
>> -}
>> + auto wm = std::make_shared<MirWindowManagerImpl>(displayLayout);
>> + m_instance = wm;
>> + return wm;
>> +}
>> +
>> +MirWindowManager *MirWindowManager::instance()
>> +{
>> + // yeah, ugly
>> + return m_instance.lock().get();
> Maybe return the shared_ptr here from the weak::lock()? Bit safer.
>
>> +}
>> +
>> +std::weak_ptr<MirWindowManager> MirWindowManager::m_instance;
>

PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Unity8 CI Bot (unity8-ci-bot) wrote :

FAILED: Continuous integration, rev:438
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-1-ci/36/
Executed test runs:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-1-ci/36/rebuild

review: Needs Fixing (continuous-integration)
Nick Dedekind (nick-dedekind) 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. dependency

review: Approve
Michał Sawicz (saviq) wrote :

Please rebase on the rewritten prerequisite, otherwise we're conflicting:

https://ci-train.ubuntu.com/job/ubuntu-landing-019-1-build/19/console

review: Approve
Michał Sawicz (saviq) wrote :

Rather.

review: Needs Fixing
Daniel d'Andrada (dandrader) wrote :

> Please rebase on the rewritten prerequisite, otherwise we're conflicting:
>
> https://ci-train.ubuntu.com/job/ubuntu-landing-019-1-build/19/console

Done.

Michał Sawicz (saviq) :
review: Abstain
lp:~dandrader/qtmir/initialSurfaceGeom updated on 2016-02-11
394. By Nick Dedekind on 2016-02-01

merged with trunk

395. By Nick Dedekind on 2016-02-10

merged with trunk

396. By Daniel d'Andrada on 2016-02-11

Remove the useless TaskController

It was just forwarding calls between ApplicationManager and ApplicationCrontroller.
Had no logic of its own.

ApplicationCrontroller was then renamed to TaskController as the latter has a better API and it also keeps ApplicationManager code more or less untouched.

Tests have been improved a bit by better emulating TaskController behavior

397. By Daniel d'Andrada on 2016-02-11

Let shell decide the initial surface size

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/modules/Unity/Application/application.cpp'
2--- src/modules/Unity/Application/application.cpp 2016-02-11 11:55:56 +0000
3+++ src/modules/Unity/Application/application.cpp 2016-02-11 11:55:57 +0000
4@@ -798,4 +798,20 @@
5 connect(m_closeTimer, &Timer::timeout, this, &Application::stop);
6 }
7
8+QSize Application::initialSurfaceSize() const
9+{
10+ return m_initialSurfaceSize;
11+}
12+
13+void Application::setInitialSurfaceSize(const QSize &size)
14+{
15+ qCDebug(QTMIR_APPLICATIONS).nospace() << "Application::setInitialSurfaceSize - appId=" << appId()
16+ << " size=" << size;
17+
18+ if (size != m_initialSurfaceSize) {
19+ m_initialSurfaceSize = size;
20+ Q_EMIT initialSurfaceSizeChanged(m_initialSurfaceSize);
21+ }
22+}
23+
24 } // namespace qtmir
25
26=== modified file 'src/modules/Unity/Application/application.h'
27--- src/modules/Unity/Application/application.h 2016-02-11 11:55:56 +0000
28+++ src/modules/Unity/Application/application.h 2016-02-11 11:55:57 +0000
29@@ -108,6 +108,8 @@
30 bool isTouchApp() const override;
31 bool exemptFromLifecycle() const override;
32 void setExemptFromLifecycle(bool) override;
33+ QSize initialSurfaceSize() const override;
34+ void setInitialSurfaceSize(const QSize &size) override;
35
36 void setStage(Stage stage);
37
38@@ -188,6 +190,7 @@
39 ProcessState m_processState;
40 AbstractTimer *m_closeTimer;
41 bool m_exemptFromLifecycle;
42+ QSize m_initialSurfaceSize;
43
44 friend class ApplicationManager;
45 friend class SessionManager;
46
47=== modified file 'src/modules/Unity/Application/application_manager.cpp'
48--- src/modules/Unity/Application/application_manager.cpp 2016-02-11 11:55:56 +0000
49+++ src/modules/Unity/Application/application_manager.cpp 2016-02-11 11:55:57 +0000
50@@ -32,6 +32,7 @@
51 #include "sessionlistener.h"
52 #include "sessionauthorizer.h"
53 #include "logging.h"
54+#include <mirwindowmanager.h>
55
56 // mir
57 #include <mir/scene/surface.h>
58@@ -118,7 +119,7 @@
59 return nullptr;
60 }
61
62- auto mirServer = nativeInterface->m_mirServer;
63+ auto mirServer = nativeInterface->m_mirServer.lock();
64
65 SessionListener *sessionListener = static_cast<SessionListener*>(nativeInterface->nativeResourceForIntegration("SessionListener"));
66 SessionAuthorizer *sessionAuthorizer = static_cast<SessionAuthorizer*>(nativeInterface->nativeResourceForIntegration("SessionAuthorizer"));
67@@ -146,6 +147,9 @@
68 connectToSessionListener(appManager, sessionListener);
69 connectToSessionAuthorizer(appManager, sessionAuthorizer);
70 connectToTaskController(appManager, taskController.data());
71+ connect(mirServer->windowManager(), &MirWindowManager::sessionAboutToCreateSurface,
72+ appManager, &ApplicationManager::onSessionAboutToCreateSurface,
73+ Qt::BlockingQueuedConnection);
74
75 // Emit signal to notify Upstart that Mir is ready to receive client connections
76 // see http://upstart.ubuntu.com/cookbook/#expect-stop
77@@ -858,4 +862,23 @@
78 return nullptr;
79 }
80
81+void ApplicationManager::onSessionAboutToCreateSurface(
82+ const std::shared_ptr<mir::scene::Session> &session, int type, QSize &size)
83+{
84+ if (type == mir_surface_type_normal) {
85+ Application* application = findApplicationWithSession(session);
86+
87+ if (application) {
88+ qCDebug(QTMIR_APPLICATIONS).nospace() << "ApplicationManager::onSessionAboutToCreateSurface appId="
89+ << application->appId();
90+ size = application->initialSurfaceSize();
91+ } else {
92+ qCDebug(QTMIR_APPLICATIONS).nospace() << "ApplicationManager::onSessionAboutToCreateSurface unknown app";
93+ }
94+ } else {
95+ qCDebug(QTMIR_APPLICATIONS).nospace() << "ApplicationManager::onSessionAboutToCreateSurface type=" << type
96+ << " NOOP";
97+ }
98+}
99+
100 } // namespace qtmir
101
102=== modified file 'src/modules/Unity/Application/application_manager.h'
103--- src/modules/Unity/Application/application_manager.h 2016-02-11 11:55:56 +0000
104+++ src/modules/Unity/Application/application_manager.h 2016-02-11 11:55:57 +0000
105@@ -137,6 +137,8 @@
106
107 private Q_SLOTS:
108 void onAppDataChanged(const int role);
109+ void onSessionAboutToCreateSurface(const std::shared_ptr<mir::scene::Session> &session,
110+ int type, QSize &size);
111
112 private:
113 void setFocused(Application *application);
114
115=== modified file 'src/platforms/mirserver/mirserver.cpp'
116--- src/platforms/mirserver/mirserver.cpp 2015-11-10 11:07:23 +0000
117+++ src/platforms/mirserver/mirserver.cpp 2016-02-11 11:55:57 +0000
118@@ -107,7 +107,9 @@
119 override_the_window_manager_builder([this](mir::shell::FocusController* focus_controller)
120 -> std::shared_ptr<mir::shell::WindowManager>
121 {
122- return {MirWindowManager::create(focus_controller, the_shell_display_layout())};
123+ auto windowManager = MirWindowManager::create(focus_controller, the_shell_display_layout());
124+ m_windowManager = windowManager;
125+ return windowManager;
126 });
127
128 wrap_display_configuration_policy(
129@@ -193,3 +195,8 @@
130 std::weak_ptr<MirShell> m_shell = the_shell();
131 return m_shell.lock().get();
132 }
133+
134+MirWindowManager *MirServer::windowManager()
135+{
136+ return m_windowManager.lock().get();
137+}
138
139=== modified file 'src/platforms/mirserver/mirserver.h'
140--- src/platforms/mirserver/mirserver.h 2015-08-20 10:16:54 +0000
141+++ src/platforms/mirserver/mirserver.h 2016-02-11 11:55:57 +0000
142@@ -27,6 +27,7 @@
143 using MirShell = mir::shell::Shell;
144 class PromptSessionListener;
145 class ScreenController;
146+class MirWindowManager;
147
148 // We use virtual inheritance of mir::Server to facilitate derived classes (e.g. testing)
149 // calling initialization functions before MirServer is constructed.
150@@ -61,11 +62,12 @@
151 SessionAuthorizer *sessionAuthorizer();
152 SessionListener *sessionListener();
153 PromptSessionListener *promptSessionListener();
154+ MirWindowManager *windowManager();
155 MirShell *shell();
156
157 private:
158 std::weak_ptr<MirShell> m_shell;
159- std::shared_ptr<QtEventFeeder> m_qtEventFeeder;
160+ std::weak_ptr<MirWindowManager> m_windowManager;
161 const QSharedPointer<ScreenController> m_screenController;
162 };
163
164
165=== modified file 'src/platforms/mirserver/mirwindowmanager.cpp'
166--- src/platforms/mirserver/mirwindowmanager.cpp 2015-12-02 12:27:45 +0000
167+++ src/platforms/mirserver/mirwindowmanager.cpp 2016-02-11 11:55:57 +0000
168@@ -100,16 +100,26 @@
169 {
170 tracepoint(qtmirserver, surfacePlacementStart);
171
172- // TODO: Callback unity8 so that it can make a decision on that.
173- // unity8 must bear in mind that the called function will be on a Mir thread though.
174- // The QPA shouldn't be deciding for itself on such things.
175-
176+ QSize initialSize;
177+ // can be connected to via Qt::BlockingQueuedConnection to alter surface initial size
178+ {
179+ int surfaceType = requestParameters.type.is_set() ? requestParameters.type.value() : -1;
180+ Q_EMIT sessionAboutToCreateSurface(session, surfaceType, initialSize);
181+ }
182 ms::SurfaceCreationParameters placedParameters = requestParameters;
183
184- // Just make it fullscreen for now
185- mir::geometry::Rectangle rect{requestParameters.top_left, requestParameters.size};
186- m_displayLayout->size_to_output(rect);
187- placedParameters.size = rect.size;
188+ if (initialSize.isValid()) {
189+ placedParameters.size.width = mir::geometry::Width(initialSize.width());
190+ placedParameters.size.height = mir::geometry::Height(initialSize.height());
191+ } else {
192+ qCWarning(QTMIR_MIR_MESSAGES) << "MirWindowManagerImpl::add_surface(): didn't get a initial surface"
193+ " size from shell. Falling back to fullscreen placement";
194+ // This is bad. Fallback to fullscreen
195+ mir::geometry::Rectangle rect{requestParameters.top_left, requestParameters.size};
196+ m_displayLayout->size_to_output(rect);
197+ placedParameters.size = rect.size;
198+ }
199+
200
201 qCDebug(QTMIR_MIR_MESSAGES) << "MirWindowManagerImpl::add_surface(): size requested ("
202 << requestParameters.size.width.as_int() << "," << requestParameters.size.height.as_int() << ") and placed ("
203@@ -174,9 +184,9 @@
204 }
205 }
206
207-std::unique_ptr<MirWindowManager> MirWindowManager::create(
208+std::shared_ptr<MirWindowManager> MirWindowManager::create(
209 mir::shell::FocusController* /*focus_controller*/,
210 const std::shared_ptr<mir::shell::DisplayLayout> &displayLayout)
211 {
212- return std::make_unique<MirWindowManagerImpl>(displayLayout);
213+ return std::make_shared<MirWindowManagerImpl>(displayLayout);
214 }
215
216=== modified file 'src/platforms/mirserver/mirwindowmanager.h'
217--- src/platforms/mirserver/mirwindowmanager.h 2015-11-19 12:55:57 +0000
218+++ src/platforms/mirserver/mirwindowmanager.h 2016-02-11 11:55:57 +0000
219@@ -20,6 +20,7 @@
220 #include <mir/shell/window_manager.h>
221
222 #include <QObject>
223+#include <QSize>
224
225 namespace mir {
226 namespace shell {
227@@ -33,10 +34,14 @@
228 Q_OBJECT
229
230 public:
231-
232- static std::unique_ptr<MirWindowManager> create(
233+ static std::shared_ptr<MirWindowManager> create(
234 mir::shell::FocusController* focus_controller,
235 const std::shared_ptr<mir::shell::DisplayLayout> &displayLayout);
236+
237+Q_SIGNALS:
238+ // requires Qt::BlockingQueuedConnection!!
239+ void sessionAboutToCreateSurface(const std::shared_ptr<mir::scene::Session> &session,
240+ int type, QSize &size);
241 };
242
243 #endif /* QPAMIRSERVER_WINDOW_MANAGER_H */
244
245=== modified file 'tests/mirserver/WindowManager/window_manager.cpp'
246--- tests/mirserver/WindowManager/window_manager.cpp 2016-01-22 17:51:40 +0000
247+++ tests/mirserver/WindowManager/window_manager.cpp 2016-02-11 11:55:57 +0000
248@@ -81,7 +81,7 @@
249
250 StubFocusController focus_controller;
251
252- const std::unique_ptr<MirWindowManager> window_manager =
253+ const std::shared_ptr<MirWindowManager> window_manager =
254 MirWindowManager::create(&focus_controller, mock_display_layout);
255
256 const Rectangle arbitrary_display{{0, 0}, {97, 101}};

Subscribers

People subscribed via source and target branches