452048 - Scrolling still slow when the scrolled element is partially offscreen

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Arpad Borsos [:Swatinem]]

Bug 447739 should have fixed the problem in the first place, however, the fix does not work on my system. I still see a big gab between the performance of the fully visible iframe (20991ms) vs the partially offscreen one (116863ms)
Some other observation: Only ~5 pixels of the top of the iframe are offscreen, so 99% of the iframe are visible.
When I resize the window to be much smaller (the visible area of the iframe is only ~20 pixels high), I get 11051ms
I get to 20s when only 1/5th of the iframe is visible.

System information:
Firefox compiled from source, rev d48d40cf07e5 + some custom patches
--enable-optimize --disable-debug
Linux x86_64 with proprietary nvidia drivers (Ubuntu 8.04 with recent updates from hardy-proposed)

I will try do digg into profiling, but I've never done that before.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

What we really need here is to find out which area(s) of the screen get repainted at every scroll step. That is going to show us the real problem.

It would also be helpful to see if other people can see the problem on Mac or Windows. I'm pretty sure this is not a problem on Mac.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

In a debug build, you can set gDumpRepaintRegionForCopy=1 in gdb and get useful output.

Comment 3

[Arpad Borsos [:Swatinem]]

On Vista, I get 18600ms vs. 27295ms
It does run slower, but not as slow as on Linux. I'm just compiling a debug build on Linux and will try with gDumpRepaintRegionForCopy=1 shortly.

Comment 4

[Arpad Borsos [:Swatinem]]

Finally got it to work.
This is with the iframe completely visible:

Adding rect 0,0,0,0 for frame 0x7fb40e8ef838
Adding rect 0,0,0,0 for frame 0x7fb40e8ef838
Repaint region for copy --- before optimization (area 60,60,50100,36000, frame 0x7fb40e727b58):
Clip (nil)() (0,0,59640,49200)
    CanvasBackground 0x7fb40e8ef838(Canvas(html)(-1)) (0,0,59640,49200) uniform
Clip (nil)() (0,0,59640,49200)
    Border 0x7fb40e4db7a8(HTMLScroll(div)(1)) (0,0,51120,36120)
    Clip (nil)() (60,60,50100,36000)
        Summary 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-58440,50100,120000) moving
    Clip 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,60,50100,36000) moving
        Summary 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-58440,50100,120000) moving
    Clip (nil)() (60,60,50100,36000)
        Summary 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-58440,50100,120000) moving
        Summary 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-58440,50100,120000) moving
    Clip 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,60,50100,36000) moving
        Summary 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-58440,50100,120000) moving
Clip 0x7fb40e452020(SVGOuterSVG(svg)(1)) (0,0,59640,49200) moving
    Clip 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,60,50100,36000) moving
        Summary 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-58440,50100,120000) moving
Repaint region for copy --- after optimization:
Clip 0x7fb40e452020(SVGOuterSVG(svg)(1)) (0,0,59640,49200) moving
    CanvasBackground 0x7fb40e8ef838(Canvas(html)(-1)) (0,0,59640,49200) uniform
    Clip 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,60,50100,36000) moving
        Summary 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-58440,50100,120000) moving
        Summary 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-58440,50100,120000) moving
        Summary 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-58440,50100,120000) moving
        Summary 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-58440,50100,120000) moving
        Summary 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-58440,50100,120000) moving
        Summary 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-58440,50100,120000) moving

and with the iframe partially offscreen:

Adding rect 0,0,0,0 for frame 0x7fb40e8ef838
Adding rect 60,-240,50100,240 for frame 0x7fb40e8ef838
Repaint region for copy --- before optimization (area 60,-240,50100,36000, frame 0x7fb40e727b58):
Clip (nil)() (0,0,58740,37140)
    CanvasBackground 0x7fb40e8ef838(Canvas(html)(-1)) (0,-300,58740,37440) uniform
Clip (nil)() (0,0,58740,37140)
    Border 0x7fb40e4db7a8(HTMLScroll(div)(1)) (0,-300,51120,36120)
    Clip (nil)() (60,-240,50100,36000)
        Summary 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-58740,50100,120000) moving
    Clip 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-240,50100,36000) moving
        Summary 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-58740,50100,120000) moving
    Clip (nil)() (60,-240,50100,36000)
        Summary 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-58740,50100,120000) moving
        Summary 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-58740,50100,120000) moving
    Clip 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-240,50100,36000) moving
        Summary 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-58740,50100,120000) moving
Clip 0x7fb40e452020(SVGOuterSVG(svg)(1)) (0,0,58740,37140) moving
    Clip 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-240,50100,36000) moving
        Summary 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-58740,50100,120000) moving
Repaint region for copy --- after optimization:
Clip 0x7fb40e452020(SVGOuterSVG(svg)(1)) (0,0,58740,37140) moving
    CanvasBackground 0x7fb40e8ef838(Canvas(html)(-1)) (0,-300,58740,37440) uniform
    Clip 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-240,50100,36000) moving
        Summary 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-58740,50100,120000) moving
        Summary 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-58740,50100,120000) moving
        Summary 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-58740,50100,120000) moving
        Summary 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-58740,50100,120000) moving
        Summary 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-58740,50100,120000) moving
        Summary 0x7fb40e452020(SVGOuterSVG(svg)(1)) (60,-58740,50100,120000) moving

I hope this helps, took me ages to get debugging working.

Comment 5

[Arpad Borsos [:Swatinem]]

Sorry for yet another comment:
The "Adding Rect" bits are actually below the output, not above. I made a mistake when copying it here.

So what I see is that it adds 2 empty rects when the iframe is fully visible, but one empty and one very large one when the iframe is partially offscreen. Could that be the problem?

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

That's definitely the problem. I wonder if something strange is happening due to 64-bit. Are you able to try a 32-bit build to see if it has the same problem?

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Alternatively, next time Karl's in the office I can try it on his machine.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(I just rechecked on my Mac and it's definitely fixed there.)

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Oh, was your Vista test 32-bit or 64?

Comment 10

[Arpad Borsos [:Swatinem]]

Looks like the ia32-libs package on hardy doesn't provide a 32bit version of libdbus-glib-1.so.2 anymore, so I can't test a 32bit nightly :(
My Vista System was 32bit though.

Comment 11

[Karl Tomlinson (:karlt)]

On Gentoo Linux:
debug 64-bit:     fully visible: 44.7, partially offscreen: 195.6
optimized 32-bit: fully visible: 29.3, partially offscreen: 122.3

Comment 12

[Arpad Borsos [:Swatinem]]

I've played around with gdb a bit:
fully visible iframe:

Breakpoint 1, AccumulateItemInRegion (aRegion=0x7fff396477d0, 
    aAreaRect=@0x7fff39646e60, aItemRect=@0x7fff39646ea0, 
    aExclude=@0x7fff39646e00, aItem=0x7f01165da020)
(gdb) p *aRegion
$3 = {mRectCount = 0, mCurRect = 0x7fff396477e0, 
  mRectListHead = {<nsRegion::nsRectFast> = {<nsRect> = {x = 0, y = 0, 
        width = 0, height = 0}, <No data fields>}, prev = 0x7fff396477e0, 
    next = 0x7fff396477e0}, mBoundRect = {<nsRect> = {x = 0, y = 0, width = 0, 
      height = 0}, <No data fields>}}
(gdb) p *aRegion->mCurRect
$4 = {<nsRegion::nsRectFast> = {<nsRect> = {x = 0, y = 0, width = 0, 
      height = 0}, <No data fields>}, prev = 0x7fff396477e0, 
  next = 0x7fff396477e0}
(gdb) p aAreaRect
$5 = (const nsRect &) @0x7fff39646e60: {x = 60, y = 60, width = 50100, 
  height = 34860}
(gdb) p aItemRect
$6 = (const nsRect &) @0x7fff39646ea0: {x = 60, y = 60, width = 50100, 
  height = 36000}
(gdb) p aExclude
$7 = (const nsRect &) @0x7fff39646e00: {x = 60, y = 60, width = 50100, 
  height = 34860}
(gdb) p *aItem->mFrame
$12 = {<nsISupports> = {_vptr.nsISupports = 0x7f01205c5a10}, mRect = {x = 0, 
    y = 0, width = 58740, height = 37440}, mContent = 0x7f011634bb40, 
  mStyleContext = 0x7f0117a75120, mParent = 0x7f011b37aa68, 
  mNextSibling = 0x0, mState = 270336}
(gdb) p *aItem->mAbove->mFrame
$11 = {<nsISupports> = {_vptr.nsISupports = 0x7f0120661150}, mRect = {x = 0, 
    y = 0, width = 50100, height = 120000}, mContent = 0x7f0118c8b080, 
  mStyleContext = 0x7f0117a77e68, mParent = 0x7f0117a77b58, 
  mNextSibling = 0x7f01165d0c58, mState = 1056784}
(gdb) p damageRect
$15 = {x = 60, y = 60, width = 50100, height = 34860}
(gdb) p r
$16 = {mRectCount = 0, mCurRect = 0x7fff39646cc0, 
  mRectListHead = {<nsRegion::nsRectFast> = {<nsRect> = {x = 0, y = 0, 
        width = 0, height = 0}, <No data fields>}, prev = 0x7fff39646cc0, 
    next = 0x7fff39646cc0}, mBoundRect = {<nsRect> = {x = 0, y = 0, width = 0, 
      height = 0}, <No data fields>}}
(gdb) step
Adding rect 0,0,0,0 for frame 0x7f011b37a838
(gdb) p *aRegion
$17 = {mRectCount = 0, mCurRect = 0x7fff396477e0, 
  mRectListHead = {<nsRegion::nsRectFast> = {<nsRect> = {x = 0, y = 0, 
        width = 0, height = 0}, <No data fields>}, prev = 0x7fff396477e0, 
    next = 0x7fff396477e0}, mBoundRect = {<nsRect> = {x = 0, y = 0, width = 0, 
      height = 0}, <No data fields>}}
(gdb) p *aRegion->mCurRect
$18 = {<nsRegion::nsRectFast> = {<nsRect> = {x = 0, y = 0, width = 0, 
      height = 0}, <No data fields>}, prev = 0x7fff396477e0, 
  next = 0x7fff396477e0}

==============================================================================
And now the partially offscreen one:

Breakpoint 1, AccumulateItemInRegion (aRegion=0x7fff396477d0, 
    aAreaRect=@0x7fff39646e50, aItemRect=@0x7fff39646e40, 
    aExclude=@0x7fff39646e00, aItem=0x7f012866c820)
(gdb) p *aRegion
$19 = {mRectCount = 1, mCurRect = 0x7f0123f0f208, 
  mRectListHead = {<nsRegion::nsRectFast> = {<nsRect> = {x = 0, y = 0, 
        width = 0, height = 0}, <No data fields>}, prev = 0x7f0123f0f208, 
    next = 0x7f0123f0f208}, mBoundRect = {<nsRect> = {x = 60, y = -240, 
      width = 50100, height = 240}, <No data fields>}}
(gdb) p *aRegion->mCurRect
$20 = {<nsRegion::nsRectFast> = {<nsRect> = {x = 60, y = -240, width = 50100, 
      height = 240}, <No data fields>}, prev = 0x7fff396477e0, 
  next = 0x7fff396477e0}
(gdb) p aAreaRect
$21 = (const nsRect &) @0x7fff39646e50: {x = 60, y = -240, width = 50100, 
  height = 34860}
(gdb) p aItemRect
$22 = (const nsRect &) @0x7fff39646e40: {x = 60, y = -1140, width = 50100, 
  height = 35760}
(gdb) p aExclude
$23 = (const nsRect &) @0x7fff39646e00: {x = 60, y = 0, width = 50100, 
  height = 34620}
(gdb) p *aItem->mFrame
$26 = {<nsISupports> = {_vptr.nsISupports = 0x7f01205c5a10}, mRect = {x = 0, 
    y = -300, width = 58740, height = 37440}, mContent = 0x7f011634bb40, 
  mStyleContext = 0x7f0117a73d88, mParent = 0x7f011b37aa68, 
  mNextSibling = 0x0, mState = 270336}
(gdb) p *aItem->mAbove->mFrame
$27 = {<nsISupports> = {_vptr.nsISupports = 0x7f0120661150}, mRect = {x = 0, 
    y = 0, width = 50100, height = 120000}, mContent = 0x7f0118c8b080, 
  mStyleContext = 0x7f0117a770b8, mParent = 0x7f0117a77b58, 
  mNextSibling = 0x7f01165d0c58, mState = 1056784}
[...]
Adding rect 60,-240,50100,240 for frame 0x7f011b37a838
(gdb) p damageRect
$29 = {x = 60, y = -240, width = 50100, height = 34860}
(gdb) p r
$30 = {mRectCount = 1, mCurRect = 0x7f0123f0f208, 
  mRectListHead = {<nsRegion::nsRectFast> = {<nsRect> = {x = 0, y = 0, 
        width = 0, height = 0}, <No data fields>}, prev = 0x7f0123f0f208, 
    next = 0x7f0123f0f208}, mBoundRect = {<nsRect> = {x = 60, y = -240, 
      width = 50100, height = 240}, <No data fields>}}
(gdb) p *aRegion
$31 = {mRectCount = 1, mCurRect = 0x7f0123f0f0a8, 
  mRectListHead = {<nsRegion::nsRectFast> = {<nsRect> = {x = 0, y = 0, 
        width = 0, height = 0}, <No data fields>}, prev = 0x7f0123f0f0a8, 
    next = 0x7f0123f0f0a8}, mBoundRect = {<nsRect> = {x = 60, y = -240, 
      width = 50100, height = 240}, <No data fields>}}
(gdb) p *aRegion->mCurRect
$32 = {<nsRegion::nsRectFast> = {<nsRect> = {x = 60, y = -240, width = 50100, 
      height = 240}, <No data fields>}, prev = 0x7fff396477e0, 
  next = 0x7fff396477e0}

I hardly understand anything of this, but I hope it helps anyway.

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

What's happening is that nsLayoutUtils::ComputeRepaintRegionForCopy looks at what's being scrolled and decides that the strip of the inner scrolling DIV that is outside the viewport needs to be repainted, since we blitted some of the document background up there and the document background is clipped to the viewport. ComputeRepaintRegionForCopy doesn't know that that area is not visible. But that's not so bad since it's just a small area. The big problem is then that nsViewManager::CanScrollWithBitBlt goes into the GTK2/OS2 path and returns *false* because the update region is nonempty, forcing us to repaint the entire window for the scroll operation :-(.

This is fragile and hard to fix, really needing Compositor. It's Linux-only and not too common so I think I'll just leave it alone for now.

Comment 14

[Arpad Borsos [:Swatinem]]

Here is a status report after testing with a recent m-c build which includes the compositor:
3.5, fully visible: 26867ms
m-c, fully visible: 25330ms

3.5, partially offscreen: 91611ms
m-c, partially offscreen: 24034ms

So this bug has been fixed, thanks a lot for your efforts ;)

Comment 15

[Jason Bassford]

No bug or patch referenced as the fix.

Comment 16

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The bug referenced was bug 352093.