Unity 8

Merge lp:~gerboland/unity8/initialSurfaceGeometry into lp:unity8

  1. initialSurfaceGeometry
  2. Merge into trunk
Proposed by Gerry Boland on 2014-08-21
Status: Work in progress
Proposed branch: lp:~gerboland/unity8/initialSurfaceGeometry
Merge into: lp:unity8
Prerequisite: lp:~dandrader/unity8/lifecycle
Diff against target: 190 lines (+72/-3)
8 files modified
debian/control (+2/-2)
qml/Stages/PhoneStage.qml (+13/-0)
qml/Stages/TabletStage.qml (+21/-0)
tests/mocks/Unity/Application/ApplicationManager.cpp (+20/-0)
tests/mocks/Unity/Application/ApplicationManager.h (+4/-0)
tests/plugins/Unity/Launcher/CMakeLists.txt (+1/-1)
tests/plugins/Unity/Launcher/launchermodeltest.cpp (+2/-0)
tests/qmltests/Stages/tst_PhoneStage.qml (+9/-0)
To merge this branch: bzr merge lp:~gerboland/unity8/initialSurfaceGeometry
Reviewer Review Type Date Requested Status
Albert Astals Cid (community) 2014-08-21 Needs Fixing on 2015-01-19
PS Jenkins bot continuous-integration 2014-08-21 Needs Fixing on 2015-01-15
Michał Sawicz 2014-08-21 Approve on 2014-08-21
Daniel d'Andrada 2014-08-21 Pending
Review via email: mp+231726@code.launchpad.net

This proposal supersedes a proposal from 2014-08-12.

Commit Message

Implement a surfaceSizer exported by qtmir to override the initial surface geometry requested by clients.

The SurfaceSizer is a Javascript function defined by unity8 which is registered with QtMir as a callback function whenever a client requests a new surface from Mir. It allows unity8 to override the client-requested width & height of the surface. This fixes the startup-resize visual glitching evident right now.

Description of the Change

Implement a surfaceSizer exported by qtmir to override the initial surface geometry requested by clients

 * Are there any related MPs required for this MP to build/function as expected? Please list.
https://code.launchpad.net/~gerboland/unity-api/surfaceSizerCallback/+merge/231698
https://code.launchpad.net/~gerboland/qtmir/initialSurfaceGeometry/+merge/231725
 * Did you perform an exploratory manual test run of your code change and any related functionality?
Y
 * Did you make sure that your branch does not contain spurious tags?
Y
 * 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.
Michał Sawicz (saviq) wrote : Posted in a previous version of this proposal

unity8-fake-env should provide unity-applications-4 (or 3?).

review: Needs Fixing
Michał Sawicz (saviq) wrote : Posted in a previous version of this proposal

Oh and you need to depend on the new unity-api.

review: Needs Fixing
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Gerry Boland (gerboland) wrote : Posted in a previous version of this proposal

Updated to newer unity-api and comments addressed

PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Daniel d'Andrada (dandrader) wrote : Posted in a previous version of this proposal

Commented in the diff.

review: Needs Information
Gerry Boland (gerboland) : Posted in a previous version of this proposal
Gerry Boland (gerboland) : Posted in a previous version of this proposal
Daniel d'Andrada (dandrader) : Posted in a previous version of this proposal
review: Needs Information
Albert Astals Cid (aacid) wrote : Posted in a previous version of this proposal

Text conflict in debian/control
1 conflicts encountered.

review: Needs Fixing
Gerry Boland (gerboland) : Posted in a previous version of this proposal
Gerry Boland (gerboland) wrote : Posted in a previous version of this proposal

@Daniel: replied to your question (sorry for delay, forgot to hit "save comment" until now)

PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Daniel d'Andrada (dandrader) wrote : Posted in a previous version of this proposal

See diff comment.

review: Needs Fixing
Gerry Boland (gerboland) wrote : Posted in a previous version of this proposal

Reply inline

Daniel d'Andrada (dandrader) : Posted in a previous version of this proposal
review: Approve
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Albert Astals Cid (aacid) wrote : Posted in a previous version of this proposal

Code does merge properly now

review: Abstain
Michał Sawicz (saviq) : Posted in a previous version of this proposal
review: Abstain
Michał Sawicz (saviq) wrote : Posted in a previous version of this proposal

This conflicts with lp:~dandrader/unity8/lifecycle that's already approved and slated for release soon, maybe rebase on that one and we'll land them together?

review: Needs Information
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Reply
Michał Sawicz (saviq) :
review: Needs Information
Reply
Gerry Boland (gerboland) :
Reply
Gerry Boland (gerboland) :
Reply
Michał Sawicz (saviq) :
Reply
Michał Sawicz (saviq) wrote :

 * Did you perform an exploratory manual test run of the code change and any related functionality?
Will do so in silo.

 * Did CI run pass? If not, please explain why.
N, missing dependencies

review: Approve
Reply
Gerry Boland (gerboland) wrote :

Is finally ready to be re-evaluated!

Reply
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Reply
lp:~gerboland/unity8/initialSurfaceGeometry updated on 2015-01-15
1160. By Gerry Boland on 2015-01-15

Remove Dash block workaround

1161. By Gerry Boland on 2015-01-15

Merge trunk

Albert Astals Cid (aacid) wrote :

Code looks good, haven't time to test. If nobody beats me to it, i'll do on Monday

review: Approve (code)
Reply
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Reply
Albert Astals Cid (aacid) wrote :

I'm going to put the Needs Fixing here but not sure if it belongs here or to the other two in the dependency chain.

When running Gallery with this patch, around 50% of the time i get the events view to do a weird jump when growing.

Can you please try and see if you can reproduce it? Because it looks weird and it doesn't seem to happen without the patch.

review: Needs Fixing
Reply
Gerry Boland (gerboland) wrote :

Just an update: so the visual glitching you see was for fullscreen apps only.

The previous behaviour actually suited fullscreen apps better, as before this work all app surfaces were initially fullscreen. Regular surfaces were only resized to fit under the panel after app drew its first frame - which is visually glitchy.

This work was to allow shell to give the app the correct initial surface size from the beginning, as opposed to a later resizing - and it works well for all except fullscreen apps - which I totally missed.

I've pushed changes to qtmir that grab the state of the initial surface (fullscreen, normal...) and so shell can make a more informed decision. However that's not working, as qtubuntu only declares a surface fullscreen *after* the surface was created. So need to fix that before this is ready

Reply

Unmerged revisions

1161. By Gerry Boland on 2015-01-15

Merge trunk

1160. By Gerry Boland on 2015-01-15

Remove Dash block workaround

1159. By Gerry Boland on 2015-01-13

Merge trunk

1158. By Gerry Boland on 2015-01-05

Merge trunk

1157. By Gerry Boland on 2014-12-09

Delay Dash DBus service creation slightly, to avoid possible shell deadlock

1156. By Gerry Boland on 2014-12-09

Merge trunk

1155. By Gerry Boland on 2014-12-08

Bump unity-api & qtmir dep

1154. By Gerry Boland on 2014-12-03

I hate you whitespace cheker

1153. By Gerry Boland on 2014-12-03

Undo all changes to Shell and Panel, start afresh

1152. By Gerry Boland on 2014-12-03

Merge current trunk

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/control'
2--- debian/control 2015-01-09 10:41:11 +0000
3+++ debian/control 2015-01-15 16:14:21 +0000
4@@ -27,7 +27,7 @@
5 libqmenumodel-dev (>= 0.2.9),
6 libqt5xmlpatterns5-dev,
7 libsystemsettings-dev,
8- libunity-api-dev (>= 7.94),
9+ libunity-api-dev (>= 7.95),
10 libusermetricsoutput1-dev,
11 libxcb1-dev,
12 pkg-config,
13@@ -91,7 +91,7 @@
14 qml-module-qt-labs-folderlistmodel,
15 qml-module-qtquick-xmllistmodel,
16 qtdeclarative5-gsettings1.0,
17- qtdeclarative5-qtmir-plugin (>= 0.4.4),
18+ qtdeclarative5-qtmir-plugin (>= 0.4.5),
19 qtdeclarative5-ubuntu-telephony0.1,
20 qtdeclarative5-ubuntu-web-plugin,
21 ubuntu-system-settings,
22
23=== modified file 'qml/Stages/PhoneStage.qml'
24--- qml/Stages/PhoneStage.qml 2014-12-01 13:22:12 +0000
25+++ qml/Stages/PhoneStage.qml 2015-01-15 16:14:21 +0000
26@@ -99,6 +99,7 @@
27
28 QtObject {
29 id: priv
30+ objectName: "priv"
31
32 property string focusedAppId: ApplicationManager.focusedApplicationId
33 property var focusedApplication: ApplicationManager.findApplication(focusedAppId)
34@@ -118,6 +119,18 @@
35 return -1;
36 }
37
38+ function surfaceSizer(surface) {
39+ surface.width = root.width;
40+ surface.height = root.height - maximizedAppTopMargin;
41+ return surface;
42+ }
43+
44+ Component.onCompleted: {
45+ ApplicationManager.surfaceAboutToBeCreatedCallback = priv.surfaceSizer;
46+ }
47+ Component.onDestruction: {
48+ ApplicationManager.surfaceAboutToBeCreatedCallback = null;
49+ }
50 }
51
52 Flickable {
53
54=== modified file 'qml/Stages/TabletStage.qml'
55--- qml/Stages/TabletStage.qml 2014-12-01 13:22:12 +0000
56+++ qml/Stages/TabletStage.qml 2015-01-15 16:14:21 +0000
57@@ -110,6 +110,27 @@
58 }
59 return oneWayFlick;
60 }
61+
62+ function surfaceSizer(surface) {
63+ surface.width = root.width;
64+ surface.height = root.height - maximizedAppTopMargin;
65+
66+ if (surface.appId) { // if app side stage, set different width
67+ var app = ApplicationManager.findApplication(surface.appId);
68+ if (app && app.stage === ApplicationInfoInterface.SideStage) {
69+ surface.width = spreadView.sideStageWidth;
70+ }
71+ }
72+
73+ return surface;
74+ }
75+
76+ Component.onCompleted: {
77+ ApplicationManager.surfaceAboutToBeCreatedCallback = priv.surfaceSizer;
78+ }
79+ Component.onDestruction: {
80+ ApplicationManager.surfaceAboutToBeCreatedCallback = null;
81+ }
82 }
83
84 Connections {
85
86=== modified file 'tests/mocks/Unity/Application/ApplicationManager.cpp'
87--- tests/mocks/Unity/Application/ApplicationManager.cpp 2014-12-15 19:20:16 +0000
88+++ tests/mocks/Unity/Application/ApplicationManager.cpp 2015-01-15 16:14:21 +0000
89@@ -51,6 +51,7 @@
90 : ApplicationManagerInterface(parent)
91 , m_suspended(false)
92 , m_forceDashActive(false)
93+ , m_surfaceAboutToBeCreatedCallback(QJSValue::UndefinedValue)
94 {
95 m_roleNames.insert(RoleSession, "session");
96 m_roleNames.insert(RoleFullscreen, "fullscreen");
97@@ -365,6 +366,25 @@
98 Q_EMIT focusedApplicationIdChanged();
99 }
100
101+QJSValue ApplicationManager::surfaceAboutToBeCreatedCallback() const
102+{
103+ return m_surfaceAboutToBeCreatedCallback;
104+}
105+
106+void ApplicationManager::setSurfaceAboutToBeCreatedCallback(const QJSValue &callback)
107+{
108+ if (m_surfaceAboutToBeCreatedCallback.equals(callback))
109+ return;
110+
111+ if (callback.isCallable()) {
112+ m_surfaceAboutToBeCreatedCallback = callback;
113+ } else {
114+ qWarning() << "ApplicationManager::setSurfaceAboutToBeCreatedCallback - attempted to register a non-function!";
115+ m_surfaceAboutToBeCreatedCallback = QJSValue::UndefinedValue;
116+ }
117+ Q_EMIT surfaceAboutToBeCreatedCallbackChanged();
118+}
119+
120 void ApplicationManager::buildListOfAvailableApplications()
121 {
122 ApplicationInfo *application;
123
124=== modified file 'tests/mocks/Unity/Application/ApplicationManager.h'
125--- tests/mocks/Unity/Application/ApplicationManager.h 2014-09-29 09:54:30 +0000
126+++ tests/mocks/Unity/Application/ApplicationManager.h 2015-01-15 16:14:21 +0000
127@@ -105,6 +105,9 @@
128 bool forceDashActive() const override;
129 void setForceDashActive(bool forceDashActive) override;
130
131+ QJSValue surfaceAboutToBeCreatedCallback() const override;
132+ void setSurfaceAboutToBeCreatedCallback(const QJSValue &callback) override;
133+
134 // Only for testing
135 Q_INVOKABLE QStringList availableApplications();
136 Q_INVOKABLE ApplicationInfo* add(QString appId);
137@@ -128,6 +131,7 @@
138 void onWindowCreated();
139 bool m_suspended;
140 bool m_forceDashActive;
141+ QJSValue m_surfaceAboutToBeCreatedCallback;
142 QList<ApplicationInfo*> m_runningApplications;
143 QList<ApplicationInfo*> m_availableApplications;
144 QTimer m_windowCreatedTimer;
145
146=== modified file 'tests/plugins/Unity/Launcher/CMakeLists.txt'
147--- tests/plugins/Unity/Launcher/CMakeLists.txt 2014-10-31 08:49:08 +0000
148+++ tests/plugins/Unity/Launcher/CMakeLists.txt 2015-01-15 16:14:21 +0000
149@@ -39,7 +39,7 @@
150 unity8-private
151 ${GSETTINGS_QT_LDFLAGS}
152 )
153-qt5_use_modules(launchermodeltestExec Test Core DBus Xml Gui)
154+qt5_use_modules(launchermodeltestExec Test Core DBus Xml Gui Qml)
155
156 # copy .desktop files into build directory for shadow builds
157 file(GLOB DESKTOP_FILES *.desktop)
158
159=== modified file 'tests/plugins/Unity/Launcher/launchermodeltest.cpp'
160--- tests/plugins/Unity/Launcher/launchermodeltest.cpp 2014-11-04 12:54:03 +0000
161+++ tests/plugins/Unity/Launcher/launchermodeltest.cpp 2015-01-15 16:14:21 +0000
162@@ -116,6 +116,8 @@
163 void setSuspended(bool) {}
164 bool forceDashActive() const { return false; }
165 void setForceDashActive(bool) {}
166+ QJSValue surfaceAboutToBeCreatedCallback() const override { return QJSValue(false); }
167+ void setSurfaceAboutToBeCreatedCallback(const QJSValue &callback) override { Q_UNUSED(callback); }
168
169 private:
170 QList<MockApp*> m_list;
171
172=== modified file 'tests/qmltests/Stages/tst_PhoneStage.qml'
173--- tests/qmltests/Stages/tst_PhoneStage.qml 2015-01-09 08:31:03 +0000
174+++ tests/qmltests/Stages/tst_PhoneStage.qml 2015-01-15 16:14:21 +0000
175@@ -279,6 +279,15 @@
176 compare(ApplicationManager.focusedApplicationId, selectedApp.appId);
177 }
178
179+ function test_surfaceSizer() {
180+ var surface = { width: 100, height: 200 };
181+ var phoneStagePriv = findInvisibleChild(phoneStage, "priv");
182+
183+ surface = phoneStagePriv.surfaceSizer(surface);
184+ compare(surface.width, phoneStage.width);
185+ compare(surface.height, phoneStage.height - phoneStage.maximizedAppTopMargin);
186+ }
187+
188 function test_orientationChangeSentToFocusedApp() {
189 phoneStage.orientation = Qt.PortraitOrientation;
190 addApps(1);

Subscribers

People subscribed via source and target branches