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

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 :
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 :
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 :
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 :
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
=== modified file 'qml/Stages/ApplicationWindow.qml'
--- qml/Stages/ApplicationWindow.qml 2015-11-23 13:52:39 +0000
+++ qml/Stages/ApplicationWindow.qml 2016-02-02 18:08:07 +0000
@@ -1,5 +1,5 @@
1/*1/*
2 * Copyright 2014-2015 Canonical Ltd.2 * Copyright 2014-2016 Canonical Ltd.
3 *3 *
4 * This program is free software; you can redistribute it and/or modify4 * This program is free software; you can redistribute it and/or modify
5 * it under the terms of the GNU Lesser General Public License as published by5 * it under the terms of the GNU Lesser General Public License as published by
@@ -82,6 +82,12 @@
82 property bool surfaceOldEnoughToBeResized: false82 property bool surfaceOldEnoughToBeResized: false
83 }83 }
8484
85 Binding {
86 target: root.application
87 property: "initialSurfaceSize"
88 value: Qt.size(root.requestedWidth, root.requestedHeight)
89 }
90
85 Timer {91 Timer {
86 id: surfaceInitTimer92 id: surfaceInitTimer
87 interval: 10093 interval: 100
8894
=== modified file 'tests/mocks/Unity/Application/ApplicationInfo.cpp'
--- tests/mocks/Unity/Application/ApplicationInfo.cpp 2015-12-08 15:36:53 +0000
+++ tests/mocks/Unity/Application/ApplicationInfo.cpp 2016-02-02 18:08:07 +0000
@@ -1,5 +1,5 @@
1/*1/*
2 * Copyright (C) 2013-2015 Canonical, Ltd.2 * Copyright (C) 2013-2016 Canonical, Ltd.
3 *3 *
4 * This program is free software; you can redistribute it and/or modify4 * This program is free software; you can redistribute it and/or modify
5 * it under the terms of the GNU General Public License as published by5 * it under the terms of the GNU General Public License as published by
@@ -278,3 +278,16 @@
278 Q_EMIT exemptFromLifecycleChanged(m_exemptFromLifecycle);278 Q_EMIT exemptFromLifecycleChanged(m_exemptFromLifecycle);
279 }279 }
280}280}
281
282QSize ApplicationInfo::initialSurfaceSize() const
283{
284 return m_initialSurfaceSize;
285}
286
287void ApplicationInfo::setInitialSurfaceSize(const QSize &size)
288{
289 if (size != m_initialSurfaceSize) {
290 m_initialSurfaceSize = size;
291 Q_EMIT initialSurfaceSizeChanged(m_initialSurfaceSize);
292 }
293}
281294
=== modified file 'tests/mocks/Unity/Application/ApplicationInfo.h'
--- tests/mocks/Unity/Application/ApplicationInfo.h 2015-12-07 12:35:11 +0000
+++ tests/mocks/Unity/Application/ApplicationInfo.h 2016-02-02 18:08:07 +0000
@@ -1,5 +1,5 @@
1/*1/*
2 * Copyright (C) 2013-2015 Canonical, Ltd.2 * Copyright (C) 2013-2016 Canonical, Ltd.
3 *3 *
4 * This program is free software; you can redistribute it and/or modify4 * This program is free software; you can redistribute it and/or modify
5 * it under the terms of the GNU General Public License as published by5 * it under the terms of the GNU General Public License as published by
@@ -95,6 +95,8 @@
95 bool exemptFromLifecycle() const override;95 bool exemptFromLifecycle() const override;
96 void setExemptFromLifecycle(bool) override;96 void setExemptFromLifecycle(bool) override;
9797
98 QSize initialSurfaceSize() const override;
99 void setInitialSurfaceSize(const QSize &size) override;
98public:100public:
99 void setSession(Session* session);101 void setSession(Session* session);
100 Session* session() const { return m_session; }102 Session* session() const { return m_session; }
@@ -129,6 +131,7 @@
129 RequestedState m_requestedState;131 RequestedState m_requestedState;
130 bool m_isTouchApp;132 bool m_isTouchApp;
131 bool m_exemptFromLifecycle;133 bool m_exemptFromLifecycle;
134 QSize m_initialSurfaceSize;
132135
133 bool m_manualSurfaceCreation;136 bool m_manualSurfaceCreation;
134};137};
135138
=== modified file 'tests/plugins/Unity/Launcher/launchermodeltest.cpp'
--- tests/plugins/Unity/Launcher/launchermodeltest.cpp 2015-12-03 18:10:39 +0000
+++ tests/plugins/Unity/Launcher/launchermodeltest.cpp 2016-02-02 18:08:07 +0000
@@ -61,6 +61,8 @@
61 bool isTouchApp() const override { return true; }61 bool isTouchApp() const override { return true; }
62 bool exemptFromLifecycle() const override { return false; }62 bool exemptFromLifecycle() const override { return false; }
63 void setExemptFromLifecycle(bool) override {}63 void setExemptFromLifecycle(bool) override {}
64 QSize initialSurfaceSize() const override { return QSize(); }
65 void setInitialSurfaceSize(const QSize &) override {}
6466
65 // Methods used for mocking (not in the interface)67 // Methods used for mocking (not in the interface)
66 void setFocused(bool focused) { m_focused = focused; Q_EMIT focusedChanged(focused); }68 void setFocused(bool focused) { m_focused = focused; Q_EMIT focusedChanged(focused); }

Subscribers

People subscribed via source and target branches