447739 - Slow scrolling when the scrolled element is partially offscreen

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

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

Attachment: fix

See attachment #325218 [details]. If you shrink the window so that the scrolling DIV is partially offscreen, the test runs much slower. This is because we are repainting the entire window at every step.

Debugging shows that the problem is the DIV is transparent so the non-moving canvas is visible. That wouldn't normally be a problem since we know the canvas has a uniform color so the IsUniform test in AddItemsToRegion kicks in and we ignore it. But when the scrolling DIV is partially offscreen, we notice that the canvas doesn't contain the whole scrolled area so 'skip' gets set to false.

So we need to be a bit smarter here. Instead of using IsUniform as an all-or-nothing optimization, we need to limit what gets invalidated to exclude the area where a uniform item was just bitblitted to itself. This fixes the bug.

This bug is potentially quite serious. It doesn't matter what's in the scrolling DIV, any transparent scrolling element on a solid background that ends up partially outside the viewport is going to trigger this.

Comment 1

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

Comment on attachment 331050 [details] [diff] [review]
fix

In AccumulateItemInRegion, modifying a non-DEBUG variable (damageRect)
inside #ifdef DEBUG seems like a bad idea.  Could you use an additional
variable for the printf instead?

+          AccumulateItemInRegion(aRegion, aRect + aDelta, r + aDelta,
exclude + aDelta, item);

I don't quite follow the logic of what gets aDelta added to it.  Could
you double-check that aDelta should be added to exclude?  And maybe add
comments explaining why?

Also, maybe wrap that long line?

r+sr=dbaron

Comment 2

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

Pushed with changes, changeset 3c275aadab36.

Comment 3

[Arpad Borsos [:Swatinem]]

Hm, this patch regressed performance really bad.
I now get 118746ms on trunk vs. 72532ms on 3.0.1 when the iframe is completely visible. Without this patch, I got like 20s on trunk. At least it does not slow down any more when the iframe is partially offscreen. 3.0.1 goes down to 183319ms in that case.

Comment 4

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

Which platform? You should probably file a new bug for that, thanks.

Comment 5

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

So I rechecked on Mac and with the whole iframe visible, we're not painting more than we were before. It may be a platform dependent issue.

Comment 6

[Boris Zbarsky [:bzbarsky]]

This might have caused bug 451080.