938785 - [Settings] App background doesn't scroll with content forcing a fullscreen layer

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Settings, defect)

Description

[Benoit Girard (:BenWa)]

STR:
1) Turn on FPS counter. The 3rd counter reading is overdraw (the rightmost one). It should be sitting at about 200ish. However when we start scrolling we jump to 280-300. This is because the text is scrolling but not the background.

We need to move the background image inside the scrolling elements. This should prevent the creation of the extra layer improving performance.

This should be a low effort, high reward bug for scrolling perf.

Comment 1

[Benoit Girard (:BenWa)]

Fabien do you think someone could look at this? Let me know if you need more information.

Fixing this should increase FPS, lower memory and decrease the latency of starting a scroll.

Comment 2

[Fabien Cazenave [:kaze]]

The “low effort / high reward” part teases me but I’m quite busy with COMMS-related tasks at the moment.

Pavel, can you have a look at that? (please NI’ me if you can’t)

Comment 3

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Attachment: patch for Gaia/master

Hey Kaze,
I keep line 28 in "settings.css" and add the same background on line 11 in "list.css" because we need the background pattern when the app content is less then viewport :) and the result is 196

Comment 4

[Preeti Raghunath(:Preeti)]

Mike,

Please check if this is perf related.

Comment 5

[Benoit Girard (:BenWa)]

This fixes the issue for me. Thanks!

Comment 6

[Benoit Girard (:BenWa)]

With this fixed and bug 917416 I have the Settings app scrolling smoothly on a hidpi device.

Comment 7

[Andrew Overholt [:overholt]]

This bug improves scrolling.  In comment #6 Benoit alluded to bug 917416 which reduces memory usage but has a non-zero regression risk and poor test coverage so let's not uplift that.

Comment 8

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 833250 [details] [review]
patch for Gaia/master

Arthur, Evelyn, could you review this? Thanks!

Comment 9

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 833250 [details] [review]
patch for Gaia/master

Looks good to me. As we still need to keep the background for section elements, I would suggest to leave a brief description on this bug in list.css so that the tweak won't be seen as a redundant style and be removed by mistake.

Comment 10

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Thanks Arthur :),

I left a comment on settitngs.css

Landed in master:
https://github.com/mozilla-b2g/gaia/commit/67fcbb16bd1085841c9fceb87fa1d9fb15806420

Comment 11

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted 67fcbb16bd1085841c9fceb87fa1d9fb15806420 to:
v1.2: 2146bc9bdf57b77878f295a5b6d02e34422e5e45