viewBox on nested SVG causes wrong content size for relative values

https://bugs.webkit.org/show_bug.cgi?id=69459

Reviewed by Nikolas.

Source/WebCore:

In the past we just checked the change of the viewport size of the root SVG element. If the size changed, all childs
with relative length values needed a relayout. We did not consider that we might have other viewports in the document.
Childs with relative lengths had a strange zooming, if just the viewport size of an inner SVG element changed.

With this patch we check if the size of the nearest viewport changes. Is this the case, childs with relative lengths
need a relayout.

Test: inner-svg-change-viewBox.svg

* rendering/svg/RenderSVGContainer.cpp:
(WebCore::RenderSVGContainer::layout):
* rendering/svg/RenderSVGContainer.h:
(RenderSVGContainer):
(WebCore::RenderSVGContainer::determineIfLayoutSizeChanged): Check if we need layout and have relative length values.
* rendering/svg/RenderSVGRoot.cpp:
(WebCore::RenderSVGRoot::layout): Remove resetting 'viewport size changed' flag for code operability. No influence on the layout.
* rendering/svg/RenderSVGViewportContainer.cpp:
(WebCore::RenderSVGViewportContainer::RenderSVGViewportContainer): Add a flag that indicates that the viewport size changes.
(WebCore::RenderSVGViewportContainer::determineIfLayoutSizeChanged): The flag gets set during the layout phase of the SVG element if the size changes.
(WebCore):
* rendering/svg/RenderSVGViewportContainer.h:
(WebCore::RenderSVGViewportContainer::isLayoutSizeChanged): Added getter to get flag status.
(RenderSVGViewportContainer):
(WebCore::toRenderSVGViewportContainer): Added casting function for constant RenderObjects.
(WebCore):
* rendering/svg/SVGRenderSupport.cpp:
(WebCore::layoutSizeOfNearestViewportChanged): Search the nearest viewport and check if the size changed.
(WebCore):
(WebCore::SVGRenderSupport::layoutChildren): Don't check the roots viewport for size changes, but the nearest viewport.
* svg/SVGSVGElement.cpp:
(WebCore::SVGSVGElement::svgAttributeChanged): Added viewBoxAttr to the list of attributes that cause relayout.

LayoutTests:

Test relayout of content of inner SVG on change of relative length values.

* svg/repaint/inner-svg-change-viewBox-expected.png: Added.
* svg/repaint/inner-svg-change-viewBox-expected.txt: Added.
* svg/repaint/inner-svg-change-viewBox.svg: Added.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@107108 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 8ac621b..0daa57d 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2012-02-08  Dirk Schulze  <krit@webkit.org>
+
+        viewBox on nested SVG causes wrong content size for relative values
+        https://bugs.webkit.org/show_bug.cgi?id=69459
+
+        Reviewed by Nikolas.
+
+        Test relayout of content of inner SVG on change of relative length values.
+
+        * svg/repaint/inner-svg-change-viewBox-expected.png: Added.
+        * svg/repaint/inner-svg-change-viewBox-expected.txt: Added.
+        * svg/repaint/inner-svg-change-viewBox.svg: Added.
+
 2012-02-08  Gregg Tavares  <gman@chromium.org>
 
         Implement new WEBGL compressed texture extensions

diff --git a/LayoutTests/svg/repaint/inner-svg-change-viewBox-expected.png b/LayoutTests/svg/repaint/inner-svg-change-viewBox-expected.png
new file mode 100644 (file)
index 0000000..261fa5f
Binary files /dev/null and b/LayoutTests/svg/repaint/inner-svg-change-viewBox-expected.png differ

diff --git a/LayoutTests/svg/repaint/inner-svg-change-viewBox-expected.txt b/LayoutTests/svg/repaint/inner-svg-change-viewBox-expected.txt
new file mode 100644 (file)
index 0000000..69f1070
--- /dev/null
+++ b/LayoutTests/svg/repaint/inner-svg-change-viewBox-expected.txt
@@ -0,0 +1,6 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 200x200
+  RenderSVGRoot {svg} at (0,0) size 100x100
+    RenderSVGViewportContainer {svg} at (0,0) size 100x100
+      RenderSVGRect {rect} at (0,0) size 100x100 [fill={[type=SOLID] [color=#008000]}] [x=0.00] [y=0.00] [width=100.00] [height=100.00]

diff --git a/LayoutTests/svg/repaint/inner-svg-change-viewBox.svg b/LayoutTests/svg/repaint/inner-svg-change-viewBox.svg
new file mode 100644 (file)
index 0000000..1b23bcb
--- /dev/null
+++ b/LayoutTests/svg/repaint/inner-svg-change-viewBox.svg
@@ -0,0 +1,14 @@
+<svg width="200" height="200" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" onload="runRepaintTest()">
+<script xlink:href="../../fast/repaint/resources/repaint.js"></script>
+<script>
+<![CDATA[
+    function repaintTest() {
+        document.getElementById('s').setAttribute("viewBox", "0 0 200 200");
+    }
+]]>
+</script>
+
+<svg id="s" viewBox="0 0 800 800">
+    <rect fill="green" width="50%" height="50%"/>
+</svg>
+</svg>

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index df32d41..6bc0c8d 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,42 @@
+2012-02-08  Dirk Schulze  <krit@webkit.org>
+
+        viewBox on nested SVG causes wrong content size for relative values
+        https://bugs.webkit.org/show_bug.cgi?id=69459
+
+        Reviewed by Nikolas.
+
+        In the past we just checked the change of the viewport size of the root SVG element. If the size changed, all childs
+        with relative length values needed a relayout. We did not consider that we might have other viewports in the document.
+        Childs with relative lengths had a strange zooming, if just the viewport size of an inner SVG element changed.
+
+        With this patch we check if the size of the nearest viewport changes. Is this the case, childs with relative lengths
+        need a relayout.
+
+        Test: inner-svg-change-viewBox.svg
+
+        * rendering/svg/RenderSVGContainer.cpp:
+        (WebCore::RenderSVGContainer::layout):
+        * rendering/svg/RenderSVGContainer.h:
+        (RenderSVGContainer):
+        (WebCore::RenderSVGContainer::determineIfLayoutSizeChanged): Check if we need layout and have relative length values.
+        * rendering/svg/RenderSVGRoot.cpp:
+        (WebCore::RenderSVGRoot::layout): Remove resetting 'viewport size changed' flag for code operability. No influence on the layout.
+        * rendering/svg/RenderSVGViewportContainer.cpp:
+        (WebCore::RenderSVGViewportContainer::RenderSVGViewportContainer): Add a flag that indicates that the viewport size changes.
+        (WebCore::RenderSVGViewportContainer::determineIfLayoutSizeChanged): The flag gets set during the layout phase of the SVG element if the size changes.
+        (WebCore):
+        * rendering/svg/RenderSVGViewportContainer.h:
+        (WebCore::RenderSVGViewportContainer::isLayoutSizeChanged): Added getter to get flag status.
+        (RenderSVGViewportContainer):
+        (WebCore::toRenderSVGViewportContainer): Added casting function for constant RenderObjects.
+        (WebCore):
+        * rendering/svg/SVGRenderSupport.cpp:
+        (WebCore::layoutSizeOfNearestViewportChanged): Search the nearest viewport and check if the size changed.
+        (WebCore):
+        (WebCore::SVGRenderSupport::layoutChildren): Don't check the roots viewport for size changes, but the nearest viewport.
+        * svg/SVGSVGElement.cpp:
+        (WebCore::SVGSVGElement::svgAttributeChanged): Added viewBoxAttr to the list of attributes that cause relayout.
+
 2012-02-08  Gregg Tavares  <gman@google.com>
 
         Implement new WEBGL compressed texture extensions

diff --git a/Source/WebCore/rendering/svg/RenderSVGContainer.cpp b/Source/WebCore/rendering/svg/RenderSVGContainer.cpp
index cffa755..5155655 100644 (file)
--- a/Source/WebCore/rendering/svg/RenderSVGContainer.cpp
+++ b/Source/WebCore/rendering/svg/RenderSVGContainer.cpp
@@ -63,6 +63,9 @@ void RenderSVGContainer::layout()
     // Allow RenderSVGTransformableContainer to update its transform.
     bool updatedTransform = calculateLocalTransform();
 
+    // RenderSVGViewportContainer needs to set the 'layout size changed' flag.
+    determineIfLayoutSizeChanged();
+
     SVGRenderSupport::layoutChildren(this, selfNeedsLayout() || SVGRenderSupport::filtersForceContainerLayout(this));
 
     // Invalidate all resources of this client if our layout changed.

diff --git a/Source/WebCore/rendering/svg/RenderSVGContainer.h b/Source/WebCore/rendering/svg/RenderSVGContainer.h
index 60b91e1..c7d9f20 100644 (file)
--- a/Source/WebCore/rendering/svg/RenderSVGContainer.h
+++ b/Source/WebCore/rendering/svg/RenderSVGContainer.h
@@ -67,6 +67,8 @@ protected:
     virtual void applyViewportClip(PaintInfo&) { }
     virtual bool pointIsInsideViewportClip(const FloatPoint& /*pointInParent*/) { return true; }
 
+    virtual void determineIfLayoutSizeChanged() { }
+
     bool selfWillPaint();
     void updateCachedBoundaries();
 

diff --git a/Source/WebCore/rendering/svg/RenderSVGRoot.cpp b/Source/WebCore/rendering/svg/RenderSVGRoot.cpp
index 507f383..ba821cc 100644 (file)
--- a/Source/WebCore/rendering/svg/RenderSVGRoot.cpp
+++ b/Source/WebCore/rendering/svg/RenderSVGRoot.cpp
@@ -214,7 +214,6 @@ void RenderSVGRoot::layout()
     SVGSVGElement* svg = static_cast<SVGSVGElement*>(node());
     m_isLayoutSizeChanged = needsLayout || (svg->hasRelativeLengths() && oldSize != size());
     SVGRenderSupport::layoutChildren(this, needsLayout || SVGRenderSupport::filtersForceContainerLayout(this));
-    m_isLayoutSizeChanged = false;
 
     // At this point LayoutRepainter already grabbed the old bounds,
     // recalculate them now so repaintAfterLayout() uses the new bounds.

diff --git a/Source/WebCore/rendering/svg/RenderSVGViewportContainer.cpp b/Source/WebCore/rendering/svg/RenderSVGViewportContainer.cpp
index 8b7e65b..1021aff 100644 (file)
--- a/Source/WebCore/rendering/svg/RenderSVGViewportContainer.cpp
+++ b/Source/WebCore/rendering/svg/RenderSVGViewportContainer.cpp
@@ -34,9 +34,18 @@ namespace WebCore {
 
 RenderSVGViewportContainer::RenderSVGViewportContainer(SVGStyledElement* node)
     : RenderSVGContainer(node)
+    , m_isLayoutSizeChanged(false)
 {
 }
 
+void RenderSVGViewportContainer::determineIfLayoutSizeChanged()
+{
+    if (!node()->hasTagName(SVGNames::svgTag))
+        return;
+
+    m_isLayoutSizeChanged = static_cast<SVGSVGElement*>(node())->hasRelativeLengths() && selfNeedsLayout();
+}
+
 void RenderSVGViewportContainer::applyViewportClip(PaintInfo& paintInfo)
 {
     if (SVGRenderSupport::isOverflowHidden(this))

diff --git a/Source/WebCore/rendering/svg/RenderSVGViewportContainer.h b/Source/WebCore/rendering/svg/RenderSVGViewportContainer.h
index 07be7eb..808d672 100644 (file)
--- a/Source/WebCore/rendering/svg/RenderSVGViewportContainer.h
+++ b/Source/WebCore/rendering/svg/RenderSVGViewportContainer.h
@@ -35,6 +35,10 @@ public:
     explicit RenderSVGViewportContainer(SVGStyledElement*);
     FloatRect viewport() const { return m_viewport; }
 
+    bool isLayoutSizeChanged() const { return m_isLayoutSizeChanged; }
+
+    virtual void determineIfLayoutSizeChanged();
+
 private:
     virtual bool isSVGContainer() const { return true; }
     virtual bool isSVGViewportContainer() const { return true; }
@@ -50,6 +54,7 @@ private:
 
     FloatRect m_viewport;
     mutable AffineTransform m_localToParentTransform;
+    bool m_isLayoutSizeChanged : 1;
 };
   
 inline RenderSVGViewportContainer* toRenderSVGViewportContainer(RenderObject* object)
@@ -58,6 +63,12 @@ inline RenderSVGViewportContainer* toRenderSVGViewportContainer(RenderObject* ob
     return static_cast<RenderSVGViewportContainer*>(object);
 }
 
+inline const RenderSVGViewportContainer* toRenderSVGViewportContainer(const RenderObject* object)
+{
+    ASSERT(!object || !strcmp(object->renderName(), "RenderSVGViewportContainer"));
+    return static_cast<const RenderSVGViewportContainer*>(object);
+}
+
 // This will catch anyone doing an unnecessary cast.
 void toRenderSVGViewportContainer(const RenderSVGViewportContainer*);
 

diff --git a/Source/WebCore/rendering/svg/SVGRenderSupport.cpp b/Source/WebCore/rendering/svg/SVGRenderSupport.cpp
index 2a3801b..6457ac0 100644 (file)
--- a/Source/WebCore/rendering/svg/SVGRenderSupport.cpp
+++ b/Source/WebCore/rendering/svg/SVGRenderSupport.cpp
@@ -39,6 +39,7 @@
 #include "RenderSVGResourceMarker.h"
 #include "RenderSVGResourceMasker.h"
 #include "RenderSVGRoot.h"
+#include "RenderSVGViewportContainer.h"
 #include "SVGResources.h"
 #include "SVGResourcesCache.h"
 #include "SVGStyledElement.h"
@@ -231,9 +232,22 @@ static inline void invalidateResourcesOfChildren(RenderObject* start)
         invalidateResourcesOfChildren(child);
 }
 
+static inline bool layoutSizeOfNearestViewportChanged(const RenderObject* start)
+{
+    while (start && !start->isSVGRoot() && !start->isSVGViewportContainer())
+        start = start->parent();
+
+    ASSERT(start);
+    ASSERT(start->isSVGRoot() || start->isSVGViewportContainer());
+    if (start->isSVGViewportContainer())
+        return toRenderSVGViewportContainer(start)->isLayoutSizeChanged();
+
+    return toRenderSVGRoot(start)->isLayoutSizeChanged();
+}
+
 void SVGRenderSupport::layoutChildren(RenderObject* start, bool selfNeedsLayout)
 {
-    bool layoutSizeChanged = findTreeRootObject(start)->isLayoutSizeChanged();
+    bool layoutSizeChanged = layoutSizeOfNearestViewportChanged(start);
     HashSet<RenderObject*> notlayoutedObjects;
 
     for (RenderObject* child = start->firstChild(); child; child = child->nextSibling()) {

diff --git a/Source/WebCore/svg/SVGSVGElement.cpp b/Source/WebCore/svg/SVGSVGElement.cpp
index 590eec9..8d6f1b0 100644 (file)
--- a/Source/WebCore/svg/SVGSVGElement.cpp
+++ b/Source/WebCore/svg/SVGSVGElement.cpp
@@ -303,7 +303,8 @@ void SVGSVGElement::svgAttributeChanged(const QualifiedName& attrName)
     if (updateRelativeLengths
         || SVGLangSpace::isKnownAttribute(attrName)
         || SVGExternalResourcesRequired::isKnownAttribute(attrName)
-        || SVGZoomAndPan::isKnownAttribute(attrName)) {
+        || SVGZoomAndPan::isKnownAttribute(attrName)
+        || attrName == SVGNames::viewBoxAttr) {
         if (renderer())
             RenderSVGResource::markForLayoutAndParentResourceInvalidation(renderer());
         return;