From ef8159d6c7af19e5580f8e710d0c9ead60290af0 Mon Sep 17 00:00:00 2001 From: "krit@webkit.org" Date: Wed, 8 Feb 2012 19:19:03 +0000 Subject: [PATCH] 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 --- LayoutTests/ChangeLog | 13 +++++++ .../repaint/inner-svg-change-viewBox-expected.png | Bin 0 -> 3318 bytes .../repaint/inner-svg-change-viewBox-expected.txt | 6 ++++ .../svg/repaint/inner-svg-change-viewBox.svg | 14 ++++++++ Source/WebCore/ChangeLog | 39 +++++++++++++++++++++ .../WebCore/rendering/svg/RenderSVGContainer.cpp | 3 ++ Source/WebCore/rendering/svg/RenderSVGContainer.h | 2 ++ Source/WebCore/rendering/svg/RenderSVGRoot.cpp | 1 - .../rendering/svg/RenderSVGViewportContainer.cpp | 9 +++++ .../rendering/svg/RenderSVGViewportContainer.h | 11 ++++++ Source/WebCore/rendering/svg/SVGRenderSupport.cpp | 16 ++++++++- Source/WebCore/svg/SVGSVGElement.cpp | 3 +- 12 files changed, 114 insertions(+), 3 deletions(-) create mode 100644 LayoutTests/svg/repaint/inner-svg-change-viewBox-expected.png create mode 100644 LayoutTests/svg/repaint/inner-svg-change-viewBox-expected.txt create mode 100644 LayoutTests/svg/repaint/inner-svg-change-viewBox.svg diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index 8ac621b..0daa57d 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,16 @@ +2012-02-08 Dirk Schulze + + 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 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 index 0000000000000000000000000000000000000000..261fa5f8420a003890811e9f8970acbf95565bc1 GIT binary patch literal 3318 zcmeAS@N?(olHy`uVBq!ia0y~yU{+vYV2a>i1B%QlYbpRznkB9gCCM47$=SuFxeVrs zh8E_QW|kHymd3`G7KW*osfLM$CYF|F7AY1csm2G+E?CdNz|-OB;uumf=k3jlyv+sz ztd7D<-CwX-mG6pJT(PW4{qFTweII))D|#!fkI#;|zO{;p;XwS)HF7}P-Z(b^Nl8W~ z28I+C4hDt^f(i@_4sN4@BPc-jeV(`49GD7be_&)}U@&50VPH7HA;7?(pzOfF(9pv$ zDmcOdq + + + + + + + diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index df32d41..6bc0c8d 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,42 @@ +2012-02-08 Dirk Schulze + + 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 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 --- 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 --- 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 --- 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(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 --- 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(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 --- 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(object); } +inline const RenderSVGViewportContainer* toRenderSVGViewportContainer(const RenderObject* object) +{ + ASSERT(!object || !strcmp(object->renderName(), "RenderSVGViewportContainer")); + return static_cast(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 --- 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 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 --- 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; -- 2.7.4