VERIFIED FIXED in Firefox 28

Status

()

Core
SVG
--
major
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: smaug, Assigned: mstange)

Tracking

({regression})

Trunk
mozilla29
x86_64
All
regression
Points:
---

Firefox Tracking Flags

(firefox28+ verified, firefox29+ verified)

Details

Attachments

(10 attachments)

1.16 KB, patch
roc
: review+
Details | Diff | Splinter Review
1.07 KB, patch
roc
: review+
Details | Diff | Splinter Review
1.90 KB, patch
roc
: review+
Details | Diff | Splinter Review
1.35 KB, patch
roc
: review+
Details | Diff | Splinter Review
9.93 KB, patch
roc
: review+
Details | Diff | Splinter Review
732 bytes, patch
roc
: review+
Details | Diff | Splinter Review
1.73 KB, patch
roc
: review+
Details | Diff | Splinter Review
1023 bytes, patch
roc
: review+
Details | Diff | Splinter Review
20.62 KB, patch
roc
: review+
Details | Diff | Splinter Review
1.06 KB, patch
roc
: review+
Details | Diff | Splinter Review
Load http://w3c.github.io/webcomponents/spec/shadow/#distribution-results
Make sure chapter '4.3 Distribution Results' is on top. Try to scroll.
release, beta and aurora are behaving somewhat ok, but trunk is giving perhaps 2fps scrolling speed.
Scrolling is pretty bad on windows too, not only Linux.
OS: Linux → All
AFAICT, the regression range should be:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6ecf0c4dfcbe&tochange=3c44c1f43a67
(Assignee)

Comment 3

4 years ago
It's probably a regression from bug 924102 / bug 924103. I'll take a look.
Assignee: nobody → mstange
Status: NEW → ASSIGNED
(Assignee)

Comment 4

4 years ago
This is a great testcase! I've found a good number of problems with it.
Blocks: 924103
Keywords: regressionwindow-wanted
(Assignee)

Comment 5

4 years ago
Created attachment 8342365 [details] [diff] [review]
Part 1: IN compositing intersects, FilterSupport part

This is the root cause of the slowdown. The shadows in the testcase are created by having a composite filter with operator IN apply on the blurred shadow mask and a flood filter primitive that carries the shadow color. The filter primitive subregion of the flood filter is much bigger than the shadow, so we create unnecessarily big surfaces. Knowing that the IN composite mode will have a result that's only sized to the intersection of the inputs helps us a lot here.
Attachment #8342365 - Flags: review?(roc)
(Assignee)

Comment 6

4 years ago
Created attachment 8342367 [details] [diff] [review]
Part 2: IN compositing intersects, FilterNodeSoftware part

The same for FilterNodeCompositeSoftware.
Attachment #8342367 - Flags: review?(roc)
(Assignee)

Updated

4 years ago
Attachment #8342367 - Attachment description: Part 2: Part 1: IN compositing intersects, FilterNodeSoftware part → Part 2: IN compositing intersects, FilterNodeSoftware part
(Assignee)

Comment 7

4 years ago
Created attachment 8342370 [details] [diff] [review]
Part 3: Smarter extents for arithmetic compositing

We can do a similar optimization depending on the arithmetic coefficents. Not needed for this testcase, but nice to have.
Attachment #8342370 - Flags: review?(roc)
(Assignee)

Comment 8

4 years ago
Created attachment 8342371 [details] [diff] [review]
Part 4: Smarter extents for FilterNodeArithmeticCombineSoftware

Same as part 3 but for FilterNodeArithmeticCombineSoftware.
Attachment #8342371 - Flags: review?(roc)
(Assignee)

Comment 9

4 years ago
Created attachment 8342374 [details] [diff] [review]
Part 5: Be consistent about null inputs and empty output rects

Consistently treating null inputs as complete transparency lets us use empty output rects without breaking stuff.
Attachment #8342374 - Flags: review?(roc)
(Assignee)

Comment 10

4 years ago
Created attachment 8342378 [details] [diff] [review]
Part 6: Make transparent floods report an empty output rect.

We use flood filters with transparent black when our SourceGraphic does not intersect with the parts needed by the filter. (They're created in FilterCachedColorModels when the input filter is null.)
Making the output rect empty lets us pipe nullptr through much of the filter graph.
Attachment #8342378 - Flags: review?(roc)
(Assignee)

Comment 11

4 years ago
Created attachment 8342382 [details] [diff] [review]
Part 7: No output padding

This part is actually important for performance. Padding the surface is completely unnecessary if we calculate the right rectangles, and I was just too lazy to do that before.
Attachment #8342382 - Flags: review?(roc)
(Assignee)

Comment 12

4 years ago
Created attachment 8342383 [details] [diff] [review]
Part 8: Don't let transparent feFlood primitives contribute to the filter extents

Like part 6, but for FilterSupport.
Attachment #8342383 - Flags: review?(roc)
(Assignee)

Comment 13

4 years ago
Created attachment 8342384 [details] [diff] [review]
Part 9: Improve debug output
Attachment #8342384 - Flags: review?(roc)
(Assignee)

Comment 14

4 years ago
Created attachment 8342385 [details] [diff] [review]
Part 10: Simplify crop filter

Unrelated to this bug, but it jumped out to me while doing the nullptr stuff.
Attachment #8342385 - Flags: review?(roc)
Attachment #8342365 - Flags: review?(roc) → review+
Attachment #8342367 - Flags: review?(roc) → review+
Attachment #8342370 - Flags: review?(roc) → review+
Attachment #8342371 - Flags: review?(roc) → review+
Attachment #8342374 - Flags: review?(roc) → review+
Attachment #8342378 - Flags: review?(roc) → review+
Attachment #8342382 - Flags: review?(roc) → review+
Attachment #8342383 - Flags: review?(roc) → review+
Attachment #8342384 - Flags: review?(roc) → review+
Attachment #8342385 - Flags: review?(roc) → review+
Tested the tryserver build, and performance seems to be back to where it was before bug 924103
Any chance to get this landed? We need this now on Aurora too.
Status: ASSIGNED → NEW
status-firefox28: --- → affected
status-firefox29: --- → affected
tracking-firefox28: --- → ?
tracking-firefox29: --- → ?
(Assignee)

Comment 17

4 years ago
pushed to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1edd10e96bdf
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ae3923c4474
https://hg.mozilla.org/integration/mozilla-inbound/rev/693950e89aaa
https://hg.mozilla.org/integration/mozilla-inbound/rev/64b5493f8864
https://hg.mozilla.org/integration/mozilla-inbound/rev/82248770b776
https://hg.mozilla.org/integration/mozilla-inbound/rev/5e9402ef85fe
https://hg.mozilla.org/integration/mozilla-inbound/rev/2bb0a3f8d79a
https://hg.mozilla.org/integration/mozilla-inbound/rev/a6f55cce82be
https://hg.mozilla.org/integration/mozilla-inbound/rev/1a6cfda25087
https://hg.mozilla.org/integration/mozilla-inbound/rev/59835a0ef577

I'll request Aurora approval once this has been in Nightly for a few days.

Comment 18

4 years ago
FWIW I'm on M/I with this patch and scrolling is still choppy around all the SVG stuff. Should it be as smooth as a page without any SVG?
I was comparing to the builds before bug 924103 landed.

And yes, the scrolling is still a bit bad, but I think it is about the same as before bug 924103.

Comment 20

4 years ago
I compared the scrolling to IE11 under Windows 8.1 and it doesn't seem to sputter when it hits the SVG stuff.
This bug is only about the regression caused by bug 924103. 

For other issues there are other bugs, and if there isn't, file a new one.
https://hg.mozilla.org/mozilla-central/rev/1edd10e96bdf
https://hg.mozilla.org/mozilla-central/rev/0ae3923c4474
https://hg.mozilla.org/mozilla-central/rev/693950e89aaa
https://hg.mozilla.org/mozilla-central/rev/64b5493f8864
https://hg.mozilla.org/mozilla-central/rev/82248770b776
https://hg.mozilla.org/mozilla-central/rev/5e9402ef85fe
https://hg.mozilla.org/mozilla-central/rev/2bb0a3f8d79a
https://hg.mozilla.org/mozilla-central/rev/a6f55cce82be
https://hg.mozilla.org/mozilla-central/rev/1a6cfda25087
https://hg.mozilla.org/mozilla-central/rev/59835a0ef577
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
How big of an impact do you feel this will have on our users?
Flags: needinfo?(roc)
I think we should fix this on Aurora either by uplifting these patches or backing out 924103 on Aurora.
Flags: needinfo?(roc)
tracking-firefox28: ? → +
tracking-firefox29: ? → +
So if we get these nominated soon we could potentially get them landed before everyone leaves for vacation.
Flags: needinfo?(roc)
(Assignee)

Comment 26

4 years ago
Comment on attachment 8342365 [details] [diff] [review]
Part 1: IN compositing intersects, FilterSupport part

I'm requesting approval for all patches in this bug.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 924103
User impact if declined: slow scrolling in certain scenarios
Testing completed (on m-c, etc.): one week on m-c
Risk to taking this patch (and alternatives if risky): low, alternative is backing out bug 924103
String or IDL/UUID changes made by this patch: none
Attachment #8342365 - Flags: approval-mozilla-aurora?
Flags: needinfo?(roc)
status-firefox29: affected → fixed
Attachment #8342365 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/d06c5131e573
https://hg.mozilla.org/releases/mozilla-aurora/rev/462fcf308863
https://hg.mozilla.org/releases/mozilla-aurora/rev/c32beacc14f4
https://hg.mozilla.org/releases/mozilla-aurora/rev/4321400b4a7c
https://hg.mozilla.org/releases/mozilla-aurora/rev/96bd2d5ba2e2
https://hg.mozilla.org/releases/mozilla-aurora/rev/4268a977d520
https://hg.mozilla.org/releases/mozilla-aurora/rev/96277064d324
https://hg.mozilla.org/releases/mozilla-aurora/rev/37c7dbcbd689
https://hg.mozilla.org/releases/mozilla-aurora/rev/27f6b16901f6
https://hg.mozilla.org/releases/mozilla-aurora/rev/87a3450489c7
status-firefox28: affected → fixed
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:28.0) Gecko/20100101 Firefox/28.0
Mozilla/5.0 (X11; Linux x86_64; rv:28.0) Gecko/20100101 Firefox/28.0
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0

Performance is similar or better than builds without patch from bug 924103 on latest Aurora 28.0a2 (buildID: 20140103004002).
status-firefox28: fixed → verified
Reproduced nightly 2013-12-03.
Verified fixed 29.0a1 (2014-01-09), win 7 x64.
Status: RESOLVED → VERIFIED
status-firefox29: fixed → verified
You need to log in before you can comment on or make changes to this bug.