Merge lp:~3v1n0/gnome-shell/bionic-patches-picks-reorder into lp:~ubuntu-desktop/gnome-shell/ubuntu

Proposed by Marco Trevisan (Treviño)
Status: Merged
Merged at revision: 145
Proposed branch: lp:~3v1n0/gnome-shell/bionic-patches-picks-reorder
Merge into: lp:~ubuntu-desktop/gnome-shell/ubuntu
Diff against target: 243 lines (+168/-25)
4 files modified
debian/changelog (+3/-0)
debian/patches/js-ui-Choose-some-actors-to-cache-on-the-GPU.patch (+133/-0)
debian/patches/series (+6/-3)
debian/patches/ui-Theme-lookup-should-respect-XDG_DATA_DIRS.patch (+26/-22)
To merge this branch: bzr merge lp:~3v1n0/gnome-shell/bionic-patches-picks-reorder
Reviewer Review Type Date Requested Status
Jeremy Bícha Pending
Review via email: mp+343479@code.launchpad.net

Commit message

debian/patches/series: reorder patches, reimport xdg dir patch add GPU caching

Description of the change

Ideally I wanted to put them as first in the list, but then some don't apply anymore, so let's stick to this.

To post a comment you must log in.
146. By Marco Trevisan (Treviño)

debian/patches/ui-Theme-lookup-should-respect-XDG_DATA_DIRS.patch: actually cherry-pick from upstream

147. By Marco Trevisan (Treviño)

debian/patches/js-ui-Choose-some-actors-to-cache-on-the-GPU.patch: include duflu change

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

In the previous version I didn't properly cherry-picked the XDG dirs support patch

Plus added MP from Daniel

Revision history for this message
Jeremy Bícha (jbicha) wrote :

Sorry, it's late to be adding yet more patches to our patch queue.

The upstream comments on https://bugzilla.gnome.org/792633 were skeptical that this was the right thing to do in gnome-shell master itself. Therefore, I merged the other changes but not the js-ui-Choose-some-actors-to-cache-on-the-GPU.patch and uploaded to bionic now.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2018-04-17 18:21:56 +0000
3+++ debian/changelog 2018-04-18 07:28:06 +0000
4@@ -30,6 +30,9 @@
5 - Fix possible crash on cache loading
6 * st-texture-cache-Don-t-add-NULL-textures-to-cache.patch:
7 - Fix crash when deleting NULL textures from cache (LP: #1754445)
8+ * js-ui-Choose-some-actors-to-cache-on-the-GPU.patch:
9+ - Improve rendering of shell elements moving rendering to GPU
10+ (LP: #1744001)
11
12 -- Marco Trevisan (Treviño) <marco@ubuntu.com> Tue, 17 Apr 2018 14:40:17 -0400
13
14
15=== added file 'debian/patches/js-ui-Choose-some-actors-to-cache-on-the-GPU.patch'
16--- debian/patches/js-ui-Choose-some-actors-to-cache-on-the-GPU.patch 1970-01-01 00:00:00 +0000
17+++ debian/patches/js-ui-Choose-some-actors-to-cache-on-the-GPU.patch 2018-04-18 07:28:06 +0000
18@@ -0,0 +1,133 @@
19+From: Daniel van Vugt <daniel.van.vugt@canonical.com>
20+Date: Fri, 6 Apr 2018 05:26:58 -0500
21+Subject: js/ui: Choose some actors to cache on the GPU
22+
23+Adds a wrapper function to flag actors that are good candidates for
24+caching in texture memory (what Clutter calls "offscreen redirect"),
25+thereby mostly eliminating their repaint overhead.
26+
27+This isn't exactly groundbreaking, it's how you're meant to use
28+OpenGL in the first place. But the difficulty is in the design of
29+Clutter which has some peculiarities making universal caching
30+inefficient at the moment:
31+
32+ * Repainting an offscreen actor is measurably slower than repainting
33+ the same actor if it was uncached. But only by less than 100%,
34+ so if an actor can avoid changing every frame then caching is usually
35+ more efficient over that timeframe.
36+
37+ * The cached painting from a container typically includes its children,
38+ so you can't cache containers whose children are usually animating at
39+ full frame rate. That results in a performance loss.
40+ This could be remedied in future by Clutter explicitly separating a
41+ container's background painting from its child painting and always
42+ caching the background (as StWidget tries to in some cases already).
43+
44+So this commit selects just a few areas where caching has been verified
45+to be beneficial, and many use cases now see their CPU usage halved:
46+
47+One small window active...... 10% -> 7% (-30%)
48+...under a panel menu........ 23% -> 9% (-61%)
49+One maximized window active.. 12% -> 9% (-25%)
50+...under a panel menu........ 23% -> 11% (-52%)
51+...under a shell dialog...... 22% -> 12% (-45%)
52+...in activities overview.... 32% -> 17% (-47%)
53+(on an i7-7700)
54+
55+Also a couple of bugs are fixed by this:
56+
57+https://bugzilla.gnome.org/show_bug.cgi?id=792634
58+https://bugzilla.gnome.org/show_bug.cgi?id=792633
59+
60+Bug-GNOME: https://bugzilla.gnome.org/show_bug.cgi?id=792634
61+Bug-GNOME: https://bugzilla.gnome.org/show_bug.cgi?id=792633
62+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/gnome-shell/+bug/1744001
63+Forwarded: yes, https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/73
64+---
65+ js/ui/boxpointer.js | 1 +
66+ js/ui/dash.js | 1 +
67+ js/ui/dialog.js | 2 ++
68+ js/ui/main.js | 16 ++++++++++++++++
69+ js/ui/panel.js | 1 +
70+ 5 files changed, 21 insertions(+)
71+
72+diff --git a/js/ui/boxpointer.js b/js/ui/boxpointer.js
73+index 47f718a..602646a 100644
74+--- a/js/ui/boxpointer.js
75++++ b/js/ui/boxpointer.js
76+@@ -44,6 +44,7 @@ var BoxPointer = new Lang.Class({
77+ y_fill: true });
78+ this._container = new Shell.GenericContainer();
79+ this.actor.set_child(this._container);
80++ Main.hintActorSeldomChanges(this.actor);
81+ this._container.connect('get-preferred-width', this._getPreferredWidth.bind(this));
82+ this._container.connect('get-preferred-height', this._getPreferredHeight.bind(this));
83+ this._container.connect('allocate', this._allocate.bind(this));
84+diff --git a/js/ui/dash.js b/js/ui/dash.js
85+index 5ee2476..14864f1 100644
86+--- a/js/ui/dash.js
87++++ b/js/ui/dash.js
88+@@ -402,6 +402,7 @@ var Dash = new Lang.Class({
89+ clip_to_allocation: true });
90+ this._box._delegate = this;
91+ this._container.add_actor(this._box);
92++ Main.hintActorSeldomChanges(this._container);
93+
94+ this._showAppsIcon = new ShowAppsIcon();
95+ this._showAppsIcon.childScale = 1;
96+diff --git a/js/ui/dialog.js b/js/ui/dialog.js
97+index cfa192d..89db963 100644
98+--- a/js/ui/dialog.js
99++++ b/js/ui/dialog.js
100+@@ -6,6 +6,7 @@ const GObject = imports.gi.GObject;
101+ const Pango = imports.gi.Pango;
102+ const St = imports.gi.St;
103+ const Lang = imports.lang;
104++const Main = imports.ui.main;
105+
106+ var Dialog = new Lang.Class({
107+ Name: 'Dialog',
108+@@ -40,6 +41,7 @@ var Dialog = new Lang.Class({
109+ // mode accordingly so wrapped labels are handled correctly during
110+ // size requests.
111+ this._dialog.request_mode = Clutter.RequestMode.HEIGHT_FOR_WIDTH;
112++ Main.hintActorSeldomChanges(this._dialog);
113+
114+ this.contentLayout = new St.BoxLayout({ vertical: true,
115+ style_class: "modal-dialog-content-box" });
116+diff --git a/js/ui/main.js b/js/ui/main.js
117+index d86cf9e..5277cf7 100644
118+--- a/js/ui/main.js
119++++ b/js/ui/main.js
120+@@ -716,3 +716,19 @@ function showRestartMessage(message) {
121+ let restartMessage = new RestartMessage(message);
122+ restartMessage.open();
123+ }
124++
125++/**
126++ * hintActorSeldomChanges:
127++ * @actor: A clutter actor
128++ *
129++ * Flag an actor as known to not need repainting very often. Such actors can
130++ * have their painting cached in GPU memory so that future repaints triggered
131++ * by other actors don't incur a repaint of @actor. This can provide dramatic
132++ * performance benefits if used wisely.
133++ *
134++ * This hint needs to be provided manually because clutter presently lacks
135++ * a good enough heuristic to toggle the optimization automatically.
136++ */
137++function hintActorSeldomChanges(actor) {
138++ actor.set_offscreen_redirect(Clutter.OffscreenRedirect.ALWAYS);
139++}
140+diff --git a/js/ui/panel.js b/js/ui/panel.js
141+index 2f59324..2237ead 100644
142+--- a/js/ui/panel.js
143++++ b/js/ui/panel.js
144+@@ -772,6 +772,7 @@ var Panel = new Lang.Class({
145+ this.actor = new Shell.GenericContainer({ name: 'panel',
146+ reactive: true });
147+ this.actor._delegate = this;
148++ Main.hintActorSeldomChanges(this.actor);
149+
150+ this._sessionStyle = null;
151+
152
153=== modified file 'debian/patches/series'
154--- debian/patches/series 2018-04-17 10:25:59 +0000
155+++ debian/patches/series 2018-04-18 07:28:06 +0000
156@@ -11,16 +11,19 @@
157 ubuntu_background_login.patch
158 ubuntu_gdm_alternatives.patch
159 ubuntu_block_mode_extension_update.patch
160+# Cherry picks from upstream
161 ui-Theme-lookup-should-respect-XDG_DATA_DIRS.patch
162-workaround_crasher_fractional_scaling.patch
163-shell-ignore-invalid-window-monitor-index.patch
164 workspaceThumbnail-only-update-_porthole-if-the-overview-.patch
165 workspaceThumbnail-rebuild-thumbnails-if-workareas-size-c.patch
166 workspaceThumbnail-initialize-porthole-based-on-workArea.patch
167 popupMenu-Fix-wrong-call-to-clutter_actor_add_child.patch
168+volume-Add-back-sound-feedback-on-scroll.patch
169+# End of Cherry-picked patches
170+workaround_crasher_fractional_scaling.patch
171+shell-ignore-invalid-window-monitor-index.patch
172 StIcon-only-compute-shadow-pipeline-when-the-texture-is-p.patch
173-volume-Add-back-sound-feedback-on-scroll.patch
174 js-fix-invalid-access-errors.patch
175 workspace-fix-repositioned-windows-in-activities.patch
176 st-texture-cache-Cancel-sliced-image-loading-on-target-ac.patch
177 st-texture-cache-Don-t-add-NULL-textures-to-cache.patch
178+js-ui-Choose-some-actors-to-cache-on-the-GPU.patch
179
180=== modified file 'debian/patches/ui-Theme-lookup-should-respect-XDG_DATA_DIRS.patch'
181--- debian/patches/ui-Theme-lookup-should-respect-XDG_DATA_DIRS.patch 2018-04-17 18:09:32 +0000
182+++ debian/patches/ui-Theme-lookup-should-respect-XDG_DATA_DIRS.patch 2018-04-18 07:28:06 +0000
183@@ -1,34 +1,38 @@
184-Description: ui: Theme lookup should respect XDG_DATA_DIRS
185- mods, extensions and others GNOME Shell assets lookup into each dir, relative
186- to XDG_DATA_DIRS. However, this isn't the case for themes (which can be
187- referenced by a mod in a different XDG_DATA_DIR), hardcoding global.datadir.
188- The fix is to have the theme finding pattern following the same logic than
189- other elements.
190-Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/gnome-shell/+bug/1760039
191-Forwarded: https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/70
192-Author: Didier Roche <didrocks@ubuntu.com>
193+From: Didier Roche <didrocks@ubuntu.com>
194+Date: Fri, 30 Mar 2018 01:44:14 -0600
195+Subject: ui: Theme lookup should respect XDG_DATA_DIRS
196+
197+Modes, extensions and other GNOME Shell assets are searched in appropriate
198+subdirectories of each directory in XDG_DATA_DIRS, falling back
199+to global.datadir.
200+However, this isn't the case for themes, which are currently always expected
201+in global.datadir, even when referenced by a mode in a different XDG_DATA_DIR.
202+
203+The fix is to have the theme finding pattern follow the same logic as other
204+elements.
205+Fixes #167.
206+
207+Origin: https://gitlab.gnome.org/GNOME/gnome-shell/commit/d6d09fd
208+---
209+ js/ui/main.js | 8 ++++++++
210+ 1 file changed, 8 insertions(+)
211+
212 diff --git a/js/ui/main.js b/js/ui/main.js
213-index 7c1214e0a..99edd28d7 100644
214+index 2cfe941..d86cf9e 100644
215 --- a/js/ui/main.js
216 +++ b/js/ui/main.js
217-@@ -254,9 +254,14 @@ function _getStylesheet(name) {
218+@@ -256,6 +256,14 @@ function _getStylesheet(name) {
219 if (stylesheet.query_exists(null))
220 return stylesheet;
221
222-- stylesheet = Gio.File.new_for_path(global.datadir + '/theme/' + name);
223-- if (stylesheet.query_exists(null))
224-- return stylesheet;
225 + let dataDirs = GLib.get_system_data_dirs();
226 + for (let i = 0; i < dataDirs.length; i++) {
227 + let path = GLib.build_filenamev([dataDirs[i], 'gnome-shell', 'theme', name]);
228 + let stylesheet = Gio.file_new_for_path(path);
229-+ if (stylesheet.query_exists(null)) {
230++ if (stylesheet.query_exists(null))
231 + return stylesheet;
232-+ }
233 + }
234-
235- return null;
236- }
237---
238-2.15.1
239-
240++
241+ stylesheet = Gio.File.new_for_path(global.datadir + '/theme/' + name);
242+ if (stylesheet.query_exists(null))
243+ return stylesheet;

Subscribers

People subscribed via source and target branches