Diff
Modified: trunk/LayoutTests/ChangeLog (99459 => 99460)
--- trunk/LayoutTests/ChangeLog 2011-11-07 20:37:06 UTC (rev 99459)
+++ trunk/LayoutTests/ChangeLog 2011-11-07 20:40:24 UTC (rev 99460)
@@ -1,3 +1,17 @@
+2011-11-07 Tim Horton <[email protected]>
+
+ getBBox() on a SVGPathElement with curves incorrectly includes control points
+ https://bugs.webkit.org/show_bug.cgi?id=53512
+ <rdar://problem/9861154>
+
+ Reviewed by Oliver Hunt.
+
+ Add a test that ensures that getBBox does not include control points in the rect it returns.
+
+ * platform/chromium/test_expectations.txt:
+ * svg/custom/getBBox-path-expected.txt: Added.
+ * svg/custom/getBBox-path.svg: Added.
+
2011-11-07 [email protected] <[email protected]>
Web Inspector: Suggest box should consume enter key pressed event.
Modified: trunk/LayoutTests/platform/chromium/test_expectations.txt (99459 => 99460)
--- trunk/LayoutTests/platform/chromium/test_expectations.txt 2011-11-07 20:37:06 UTC (rev 99459)
+++ trunk/LayoutTests/platform/chromium/test_expectations.txt 2011-11-07 20:40:24 UTC (rev 99460)
@@ -3899,3 +3899,6 @@
BUGWK59552 SNOWLEOPARD DEBUG : fast/frames/set-parent-src-synchronously.html = PASS CRASH
BUGWK71658 MAC DEBUG : fast/frames/sandboxed-iframe-navigation-targetlink.html = PASS TIMEOUT
+
+// Introduced due to BUGWK53512, fails under Skia right now
+BUGWK65939 : svg/custom/getBBox-path.svg = TEXT
Added: trunk/LayoutTests/svg/custom/getBBox-path-expected.txt (0 => 99460)
--- trunk/LayoutTests/svg/custom/getBBox-path-expected.txt (rev 0)
+++ trunk/LayoutTests/svg/custom/getBBox-path-expected.txt 2011-11-07 20:40:24 UTC (rev 99460)
@@ -0,0 +1 @@
+100 37.5 PASS
Added: trunk/LayoutTests/svg/custom/getBBox-path.svg (0 => 99460)
--- trunk/LayoutTests/svg/custom/getBBox-path.svg (rev 0)
+++ trunk/LayoutTests/svg/custom/getBBox-path.svg 2011-11-07 20:40:24 UTC (rev 99460)
@@ -0,0 +1,21 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<svg xmlns="http://www.w3.org/2000/svg" _onload_="init()">
+ <script type="text/_javascript_">
+ <![CDATA[
+ function init()
+ {
+ if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+ var txt = document.getElementById("text");
+ size = document.getElementById("shape").getBBox();
+ var passState = "FAIL";
+ if(size.width == 100 && size.height == 37.5)
+ passState = "PASS";
+ var textNode = document.createTextNode(size.width + " " + size.height + " " + passState);
+ txt.appendChild(textNode);
+ }
+ ]]>
+ </script>
+ <path id="shape" d="M100,100 C125,150 175,150 200,100 z" />
+ <text id="text" x="50" y="50" />
+</svg>
Modified: trunk/Source/WebCore/ChangeLog (99459 => 99460)
--- trunk/Source/WebCore/ChangeLog 2011-11-07 20:37:06 UTC (rev 99459)
+++ trunk/Source/WebCore/ChangeLog 2011-11-07 20:40:24 UTC (rev 99460)
@@ -1,3 +1,42 @@
+2011-11-07 Tim Horton <[email protected]>
+
+ getBBox() on a SVGPathElement with curves incorrectly includes control points
+ https://bugs.webkit.org/show_bug.cgi?id=53512
+ <rdar://problem/9861154>
+
+ Reviewed by Oliver Hunt.
+
+ Split Path::boundingRect() into two, adding Path::fastBoundingRect()
+ for a rough estimate of the bounding rect (always equal to or larger
+ than boundingRect()). fastBoundingRect() currently falls back to
+ boundingRect() for all ports besides CG, though in most cases
+ (on a port-by-port basis) the current implementation of boundingRect()
+ will need to become fastBoundingRect(), and a new, more accurate method will
+ be implemented for boundingRect().
+
+ All previous callers of boundingRect() are transitioned to using fastBoundingRect()
+ except SVGPathElement::getBBox, which wants an accurate bounding box.
+
+ The CoreGraphics implementation of Path::boundingRect() called
+ CGPathGetBoundingBox, which includes the path's control points in its
+ calculations. Snow Leopard added CGPathGetPathBoundingBox, which
+ finds the bounding box of only points within the path, and does not
+ include control points. On Snow Leopard and above, we now use the latter.
+
+ Test: svg/custom/getBBox-path.svg
+
+ * html/HTMLAreaElement.cpp:
+ * html/canvas/CanvasRenderingContext2D.cpp:
+ * platform/graphics/Path.cpp:
+ * platform/graphics/Path.h:
+ * platform/graphics/cg/GraphicsContextCG.cpp:
+ * platform/graphics/cg/PathCG.cpp:
+ (WebCore::Path::boundingRect):
+ * rendering/RenderObject.h:
+ * rendering/svg/RenderSVGPath.cpp:
+ * svg/SVGPathElement.cpp:
+ * svg/SVGPathElement.h:
+
2011-11-07 Vsevolod Vlasov <[email protected]>
Web Inspector: Suggest box should be open immediately if forced by Ctrl+Space.
Modified: trunk/Source/WebCore/html/HTMLAreaElement.cpp (99459 => 99460)
--- trunk/Source/WebCore/html/HTMLAreaElement.cpp 2011-11-07 20:37:06 UTC (rev 99459)
+++ trunk/Source/WebCore/html/HTMLAreaElement.cpp 2011-11-07 20:40:24 UTC (rev 99460)
@@ -120,7 +120,7 @@
LayoutRect HTMLAreaElement::computeRect(RenderObject* obj) const
{
- return enclosingLayoutRect(computePath(obj).boundingRect());
+ return enclosingLayoutRect(computePath(obj).fastBoundingRect());
}
Path HTMLAreaElement::getRegion(const LayoutSize& size) const
Modified: trunk/Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp (99459 => 99460)
--- trunk/Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp 2011-11-07 20:37:06 UTC (rev 99459)
+++ trunk/Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp 2011-11-07 20:40:24 UTC (rev 99460)
@@ -771,7 +771,7 @@
if (m_path.isEmpty())
return;
- FloatRect boundRect = m_path.boundingRect();
+ FloatRect boundRect = m_path.fastBoundingRect();
if (boundRect.width() || boundRect.height())
m_path.closeSubpath();
}
@@ -956,7 +956,7 @@
didDrawEntireCanvas();
} else {
c->fillPath(m_path);
- didDraw(m_path.boundingRect());
+ didDraw(m_path.fastBoundingRect());
}
}
@@ -974,7 +974,7 @@
return;
if (!m_path.isEmpty()) {
- FloatRect dirtyRect = m_path.boundingRect();
+ FloatRect dirtyRect = m_path.fastBoundingRect();
// Fast approximation of the stroke's bounding rect.
// This yields a slightly oversized rect but is very fast
// compared to Path::strokeBoundingRect().
@@ -1580,7 +1580,7 @@
IntRect canvasRect(0, 0, canvas()->width(), canvas()->height());
canvasRect = canvas()->baseTransform().mapRect(canvasRect);
Path path = transformAreaToDevice(area);
- IntRect bufferRect = enclosingIntRect(path.boundingRect());
+ IntRect bufferRect = enclosingIntRect(path.fastBoundingRect());
IntPoint originalLocation = bufferRect.location();
bufferRect.intersect(canvasRect);
if (croppedOffset)
Modified: trunk/Source/WebCore/platform/graphics/Path.cpp (99459 => 99460)
--- trunk/Source/WebCore/platform/graphics/Path.cpp 2011-11-07 20:37:06 UTC (rev 99459)
+++ trunk/Source/WebCore/platform/graphics/Path.cpp 2011-11-07 20:40:24 UTC (rev 99460)
@@ -163,4 +163,11 @@
closeSubpath();
}
+#if !USE(CG)
+FloatRect Path::fastBoundingRect() const
+{
+ return boundingRect();
}
+#endif
+
+}
Modified: trunk/Source/WebCore/platform/graphics/Path.h (99459 => 99460)
--- trunk/Source/WebCore/platform/graphics/Path.h 2011-11-07 20:37:06 UTC (rev 99459)
+++ trunk/Source/WebCore/platform/graphics/Path.h 2011-11-07 20:40:24 UTC (rev 99460)
@@ -105,7 +105,10 @@
bool contains(const FloatPoint&, WindRule rule = RULE_NONZERO) const;
bool strokeContains(StrokeStyleApplier*, const FloatPoint&) const;
+ // fastBoundingRect() should equal or contain boundingRect(); boundingRect()
+ // should perfectly bound the points within the path.
FloatRect boundingRect() const;
+ FloatRect fastBoundingRect() const;
FloatRect strokeBoundingRect(StrokeStyleApplier* = 0) const;
float length() const;
Modified: trunk/Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp (99459 => 99460)
--- trunk/Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp 2011-11-07 20:37:06 UTC (rev 99459)
+++ trunk/Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp 2011-11-07 20:40:24 UTC (rev 99460)
@@ -627,7 +627,7 @@
if (m_state.fillGradient) {
if (hasShadow()) {
- FloatRect rect = path.boundingRect();
+ FloatRect rect = path.fastBoundingRect();
CGLayerRef layer = CGLayerCreateWithContext(context, CGSizeMake(rect.width(), rect.height()), 0);
CGContextRef layerContext = CGLayerGetContext(layer);
@@ -682,7 +682,7 @@
if (m_state.strokeGradient) {
if (hasShadow()) {
- FloatRect rect = path.boundingRect();
+ FloatRect rect = path.fastBoundingRect();
float lineWidth = strokeThickness();
float doubleLineWidth = lineWidth * 2;
float layerWidth = ceilf(rect.width() + doubleLineWidth);
Modified: trunk/Source/WebCore/platform/graphics/cg/PathCG.cpp (99459 => 99460)
--- trunk/Source/WebCore/platform/graphics/cg/PathCG.cpp 2011-11-07 20:37:06 UTC (rev 99459)
+++ trunk/Source/WebCore/platform/graphics/cg/PathCG.cpp 2011-11-07 20:40:24 UTC (rev 99460)
@@ -124,7 +124,7 @@
bool Path::contains(const FloatPoint &point, WindRule rule) const
{
- if (!boundingRect().contains(point))
+ if (!fastBoundingRect().contains(point))
return false;
// CGPathContainsPoint returns false for non-closed paths, as a work-around, we copy and close the path first. Radar 4758998 asks for a better CG API to use
@@ -163,9 +163,20 @@
FloatRect Path::boundingRect() const
{
+ // CGPathGetBoundingBox includes the path's control points, CGPathGetPathBoundingBox
+ // does not, but only exists on 10.6 and above.
+#if !defined(BUILDING_ON_LEOPARD)
+ return CGPathGetPathBoundingBox(m_path);
+#else
return CGPathGetBoundingBox(m_path);
+#endif
}
+FloatRect Path::fastBoundingRect() const
+{
+ return CGPathGetBoundingBox(m_path);
+}
+
FloatRect Path::strokeBoundingRect(StrokeStyleApplier* applier) const
{
CGContextRef context = scratchContext();
Modified: trunk/Source/WebCore/rendering/RenderObject.h (99459 => 99460)
--- trunk/Source/WebCore/rendering/RenderObject.h 2011-11-07 20:37:06 UTC (rev 99459)
+++ trunk/Source/WebCore/rendering/RenderObject.h 2011-11-07 20:40:24 UTC (rev 99460)
@@ -421,7 +421,7 @@
virtual void setNeedsBoundariesUpdate();
// Per SVG 1.1 objectBoundingBox ignores clipping, masking, filter effects, opacity and stroke-width.
- // This is used for all computation of objectBoundingBox relative units and by SVGLocateable::getBBox().
+ // This is used for all computation of objectBoundingBox relative units and by SVGLocatable::getBBox().
// NOTE: Markers are not specifically ignored here by SVG 1.1 spec, but we ignore them
// since stroke-width is ignored (and marker size can depend on stroke-width).
// objectBoundingBox is returned local coordinates.
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGPath.cpp (99459 => 99460)
--- trunk/Source/WebCore/rendering/svg/RenderSVGPath.cpp 2011-11-07 20:37:06 UTC (rev 99459)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGPath.cpp 2011-11-07 20:40:24 UTC (rev 99460)
@@ -357,7 +357,7 @@
}
// Cache _unclipped_ fill bounding box, used for calculations in resources
- m_fillBoundingBox = m_path.boundingRect();
+ m_fillBoundingBox = m_path.fastBoundingRect();
// Spec(11.4): Any zero length subpath shall not be stroked if the ‘stroke-linecap’ property has a value of butt
// but shall be stroked if the ‘stroke-linecap’ property has a value of round or square
Modified: trunk/Source/WebCore/svg/SVGPathElement.cpp (99459 => 99460)
--- trunk/Source/WebCore/svg/SVGPathElement.cpp 2011-11-07 20:37:06 UTC (rev 99459)
+++ trunk/Source/WebCore/svg/SVGPathElement.cpp 2011-11-07 20:40:24 UTC (rev 99460)
@@ -76,6 +76,7 @@
: SVGStyledTransformableElement(tagName, document)
, m_pathByteStream(SVGPathByteStream::create())
, m_pathSegList(PathSegUnalteredRole)
+ , m_cachedBBoxRectIsValid(false)
{
ASSERT(hasTagName(SVGNames::pathTag));
registerAnimatedPropertiesForSVGPathElement();
@@ -266,6 +267,7 @@
SVGPathParserFactory* factory = SVGPathParserFactory::self();
factory->buildSVGPathSegListFromByteStream(m_pathByteStream.get(), this, newList, UnalteredParsing);
m_pathSegList.value = newList;
+ m_cachedBBoxRectIsValid = false;
}
if (renderer)
@@ -350,6 +352,8 @@
}
invalidateSVGAttributes();
+
+ m_cachedBBoxRectIsValid = false;
RenderSVGPath* renderer = static_cast<RenderSVGPath*>(this->renderer());
if (!renderer)
@@ -359,6 +363,25 @@
RenderSVGResource::markForLayoutAndParentResourceInvalidation(renderer);
}
+FloatRect SVGPathElement::getBBox(StyleUpdateStrategy styleUpdateStrategy)
+{
+ if (styleUpdateStrategy == AllowStyleUpdate)
+ this->document()->updateLayoutIgnorePendingStylesheets();
+
+ RenderSVGPath* renderer = static_cast<RenderSVGPath*>(this->renderer());
+
+ // FIXME: Eventually we should support getBBox for detached elements.
+ if (!renderer)
+ return FloatRect();
+
+ if (!m_cachedBBoxRectIsValid) {
+ m_cachedBBoxRect = renderer->path().boundingRect();
+ m_cachedBBoxRectIsValid = true;
+ }
+
+ return m_cachedBBoxRect;
}
+}
+
#endif // ENABLE(SVG)
Modified: trunk/Source/WebCore/svg/SVGPathElement.h (99459 => 99460)
--- trunk/Source/WebCore/svg/SVGPathElement.h 2011-11-07 20:37:06 UTC (rev 99459)
+++ trunk/Source/WebCore/svg/SVGPathElement.h 2011-11-07 20:40:24 UTC (rev 99460)
@@ -97,6 +97,8 @@
static const SVGPropertyInfo* dPropertyInfo();
+ virtual FloatRect getBBox(StyleUpdateStrategy = AllowStyleUpdate);
+
private:
SVGPathElement(const QualifiedName&, Document*);
@@ -126,6 +128,8 @@
OwnPtr<SVGPathByteStream> m_pathByteStream;
mutable SVGSynchronizableAnimatedProperty<SVGPathSegList> m_pathSegList;
RefPtr<SVGAnimatedPathSegListPropertyTearOff> m_animatablePathSegList;
+ FloatRect m_cachedBBoxRect;
+ bool m_cachedBBoxRectIsValid;
};
} // namespace WebCore
