Reset m_didTransformToRootUpdate flag during invalidationTreeIfNeeded

Source/core/rendering/svg/RenderSVGTransformableContainer.cpp

Index: Source/core/rendering/svg/RenderSVGTransformableContainer.cpp
diff --git a/Source/core/rendering/svg/RenderSVGTransformableContainer.cpp b/Source/core/rendering/svg/RenderSVGTransformableContainer.cpp
index 7778c9d5e347d1b983fd24bfc4ab7c79630e31f4..71420f28c608013d940795df0251746f478e90dc 100644
--- a/Source/core/rendering/svg/RenderSVGTransformableContainer.cpp
+++ b/Source/core/rendering/svg/RenderSVGTransformableContainer.cpp
@@ -96,7 +96,9 @@ bool RenderSVGTransformableContainer::calculateLocalTransform()
         m_lastTranslation = translation;
     }
 
-    m_didTransformToRootUpdate = m_needsTransformUpdate || SVGRenderSupport::transformToRootChanged(parent());
+    if (m_needsTransformUpdate || SVGRenderSupport::transformToRootChanged(parent()))
+        m_didTransformToRootUpdate = true;

[dsinclair, 2014/07/23 19:20:59]

I don't think we want to do this. We use this flag to determine if we can skip
the tree walk. So, I think what we care about is the result of the last layout.

If the last layout sets this to true, then we can skip the tree walk as the
container will handle the invalidations.

If the last layout sets this to false, then we have to do the tree walk as we
don't know if the container will do the invalidations for us.

This change will, potentially, set this to true on the first layout but then on
the last layout, if it should be false we'll leave the flag as true. So the
container won't issue the invalidation for us, and we'll terminate the tree walk
early. Leaving the subtree as potentially missed invalidations.

[Xianzhu, 2014/07/23 19:39:04]

I think the full invalidation on transform change should happen in the next
invalidateTreeIfNeeded(). If this is true, it is correct to skip subtree walking
if the flag is set in any layout since the last invalidateTreeIfNeeded().

I didn't find any direct paint invalidation code in svg. However I'm not
familiar with SVG code.

[Julien - ping for review, 2014/07/23 20:05:24]

I agree with Xianzhu on this one, it's conceptually wrong to ignore all layouts
but the last one when generating invalidations. This is because now the order of
layouts will have an impact on our invalidations.

[fs, 2014/07/24 08:43:56]

If there's N layouts to one repaint, and the container is moved (transform
changed) in one of them, then repainting only the container should be sufficient
because the repaint area of the container is the union of all it's children.
From that PoV accumulating into it seems fine (I think I'd just have |= though
for a smaller change =)). This flag is still used to do funky stuff to text
metrics during layout though (see SVGRenderSupport).

+
     if (!m_needsTransformUpdate)
         return false;
 
@@ -106,4 +108,12 @@ bool RenderSVGTransformableContainer::calculateLocalTransform()
     return true;
 }
 
+void RenderSVGTransformableContainer::clearPaintInvalidationState()
+{
+    RenderSVGContainer::clearPaintInvalidationState();
+
+    ASSERT(!m_needsTransformUpdate);
+    m_didTransformToRootUpdate = false;
+}
+
 }