Closed Bug 800198 Opened 7 years ago Closed 7 years ago

Stop unnecessarily invalidating transformed frames when the page is scrolled

Categories

(Core :: SVG, defect)

17 Branch
x86_64
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla19
Tracking Status
firefox18 + fixed
firefox19 + fixed

People

(Reporter: epinal99-bugzilla2, Assigned: mattwoodrow)

References

Details

(Keywords: regression)

Attachments

(3 files)

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.
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
Blocks: 776054
Thanks for the reg range.
Also seeing very slow scrolling on this page which uses SVG images:

http://www.circuits.io/
Passing this to jwatt to take a look since it's a possible regression from his work on things that landed in 16.
Assignee: nobody → 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.
Blocks: dlbi
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.
Attached file reduced testcase
This reduced testcase shows how we invalidate the area of the transformed contents of the SVG each time we scroll.
This problem is not confined to SVG. The same thing happens with HTML that has CSS transforms applied.
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.
(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 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.
Attachment #671596 - Flags: review?(roc) → review+
(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.
Assignee: jwatt → matt.woodrow
No longer blocks: 776054
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.
Summary: Scrolling slowdown and hang on SVG demo → Stop unnecessarily invalidating transformed frames when the page is scrolled
https://hg.mozilla.org/mozilla-central/rev/4ea9ef4e135e
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
(In reply to Ed Morley [:edmorley UTC+0] from comment #16)
> https://hg.mozilla.org/mozilla-central/rev/4ea9ef4e135e

Ready for uplift Matt?
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
Attachment #671596 - Flags: approval-mozilla-aurora?
Attachment #671596 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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).
(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
You need to log in before you can comment on or make changes to this bug.