Created attachment 206784 [details] Testcase Steps to reproduce the problem: Use feImage to reference a branch that contains any radialGradient (including an empty definition). See the attached feimage-radial-broken.svg. What is the expected behavior? feImage should not be affected by the presence of a radialGradient declaration. The attached SVG should show two identical rectangles with black bars. What went wrong? The feImage is drawn using the wrong transform. The transform changes with the size of the containing SVG (e.g. window resize). If one removes the empty radialGradient declaration, the image is drawn correctly. Works on Opera 12.10 and IE 10, fails on Chrome and Safari. See also crbug.com/260709, crbug.com/257648.
Created attachment 209544 [details] Patch
Created attachment 209548 [details] Patch
Comment on attachment 209548 [details] Patch Can the test really reproduce the problem? IIRC without your patch the test would pass as well. Wouldn't it? Maybe a ref test would be better here? Fix looks good.
(In reply to comment #3) > (From update of attachment 209548 [details]) > Can the test really reproduce the problem? IIRC without your patch the test would pass as well. Wouldn't it? Maybe a ref test would be better here? > > Fix looks good. You are right, will fix.
Created attachment 209772 [details] Patch
Comment on attachment 209772 [details] Patch Clearing flags on attachment: 209772 Committed r154713: <http://trac.webkit.org/changeset/154713>
All reviewed patches have been landed. Closing bug.
I think we should roll out this patch. This patch stops us from propagating the relative length flag up the tree for renderers that are not SVGGraphicsElements. This has the effect of fixing this specific issue because the gradient element is not an SVGGraphicsElement, but it masks the real bug: SVGFEImage.cpp doesn't understand relative lengths (http://trac.webkit.org/browser/trunk/Source/WebCore/svg/graphics/filters/SVGFEImage.cpp?rev=167295#L94). This probably regressed the propagation of relative lengths for non-SVGGraphicsElements such as SVGPatternElements. This small tweak to the testcase will still trigger the root cause: <svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="300px" height="300px"> <g> <filter filterUnits="userSpaceOnUse" primitiveUnits="userSpaceOnUse" id="flt" x="0" y="0" width="50" height="100"> <feImage xlink:href="#img" /> </filter> <rect filter="url(#flt)" width="50" height="100"/> <g transform="translate(50 0)"> <g id="img"> <rect width="1%" height="100" fill="green"/> <rect width="50" height="100" fill="green"/> </g> </g> </g> </svg> We should probably also fix the quite interesting while loop here: http://trac.webkit.org/browser/trunk/Source/WebCore/svg/SVGElement.cpp#L1130
Philip, aren't all resources (such as patterns, gradients, masks etc.) only relative to their point of use and only reflected in the appearance of their point of use? And since their point of use can be anywhere in the DOM (or in a different document altogether), why would they need to propagate any of their properties to their parent?
(In reply to comment #9) > Philip, aren't all resources (such as patterns, gradients, masks etc.) only relative to their point of use and only reflected in the appearance of their point of use? And since their point of use can be anywhere in the DOM (or in a different document altogether), why would they need to propagate any of their properties to their parent? What about clipPath? It is not an SVGGraphicsElement but it grabs it's bounds from its parent so we need to propagate its relative length info up the tree. See http://jsfiddle.net/6UxrQ/1 The more general issue with this patch is the testcase I posted in #8. This patch papers over a small subset of a real bug in SVGFeImage.cpp.
(In reply to comment #10) > (In reply to comment #9) > > Philip, aren't all resources (such as patterns, gradients, masks etc.) only relative to their point of use and only reflected in the appearance of their point of use? And since their point of use can be anywhere in the DOM (or in a different document altogether), why would they need to propagate any of their properties to their parent? > > What about clipPath? It is not an SVGGraphicsElement but it grabs it's bounds from its parent so we need to propagate its relative length info up the tree. See http://jsfiddle.net/6UxrQ/1 > > The more general issue with this patch is the testcase I posted in #8. This patch papers over a small subset of a real bug in SVGFeImage.cpp. Does Blink have a solution for the real problem yet?
(In reply to comment #11) > (In reply to comment #10) > > (In reply to comment #9) > > > Philip, aren't all resources (such as patterns, gradients, masks etc.) only relative to their point of use and only reflected in the appearance of their point of use? And since their point of use can be anywhere in the DOM (or in a different document altogether), why would they need to propagate any of their properties to their parent? > > > > What about clipPath? It is not an SVGGraphicsElement but it grabs it's bounds from its parent so we need to propagate its relative length info up the tree. See http://jsfiddle.net/6UxrQ/1 > > > > The more general issue with this patch is the testcase I posted in #8. This patch papers over a small subset of a real bug in SVGFeImage.cpp. > > Does Blink have a solution for the real problem yet? Sadly, Blink does not.
This seems to work properly in Safari, but not in Chrome or Firefox.
<rdar://problem/97100663>