Diff
Modified: trunk/Source/WebCore/ChangeLog (122423 => 122424)
--- trunk/Source/WebCore/ChangeLog 2012-07-12 07:53:08 UTC (rev 122423)
+++ trunk/Source/WebCore/ChangeLog 2012-07-12 08:10:23 UTC (rev 122424)
@@ -1,3 +1,64 @@
+2012-07-12 Philip Rogers <[email protected]>
+
+ Refactor RenderSVGShape bounding box code
+ https://bugs.webkit.org/show_bug.cgi?id=90655
+
+ Reviewed by Nikolas Zimmermann.
+
+ RenderSVGShape::objectBoundingBox worked differently than RenderSVGShape::strokeBoundingBox by
+ not caching the object bounding box and instead computing it on each call. For consistency and
+ performance objectBoundingBox has been refactored to return a cached value.
+
+ createShape has been renamed updateShapeFromElement for understandability. updateShapeFromElement
+ now updates the internal state of the shape (bounding boxes, etc) from the associated element.
+ RenderSVGShape::inflateWithStrokeAndMarkerBounds has been merged into
+ RenderSVGShape::calculateStrokeBoundingBox which is called from updateShapeFromElement.
+
+ After this change all bounding box computation is now handled in updateShapeFromElement. Because
+ subclasses override updateShapeFromElement it will be easy for them to have custom bounding box
+ code there (as will happen for RenderSVGPath in a followup patch).
+
+ strokeBoundingBox and objectBoundingBox are now able to return their cached values immediately
+ in RenderSVGRect and RenderSVGEllipse instead of checking their fallback state on each call.
+
+ Additionally, to save space RenderSVGEllipse and RenderSVGRect now use the m_fillBoundingBox and
+ m_strokeBoundingBox of RenderSVGShape instead of having their own.
+
+ This patch also removes setStrokeAndMarkerBoundingBox that was previously dead code.
+
+ No new tests, just a refactoring.
+
+ * rendering/svg/RenderSVGEllipse.cpp:
+ (WebCore::RenderSVGEllipse::updateShapeFromElement):
+ (WebCore):
+ (WebCore::RenderSVGEllipse::fillShape):
+ (WebCore::RenderSVGEllipse::strokeShape):
+ (WebCore::RenderSVGEllipse::shapeDependentStrokeContains):
+ * rendering/svg/RenderSVGEllipse.h:
+ (RenderSVGEllipse):
+ (WebCore::RenderSVGEllipse::isEmpty):
+ * rendering/svg/RenderSVGRect.cpp:
+ (WebCore::RenderSVGRect::updateShapeFromElement):
+ (WebCore):
+ (WebCore::RenderSVGRect::fillShape):
+ (WebCore::RenderSVGRect::strokeShape):
+ (WebCore::RenderSVGRect::shapeDependentStrokeContains):
+ (WebCore::RenderSVGRect::shapeDependentFillContains):
+ * rendering/svg/RenderSVGRect.h:
+ (RenderSVGRect):
+ (WebCore::RenderSVGRect::isEmpty):
+ * rendering/svg/RenderSVGShape.cpp:
+ (WebCore::RenderSVGShape::updateShapeFromElement):
+ (WebCore):
+ (WebCore::RenderSVGShape::layout):
+ (WebCore::RenderSVGShape::calculateObjectBoundingBox):
+ (WebCore::RenderSVGShape::calculateStrokeBoundingBox):
+ (WebCore::RenderSVGShape::updateRepaintBoundingBox):
+ * rendering/svg/RenderSVGShape.h:
+ (RenderSVGShape):
+ (WebCore::RenderSVGShape::objectBoundingBox):
+ (WebCore::RenderSVGShape::strokeBoundingBox):
+
2012-07-12 Kent Tamura <[email protected]>
Do not save the form state signature if nothing is saved
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGEllipse.cpp (122423 => 122424)
--- trunk/Source/WebCore/rendering/svg/RenderSVGEllipse.cpp 2012-07-12 07:53:08 UTC (rev 122423)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGEllipse.cpp 2012-07-12 08:10:23 UTC (rev 122424)
@@ -46,18 +46,18 @@
{
}
-void RenderSVGEllipse::createShape()
+void RenderSVGEllipse::updateShapeFromElement()
{
// Before creating a new object we need to clear the cached bounding box
// to avoid using garbage.
- m_boundingBox = FloatRect();
- m_outerStrokeRect = FloatRect();
+ m_fillBoundingBox = FloatRect();
+ m_strokeBoundingBox = FloatRect();
m_center = FloatPoint();
m_radii = FloatSize();
// Fallback to RenderSVGShape if shape has a non-scaling stroke.
if (hasNonScalingStroke()) {
- RenderSVGShape::createShape();
+ RenderSVGShape::updateShapeFromElement();
m_usePathFallback = true;
return;
} else
@@ -69,10 +69,10 @@
if (m_radii.width() <= 0 || m_radii.height() <= 0)
return;
- m_boundingBox = FloatRect(m_center.x() - m_radii.width(), m_center.y() - m_radii.height(), 2 * m_radii.width(), 2 * m_radii.height());
- m_outerStrokeRect = m_boundingBox;
+ m_fillBoundingBox = FloatRect(m_center.x() - m_radii.width(), m_center.y() - m_radii.height(), 2 * m_radii.width(), 2 * m_radii.height());
+ m_strokeBoundingBox = m_fillBoundingBox;
if (style()->svgStyle()->hasStroke())
- m_outerStrokeRect.inflate(strokeWidth() / 2);
+ m_strokeBoundingBox.inflate(strokeWidth() / 2);
}
void RenderSVGEllipse::calculateRadiiAndCenter()
@@ -97,27 +97,13 @@
m_center = FloatPoint(ellipse->cx().value(lengthContext), ellipse->cy().value(lengthContext));
}
-FloatRect RenderSVGEllipse::objectBoundingBox() const
-{
- if (m_usePathFallback)
- return RenderSVGShape::objectBoundingBox();
- return m_boundingBox;
-}
-
-FloatRect RenderSVGEllipse::strokeBoundingBox() const
-{
- if (m_usePathFallback)
- return RenderSVGShape::strokeBoundingBox();
- return m_outerStrokeRect;
-}
-
void RenderSVGEllipse::fillShape(GraphicsContext* context) const
{
if (m_usePathFallback) {
RenderSVGShape::fillShape(context);
return;
}
- context->fillEllipse(m_boundingBox);
+ context->fillEllipse(m_fillBoundingBox);
}
void RenderSVGEllipse::strokeShape(GraphicsContext* context) const
@@ -128,7 +114,7 @@
RenderSVGShape::strokeShape(context);
return;
}
- context->strokeEllipse(m_boundingBox);
+ context->strokeEllipse(m_fillBoundingBox);
}
bool RenderSVGEllipse::shapeDependentStrokeContains(const FloatPoint& point)
@@ -137,7 +123,7 @@
// to fall back to RenderSVGShape::shapeDependentStrokeContains in these cases.
if (m_usePathFallback || !hasSmoothStroke()) {
if (!hasPath())
- RenderSVGShape::createShape();
+ RenderSVGShape::updateShapeFromElement();
return RenderSVGShape::shapeDependentStrokeContains(point);
}
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGEllipse.h (122423 => 122424)
--- trunk/Source/WebCore/rendering/svg/RenderSVGEllipse.h 2012-07-12 07:53:08 UTC (rev 122423)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGEllipse.h 2012-07-12 08:10:23 UTC (rev 122424)
@@ -41,19 +41,15 @@
private:
virtual const char* renderName() const { return "RenderSVGEllipse"; }
- virtual void createShape();
- virtual bool isEmpty() const { return m_usePathFallback ? RenderSVGShape::isEmpty() : m_boundingBox.isEmpty(); };
+ virtual void updateShapeFromElement();
+ virtual bool isEmpty() const { return m_usePathFallback ? RenderSVGShape::isEmpty() : m_fillBoundingBox.isEmpty(); };
virtual void fillShape(GraphicsContext*) const;
virtual void strokeShape(GraphicsContext*) const;
- virtual FloatRect objectBoundingBox() const;
- virtual FloatRect strokeBoundingBox() const;
virtual bool shapeDependentStrokeContains(const FloatPoint&);
virtual bool shapeDependentFillContains(const FloatPoint&, const WindRule) const;
void calculateRadiiAndCenter();
private:
- FloatRect m_boundingBox;
- FloatRect m_outerStrokeRect;
FloatPoint m_center;
FloatSize m_radii;
bool m_usePathFallback;
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGRect.cpp (122423 => 122424)
--- trunk/Source/WebCore/rendering/svg/RenderSVGRect.cpp 2012-07-12 07:53:08 UTC (rev 122423)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGRect.cpp 2012-07-12 08:10:23 UTC (rev 122424)
@@ -46,11 +46,11 @@
{
}
-void RenderSVGRect::createShape()
+void RenderSVGRect::updateShapeFromElement()
{
// Before creating a new object we need to clear the cached bounding box
// to avoid using garbage.
- m_boundingBox = FloatRect();
+ m_fillBoundingBox = FloatRect();
m_innerStrokeRect = FloatRect();
m_outerStrokeRect = FloatRect();
SVGRectElement* rect = static_cast<SVGRectElement*>(node());
@@ -58,7 +58,7 @@
// Fallback to RenderSVGShape if rect has rounded corners or a non-scaling stroke.
if (rect->hasAttribute(SVGNames::rxAttr) || rect->hasAttribute(SVGNames::ryAttr) || hasNonScalingStroke()) {
- RenderSVGShape::createShape();
+ RenderSVGShape::updateShapeFromElement();
m_usePathFallback = true;
return;
} else
@@ -69,12 +69,12 @@
if (boundingBoxSize.isEmpty())
return;
- m_boundingBox = FloatRect(FloatPoint(rect->x().value(lengthContext), rect->y().value(lengthContext)), boundingBoxSize);
+ m_fillBoundingBox = FloatRect(FloatPoint(rect->x().value(lengthContext), rect->y().value(lengthContext)), boundingBoxSize);
// To decide if the stroke contains a point we create two rects which represent the inner and
// the outer stroke borders. A stroke contains the point, if the point is between them.
- m_innerStrokeRect = m_boundingBox;
- m_outerStrokeRect = m_boundingBox;
+ m_innerStrokeRect = m_fillBoundingBox;
+ m_outerStrokeRect = m_fillBoundingBox;
if (style()->svgStyle()->hasStroke()) {
float strokeWidth = this->strokeWidth();
@@ -82,29 +82,15 @@
m_outerStrokeRect.inflate(strokeWidth / 2);
}
- m_strokeBoundingRect = m_outerStrokeRect;
+ m_strokeBoundingBox = m_outerStrokeRect;
#if USE(CG)
// CoreGraphics can inflate the stroke by 1px when drawing a rectangle with antialiasing disabled at non-integer coordinates, we need to compensate.
if (style()->svgStyle()->shapeRendering() == SR_CRISPEDGES)
- m_strokeBoundingRect.inflate(1);
+ m_strokeBoundingBox.inflate(1);
#endif
}
-FloatRect RenderSVGRect::objectBoundingBox() const
-{
- if (m_usePathFallback)
- return RenderSVGShape::objectBoundingBox();
- return m_boundingBox;
-}
-
-FloatRect RenderSVGRect::strokeBoundingBox() const
-{
- if (m_usePathFallback)
- return RenderSVGShape::strokeBoundingBox();
- return m_strokeBoundingRect;
-}
-
void RenderSVGRect::fillShape(GraphicsContext* context) const
{
if (m_usePathFallback) {
@@ -120,12 +106,12 @@
if (context->hasShadow()) {
GraphicsContextStateSaver stateSaver(*context);
context->clearShadow();
- context->fillRect(m_boundingBox);
+ context->fillRect(m_fillBoundingBox);
return;
}
#endif
- context->fillRect(m_boundingBox);
+ context->fillRect(m_fillBoundingBox);
}
void RenderSVGRect::strokeShape(GraphicsContext* context) const
@@ -138,7 +124,7 @@
return;
}
- context->strokeRect(m_boundingBox, strokeWidth());
+ context->strokeRect(m_fillBoundingBox, strokeWidth());
}
bool RenderSVGRect::shapeDependentStrokeContains(const FloatPoint& point)
@@ -147,7 +133,7 @@
// to fall back to RenderSVGShape::shapeDependentStrokeContains in these cases.
if (m_usePathFallback || !hasSmoothStroke()) {
if (!hasPath())
- RenderSVGShape::createShape();
+ RenderSVGShape::updateShapeFromElement();
return RenderSVGShape::shapeDependentStrokeContains(point);
}
@@ -158,7 +144,7 @@
{
if (m_usePathFallback)
return RenderSVGShape::shapeDependentFillContains(point, fillRule);
- return m_boundingBox.contains(point.x(), point.y());
+ return m_fillBoundingBox.contains(point.x(), point.y());
}
}
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGRect.h (122423 => 122424)
--- trunk/Source/WebCore/rendering/svg/RenderSVGRect.h 2012-07-12 07:53:08 UTC (rev 122423)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGRect.h 2012-07-12 08:10:23 UTC (rev 122424)
@@ -42,20 +42,16 @@
private:
virtual const char* renderName() const { return "RenderSVGRect"; }
- virtual void createShape();
- virtual bool isEmpty() const { return m_usePathFallback ? RenderSVGShape::isEmpty() : m_boundingBox.isEmpty(); };
+ virtual void updateShapeFromElement();
+ virtual bool isEmpty() const { return m_usePathFallback ? RenderSVGShape::isEmpty() : m_fillBoundingBox.isEmpty(); };
virtual void fillShape(GraphicsContext*) const;
virtual void strokeShape(GraphicsContext*) const;
- virtual FloatRect objectBoundingBox() const;
- virtual FloatRect strokeBoundingBox() const;
virtual bool shapeDependentStrokeContains(const FloatPoint&);
virtual bool shapeDependentFillContains(const FloatPoint&, const WindRule) const;
private:
- FloatRect m_boundingBox;
FloatRect m_innerStrokeRect;
FloatRect m_outerStrokeRect;
- FloatRect m_strokeBoundingRect;
bool m_usePathFallback;
};
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGShape.cpp (122423 => 122424)
--- trunk/Source/WebCore/rendering/svg/RenderSVGShape.cpp 2012-07-12 07:53:08 UTC (rev 122423)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGShape.cpp 2012-07-12 08:10:23 UTC (rev 122424)
@@ -64,9 +64,9 @@
{
}
-void RenderSVGShape::createShape()
+void RenderSVGShape::updateShapeFromElement()
{
- ASSERT(!m_path);
+ m_path.clear();
m_path = adoptPtr(new Path);
ASSERT(RenderSVGShape::isEmpty());
@@ -74,6 +74,9 @@
updatePathFromGraphicsElement(element, path());
processZeroLengthSubpaths();
processMarkerPositions();
+
+ m_fillBoundingBox = calculateObjectBoundingBox();
+ m_strokeBoundingBox = calculateStrokeBoundingBox();
}
bool RenderSVGShape::isEmpty() const
@@ -86,11 +89,6 @@
context->fillPath(path());
}
-FloatRect RenderSVGShape::objectBoundingBox() const
-{
- return path().fastBoundingRect();
-}
-
void RenderSVGShape::strokeShape(GraphicsContext* context) const
{
if (style()->svgStyle()->hasVisibleStroke())
@@ -162,11 +160,11 @@
bool updateCachedBoundariesInParents = false;
- bool needsShapeUpdate = m_needsShapeUpdate;
- if (needsShapeUpdate || m_needsBoundariesUpdate) {
- m_path.clear();
- createShape();
+ if (m_needsShapeUpdate || m_needsBoundariesUpdate) {
+ updateShapeFromElement();
m_needsShapeUpdate = false;
+ updateRepaintBoundingBox();
+ m_needsBoundariesUpdate = false;
updateCachedBoundariesInParents = true;
}
@@ -180,13 +178,6 @@
if (everHadLayout() && selfNeedsLayout())
SVGResourcesCache::clientLayoutChanged(this);
- // At this point LayoutRepainter already grabbed the old bounds,
- // recalculate them now so repaintAfterLayout() uses the new bounds.
- if (needsShapeUpdate || m_needsBoundariesUpdate) {
- updateCachedBoundaries();
- m_needsBoundariesUpdate = false;
- }
-
// If our bounds changed, notify the parents.
if (updateCachedBoundariesInParents)
RenderSVGModelObject::setNeedsBoundariesUpdate();
@@ -439,29 +430,44 @@
return boundaries;
}
-void RenderSVGShape::updateCachedBoundaries()
+FloatRect RenderSVGShape::calculateObjectBoundingBox() const
{
- if (isEmpty()) {
- m_fillBoundingBox = FloatRect();
- m_strokeAndMarkerBoundingBox = FloatRect();
- m_repaintBoundingBox = FloatRect();
- return;
+ return path().fastBoundingRect();
+}
+
+FloatRect RenderSVGShape::calculateStrokeBoundingBox() const
+{
+ ASSERT(m_path);
+ FloatRect strokeBoundingBox = m_fillBoundingBox;
+
+ const SVGRenderStyle* svgStyle = style()->svgStyle();
+ if (svgStyle->hasStroke()) {
+ BoundingRectStrokeStyleApplier strokeStyle(this, style());
+ if (hasNonScalingStroke()) {
+ AffineTransform nonScalingTransform = nonScalingStrokeTransform();
+ if (nonScalingTransform.isInvertible()) {
+ Path* usePath = nonScalingStrokePath(m_path.get(), nonScalingTransform);
+ FloatRect strokeBoundingRect = usePath->strokeBoundingRect(&strokeStyle);
+ strokeBoundingRect = nonScalingTransform.inverse().mapRect(strokeBoundingRect);
+ strokeBoundingBox.unite(strokeBoundingRect);
+ }
+ } else
+ strokeBoundingBox.unite(path().strokeBoundingRect(&strokeStyle));
+
+ // FIXME: zero-length subpaths do not respect vector-effect = non-scaling-stroke.
+ float strokeWidth = this->strokeWidth();
+ for (size_t i = 0; i < m_zeroLengthLinecapLocations.size(); ++i)
+ strokeBoundingBox.unite(zeroLengthSubpathRect(m_zeroLengthLinecapLocations[i], strokeWidth));
}
- // Cache _unclipped_ fill bounding box, used for calculations in resources
- m_fillBoundingBox = objectBoundingBox();
+ if (!m_markerPositions.isEmpty())
+ strokeBoundingBox.unite(markerRect(strokeWidth()));
- // Add zero-length sub-path linecaps to the fill box
- // FIXME: zero-length subpaths do not respect vector-effect = non-scaling-stroke.
- float strokeWidth = this->strokeWidth();
- for (size_t i = 0; i < m_zeroLengthLinecapLocations.size(); ++i)
- m_fillBoundingBox.unite(zeroLengthSubpathRect(m_zeroLengthLinecapLocations[i], strokeWidth));
+ return strokeBoundingBox;
+}
- // Cache _unclipped_ stroke bounding box, used for calculations in resources (includes marker boundaries)
- m_strokeAndMarkerBoundingBox = m_fillBoundingBox;
- if (hasPath())
- inflateWithStrokeAndMarkerBounds();
- // Cache smallest possible repaint rectangle
+void RenderSVGShape::updateRepaintBoundingBox()
+{
m_repaintBoundingBox = strokeBoundingBox();
SVGRenderSupport::intersectRepaintRectWithResources(this, m_repaintBoundingBox);
}
@@ -482,28 +488,6 @@
&& svgStyle->capStyle() == svgStyle->initialCapStyle();
}
-void RenderSVGShape::inflateWithStrokeAndMarkerBounds()
-{
- const SVGRenderStyle* svgStyle = style()->svgStyle();
- if (svgStyle->hasStroke()) {
- BoundingRectStrokeStyleApplier strokeStyle(this, style());
-
- // SVG1.2 Tiny only defines non scaling stroke for the stroke but not markers.
- if (hasNonScalingStroke()) {
- AffineTransform nonScalingTransform = nonScalingStrokeTransform();
- if (nonScalingTransform.isInvertible()) {
- Path* usePath = nonScalingStrokePath(m_path.get(), nonScalingTransform);
- FloatRect strokeBoundingRect = usePath->strokeBoundingRect(&strokeStyle);
- strokeBoundingRect = nonScalingTransform.inverse().mapRect(strokeBoundingRect);
- m_strokeAndMarkerBoundingBox.unite(strokeBoundingRect);
- }
- } else
- m_strokeAndMarkerBoundingBox.unite(path().strokeBoundingRect(&strokeStyle));
- }
- if (!m_markerPositions.isEmpty())
- m_strokeAndMarkerBoundingBox.unite(markerRect(strokeWidth()));
-}
-
void RenderSVGShape::drawMarkers(PaintInfo& paintInfo)
{
ASSERT(!m_markerPositions.isEmpty());
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGShape.h (122423 => 122424)
--- trunk/Source/WebCore/rendering/svg/RenderSVGShape.h 2012-07-12 07:53:08 UTC (rev 122423)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGShape.h 2012-07-12 08:10:23 UTC (rev 122424)
@@ -83,11 +83,8 @@
}
protected:
- virtual void createShape();
+ virtual void updateShapeFromElement();
virtual bool isEmpty() const;
- virtual FloatRect objectBoundingBox() const;
- virtual FloatRect strokeBoundingBox() const { return m_strokeAndMarkerBoundingBox; }
- void setStrokeAndMarkerBoundingBox(FloatRect rect) { m_strokeAndMarkerBoundingBox = rect; }
virtual bool shapeDependentStrokeContains(const FloatPoint&);
virtual bool shapeDependentFillContains(const FloatPoint&, const WindRule) const;
float strokeWidth() const;
@@ -95,6 +92,9 @@
bool hasNonScalingStroke() const { return style()->svgStyle()->vectorEffect() == VE_NON_SCALING_STROKE; }
bool hasSmoothStroke() const;
+ FloatRect m_fillBoundingBox;
+ FloatRect m_strokeBoundingBox;
+
private:
// Hit-detection separated for the fill and the stroke
bool fillContains(const FloatPoint&, bool requiresFill = true, const WindRule fillRule = RULE_NONZERO);
@@ -113,8 +113,13 @@
virtual bool nodeAtFloatPoint(const HitTestRequest&, HitTestResult&, const FloatPoint& pointInParent, HitTestAction);
- void updateCachedBoundaries();
+ virtual FloatRect objectBoundingBox() const { return m_fillBoundingBox; }
+ virtual FloatRect strokeBoundingBox() const { return m_strokeBoundingBox; }
+ FloatRect calculateObjectBoundingBox() const;
+ FloatRect calculateStrokeBoundingBox() const;
+ void updateRepaintBoundingBox();
+
AffineTransform nonScalingStrokeTransform() const;
bool setupNonScalingStrokeContext(AffineTransform&, GraphicsContextStateSaver&);
Path* nonScalingStrokePath(const Path*, const AffineTransform&) const;
@@ -132,12 +137,9 @@
void strokePath(RenderStyle*, GraphicsContext*, Path*, RenderSVGResource*,
const Color&, int);
void fillAndStrokePath(GraphicsContext*);
- void inflateWithStrokeAndMarkerBounds();
void drawMarkers(PaintInfo&);
private:
- FloatRect m_fillBoundingBox;
- FloatRect m_strokeAndMarkerBoundingBox;
FloatRect m_repaintBoundingBox;
AffineTransform m_localTransform;
OwnPtr<Path> m_path;
