Bug 111587

Summary: SVG Pattern pixelated on inline SVG with CSS transforms
Product: WebKit Reporter: Dirk Schulze <krit>
Component: SVGAssignee: Florin Malita <fmalita>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, dglazkov, dino, d-r, eric, esprehn+autocc, fmalita, gyuyoung.kim, ojan.autocc, pdr, rakuco, rniwa, schenney, simon.fraser, thorton, webkit.review.bot, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Pixelated pattern caused by scaling HTML elements
none
Patch
none
Same problem with clips, masks & filters.
none
Patch
none
Switch paint() paths to getCTM.
none
Patch none

Description Dirk Schulze 2013-03-06 08:59:08 PST
Created attachment 191761 [details]
Pixelated pattern caused by scaling HTML elements

If an SVG document is embedded inline into an HTML element, SVG patterns of this SVG document do not respect the transform of ancestor HTML elements. This causes pixelation.
Comment 1 Florin Malita 2013-03-06 10:58:07 PST
This is most likely affecting clips & masks also, as they all use SVGRenderingContext::calculateTransformationToOutermostSVGCoordinateSystem() when calculating the image buffer size.
Comment 2 Dirk Schulze 2013-03-07 19:25:39 PST
(In reply to comment #1)
> This is most likely affecting clips & masks also, as they all use SVGRenderingContext::calculateTransformationToOutermostSVGCoordinateSystem() when calculating the image buffer size.

Could you find a way to get the transformation matrix from SVG root to page transform? I think you said need to go to the tree and calculate the transform manually, right?
Comment 3 Florin Malita 2013-03-08 06:23:53 PST
(In reply to comment #2)
> (In reply to comment #1)
> > This is most likely affecting clips & masks also, as they all use SVGRenderingContext::calculateTransformationToOutermostSVGCoordinateSystem() when calculating the image buffer size.
> 
> Could you find a way to get the transformation matrix from SVG root to page transform? I think you said need to go to the tree and calculate the transform manually, right?

Yes, I do have a patch that continues walking up the layer tree from SVGRoot and picks up the affine portion of CSS transforms.

Seems to work and fixes the problem at hand, but I was just trying to figure whether there's a more straightforward approach or if there are some strange interactions with more complex CSS transforms.

It may be good enough though, so I'll add a test and put it up for review.
Comment 4 Florin Malita 2013-03-08 12:10:34 PST
Created attachment 192265 [details]
Patch
Comment 5 WebKit Review Bot 2013-03-08 12:12:40 PST
Attachment 192265 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/platform/chromium-linux/svg/custom/resources-css-scaled-expected.png', u'LayoutTests/platform/chromium-linux/svg/custom/resources-css-scaled-expected.txt', u'LayoutTests/platform/chromium/TestExpectations', u'LayoutTests/platform/efl/TestExpectations', u'LayoutTests/platform/gtk/TestExpectations', u'LayoutTests/platform/mac/TestExpectations', u'LayoutTests/platform/qt/TestExpectations', u'LayoutTests/svg/custom/resources-css-scaled.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp', u'Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp', u'Source/WebCore/rendering/svg/RenderSVGResourceMasker.cpp', u'Source/WebCore/rendering/svg/RenderSVGResourcePattern.cpp', u'Source/WebCore/rendering/svg/SVGRenderingContext.cpp', u'Source/WebCore/rendering/svg/SVGRenderingContext.h']" exit_code: 1
LayoutTests/platform/chromium-linux/svg/custom/resources-css-scaled-expected.png:0:  Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png").  [image/png] [5]
Total errors found: 1 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Florin Malita 2013-03-08 12:15:14 PST
Created attachment 192266 [details]
Same problem with clips, masks & filters.
Comment 7 Simon Fraser (smfr) 2013-03-08 13:02:08 PST
Comment on attachment 192265 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=192265&action=review

> Source/WebCore/rendering/svg/SVGRenderingContext.cpp:215
> +    for (RenderLayer* layer = renderer->enclosingLayer(); layer; layer = layer->parent()) {
> +        TransformationMatrix* layerTransform = layer->transform();
> +        if (layerTransform)
> +            absoluteTransform = layerTransform->toAffineTransform() * absoluteTransform;

I think this should probably stop at a composited layer. Then the resolution will match that of the composited layer contents (but we might have an extra scale to take into account when we fix bug 27684).
Comment 8 Florin Malita 2013-03-08 13:30:42 PST
Created attachment 192279 [details]
Patch

Thanks Simon, updated.
Comment 9 WebKit Review Bot 2013-03-08 13:33:11 PST
Attachment 192279 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/platform/chromium-linux/svg/custom/resources-css-scaled-expected.png', u'LayoutTests/platform/chromium-linux/svg/custom/resources-css-scaled-expected.txt', u'LayoutTests/platform/chromium/TestExpectations', u'LayoutTests/platform/efl/TestExpectations', u'LayoutTests/platform/gtk/TestExpectations', u'LayoutTests/platform/mac/TestExpectations', u'LayoutTests/platform/qt/TestExpectations', u'LayoutTests/svg/custom/resources-css-scaled.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp', u'Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp', u'Source/WebCore/rendering/svg/RenderSVGResourceMasker.cpp', u'Source/WebCore/rendering/svg/RenderSVGResourcePattern.cpp', u'Source/WebCore/rendering/svg/SVGRenderingContext.cpp', u'Source/WebCore/rendering/svg/SVGRenderingContext.h']" exit_code: 1
LayoutTests/platform/chromium-linux/svg/custom/resources-css-scaled-expected.png:0:  Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png").  [image/png] [5]
Total errors found: 1 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 Dirk Schulze 2013-03-08 13:44:53 PST
Comment on attachment 192279 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=192279&action=review

> Source/WebCore/rendering/svg/SVGRenderingContext.cpp:202
> +    for (; renderer; renderer = renderer->parent()) {

For these kind of loops I like while more and it is used in a lot of other places as well. But I guess it is a matter of taste :)

> Source/WebCore/rendering/svg/SVGRenderingContext.cpp:212
> +    // Continue walking up the layer tree, accumulating CSS transforms.
> +    for (RenderLayer* layer = renderer->enclosingLayer(); layer; layer = layer->parent()) {

What if the SVG root is in a foreignObject? Maybe the check above should be updated to look if there is a parent SVG renderer?

> LayoutTests/ChangeLog:8
> +        * platform/chromium-linux/svg/custom/resources-css-scaled-expected.png: Added.

You should be able to create a ref test instead of the image.
Comment 11 Philip Rogers 2013-03-08 13:48:29 PST
Comment on attachment 192279 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=192279&action=review

> Source/WebCore/rendering/svg/SVGRenderingContext.cpp:196
> +void SVGRenderingContext::calculateTransformationToOutermostCoordinateSystem(const RenderObject* renderer, AffineTransform& absoluteTransform)

I don't think we are doing the right thing here.

What is returned from this function (now that you're going with smfr's suggestion) should be the exact same as context->getCTM(). In most of the callers of this function, the context is readily available, and in the others it's easily plumbed through.
Comment 12 Florin Malita 2013-03-08 13:57:22 PST
(In reply to comment #11)
> (From update of attachment 192279 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=192279&action=review
> 
> > Source/WebCore/rendering/svg/SVGRenderingContext.cpp:196
> > +void SVGRenderingContext::calculateTransformationToOutermostCoordinateSystem(const RenderObject* renderer, AffineTransform& absoluteTransform)
> 
> I don't think we are doing the right thing here.
> 
> What is returned from this function (now that you're going with smfr's suggestion) should be the exact same as context->getCTM(). In most of the callers of this function, the context is readily available, and in the others it's easily plumbed through.

The reason I'm hesitant about using getCTM() is the very existance of this method: why did we not rely on getCTM() from the beginning? From chatting with Dirk on IRC, it seems there might have been valid reasons (Qt platform differences) which forced us in this direction.
Comment 13 Philip Rogers 2013-03-08 14:11:02 PST
(In reply to comment #12)
> (In reply to comment #11)
> > (From update of attachment 192279 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=192279&action=review
> > 
> > > Source/WebCore/rendering/svg/SVGRenderingContext.cpp:196
> > > +void SVGRenderingContext::calculateTransformationToOutermostCoordinateSystem(const RenderObject* renderer, AffineTransform& absoluteTransform)
> > 
> > I don't think we are doing the right thing here.
> > 
> > What is returned from this function (now that you're going with smfr's suggestion) should be the exact same as context->getCTM(). In most of the callers of this function, the context is readily available, and in the others it's easily plumbed through.
> 
> The reason I'm hesitant about using getCTM() is the very existance of this method: why did we not rely on getCTM() from the beginning? From chatting with Dirk on IRC, it seems there might have been valid reasons (Qt platform differences) which forced us in this direction.

On every paint you're going to crawl up the entire tree doing matrix multiplies at each step so the QT port doesn't break? Lets just fix QT's getCTM() if necessary :)
Comment 14 Dirk Schulze 2013-03-08 14:25:05 PST
(In reply to comment #13)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > (From update of attachment 192279 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=192279&action=review
> > > 
> > > > Source/WebCore/rendering/svg/SVGRenderingContext.cpp:196
> > > > +void SVGRenderingContext::calculateTransformationToOutermostCoordinateSystem(const RenderObject* renderer, AffineTransform& absoluteTransform)
> > > 
> > > I don't think we are doing the right thing here.
> > > 
> > > What is returned from this function (now that you're going with smfr's suggestion) should be the exact same as context->getCTM(). In most of the callers of this function, the context is readily available, and in the others it's easily plumbed through.
> > 
> > The reason I'm hesitant about using getCTM() is the very existance of this method: why did we not rely on getCTM() from the beginning? From chatting with Dirk on IRC, it seems there might have been valid reasons (Qt platform differences) which forced us in this direction.
> 
> On every paint you're going to crawl up the entire tree doing matrix multiplies at each step so the QT port doesn't break? Lets just fix QT's getCTM() if necessary :)

Nothing guarantees that CTM of 1,0,0,1,0,0 is in real screen coordinates. I don't think that helps.
Comment 15 Philip Rogers 2013-03-08 15:09:57 PST
(In reply to comment #14)
> (In reply to comment #13)
> > (In reply to comment #12)
> > > (In reply to comment #11)
> > > > (From update of attachment 192279 [details] [details] [details] [details])
> > > > View in context: https://bugs.webkit.org/attachment.cgi?id=192279&action=review
> > > > 
> > > > > Source/WebCore/rendering/svg/SVGRenderingContext.cpp:196
> > > > > +void SVGRenderingContext::calculateTransformationToOutermostCoordinateSystem(const RenderObject* renderer, AffineTransform& absoluteTransform)
> > > > 
> > > > I don't think we are doing the right thing here.
> > > > 
> > > > What is returned from this function (now that you're going with smfr's suggestion) should be the exact same as context->getCTM(). In most of the callers of this function, the context is readily available, and in the others it's easily plumbed through.
> > > 
> > > The reason I'm hesitant about using getCTM() is the very existance of this method: why did we not rely on getCTM() from the beginning? From chatting with Dirk on IRC, it seems there might have been valid reasons (Qt platform differences) which forced us in this direction.
> > 
> > On every paint you're going to crawl up the entire tree doing matrix multiplies at each step so the QT port doesn't break? Lets just fix QT's getCTM() if necessary :)
> 
> Nothing guarantees that CTM of 1,0,0,1,0,0 is in real screen coordinates. I don't think that helps.

My thinking is that we should be rasterizing in layer coordinates, not screen coordinates.

Maybe that is not best for users? Is this specified?
Comment 16 Dirk Schulze 2013-03-08 15:52:53 PST
(In reply to comment #15)
> (In reply to comment #14)
> > (In reply to comment #13)
> > > (In reply to comment #12)
> > > > (In reply to comment #11)
> > > > > (From update of attachment 192279 [details] [details] [details] [details] [details])
> > > > > View in context: https://bugs.webkit.org/attachment.cgi?id=192279&action=review
> > > > > 
> > > > > > Source/WebCore/rendering/svg/SVGRenderingContext.cpp:196
> > > > > > +void SVGRenderingContext::calculateTransformationToOutermostCoordinateSystem(const RenderObject* renderer, AffineTransform& absoluteTransform)
> > > > > 
> > > > > I don't think we are doing the right thing here.
> > > > > 
> > > > > What is returned from this function (now that you're going with smfr's suggestion) should be the exact same as context->getCTM(). In most of the callers of this function, the context is readily available, and in the others it's easily plumbed through.
> > > > 
> > > > The reason I'm hesitant about using getCTM() is the very existance of this method: why did we not rely on getCTM() from the beginning? From chatting with Dirk on IRC, it seems there might have been valid reasons (Qt platform differences) which forced us in this direction.
> > > 
> > > On every paint you're going to crawl up the entire tree doing matrix multiplies at each step so the QT port doesn't break? Lets just fix QT's getCTM() if necessary :)
> > 
> > Nothing guarantees that CTM of 1,0,0,1,0,0 is in real screen coordinates. I don't think that helps.
> 
> My thinking is that we should be rasterizing in layer coordinates, not screen coordinates.
> 
> Maybe that is not best for users? Is this specified?

This is an implementation detail. It is not specified :)
Comment 17 Build Bot 2013-03-08 21:44:20 PST
Comment on attachment 192279 [details]
Patch

Attachment 192279 [details] did not pass win-ews (win):
Output: http://webkit-commit-queue.appspot.com/results/17077614
Comment 18 Build Bot 2013-03-08 21:56:55 PST
Comment on attachment 192279 [details]
Patch

Attachment 192279 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17047580
Comment 19 Build Bot 2013-03-09 00:45:35 PST
Comment on attachment 192279 [details]
Patch

Attachment 192279 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17109407
Comment 20 Florin Malita 2013-03-11 07:22:36 PDT
(In reply to comment #13)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > (From update of attachment 192279 [details] [details] [details])
> > > In most of the callers of this function, the context is readily available, and in the others it's easily plumbed through.

This is not 100% correct: one of its users is SVGRenderingContext::calculateScreenFontSizeScalingFactor() which is also called on layout()/styleDidChange() - where a GC is simply not available. So it seems we would have to keep the old approach for this path alone.


> > The reason I'm hesitant about using getCTM() is the very existance of this method: why did we not rely on getCTM() from the beginning? From chatting with Dirk on IRC, it seems there might have been valid reasons (Qt platform differences) which forced us in this direction.
> 
> On every paint you're going to crawl up the entire tree doing matrix multiplies at each step so the QT port doesn't break? Lets just fix QT's getCTM() if necessary :)

We would be crawling the layer tree which is quite sparse - so the overhead is minimal. That said, using CTM directly does sound appealing (if correct) on the paint path. I can whip up an experimental patch just to see what breaks.
Comment 21 Florin Malita 2013-03-11 07:31:42 PDT
Created attachment 192467 [details]
Switch paint() paths to getCTM.

Experimental patch to observe bots fallout - not for review.
Comment 22 WebKit Review Bot 2013-03-11 08:13:33 PDT
Comment on attachment 192467 [details]
Switch paint() paths to getCTM.

Attachment 192467 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17113594

New failing tests:
svg/custom/absolute-sized-content-with-resources.xhtml
svg/custom/non-scaling-stroke.svg
Comment 23 Florin Malita 2013-03-11 10:25:59 PDT
(In reply to comment #10)
> (From update of attachment 192279 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=192279&action=review
> 
> > Source/WebCore/rendering/svg/SVGRenderingContext.cpp:202
> > +    for (; renderer; renderer = renderer->parent()) {
> 
> For these kind of loops I like while more and it is used in a lot of other places as well. But I guess it is a matter of taste :)

Sounds good, I'll update :)


> > Source/WebCore/rendering/svg/SVGRenderingContext.cpp:212
> > +    // Continue walking up the layer tree, accumulating CSS transforms.
> > +    for (RenderLayer* layer = renderer->enclosingLayer(); layer; layer = layer->parent()) {
> 
> What if the SVG root is in a foreignObject? Maybe the check above should be updated to look if there is a parent SVG renderer?

I've seen that type of test in other places, but I don't think it's buying us much: it only catches a narrow case where the SVG root is a direct descendant of fO - but if it's further down the tree (say there's a <div> between fO and the embedded SVG) then we're not noticing. It seems we would have to walk the whole render tree to detect this, which is probably not acceptable. Besides, transformed fO content is quite busted at the moment anyway - if we land the SVG/fO layer fixes then I believe this should just work as-is.


> > LayoutTests/ChangeLog:8
> > +        * platform/chromium-linux/svg/custom/resources-css-scaled-expected.png: Added.
> 
> You should be able to create a ref test instead of the image.

Yes, sounds doable.
Comment 24 Florin Malita 2013-03-11 12:02:58 PDT
(In reply to comment #21)
> Created an attachment (id=192467) [details]
> Switch paint() paths to getCTM.
> 
> Experimental patch to observe bots fallout - not for review.

The results look encouraging: two CR failures which appear to be edge/rounding artifacts, with no visually noticeable effect.

I think we should move ahead with the current approach and pursue a getCTM switch in a separate bug.
Comment 25 Florin Malita 2013-03-11 12:04:08 PDT
Created attachment 192525 [details]
Patch

Updated per review.
Comment 26 Dirk Schulze 2013-03-11 12:55:18 PDT
Comment on attachment 192525 [details]
Patch

I am very much in favor for having a working version now, that may can be improved later. I expect a lot more work on getCTM in the short term. And, as fmailita says, would always require a context, which is maybe not the case in some situations. r+ from me.
Comment 27 Build Bot 2013-03-12 04:32:50 PDT
Comment on attachment 192525 [details]
Patch

Attachment 192525 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17173107

New failing tests:
editing/selection/selection-modify-crash.html
Comment 28 Florin Malita 2013-03-12 07:26:21 PDT
(In reply to comment #27)
> (From update of attachment 192525 [details])
> Attachment 192525 [details] did not pass mac-ews (mac):
> Output: http://webkit-commit-queue.appspot.com/results/17173107
> 
> New failing tests:
> editing/selection/selection-modify-crash.html

I don't think this is related, hopefully a flake. Let's see if the CQ bots agree.
Comment 29 WebKit Review Bot 2013-03-12 07:41:01 PDT
Comment on attachment 192525 [details]
Patch

Clearing flags on attachment: 192525

Committed r145541: <http://trac.webkit.org/changeset/145541>
Comment 30 WebKit Review Bot 2013-03-12 07:41:08 PDT
All reviewed patches have been landed.  Closing bug.
Comment 31 Tim Horton 2013-04-23 15:22:01 PDT
Did you intend to accumulate CSS transforms translation components too? This is breaking painting of a few test cases I have (I'll try to reduce them to a postable state).
Comment 32 Florin Malita 2013-04-23 15:47:34 PDT
(In reply to comment #31)
> Did you intend to accumulate CSS transforms translation components too? This is breaking painting of a few test cases I have (I'll try to reduce them to a postable state).

Are CSS translations not reflected in the layer transform? If they're stored as layer offsets only, then yes, this patch misses that.

But I don't think the patch itself introduced this breakage - the previous tree crawl was stopping even earlier, at the SVG root boundary.
Comment 33 Tim Horton 2013-04-23 15:53:01 PDT
(In reply to comment #32)
> (In reply to comment #31)
> > Did you intend to accumulate CSS transforms translation components too? This is breaking painting of a few test cases I have (I'll try to reduce them to a postable state).
> 
> Are CSS translations not reflected in the layer transform? If they're stored as layer offsets only, then yes, this patch misses that.
> 
> But I don't think the patch itself introduced this breakage - the previous tree crawl was stopping even earlier, at the SVG root boundary.

They *are* in the transform, and I don't think we want to include them! Stopping at the SVG root boundary would cause them to not be included. Including them means that the GraphicsContexts returned by SVG image buffer creation code will be translated by an additional offset that SVG doesn't know about (and doesn't include when painting!).

Definitely caused by this patch.

I'm going to remove the translation component when crawling the tree, that fixes my bug without reintroducing yours.
Comment 34 Florin Malita 2013-04-23 16:17:42 PDT
(In reply to comment #33)
> (In reply to comment #32)
> > (In reply to comment #31)
> > > Did you intend to accumulate CSS transforms translation components too? This is breaking painting of a few test cases I have (I'll try to reduce them to a postable state).
> > 
> > Are CSS translations not reflected in the layer transform? If they're stored as layer offsets only, then yes, this patch misses that.
> > 
> > But I don't think the patch itself introduced this breakage - the previous tree crawl was stopping even earlier, at the SVG root boundary.
> 
> They *are* in the transform, and I don't think we want to include them! Stopping at the SVG root boundary would cause them to not be included. Including them means that the GraphicsContexts returned by SVG image buffer creation code will be translated by an additional offset that SVG doesn't know about (and doesn't include when painting!).

Ah, I see - I misread your comment as asking for translation to be included. I'm away from my workstation now, but at the time I remember convincing myself that all users only cared about the scale component anyway. Seems that I was wrong.

> Definitely caused by this patch.
> 
> I'm going to remove the translation component when crawling the tree, that fixes my bug without reintroducing yours.

Sure, that's the obvious middle ground but then I'd also suggest renaming the method to something more appropriate.

I'm kind of curious how cumulative SVG translations are not causing the same problem if present in the ImageBuffer GC. I assume we compensate for them somehow, but not for CSS. Will find out tomorrow...
Comment 35 Tim Horton 2013-04-23 17:55:02 PDT
(In reply to comment #34)
> (In reply to comment #33)
> > (In reply to comment #32)
> > > (In reply to comment #31)
> > > > Did you intend to accumulate CSS transforms translation components too? This is breaking painting of a few test cases I have (I'll try to reduce them to a postable state).
> > > 
> > > Are CSS translations not reflected in the layer transform? If they're stored as layer offsets only, then yes, this patch misses that.
> > > 
> > > But I don't think the patch itself introduced this breakage - the previous tree crawl was stopping even earlier, at the SVG root boundary.
> > 
> > They *are* in the transform, and I don't think we want to include them! Stopping at the SVG root boundary would cause them to not be included. Including them means that the GraphicsContexts returned by SVG image buffer creation code will be translated by an additional offset that SVG doesn't know about (and doesn't include when painting!).
> 
> Ah, I see - I misread your comment as asking for translation to be included. I'm away from my workstation now, but at the time I remember convincing myself that all users only cared about the scale component anyway. Seems that I was wrong.

I was wrong, slightly. It's the call from RenderSVGResourceMasker::applyResource to SVGRenderingContext::clipToImageBuffer that goes wrong. I'm going to file another bug so we can stop talking on this resolved one :)

> > Definitely caused by this patch.
> > 
> > I'm going to remove the translation component when crawling the tree, that fixes my bug without reintroducing yours.
> 
> Sure, that's the obvious middle ground but then I'd also suggest renaming the method to something more appropriate.
> 
> I'm kind of curious how cumulative SVG translations are not causing the same problem if present in the ImageBuffer GC. I assume we compensate for them somehow, but not for CSS. Will find out tomorrow...