Unity 8

Merge lp:~dandrader/unity8/surfaceDrawn into lp:unity8

  1. surfaceDrawn
  2. Merge into trunk
Proposed by Daniel d'Andrada
Status: Rejected
Rejected by: Daniel d'Andrada
Proposed branch: lp:~dandrader/unity8/surfaceDrawn
Merge into: lp:unity8
Diff against target: 400 lines (+107/-23)
8 files modified
CMakeLists.txt (+1/-1)
debian/control (+3/-3)
qml/Stages/ApplicationWindow.qml (+44/-8)
qml/Stages/SurfaceContainer.qml (+2/-1)
tests/mocks/Unity/Application/ApplicationInfo.cpp (+9/-1)
tests/mocks/Unity/Application/MirSurface.cpp (+8/-0)
tests/mocks/Unity/Application/MirSurface.h (+5/-0)
tests/qmltests/Stages/tst_ApplicationWindow.qml (+35/-9)
To merge this branch: bzr merge lp:~dandrader/unity8/surfaceDrawn
Reviewer Review Type Date Requested Status
Daniel d'Andrada (community) Disapprove
Unity8 CI Bot continuous-integration Needs Fixing
Nick Dedekind (community) Needs Fixing
Review via email: mp+294427@code.launchpad.net

Commit message

Ensure MirSurface is drawn before showing it

It's now possible to show prompt surfaces while the application surface is still blank (under a splash screen)

Description of the change

This is a better fix and thus replaces https://code.launchpad.net/~dandrader/qtmir/promptBeforeSurfaceDraws/+merge/294297

* Are there any related MPs required for this MP to build/function as expected? Please list.

https://code.launchpad.net/~dandrader/qtmir/surfaceDrawn/+merge/294428
https://code.launchpad.net/~dandrader/unity-api/surfaceDrawn/+merge/294426

* 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

* If you changed the UI, has there been a design review?
N/A

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

FAILED: Continuous integration, rev:2395
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1196/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1599
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1554
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1554
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1554/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1554/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1554/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1554/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1554/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1554/console

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Nick Dedekind (nick-dedekind) :
review: Needs Fixing
lp:~dandrader/unity8/surfaceDrawn updated
2396. By Daniel d'Andrada

No need to use a loader

Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

FAILED: Continuous integration, rev:2396
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1226/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1643
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1596
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1596
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1589/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1589/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1589/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1589/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1589/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1589/console

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

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

On 12/05/2016 07:25, Nick Dedekind wrote:
> No recursion - Doesn't need to be a Loader here.

Right! Fixed.

> Think there must be a better way so that we don't need surface repeaters in both ApplicationWindow & SurfaceContainer.

I also don't like the code duplication, but nothing better came to mind.
At least the code duplicated is rather simple.

> Can the transitions in ApplicationWindow not manage the surfaceItem visibility rather than the surfaceContainer visibilty? That was the children will still be visible.

Won't work. If they're kept together the splash will either be in front
of them all or behind them all. I have to split them so that the splash
only covers (and thus visually replaces) the root surface.

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

This is not enough for bug 1581559

review: Disapprove

Unmerged revisions

2396. By Daniel d'Andrada

No need to use a loader

2395. By Daniel d'Andrada

Ensure MirSurface is drawn before showing it

It's now possible to show prompt surfaces while the application surface
is still blank (under a splash screen)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'CMakeLists.txt'
2--- CMakeLists.txt 2016-04-27 15:01:10 +0000
3+++ CMakeLists.txt 2016-05-16 17:37:56 +0000
4@@ -57,7 +57,7 @@
5 find_package(Qt5Concurrent 5.4 REQUIRED)
6 find_package(Qt5Sql 5.4 REQUIRED)
7
8-pkg_check_modules(APPLICATION_API REQUIRED unity-shell-application=15)
9+pkg_check_modules(APPLICATION_API REQUIRED unity-shell-application=16)
10 pkg_check_modules(GEONAMES REQUIRED geonames>=0.2)
11 pkg_check_modules(GIO REQUIRED gio-2.0>=2.32)
12 pkg_check_modules(GLIB REQUIRED glib-2.0>=2.32)
13
14=== modified file 'debian/control'
15--- debian/control 2016-05-04 18:09:25 +0000
16+++ debian/control 2016-05-16 17:37:56 +0000
17@@ -30,7 +30,7 @@
18 libqt5xmlpatterns5-dev,
19 libsystemsettings-dev,
20 libudev-dev,
21- libunity-api-dev (>= 7.111),
22+ libunity-api-dev (>= 7.112),
23 libusermetricsoutput1-dev,
24 # Need those X11 libs touch emulation from mouse events in manual QML tests on a X11 desktop
25 libx11-dev[!armhf],
26@@ -133,7 +133,7 @@
27 qtdeclarative5-ubuntu-ui-toolkit-plugin (>= 1.3.1845) | qtdeclarative5-ubuntu-ui-toolkit-plugin-gles (>= 1.3.1845),
28 qtdeclarative5-unity-notifications-plugin (>= 0.1.2) | unity-notifications-impl,
29 ubuntu-thumbnailer-impl-0,
30- unity-application-impl-15,
31+ unity-application-impl-16,
32 unity-notifications-impl-3,
33 unity-plugin-scopes | unity-scopes-impl,
34 unity-scopes-impl-12,
35@@ -179,7 +179,7 @@
36 Depends: ${misc:Depends},
37 ${shlibs:Depends},
38 Provides: unity-application-impl,
39- unity-application-impl-15,
40+ unity-application-impl-16,
41 Replaces: unity8-autopilot (<< 8.02+15.04.20150422-0ubuntu1)
42 Description: Fake environment for running Unity 8 shell
43 Provides fake implementations of some QML modules used by Unity 8 shell
44
45=== modified file 'qml/Stages/ApplicationWindow.qml'
46--- qml/Stages/ApplicationWindow.qml 2016-04-27 15:01:10 +0000
47+++ qml/Stages/ApplicationWindow.qml 2016-05-16 17:37:56 +0000
48@@ -24,7 +24,7 @@
49 implicitHeight: surfaceContainer.implicitHeight
50
51 // to be read from outside
52- property alias interactive: surfaceContainer.interactive
53+ property bool interactive: true
54 property bool orientationChangesEnabled: d.supportsSurfaceResize ? d.surfaceOldEnoughToBeResized : true
55 readonly property string title: surface && surface.name !== "" ? surface.name : d.name
56
57@@ -63,7 +63,9 @@
58 surfaceContainer.surface = surface;
59 d.liveSurface = surface.live;
60 d.hadSurface = false;
61- surfaceInitTimer.start();
62+ if (surface.drawn) {
63+ surfaceInitTimer.start();
64+ }
65 } else {
66 if (d.surfaceInitialized) {
67 d.hadSurface = true;
68@@ -73,6 +75,15 @@
69 }
70 }
71
72+ Connections {
73+ target: surface
74+ onDrawnChanged: {
75+ if (surface.drawn) {
76+ surfaceInitTimer.start();
77+ }
78+ }
79+ }
80+
81 QtObject {
82 id: d
83
84@@ -120,6 +131,17 @@
85 || (application.supportedOrientations & Qt.InvertedLandscapeOrientation))
86
87 property bool surfaceOldEnoughToBeResized: false
88+
89+ property var focusedChild: {
90+ if (promptSurfacesRepeater.count == 0) {
91+ return surfaceContainer;
92+ } else {
93+ return promptSurfacesRepeater.itemAt(promptSurfacesRepeater.count - 1);
94+ }
95+ }
96+ onFocusedChildChanged: {
97+ focusedChild.focus = true;
98+ }
99 }
100
101 Binding {
102@@ -159,6 +181,16 @@
103 }
104 }
105
106+ SurfaceContainer {
107+ id: surfaceContainer
108+ requestedWidth: root.requestedWidth
109+ requestedHeight: root.requestedHeight
110+ interactive: root.interactive
111+ surfaceOrientationAngle: application && application.rotatesWindowContents ? root.surfaceOrientationAngle : 0
112+ focus: true
113+ loadSurfaceChildren: false
114+ }
115+
116 Loader {
117 id: splashLoader
118 visible: active
119@@ -178,12 +210,16 @@
120 }
121 }
122
123- SurfaceContainer {
124- id: surfaceContainer
125- requestedWidth: root.requestedWidth
126- requestedHeight: root.requestedHeight
127- surfaceOrientationAngle: application && application.rotatesWindowContents ? root.surfaceOrientationAngle : 0
128- focus: true
129+ Repeater {
130+ id: promptSurfacesRepeater
131+ model: surfaceContainer.surface ? surfaceContainer.surface.promptSurfaceList : null
132+ delegate: SurfaceContainer {
133+ interactive: index == (promptSurfacesRepeater.count - 1) && root.interactive
134+ surface: model.surface
135+ width: root.width
136+ height: root.height
137+ inPromptSession: true
138+ }
139 }
140
141 // SurfaceContainer size drives ApplicationWindow size
142
143=== modified file 'qml/Stages/SurfaceContainer.qml'
144--- qml/Stages/SurfaceContainer.qml 2016-04-04 13:38:56 +0000
145+++ qml/Stages/SurfaceContainer.qml 2016-05-16 17:37:56 +0000
146@@ -34,6 +34,7 @@
147 property int surfaceOrientationAngle: 0
148 property bool resizeSurface: true
149 property bool inPromptSession: false
150+ property bool loadSurfaceChildren: true
151
152 onSurfaceChanged: {
153 // Not a binding because animations might remove the surface from the surfaceItem
154@@ -125,7 +126,7 @@
155 Repeater {
156 id: childSurfacesRepeater
157 objectName: "childSurfacesRepeater"
158- model: root.surface ? root.surface.promptSurfaceList : null
159+ model: root.loadSurfaceChildren && root.surface ? root.surface.promptSurfaceList : null
160
161 delegate: Loader {
162 objectName: "childDelegate" + index
163
164=== modified file 'tests/mocks/Unity/Application/ApplicationInfo.cpp'
165--- tests/mocks/Unity/Application/ApplicationInfo.cpp 2016-04-27 15:01:10 +0000
166+++ tests/mocks/Unity/Application/ApplicationInfo.cpp 2016-05-16 17:37:56 +0000
167@@ -61,7 +61,11 @@
168
169 m_surfaceCreationTimer.setSingleShot(true);
170 m_surfaceCreationTimer.setInterval(500);
171- connect(&m_surfaceCreationTimer, &QTimer::timeout, this, &ApplicationInfo::createSurface);
172+ connect(&m_surfaceCreationTimer, &QTimer::timeout, this, [this]() {
173+ this->createSurface();
174+ auto surface = static_cast<MirSurface*>(m_surfaceList.get(m_surfaceList.count() - 1));
175+ QTimer::singleShot(50, surface, &MirSurface::drawFirstFrame);
176+ });
177 }
178
179 ApplicationInfo::ApplicationInfo(QObject *parent)
180@@ -238,6 +242,10 @@
181 if (value != m_manualSurfaceCreation) {
182 m_manualSurfaceCreation = value;
183 Q_EMIT manualSurfaceCreationChanged(value);
184+
185+ if (m_manualSurfaceCreation && m_surfaceCreationTimer.isActive()) {
186+ m_surfaceCreationTimer.stop();
187+ }
188 }
189 }
190
191
192=== modified file 'tests/mocks/Unity/Application/MirSurface.cpp'
193--- tests/mocks/Unity/Application/MirSurface.cpp 2016-04-27 15:01:10 +0000
194+++ tests/mocks/Unity/Application/MirSurface.cpp 2016-05-16 17:37:56 +0000
195@@ -307,6 +307,14 @@
196 }
197 }
198
199+void MirSurface::drawFirstFrame()
200+{
201+ DEBUG_MSG("");
202+ Q_ASSERT(!m_drawn);
203+ m_drawn = true;
204+ Q_EMIT drawnChanged(true);
205+}
206+
207 void MirSurface::doResize(int width, int height)
208 {
209 bool changed = false;
210
211=== modified file 'tests/mocks/Unity/Application/MirSurface.h'
212--- tests/mocks/Unity/Application/MirSurface.h 2016-04-27 15:01:10 +0000
213+++ tests/mocks/Unity/Application/MirSurface.h 2016-05-16 17:37:56 +0000
214@@ -109,6 +109,8 @@
215
216 unity::shell::application::MirSurfaceListInterface* promptSurfaceList() override;
217
218+ bool drawn() const override { return m_drawn; }
219+
220 Q_INVOKABLE void raise() override;
221
222 ////
223@@ -132,6 +134,8 @@
224
225 Q_INVOKABLE void createPromptSurface();
226
227+ Q_INVOKABLE void drawFirstFrame();
228+
229 /////
230 // internal mock stuff
231
232@@ -183,6 +187,7 @@
233 bool m_activeFocus;
234 int m_width;
235 int m_height;
236+ bool m_drawn{false};
237
238 int m_minimumWidth{0};
239 int m_minimumHeight{0};
240
241=== modified file 'tests/qmltests/Stages/tst_ApplicationWindow.qml'
242--- tests/qmltests/Stages/tst_ApplicationWindow.qml 2016-04-04 13:37:49 +0000
243+++ tests/qmltests/Stages/tst_ApplicationWindow.qml 2016-05-16 17:37:56 +0000
244@@ -33,7 +33,6 @@
245 Component.onCompleted: {
246 root.fakeApplication = ApplicationManager.add("gallery-app");
247 root.fakeApplication.manualSurfaceCreation = true;
248- root.fakeApplication.setState(ApplicationInfoInterface.Starting);
249 applicationWindowLoader.item.application = root.fakeApplication;
250 }
251 property QtObject fakeApplication: null
252@@ -78,7 +77,7 @@
253 Layout.fillWidth: true
254
255 CheckBox {
256- id: surfaceCheckbox;
257+ id: surfaceCheckbox
258 checked: false;
259 activeFocusOnPress: false
260 onCheckedChanged: {
261@@ -86,10 +85,8 @@
262 return;
263
264 if (checked) {
265- applicationWindowLoader.item.surface = SurfaceManager.createSurface("foo",
266- Mir.NormalType, Mir.MaximizedState, root.fakeApplication);
267-
268- root.fakeApplication.setState(ApplicationInfoInterface.Running);
269+ root.fakeApplication.createSurface();
270+ applicationWindowLoader.item.surface = root.fakeApplication.surfaceList.get(0);
271 } else {
272 if (applicationWindowLoader.item.surface) {
273 applicationWindowLoader.item.surface.setLive(false);
274@@ -101,6 +98,23 @@
275 text: "Surface"
276 anchors.verticalCenter: parent.verticalCenter
277 }
278+
279+ Item { width: units.gu(2) }
280+
281+ Button {
282+ id: drawButton;
283+ activeFocusOnPress: false
284+ property var surface: root.fakeApplication && root.fakeApplication.surfaceList.count > 0
285+ ? root.fakeApplication.surfaceList.get(0) : null
286+ enabled: surface !== null && !surface.drawn
287+ onClicked: {
288+ if (applicationWindowLoader.status !== Loader.Ready)
289+ return;
290+
291+ surface.drawFirstFrame();
292+ }
293+ text: "Draw"
294+ }
295 }
296
297 Rectangle {
298@@ -246,11 +260,12 @@
299
300 setApplicationState(appStarting);
301 surfaceCheckbox.checked = false;
302-
303- ApplicationManager.stopApplication("gallery-app");
304+ drawButton.clicked();
305+
306+ killApps();
307+
308 root.fakeApplication = ApplicationManager.add("gallery-app");
309 root.fakeApplication.manualSurfaceCreation = true;
310- root.fakeApplication.setState(ApplicationInfoInterface.Starting);
311
312 applicationWindowLoader.active = true;
313
314@@ -270,6 +285,7 @@
315
316 if (data.swapInitOrder) {
317 surfaceCheckbox.checked = true;
318+ drawButton.clicked();
319 } else {
320 setApplicationState(appRunning);
321 }
322@@ -280,6 +296,7 @@
323 setApplicationState(appRunning);
324 } else {
325 surfaceCheckbox.checked = true;
326+ drawButton.clicked();
327 }
328
329 tryCompare(stateGroup, "state", "surface");
330@@ -287,6 +304,7 @@
331
332 function test_suspendedAppShowsSurface() {
333 surfaceCheckbox.checked = true;
334+ drawButton.clicked();
335 setApplicationState(appRunning);
336
337 tryCompare(stateGroup, "state", "surface");
338@@ -301,6 +319,7 @@
339
340 function test_killedAppShowsScreenshot() {
341 surfaceCheckbox.checked = true;
342+ drawButton.clicked();
343 setApplicationState(appRunning);
344 tryCompare(stateGroup, "state", "surface");
345
346@@ -321,6 +340,7 @@
347 var screenshotImage = findChild(applicationWindow, "screenshotImage");
348
349 surfaceCheckbox.checked = true;
350+ drawButton.clicked();
351 setApplicationState(appRunning);
352 tryCompare(stateGroup, "state", "surface");
353 waitUntilTransitionsEnd(stateGroup);
354@@ -348,6 +368,7 @@
355 verify(stateGroup.state === "screenshot");
356
357 surfaceCheckbox.checked = true;
358+ drawButton.clicked();
359
360 tryCompare(stateGroup, "state", "surface");
361 tryCompare(screenshotImage, "status", Image.Null);
362@@ -355,6 +376,7 @@
363
364 function test_appCrashed() {
365 surfaceCheckbox.checked = true;
366+ drawButton.clicked();
367 setApplicationState(appRunning);
368 tryCompare(stateGroup, "state", "surface");
369 waitUntilTransitionsEnd(stateGroup);
370@@ -369,6 +391,7 @@
371
372 function test_keepSurfaceWhileInvisible() {
373 surfaceCheckbox.checked = true;
374+ drawButton.clicked();
375 setApplicationState(appRunning);
376 tryCompare(stateGroup, "state", "surface");
377 waitUntilTransitionsEnd(stateGroup);
378@@ -390,6 +413,7 @@
379 function test_touchesReachSurfaceWhenItsShown() {
380 setApplicationState(appRunning);
381 surfaceCheckbox.checked = true;
382+ drawButton.clicked();
383
384 tryCompare(stateGroup, "state", "surface");
385
386@@ -410,6 +434,7 @@
387
388 function test_showNothingOnSuddenSurfaceLoss() {
389 surfaceCheckbox.checked = true;
390+ drawButton.clicked();
391 setApplicationState(appRunning);
392 tryCompare(stateGroup, "state", "surface");
393 waitUntilTransitionsEnd(stateGroup);
394@@ -423,6 +448,7 @@
395 applicationWindow.interactive = false;
396 applicationWindow.interactive = true;
397 surfaceCheckbox.checked = true;
398+ drawButton.clicked();
399
400 compare(applicationWindow.surface.activeFocus, true);
401

Subscribers

People subscribed via source and target branches