Closed
Bug 974081
Opened 11 years ago
Closed 11 years ago
Scrolling w/ the SMS App is Choppy and Checkerboards
Categories
(Firefox OS Graveyard :: Gaia::SMS, defect, P1)
Tracking
(Not tracked)
RESOLVED
DUPLICATE
of bug 975221
1.4 S2 (28feb)
People
(Reporter: mchang, Assigned: mchang)
References
Details
(Keywords: perf, Whiteboard: [c=handeye p=4 s= u=1.4])
Attachments
(5 files)
Scrolling with a make reference-workload-light on a hamachi is really slow and choppy with the light reference workload. Find and fix the problems.
Assignee | ||
Comment 1•11 years ago
|
||
Assignee | ||
Comment 2•11 years ago
|
||
Assignee | ||
Comment 3•11 years ago
|
||
Messages overpainting while scrolling.
Comment 4•11 years ago
|
||
Can you play with the inspect and see if you can narrow down what's responsible for the over invalidation? That would certainly explain why some random Rasterize boxes are large.
Comment 5•11 years ago
|
||
I think this happens also in the Call log.
This regressed about 1 week ago, it could make sense to try a bisect.
Assignee | ||
Comment 6•11 years ago
|
||
Just tried with commit 28f9dd73bccfafb62b6f883296ed42f2518e0fd9 from bug 967878, putting a scrollable background. That didn't seem to help.
Comment 7•11 years ago
|
||
I think this is a regression in Gecko, because this worked fine back when I merged bug 967878.
Assignee | ||
Comment 8•11 years ago
|
||
Yup, just tried with gaia master and gecko 166464:c78aa51b8a9c from feb 1st and scrolling is a lot smoother. We checkerboard, but it's much smoother. Bissecting now.
Assignee | ||
Comment 9•11 years ago
|
||
Found this while bisecting:
The first bad revision is:
changeset: 167385:477e7d2eb1d7
user: Timothy Nikkel <tnikkel@gmail.com>
date: Thu Feb 06 16:46:21 2014 -0600
summary: Bug 965593. Only use large z-index on root scroll frames to make overlay scrollbars draw above other content. r=roc
Assignee | ||
Comment 10•11 years ago
|
||
Just tested by backing out 167385:477e7d2eb1d7, scrolling feels a lot smoother. The scroll bar is kind of jumpy, but overall feels better.
Assignee | ||
Comment 11•11 years ago
|
||
Backout bug 965593 due to SMS scrolling regression.
Assignee | ||
Updated•11 years ago
|
Attachment #8378017 -
Attachment description: Backout Bug → Backout Bug 965593
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 12•11 years ago
|
||
What about bug 965593 makes us paint more? Bug 965593 is a 1.3 blocker so we can't simply backout, we'll need to consider our options how to fix bug 965593 differently or how to fix this bug differently, or which bad bug we want to live with.
Comment 13•11 years ago
|
||
Looking at the layer dump it looks like we get a container layer that is not visible but has populated frame metrics. This implies it is a scroll info layer, which probably means we failed to merge all of the scroll layer items that contained the scrolled content.
Comment 14•11 years ago
|
||
It's the background added in bug 967878 that goes below the scrollbars that causes the scrollable layer creation to fail.
My inclination would be to back out bug 965593, and then figure out why the scrolling experience is so bad when we fail to create a scrollable layer in bug 965593. This bug shows that the scrolling experience isn't completely horrible when we fail to build a scrollable layer, so there must be something else making bug 965593 worse.
Comment 15•11 years ago
|
||
(In reply to Timothy Nikkel (:tn) from comment #14)
> This bug shows that the scrolling experience isn't completely
> horrible when we fail to build a scrollable layer, so there must be
> something else making bug 965593 worse.
It might be that at the time we were experiencing bug 965593, we hadn't reduced the paint interval to 40ms and so it looked a lot worse when it fell back to sync scrolling. Bug 965593 will probably be less severe now if the patch for it were to be backed out.
I'm sort of on the fence about this. As I said before I like that we're consistent with the z-index between B2G and OS X desktop but on the other hand maybe we need to fix the layerization stuff properly first. So yeah, if you think it's better to back out bug 965593 then I'm ok with it.
Comment 16•11 years ago
|
||
Does bug 965593 need backing out from v1.3 as well?
Assignee | ||
Comment 17•11 years ago
|
||
This layer tree is very interesting, and I didn't notice it the first time around. We gralloc a lot fewer GrallocTextureHosts w/ bug 965593, and if I'm reading this right, we're only single buffering. Without the patch, we have double buffered thebes layers everywhere (which is what I'm more used to seeing). Could that explain the stuttering?
Assignee | ||
Updated•11 years ago
|
Attachment #8377846 -
Attachment description: Layer Tree while Scrolling → Layer Tree while Scrolling w/ bug 965593
Assignee | ||
Updated•11 years ago
|
Attachment #8378418 -
Attachment description: Layer Tree w/o Patch → Layer Tree w/o bug 965593
Comment 18•11 years ago
|
||
(In reply to Timothy Nikkel (:tn) from comment #14)
> It's the background added in bug 967878 that goes below the scrollbars that
> causes the scrollable layer creation to fail.
>
> My inclination would be to back out bug 965593, and then figure out why the
> scrolling experience is so bad when we fail to create a scrollable layer in
> bug 965593. This bug shows that the scrolling experience isn't completely
> horrible when we fail to build a scrollable layer, so there must be
> something else making bug 965593 worse.
FYI I cooked up a WIP on bug 972666 which solves this problem by inserting a div just inside the overflow:scroll container. The div contains all the scrollable content but also has position:relative;z-index:0 set to force a new stacking context. This has the net effect that all the content inside the overflow:scroll container that uses z-index sits in its own stacking context under the scrollbar, and so there's less chance of sandwiching the scrollbar.
Assignee | ||
Comment 19•11 years ago
|
||
After talking with :kats and :tn, we decided to back out bug 965593 everywhere. A fix to 965593 is too risky for 1.3, so make bug 965593 a 1.4 blocker instead.
Assignee | ||
Comment 20•11 years ago
|
||
Ryan, please backout bug 965593 everywhere. Thanks!
Flags: needinfo?(ryanvm)
Comment 21•11 years ago
|
||
I can do the backout once tn has posted his summary and we get confirmation from release drivers.
Flags: needinfo?(ryanvm) → needinfo?(bugmail.mozilla)
Comment 22•11 years ago
|
||
Clearing needinfo - we are not backing this out.
Flags: needinfo?(bugmail.mozilla)
Keywords: checkin-needed
Comment 23•11 years ago
|
||
Luckily for us our partners noticed this regression too. Tapas filed the same bug and managed to convince Jason that bug 965593 needs to be backed out. Duping this one.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → DUPLICATE
Updated•11 years ago
|
Target Milestone: --- → 1.4 S2 (28feb)
You need to log in
before you can comment on or make changes to this bug.
Description
•