Reset m_didTransformToRootUpdate flag during invalidationTreeIfNeeded

Source/core/rendering/svg/RenderSVGTransformableContainer.cpp

 /*
  * Copyright (C) 2004, 2005 Nikolas Zimmermann <zimmermann@kde.org>
  * Copyright (C) 2004, 2005, 2006 Rob Buis <buis@kde.org>
  * Copyright (C) 2009 Google, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
@@ skipping 78 matching lines @@
     if (useElement) {
         SVGLengthContext lengthContext(useElement);
         FloatSize translation(
             useElement->x()->currentValue()->value(lengthContext),
             useElement->y()->currentValue()->value(lengthContext));
         if (translation != m_lastTranslation)
             m_needsTransformUpdate = true;
         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;
 
     m_localTransform = element->animatedLocalTransform();
     m_localTransform.translate(m_lastTranslation.width(), m_lastTranslation.height());
     m_needsTransformUpdate = false;
     return true;
 }
 
+void RenderSVGTransformableContainer::clearPaintInvalidationState()
+{
+    RenderSVGContainer::clearPaintInvalidationState();
+
+    ASSERT(!m_needsTransformUpdate);
+    m_didTransformToRootUpdate = false;
 }
+
+}