Panning is jerky in chrome scrollboxes

RESOLVED WONTFIX

Status

defect
P2
normal
RESOLVED WONTFIX
9 years ago
2 months ago

People

(Reporter: mfinkle, Assigned: vingtetun)

Tracking

({relnote})

Firefox Tracking Flags

(fennec-)

Details

Attachments

(1 attachment)

Slowing panning the Perference list or Add-ons Manager results in a jerky motion. I'm not sure it it is scroll indicator related, or if the scrolling is really jumping back and forth because the panning is getting positive and negative scroll increments.
tracking-fennec: --- → ?
You can verify if it is related to scroll indicators by turning ui.scrollbarsCanOverlapContent to 0. If it helps this is probably a Layer story.
tracking-fennec: ? → 2.0+

Comment 2

9 years ago
Can someone get a layer tree dump of what is going on here?
Duplicate of this bug: 625766
Nothing in the layer tree itself looks obviously wrong, but for some reason we're creating a temporary layer manager when painting the preferences during scrolling.  That's bad.  Looking into why.
nsDisplayOpacity::GetLayerState is determining that ChildrenCanBeInactive() during scrolling.  Not sure what UI element(s) this corresponds to, but this definitely seems wrong.  Need to run, will investigate more in a while.
If I Alt-Tab switch away from the prefs area, then switch back and scroll down the smallest amount I can manage, I see

  redrawing thebes(0x1d2b410): <x=0, y=0, w=800, h=500>, 


  redrawing thebes(0x1d2b410): <x=0, y=74, w=800, h=2>, <x=0, y=115, w=800, h=385>, 
  redrawing thebes(0x7f82844595c0): <x=0, y=2, w=800, h=424>, 
  redrawing thebes(0x7f82844591d0): <x=792, y=77, w=6, h=197>, 

The first repaint (first line) is expected and comes from invalidating all content on window hide, which wouldn't happen on device.  The second one (second group of lines) looks suspicious.  It appears that we're repainting pretty much the entire prefs window, i.e. not using our scrolling optimizations.

The layer tree here is approximately (got it from another run) the following

-1795057664[1457ab0]: BasicLayerManager (0x7f117cb4cb20)
-1795057664[1457ab0]:   BasicContainerLayer (0x19528b0) [visible=< (x=0, y=0, w=800, h=500); >] [opaqueContent] [metrics={ viewport=(x=0, y=0, w=800, h=500) viewportScroll=(x=0, y=0) displayport=(x=0, y=0, w=0, h=0) scrollId=0 }]
-1795057664[1457ab0]:     BasicThebesLayer (0x1972b80) [visible=< (x=0, y=0, w=800, h=16); (x=0, y=16, w=800, h=60); (x=0, y=116, w=800, h=384); >] [opaqueContent] [valid=< (x=0, y=0, w=800, h=16); (x=0, y=16, w=800, h=60); (x=0, y=116, w=800, h=384); >]
-1795057664[1457ab0]:     BasicThebesLayer (0x7f117cdde0e0) [transform=[ 1 0; 0 1; 0 75; ]] [visible=< (x=0, y=1, w=800, h=424); >] [valid=< (x=0, y=1, w=800, h=424); >]
-1795057664[1457ab0]:     BasicThebesLayer (0x7f117c471280) [visible=< (x=792, y=77, w=6, h=197); >] [valid=< (x=792, y=77, w=6, h=197); >]
-1795057664[1457ab0]:     BasicThebesLayer (0x7f117c728030) [transform=[ 1 0; 0 1; 0 75; ]] [visible=< (x=0, y=1, w=800, h=275); (x=8, y=318, w=766, h=31); (x=0, y=374, w=800, h=32); (x=8, y=424, w=661, h=1); >] [componentAlpha]

I'm still not absolutely sure which layers correspond with each UI element, but I have guesses now.

I'm going to try a quick head-in-the-sand-esque hack, letting nsDisplayOpacity get their own layers, to see if that makes this go away.

mfinkle/stechz/mbrubeck, it would help in investigating further if one of you guys could briefly summarize how the preferences area is scrolled by the frontend.
(In reply to comment #5)
> nsDisplayOpacity::GetLayerState is determining that ChildrenCanBeInactive()
> during scrolling.  Not sure what UI element(s) this corresponds to, but this
> definitely seems wrong.  Need to run, will investigate more in a while.

That doesn't seem wrong on the face of it to me (that would be the normal case in my mind, unless the opacity-ness is being animated), but it could be wrong in some situations.
What are these scroll indicators? A screenshot would probably be helpful.
(In reply to comment #7)
> (In reply to comment #5)
> > nsDisplayOpacity::GetLayerState is determining that ChildrenCanBeInactive()
> > during scrolling.  Not sure what UI element(s) this corresponds to, but this
> > definitely seems wrong.  Need to run, will investigate more in a while.
> 
> That doesn't seem wrong on the face of it to me (that would be the normal case
> in my mind, unless the opacity-ness is being animated), but it could be wrong
> in some situations.

I don't think this is what we want during scrolling because it requires an unnecessary (AFAICT, AIUI) compositing operation: render the DisplayOpacity content, composite to parent thebes layer, draw to target.  With basic layers, we pay an extra memcpy of that region of the parent layer when we could otherwise have just composited directly to the target.  With OGL layers, we additionally pay for compositing on the CPU instead of the GPU.  (Again, IIUC.)  This does look like a useful optimization for basic layers that are in a steady, not-scrolling state since we avoid recompositing on subsequent repaints.

But I don't think this is the real problem here; the problem appears to be that we're not taking the scrolling fast path of rotating a thebes buffer and rendering the leftover area.
Posted image Scroll indicators
The scroll indicators are the 

-1795057664[1457ab0]:     BasicThebesLayer (0x7f117c471280) [visible=< (x=792,
y=77, w=6, h=197); >] [valid=< (x=792, y=77, w=6, h=197); >]

layer in the dump above.
Er, in the screenshot above, there's only a scroll *indicator* (singular), and it's the skinny translucent dark gray thing on the far right.
In the temp layer manager case we'd only have to do the slow composite once, because the result is retained in the parent layer. In the active layer case we have to do the (fast) composite of the extra layers for every frame of scrolling. For accelerated layers these composites are fast, but for basic they are the same speed, or am I thinking about this wrong?
(FTR, probably more of a side issue, but ...)

Hmm no, you're right for basic layers, I'm wrong.  Either way we have to render the entire DisplayOpacity subtree to a temp surface, then composite that to a target.  Compositing to the parent layer should be as fast as compositing directly to the target.  I was wrong in missing that if we were to composite directly to the target, we still would have had to copy the entire parent layer surface to the target first; i.e. we couldn't have skipped any pixels in the parent covered by the DisplayOpacity subtree.  Thanks for the correction.

That's likely not optimal for GPU-composited layers as you mention, but again that doesn't appear to be the problem here.  It might be interesting to look at changing that heuristic in a separate bug though.
Yeah, a side issue, I didn't mean to hijack things.
(In reply to comment #6)

> mfinkle/stechz/mbrubeck, it would help in investigating further if one of you
> guys could briefly summarize how the preferences area is scrolled by the
> frontend.

In case you still need it: We handle all mouse input in input.js and then delegate events/actions to various different chunks of code. For chrome scrollboxes, we use a default dragger, found here:
http://mxr.mozilla.org/mobile-browser/source/chrome/content/input.js#584

The _defaultDragger._showScrollbars method handles flipping the attribute used to get CSS to turn the scroll indicators on and off.
(In reply to comment #4)
> Nothing in the layer tree itself looks obviously wrong, but for some reason
> we're creating a temporary layer manager when painting the preferences during
> scrolling.  That's bad.  Looking into why.

I've not ideas of the reason, but could that be relate to the fact those scrollboxes do not use the usual path for scrollbox because we want scrollbars to appear above the content: http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsGfxScrollFrame.cpp#1822 ?

This special path can be disabled by setting the pref ui.scrollbarsCanOverlapContent to 0
Duplicate of this bug: 626499
Assignee: nobody → 21
Duplicate of this bug: 627087
bug 627087 has a video of some of the problems
Keywords: relnote
tracking-fennec: 2.0+ → 2.0-
Priority: -- → P2
Bug 604533 would have made panning chrome worse and bug 635035 should have won all that back.  Is this better after 635035?
I'm working on related issues in bug 657893.

Comment 23

2 months ago
Closing all opened bug in a graveyard component
Status: NEW → RESOLVED
Closed: 2 months ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.