459512 - Translated elements with filters don't work any more

Status: RESOLVED FIXED

(Core :: SVG, defect)

Comment 1

[Robert Longson [:longsonr]]

http://www.w3.org/Graphics/SVG/Test/20061213/htmlObjectHarness/full-filters-example-01-b.html and http://www.w3.org/Graphics/SVG/Test/20061213/htmlObjectHarness/full-filters-gauss-01-b.html are also broken.

Comment 2

[Robert Longson [:longsonr]]

I see missing filters in the examples. One the top left filter is displayed. In the example-01-b I only see a grey box.

A regression range might help.

Comment 3

[Martijn Wargers (dead)]

This regressed between 2008-09-10 and 2008-09-11:
http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2008-09-10+04%3A00%3A00&enddate=2008-09-11+12%3A00%3A00
Regression from bug 450340?

Comment 4

[Robert Longson [:longsonr]]

Something goes wrong when the filter is transformed. I.e. having the use within a <g transform="...".

Comment 6

[Robert Longson [:longsonr]]

Any ideas what's happening here roc?

Comment 7

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

Nothing springs to mind. Got a reduced testcase?

Comment 8

[Robert Longson [:longsonr]]

Is the one in bug 459675 any good?

Comment 9

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

That one passes for me on Mac. The w3.org tests fail though.

Comment 10

[Robert Longson [:longsonr]]

Attachment: reduced testcase

If I take the path that is within the morphologySource <g>, replace the use with it and stick the filter directly on the path I do see something so this is about as small as it gets I think.

Comment 11

[Robert Longson [:longsonr]]

Attachment: WIP

This makes filters display and returns us to the way we processed filters before bug 450340 i.e. passing null as the dirtyRect but it's wallpaper.

clearly there is a problem in nsSVGFilterInstance::BuildSourceImages in the calculation of the dirty rect. Some translation is missing. Can you see what it is roc?

Comment 12

[Robert Longson [:longsonr]]

Not so much a review request as a cry for help ;-)

Comment 13

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

Attachment: Slightly simpler testcase

Comment 14

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

In nsSVGFilterInstance::BuildSourceImages, we render the source <g>, but that fails to paint the <rect> child because in nsSVGUtils::PaintChildWithEffects the <rect>'s frame's mRect is tested against the dirty rect and they don't intersect. The problem is, mRect is relative to the root <svg>, but aDirty is in "user space" for the frame.

Comment 15

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

Attachment: fix

-- Transform the dirty rect from user space CSS pixels to outer-SVG device pixels
-- Instead of using SetOverrideCTM, just set the transform the gfxContext. By doing this in the nsSVGFilterInstance, we can remove the transform parameter from nsSVGFilterPaintCallback::Paint. This works because SetMatrixPropagation(PR_FALSE) is guaranteed to be in effect (it actually happens in nsAutoFilterInstance::nsAutoFilterInstance in nsSVGFilterFrame). The reason I'm doing this here is that I think the current code is buggy, since it doesn't do SetOverrideCTM(nsnull) and an associated NotifySVGChanged, which it really should.

Also with the latter change, there are no more real users of SetOverrideCTM and we can remove that stuff completely! I'll file a separate bug for that cleanup.

Comment 16

[Mats Palmgren (inactive)]

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

sr=mats

In nsSVGFilterPaintCallback.h - you forgot to remove "@param aTransform ..."
In nsSVGUtils.cpp - please end comment sentences with a period.

Comment 17

[Robert Longson [:longsonr]]

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


I've moved the good bits of my original patch to bug 460210

Comment 18

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

Pushed dfdda0399888, thanks