Unity 8

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

  1. initialSurfaceGeom
  2. Merge into trunk
Proposed by Daniel d'Andrada
Status: Merged
Approved by: Michał Sawicz
Approved revision: 2140
Merged at revision: 2184
Proposed branch: lp:~dandrader/unity8/initialSurfaceGeom
Merge into: lp:unity8
Diff against target: 92 lines (+27/-3)
4 files modified
qml/Stages/ApplicationWindow.qml (+7/-1)
tests/mocks/Unity/Application/ApplicationInfo.cpp (+14/-1)
tests/mocks/Unity/Application/ApplicationInfo.h (+4/-1)
tests/plugins/Unity/Launcher/launchermodeltest.cpp (+2/-0)
To merge this branch: bzr merge lp:~dandrader/unity8/initialSurfaceGeom
Reviewer Review Type Date Requested Status
Michał Sawicz Abstain
PS Jenkins bot (community) continuous-integration Needs Fixing
Unity8 CI Bot continuous-integration Needs Fixing
Nick Dedekind (community) Approve
Review via email: mp+283223@code.launchpad.net

Commit message

Set initial surface size

Removes flicker on start up in desktop mode

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/unity-api/initialSurfaceGeom/+merge/283222
https://code.launchpad.net/~dandrader/qtmir/initialSurfaceGeom/+merge/283227

* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes. Tested on staged and windowed mode on Nexus 4, windowed mode on Nexus 7.
Also ran ubuntu-ui-toolkit autopilot tests and there were no regressions.

* Did you make sure that your branch does not contain spurious tags?
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:2137
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/142/
Executed test runs:

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

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

FAILED: Continuous integration, rev:2137
http://jenkins.qa.ubuntu.com/job/unity8-ci/7106/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/6082/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-xenial-touch/521/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/1811/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-qmluitest-xenial-amd64/514/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/1706/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/1706/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-xenial-amd64-ci/513/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-xenial-i386-ci/512/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/6093/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/519/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/7106/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

84 +QSize ApplicationInfo::initialSurfaceSize() const
111 + QSize initialSurfaceSize() const override;
112 + void setInitialSurfaceSize(const QSize &size) override;

Surface[Geometry] rather than Surface[Size]

review: Needs Fixing
Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

>
> 84 +QSize ApplicationInfo::initialSurfaceSize() const
> 111 + QSize initialSurfaceSize() const override;
> 112 + void setInitialSurfaceSize(const QSize &size) override;
>
> Surface[Geometry] rather than Surface[Size]

or the other way around...

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

fixed

Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

Tested and works

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

FAILED: Continuous integration, rev:2138
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/181/
Executed test runs:

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

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

FAILED: Continuous integration, rev:2138
http://jenkins.qa.ubuntu.com/job/unity8-ci/7139/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/6163/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-xenial-touch/554/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/1844/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-qmluitest-xenial-amd64/547/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/1739/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/1739/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-xenial-amd64-ci/546/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-xenial-i386-ci/545/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/6174/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/552/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/7139/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Michał Sawicz (saviq) wrote :

Something's not right here (or with sizeHints) when first starting:

http://imgur.com/XipVdgz

The bright bar at the top is actually the dash on startup...

Here's all logs when I switch to windowed mode (staged mode looked fine):

$ cat unity8.log
qtmir.surfaces: MirSurface[0x25da7a0,"unity8-dash"]::setFocus(false)
file:///usr/share/unity8//Stages/DesktopStage.qml:252:9: QML CrossFadeImage: Binding loop detected for property "sourceSize.height"
qtmir.surfaces: MirSurfaceItem::MirSurfaceItem
qtmir.surfaces: MirSurfaceItem::setSurface surface=qtmir::MirSurface(0x25da7a0)
qtmir.surfaces: MirSurface[0x25da7a0,"unity8-dash"]::registerView(36788008) after=2
qtmir.surfaces: MirSurface[0x25da7a0,"unity8-dash"]::setFocus(false)
qtmir.surfaces: Setting keymap with empty layout is not supported
qtmir.surfaces: MirSurfaceItem::setOrientationAngle(0)
qtmir.applications: Application::setInitialSurfaceSize - appId="unity8-dash" size=QSize(-1, -1)
qtmir.surfaces: MirSurfaceItem::MirSurfaceItem
qtmir.surfaces: MirSurfaceItem::setSurface surface=qtmir::MirSurface(0x25da7a0)
qtmir.surfaces: MirSurface[0x25da7a0,"unity8-dash"]::registerView(37781296) after=3
qtmir.surfaces: MirSurface[0x25da7a0,"unity8-dash"]::setFocus(false)
qtmir.surfaces: Setting keymap with empty layout is not supported
qtmir.surfaces: MirSurfaceItem::setOrientationAngle(0)
qtmir.surfaces: MirSurface[0x25da7a0,"unity8-dash"]::setFocus(true)
qtmir.applications: Application::setInitialSurfaceSize - appId="unity8-dash" size=QSize(180, 126)
qtmir.applications: Application::setInitialSurfaceSize - appId="unity8-dash" size=QSize(0, 126)
qtmir.applications: Application::setInitialSurfaceSize - appId="unity8-dash" size=QSize(0, -108)

^^^^ this looks wrong

qtmir.surfaces: MirSurfaceItem::~MirSurfaceItem - this= qtmir::MirSurfaceItem (this = 0x24c8c18 , name= "surfaceItem" , parent = 0x0 , geometry = QRectF(0,0 1920x1146) , z = 0 )
qtmir.surfaces: MirSurfaceItem::setSurface surface=QObject(0x0)
qtmir.surfaces: MirSurface[0x25da7a0,"unity8-dash"]::unregisterView(38571032) after=2 live=true
qtmir.surfaces: MirSurface[0x27077e0,"-"]::resize old (1920,1146), new (1776,1146)
qtmir.clipboard: D-Bus GetContents - returning 0 bytes
loadExtendedAttributes: menu item does not contain 'x-canonical-scroll-action'
loadExtendedAttributes: menu item does not contain 'x-canonical-secondary-action'
loadExtendedAttributes: menu item does not contain 'x-canonical-scroll-action'
loadExtendedAttributes: menu item does not contain 'x-canonical-secondary-action'
loadExtendedAttributes: menu item does not contain 'x-canonical-scroll-action'
loadExtendedAttributes: menu item does not contain 'x-canonical-secondary-action'
loadExtendedAttributes: menu item does not contain 'x-canonical-scroll-action'
loadExtendedAttributes: menu item does not contain 'x-canonical-secondary-action'

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

FAILED: Continuous integration, rev:2140
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/287/
Executed test runs:

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

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

FAILED: Continuous integration, rev:2140
http://jenkins.qa.ubuntu.com/job/unity8-ci/7229/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/6370/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-xenial-touch/644/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/1934/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-qmluitest-xenial-amd64/637/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/1829/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/1829/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-xenial-amd64-ci/636/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-xenial-i386-ci/635/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/6381/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/642/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/7229/rebuild

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

This mean ApplicationWindow.requestedHeight was set to (at least momentarily, cant' tell the whole story from that snippet) -108

~dandrader/unity8/initialSurfaceGeom has absolutely no influence over the ApplicationWindow.requestedHeight value. You're hitting the wrong MP.

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

Installed initialSurfaceGeom + sizeHints on my nexus 7.

Then, in a terminal, was doing:
$ gsettings set com.canonical.Unity8 usage-mode Automatic
$ gsettings set com.canonical.Unity8 usage-mode Windowed
$ gsettings set com.canonical.Unity8 usage-mode Automatic
$ gsettings set com.canonical.Unity8 usage-mode Windowed
and so on (to switch between TabletStage and DesktopStage)...

Couldn't reproduce the issue. Need more details on how to reproduce it. Maybe it only happens when you have all MPs from https://requests.ci-train.ubuntu.com/#/ticket/938 installed (didn't try that yet)?

Revision history for this message
Nick Dedekind (nick-dedekind) wrote :
Download full text (3.2 KiB)

> Something's not right here (or with sizeHints) when first starting:
>
> http://imgur.com/XipVdgz
>
> The bright bar at the top is actually the dash on startup...
>
> Here's all logs when I switch to windowed mode (staged mode looked fine):
>
> $ cat unity8.log
> qtmir.surfaces: MirSurface[0x25da7a0,"unity8-dash"]::setFocus(false)
> file:///usr/share/unity8//Stages/DesktopStage.qml:252:9: QML CrossFadeImage:
> Binding loop detected for property "sourceSize.height"
> qtmir.surfaces: MirSurfaceItem::MirSurfaceItem
> qtmir.surfaces: MirSurfaceItem::setSurface
> surface=qtmir::MirSurface(0x25da7a0)
> qtmir.surfaces: MirSurface[0x25da7a0,"unity8-dash"]::registerView(36788008)
> after=2
> qtmir.surfaces: MirSurface[0x25da7a0,"unity8-dash"]::setFocus(false)
> qtmir.surfaces: Setting keymap with empty layout is not supported
> qtmir.surfaces: MirSurfaceItem::setOrientationAngle(0)
> qtmir.applications: Application::setInitialSurfaceSize - appId="unity8-dash"
> size=QSize(-1, -1)
> qtmir.surfaces: MirSurfaceItem::MirSurfaceItem
> qtmir.surfaces: MirSurfaceItem::setSurface
> surface=qtmir::MirSurface(0x25da7a0)
> qtmir.surfaces: MirSurface[0x25da7a0,"unity8-dash"]::registerView(37781296)
> after=3
> qtmir.surfaces: MirSurface[0x25da7a0,"unity8-dash"]::setFocus(false)
> qtmir.surfaces: Setting keymap with empty layout is not supported
> qtmir.surfaces: MirSurfaceItem::setOrientationAngle(0)
> qtmir.surfaces: MirSurface[0x25da7a0,"unity8-dash"]::setFocus(true)
> qtmir.applications: Application::setInitialSurfaceSize - appId="unity8-dash"
> size=QSize(180, 126)
> qtmir.applications: Application::setInitialSurfaceSize - appId="unity8-dash"
> size=QSize(0, 126)
> qtmir.applications: Application::setInitialSurfaceSize - appId="unity8-dash"
> size=QSize(0, -108)
>
> ^^^^ this looks wrong
>
> qtmir.surfaces: MirSurfaceItem::~MirSurfaceItem - this= qtmir::MirSurfaceItem
> (this = 0x24c8c18 , name= "surfaceItem" , parent = 0x0 , geometry = QRectF(0,0
> 1920x1146) , z = 0 )
> qtmir.surfaces: MirSurfaceItem::setSurface surface=QObject(0x0)
> qtmir.surfaces: MirSurface[0x25da7a0,"unity8-dash"]::unregisterView(38571032)
> after=2 live=true
> qtmir.surfaces: MirSurface[0x27077e0,"-"]::resize old (1920,1146), new
> (1776,1146)
> qtmir.clipboard: D-Bus GetContents - returning 0 bytes
> loadExtendedAttributes: menu item does not contain 'x-canonical-scroll-action'
> loadExtendedAttributes: menu item does not contain 'x-canonical-secondary-
> action'
> loadExtendedAttributes: menu item does not contain 'x-canonical-scroll-action'
> loadExtendedAttributes: menu item does not contain 'x-canonical-secondary-
> action'
> loadExtendedAttributes: menu item does not contain 'x-canonical-scroll-action'
> loadExtendedAttributes: menu item does not contain 'x-canonical-secondary-
> action'
> loadExtendedAttributes: menu item does not contain 'x-canonical-scroll-action'
> loadExtendedAttributes: menu item does not contain 'x-canonical-secondary-
> action'

I think you may have hit the same problem I was seeing with windows getting small geo when switching to desktop mode in the form factor work. There's a bug in the Desktop stage which screws up the requested height/width.

...

Read more...

Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

And i think it only happens if the app is fullscreen or maximized.

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

The offending MP seems to be lp:~lukas-kde/unity8/windowOpenCloseAnimations afterall:
https://code.launchpad.net/~lukas-kde/unity8/windowOpenCloseAnimations/+merge/284775/comments/726615

Revision history for this message
Michał Sawicz (saviq) :
review: Abstain

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'qml/Stages/ApplicationWindow.qml'
2--- qml/Stages/ApplicationWindow.qml 2015-11-23 13:52:39 +0000
3+++ qml/Stages/ApplicationWindow.qml 2016-02-02 18:08:07 +0000
4@@ -1,5 +1,5 @@
5 /*
6- * Copyright 2014-2015 Canonical Ltd.
7+ * Copyright 2014-2016 Canonical Ltd.
8 *
9 * This program is free software; you can redistribute it and/or modify
10 * it under the terms of the GNU Lesser General Public License as published by
11@@ -82,6 +82,12 @@
12 property bool surfaceOldEnoughToBeResized: false
13 }
14
15+ Binding {
16+ target: root.application
17+ property: "initialSurfaceSize"
18+ value: Qt.size(root.requestedWidth, root.requestedHeight)
19+ }
20+
21 Timer {
22 id: surfaceInitTimer
23 interval: 100
24
25=== modified file 'tests/mocks/Unity/Application/ApplicationInfo.cpp'
26--- tests/mocks/Unity/Application/ApplicationInfo.cpp 2015-12-08 15:36:53 +0000
27+++ tests/mocks/Unity/Application/ApplicationInfo.cpp 2016-02-02 18:08:07 +0000
28@@ -1,5 +1,5 @@
29 /*
30- * Copyright (C) 2013-2015 Canonical, Ltd.
31+ * Copyright (C) 2013-2016 Canonical, Ltd.
32 *
33 * This program is free software; you can redistribute it and/or modify
34 * it under the terms of the GNU General Public License as published by
35@@ -278,3 +278,16 @@
36 Q_EMIT exemptFromLifecycleChanged(m_exemptFromLifecycle);
37 }
38 }
39+
40+QSize ApplicationInfo::initialSurfaceSize() const
41+{
42+ return m_initialSurfaceSize;
43+}
44+
45+void ApplicationInfo::setInitialSurfaceSize(const QSize &size)
46+{
47+ if (size != m_initialSurfaceSize) {
48+ m_initialSurfaceSize = size;
49+ Q_EMIT initialSurfaceSizeChanged(m_initialSurfaceSize);
50+ }
51+}
52
53=== modified file 'tests/mocks/Unity/Application/ApplicationInfo.h'
54--- tests/mocks/Unity/Application/ApplicationInfo.h 2015-12-07 12:35:11 +0000
55+++ tests/mocks/Unity/Application/ApplicationInfo.h 2016-02-02 18:08:07 +0000
56@@ -1,5 +1,5 @@
57 /*
58- * Copyright (C) 2013-2015 Canonical, Ltd.
59+ * Copyright (C) 2013-2016 Canonical, Ltd.
60 *
61 * This program is free software; you can redistribute it and/or modify
62 * it under the terms of the GNU General Public License as published by
63@@ -95,6 +95,8 @@
64 bool exemptFromLifecycle() const override;
65 void setExemptFromLifecycle(bool) override;
66
67+ QSize initialSurfaceSize() const override;
68+ void setInitialSurfaceSize(const QSize &size) override;
69 public:
70 void setSession(Session* session);
71 Session* session() const { return m_session; }
72@@ -129,6 +131,7 @@
73 RequestedState m_requestedState;
74 bool m_isTouchApp;
75 bool m_exemptFromLifecycle;
76+ QSize m_initialSurfaceSize;
77
78 bool m_manualSurfaceCreation;
79 };
80
81=== modified file 'tests/plugins/Unity/Launcher/launchermodeltest.cpp'
82--- tests/plugins/Unity/Launcher/launchermodeltest.cpp 2015-12-03 18:10:39 +0000
83+++ tests/plugins/Unity/Launcher/launchermodeltest.cpp 2016-02-02 18:08:07 +0000
84@@ -61,6 +61,8 @@
85 bool isTouchApp() const override { return true; }
86 bool exemptFromLifecycle() const override { return false; }
87 void setExemptFromLifecycle(bool) override {}
88+ QSize initialSurfaceSize() const override { return QSize(); }
89+ void setInitialSurfaceSize(const QSize &) override {}
90
91 // Methods used for mocking (not in the interface)
92 void setFocused(bool focused) { m_focused = focused; Q_EMIT focusedChanged(focused); }

Subscribers

People subscribed via source and target branches