Closed Bug 407463 Opened 16 years ago Closed 16 years ago

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


(Core :: SVG, defect, P3)






(Reporter: mtschrep, Assigned: longsonr)




(Keywords: perf)


(5 files, 1 obsolete file)

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">

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?
Attachment #292183 - Attachment description: SVG File with commented out filter body → SVG File with empty filter body
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
Attached patch on second thoughts... (obsolete) — Splinter Review
On second thoughts we can do something as such filters are invalid. With this patch we render correctly.
Assignee: nobody → longsonr
Attachment #295799 - Flags: superreview?(tor)
Attachment #295799 - Flags: review?(tor)
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 ... which seems to hang the browser for a minute or two when you load it.
That's bug 403932. This patch is more about correctness than speed up anyway, with it we pass one more of the w3c testcases.
With this patch we won't draw an element that references a nonexistent filter/mask/clipPath.  Is that really what we should be doing?

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

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 
Seems to me to confirm that this is what we should be doing.
Attachment #299832 - Flags: superreview?(tor)
Attachment #299832 - Flags: superreview+
Attachment #299832 - Flags: review?(tor)
Attachment #299832 - Flags: review+
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?
Comment on attachment 299832 [details] [diff] [review]
update to tip and add reftests

oops blocking
Attachment #299832 - Flags: approval1.9?
checked in.
Closed: 16 years ago
Resolution: --- → FIXED
Keywords: perf
You need to log in before you can comment on or make changes to this bug.