800198 - Stop unnecessarily invalidating transformed frames when the page is scrolled

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Loic]

STR:
1) Open this SVG demo: http://www.jasondavies.com/factorisation-diagrams/
2) Scroll the page with the vertical scrollbar or the middle button

Result: 
Scrolling is slowed down and isn't as smooth as in FF16. After scrolling the page during a while, f you switch to another tab then you come back, there is a slight hang to display the tab with the SVG demo.

m-c
good=2012-08-02
bad=2012-08-03
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=588424024294&tochange=89dcadd42ec4

There are some bugs about SVG patches, like bug 655877 or bug 776054.

Comment 1

[Alice0775 White]

Regression window(cached m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/a799b5bff84c
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120801155239
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/b077c43a4306
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120801163038
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=a799b5bff84c&tochange=b077c43a4306

And svg.display-lists.painting.enabled = false helps

Comment 2

[Loic]

Thanks for the reg range.

Comment 3

[timbugzilla]

Also seeing very slow scrolling on this page which uses SVG images:

http://www.circuits.io/

Comment 4

[Lukas Blakk [:lsblakk] use ?needinfo]

Passing this to jwatt to take a look since it's a possible regression from his work on things that landed in 16.

Comment 5

[Jonathan Watt [:jwatt]]

Regarding the regression range from comment 1, I can't really see much of a difference on Mac. When I do see a huge slowdown is much later when bug 539356 (DLBI) landed.

Comment 6

[Jonathan Watt [:jwatt]]

The issue after DLBI landed is that when we scroll we now invalidate and repaint the areas of all the SVGs that are onscreen instead of just blitting them.

Comment 7

[Jonathan Watt [:jwatt]]

Attachment: reduced testcase

This reduced testcase shows how we invalidate the area of the transformed contents of the SVG each time we scroll.

Comment 8

[Jonathan Watt [:jwatt]]

Attachment: reduced testcase showing the same problem with HTML and CSS transforms

This problem is not confined to SVG. The same thing happens with HTML that has CSS transforms applied.

Comment 9

[Jonathan Watt [:jwatt]]

I'm finding it a bit of a headache to debug, but one thing I've noticed is that in ContainerState::ProcessDisplayItems |isFixed = mBuilder->IsFixedItem(item, &activeScrolledRoot)| incorrectly sets isFixed to true in these testcases. That seems to be because nsDisplayListBuilder::IsFixedItem calls nsLayoutUtils::IsScrolledByRootContentDocumentDisplayportScrolling assuming it will return true if the the item is scrolled. However, Matt tells me that this is only valid on Mobile, and it will _always_ return false on non-Mobile. Clearly that's wrong.

Comment 10

[Alice0775 White]

(In reply to Jonathan Watt [:jwatt] from comment #5)
> Regarding the regression range from comment 1, I can't really see much of a
> difference on Mac. 

So,I think this is Windows specific Bug.

> When I do see a huge slowdown is much later when bug
> 539356 (DLBI) landed.

This bug happens since Firefox17 and later.
So,I think the bug 539356 (DLBI) that you pointed out to be a different bug.

Comment 11

[Matt Woodrow (:mattwoodrow)]

Attachment: Make mContainerReferenceFrame the reference frame for the items inside the container, not the container itself

Comment 12

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

Comment on attachment 671596 [details] [diff] [review]
Make mContainerReferenceFrame the reference frame for the items inside the container, not the container itself

Review of attachment 671596 [details] [diff] [review]:
-----------------------------------------------------------------

This could be tested pretty easily.

::: layout/base/nsDisplayList.h
@@ +2595,5 @@
>    virtual uint32_t GetPerFrameKey() MOZ_OVERRIDE { return (mIndex << nsDisplayItem::TYPE_BITS) | nsDisplayItem::GetPerFrameKey(); }
>  
> +  virtual const nsIFrame* ReferenceFrameForChildren() const MOZ_OVERRIDE {
> +    if (!mTransformGetter) {
> +      return mFrame;

Add a comment explaining that transform display items with mTransformGetters aren't reference frames.

Comment 13

[Jonathan Watt [:jwatt]]

(In reply to Alice0775 White from comment #10)
> So,I think this is Windows specific Bug.

Seems like this bug has become about fixing the later DLBI issue, so let's open a different bug for that issue.

Comment 14

[Jonathan Watt [:jwatt]]

Created (In reply to Jonathan Watt [:jwatt] from comment #13)
> Seems like this bug has become about fixing the later DLBI issue, so let's
> open a different bug for that issue.

Cloned to bug 801949.

Also spun off bug 801918 and bug 801923 for other issues mentioned here.

Comment 15

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4ea9ef4e135e

Comment 16

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4ea9ef4e135e

Comment 17

[Alex Keybl [:akeybl]]

(In reply to Ed Morley [:edmorley UTC+0] from comment #16)
> https://hg.mozilla.org/mozilla-central/rev/4ea9ef4e135e

Ready for uplift Matt?

Comment 18

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 671596 [details] [diff] [review]
Make mContainerReferenceFrame the reference frame for the items inside the container, not the container itself

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bu 539356
User impact if declined: Scrolling performance impacted.
Testing completed (on m-c, etc.): Been on m-c for a few weeks without issues.
Risk to taking this patch (and alternatives if risky): Very low risk.
String or UUID changes made by this patch: None

Comment 19

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/20b686450e86

Comment 20

[Bogdan Maris, Desktop Test Engineering]

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/20100101 Firefox/19.0
Mozilla/5.0 (X11; Linux i686; rv:19.0) Gecko/20100101 Firefox/19.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:19.0) Gecko/20100101 Firefox/19.0

I can reproduce this on Firefox 19 beta 5 (buildID: 20130206083616), latest Nightly (buildID: 20130211031055), latest Aurora (buildID: 20130211042016).

Comment 21

[Alice0775 White]

(In reply to Jonathan Watt [:jwatt] from comment #13)
> (In reply to Alice0775 White from comment #10)
> > So,I think this is Windows specific Bug.
> 
> Seems like this bug has become about fixing the later DLBI issue, so let's
> open a different bug for that issue.

Filed Bug 840503