SVG <use> elements will appear with x="0" y="0" statically and can moved using any other values in JS, but exactly x=0 and y=0 results in no visual difference. Same problem when removing both x and y attributes.
Created attachment 81217 [details] Example of the problem, the green rect should cover the red one after one second.
Created attachment 101798 [details] Patch
Comment on attachment 101798 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=101798&action=review > Source/WebCore/rendering/svg/SVGShadowTreeElements.cpp:53 > + m_yOffset = y; > + if (renderer()) > + static_cast<RenderSVGTransformableContainer*>(renderer())->setNeedsTransformUpdate(); Si it is not only 0,0 affected but every other position as well? Because according your patch I don't see a reason why just 0,0 should be affected.
Comment on attachment 101798 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=101798&action=review >> Source/WebCore/rendering/svg/SVGShadowTreeElements.cpp:53 >> + static_cast<RenderSVGTransformableContainer*>(renderer())->setNeedsTransformUpdate(); > > Si it is not only 0,0 affected but every other position as well? Because according your patch I don't see a reason why just 0,0 should be affected. The more interessting question is why would you need a "transform update" if transform did not change? Note that RenderSVGShadowTreeRootContainer inherits from RenderSVGTransformableContainer, and bool RenderSVGTransformableContainer::calculateLocalTransform() contains special code to apply the container offset to the local transform. This should work even _without_ touching setNeedsTransformUpdate. If it doesn't there's another bug that you're hiding by this workaround. r- for this reason.
Good point Dirk - why is only x/y=0 affected. Here's the answer: bool RenderSVGTransformableContainer::calculateLocalTransform() { SVGStyledTransformableElement* element = static_cast<SVGStyledTransformableElement*>(node()); ...... FloatSize translation = static_cast<SVGShadowTreeContainerElement*>(element)->containerTranslation(); if (!translation.width() && !translation.height()) return needsUpdate; if translation width/height (corresponds to x/y) is 0 we'll ignore updating the transform. We need to store the previous translation, and compare it with the current translation, if they are equal just "return needsUpdate" otherwhise go on.... That explains why only x=0&&y=0 failed here, and avoids the setNeedsTransformUpdate hack from your current patch...
Hi guys, (In reply to comment #5) > Good point Dirk - why is only x/y=0 affected. > Here's the answer: > > bool RenderSVGTransformableContainer::calculateLocalTransform() > { > SVGStyledTransformableElement* element = static_cast<SVGStyledTransformableElement*>(node()); > ...... > > FloatSize translation = static_cast<SVGShadowTreeContainerElement*>(element)->containerTranslation(); > if (!translation.width() && !translation.height()) > return needsUpdate; > > if translation width/height (corresponds to x/y) is 0 we'll ignore updating the transform. > We need to store the previous translation, and compare it with the current translation, if they are equal just "return needsUpdate" otherwhise go on.... > > That explains why only x=0&&y=0 failed here, and avoids the setNeedsTransformUpdate hack from your current patch... Was just about to hint at the same code (just woken up) :) Will have a look at it again. Cheers, Rob.
Created attachment 101814 [details] Patch
Comment on attachment 101814 [details] Patch Excellent patch, r=me!
Landed in r91644.