445079 - Make foreignObject work in filters (and patterns)

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

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

Attachment: fix

Currently foreignObject doesn't work when 'filter' is applied to it, because filters create a new nsSVGRenderState with no embedded nsIRenderingContext. We should just create one on demand. One issue is that the gfxContext wrapped in an nsIRenderingContext must be heap-allocated; this patch changes nsSVGRenderState to target a temporary surface instead of a temporary context, and makes nsSVGRenderState manage the gfxContext itself.

Comment 1

[Robert Longson [:longsonr]]

Comment on attachment 329382 [details] [diff] [review]
fix

>diff -NrpU12 mozilla-checkin.6692da1caef5/layout/reftests/svg/filter-foreignobject-01.svg mozilla-checkin/layout/reftests/svg/filter-foreignobject-01.svg
>--- mozilla-checkin.6692da1caef5/layout/reftests/svg/filter-foreignobject-01.svg	1970-01-01 12:00:00.000000000 +1200
>+++ mozilla-checkin/layout/reftests/svg/filter-foreignobject-01.svg	2008-07-14 

Can you change the filename case to foreignObject so that it matches the other foreignObject files in this directory please?

Comment 2

[Mats Palmgren (inactive)]

Comment on attachment 329382 [details] [diff] [review]
fix

sr=mats

It looks like nsSVGFilterInstance.cpp does not need
#include "gfxContext.h"

Comment 3

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

Pushed 6c8c95fcaf9b with comments.

Comment 4

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

I backed out ... the test failed on Windows. I'm not sure why.

Comment 5

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

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1216092112.1216095075.13838.gz

Comment 6

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

I think bug 445410 is contributing to the problem. However, the main issue is that the filter region is not pixel-aligned (because of the default -10%/+20% values) and so there's inevitable smearing when we composite a temporary surface into that rectangle. I think perhaps we should snap the filter region to pixels.

Comment 7

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

But I'd like to land the rest of the filter patches from my branch before I fix that, otherwise there will be conflicts.

Comment 8

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(In reply to comment #6)
> I think perhaps we should snap the filter region to pixels.

That sounds like a good plan to me.  Would you round the region outwards, or just snap to nearest?

Comment 9

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

Probably snap to nearest.

Comment 10

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

Pushed 8c1b20b1be5e.

I didn't solve the underlying smearing problem that caused the test failure. I just worked around it by making the filter region precisely the viewport. I don't have time to solve the smearing problem right now and I needed to land this fix to keep making progress landing bling-branch patches.

Comment 11

[Hans Schmucker]

Verified. Works perfectly with Canvas.. now we only need Tamarin to speed up software filters and we have a Photoshop killer :)