- 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.