QtMir

Merge lp:~unity-team/qtmir/multiSurfaceApp into lp:qtmir

  1. multiSurfaceApp
  2. Merge into trunk
Proposed by Michał Sawicz on 2015-12-07
Status: Merged
Approved by: Gerry Boland on 2015-12-09
Approved revision: 426
Merged at revision: 426
Proposed branch: lp:~unity-team/qtmir/multiSurfaceApp
Merge into: lp:qtmir
Prerequisite: lp:~unity-team/qtmir/surfaceItemFillMode
Diff against target: 586 lines (+135/-99)
10 files modified
src/modules/Unity/Application/mirsurface.cpp (+15/-19)
src/modules/Unity/Application/mirsurfaceitem.cpp (+1/-2)
src/modules/Unity/Application/mirsurfacemanager.cpp (+1/-1)
src/modules/Unity/Application/session.cpp (+78/-57)
src/modules/Unity/Application/session.h (+8/-4)
src/modules/Unity/Application/session_interface.h (+16/-4)
tests/modules/ApplicationManager/application_manager_test.cpp (+1/-1)
tests/modules/SessionManager/session_test.cpp (+3/-3)
tests/modules/common/fake_session.h (+4/-2)
tests/modules/common/mock_session.h (+8/-6)
To merge this branch: bzr merge lp:~unity-team/qtmir/multiSurfaceApp
Reviewer Review Type Date Requested Status
Gerry Boland 2015-12-07 Approve on 2015-12-09
PS Jenkins bot continuous-integration 2015-12-07 Needs Fixing on 2015-12-07
Michael Zanetti 2015-12-07 Pending
Review via email: mp+279759@code.launchpad.net

This proposal supersedes a proposal from 2015-12-01.

Commit Message

Make Session hold multiple surfaces

+ Standardize MirSurface debug messages and account for multiple surfaces per app

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/unity8/multiSurfaceApp/+merge/279112

Because of prereq:
https://code.launchpad.net/~dandrader/unity-api/surfaceItemFillMode2/+merge/278955

* 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.
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Gerry Boland (gerboland) wrote : Posted in a previous version of this proposal

Please merge trunk, many conflicts have appeared.

+void Session::appendSurface
+void Session::reallyAppendSurface
these names confuse me. Perhaps rename to something like:
appendSurface -> registerSurface
reallyAppendSurface -> appendSurface

 qDebug() << "Application::suspend - no surface to call stopFrameDropper() on!";
I think we can move this to category logging

+ return const_cast<ObjectListModel<MirSurfaceInterface>*>(&m_surfaces);
const_cast needed here? Can't the pointer be a pointer to a const object?

=== modified file 'src/modules/Unity/Application/session.h'
+ QList<MirSurfaceInterface*> m_blankSurfaces;
What use is this? I only see you append to and remove from this list, but never use it for anything.

+ Q_PROPERTY(MirSurfaceInterface* lastSurface READ lastSurface NOTIFY lastSurfaceChanged CONSTANT);
Can you add a "REMOVEME" comment to this, this is not an API I want to keep. It also not very correct, as what if the surface pointed to is removed, but other surfaces exist - should it not be updated?

review: Needs Fixing
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Daniel d'Andrada (dandrader) wrote : Posted in a previous version of this proposal

> Please merge trunk, many conflicts have appeared.

Done.

> +void Session::appendSurface
> +void Session::reallyAppendSurface
> these names confuse me. Perhaps rename to something like:
> appendSurface -> registerSurface
> reallyAppendSurface -> appendSurface

Done.

> qDebug() << "Application::suspend - no surface to call stopFrameDropper()
> on!";
> I think we can move this to category logging

Done.

> + return const_cast<ObjectListModel<MirSurfaceInterface>*>(&m_surfaces);
> const_cast needed here? Can't the pointer be a pointer to a const object?

Right. I was just blindly following the signature of childSessions. Fixed.

> === modified file 'src/modules/Unity/Application/session.h'
> + QList<MirSurfaceInterface*> m_blankSurfaces;
> What use is this? I only see you append to and remove from this list, but
> never use it for anything.

Originally it was meant to keep track of the surfaces that were registered but not yet appended. But later it turned out that this wasn't needed after all. Removed.

> + Q_PROPERTY(MirSurfaceInterface* lastSurface READ lastSurface NOTIFY
> lastSurfaceChanged CONSTANT);
> Can you add a "REMOVEME" comment to this, this is not an API I want to keep.
> It also not very correct, as what if the surface pointed to is removed, but
> other surfaces exist - should it not be updated?

This will be removed once unity8 starts using Session.surfaces. Ie, once it actually starts supporting multiple surfaces per application. It was just way too awkward to have untiy8 fetching the last surface from Session.surfaces in QML. Hence this property.

Added the FIXME comment to the property declaration.

lastSurface is being updated correctly as the surfaces model changes.

Gerry Boland (gerboland) wrote : Posted in a previous version of this proposal

Code looks ok to me. There's a unity8 change needed to enable this fully though. Could you please link that MP here.

review: Approve (code)
Daniel d'Andrada (dandrader) wrote : Posted in a previous version of this proposal

> Code looks ok to me. There's a unity8 change needed to enable this fully
> though. Could you please link that MP here.

Done

Michael Zanetti (mzanetti) wrote : Posted in a previous version of this proposal

I've tested it and it works.

review: Approve (functional testing)
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Michael Zanetti (mzanetti) wrote : Posted in a previous version of this proposal

have a bunch of build failures here

tests/framework/fake_mirsurface.h:113: Error: Not a signal declaration

trying with -DNO_TESTS:

In file included from /home/micha/Develop/reviews/multiSurfaceApp/src/platforms/mirserver/mirsingleton.cpp:1:0:
/home/micha/Develop/reviews/multiSurfaceApp/src/platforms/mirserver/mirsingleton.h:33:10: error: ‘void qtmir::Mir::setCursorName(const QString&)’ marked ‘override’, but does not override
     void setCursorName(const QString &cursorName) override;
          ^
/home/micha/Develop/reviews/multiSurfaceApp/src/platforms/mirserver/mirsingleton.h:34:13: error: ‘QString qtmir::Mir::cursorName() const’ marked ‘override’, but does not override
     QString cursorName() const override;
             ^
/home/micha/Develop/reviews/multiSurfaceApp/src/platforms/mirserver/mirsingleton.cpp: In member function ‘void qtmir::Mir::setCursorName(const QString&)’:
/home/micha/Develop/reviews/multiSurfaceApp/src/platforms/mirserver/mirsingleton.cpp:26:46: error: ‘cursorNameChanged’ was not declared in this scope
         Q_EMIT cursorNameChanged(m_cursorName);

Also got this when trying to build on the phone

/home/phablet/real_home/qtmir-multiSurfaceApp/src/modules/Unity/Application/session.cpp:315:65: error: 'm_surface' was not declared in this scope
     qCDebug(QTMIR_SESSIONS) << "Session::close - " << name() << m_surface;
                                                                 ^
/home/phablet/real_home/qtmir-multiSurfaceApp/src/modules/Unity/Application/session.cpp:316:9: error: 'm_surface' was not declared in this scope
     if (m_surface) {
         ^

Not sure if the last one is just masked by the other on my desktop build or if you fixed that in the meantime.

review: Needs Fixing
Daniel d'Andrada (dandrader) wrote : Posted in a previous version of this proposal

On 04/12/2015 14:12, Michael Zanetti wrote:
> Review: Needs Fixing
>
> have a bunch of build failures here
>
> tests/framework/fake_mirsurface.h:113: Error: Not a signal declaration
>
> trying with -DNO_TESTS:
>
> In file included from /home/micha/Develop/reviews/multiSurfaceApp/src/platforms/mirserver/mirsingleton.cpp:1:0:
> /home/micha/Develop/reviews/multiSurfaceApp/src/platforms/mirserver/mirsingleton.h:33:10: error: ‘void qtmir::Mir::setCursorName(const QString&)’ marked ‘override’, but does not override
> void setCursorName(const QString &cursorName) override;
> ^
> /home/micha/Develop/reviews/multiSurfaceApp/src/platforms/mirserver/mirsingleton.h:34:13: error: ‘QString qtmir::Mir::cursorName() const’ marked ‘override’, but does not override
> QString cursorName() const override;
> ^
> /home/micha/Develop/reviews/multiSurfaceApp/src/platforms/mirserver/mirsingleton.cpp: In member function ‘void qtmir::Mir::setCursorName(const QString&)’:
> /home/micha/Develop/reviews/multiSurfaceApp/src/platforms/mirserver/mirsingleton.cpp:26:46: error: ‘cursorNameChanged’ was not declared in this scope
> Q_EMIT cursorNameChanged(m_cursorName);
>
>
>
> Also got this when trying to build on the phone
>
> /home/phablet/real_home/qtmir-multiSurfaceApp/src/modules/Unity/Application/session.cpp:315:65: error: 'm_surface' was not declared in this scope
> qCDebug(QTMIR_SESSIONS) << "Session::close - " << name() << m_surface;
> ^
> /home/phablet/real_home/qtmir-multiSurfaceApp/src/modules/Unity/Application/session.cpp:316:9: error: 'm_surface' was not declared in this scope
> if (m_surface) {
> ^
>
> Not sure if the last one is just masked by the other on my desktop build or if you fixed that in the meantime.

Just built it here. You sure you had
lp:~dandrader/unity-api/surfaceItemFillMode2 installed?

Daniel d'Andrada (dandrader) wrote : Posted in a previous version of this proposal

> On 04/12/2015 14:12, Michael Zanetti wrote:
> > Review: Needs Fixing
> >
> > have a bunch of build failures here
> >
> > tests/framework/fake_mirsurface.h:113: Error: Not a signal declaration
> >
> > trying with -DNO_TESTS:
> >
> > In file included from /home/micha/Develop/reviews/multiSurfaceApp/src/platfo
> rms/mirserver/mirsingleton.cpp:1:0:
> > /home/micha/Develop/reviews/multiSurfaceApp/src/platforms/mirserver/mirsingl
> eton.h:33:10: error: ‘void qtmir::Mir::setCursorName(const QString&)’ marked
> ‘override’, but does not override
> > void setCursorName(const QString &cursorName) override;
> > ^
> > /home/micha/Develop/reviews/multiSurfaceApp/src/platforms/mirserver/mirsingl
> eton.h:34:13: error: ‘QString qtmir::Mir::cursorName() const’ marked
> ‘override’, but does not override
> > QString cursorName() const override;
> > ^
> > /home/micha/Develop/reviews/multiSurfaceApp/src/platforms/mirserver/mirsingl
> eton.cpp: In member function ‘void qtmir::Mir::setCursorName(const QString&)’:
> > /home/micha/Develop/reviews/multiSurfaceApp/src/platforms/mirserver/mirsingl
> eton.cpp:26:46: error: ‘cursorNameChanged’ was not declared in this scope
> > Q_EMIT cursorNameChanged(m_cursorName);
> >
> >
> >
> > Also got this when trying to build on the phone
> >
> > /home/phablet/real_home/qtmir-
> multiSurfaceApp/src/modules/Unity/Application/session.cpp:315:65: error:
> 'm_surface' was not declared in this scope
> > qCDebug(QTMIR_SESSIONS) << "Session::close - " << name() << m_surface;
> > ^
> > /home/phablet/real_home/qtmir-
> multiSurfaceApp/src/modules/Unity/Application/session.cpp:316:9: error:
> 'm_surface' was not declared in this scope
> > if (m_surface) {
> > ^
> >
> > Not sure if the last one is just masked by the other on my desktop build or
> if you fixed that in the meantime.
>
>
>
> Just built it here. You sure you had
> lp:~dandrader/unity-api/surfaceItemFillMode2 installed?

sorry, I was building unity8 instead.

Fixed. It was due to a bad merge with polite-close.

PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Gerry Boland (gerboland) wrote :

Will approve on the grounds it has been tested in silo, and code looks reasonable. Code has changed a bit since I approved it before, and lost history makes it hard to see what changed.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/modules/Unity/Application/mirsurface.cpp'
2--- src/modules/Unity/Application/mirsurface.cpp 2015-12-07 12:03:42 +0000
3+++ src/modules/Unity/Application/mirsurface.cpp 2015-12-07 12:03:42 +0000
4@@ -35,6 +35,8 @@
5
6 using namespace qtmir;
7
8+#define DEBUG_MSG qCDebug(QTMIR_SURFACES).nospace() << "MirSurface[" << (void*)this << "," << appId() <<"]::" << __func__
9+
10 namespace {
11
12 // Would be better if QMouseEvent had nativeModifiers
13@@ -217,7 +219,7 @@
14 Q_ASSERT(m_views.isEmpty());
15
16 if (m_session) {
17- m_session->setSurface(nullptr);
18+ m_session->removeSurface(this);
19 }
20
21 QMutexLocker locker(&m_mutex);
22@@ -301,12 +303,11 @@
23
24 locker.unlock();
25 if (updateTexture()) {
26- qCDebug(QTMIR_SURFACES).nospace() << "MirSurface[" << appId() << "]::dropPendingBuffer() dropped=1 left=" << framesPending-1;
27+ DEBUG_MSG << "() dropped=1 left=" << framesPending-1;
28 } else {
29 // If we haven't managed to update the texture, don't keep banging away.
30 m_frameDropperTimer.stop();
31- qCDebug(QTMIR_SURFACES).nospace() << "MirSurface[" << appId() << "]::dropPendingBuffer() dropped=0"
32- << " left=" << framesPending << " - failed to upate texture";
33+ DEBUG_MSG << "() dropped=0" << " left=" << framesPending << " - failed to upate texture";
34 }
35 Q_EMIT frameDropped();
36 } else {
37@@ -320,13 +321,13 @@
38
39 void MirSurface::stopFrameDropper()
40 {
41- qCDebug(QTMIR_SURFACES).nospace() << "MirSurface[" << appId() << "]::stopFrameDropper()";
42+ DEBUG_MSG << "()";
43 m_frameDropperTimer.stop();
44 }
45
46 void MirSurface::startFrameDropper()
47 {
48- qCDebug(QTMIR_SURFACES).nospace() << "MirSurface[" << appId() << "]::startFrameDropper()";
49+ DEBUG_MSG << "()";
50 if (!m_frameDropperTimer.isActive()) {
51 m_frameDropperTimer.start();
52 }
53@@ -421,12 +422,11 @@
54 // Temporary hotfix for http://pad.lv/1483752
55 if (m_session->childSessions()->rowCount() > 0) {
56 // has child trusted session, ignore any focus change attempts
57- qCDebug(QTMIR_SURFACES).nospace() << "MirSurface[" << appId() << "]::setFocus(" << focus
58- << ") - has child trusted session, ignore any focus change attempts";
59+ DEBUG_MSG << "(" << focus << ") - has child trusted session, ignore any focus change attempts";
60 return;
61 }
62
63- qCDebug(QTMIR_SURFACES).nospace() << "MirSurface[" << appId() << "]::setFocus(" << focus << ")";
64+ DEBUG_MSG << "(" << focus << ")";
65
66 if (focus) {
67 m_shell->set_surface_attribute(m_session->session(), m_surface, mir_surface_attrib_focus, mir_surface_focused);
68@@ -452,9 +452,8 @@
69 if (clientIsRunning() && mirSizeIsDifferent) {
70 mir::geometry::Size newMirSize(width, height);
71 m_surface->resize(newMirSize);
72- qCDebug(QTMIR_SURFACES).nospace() << "MirSurface[" << appId() << "]::resize"
73- << " old (" << mirWidth << "," << mirHeight << ")"
74- << ", new (" << width << "," << height << ")";
75+ DEBUG_MSG << " old (" << mirWidth << "," << mirHeight << ")"
76+ << ", new (" << width << "," << height << ")";
77 }
78 }
79
80@@ -681,8 +680,7 @@
81 void MirSurface::registerView(qintptr viewId)
82 {
83 m_views.insert(viewId, MirSurface::View{false});
84- qCDebug(QTMIR_SURFACES).nospace() << "MirSurface[" << appId() << "]::registerView(" << viewId << ")"
85- << " after=" << m_views.count();
86+ DEBUG_MSG << "(" << viewId << ")" << " after=" << m_views.count();
87 if (m_views.count() == 1) {
88 Q_EMIT isBeingDisplayedChanged();
89 }
90@@ -690,9 +688,8 @@
91
92 void MirSurface::unregisterView(qintptr viewId)
93 {
94- qCDebug(QTMIR_SURFACES).nospace() << "MirSurface[" << appId() << "]::unregisterView(" << viewId << ")"
95- << " after=" << m_views.count() << " live=" << m_live;
96 m_views.remove(viewId);
97+ DEBUG_MSG << "(" << viewId << ")" << " after=" << m_views.count() << " live=" << m_live;
98 if (m_views.count() == 0) {
99 Q_EMIT isBeingDisplayedChanged();
100 if (m_session.isNull() || !m_live) {
101@@ -723,7 +720,7 @@
102 }
103
104 if (newVisible != visible()) {
105- qCDebug(QTMIR_SURFACES).nospace() << "MirSurface[" << appId() << "]::updateVisibility(" << newVisible << ")";
106+ DEBUG_MSG << "(" << newVisible << ")";
107
108 m_surface->configure(mir_surface_attrib_visibility,
109 newVisible ? mir_surface_visibility_exposed : mir_surface_visibility_occluded);
110@@ -767,8 +764,7 @@
111
112 void MirSurface::setCursor(const QCursor &cursor)
113 {
114- qCDebug(QTMIR_SURFACES).nospace() << "MirSurface[" << appId() << "]::setCursor("
115- << qtCursorShapeToStr(cursor.shape()) << ")";
116+ DEBUG_MSG << "(" << qtCursorShapeToStr(cursor.shape()) << ")";
117
118 m_cursor = cursor;
119 Q_EMIT cursorChanged(m_cursor);
120
121=== modified file 'src/modules/Unity/Application/mirsurfaceitem.cpp'
122--- src/modules/Unity/Application/mirsurfaceitem.cpp 2015-12-07 12:03:42 +0000
123+++ src/modules/Unity/Application/mirsurfaceitem.cpp 2015-12-07 12:03:42 +0000
124@@ -236,8 +236,6 @@
125 QSGDefaultImageNode *node = static_cast<QSGDefaultImageNode*>(oldNode);
126 if (!node) {
127 node = new QSGDefaultImageNode;
128- node->setTexture(m_textureProvider->texture());
129-
130 node->setMipmapFiltering(QSGTexture::None);
131 node->setHorizontalWrapMode(QSGTexture::ClampToEdge);
132 node->setVerticalWrapMode(QSGTexture::ClampToEdge);
133@@ -246,6 +244,7 @@
134 node->markDirty(QSGNode::DirtyMaterial);
135 }
136 }
137+ node->setTexture(m_textureProvider->texture());
138
139 if (m_fillMode == PadOrCrop) {
140 const QSize &textureSize = m_textureProvider->texture()->textureSize();
141
142=== modified file 'src/modules/Unity/Application/mirsurfacemanager.cpp'
143--- src/modules/Unity/Application/mirsurfacemanager.cpp 2015-10-09 15:35:05 +0000
144+++ src/modules/Unity/Application/mirsurfacemanager.cpp 2015-12-07 12:03:42 +0000
145@@ -110,7 +110,7 @@
146 }
147
148 if (session)
149- session->setSurface(qmlSurface);
150+ session->registerSurface(qmlSurface);
151
152 // Only notify QML of surface creation once it has drawn its first frame.
153 connect(qmlSurface, &MirSurfaceInterface::firstFrameDrawn, this, [=]() {
154
155=== modified file 'src/modules/Unity/Application/session.cpp'
156--- src/modules/Unity/Application/session.cpp 2015-12-07 12:03:42 +0000
157+++ src/modules/Unity/Application/session.cpp 2015-12-07 12:03:42 +0000
158@@ -68,7 +68,6 @@
159 : SessionInterface(parent)
160 , m_session(session)
161 , m_application(nullptr)
162- , m_surface(nullptr)
163 , m_parentSession(nullptr)
164 , m_children(new SessionModel(this))
165 , m_fullscreen(false)
166@@ -108,10 +107,15 @@
167 void Session::doSuspend()
168 {
169 Q_ASSERT(m_state == Session::Suspending);
170- if (m_surface) {
171- m_surface->stopFrameDropper();
172+
173+
174+ auto surfaceList = m_surfaces.list();
175+ if (surfaceList.empty()) {
176+ qCDebug(QTMIR_SESSIONS) << "Application::suspend - no surface to call stopFrameDropper() on!";
177 } else {
178- qDebug() << "Application::suspend - no surface to call stopFrameDropper() on!";
179+ for (int i = 0; i < surfaceList.count(); ++i) {
180+ surfaceList[i]->stopFrameDropper();
181+ }
182 }
183 setState(Suspended);
184 }
185@@ -142,14 +146,9 @@
186 return m_application;
187 }
188
189-MirSurfaceInterface* Session::surface() const
190+const ObjectListModel<MirSurfaceInterface>* Session::surfaces() const
191 {
192- // Only notify QML of surface creation once it has drawn its first frame.
193- if (m_surface && m_surface->isFirstFrameDrawn()) {
194- return m_surface;
195- } else {
196- return nullptr;
197- }
198+ return &m_surfaces;
199 }
200
201 SessionInterface* Session::parentSession() const
202@@ -191,55 +190,57 @@
203 Q_EMIT applicationChanged(application);
204 }
205
206-void Session::setSurface(MirSurfaceInterface *newSurface)
207+void Session::registerSurface(MirSurfaceInterface *newSurface)
208 {
209- qCDebug(QTMIR_SESSIONS) << "Session::setSurface - session=" << name() << "surface=" << newSurface;
210-
211- if (newSurface == m_surface) {
212- return;
213- }
214-
215- if (m_surface) {
216- m_surface->disconnect(this);
217- }
218-
219- MirSurfaceInterface *previousSurface = surface();
220- m_surface = newSurface;
221-
222- if (newSurface) {
223- connect(newSurface, &MirSurfaceInterface::stateChanged,
224- this, &Session::updateFullscreenProperty);
225-
226- // Only notify QML of surface creation once it has drawn its first frame.
227- if (m_surface->isFirstFrameDrawn()) {
228- setState(Running);
229- } else {
230- connect(newSurface, &MirSurfaceInterface::firstFrameDrawn,
231- this, &Session::onFirstSurfaceFrameDrawn);
232- }
233- }
234-
235- if (previousSurface != surface()) {
236- qCDebug(QTMIR_SESSIONS).nospace() << "Session::surfaceChanged - session=" << this
237- << " surface=" << m_surface;
238- Q_EMIT surfaceChanged(m_surface);
239+ qCDebug(QTMIR_SESSIONS) << "Session::resgisterSurface - session=" << name() << "surface=" << newSurface;
240+
241+ // Only notify QML of surface creation once it has drawn its first frame.
242+ if (newSurface->isFirstFrameDrawn()) {
243+ appendSurface(newSurface);
244+ } else {
245+ connect(newSurface, &MirSurfaceInterface::firstFrameDrawn,
246+ this, [this, newSurface]() { this->appendSurface(newSurface); });
247 }
248
249 updateFullscreenProperty();
250 }
251
252-void Session::onFirstSurfaceFrameDrawn()
253-{
254- qCDebug(QTMIR_SESSIONS).nospace() << "Session::surfaceChanged - session=" << this
255- << " surface=" << m_surface;
256- Q_EMIT surfaceChanged(m_surface);
257- setState(Running);
258+void Session::appendSurface(MirSurfaceInterface *newSurface)
259+{
260+ qCDebug(QTMIR_SESSIONS) << "Session::appendSurface - session=" << name() << "surface=" << newSurface;
261+
262+ connect(newSurface, &MirSurfaceInterface::stateChanged,
263+ this, &Session::updateFullscreenProperty);
264+
265+ m_surfaces.insert(m_surfaces.rowCount(), newSurface);
266+
267+ Q_EMIT lastSurfaceChanged(newSurface);
268+
269+ if (m_state == Starting) {
270+ setState(Running);
271+ }
272+}
273+
274+void Session::removeSurface(MirSurfaceInterface* surface)
275+{
276+ qCDebug(QTMIR_SESSIONS) << "Session::removeSurface - session=" << name() << "surface=" << surface;
277+
278+ surface->disconnect(this);
279+
280+ if (m_surfaces.contains(surface)) {
281+ bool lastSurfaceWasRemoved = lastSurface() == surface;
282+ m_surfaces.remove(surface);
283+ if (lastSurfaceWasRemoved) {
284+ Q_EMIT lastSurfaceChanged(lastSurface());
285+ }
286+ }
287 }
288
289 void Session::updateFullscreenProperty()
290 {
291- if (m_surface) {
292- setFullscreen(m_surface->state() == Mir::FullscreenState);
293+ if (m_surfaces.rowCount() > 0) {
294+ // TODO: Figure out something better
295+ setFullscreen(m_surfaces.list().at(0)->state() == Mir::FullscreenState);
296 } else {
297 // Keep the current value of the fullscreen property until we get a new
298 // surface
299@@ -289,8 +290,11 @@
300 Q_ASSERT(m_suspendTimer->isActive());
301 m_suspendTimer->stop();
302 } else if (m_state == Suspended) {
303- Q_ASSERT(m_surface);
304- m_surface->startFrameDropper();
305+ Q_ASSERT(m_surfaces.rowCount() > 0);
306+ auto surfaceList = m_surfaces.list();
307+ for (int i = 0; i < surfaceList.count(); ++i) {
308+ surfaceList[i]->startFrameDropper();
309+ }
310 }
311
312 session()->set_lifecycle_state(mir_lifecycle_state_resumed);
313@@ -308,9 +312,11 @@
314
315 void Session::close()
316 {
317- qCDebug(QTMIR_SESSIONS) << "Session::close - " << name() << m_surface;
318- if (m_surface) {
319- m_surface->close();
320+ qCDebug(QTMIR_SESSIONS) << "Session::close - " << name();
321+
322+ auto surfaceList = m_surfaces.list();
323+ for (int i = 0; i < surfaceList.count(); ++i) {
324+ surfaceList[i]->close();
325 }
326 }
327
328@@ -321,10 +327,16 @@
329 if (m_state != Stopped) {
330
331 stopPromptSessions();
332+
333 if (m_suspendTimer->isActive())
334 m_suspendTimer->stop();
335- if (m_surface)
336- m_surface->stopFrameDropper();
337+
338+ {
339+ auto surfaceList = m_surfaces.list();
340+ for (int i = 0; i < surfaceList.count(); ++i) {
341+ surfaceList[i]->stopFrameDropper();
342+ }
343+ }
344
345 foreachChildSession([](SessionInterface* session) {
346 session->stop();
347@@ -460,4 +472,13 @@
348 }
349 }
350
351+MirSurfaceInterface* Session::lastSurface() const
352+{
353+ if (m_surfaces.rowCount() > 0) {
354+ return m_surfaces.list().last();
355+ } else {
356+ return nullptr;
357+ }
358+}
359+
360 } // namespace qtmir
361
362=== modified file 'src/modules/Unity/Application/session.h'
363--- src/modules/Unity/Application/session.h 2015-12-07 12:03:42 +0000
364+++ src/modules/Unity/Application/session.h 2015-12-07 12:03:42 +0000
365@@ -52,14 +52,17 @@
366 //getters
367 QString name() const override;
368 unity::shell::application::ApplicationInfoInterface* application() const override;
369- MirSurfaceInterface* surface() const override;
370+ MirSurfaceInterface* lastSurface() const override;
371+ const ObjectListModel<MirSurfaceInterface>* surfaces() const override;
372 SessionInterface* parentSession() const override;
373 State state() const override;
374 bool fullscreen() const override;
375 bool live() const override;
376
377 void setApplication(unity::shell::application::ApplicationInfoInterface* item) override;
378- void setSurface(MirSurfaceInterface* surface) override;
379+
380+ void registerSurface(MirSurfaceInterface* surface) override;
381+ void removeSurface(MirSurfaceInterface* surface) override;
382
383 void suspend() override;
384 void resume() override;
385@@ -89,7 +92,6 @@
386
387 private Q_SLOTS:
388 void updateFullscreenProperty();
389- void onFirstSurfaceFrameDrawn();
390
391 private:
392 void setParentSession(Session* session);
393@@ -98,9 +100,11 @@
394
395 void stopPromptSessions();
396
397+ void appendSurface(MirSurfaceInterface* surface);
398+
399 std::shared_ptr<mir::scene::Session> m_session;
400 Application* m_application;
401- MirSurfaceInterface* m_surface;
402+ ObjectListModel<MirSurfaceInterface> m_surfaces;
403 SessionInterface* m_parentSession;
404 SessionModel* m_children;
405 bool m_fullscreen;
406
407=== modified file 'src/modules/Unity/Application/session_interface.h'
408--- src/modules/Unity/Application/session_interface.h 2015-12-07 12:03:42 +0000
409+++ src/modules/Unity/Application/session_interface.h 2015-12-07 12:03:42 +0000
410@@ -24,8 +24,12 @@
411 #include <unity/shell/application/ApplicationInfoInterface.h>
412
413 // local
414+#include "objectlistmodel.h"
415 #include "sessionmodel.h"
416
417+// Qt
418+#include <QQmlListProperty>
419+
420 namespace mir {
421 namespace scene {
422 class Session;
423@@ -39,7 +43,12 @@
424
425 class SessionInterface : public QObject {
426 Q_OBJECT
427- Q_PROPERTY(MirSurfaceInterface* surface READ surface NOTIFY surfaceChanged)
428+
429+ // FIXME: Remove this once unity8 starts trully supporting multiple surfaces per applicaton.
430+ // Ie, remove this once untiy8 moves from using this property to using the surfaces one.
431+ Q_PROPERTY(MirSurfaceInterface* lastSurface READ lastSurface NOTIFY lastSurfaceChanged);
432+
433+ Q_PROPERTY(const ObjectListModel<MirSurfaceInterface>* surfaces READ surfaces CONSTANT);
434 Q_PROPERTY(unity::shell::application::ApplicationInfoInterface* application READ application NOTIFY applicationChanged DESIGNABLE false)
435 Q_PROPERTY(SessionInterface* parentSession READ parentSession NOTIFY parentSessionChanged DESIGNABLE false)
436 Q_PROPERTY(SessionModel* childSessions READ childSessions DESIGNABLE false CONSTANT)
437@@ -62,7 +71,8 @@
438 //getters
439 virtual QString name() const = 0;
440 virtual unity::shell::application::ApplicationInfoInterface* application() const = 0;
441- virtual MirSurfaceInterface* surface() const = 0;
442+ virtual MirSurfaceInterface* lastSurface() const = 0;
443+ virtual const ObjectListModel<MirSurfaceInterface>* surfaces() const = 0;
444 virtual SessionInterface* parentSession() const = 0;
445 virtual SessionModel* childSessions() const = 0;
446 virtual State state() const = 0;
447@@ -73,7 +83,8 @@
448
449 // For MirSurface and MirSurfaceManager use
450
451- virtual void setSurface(MirSurfaceInterface* surface) = 0;
452+ virtual void registerSurface(MirSurfaceInterface* surface) = 0;
453+ virtual void removeSurface(MirSurfaceInterface* surface) = 0;
454
455 // For Application use
456
457@@ -99,16 +110,17 @@
458 virtual void removePromptSession(const std::shared_ptr<mir::scene::PromptSession>& session) = 0;
459
460 Q_SIGNALS:
461- void surfaceChanged(MirSurfaceInterface*);
462 void parentSessionChanged(SessionInterface*);
463 void applicationChanged(unity::shell::application::ApplicationInfoInterface* application);
464 void stateChanged(State state);
465 void fullscreenChanged(bool fullscreen);
466 void liveChanged(bool live);
467+ void lastSurfaceChanged(MirSurfaceInterface* surface);
468 };
469
470 } // namespace qtmir
471
472 Q_DECLARE_METATYPE(qtmir::SessionInterface*)
473+Q_DECLARE_METATYPE(qtmir::ObjectListModel<qtmir::MirSurfaceInterface>*)
474
475 #endif // SESSION_INTERFACE_H
476
477=== modified file 'tests/modules/ApplicationManager/application_manager_test.cpp'
478--- tests/modules/ApplicationManager/application_manager_test.cpp 2015-12-07 12:03:42 +0000
479+++ tests/modules/ApplicationManager/application_manager_test.cpp 2015-12-07 12:03:42 +0000
480@@ -50,7 +50,7 @@
481
482 SessionInterface* qmlSession = sessionManager.findSession(mirSession);
483 if (qmlSession) {
484- qmlSession->setSurface(qmlSurface);
485+ qmlSession->registerSurface(qmlSurface);
486 }
487
488 // I assume that applicationManager ignores the mirSurface parameter, so sending
489
490=== modified file 'tests/modules/SessionManager/session_test.cpp'
491--- tests/modules/SessionManager/session_test.cpp 2015-10-08 11:20:30 +0000
492+++ tests/modules/SessionManager/session_test.cpp 2015-12-07 12:03:42 +0000
493@@ -58,7 +58,7 @@
494 EXPECT_EQ(Session::Starting, session->state());
495
496 FakeMirSurface *surface = new FakeMirSurface;
497- session->setSurface(surface);
498+ session->registerSurface(surface);
499
500 // Still on Starting as the surface hasn't drawn its first frame yet
501 EXPECT_EQ(Session::Starting, session->state());
502@@ -228,7 +228,7 @@
503 auto session = std::make_shared<qtmir::Session>(mirSession, mirServer->the_prompt_session_manager());
504 {
505 FakeMirSurface *surface = new FakeMirSurface;
506- session->setSurface(surface);
507+ session->registerSurface(surface);
508 surface->drawFirstFrame();
509 }
510 EXPECT_EQ(Session::Running, session->state());
511@@ -259,7 +259,7 @@
512 auto session = std::make_shared<qtmir::Session>(mirSession, mirServer->the_prompt_session_manager());
513 {
514 FakeMirSurface *surface = new FakeMirSurface;
515- session->setSurface(surface);
516+ session->registerSurface(surface);
517 surface->drawFirstFrame();
518 }
519 EXPECT_EQ(Session::Running, session->state());
520
521=== modified file 'tests/modules/common/fake_session.h'
522--- tests/modules/common/fake_session.h 2015-12-07 12:03:42 +0000
523+++ tests/modules/common/fake_session.h 2015-12-07 12:03:42 +0000
524@@ -39,7 +39,8 @@
525
526 QString name() const override { return QString("foo-session"); }
527 unity::shell::application::ApplicationInfoInterface* application() const override { return m_application; }
528- MirSurfaceInterface* surface() const override { return nullptr; }
529+ MirSurfaceInterface* lastSurface() const override { return nullptr; }
530+ const ObjectListModel<MirSurfaceInterface>* surfaces() const override { return nullptr; }
531 SessionInterface* parentSession() const override { return nullptr; }
532 SessionModel* childSessions() const override { return nullptr; }
533 State state() const override { return m_state; }
534@@ -50,7 +51,8 @@
535
536 // For MirSurfaceItem and MirSurfaceManager use
537
538- void setSurface(MirSurfaceInterface*) override {}
539+ void registerSurface(MirSurfaceInterface*) override {}
540+ void removeSurface(MirSurfaceInterface*) override {}
541
542 // For Application use
543
544
545=== modified file 'tests/modules/common/mock_session.h'
546--- tests/modules/common/mock_session.h 2015-12-07 12:03:42 +0000
547+++ tests/modules/common/mock_session.h 2015-12-07 12:03:42 +0000
548@@ -36,16 +36,22 @@
549
550 MOCK_CONST_METHOD0(name, QString());
551 MOCK_CONST_METHOD0(application, unity::shell::application::ApplicationInfoInterface*());
552- MOCK_CONST_METHOD0(surface, MirSurfaceInterface*());
553+ MOCK_CONST_METHOD0(lastSurface, MirSurfaceInterface*());
554+ MOCK_CONST_METHOD0(surfaces, const ObjectListModel<MirSurfaceInterface>*());
555 MOCK_CONST_METHOD0(parentSession, SessionInterface*());
556+ MOCK_CONST_METHOD0(childSessions, SessionModel*());
557
558 MOCK_CONST_METHOD0(state, State());
559
560 MOCK_CONST_METHOD0(fullscreen, bool());
561 MOCK_CONST_METHOD0(live, bool());
562
563+ MOCK_CONST_METHOD0(session, std::shared_ptr<mir::scene::Session>());
564+
565+ MOCK_METHOD1(registerSurface, void(MirSurfaceInterface* surface));
566+ MOCK_METHOD1(removeSurface, void(MirSurfaceInterface* surface));
567+
568 MOCK_METHOD1(setApplication, void(unity::shell::application::ApplicationInfoInterface* item));
569- MOCK_METHOD1(setSurface, void(MirSurfaceInterface* surface));
570
571 MOCK_METHOD0(suspend, void());
572 MOCK_METHOD0(resume, void());
573@@ -57,13 +63,9 @@
574 MOCK_METHOD1(removeChildSession, void(SessionInterface* session));
575 MOCK_CONST_METHOD1(foreachChildSession, void(std::function<void(SessionInterface* session)> f));
576
577- MOCK_CONST_METHOD0(session, std::shared_ptr<mir::scene::Session>());
578-
579 MOCK_CONST_METHOD0(activePromptSession, std::shared_ptr<mir::scene::PromptSession>());
580 MOCK_CONST_METHOD1(foreachPromptSession, void(std::function<void(const std::shared_ptr<mir::scene::PromptSession>&)> f));
581
582- MOCK_CONST_METHOD0(childSessions, SessionModel*());
583-
584 void setState(State state) {
585 if (m_state != state) {
586 m_state = state;

Subscribers

People subscribed via source and target branches