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

Proposed by Daniel d'Andrada
Status: Superseded
Proposed branch: lp:~dandrader/qtmir/multiSurfaceApp
Merge into: lp:qtmir
Prerequisite: lp:~dandrader/qtmir/surfaceItemFillMode
Diff against target: 610 lines (+138/-102)
11 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/framework/fake_session.cpp (+3/-3)
tests/framework/fake_session.h (+4/-2)
tests/framework/mock_session.h (+8/-6)
tests/modules/ApplicationManager/application_manager_test.cpp (+1/-1)
tests/modules/SessionManager/session_test.cpp (+3/-3)
To merge this branch: bzr merge lp:~dandrader/qtmir/multiSurfaceApp
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Needs Fixing
Michael Zanetti (community) Needs Fixing
Gerry Boland code Pending
Review via email: mp+279113@code.launchpad.net

This proposal supersedes a proposal from 2015-08-07.

This proposal has been superseded by a proposal from 2015-12-07.

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.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
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
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
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.

Revision history for this message
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)
Revision history for this message
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

Revision history for this message
Michael Zanetti (mzanetti) wrote : Posted in a previous version of this proposal

I've tested it and it works.

review: Approve (functional testing)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
lp:~dandrader/qtmir/multiSurfaceApp updated
423. By Timo Jyrinki

Rebuild against Qt 5.5.1.

424. By Daniel d'Andrada

Merge lp:~dandrader/qtmir/surfaceItemFillMode

425. By Daniel d'Andrada

Make Session hold multiple surfaces

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

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Michael Zanetti (mzanetti) wrote :

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
Revision history for this message
Daniel d'Andrada (dandrader) wrote :

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?

lp:~dandrader/qtmir/multiSurfaceApp updated
426. By Daniel d'Andrada

Merge surfaceItemFillMode again

427. By Daniel d'Andrada

Fix bad merge with polite-close

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

> 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.

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

Unmerged revisions

427. By Daniel d'Andrada

Fix bad merge with polite-close

426. By Daniel d'Andrada

Merge surfaceItemFillMode again

425. By Daniel d'Andrada

Make Session hold multiple surfaces

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

424. By Daniel d'Andrada

Merge lp:~dandrader/qtmir/surfaceItemFillMode

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-04 18:48:40 +0000
3+++ src/modules/Unity/Application/mirsurface.cpp 2015-12-04 18:48:40 +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-04 18:48:40 +0000
123+++ src/modules/Unity/Application/mirsurfaceitem.cpp 2015-12-04 18:48:40 +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-04 18:48:40 +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-04 18:48:40 +0000
157+++ src/modules/Unity/Application/session.cpp 2015-12-04 18:48:40 +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-04 18:48:40 +0000
364+++ src/modules/Unity/Application/session.h 2015-12-04 18:48:40 +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-04 18:48:40 +0000
409+++ src/modules/Unity/Application/session_interface.h 2015-12-04 18:48:40 +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/framework/fake_session.cpp'
478--- tests/framework/fake_session.cpp 2015-12-04 18:48:40 +0000
479+++ tests/framework/fake_session.cpp 2015-12-04 18:48:40 +0000
480@@ -36,7 +36,9 @@
481
482 unity::shell::application::ApplicationInfoInterface *FakeSession::application() const { return m_application; }
483
484-MirSurfaceInterface *FakeSession::surface() const { return nullptr; }
485+MirSurfaceInterface *FakeSession::lastSurface() const { return nullptr; }
486+
487+const ObjectListModel<MirSurfaceInterface>* FakeSession::surfaces() const { return nullptr; }
488
489 SessionInterface *FakeSession::parentSession() const { return nullptr; }
490
491@@ -50,8 +52,6 @@
492
493 std::shared_ptr<mir::scene::Session> FakeSession::session() const { return nullptr; }
494
495-void FakeSession::setSurface(MirSurfaceInterface *) {}
496-
497 void FakeSession::setApplication(unity::shell::application::ApplicationInfoInterface *app)
498 {
499 if (m_application != app) {
500
501=== modified file 'tests/framework/fake_session.h'
502--- tests/framework/fake_session.h 2015-12-04 18:48:40 +0000
503+++ tests/framework/fake_session.h 2015-12-04 18:48:40 +0000
504@@ -35,7 +35,8 @@
505
506 QString name() const override;
507 unity::shell::application::ApplicationInfoInterface* application() const override;
508- MirSurfaceInterface* surface() const override;
509+ MirSurfaceInterface* lastSurface() const override;
510+ const ObjectListModel<MirSurfaceInterface>* surfaces() const override;
511 SessionInterface* parentSession() const override;
512 SessionModel* childSessions() const override;
513 State state() const override;
514@@ -46,7 +47,8 @@
515
516 // For MirSurfaceItem and MirSurfaceManager use
517
518- void setSurface(MirSurfaceInterface*) override;
519+ void registerSurface(MirSurfaceInterface*) override {}
520+ void removeSurface(MirSurfaceInterface*) override {}
521
522 // For Application use
523
524
525=== modified file 'tests/framework/mock_session.h'
526--- tests/framework/mock_session.h 2015-12-04 18:48:40 +0000
527+++ tests/framework/mock_session.h 2015-12-04 18:48:40 +0000
528@@ -31,16 +31,22 @@
529
530 MOCK_CONST_METHOD0(name, QString());
531 MOCK_CONST_METHOD0(application, unity::shell::application::ApplicationInfoInterface*());
532- MOCK_CONST_METHOD0(surface, MirSurfaceInterface*());
533+ MOCK_CONST_METHOD0(lastSurface, MirSurfaceInterface*());
534+ MOCK_CONST_METHOD0(surfaces, const ObjectListModel<MirSurfaceInterface>*());
535 MOCK_CONST_METHOD0(parentSession, SessionInterface*());
536+ MOCK_CONST_METHOD0(childSessions, SessionModel*());
537
538 MOCK_CONST_METHOD0(state, State());
539
540 MOCK_CONST_METHOD0(fullscreen, bool());
541 MOCK_CONST_METHOD0(live, bool());
542
543+ MOCK_CONST_METHOD0(session, std::shared_ptr<mir::scene::Session>());
544+
545+ MOCK_METHOD1(registerSurface, void(MirSurfaceInterface* surface));
546+ MOCK_METHOD1(removeSurface, void(MirSurfaceInterface* surface));
547+
548 MOCK_METHOD1(setApplication, void(unity::shell::application::ApplicationInfoInterface* item));
549- MOCK_METHOD1(setSurface, void(MirSurfaceInterface* surface));
550
551 MOCK_METHOD0(suspend, void());
552 MOCK_METHOD0(resume, void());
553@@ -52,13 +58,9 @@
554 MOCK_METHOD1(removeChildSession, void(SessionInterface* session));
555 MOCK_CONST_METHOD1(foreachChildSession, void(std::function<void(SessionInterface* session)> f));
556
557- MOCK_CONST_METHOD0(session, std::shared_ptr<mir::scene::Session>());
558-
559 MOCK_CONST_METHOD0(activePromptSession, std::shared_ptr<mir::scene::PromptSession>());
560 MOCK_CONST_METHOD1(foreachPromptSession, void(std::function<void(const std::shared_ptr<mir::scene::PromptSession>&)> f));
561
562- MOCK_CONST_METHOD0(childSessions, SessionModel*());
563-
564 void setState(State state);
565
566 void doSuspend();
567
568=== modified file 'tests/modules/ApplicationManager/application_manager_test.cpp'
569--- tests/modules/ApplicationManager/application_manager_test.cpp 2015-12-04 18:48:40 +0000
570+++ tests/modules/ApplicationManager/application_manager_test.cpp 2015-12-04 18:48:40 +0000
571@@ -50,7 +50,7 @@
572
573 SessionInterface* qmlSession = sessionManager.findSession(mirSession);
574 if (qmlSession) {
575- qmlSession->setSurface(qmlSurface);
576+ qmlSession->registerSurface(qmlSurface);
577 }
578
579 // I assume that applicationManager ignores the mirSurface parameter, so sending
580
581=== modified file 'tests/modules/SessionManager/session_test.cpp'
582--- tests/modules/SessionManager/session_test.cpp 2015-12-04 18:48:40 +0000
583+++ tests/modules/SessionManager/session_test.cpp 2015-12-04 18:48:40 +0000
584@@ -58,7 +58,7 @@
585 EXPECT_EQ(Session::Starting, session->state());
586
587 FakeMirSurface *surface = new FakeMirSurface;
588- session->setSurface(surface);
589+ session->registerSurface(surface);
590
591 // Still on Starting as the surface hasn't drawn its first frame yet
592 EXPECT_EQ(Session::Starting, session->state());
593@@ -228,7 +228,7 @@
594 auto session = std::make_shared<qtmir::Session>(mirSession, mirServer->the_prompt_session_manager());
595 {
596 FakeMirSurface *surface = new FakeMirSurface;
597- session->setSurface(surface);
598+ session->registerSurface(surface);
599 surface->drawFirstFrame();
600 }
601 EXPECT_EQ(Session::Running, session->state());
602@@ -259,7 +259,7 @@
603 auto session = std::make_shared<qtmir::Session>(mirSession, promptSessionManager);
604 {
605 FakeMirSurface *surface = new FakeMirSurface;
606- session->setSurface(surface);
607+ session->registerSurface(surface);
608 surface->drawFirstFrame();
609 }
610 EXPECT_EQ(Session::Running, session->state());

Subscribers

People subscribed via source and target branches