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 b4b997fc40eae9a4716601d5dea6fedb79105bef..430653cc2cff96a7eaec35708d773566b5dd7ee4 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 5c0089497ec323fa7dba6777b7ec377bcdb6c296..715a19472b4e9efe55db8320a10affd227f37598 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 e16bfa33cb5826640772fa5ea205323f383627b0..a1fd8321d3ce46ee2e150a66631af817179d8beb 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 84621f1756ab716a12d7a8c896341c02dd7de859..5be0366386c45cb6516d15f7a3b2ffda1d3cff78 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 c3e4292ca2656cfc0c38d660dcfe22b0c3ed5981..fb06bf175cb80dabc2011b8051b319d8d30b9736 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 5a11dee3fb8228f0178c2d359bc78db0c3e2e806..fc66af34bcfed884c6d18a3d458a87e1953b9f7c 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 0f12ea7cab69a7d76216a874634f05b06d3bab44..faaf905a45b283db81538298434ab2519fc1cb83 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 51c9d1bc075fd681f516f894b09bb788ae3cec0d..e25b12f0601f888dd4de58353e18bc00b004fa62 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