Title: [111176] trunk
Revision
111176
Author
[email protected]
Date
2012-03-19 07:38:20 -0700 (Mon, 19 Mar 2012)

Log Message

inspector highlight of SVG root element with viewbox does not match dimensions of element
https://bugs.webkit.org/show_bug.cgi?id=78037

Patch by Max Vujovic <[email protected]> on 2012-03-19
Reviewed by Nikolas Zimmermann.

Source/WebCore:

Functions such as RenderBox::absoluteQuads and DOMNodeHighlighter::getOrDrawNodeHighlight
eventually call RenderSVGRoot::mapLocalToContainer, passing along local CSS box coordinates.

However, before this patch, RenderSVGRoot::mapLocalToContainer expected local SVG viewport
coordinates. This caused the inspector highlight to be incorrectly sized and positioned.

Now, RenderSVGRoot::mapLocalToContainer expects local CSS box coordinates, like other HTML
renderers.

Test: inspector/elements/highlight-svg-root.html

* dom/Element.cpp:
(WebCore::Element::getBoundingClientRect):
    Now, the SVG root element can use the code path for elements with CSS boxes to calculate
    its bounding client rect.

* rendering/svg/RenderSVGRoot.cpp:
(WebCore::RenderSVGRoot::mapLocalToContainer):
    RenderSVGRoot::mapLocalToContainer no longer needs to apply the
    localToBorderBoxTransform to convert from local SVG viewport coordinates to local CSS
    box coordinates. Now, it receives local CSS box coordinates.

* rendering/svg/RenderSVGRoot.h:
(WebCore::RenderSVGRoot::localToBorderBoxTransform):
    RenderSVGRoot::localToBorderBoxTransform has been exposed for child elements in the SVG
    namespace to use when mapping from local SVG viewport coordinates to local CSS box
    coordinates.

(RenderSVGRoot):
* rendering/svg/SVGRenderSupport.cpp:
(WebCore::SVGRenderSupport::mapLocalToContainer):
    Elements in the SVG namespace should now apply the localToBorderBoxTransform at the
    SVG/HTML boundary (aka RenderSVGRoot) when mapping their coordinates up the render tree.

* svg/SVGSVGElement.cpp:
(WebCore::SVGSVGElement::localCoordinateSpaceTransform):
    Same as above. This method is eventually used in the SVGLocatable::getScreenCTM
    calculation.

LayoutTests:

* inspector/elements/highlight-svg-root-expected.txt: Added.
* inspector/elements/highlight-svg-root.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (111175 => 111176)


--- trunk/LayoutTests/ChangeLog	2012-03-19 14:35:02 UTC (rev 111175)
+++ trunk/LayoutTests/ChangeLog	2012-03-19 14:38:20 UTC (rev 111176)
@@ -1,3 +1,13 @@
+2012-03-19  Max Vujovic  <[email protected]>
+
+        inspector highlight of SVG root element with viewbox does not match dimensions of element
+        https://bugs.webkit.org/show_bug.cgi?id=78037
+
+        Reviewed by Nikolas Zimmermann.
+
+        * inspector/elements/highlight-svg-root-expected.txt: Added.
+        * inspector/elements/highlight-svg-root.html: Added.
+
 2012-03-19  Keishi Hattori  <[email protected]>
 
         [chromium] Adding slow to full-screen-restrictions.html on WIN.

Added: trunk/LayoutTests/inspector/elements/highlight-svg-root-expected.txt (0 => 111176)


--- trunk/LayoutTests/inspector/elements/highlight-svg-root-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/inspector/elements/highlight-svg-root-expected.txt	2012-03-19 14:38:20 UTC (rev 111176)
@@ -0,0 +1,7 @@
+This test verifies the position and size of the highlight rectangles overlayed on an SVG root element.
+
+margin rect is 160 x 160 at (0, 0)
+border rect is 150 x 150 at (5, 5)
+padding rect is 130 x 130 at (15, 15)
+content rect is 100 x 100 at (30, 30)
+

Added: trunk/LayoutTests/inspector/elements/highlight-svg-root.html (0 => 111176)


--- trunk/LayoutTests/inspector/elements/highlight-svg-root.html	                        (rev 0)
+++ trunk/LayoutTests/inspector/elements/highlight-svg-root.html	2012-03-19 14:38:20 UTC (rev 111176)
@@ -0,0 +1,56 @@
+<!DOCTYPE html>
+<html>
+<head>
+
+<style>
+
+body {
+    margin: 0;
+}
+#svg-root {
+    margin: 5px;
+    border: solid 10px aqua;
+    padding: 15px;
+    background-color: blue;
+}
+
+</style>
+
+<script src=""
+<script src=""
+<script>
+
+function dumpInspectorHighlightRects()
+{
+    var rectNames = ["margin", "border", "padding", "content"];
+    var rects = window.internals.inspectorHighlightRects(document);
+    for (var i = 0; i < rects.length; i++)
+    {
+        var rectName = (i < rectNames.length ? rectNames[i] : "untitled");
+        var rect = rects.item(i);
+        var line = rectName + " rect is " + rect.width + " x " + rect.height + " at (" + rect.top + ", " + rect.left + ")<br/>";
+        document.getElementById("console").innerHTML += line;
+    }
+}
+
+function test()
+{
+    function nodeSelected(node)
+    {
+        RuntimeAgent.evaluate("dumpInspectorHighlightRects()", InspectorTest.completeTest);
+    }
+
+    InspectorTest.selectNodeWithId("svg-root", nodeSelected);
+}
+
+</script>
+</head>
+
+<body _onload_="runTest()">
+
+<svg id="svg-root" width="100" height="100" viewbox="0 0 50 50"></svg>
+<p id="description">This test verifies the position and size of the highlight rectangles overlayed on an SVG root element.</p>
+<div id="console"></div>
+
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (111175 => 111176)


--- trunk/Source/WebCore/ChangeLog	2012-03-19 14:35:02 UTC (rev 111175)
+++ trunk/Source/WebCore/ChangeLog	2012-03-19 14:38:20 UTC (rev 111176)
@@ -1,3 +1,49 @@
+2012-03-19  Max Vujovic  <[email protected]>
+
+        inspector highlight of SVG root element with viewbox does not match dimensions of element
+        https://bugs.webkit.org/show_bug.cgi?id=78037
+
+        Reviewed by Nikolas Zimmermann.
+
+        Functions such as RenderBox::absoluteQuads and DOMNodeHighlighter::getOrDrawNodeHighlight
+        eventually call RenderSVGRoot::mapLocalToContainer, passing along local CSS box coordinates.
+
+        However, before this patch, RenderSVGRoot::mapLocalToContainer expected local SVG viewport
+        coordinates. This caused the inspector highlight to be incorrectly sized and positioned.
+
+        Now, RenderSVGRoot::mapLocalToContainer expects local CSS box coordinates, like other HTML
+        renderers.
+
+        Test: inspector/elements/highlight-svg-root.html
+
+        * dom/Element.cpp:
+        (WebCore::Element::getBoundingClientRect):
+            Now, the SVG root element can use the code path for elements with CSS boxes to calculate
+            its bounding client rect.
+
+        * rendering/svg/RenderSVGRoot.cpp:
+        (WebCore::RenderSVGRoot::mapLocalToContainer):
+            RenderSVGRoot::mapLocalToContainer no longer needs to apply the
+            localToBorderBoxTransform to convert from local SVG viewport coordinates to local CSS
+            box coordinates. Now, it receives local CSS box coordinates.
+
+        * rendering/svg/RenderSVGRoot.h:
+        (WebCore::RenderSVGRoot::localToBorderBoxTransform):
+            RenderSVGRoot::localToBorderBoxTransform has been exposed for child elements in the SVG
+            namespace to use when mapping from local SVG viewport coordinates to local CSS box
+            coordinates.
+
+        (RenderSVGRoot):
+        * rendering/svg/SVGRenderSupport.cpp:
+        (WebCore::SVGRenderSupport::mapLocalToContainer):
+            Elements in the SVG namespace should now apply the localToBorderBoxTransform at the
+            SVG/HTML boundary (aka RenderSVGRoot) when mapping their coordinates up the render tree.
+
+        * svg/SVGSVGElement.cpp:
+        (WebCore::SVGSVGElement::localCoordinateSpaceTransform):
+            Same as above. This method is eventually used in the SVGLocatable::getScreenCTM
+            calculation.
+
 2012-03-19  Hironori Bono  <[email protected]>
 
         Add a copy constructor to CollapsedBorderValue

Modified: trunk/Source/WebCore/dom/Element.cpp (111175 => 111176)


--- trunk/Source/WebCore/dom/Element.cpp	2012-03-19 14:35:02 UTC (rev 111175)
+++ trunk/Source/WebCore/dom/Element.cpp	2012-03-19 14:38:20 UTC (rev 111176)
@@ -547,7 +547,7 @@
 
     Vector<FloatQuad> quads;
 #if ENABLE(SVG)
-    if (isSVGElement() && renderer()) {
+    if (isSVGElement() && renderer() && !renderer()->isSVGRoot()) {
         // Get the bounding rectangle from the SVG model.
         SVGElement* svgElement = static_cast<SVGElement*>(this);
         FloatRect localRect;

Modified: trunk/Source/WebCore/rendering/svg/RenderSVGRoot.cpp (111175 => 111176)


--- trunk/Source/WebCore/rendering/svg/RenderSVGRoot.cpp	2012-03-19 14:35:02 UTC (rev 111175)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGRoot.cpp	2012-03-19 14:38:20 UTC (rev 111176)
@@ -365,13 +365,14 @@
     repaintRect = rect;
 }
 
+// This method expects local CSS box coordinates.
+// Callers with local SVG viewport coordinates should first apply the localToBorderBoxTransform
+// to convert from SVG viewport coordinates to local CSS box coordinates.
 void RenderSVGRoot::mapLocalToContainer(RenderBoxModelObject* repaintContainer, bool fixed, bool useTransforms, TransformState& transformState, bool* wasFixed) const
 {
     ASSERT(!fixed); // We should have no fixed content in the SVG rendering tree.
     ASSERT(useTransforms); // mapping a point through SVG w/o respecting trasnforms is useless.
 
-    // Transform to our border box and let RenderBox transform the rest of the way.
-    transformState.applyTransform(m_localToBorderBoxTransform);
     RenderReplaced::mapLocalToContainer(repaintContainer, fixed, useTransforms, transformState, wasFixed);
 }
 

Modified: trunk/Source/WebCore/rendering/svg/RenderSVGRoot.h (111175 => 111176)


--- trunk/Source/WebCore/rendering/svg/RenderSVGRoot.h	2012-03-19 14:35:02 UTC (rev 111175)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGRoot.h	2012-03-19 14:38:20 UTC (rev 111176)
@@ -55,6 +55,9 @@
 
     virtual bool hasRelativeDimensions() const;
 
+    // localToBorderBoxTransform maps local SVG viewport coordinates to local CSS box coordinates.  
+    const AffineTransform& localToBorderBoxTransform() const { return m_localToBorderBoxTransform; }
+
 private:
     virtual RenderObjectChildList* virtualChildren() { return children(); }
     virtual const RenderObjectChildList* virtualChildren() const { return children(); }

Modified: trunk/Source/WebCore/rendering/svg/SVGRenderSupport.cpp (111175 => 111176)


--- trunk/Source/WebCore/rendering/svg/SVGRenderSupport.cpp	2012-03-19 14:35:02 UTC (rev 111175)
+++ trunk/Source/WebCore/rendering/svg/SVGRenderSupport.cpp	2012-03-19 14:38:20 UTC (rev 111176)
@@ -73,7 +73,16 @@
 void SVGRenderSupport::mapLocalToContainer(const RenderObject* object, RenderBoxModelObject* repaintContainer, TransformState& transformState, bool* wasFixed)
 {
     transformState.applyTransform(object->localToParentTransform());
-    object->parent()->mapLocalToContainer(repaintContainer, false, true, transformState, wasFixed);
+
+    RenderObject* parent = object->parent();
+    
+    // At the SVG/HTML boundary (aka RenderSVGRoot), we apply the localToBorderBoxTransform 
+    // to map an element from SVG viewport coordinates to CSS box coordinates.
+    // RenderSVGRoot's mapLocalToContainer method expects CSS box coordinates.
+    if (parent->isSVGRoot())
+        transformState.applyTransform(toRenderSVGRoot(parent)->localToBorderBoxTransform());
+    
+    parent->mapLocalToContainer(repaintContainer, false, true, transformState, wasFixed);
 }
 
 void SVGRenderSupport::computeContainerBoundingBoxes(const RenderObject* container, FloatRect& objectBoundingBox, FloatRect& strokeBoundingBox, FloatRect& repaintBoundingBox)

Modified: trunk/Source/WebCore/svg/SVGSVGElement.cpp (111175 => 111176)


--- trunk/Source/WebCore/svg/SVGSVGElement.cpp	2012-03-19 14:35:02 UTC (rev 111175)
+++ trunk/Source/WebCore/svg/SVGSVGElement.cpp	2012-03-19 14:38:20 UTC (rev 111176)
@@ -439,13 +439,20 @@
         transform.translate(x().value(lengthContext), y().value(lengthContext));
     } else if (mode == SVGLocatable::ScreenScope) {
         if (RenderObject* renderer = this->renderer()) {
+            FloatPoint location;
+            
+            // At the SVG/HTML boundary (aka RenderSVGRoot), we apply the localToBorderBoxTransform 
+            // to map an element from SVG viewport coordinates to CSS box coordinates.
+            // RenderSVGRoot's localToAbsolute method expects CSS box coordinates.
+            if (renderer->isSVGRoot())
+                location = toRenderSVGRoot(renderer)->localToBorderBoxTransform().mapPoint(location);
+            
             // Translate in our CSS parent coordinate space
             // FIXME: This doesn't work correctly with CSS transforms.
-            FloatPoint location = renderer->localToAbsolute(FloatPoint(), false, true);
+            location = renderer->localToAbsolute(location, false, true);
 
-            // Be careful here! localToAbsolute() includes the x/y offset coming from the viewBoxToViewTransform(), because
-            // RenderSVGRoot::localToBorderBoxTransform() (called through mapLocalToContainer(), called from localToAbsolute())
-            // also takes the viewBoxToViewTransform() into account, so we have to subtract it here (original cause of bug #27183)
+            // Be careful here! localToBorderBoxTransform() included the x/y offset coming from the viewBoxToViewTransform(),
+            // so we have to subtract it here (original cause of bug #27183)
             transform.translate(location.x() - viewBoxTransform.e(), location.y() - viewBoxTransform.f());
 
             // Respect scroll offset.
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to