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

RESOLVED FIXED

Status

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: BenWa, Assigned: pivanov)

Tracking

({perf})

unspecified
x86
Mac OS X

Firefox Tracking Flags

(blocking-b2g:koi+, b2g-v1.2 fixed)

Details

(Whiteboard: [c=handeye p= s=2013.11.22 u=1.2])

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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.
(Reporter)

Updated

5 years ago
Blocks: 937350
(Reporter)

Comment 1

5 years ago
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.
blocking-b2g: --- → koi?
Flags: needinfo?(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)
Flags: needinfo?(kaze) → needinfo?(pivanov)
Created attachment 833250 [details] [review]
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
Attachment #833250 - Flags: review?(kaze)
Flags: needinfo?(pivanov)
Mike,

Please check if this is perf related.
Flags: needinfo?(mlee)
(Reporter)

Comment 5

5 years ago
This fixes the issue for me. Thanks!
(Reporter)

Comment 6

5 years ago
With this fixed and bug 917416 I have the Settings app scrolling smoothly on a hidpi device.
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.
blocking-b2g: koi? → koi+
Comment on attachment 833250 [details] [review]
patch for Gaia/master

Arthur, Evelyn, could you review this? Thanks!
Attachment #833250 - Flags: review?(kaze)
Attachment #833250 - Flags: review?(ehung)
Attachment #833250 - Flags: review?(arthur.chen)
Assignee: nobody → pivanov
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.
Attachment #833250 - Flags: review?(ehung)
Attachment #833250 - Flags: review?(arthur.chen)
Attachment #833250 - Flags: review+
Thanks Arthur :),

I left a comment on settitngs.css

Landed in master:
https://github.com/mozilla-b2g/gaia/commit/67fcbb16bd1085841c9fceb87fa1d9fb15806420
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Flags: needinfo?(mlee)
Keywords: perf
Whiteboard: [c=handeye p= s=2013.11.22 u=1.2]
Uplifted 67fcbb16bd1085841c9fceb87fa1d9fb15806420 to:
v1.2: 2146bc9bdf57b77878f295a5b6d02e34422e5e45
status-b2g-v1.2: --- → fixed
You need to log in before you can comment on or make changes to this bug.