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
=== modified file 'src/modules/Unity/Application/mirsurface.cpp'
--- src/modules/Unity/Application/mirsurface.cpp 2015-12-04 18:48:40 +0000
+++ src/modules/Unity/Application/mirsurface.cpp 2015-12-04 18:48:40 +0000
@@ -35,6 +35,8 @@
3535
36using namespace qtmir;36using namespace qtmir;
3737
38#define DEBUG_MSG qCDebug(QTMIR_SURFACES).nospace() << "MirSurface[" << (void*)this << "," << appId() <<"]::" << __func__
39
38namespace {40namespace {
3941
40// Would be better if QMouseEvent had nativeModifiers42// Would be better if QMouseEvent had nativeModifiers
@@ -217,7 +219,7 @@
217 Q_ASSERT(m_views.isEmpty());219 Q_ASSERT(m_views.isEmpty());
218220
219 if (m_session) {221 if (m_session) {
220 m_session->setSurface(nullptr);222 m_session->removeSurface(this);
221 }223 }
222224
223 QMutexLocker locker(&m_mutex);225 QMutexLocker locker(&m_mutex);
@@ -301,12 +303,11 @@
301303
302 locker.unlock();304 locker.unlock();
303 if (updateTexture()) {305 if (updateTexture()) {
304 qCDebug(QTMIR_SURFACES).nospace() << "MirSurface[" << appId() << "]::dropPendingBuffer() dropped=1 left=" << framesPending-1;306 DEBUG_MSG << "() dropped=1 left=" << framesPending-1;
305 } else {307 } else {
306 // If we haven't managed to update the texture, don't keep banging away.308 // If we haven't managed to update the texture, don't keep banging away.
307 m_frameDropperTimer.stop();309 m_frameDropperTimer.stop();
308 qCDebug(QTMIR_SURFACES).nospace() << "MirSurface[" << appId() << "]::dropPendingBuffer() dropped=0"310 DEBUG_MSG << "() dropped=0" << " left=" << framesPending << " - failed to upate texture";
309 << " left=" << framesPending << " - failed to upate texture";
310 }311 }
311 Q_EMIT frameDropped();312 Q_EMIT frameDropped();
312 } else {313 } else {
@@ -320,13 +321,13 @@
320321
321void MirSurface::stopFrameDropper()322void MirSurface::stopFrameDropper()
322{323{
323 qCDebug(QTMIR_SURFACES).nospace() << "MirSurface[" << appId() << "]::stopFrameDropper()";324 DEBUG_MSG << "()";
324 m_frameDropperTimer.stop();325 m_frameDropperTimer.stop();
325}326}
326327
327void MirSurface::startFrameDropper()328void MirSurface::startFrameDropper()
328{329{
329 qCDebug(QTMIR_SURFACES).nospace() << "MirSurface[" << appId() << "]::startFrameDropper()";330 DEBUG_MSG << "()";
330 if (!m_frameDropperTimer.isActive()) {331 if (!m_frameDropperTimer.isActive()) {
331 m_frameDropperTimer.start();332 m_frameDropperTimer.start();
332 }333 }
@@ -421,12 +422,11 @@
421 // Temporary hotfix for http://pad.lv/1483752422 // Temporary hotfix for http://pad.lv/1483752
422 if (m_session->childSessions()->rowCount() > 0) {423 if (m_session->childSessions()->rowCount() > 0) {
423 // has child trusted session, ignore any focus change attempts424 // has child trusted session, ignore any focus change attempts
424 qCDebug(QTMIR_SURFACES).nospace() << "MirSurface[" << appId() << "]::setFocus(" << focus425 DEBUG_MSG << "(" << focus << ") - has child trusted session, ignore any focus change attempts";
425 << ") - has child trusted session, ignore any focus change attempts";
426 return;426 return;
427 }427 }
428428
429 qCDebug(QTMIR_SURFACES).nospace() << "MirSurface[" << appId() << "]::setFocus(" << focus << ")";429 DEBUG_MSG << "(" << focus << ")";
430430
431 if (focus) {431 if (focus) {
432 m_shell->set_surface_attribute(m_session->session(), m_surface, mir_surface_attrib_focus, mir_surface_focused);432 m_shell->set_surface_attribute(m_session->session(), m_surface, mir_surface_attrib_focus, mir_surface_focused);
@@ -452,9 +452,8 @@
452 if (clientIsRunning() && mirSizeIsDifferent) {452 if (clientIsRunning() && mirSizeIsDifferent) {
453 mir::geometry::Size newMirSize(width, height);453 mir::geometry::Size newMirSize(width, height);
454 m_surface->resize(newMirSize);454 m_surface->resize(newMirSize);
455 qCDebug(QTMIR_SURFACES).nospace() << "MirSurface[" << appId() << "]::resize"455 DEBUG_MSG << " old (" << mirWidth << "," << mirHeight << ")"
456 << " old (" << mirWidth << "," << mirHeight << ")"456 << ", new (" << width << "," << height << ")";
457 << ", new (" << width << "," << height << ")";
458 }457 }
459}458}
460459
@@ -681,8 +680,7 @@
681void MirSurface::registerView(qintptr viewId)680void MirSurface::registerView(qintptr viewId)
682{681{
683 m_views.insert(viewId, MirSurface::View{false});682 m_views.insert(viewId, MirSurface::View{false});
684 qCDebug(QTMIR_SURFACES).nospace() << "MirSurface[" << appId() << "]::registerView(" << viewId << ")"683 DEBUG_MSG << "(" << viewId << ")" << " after=" << m_views.count();
685 << " after=" << m_views.count();
686 if (m_views.count() == 1) {684 if (m_views.count() == 1) {
687 Q_EMIT isBeingDisplayedChanged();685 Q_EMIT isBeingDisplayedChanged();
688 }686 }
@@ -690,9 +688,8 @@
690688
691void MirSurface::unregisterView(qintptr viewId)689void MirSurface::unregisterView(qintptr viewId)
692{690{
693 qCDebug(QTMIR_SURFACES).nospace() << "MirSurface[" << appId() << "]::unregisterView(" << viewId << ")"
694 << " after=" << m_views.count() << " live=" << m_live;
695 m_views.remove(viewId);691 m_views.remove(viewId);
692 DEBUG_MSG << "(" << viewId << ")" << " after=" << m_views.count() << " live=" << m_live;
696 if (m_views.count() == 0) {693 if (m_views.count() == 0) {
697 Q_EMIT isBeingDisplayedChanged();694 Q_EMIT isBeingDisplayedChanged();
698 if (m_session.isNull() || !m_live) {695 if (m_session.isNull() || !m_live) {
@@ -723,7 +720,7 @@
723 }720 }
724721
725 if (newVisible != visible()) {722 if (newVisible != visible()) {
726 qCDebug(QTMIR_SURFACES).nospace() << "MirSurface[" << appId() << "]::updateVisibility(" << newVisible << ")";723 DEBUG_MSG << "(" << newVisible << ")";
727724
728 m_surface->configure(mir_surface_attrib_visibility,725 m_surface->configure(mir_surface_attrib_visibility,
729 newVisible ? mir_surface_visibility_exposed : mir_surface_visibility_occluded);726 newVisible ? mir_surface_visibility_exposed : mir_surface_visibility_occluded);
@@ -767,8 +764,7 @@
767764
768void MirSurface::setCursor(const QCursor &cursor)765void MirSurface::setCursor(const QCursor &cursor)
769{766{
770 qCDebug(QTMIR_SURFACES).nospace() << "MirSurface[" << appId() << "]::setCursor("767 DEBUG_MSG << "(" << qtCursorShapeToStr(cursor.shape()) << ")";
771 << qtCursorShapeToStr(cursor.shape()) << ")";
772768
773 m_cursor = cursor;769 m_cursor = cursor;
774 Q_EMIT cursorChanged(m_cursor);770 Q_EMIT cursorChanged(m_cursor);
775771
=== modified file 'src/modules/Unity/Application/mirsurfaceitem.cpp'
--- src/modules/Unity/Application/mirsurfaceitem.cpp 2015-12-04 18:48:40 +0000
+++ src/modules/Unity/Application/mirsurfaceitem.cpp 2015-12-04 18:48:40 +0000
@@ -236,8 +236,6 @@
236 QSGDefaultImageNode *node = static_cast<QSGDefaultImageNode*>(oldNode);236 QSGDefaultImageNode *node = static_cast<QSGDefaultImageNode*>(oldNode);
237 if (!node) {237 if (!node) {
238 node = new QSGDefaultImageNode;238 node = new QSGDefaultImageNode;
239 node->setTexture(m_textureProvider->texture());
240
241 node->setMipmapFiltering(QSGTexture::None);239 node->setMipmapFiltering(QSGTexture::None);
242 node->setHorizontalWrapMode(QSGTexture::ClampToEdge);240 node->setHorizontalWrapMode(QSGTexture::ClampToEdge);
243 node->setVerticalWrapMode(QSGTexture::ClampToEdge);241 node->setVerticalWrapMode(QSGTexture::ClampToEdge);
@@ -246,6 +244,7 @@
246 node->markDirty(QSGNode::DirtyMaterial);244 node->markDirty(QSGNode::DirtyMaterial);
247 }245 }
248 }246 }
247 node->setTexture(m_textureProvider->texture());
249248
250 if (m_fillMode == PadOrCrop) {249 if (m_fillMode == PadOrCrop) {
251 const QSize &textureSize = m_textureProvider->texture()->textureSize();250 const QSize &textureSize = m_textureProvider->texture()->textureSize();
252251
=== modified file 'src/modules/Unity/Application/mirsurfacemanager.cpp'
--- src/modules/Unity/Application/mirsurfacemanager.cpp 2015-10-09 15:35:05 +0000
+++ src/modules/Unity/Application/mirsurfacemanager.cpp 2015-12-04 18:48:40 +0000
@@ -110,7 +110,7 @@
110 }110 }
111111
112 if (session)112 if (session)
113 session->setSurface(qmlSurface);113 session->registerSurface(qmlSurface);
114114
115 // Only notify QML of surface creation once it has drawn its first frame.115 // Only notify QML of surface creation once it has drawn its first frame.
116 connect(qmlSurface, &MirSurfaceInterface::firstFrameDrawn, this, [=]() {116 connect(qmlSurface, &MirSurfaceInterface::firstFrameDrawn, this, [=]() {
117117
=== modified file 'src/modules/Unity/Application/session.cpp'
--- src/modules/Unity/Application/session.cpp 2015-12-04 18:48:40 +0000
+++ src/modules/Unity/Application/session.cpp 2015-12-04 18:48:40 +0000
@@ -68,7 +68,6 @@
68 : SessionInterface(parent)68 : SessionInterface(parent)
69 , m_session(session)69 , m_session(session)
70 , m_application(nullptr)70 , m_application(nullptr)
71 , m_surface(nullptr)
72 , m_parentSession(nullptr)71 , m_parentSession(nullptr)
73 , m_children(new SessionModel(this))72 , m_children(new SessionModel(this))
74 , m_fullscreen(false)73 , m_fullscreen(false)
@@ -108,10 +107,15 @@
108void Session::doSuspend()107void Session::doSuspend()
109{108{
110 Q_ASSERT(m_state == Session::Suspending);109 Q_ASSERT(m_state == Session::Suspending);
111 if (m_surface) {110
112 m_surface->stopFrameDropper();111
112 auto surfaceList = m_surfaces.list();
113 if (surfaceList.empty()) {
114 qCDebug(QTMIR_SESSIONS) << "Application::suspend - no surface to call stopFrameDropper() on!";
113 } else {115 } else {
114 qDebug() << "Application::suspend - no surface to call stopFrameDropper() on!";116 for (int i = 0; i < surfaceList.count(); ++i) {
117 surfaceList[i]->stopFrameDropper();
118 }
115 }119 }
116 setState(Suspended);120 setState(Suspended);
117}121}
@@ -142,14 +146,9 @@
142 return m_application;146 return m_application;
143}147}
144148
145MirSurfaceInterface* Session::surface() const149const ObjectListModel<MirSurfaceInterface>* Session::surfaces() const
146{150{
147 // Only notify QML of surface creation once it has drawn its first frame.151 return &m_surfaces;
148 if (m_surface && m_surface->isFirstFrameDrawn()) {
149 return m_surface;
150 } else {
151 return nullptr;
152 }
153}152}
154153
155SessionInterface* Session::parentSession() const154SessionInterface* Session::parentSession() const
@@ -191,55 +190,57 @@
191 Q_EMIT applicationChanged(application);190 Q_EMIT applicationChanged(application);
192}191}
193192
194void Session::setSurface(MirSurfaceInterface *newSurface)193void Session::registerSurface(MirSurfaceInterface *newSurface)
195{194{
196 qCDebug(QTMIR_SESSIONS) << "Session::setSurface - session=" << name() << "surface=" << newSurface;195 qCDebug(QTMIR_SESSIONS) << "Session::resgisterSurface - session=" << name() << "surface=" << newSurface;
197196
198 if (newSurface == m_surface) {197 // Only notify QML of surface creation once it has drawn its first frame.
199 return;198 if (newSurface->isFirstFrameDrawn()) {
200 }199 appendSurface(newSurface);
201200 } else {
202 if (m_surface) {201 connect(newSurface, &MirSurfaceInterface::firstFrameDrawn,
203 m_surface->disconnect(this);202 this, [this, newSurface]() { this->appendSurface(newSurface); });
204 }
205
206 MirSurfaceInterface *previousSurface = surface();
207 m_surface = newSurface;
208
209 if (newSurface) {
210 connect(newSurface, &MirSurfaceInterface::stateChanged,
211 this, &Session::updateFullscreenProperty);
212
213 // Only notify QML of surface creation once it has drawn its first frame.
214 if (m_surface->isFirstFrameDrawn()) {
215 setState(Running);
216 } else {
217 connect(newSurface, &MirSurfaceInterface::firstFrameDrawn,
218 this, &Session::onFirstSurfaceFrameDrawn);
219 }
220 }
221
222 if (previousSurface != surface()) {
223 qCDebug(QTMIR_SESSIONS).nospace() << "Session::surfaceChanged - session=" << this
224 << " surface=" << m_surface;
225 Q_EMIT surfaceChanged(m_surface);
226 }203 }
227204
228 updateFullscreenProperty();205 updateFullscreenProperty();
229}206}
230207
231void Session::onFirstSurfaceFrameDrawn()208void Session::appendSurface(MirSurfaceInterface *newSurface)
232{209{
233 qCDebug(QTMIR_SESSIONS).nospace() << "Session::surfaceChanged - session=" << this210 qCDebug(QTMIR_SESSIONS) << "Session::appendSurface - session=" << name() << "surface=" << newSurface;
234 << " surface=" << m_surface;211
235 Q_EMIT surfaceChanged(m_surface);212 connect(newSurface, &MirSurfaceInterface::stateChanged,
236 setState(Running);213 this, &Session::updateFullscreenProperty);
214
215 m_surfaces.insert(m_surfaces.rowCount(), newSurface);
216
217 Q_EMIT lastSurfaceChanged(newSurface);
218
219 if (m_state == Starting) {
220 setState(Running);
221 }
222}
223
224void Session::removeSurface(MirSurfaceInterface* surface)
225{
226 qCDebug(QTMIR_SESSIONS) << "Session::removeSurface - session=" << name() << "surface=" << surface;
227
228 surface->disconnect(this);
229
230 if (m_surfaces.contains(surface)) {
231 bool lastSurfaceWasRemoved = lastSurface() == surface;
232 m_surfaces.remove(surface);
233 if (lastSurfaceWasRemoved) {
234 Q_EMIT lastSurfaceChanged(lastSurface());
235 }
236 }
237}237}
238238
239void Session::updateFullscreenProperty()239void Session::updateFullscreenProperty()
240{240{
241 if (m_surface) {241 if (m_surfaces.rowCount() > 0) {
242 setFullscreen(m_surface->state() == Mir::FullscreenState);242 // TODO: Figure out something better
243 setFullscreen(m_surfaces.list().at(0)->state() == Mir::FullscreenState);
243 } else {244 } else {
244 // Keep the current value of the fullscreen property until we get a new245 // Keep the current value of the fullscreen property until we get a new
245 // surface246 // surface
@@ -289,8 +290,11 @@
289 Q_ASSERT(m_suspendTimer->isActive());290 Q_ASSERT(m_suspendTimer->isActive());
290 m_suspendTimer->stop();291 m_suspendTimer->stop();
291 } else if (m_state == Suspended) {292 } else if (m_state == Suspended) {
292 Q_ASSERT(m_surface);293 Q_ASSERT(m_surfaces.rowCount() > 0);
293 m_surface->startFrameDropper();294 auto surfaceList = m_surfaces.list();
295 for (int i = 0; i < surfaceList.count(); ++i) {
296 surfaceList[i]->startFrameDropper();
297 }
294 }298 }
295299
296 session()->set_lifecycle_state(mir_lifecycle_state_resumed);300 session()->set_lifecycle_state(mir_lifecycle_state_resumed);
@@ -308,9 +312,11 @@
308312
309void Session::close()313void Session::close()
310{314{
311 qCDebug(QTMIR_SESSIONS) << "Session::close - " << name() << m_surface;315 qCDebug(QTMIR_SESSIONS) << "Session::close - " << name();
312 if (m_surface) {316
313 m_surface->close();317 auto surfaceList = m_surfaces.list();
318 for (int i = 0; i < surfaceList.count(); ++i) {
319 surfaceList[i]->close();
314 }320 }
315}321}
316322
@@ -321,10 +327,16 @@
321 if (m_state != Stopped) {327 if (m_state != Stopped) {
322328
323 stopPromptSessions();329 stopPromptSessions();
330
324 if (m_suspendTimer->isActive())331 if (m_suspendTimer->isActive())
325 m_suspendTimer->stop();332 m_suspendTimer->stop();
326 if (m_surface)333
327 m_surface->stopFrameDropper();334 {
335 auto surfaceList = m_surfaces.list();
336 for (int i = 0; i < surfaceList.count(); ++i) {
337 surfaceList[i]->stopFrameDropper();
338 }
339 }
328340
329 foreachChildSession([](SessionInterface* session) {341 foreachChildSession([](SessionInterface* session) {
330 session->stop();342 session->stop();
@@ -460,4 +472,13 @@
460 }472 }
461}473}
462474
475MirSurfaceInterface* Session::lastSurface() const
476{
477 if (m_surfaces.rowCount() > 0) {
478 return m_surfaces.list().last();
479 } else {
480 return nullptr;
481 }
482}
483
463} // namespace qtmir484} // namespace qtmir
464485
=== modified file 'src/modules/Unity/Application/session.h'
--- src/modules/Unity/Application/session.h 2015-12-04 18:48:40 +0000
+++ src/modules/Unity/Application/session.h 2015-12-04 18:48:40 +0000
@@ -52,14 +52,17 @@
52 //getters52 //getters
53 QString name() const override;53 QString name() const override;
54 unity::shell::application::ApplicationInfoInterface* application() const override;54 unity::shell::application::ApplicationInfoInterface* application() const override;
55 MirSurfaceInterface* surface() const override;55 MirSurfaceInterface* lastSurface() const override;
56 const ObjectListModel<MirSurfaceInterface>* surfaces() const override;
56 SessionInterface* parentSession() const override;57 SessionInterface* parentSession() const override;
57 State state() const override;58 State state() const override;
58 bool fullscreen() const override;59 bool fullscreen() const override;
59 bool live() const override;60 bool live() const override;
6061
61 void setApplication(unity::shell::application::ApplicationInfoInterface* item) override;62 void setApplication(unity::shell::application::ApplicationInfoInterface* item) override;
62 void setSurface(MirSurfaceInterface* surface) override;63
64 void registerSurface(MirSurfaceInterface* surface) override;
65 void removeSurface(MirSurfaceInterface* surface) override;
6366
64 void suspend() override;67 void suspend() override;
65 void resume() override;68 void resume() override;
@@ -89,7 +92,6 @@
8992
90private Q_SLOTS:93private Q_SLOTS:
91 void updateFullscreenProperty();94 void updateFullscreenProperty();
92 void onFirstSurfaceFrameDrawn();
9395
94private:96private:
95 void setParentSession(Session* session);97 void setParentSession(Session* session);
@@ -98,9 +100,11 @@
98100
99 void stopPromptSessions();101 void stopPromptSessions();
100102
103 void appendSurface(MirSurfaceInterface* surface);
104
101 std::shared_ptr<mir::scene::Session> m_session;105 std::shared_ptr<mir::scene::Session> m_session;
102 Application* m_application;106 Application* m_application;
103 MirSurfaceInterface* m_surface;107 ObjectListModel<MirSurfaceInterface> m_surfaces;
104 SessionInterface* m_parentSession;108 SessionInterface* m_parentSession;
105 SessionModel* m_children;109 SessionModel* m_children;
106 bool m_fullscreen;110 bool m_fullscreen;
107111
=== modified file 'src/modules/Unity/Application/session_interface.h'
--- src/modules/Unity/Application/session_interface.h 2015-12-04 18:48:40 +0000
+++ src/modules/Unity/Application/session_interface.h 2015-12-04 18:48:40 +0000
@@ -24,8 +24,12 @@
24#include <unity/shell/application/ApplicationInfoInterface.h>24#include <unity/shell/application/ApplicationInfoInterface.h>
2525
26// local26// local
27#include "objectlistmodel.h"
27#include "sessionmodel.h"28#include "sessionmodel.h"
2829
30// Qt
31#include <QQmlListProperty>
32
29namespace mir {33namespace mir {
30 namespace scene {34 namespace scene {
31 class Session;35 class Session;
@@ -39,7 +43,12 @@
3943
40class SessionInterface : public QObject {44class SessionInterface : public QObject {
41 Q_OBJECT45 Q_OBJECT
42 Q_PROPERTY(MirSurfaceInterface* surface READ surface NOTIFY surfaceChanged)46
47 // FIXME: Remove this once unity8 starts trully supporting multiple surfaces per applicaton.
48 // Ie, remove this once untiy8 moves from using this property to using the surfaces one.
49 Q_PROPERTY(MirSurfaceInterface* lastSurface READ lastSurface NOTIFY lastSurfaceChanged);
50
51 Q_PROPERTY(const ObjectListModel<MirSurfaceInterface>* surfaces READ surfaces CONSTANT);
43 Q_PROPERTY(unity::shell::application::ApplicationInfoInterface* application READ application NOTIFY applicationChanged DESIGNABLE false)52 Q_PROPERTY(unity::shell::application::ApplicationInfoInterface* application READ application NOTIFY applicationChanged DESIGNABLE false)
44 Q_PROPERTY(SessionInterface* parentSession READ parentSession NOTIFY parentSessionChanged DESIGNABLE false)53 Q_PROPERTY(SessionInterface* parentSession READ parentSession NOTIFY parentSessionChanged DESIGNABLE false)
45 Q_PROPERTY(SessionModel* childSessions READ childSessions DESIGNABLE false CONSTANT)54 Q_PROPERTY(SessionModel* childSessions READ childSessions DESIGNABLE false CONSTANT)
@@ -62,7 +71,8 @@
62 //getters71 //getters
63 virtual QString name() const = 0;72 virtual QString name() const = 0;
64 virtual unity::shell::application::ApplicationInfoInterface* application() const = 0;73 virtual unity::shell::application::ApplicationInfoInterface* application() const = 0;
65 virtual MirSurfaceInterface* surface() const = 0;74 virtual MirSurfaceInterface* lastSurface() const = 0;
75 virtual const ObjectListModel<MirSurfaceInterface>* surfaces() const = 0;
66 virtual SessionInterface* parentSession() const = 0;76 virtual SessionInterface* parentSession() const = 0;
67 virtual SessionModel* childSessions() const = 0;77 virtual SessionModel* childSessions() const = 0;
68 virtual State state() const = 0;78 virtual State state() const = 0;
@@ -73,7 +83,8 @@
7383
74 // For MirSurface and MirSurfaceManager use84 // For MirSurface and MirSurfaceManager use
7585
76 virtual void setSurface(MirSurfaceInterface* surface) = 0;86 virtual void registerSurface(MirSurfaceInterface* surface) = 0;
87 virtual void removeSurface(MirSurfaceInterface* surface) = 0;
7788
78 // For Application use89 // For Application use
7990
@@ -99,16 +110,17 @@
99 virtual void removePromptSession(const std::shared_ptr<mir::scene::PromptSession>& session) = 0;110 virtual void removePromptSession(const std::shared_ptr<mir::scene::PromptSession>& session) = 0;
100111
101Q_SIGNALS:112Q_SIGNALS:
102 void surfaceChanged(MirSurfaceInterface*);
103 void parentSessionChanged(SessionInterface*);113 void parentSessionChanged(SessionInterface*);
104 void applicationChanged(unity::shell::application::ApplicationInfoInterface* application);114 void applicationChanged(unity::shell::application::ApplicationInfoInterface* application);
105 void stateChanged(State state);115 void stateChanged(State state);
106 void fullscreenChanged(bool fullscreen);116 void fullscreenChanged(bool fullscreen);
107 void liveChanged(bool live);117 void liveChanged(bool live);
118 void lastSurfaceChanged(MirSurfaceInterface* surface);
108};119};
109120
110} // namespace qtmir121} // namespace qtmir
111122
112Q_DECLARE_METATYPE(qtmir::SessionInterface*)123Q_DECLARE_METATYPE(qtmir::SessionInterface*)
124Q_DECLARE_METATYPE(qtmir::ObjectListModel<qtmir::MirSurfaceInterface>*)
113125
114#endif // SESSION_INTERFACE_H126#endif // SESSION_INTERFACE_H
115127
=== modified file 'tests/framework/fake_session.cpp'
--- tests/framework/fake_session.cpp 2015-12-04 18:48:40 +0000
+++ tests/framework/fake_session.cpp 2015-12-04 18:48:40 +0000
@@ -36,7 +36,9 @@
3636
37unity::shell::application::ApplicationInfoInterface *FakeSession::application() const { return m_application; }37unity::shell::application::ApplicationInfoInterface *FakeSession::application() const { return m_application; }
3838
39MirSurfaceInterface *FakeSession::surface() const { return nullptr; }39MirSurfaceInterface *FakeSession::lastSurface() const { return nullptr; }
40
41const ObjectListModel<MirSurfaceInterface>* FakeSession::surfaces() const { return nullptr; }
4042
41SessionInterface *FakeSession::parentSession() const { return nullptr; }43SessionInterface *FakeSession::parentSession() const { return nullptr; }
4244
@@ -50,8 +52,6 @@
5052
51std::shared_ptr<mir::scene::Session> FakeSession::session() const { return nullptr; }53std::shared_ptr<mir::scene::Session> FakeSession::session() const { return nullptr; }
5254
53void FakeSession::setSurface(MirSurfaceInterface *) {}
54
55void FakeSession::setApplication(unity::shell::application::ApplicationInfoInterface *app)55void FakeSession::setApplication(unity::shell::application::ApplicationInfoInterface *app)
56{56{
57 if (m_application != app) {57 if (m_application != app) {
5858
=== modified file 'tests/framework/fake_session.h'
--- tests/framework/fake_session.h 2015-12-04 18:48:40 +0000
+++ tests/framework/fake_session.h 2015-12-04 18:48:40 +0000
@@ -35,7 +35,8 @@
3535
36 QString name() const override;36 QString name() const override;
37 unity::shell::application::ApplicationInfoInterface* application() const override;37 unity::shell::application::ApplicationInfoInterface* application() const override;
38 MirSurfaceInterface* surface() const override;38 MirSurfaceInterface* lastSurface() const override;
39 const ObjectListModel<MirSurfaceInterface>* surfaces() const override;
39 SessionInterface* parentSession() const override;40 SessionInterface* parentSession() const override;
40 SessionModel* childSessions() const override;41 SessionModel* childSessions() const override;
41 State state() const override;42 State state() const override;
@@ -46,7 +47,8 @@
4647
47 // For MirSurfaceItem and MirSurfaceManager use48 // For MirSurfaceItem and MirSurfaceManager use
4849
49 void setSurface(MirSurfaceInterface*) override;50 void registerSurface(MirSurfaceInterface*) override {}
51 void removeSurface(MirSurfaceInterface*) override {}
5052
51 // For Application use53 // For Application use
5254
5355
=== modified file 'tests/framework/mock_session.h'
--- tests/framework/mock_session.h 2015-12-04 18:48:40 +0000
+++ tests/framework/mock_session.h 2015-12-04 18:48:40 +0000
@@ -31,16 +31,22 @@
3131
32 MOCK_CONST_METHOD0(name, QString());32 MOCK_CONST_METHOD0(name, QString());
33 MOCK_CONST_METHOD0(application, unity::shell::application::ApplicationInfoInterface*());33 MOCK_CONST_METHOD0(application, unity::shell::application::ApplicationInfoInterface*());
34 MOCK_CONST_METHOD0(surface, MirSurfaceInterface*());34 MOCK_CONST_METHOD0(lastSurface, MirSurfaceInterface*());
35 MOCK_CONST_METHOD0(surfaces, const ObjectListModel<MirSurfaceInterface>*());
35 MOCK_CONST_METHOD0(parentSession, SessionInterface*());36 MOCK_CONST_METHOD0(parentSession, SessionInterface*());
37 MOCK_CONST_METHOD0(childSessions, SessionModel*());
3638
37 MOCK_CONST_METHOD0(state, State());39 MOCK_CONST_METHOD0(state, State());
3840
39 MOCK_CONST_METHOD0(fullscreen, bool());41 MOCK_CONST_METHOD0(fullscreen, bool());
40 MOCK_CONST_METHOD0(live, bool());42 MOCK_CONST_METHOD0(live, bool());
4143
44 MOCK_CONST_METHOD0(session, std::shared_ptr<mir::scene::Session>());
45
46 MOCK_METHOD1(registerSurface, void(MirSurfaceInterface* surface));
47 MOCK_METHOD1(removeSurface, void(MirSurfaceInterface* surface));
48
42 MOCK_METHOD1(setApplication, void(unity::shell::application::ApplicationInfoInterface* item));49 MOCK_METHOD1(setApplication, void(unity::shell::application::ApplicationInfoInterface* item));
43 MOCK_METHOD1(setSurface, void(MirSurfaceInterface* surface));
4450
45 MOCK_METHOD0(suspend, void());51 MOCK_METHOD0(suspend, void());
46 MOCK_METHOD0(resume, void());52 MOCK_METHOD0(resume, void());
@@ -52,13 +58,9 @@
52 MOCK_METHOD1(removeChildSession, void(SessionInterface* session));58 MOCK_METHOD1(removeChildSession, void(SessionInterface* session));
53 MOCK_CONST_METHOD1(foreachChildSession, void(std::function<void(SessionInterface* session)> f));59 MOCK_CONST_METHOD1(foreachChildSession, void(std::function<void(SessionInterface* session)> f));
5460
55 MOCK_CONST_METHOD0(session, std::shared_ptr<mir::scene::Session>());
56
57 MOCK_CONST_METHOD0(activePromptSession, std::shared_ptr<mir::scene::PromptSession>());61 MOCK_CONST_METHOD0(activePromptSession, std::shared_ptr<mir::scene::PromptSession>());
58 MOCK_CONST_METHOD1(foreachPromptSession, void(std::function<void(const std::shared_ptr<mir::scene::PromptSession>&)> f));62 MOCK_CONST_METHOD1(foreachPromptSession, void(std::function<void(const std::shared_ptr<mir::scene::PromptSession>&)> f));
5963
60 MOCK_CONST_METHOD0(childSessions, SessionModel*());
61
62 void setState(State state);64 void setState(State state);
6365
64 void doSuspend();66 void doSuspend();
6567
=== modified file 'tests/modules/ApplicationManager/application_manager_test.cpp'
--- tests/modules/ApplicationManager/application_manager_test.cpp 2015-12-04 18:48:40 +0000
+++ tests/modules/ApplicationManager/application_manager_test.cpp 2015-12-04 18:48:40 +0000
@@ -50,7 +50,7 @@
5050
51 SessionInterface* qmlSession = sessionManager.findSession(mirSession);51 SessionInterface* qmlSession = sessionManager.findSession(mirSession);
52 if (qmlSession) {52 if (qmlSession) {
53 qmlSession->setSurface(qmlSurface);53 qmlSession->registerSurface(qmlSurface);
54 }54 }
5555
56 // I assume that applicationManager ignores the mirSurface parameter, so sending56 // I assume that applicationManager ignores the mirSurface parameter, so sending
5757
=== modified file 'tests/modules/SessionManager/session_test.cpp'
--- tests/modules/SessionManager/session_test.cpp 2015-12-04 18:48:40 +0000
+++ tests/modules/SessionManager/session_test.cpp 2015-12-04 18:48:40 +0000
@@ -58,7 +58,7 @@
58 EXPECT_EQ(Session::Starting, session->state());58 EXPECT_EQ(Session::Starting, session->state());
5959
60 FakeMirSurface *surface = new FakeMirSurface;60 FakeMirSurface *surface = new FakeMirSurface;
61 session->setSurface(surface);61 session->registerSurface(surface);
6262
63 // Still on Starting as the surface hasn't drawn its first frame yet63 // Still on Starting as the surface hasn't drawn its first frame yet
64 EXPECT_EQ(Session::Starting, session->state());64 EXPECT_EQ(Session::Starting, session->state());
@@ -228,7 +228,7 @@
228 auto session = std::make_shared<qtmir::Session>(mirSession, mirServer->the_prompt_session_manager());228 auto session = std::make_shared<qtmir::Session>(mirSession, mirServer->the_prompt_session_manager());
229 {229 {
230 FakeMirSurface *surface = new FakeMirSurface;230 FakeMirSurface *surface = new FakeMirSurface;
231 session->setSurface(surface);231 session->registerSurface(surface);
232 surface->drawFirstFrame();232 surface->drawFirstFrame();
233 }233 }
234 EXPECT_EQ(Session::Running, session->state());234 EXPECT_EQ(Session::Running, session->state());
@@ -259,7 +259,7 @@
259 auto session = std::make_shared<qtmir::Session>(mirSession, promptSessionManager);259 auto session = std::make_shared<qtmir::Session>(mirSession, promptSessionManager);
260 {260 {
261 FakeMirSurface *surface = new FakeMirSurface;261 FakeMirSurface *surface = new FakeMirSurface;
262 session->setSurface(surface);262 session->registerSurface(surface);
263 surface->drawFirstFrame();263 surface->drawFirstFrame();
264 }264 }
265 EXPECT_EQ(Session::Running, session->state());265 EXPECT_EQ(Session::Running, session->state());

Subscribers

People subscribed via source and target branches