Add invalid bounding box concept to SVG containers
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 26 Mar 2012 14:36:59 +0000 (14:36 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 26 Mar 2012 14:36:59 +0000 (14:36 +0000)
https://bugs.webkit.org/show_bug.cgi?id=81104

Patch by Philip Rogers <pdr@google.com> on 2012-03-26
Reviewed by Nikolas Zimmermann.

Source/WebCore:

An empty <g> element needs to use an invalid bounding box because
an empty bounding box isn't the default state. This change
introduces the concept of an invalid object bounding box for
both RenderSVGContainer and RenderSVGRoot. Code that
does not explicitly check that the bounding box is valid
should be unaffected by this change. We use this new invalid
flag in computeContainerBoundingBoxes so that we do not
include invalid bounding boxes.

This change also contains a small fix in
RenderSVGContainer::toRenderSVGContainer which depended on
RenderSVGViewportContainer not inheriting from RenderSVGContainer,
which it now does.

Test: svg/custom/getBBox-empty-container.html

* rendering/svg/RenderSVGContainer.cpp:
(WebCore::RenderSVGContainer::RenderSVGContainer):
(WebCore::RenderSVGContainer::updateCachedBoundaries):
* rendering/svg/RenderSVGContainer.h:
(WebCore::RenderSVGContainer::isObjectBoundingBoxValid):
(RenderSVGContainer):
(WebCore::toRenderSVGContainer):
* rendering/svg/RenderSVGRoot.cpp:
(WebCore::RenderSVGRoot::RenderSVGRoot):
(WebCore::RenderSVGRoot::updateCachedBoundaries):
* rendering/svg/RenderSVGRoot.h:
(RenderSVGRoot):
* rendering/svg/SVGRenderSupport.cpp:
(WebCore):
(WebCore::updateObjectBoundingBox):
(WebCore::SVGRenderSupport::computeContainerBoundingBoxes):
* rendering/svg/SVGRenderSupport.h:
(SVGRenderSupport):

LayoutTests:

* svg/custom/getBBox-empty-container-expected.txt: Added.
* svg/custom/getBBox-empty-container.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/svg/custom/getBBox-empty-container-expected.txt [new file with mode: 0644]
LayoutTests/svg/custom/getBBox-empty-container.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/svg/RenderSVGContainer.cpp
Source/WebCore/rendering/svg/RenderSVGContainer.h
Source/WebCore/rendering/svg/RenderSVGRoot.cpp
Source/WebCore/rendering/svg/RenderSVGRoot.h
Source/WebCore/rendering/svg/SVGRenderSupport.cpp
Source/WebCore/rendering/svg/SVGRenderSupport.h

index b4b997f..430653c 100644 (file)
@@ -1,3 +1,13 @@
+2012-03-26  Philip Rogers  <pdr@google.com>
+
+        Add invalid bounding box concept to SVG containers
+        https://bugs.webkit.org/show_bug.cgi?id=81104
+
+        Reviewed by Nikolas Zimmermann.
+
+        * svg/custom/getBBox-empty-container-expected.txt: Added.
+        * svg/custom/getBBox-empty-container.html: Added.
+
 2012-03-26  Pavel Feldman  <pfeldman@chromium.org>
 
         Not reviewed: [chromium] rebaselining plugins/embed-attributes-style.
diff --git a/LayoutTests/svg/custom/getBBox-empty-container-expected.txt b/LayoutTests/svg/custom/getBBox-empty-container-expected.txt
new file mode 100644 (file)
index 0000000..292e569
--- /dev/null
@@ -0,0 +1 @@
+PASS, bounding box sizes are (100, 100) and (100, 100)
diff --git a/LayoutTests/svg/custom/getBBox-empty-container.html b/LayoutTests/svg/custom/getBBox-empty-container.html
new file mode 100644 (file)
index 0000000..0d664a9
--- /dev/null
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<html>
+<!-- Test that svg bounding boxes are not affected by empty containers -->
+    <head>
+        <script type="text/javascript">
+            function checkBoundingBoxesEqual() {
+                if (window.layoutTestController)
+                    window.layoutTestController.dumpAsText();
+
+                // Ensure both boxes are the same size, meaning the empty <g> does not affect the bbox size.
+                var bboxA = document.getElementById('ga').getBBox();
+                var bboxB = document.getElementById('gb').getBBox();
+                var results = "FAIL";
+                if (bboxA.width == bboxB.width && bboxA.height == bboxB.height)
+                    results = "PASS";
+                document.body.innerHTML = results + ", bounding box sizes are (" + 
+                    bboxA.width + ", " + bboxA.height + ") and (" + 
+                    bboxB.width + ", " + bboxB.height + ")";
+            }
+        </script>
+    </head>
+    <body onload="checkBoundingBoxesEqual()">
+        <svg width="400" height="400" xmlns="http://www.w3.org/2000/svg">    
+            <g id="ga">
+                <g></g>
+                <g transform="translate(100, 100)">
+                       <g></g>
+                    <rect x="0" y="0" width="100" height="100" rx="10" ry="10" stroke="#000000" stroke-width="1" fill="#ffffcc" />
+                </g>
+            </g>
+            <g id="gb">
+                <g transform="translate(100, 100)">
+                    <rect x="0" y="0" width="100" height="100" rx="10" ry="10" stroke="#000000" stroke-width="1" fill="#ffffcc" />
+                </g>
+            </g>
+        </svg>
+    </body>
+</html>
index 5c00894..715a194 100644 (file)
@@ -1,3 +1,45 @@
+2012-03-26  Philip Rogers  <pdr@google.com>
+
+        Add invalid bounding box concept to SVG containers
+        https://bugs.webkit.org/show_bug.cgi?id=81104
+
+        Reviewed by Nikolas Zimmermann.
+
+        An empty <g> element needs to use an invalid bounding box because
+        an empty bounding box isn't the default state. This change
+        introduces the concept of an invalid object bounding box for
+        both RenderSVGContainer and RenderSVGRoot. Code that
+        does not explicitly check that the bounding box is valid
+        should be unaffected by this change. We use this new invalid
+        flag in computeContainerBoundingBoxes so that we do not
+        include invalid bounding boxes.
+
+        This change also contains a small fix in
+        RenderSVGContainer::toRenderSVGContainer which depended on
+        RenderSVGViewportContainer not inheriting from RenderSVGContainer,
+        which it now does.
+
+        Test: svg/custom/getBBox-empty-container.html
+
+        * rendering/svg/RenderSVGContainer.cpp:
+        (WebCore::RenderSVGContainer::RenderSVGContainer):
+        (WebCore::RenderSVGContainer::updateCachedBoundaries):
+        * rendering/svg/RenderSVGContainer.h:
+        (WebCore::RenderSVGContainer::isObjectBoundingBoxValid):
+        (RenderSVGContainer):
+        (WebCore::toRenderSVGContainer):
+        * rendering/svg/RenderSVGRoot.cpp:
+        (WebCore::RenderSVGRoot::RenderSVGRoot):
+        (WebCore::RenderSVGRoot::updateCachedBoundaries):
+        * rendering/svg/RenderSVGRoot.h:
+        (RenderSVGRoot):
+        * rendering/svg/SVGRenderSupport.cpp:
+        (WebCore):
+        (WebCore::updateObjectBoundingBox):
+        (WebCore::SVGRenderSupport::computeContainerBoundingBoxes):
+        * rendering/svg/SVGRenderSupport.h:
+        (SVGRenderSupport):
+
 2012-03-26  Alexei Filippov  <alexeif@chromium.org>
 
         Web Inspector: Speed up the retainers build phase.
index e16bfa3..a1fd832 100644 (file)
@@ -40,6 +40,7 @@ namespace WebCore {
 
 RenderSVGContainer::RenderSVGContainer(SVGStyledElement* node)
     : RenderSVGModelObject(node)
+    , m_objectBoundingBoxValid(false)
     , m_needsBoundariesUpdate(true)
 {
 }
@@ -150,10 +151,11 @@ void RenderSVGContainer::addFocusRingRects(Vector<IntRect>& rects, const LayoutP
 void RenderSVGContainer::updateCachedBoundaries()
 {
     m_objectBoundingBox = FloatRect();
+    m_objectBoundingBoxValid = false;
     m_strokeBoundingBox = FloatRect();
     m_repaintBoundingBox = FloatRect();
 
-    SVGRenderSupport::computeContainerBoundingBoxes(this, m_objectBoundingBox, m_strokeBoundingBox, m_repaintBoundingBox);
+    SVGRenderSupport::computeContainerBoundingBoxes(this, m_objectBoundingBox, m_objectBoundingBoxValid, m_strokeBoundingBox, m_repaintBoundingBox);
     SVGRenderSupport::intersectRepaintRectWithResources(this, m_repaintBoundingBox);
 }
 
index 84621f1..5be0366 100644 (file)
@@ -42,6 +42,7 @@ public:
     virtual void paint(PaintInfo&, const LayoutPoint&);
     virtual void setNeedsBoundariesUpdate() { m_needsBoundariesUpdate = true; }
     virtual bool didTransformToRootUpdate() { return false; }
+    bool isObjectBoundingBoxValid() const { return m_objectBoundingBoxValid; }
 
 protected:
     virtual RenderObjectChildList* virtualChildren() { return children(); }
@@ -76,6 +77,7 @@ protected:
 private:
     RenderObjectChildList m_children;
     FloatRect m_objectBoundingBox;
+    bool m_objectBoundingBoxValid;
     FloatRect m_strokeBoundingBox;
     FloatRect m_repaintBoundingBox;
     bool m_needsBoundariesUpdate : 1;
@@ -83,15 +85,13 @@ private:
   
 inline RenderSVGContainer* toRenderSVGContainer(RenderObject* object)
 {
-    // Note: isSVGContainer is also true for RenderSVGViewportContainer, which is not derived from this.
-    ASSERT(!object || (object->isSVGContainer() && strcmp(object->renderName(), "RenderSVGViewportContainer")));
+    ASSERT(!object || object->isSVGContainer());
     return static_cast<RenderSVGContainer*>(object);
 }
 
 inline const RenderSVGContainer* toRenderSVGContainer(const RenderObject* object)
 {
-    // Note: isSVGContainer is also true for RenderSVGViewportContainer, which is not derived from this.
-    ASSERT(!object || (object->isSVGContainer() && strcmp(object->renderName(), "RenderSVGViewportContainer")));
+    ASSERT(!object || object->isSVGContainer());
     return static_cast<const RenderSVGContainer*>(object);
 }
 
index c3e4292..fb06bf1 100644 (file)
@@ -57,6 +57,7 @@ namespace WebCore {
 
 RenderSVGRoot::RenderSVGRoot(SVGStyledElement* node)
     : RenderReplaced(node)
+    , m_objectBoundingBoxValid(false)
     , m_isLayoutSizeChanged(false)
     , m_needsBoundariesOrTransformUpdate(true)
 {
@@ -392,10 +393,11 @@ void RenderSVGRoot::mapLocalToContainer(RenderBoxModelObject* repaintContainer,
 void RenderSVGRoot::updateCachedBoundaries()
 {
     m_objectBoundingBox = FloatRect();
+    m_objectBoundingBoxValid = false;
     m_strokeBoundingBox = FloatRect();
     m_repaintBoundingBox = FloatRect();
 
-    SVGRenderSupport::computeContainerBoundingBoxes(this, m_objectBoundingBox, m_strokeBoundingBox, m_repaintBoundingBox);
+    SVGRenderSupport::computeContainerBoundingBoxes(this, m_objectBoundingBox, m_objectBoundingBoxValid, m_strokeBoundingBox, m_repaintBoundingBox);
     SVGRenderSupport::intersectRepaintRectWithResources(this, m_repaintBoundingBox);
     m_repaintBoundingBox.inflate(borderAndPaddingWidth());
 }
index 5a11dee..fc66af3 100644 (file)
@@ -104,6 +104,7 @@ private:
     RenderObjectChildList m_children;
     IntSize m_containerSize;
     FloatRect m_objectBoundingBox;
+    bool m_objectBoundingBoxValid;
     FloatRect m_strokeBoundingBox;
     FloatRect m_repaintBoundingBox;
     mutable AffineTransform m_localToParentTransform;
index 0f12ea7..faaf905 100644 (file)
@@ -85,32 +85,38 @@ void SVGRenderSupport::mapLocalToContainer(const RenderObject* object, RenderBox
     parent->mapLocalToContainer(repaintContainer, false, true, transformState, wasFixed);
 }
 
-void SVGRenderSupport::computeContainerBoundingBoxes(const RenderObject* container, FloatRect& objectBoundingBox, FloatRect& strokeBoundingBox, FloatRect& repaintBoundingBox)
+// Update a bounding box taking into account the validity of the other bounding box.
+static inline void updateObjectBoundingBox(FloatRect& objectBoundingBox, bool& objectBoundingBoxValid, RenderObject* other, FloatRect otherBoundingBox)
 {
-    bool isFirstChild = true;
+    bool otherValid = other->isSVGContainer() ? toRenderSVGContainer(other)->isObjectBoundingBoxValid() : true;
+    if (!otherValid)
+        return;
+
+    if (!objectBoundingBoxValid) {
+        objectBoundingBox = otherBoundingBox;
+        objectBoundingBoxValid = true;
+        return;
+    }
 
+    objectBoundingBox.uniteEvenIfEmpty(otherBoundingBox);
+}
+
+void SVGRenderSupport::computeContainerBoundingBoxes(const RenderObject* container, FloatRect& objectBoundingBox, bool& objectBoundingBoxValid, FloatRect& strokeBoundingBox, FloatRect& repaintBoundingBox)
+{
     for (RenderObject* current = container->firstChild(); current; current = current->nextSibling()) {
         if (current->isSVGHiddenContainer())
             continue;
 
         const AffineTransform& transform = current->localToParentTransform();
         if (transform.isIdentity()) {
-            if (isFirstChild)
-                objectBoundingBox = current->objectBoundingBox();
-            else
-                objectBoundingBox.uniteEvenIfEmpty(current->objectBoundingBox());
+            updateObjectBoundingBox(objectBoundingBox, objectBoundingBoxValid, current, current->objectBoundingBox());
             strokeBoundingBox.unite(current->strokeBoundingBox());
             repaintBoundingBox.unite(current->repaintRectInLocalCoordinates());
         } else {
-            if (isFirstChild)
-                objectBoundingBox = transform.mapRect(current->objectBoundingBox());
-            else
-                objectBoundingBox.uniteEvenIfEmpty(transform.mapRect(current->objectBoundingBox()));
+            updateObjectBoundingBox(objectBoundingBox, objectBoundingBoxValid, current, transform.mapRect(current->objectBoundingBox()));
             strokeBoundingBox.unite(transform.mapRect(current->strokeBoundingBox()));
             repaintBoundingBox.unite(transform.mapRect(current->repaintRectInLocalCoordinates()));
         }
-
-        isFirstChild = false;
     }
 }
 
index 51c9d1b..e25b12f 100644 (file)
@@ -57,7 +57,7 @@ public:
     // Determines whether the passed point lies in a clipping area
     static bool pointInClippingArea(RenderObject*, const FloatPoint&);
 
-    static void computeContainerBoundingBoxes(const RenderObject* container, FloatRect& objectBoundingBox, FloatRect& strokeBoundingBox, FloatRect& repaintBoundingBox);
+    static void computeContainerBoundingBoxes(const RenderObject* container, FloatRect& objectBoundingBox, bool& objectBoundingBoxValid, FloatRect& strokeBoundingBox, FloatRect& repaintBoundingBox);
     static bool paintInfoIntersectsRepaintRect(const FloatRect& localRepaintRect, const AffineTransform& localTransform, const PaintInfo&);
 
     // Important functions used by nearly all SVG renderers centralizing coordinate transformations / repaint rect calculations