In nsSVGFilterInstance I need to call nsSVGUtils::GetRelativeRect and nsSVGUtils::UserSpace with something that resolves the lengths relative to the canvas space, and in cases where there's no nsIFrame for the canvas. At the moment you need to supply an nsIFrame to the length resolution functions, and many of them have two special cases: one for SVG frames (which end up calling methods of the frame's SVG element), and one for non-SVG frames. I didn't want to add a third overload for canvas all over the place, so I decided to introduce an interface called UserSpaceMetrics which has two implementations for the existing users but can be subclassed for the canvas case. I'm not too happy with the name "UserSpaceMetrics"... maybe "SVGLengthContext" would be better? I'm pretty sure this patch keeps the behavior of all existing methods unchanged.

I'm going to clean up the nsFilterInstance constructor in a new bug, after this one has landed.

Comment on attachment 8488184 [details] [diff] [review] part 6: Allow aTargetFrame to be null in nsFilterInstance. Review of attachment 8488184 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/svg/nsFilterInstance.cpp @@ +203,2 @@ > nsRect preFilterVOR = mTargetFrame->GetPreEffectsVisualOverflowRect(); > mTargetBounds = FrameSpaceToFilterSpace(&preFilterVOR); Shouldn't something set mTargetBounds when mTargetFrame is null? What is going to do that?

mTargetBounds looks like a leftover. Every time its value is used, it's union'ed with mTargetBBoxInFilterSpace, but as far as I can tell mTargetBBoxInFilterSpace always contains mTargetBounds. I've tested with SVG frames, non-SVG frames, and non-SVG frames with continuations. In the continuation case mTargetBBoxInFilterSpace is the bounding box over all continuations and mTargetBounds is just the first frame.

We do need mTargetBounds after all because for SVG elements with a stroke, the BBox doesn't include the stroke. This patch at least puts the unioning into one place and makes sure that mTargetBounds has a useful value even if mTargetFrame is null.

https://hg.mozilla.org/integration/mozilla-inbound/rev/fd1c65e7c68c https://hg.mozilla.org/integration/mozilla-inbound/rev/b6154a1235ed https://hg.mozilla.org/integration/mozilla-inbound/rev/769fcdda208d https://hg.mozilla.org/integration/mozilla-inbound/rev/5a4011365692 https://hg.mozilla.org/integration/mozilla-inbound/rev/53e6a6204893 https://hg.mozilla.org/integration/mozilla-inbound/rev/b7bed5a0ee60 https://hg.mozilla.org/integration/mozilla-inbound/rev/89bc7483714f https://hg.mozilla.org/integration/mozilla-inbound/rev/3bda71abf648 https://hg.mozilla.org/integration/mozilla-inbound/rev/f5ab7e76389e

https://hg.mozilla.org/mozilla-central/rev/fd1c65e7c68c https://hg.mozilla.org/mozilla-central/rev/b6154a1235ed https://hg.mozilla.org/mozilla-central/rev/769fcdda208d https://hg.mozilla.org/mozilla-central/rev/5a4011365692 https://hg.mozilla.org/mozilla-central/rev/53e6a6204893 https://hg.mozilla.org/mozilla-central/rev/b7bed5a0ee60 https://hg.mozilla.org/mozilla-central/rev/89bc7483714f https://hg.mozilla.org/mozilla-central/rev/3bda71abf648 https://hg.mozilla.org/mozilla-central/rev/f5ab7e76389e