Merge lp:~dandrader/unity-api/surfaceDrawn into lp:unity-api

Proposed by Daniel d'Andrada
Status: Rejected
Rejected by: Daniel d'Andrada
Proposed branch: lp:~dandrader/unity-api/surfaceDrawn
Merge into: lp:unity-api
Diff against target: 63 lines (+19/-1)
3 files modified
debian/changelog (+6/-0)
include/unity/shell/application/CMakeLists.txt (+1/-1)
include/unity/shell/application/MirSurfaceInterface.h (+12/-0)
To merge this branch: bzr merge lp:~dandrader/unity-api/surfaceDrawn
Reviewer Review Type Date Requested Status
Daniel d'Andrada (community) Disapprove
Nick Dedekind (community) Needs Fixing
Unity8 CI Bot continuous-integration Needs Fixing
Review via email: mp+294426@code.launchpad.net

Commit message

Added MirSurfaceInterface.drawn property

To post a comment you must log in.
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

perhaps we should make it a bit broader?
"readyToShow"?

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

On 12/05/2016 05:44, Nick Dedekind wrote:
> Review: Needs Fixing
>
> perhaps we should make it a bit broader?
> "readyToShow"?

I prefer a property name that maps more closely what's behind it. So
there's no (or little) risk of it overlapping with some other existing
or future property both in functionality and meaning.

And, more importantly, I also think it's a good practice to name
properties/functions after what they do, not after what you plan to use
them for (or connect to). MirSurface should have no say at all over
whether it should be shown in the qmlscene or not. That's up to the
application code (unity8) and its usage of MirSurfaceItems. So in a way
this name weakens the encapsulation of MirSurface.

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

This is not enough for bug 1581559

review: Disapprove

Unmerged revisions

230. By Daniel d'Andrada

Added MirSurfaceInterface.drawn property

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'debian/changelog'
--- debian/changelog 2016-04-26 15:31:50 +0000
+++ debian/changelog 2016-05-11 20:24:13 +0000
@@ -1,3 +1,9 @@
1unity-api (7.112) UNRELEASED; urgency=medium
2
3 * Added MirSurfaceInterface.drawn property
4
5 -- Daniel d'Andrada <daniel.dandrada@canonical.com> Wed, 11 May 2016 14:55:45 -0300
6
1unity-api (7.111+16.04.20160426.2-0ubuntu1) xenial; urgency=medium7unity-api (7.111+16.04.20160426.2-0ubuntu1) xenial; urgency=medium
28
3 [ Pawel Stolowski ]9 [ Pawel Stolowski ]
410
=== modified file 'include/unity/shell/application/CMakeLists.txt'
--- include/unity/shell/application/CMakeLists.txt 2016-04-13 16:40:06 +0000
+++ include/unity/shell/application/CMakeLists.txt 2016-05-11 20:24:13 +0000
@@ -7,7 +7,7 @@
77
8set(UNITY_API_LIB_HDRS ${UNITY_API_LIB_HDRS} ${headers} ${internal_headers} PARENT_SCOPE)8set(UNITY_API_LIB_HDRS ${UNITY_API_LIB_HDRS} ${headers} ${internal_headers} PARENT_SCOPE)
99
10set(VERSION 15)10set(VERSION 16)
11set(PKGCONFIG_NAME "unity-shell-application")11set(PKGCONFIG_NAME "unity-shell-application")
12set(PKGCONFIG_DESCRIPTION "Unity shell Application APIs")12set(PKGCONFIG_DESCRIPTION "Unity shell Application APIs")
13set(PKGCONFIG_REQUIRES "Qt5Core")13set(PKGCONFIG_REQUIRES "Qt5Core")
1414
=== modified file 'include/unity/shell/application/MirSurfaceInterface.h'
--- include/unity/shell/application/MirSurfaceInterface.h 2016-04-13 16:47:27 +0000
+++ include/unity/shell/application/MirSurfaceInterface.h 2016-05-11 20:24:13 +0000
@@ -139,6 +139,15 @@
139 */139 */
140 Q_PROPERTY(unity::shell::application::MirSurfaceListInterface* promptSurfaceList READ promptSurfaceList CONSTANT)140 Q_PROPERTY(unity::shell::application::MirSurfaceListInterface* promptSurfaceList READ promptSurfaceList CONSTANT)
141141
142
143 /**
144 * @brief Whether the surface has drawn its first frame
145 *
146 * It will be true if this surface has already drawn its first frame or false if it's
147 * still blank.
148 */
149 Q_PROPERTY(bool drawn READ drawn NOTIFY drawnChanged)
150
142public:151public:
143 /// @cond152 /// @cond
144 MirSurfaceInterface(QObject *parent = nullptr) : QObject(parent) {}153 MirSurfaceInterface(QObject *parent = nullptr) : QObject(parent) {}
@@ -177,6 +186,8 @@
177 virtual bool focused() const = 0;186 virtual bool focused() const = 0;
178187
179 virtual MirSurfaceListInterface* promptSurfaceList() = 0;188 virtual MirSurfaceListInterface* promptSurfaceList() = 0;
189
190 virtual bool drawn() const = 0;
180 /// @endcond191 /// @endcond
181192
182 /**193 /**
@@ -215,6 +226,7 @@
215 void shellChromeChanged(Mir::ShellChrome value);226 void shellChromeChanged(Mir::ShellChrome value);
216 void keymapChanged(const QString &value);227 void keymapChanged(const QString &value);
217 void focusedChanged(bool value);228 void focusedChanged(bool value);
229 void drawnChanged(bool value);
218 /// @endcond230 /// @endcond
219231
220 /**232 /**

Subscribers

People subscribed via source and target branches

to all changes: