Closed Bug 974081 Opened 6 years ago Closed 6 years ago

Scrolling w/ the SMS App is Choppy and Checkerboards

Categories

(Firefox OS Graveyard :: Gaia::SMS, defect, P1)

ARM
Gonk (Firefox OS)
defect

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.
Attached video messages_painting.mov
Messages overpainting while scrolling.
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.
I think this happens also in the Call log.

This regressed about 1 week ago, it could make sense to try a bisect.
Just tried with commit 28f9dd73bccfafb62b6f883296ed42f2518e0fd9 from bug 967878, putting a scrollable background. That didn't seem to help.
I think this is a regression in Gecko, because this worked fine back when I merged bug 967878.
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.
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
Just tested by backing out 167385:477e7d2eb1d7, scrolling feels a lot smoother. The scroll bar is kind of jumpy, but overall feels better.
Backout bug 965593 due to SMS scrolling regression.
Attachment #8378017 - Attachment description: Backout Bug → Backout Bug 965593
Keywords: checkin-needed
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.
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.
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.
(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.
Does bug 965593 need backing out from v1.3 as well?
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?
Attachment #8377846 - Attachment description: Layer Tree while Scrolling → Layer Tree while Scrolling w/ bug 965593
Attachment #8378418 - Attachment description: Layer Tree w/o Patch → Layer Tree w/o bug 965593
(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.
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.
Ryan, please backout bug 965593 everywhere. Thanks!
Flags: needinfo?(ryanvm)
I can do the backout once tn has posted his summary and we get confirmation from release drivers.
Flags: needinfo?(ryanvm) → needinfo?(bugmail.mozilla)
Clearing needinfo - we are not backing this out.
Flags: needinfo?(bugmail.mozilla)
Keywords: checkin-needed
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: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 975221
Target Milestone: --- → 1.4 S2 (28feb)
You need to log in before you can comment on or make changes to this bug.