<rdar://problem/9973489> REGRESSION (r66599): -[DOMNode boundingBox] returns the...
authormitz@apple.com <mitz@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 4 Oct 2011 00:10:38 +0000 (00:10 +0000)
committermitz@apple.com <mitz@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 4 Oct 2011 00:10:38 +0000 (00:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=69305

Reviewed by Simon Fraser.

Source/WebCore:

Test: svg/custom/boundingBox.html

Rather than asserting and returning the zero rect, take the transform-aware code path for computing SVG
bounding rects.

* rendering/svg/RenderSVGForeignObject.cpp:
(WebCore::RenderSVGForeignObject::mapLocalToContainer): Updated for change to SVGRenderSupport::mapLocalToContainer().
* rendering/svg/RenderSVGInline.cpp:
(WebCore::RenderSVGInline::mapLocalToContainer): Ditto.
* rendering/svg/RenderSVGModelObject.cpp:
(WebCore::RenderSVGModelObject::mapLocalToContainer): Ditto.
(WebCore::RenderSVGModelObject::absoluteRects): Replaced an incorrect assertion with code to approximate the bounding
box.
* rendering/svg/RenderSVGText.cpp:
(WebCore::RenderSVGText::mapLocalToContainer): Updated for change to SVGRenderSupport::mapLocalToContainer().
* rendering/svg/SVGRenderSupport.cpp:
(WebCore::SVGRenderSupport::mapLocalToContainer): Removed the fixed and useTransform boolean parameters.
* rendering/svg/SVGRenderSupport.h:

LayoutTests:

* svg/custom/boundingBox-expected.txt: Added.
* svg/custom/boundingBox.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/svg/custom/boundingBox-expected.txt [new file with mode: 0644]
LayoutTests/svg/custom/boundingBox.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/svg/RenderSVGForeignObject.cpp
Source/WebCore/rendering/svg/RenderSVGInline.cpp
Source/WebCore/rendering/svg/RenderSVGModelObject.cpp
Source/WebCore/rendering/svg/RenderSVGText.cpp
Source/WebCore/rendering/svg/SVGRenderSupport.cpp
Source/WebCore/rendering/svg/SVGRenderSupport.h

index dbd51eb3d2a58e991ae64aa58554603ba751d0e5..ad4e67444bdc3ffee30ebc8e32833cfa83c5ebde 100644 (file)
@@ -1,3 +1,13 @@
+2011-10-03  Dan Bernstein  <mitz@apple.com>
+
+        <rdar://problem/9973489> REGRESSION (r66599): -[DOMNode boundingBox] returns the zero rect for SVG elements
+        https://bugs.webkit.org/show_bug.cgi?id=69305
+
+        Reviewed by Simon Fraser.
+
+        * svg/custom/boundingBox-expected.txt: Added.
+        * svg/custom/boundingBox.html: Added.
+
 2011-10-03  Ryosuke Niwa  <rniwa@webkit.org>
 
         REGRESSION(r94274): cloned text input loses value
diff --git a/LayoutTests/svg/custom/boundingBox-expected.txt b/LayoutTests/svg/custom/boundingBox-expected.txt
new file mode 100644 (file)
index 0000000..79af15e
--- /dev/null
@@ -0,0 +1,3 @@
+Test that -[DOMNode boundingBox] returns reasonable results for SVG elements.
+
+PASS
diff --git a/LayoutTests/svg/custom/boundingBox.html b/LayoutTests/svg/custom/boundingBox.html
new file mode 100644 (file)
index 0000000..64220da
--- /dev/null
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.layoutTestController)
+    window.layoutTestController.dumpAsText()
+
+function runTest()
+{
+    var target = document.getElementById("target");
+    if (window.internals) {
+        var boundingBox = internals.boundingBox(target);
+        var boundingClientRect = target.getBoundingClientRect();
+        var rectsAreEqual = (boundingBox.width === boundingClientRect.width
+            && boundingBox.height === boundingClientRect.height
+            && boundingBox.left === boundingClientRect.left
+            && boundingBox.top === boundingClientRect.top);
+
+        document.getElementById("result").innerHTML = rectsAreEqual ? "PASS" : "FAIL: boundingBox: " + boundingBox.width + " x " + boundingBox.height + " @ (" + boundingBox.left + ", " + boundingBox.top + "), boundingClientRect: "
+            + boundingClientRect.width + " x " + boundingClientRect.height + " @ (" + boundingClientRect.left + ", " + boundingClientRect.top + ")";
+    }
+}
+</script>
+</head>
+<body onload="runTest()">
+<div style="height: 200px;"></div>
+<svg width=400 height=400>
+    <image width=200 height=200 x=50 y=50 id="target" />
+</svg>
+<p>
+    Test that -[DOMNode boundingBox] returns reasonable results for SVG elements.
+</p>
+<p id="result">This test can only be run in DumpRenderTree.</p>
+</body>
+</html>
index 57daa43bdee4f4bbf8d7d357d07beeb7939e3538..ef142158ec665d9abcc60ddd5b9038664822a628 100644 (file)
@@ -1,3 +1,29 @@
+2011-10-03  Dan Bernstein  <mitz@apple.com>
+
+        <rdar://problem/9973489> REGRESSION (r66599): -[DOMNode boundingBox] returns the zero rect for SVG elements
+        https://bugs.webkit.org/show_bug.cgi?id=69305
+
+        Reviewed by Simon Fraser.
+
+        Test: svg/custom/boundingBox.html
+
+        Rather than asserting and returning the zero rect, take the transform-aware code path for computing SVG
+        bounding rects.
+
+        * rendering/svg/RenderSVGForeignObject.cpp:
+        (WebCore::RenderSVGForeignObject::mapLocalToContainer): Updated for change to SVGRenderSupport::mapLocalToContainer().
+        * rendering/svg/RenderSVGInline.cpp:
+        (WebCore::RenderSVGInline::mapLocalToContainer): Ditto.
+        * rendering/svg/RenderSVGModelObject.cpp:
+        (WebCore::RenderSVGModelObject::mapLocalToContainer): Ditto.
+        (WebCore::RenderSVGModelObject::absoluteRects): Replaced an incorrect assertion with code to approximate the bounding
+        box.
+        * rendering/svg/RenderSVGText.cpp:
+        (WebCore::RenderSVGText::mapLocalToContainer): Updated for change to SVGRenderSupport::mapLocalToContainer().
+        * rendering/svg/SVGRenderSupport.cpp:
+        (WebCore::SVGRenderSupport::mapLocalToContainer): Removed the fixed and useTransform boolean parameters.
+        * rendering/svg/SVGRenderSupport.h:
+
 2011-10-03  Michael Nordman  <michaeln@google.com>
 
         A little more WebSQLDatabase thread safety.
index 01049a2ba91ff79b604bb11200ebcc749e982c70..08842714ce39323191d59e9d6a3a1691a7351046 100644 (file)
@@ -160,12 +160,9 @@ bool RenderSVGForeignObject::nodeAtPoint(const HitTestRequest&, HitTestResult&,
     return false;
 }
 
-void RenderSVGForeignObject::mapLocalToContainer(RenderBoxModelObject* repaintContainer, bool fixed, bool useTransforms, TransformState& transformState, bool* wasFixed) const
+void RenderSVGForeignObject::mapLocalToContainer(RenderBoxModelObject* repaintContainer, bool /* fixed */, bool /* useTransforms */, TransformState& transformState, bool* wasFixed) const
 {
-    // When crawling up the hierachy starting from foreignObject child content, useTransforms may not be set to true.
-    if (!useTransforms)
-        useTransforms = true;
-    SVGRenderSupport::mapLocalToContainer(this, repaintContainer, fixed, useTransforms, transformState, wasFixed);
+    SVGRenderSupport::mapLocalToContainer(this, repaintContainer, transformState, wasFixed);
 }
 
 }
index 09770efbf027fac9eba7dbc7c4ea437f6aa010b9..f4a29e284d4f3ba579bb3d8bc578f607158bcdb6 100644 (file)
@@ -77,9 +77,9 @@ void RenderSVGInline::computeRectForRepaint(RenderBoxModelObject* repaintContain
     SVGRenderSupport::computeRectForRepaint(this, repaintContainer, repaintRect, fixed);
 }
 
-void RenderSVGInline::mapLocalToContainer(RenderBoxModelObject* repaintContainer, bool useTransforms, bool fixed, TransformState& transformState, bool* wasFixed) const
+void RenderSVGInline::mapLocalToContainer(RenderBoxModelObject* repaintContainer, bool /* useTransforms */, bool /* fixed */, TransformState& transformState, bool* wasFixed) const
 {
-    SVGRenderSupport::mapLocalToContainer(this, repaintContainer, useTransforms, fixed, transformState, wasFixed);
+    SVGRenderSupport::mapLocalToContainer(this, repaintContainer, transformState, wasFixed);
 }
 
 void RenderSVGInline::absoluteQuads(Vector<FloatQuad>& quads, bool* wasFixed)
index 14c1db8e0f93eaf558aeec892a872323cd10d723..c2e3fdff805d7788d5efe7cdbbfc15a7fe3c66e9 100644 (file)
@@ -53,9 +53,9 @@ void RenderSVGModelObject::computeRectForRepaint(RenderBoxModelObject* repaintCo
     SVGRenderSupport::computeRectForRepaint(this, repaintContainer, repaintRect, fixed);
 }
 
-void RenderSVGModelObject::mapLocalToContainer(RenderBoxModelObject* repaintContainer, bool fixed, bool useTransforms, TransformState& transformState, bool* wasFixed) const
+void RenderSVGModelObject::mapLocalToContainer(RenderBoxModelObject* repaintContainer, bool /* fixed */, bool /* useTransforms */, TransformState& transformState, bool* wasFixed) const
 {
-    SVGRenderSupport::mapLocalToContainer(this, repaintContainer, fixed, useTransforms, transformState, wasFixed);
+    SVGRenderSupport::mapLocalToContainer(this, repaintContainer, transformState, wasFixed);
 }
 
 // Copied from RenderBox, this method likely requires further refactoring to work easily for both SVG and CSS Box Model content.
@@ -70,10 +70,11 @@ LayoutRect RenderSVGModelObject::outlineBoundsForRepaint(RenderBoxModelObject* r
     return containerRelativeQuad.enclosingBoundingBox();
 }
 
-void RenderSVGModelObject::absoluteRects(Vector<LayoutRect>&, const LayoutPoint&)
+void RenderSVGModelObject::absoluteRects(Vector<LayoutRect>& rects, const LayoutPoint& accumulatedOffset)
 {
-    // This code path should never be taken for SVG, as we're assuming useTransforms=true everywhere, absoluteQuads should be used.
-    ASSERT_NOT_REACHED();
+    LayoutRect rect = enclosingLayoutRect(strokeBoundingBox());
+    rect.moveBy(accumulatedOffset);
+    rects.append(rect);
 }
 
 void RenderSVGModelObject::absoluteQuads(Vector<FloatQuad>& quads, bool* wasFixed)
index af439b5d38f084f02a921b2bafa0d9b1abd47081..57b4f574ea00205d334a0067d8ff66e35455ec28 100644 (file)
@@ -93,9 +93,9 @@ void RenderSVGText::computeRectForRepaint(RenderBoxModelObject* repaintContainer
     SVGRenderSupport::computeRectForRepaint(this, repaintContainer, repaintRect, fixed);
 }
 
-void RenderSVGText::mapLocalToContainer(RenderBoxModelObject* repaintContainer, bool fixed, bool useTransforms, TransformState& transformState, bool* wasFixed) const
+void RenderSVGText::mapLocalToContainer(RenderBoxModelObject* repaintContainer, bool /* fixed */, bool /* useTransforms */, TransformState& transformState, bool* wasFixed) const
 {
-    SVGRenderSupport::mapLocalToContainer(this, repaintContainer, fixed, useTransforms, transformState, wasFixed);
+    SVGRenderSupport::mapLocalToContainer(this, repaintContainer, transformState, wasFixed);
 }
 
 static inline void recursiveUpdateScaledFont(RenderObject* start)
index 00792196d5a367bcff8ae477bdd1493804451412..2bce6a2fd65e2db127de3ba44a0b4cd01987ef08 100644 (file)
@@ -70,12 +70,10 @@ void SVGRenderSupport::computeRectForRepaint(const RenderObject* object, RenderB
     object->parent()->computeRectForRepaint(repaintContainer, repaintRect, fixed);
 }
 
-void SVGRenderSupport::mapLocalToContainer(const RenderObject* object, RenderBoxModelObject* repaintContainer, bool fixed, bool useTransforms, TransformState& transformState, bool* wasFixed)
+void SVGRenderSupport::mapLocalToContainer(const RenderObject* object, RenderBoxModelObject* repaintContainer, TransformState& transformState, bool* wasFixed)
 {
-    ASSERT(!fixed); // We should have no fixed content in the SVG rendering tree.
-    ASSERT(useTransforms); // Mapping a point through SVG w/o respecting transforms is useless.
     transformState.applyTransform(object->localToParentTransform());
-    object->parent()->mapLocalToContainer(repaintContainer, fixed, useTransforms, transformState, wasFixed);
+    object->parent()->mapLocalToContainer(repaintContainer, false, true, transformState, wasFixed);
 }
 
 bool SVGRenderSupport::prepareToRenderSVGContent(RenderObject* object, PaintInfo& paintInfo)
index 8bd795932457ed49e5dbfa7df207d2b114113438..ac90a66a841f277a9f508fe12c02f962e1492cf6 100644 (file)
@@ -63,7 +63,7 @@ public:
     // Important functions used by nearly all SVG renderers centralizing coordinate transformations / repaint rect calculations
     static LayoutRect clippedOverflowRectForRepaint(const RenderObject*, RenderBoxModelObject* repaintContainer);
     static void computeRectForRepaint(const RenderObject*, RenderBoxModelObject* repaintContainer, LayoutRect&, bool fixed);
-    static void mapLocalToContainer(const RenderObject*, RenderBoxModelObject* repaintContainer, bool useTransforms, bool fixed, TransformState&, bool* wasFixed = 0);
+    static void mapLocalToContainer(const RenderObject*, RenderBoxModelObject* repaintContainer, TransformState&, bool* wasFixed = 0);
 
     // Shared between SVG renderers and resources.
     static void applyStrokeStyleToContext(GraphicsContext*, const RenderStyle*, const RenderObject*);