SVG painting significantly slower with a empty filter than with no filter at all

RESOLVED FIXED

Status

()

P3
normal
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: mtschrep, Assigned: longsonr)

Tracking

({perf})

Trunk
PowerPC
Mac OS X
Points:
---
Bug Flags:
blocking1.9 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(5 attachments, 1 obsolete attachment)

(Reporter)

Description

11 years ago
Created attachment 292183 [details]
SVG File with empty filter body

See attachments for two sample SVG files derived from Jresig's ecma-cloud svg file.  

The first has no filter defined with id="Shadow".  The second has the following filter: 

<filter id="Shadow" filterUnits="userSpaceOnUse">
</filter>

If I load both on a recent nightly on a MacMini the page with an empty filter element beachballs like crazy - try re-sizing the window and watch it lock up.  The same file with no filter element is slow but usable.

Shark profiles attached - it looks like the filter path is going through an entirely different, an much less optimized, drawing path.   I don't think this particular use case is that interesting but it sets a pretty bad minimun performance penalty for using filters at all.  So if you've got a document with a low-cost filter you are likely to take much more of a hit than you should.
Flags: blocking1.9?
(Reporter)

Updated

11 years ago
Attachment #292183 - Attachment description: SVG File with commented out filter body → SVG File with empty filter body
(Reporter)

Comment 1

11 years ago
Created attachment 292184 [details]
Shark profile of first testcase
(Reporter)

Comment 2

11 years ago
Created attachment 292185 [details]
SVG testcase with filter element completely removed
(Reporter)

Comment 3

11 years ago
Created attachment 292186 [details]
Shark profile of second testcase
(Assignee)

Comment 4

11 years ago
The slowdown is due to the fact that for a filter you render to an offscreen surface then copy the offscreen surface to the drawing surface rather than writing to the drawing surface directly.

What if you set filterRes="2" would you expect that to be ignored as there are no filters or give you a pixellated result? Gecko should produce a pixellated result.

So the question is do we specially optimise for all the things that should make a filter undetectable i.e. no filterRes, no xlink:href (which I don't hink we support yet) etc. In the high level SVG code I think we should leave things as they are and not try for some special fast and potentially buggy code path.

Of course there may be some way to speed up what cairo is doing when it copies images.
We shouldn't optimize for the uncommon noop case, but this kind of shows that the general -baseline- speed for using any filter is very very slow.  Improving this case (without optimizations that just skip the work) would result in an improvement for every filter.

Something that could help a lot is the acquire_image_surface/XShm API that should be getting into cairo somewhat soonish, but even up to that point, there may be additional improvements that could be done.

We may even want to add some code to return a CGImageRef from a cairo surface on OSX -- that way we could even do custom code to use Core Image etc. to do the filtering...
I think we need to try and figure out some improvement here.  Any additions to cairo to help out in this case we can do (short of adding in a whole filter framework, but that shouldn't be necessary; it sounds like we just need a faster way to get at the image bits).
Flags: blocking1.9? → blocking1.9+
Priority: -- → P3
(Assignee)

Comment 7

11 years ago
Created attachment 295799 [details] [diff] [review]
on second thoughts...

On second thoughts we can do something as such filters are invalid. With this patch we render http://www.w3.org/Graphics/SVG/Test/20061213/htmlObjectHarness/full-filters-felem-01-b.html correctly.
Assignee: nobody → longsonr
Status: NEW → ASSIGNED
Attachment #295799 - Flags: superreview?(tor)
Attachment #295799 - Flags: review?(tor)
(Assignee)

Comment 8

11 years ago
Created attachment 299832 [details] [diff] [review]
update to tip and add reftests
Attachment #295799 - Attachment is obsolete: true
Attachment #299832 - Flags: superreview?(tor)
Attachment #299832 - Flags: review?(tor)
Attachment #295799 - Flags: superreview?(tor)
Attachment #295799 - Flags: review?(tor)
While I agree filters could use a lot of speeding up, I actually think the performance of Schrep's testcase is OK for 1.9. What I'm really concerned about is problems like performance on jresig's original testcase http://ejohn.org/files/ecma-cloud.svg ... which seems to hang the browser for a minute or two when you load it.
(Assignee)

Comment 10

11 years ago
That's bug 403932. This patch is more about correctness than speed up anyway, with it we pass one more of the w3c testcases.

Comment 11

11 years ago
With this patch we won't draw an element that references a nonexistent filter/mask/clipPath.  Is that really what we should be doing?
(Assignee)

Comment 12

11 years ago
From http://www.w3.org/TR/SVG/masking.html#EstablishingANewClippingPath

It is an error if the 'clip-path' property references a non-existent object or if the referenced object is not a 'clipPath' element (see Error processing).

Equivalent exists in http://www.w3.org/TR/SVG/masking.html#Masking

I think the relevant piece from Error processing is...

The document shall be rendered up to, but not including, the first element which has an error

http://www.w3.org/Graphics/SVG/Test/20061213/htmlObjectHarness/full-filters-felem-01-b.html 
Seems to me to confirm that this is what we should be doing.

Updated

11 years ago
Attachment #299832 - Flags: superreview?(tor)
Attachment #299832 - Flags: superreview+
Attachment #299832 - Flags: review?(tor)
Attachment #299832 - Flags: review+
(Assignee)

Comment 13

11 years ago
Comment on attachment 299832 [details] [diff] [review]
update to tip and add reftests

simple fixes for one of the w3c svg testcases
Attachment #299832 - Flags: approval1.9?
(Assignee)

Comment 14

11 years ago
Comment on attachment 299832 [details] [diff] [review]
update to tip and add reftests

oops blocking
Attachment #299832 - Flags: approval1.9?
(Assignee)

Comment 15

11 years ago
checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Updated

11 years ago
Keywords: perf
You need to log in before you can comment on or make changes to this bug.