Refactor RenderSVGShape to not contain fallback code
authorpdr@google.com <pdr@google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 9 Jul 2012 03:57:03 +0000 (03:57 +0000)
committerpdr@google.com <pdr@google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 9 Jul 2012 03:57:03 +0000 (03:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=90514

Reviewed by Nikolas Zimmermann.

The interaction between RenderSVGShape and {RenderSVGEllipse, RenderSVGRect}
was too coupled and it was not clear when a path existed or who controlled
falling back to path codepaths in RenderSVGShape.

This patch cleans up RenderSVGShape so that it does not track fallback state
and does not have special handling for creating a shape in strokeContains. Because
some functions of RenderSVGShape can be called without a path existing, each
of these functions has switched to using the path() function which asserts that
a path exists.

There is only one remaining use of hasPath() in RenderSVGShape which I plan
to remove in a followup patch.

This patch also cleans up RenderSVGRect and RenderSVGEllipse. These classes
now handle fallback tracking themselves and choose when to use their optimized
strokeContains codepaths.

No new tests as this is just a refactoring.

* rendering/svg/RenderSVGEllipse.cpp:
(WebCore::RenderSVGEllipse::RenderSVGEllipse):
(WebCore::RenderSVGEllipse::createShape):
(WebCore::RenderSVGEllipse::objectBoundingBox):
(WebCore::RenderSVGEllipse::strokeBoundingBox):
(WebCore::RenderSVGEllipse::fillShape):
(WebCore::RenderSVGEllipse::strokeShape):
(WebCore::RenderSVGEllipse::shapeDependentStrokeContains):
(WebCore::RenderSVGEllipse::shapeDependentFillContains):
* rendering/svg/RenderSVGEllipse.h:
(WebCore::RenderSVGEllipse::isEmpty):
(RenderSVGEllipse):
* rendering/svg/RenderSVGRect.cpp:
(WebCore::RenderSVGRect::RenderSVGRect):
(WebCore::RenderSVGRect::createShape):
(WebCore::RenderSVGRect::objectBoundingBox):
(WebCore::RenderSVGRect::strokeBoundingBox):
(WebCore::RenderSVGRect::fillShape):
(WebCore::RenderSVGRect::strokeShape):
(WebCore::RenderSVGRect::shapeDependentStrokeContains):
(WebCore::RenderSVGRect::shapeDependentFillContains):
* rendering/svg/RenderSVGRect.h:
(WebCore::RenderSVGRect::isEmpty):
(RenderSVGRect):
* rendering/svg/RenderSVGShape.cpp:
(WebCore::RenderSVGShape::RenderSVGShape):
(WebCore::RenderSVGShape::createShape):
(WebCore::RenderSVGShape::isEmpty):
(WebCore::RenderSVGShape::objectBoundingBox):
(WebCore::RenderSVGShape::shapeDependentStrokeContains):
(WebCore::RenderSVGShape::shapeDependentFillContains):
(WebCore::RenderSVGShape::strokeContains):
(WebCore::RenderSVGShape::layout):
(WebCore::RenderSVGShape::hasSmoothStroke):
(WebCore):
* rendering/svg/RenderSVGShape.h:
(RenderSVGShape):

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

Source/WebCore/ChangeLog
Source/WebCore/rendering/svg/RenderSVGEllipse.cpp
Source/WebCore/rendering/svg/RenderSVGEllipse.h
Source/WebCore/rendering/svg/RenderSVGRect.cpp
Source/WebCore/rendering/svg/RenderSVGRect.h
Source/WebCore/rendering/svg/RenderSVGShape.cpp
Source/WebCore/rendering/svg/RenderSVGShape.h

index 8d9aac6..d84708c 100644 (file)
@@ -1,3 +1,67 @@
+2012-07-08  Philip Rogers  <pdr@google.com>
+
+        Refactor RenderSVGShape to not contain fallback code
+        https://bugs.webkit.org/show_bug.cgi?id=90514
+
+        Reviewed by Nikolas Zimmermann.
+
+        The interaction between RenderSVGShape and {RenderSVGEllipse, RenderSVGRect}
+        was too coupled and it was not clear when a path existed or who controlled
+        falling back to path codepaths in RenderSVGShape.
+
+        This patch cleans up RenderSVGShape so that it does not track fallback state
+        and does not have special handling for creating a shape in strokeContains. Because
+        some functions of RenderSVGShape can be called without a path existing, each
+        of these functions has switched to using the path() function which asserts that
+        a path exists.
+
+        There is only one remaining use of hasPath() in RenderSVGShape which I plan
+        to remove in a followup patch.
+
+        This patch also cleans up RenderSVGRect and RenderSVGEllipse. These classes
+        now handle fallback tracking themselves and choose when to use their optimized
+        strokeContains codepaths.
+
+        No new tests as this is just a refactoring.
+
+        * rendering/svg/RenderSVGEllipse.cpp:
+        (WebCore::RenderSVGEllipse::RenderSVGEllipse):
+        (WebCore::RenderSVGEllipse::createShape):
+        (WebCore::RenderSVGEllipse::objectBoundingBox):
+        (WebCore::RenderSVGEllipse::strokeBoundingBox):
+        (WebCore::RenderSVGEllipse::fillShape):
+        (WebCore::RenderSVGEllipse::strokeShape):
+        (WebCore::RenderSVGEllipse::shapeDependentStrokeContains):
+        (WebCore::RenderSVGEllipse::shapeDependentFillContains):
+        * rendering/svg/RenderSVGEllipse.h:
+        (WebCore::RenderSVGEllipse::isEmpty):
+        (RenderSVGEllipse):
+        * rendering/svg/RenderSVGRect.cpp:
+        (WebCore::RenderSVGRect::RenderSVGRect):
+        (WebCore::RenderSVGRect::createShape):
+        (WebCore::RenderSVGRect::objectBoundingBox):
+        (WebCore::RenderSVGRect::strokeBoundingBox):
+        (WebCore::RenderSVGRect::fillShape):
+        (WebCore::RenderSVGRect::strokeShape):
+        (WebCore::RenderSVGRect::shapeDependentStrokeContains):
+        (WebCore::RenderSVGRect::shapeDependentFillContains):
+        * rendering/svg/RenderSVGRect.h:
+        (WebCore::RenderSVGRect::isEmpty):
+        (RenderSVGRect):
+        * rendering/svg/RenderSVGShape.cpp:
+        (WebCore::RenderSVGShape::RenderSVGShape):
+        (WebCore::RenderSVGShape::createShape):
+        (WebCore::RenderSVGShape::isEmpty):
+        (WebCore::RenderSVGShape::objectBoundingBox):
+        (WebCore::RenderSVGShape::shapeDependentStrokeContains):
+        (WebCore::RenderSVGShape::shapeDependentFillContains):
+        (WebCore::RenderSVGShape::strokeContains):
+        (WebCore::RenderSVGShape::layout):
+        (WebCore::RenderSVGShape::hasSmoothStroke):
+        (WebCore):
+        * rendering/svg/RenderSVGShape.h:
+        (RenderSVGShape):
+
 2012-07-08  Kinuko Yasuda  <kinuko@chromium.org>
 
         XHR.send should support ArrayBufferView
index c9f183a..b14f8ac 100644 (file)
@@ -38,6 +38,7 @@ namespace WebCore {
 
 RenderSVGEllipse::RenderSVGEllipse(SVGStyledTransformableElement* node)
     : RenderSVGShape(node)
+    , m_usePathFallback(false)
 {
 }
 
@@ -54,12 +55,13 @@ void RenderSVGEllipse::createShape()
     m_center = FloatPoint();
     m_radii = FloatSize();
 
-    // Fallback to RenderSVGShape if shape has a non scaling stroke.
+    // Fallback to RenderSVGShape if shape has a non-scaling stroke.
     if (hasNonScalingStroke()) {
         RenderSVGShape::createShape();
-        setIsPaintingFallback(true);
+        m_usePathFallback = true;
         return;
-    }
+    } else
+        m_usePathFallback = false;
 
     calculateRadiiAndCenter();
 
@@ -97,21 +99,21 @@ void RenderSVGEllipse::calculateRadiiAndCenter()
 
 FloatRect RenderSVGEllipse::objectBoundingBox() const
 {
-    if (isPaintingFallback())
+    if (m_usePathFallback)
         return RenderSVGShape::objectBoundingBox();
     return m_boundingBox;
 }
 
 FloatRect RenderSVGEllipse::strokeBoundingBox() const
 {
-    if (isPaintingFallback())
+    if (m_usePathFallback)
         return RenderSVGShape::strokeBoundingBox();
     return m_outerStrokeRect;
 }
 
 void RenderSVGEllipse::fillShape(GraphicsContext* context) const
 {
-    if (isPaintingFallback()) {
+    if (m_usePathFallback) {
         RenderSVGShape::fillShape(context);
         return;
     }
@@ -122,17 +124,22 @@ void RenderSVGEllipse::strokeShape(GraphicsContext* context) const
 {
     if (!style()->svgStyle()->hasVisibleStroke())
         return;
-    if (isPaintingFallback()) {
+    if (m_usePathFallback) {
         RenderSVGShape::strokeShape(context);
         return;
     }
     context->strokeEllipse(m_boundingBox);
 }
 
-bool RenderSVGEllipse::shapeDependentStrokeContains(const FloatPoint& point) const
+bool RenderSVGEllipse::shapeDependentStrokeContains(const FloatPoint& point)
 {
-    if (isPaintingFallback())
+    // The optimized contains code below does not support non-smooth strokes so we need
+    // to fall back to RenderSVGShape::shapeDependentStrokeContains in these cases.
+    if (m_usePathFallback || !hasSmoothStroke()) {
+        if (!hasPath())
+            RenderSVGShape::createShape();
         return RenderSVGShape::shapeDependentStrokeContains(point);
+    }
 
     float halfStrokeWidth = strokeWidth() / 2;
     FloatPoint center = FloatPoint(m_center.x() - point.x(), m_center.y() - point.y());
@@ -151,7 +158,7 @@ bool RenderSVGEllipse::shapeDependentStrokeContains(const FloatPoint& point) con
 
 bool RenderSVGEllipse::shapeDependentFillContains(const FloatPoint& point, const WindRule fillRule) const
 {
-    if (isPaintingFallback())
+    if (m_usePathFallback)
         return RenderSVGShape::shapeDependentFillContains(point, fillRule);
 
     FloatPoint center = FloatPoint(m_center.x() - point.x(), m_center.y() - point.y());
index 3720e9a..9e88dee 100644 (file)
@@ -42,12 +42,12 @@ private:
     virtual const char* renderName() const { return "RenderSVGEllipse"; }
 
     virtual void createShape();
-    virtual bool isEmpty() const { return hasPath() ? RenderSVGShape::isEmpty() : m_boundingBox.isEmpty(); };
+    virtual bool isEmpty() const { return m_usePathFallback ? RenderSVGShape::isEmpty() : m_boundingBox.isEmpty(); };
     virtual void fillShape(GraphicsContext*) const;
     virtual void strokeShape(GraphicsContext*) const;
     virtual FloatRect objectBoundingBox() const;
     virtual FloatRect strokeBoundingBox() const;
-    virtual bool shapeDependentStrokeContains(const FloatPoint&) const;
+    virtual bool shapeDependentStrokeContains(const FloatPoint&);
     virtual bool shapeDependentFillContains(const FloatPoint&, const WindRule) const;
     void calculateRadiiAndCenter();
 
@@ -56,6 +56,7 @@ private:
     FloatRect m_outerStrokeRect;
     FloatPoint m_center;
     FloatSize m_radii;
+    bool m_usePathFallback;
 };
 
 }
index 05429e8..120c132 100755 (executable)
@@ -38,6 +38,7 @@ namespace WebCore {
 
 RenderSVGRect::RenderSVGRect(SVGRectElement* node)
     : RenderSVGShape(node)
+    , m_usePathFallback(false)
 {
 }
 
@@ -55,12 +56,13 @@ void RenderSVGRect::createShape()
     SVGRectElement* rect = static_cast<SVGRectElement*>(node());
     ASSERT(rect);
 
-    // Fallback to RenderSVGShape if rect has rounded corners.
+    // 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();
-       setIsPaintingFallback(true);
-       return;
-    }
+        RenderSVGShape::createShape();
+        m_usePathFallback = true;
+        return;
+    } else
+        m_usePathFallback = false;
 
     SVGLengthContext lengthContext(rect);
     FloatSize boundingBoxSize(rect->width().value(lengthContext), rect->height().value(lengthContext));
@@ -91,61 +93,70 @@ void RenderSVGRect::createShape()
 
 FloatRect RenderSVGRect::objectBoundingBox() const
 {
-    if (isPaintingFallback())
+    if (m_usePathFallback)
         return RenderSVGShape::objectBoundingBox();
     return m_boundingBox;
 }
 
 FloatRect RenderSVGRect::strokeBoundingBox() const
 {
-    if (isPaintingFallback())
+    if (m_usePathFallback)
         return RenderSVGShape::strokeBoundingBox();
     return m_strokeBoundingRect;
 }
 
 void RenderSVGRect::fillShape(GraphicsContext* context) const
 {
-    if (!isPaintingFallback()) {
+    if (m_usePathFallback) {
+        RenderSVGShape::fillShape(context);
+        return;
+    }
+
 #if USE(CG)
-        // FIXME: CG implementation of GraphicsContextCG::fillRect has an own
-        // shadow drawing method, which draws an extra shadow.
-        // This is a workaround for switching off the extra shadow.
-        // https://bugs.webkit.org/show_bug.cgi?id=68899
-        if (context->hasShadow()) {
-            GraphicsContextStateSaver stateSaver(*context);
-            context->clearShadow();
-            context->fillRect(m_boundingBox);
-            return;
-        }
-#endif
+    // FIXME: CG implementation of GraphicsContextCG::fillRect has an own
+    // shadow drawing method, which draws an extra shadow.
+    // This is a workaround for switching off the extra shadow.
+    // https://bugs.webkit.org/show_bug.cgi?id=68899
+    if (context->hasShadow()) {
+        GraphicsContextStateSaver stateSaver(*context);
+        context->clearShadow();
         context->fillRect(m_boundingBox);
         return;
     }
-    RenderSVGShape::fillShape(context);
+#endif
+
+    context->fillRect(m_boundingBox);
 }
 
 void RenderSVGRect::strokeShape(GraphicsContext* context) const
 {
     if (!style()->svgStyle()->hasVisibleStroke())
         return;
-    if (!isPaintingFallback()) {
-        context->strokeRect(m_boundingBox, strokeWidth());
+
+    if (m_usePathFallback) {
+        RenderSVGShape::strokeShape(context);
         return;
     }
-    RenderSVGShape::strokeShape(context);
+
+    context->strokeRect(m_boundingBox, strokeWidth());
 }
 
-bool RenderSVGRect::shapeDependentStrokeContains(const FloatPoint& point) const
+bool RenderSVGRect::shapeDependentStrokeContains(const FloatPoint& point)
 {
-    if (isPaintingFallback())
+    // The optimized contains code below does not support non-smooth strokes so we need
+    // to fall back to RenderSVGShape::shapeDependentStrokeContains in these cases.
+    if (m_usePathFallback || !hasSmoothStroke()) {
+        if (!hasPath())
+            RenderSVGShape::createShape();
         return RenderSVGShape::shapeDependentStrokeContains(point);
+    }
 
     return m_outerStrokeRect.contains(point, FloatRect::InsideOrOnStroke) && !m_innerStrokeRect.contains(point, FloatRect::InsideButNotOnStroke);
 }
 
 bool RenderSVGRect::shapeDependentFillContains(const FloatPoint& point, const WindRule fillRule) const
 {
-    if (isPaintingFallback())
+    if (m_usePathFallback)
         return RenderSVGShape::shapeDependentFillContains(point, fillRule);
     return m_boundingBox.contains(point.x(), point.y());
 }
index bd21538..36837f7 100644 (file)
@@ -43,12 +43,12 @@ private:
     virtual const char* renderName() const { return "RenderSVGRect"; }
 
     virtual void createShape();
-    virtual bool isEmpty() const { return hasPath() ? RenderSVGShape::isEmpty() : m_boundingBox.isEmpty(); };
+    virtual bool isEmpty() const { return m_usePathFallback ? RenderSVGShape::isEmpty() : m_boundingBox.isEmpty(); };
     virtual void fillShape(GraphicsContext*) const;
     virtual void strokeShape(GraphicsContext*) const;
     virtual FloatRect objectBoundingBox() const;
     virtual FloatRect strokeBoundingBox() const;
-    virtual bool shapeDependentStrokeContains(const FloatPoint&) const;
+    virtual bool shapeDependentStrokeContains(const FloatPoint&);
     virtual bool shapeDependentFillContains(const FloatPoint&, const WindRule) const;
 
 private:
@@ -56,6 +56,7 @@ private:
     FloatRect m_innerStrokeRect;
     FloatRect m_outerStrokeRect;
     FloatRect m_strokeBoundingRect;
+    bool m_usePathFallback;
 };
 
 }
index 9c71399..92d7c28 100755 (executable)
@@ -57,7 +57,6 @@ RenderSVGShape::RenderSVGShape(SVGStyledTransformableElement* node)
     , m_needsBoundariesUpdate(false) // Default is false, the cached rects are empty from the beginning.
     , m_needsShapeUpdate(true) // Default is true, so we grab a Path object once from SVGStyledTransformableElement.
     , m_needsTransformUpdate(true) // Default is true, so we grab a AffineTransform object once from SVGStyledTransformableElement.
-    , m_fillFallback(false)
 {
 }
 
@@ -69,7 +68,7 @@ void RenderSVGShape::createShape()
 {
     ASSERT(!m_path);
     m_path = adoptPtr(new Path);
-    ASSERT(isEmpty());
+    ASSERT(RenderSVGShape::isEmpty());
 
     SVGStyledTransformableElement* element = static_cast<SVGStyledTransformableElement*>(node());
     updatePathFromGraphicsElement(element, path());
@@ -79,7 +78,7 @@ void RenderSVGShape::createShape()
 
 bool RenderSVGShape::isEmpty() const
 {
-    return m_path->isEmpty();
+    return path().isEmpty();
 }
 
 void RenderSVGShape::fillShape(GraphicsContext* context) const
@@ -89,7 +88,7 @@ void RenderSVGShape::fillShape(GraphicsContext* context) const
 
 FloatRect RenderSVGShape::objectBoundingBox() const
 {
-    return m_path->fastBoundingRect();
+    return path().fastBoundingRect();
 }
 
 void RenderSVGShape::strokeShape(GraphicsContext* context) const
@@ -98,7 +97,7 @@ void RenderSVGShape::strokeShape(GraphicsContext* context) const
         context->strokePath(path());
 }
 
-bool RenderSVGShape::shapeDependentStrokeContains(const FloatPoint& point) const
+bool RenderSVGShape::shapeDependentStrokeContains(const FloatPoint& point)
 {
     ASSERT(m_path);
     BoundingRectStrokeStyleApplier applier(this, style());
@@ -115,8 +114,7 @@ bool RenderSVGShape::shapeDependentStrokeContains(const FloatPoint& point) const
 
 bool RenderSVGShape::shapeDependentFillContains(const FloatPoint& point, const WindRule fillRule) const
 {
-    ASSERT(m_path);
-    return m_path->contains(point, fillRule);
+    return path().contains(point, fillRule);
 }
 
 bool RenderSVGShape::fillContains(const FloatPoint& point, bool requiresFill, const WindRule fillRule)
@@ -140,7 +138,6 @@ bool RenderSVGShape::strokeContains(const FloatPoint& point, bool requiresStroke
     if (requiresStroke && !RenderSVGResource::strokePaintingResource(this, style(), fallbackColor))
         return false;
 
-    const SVGRenderStyle* svgStyle = style()->svgStyle();
     for (size_t i = 0; i < m_zeroLengthLinecapLocations.size(); ++i) {
         ASSERT(style()->svgStyle()->hasStroke());
         float strokeWidth = this->strokeWidth();
@@ -155,12 +152,6 @@ bool RenderSVGShape::strokeContains(const FloatPoint& point, bool requiresStroke
         }
     }
 
-    if (!svgStyle->strokeDashArray().isEmpty() || svgStyle->strokeMiterLimit() != svgStyle->initialStrokeMiterLimit()
-        || svgStyle->joinStyle() != svgStyle->initialJoinStyle() || svgStyle->capStyle() != svgStyle->initialCapStyle()) {
-        if (!m_path)
-            RenderSVGShape::createShape();
-        return RenderSVGShape::shapeDependentStrokeContains(point);
-    }
     return shapeDependentStrokeContains(point);
 }
 
@@ -173,7 +164,6 @@ void RenderSVGShape::layout()
 
     bool needsShapeUpdate = m_needsShapeUpdate;
     if (needsShapeUpdate || m_needsBoundariesUpdate) {
-        setIsPaintingFallback(false);
         m_path.clear();
         createShape();
         m_needsShapeUpdate = false;
@@ -483,6 +473,15 @@ float RenderSVGShape::strokeWidth() const
     return style()->svgStyle()->strokeWidth().value(lengthContext);
 }
 
+bool RenderSVGShape::hasSmoothStroke() const
+{
+    const SVGRenderStyle* svgStyle = style()->svgStyle();
+    return svgStyle->strokeDashArray().isEmpty()
+        && svgStyle->strokeMiterLimit() == svgStyle->initialStrokeMiterLimit()
+        && svgStyle->joinStyle() == svgStyle->initialJoinStyle()
+        && svgStyle->capStyle() == svgStyle->initialCapStyle();
+}
+
 void RenderSVGShape::inflateWithStrokeAndMarkerBounds()
 {
     const SVGRenderStyle* svgStyle = style()->svgStyle();
index 5d97bfd..f6c22db 100644 (file)
@@ -75,7 +75,6 @@ public:
     virtual void setNeedsTransformUpdate() { m_needsTransformUpdate = true; }
     virtual void fillShape(GraphicsContext*) const;
     virtual void strokeShape(GraphicsContext*) const;
-    bool isPaintingFallback() const { return m_fillFallback; }
 
     Path& path() const
     {
@@ -89,12 +88,12 @@ protected:
     virtual FloatRect objectBoundingBox() const;
     virtual FloatRect strokeBoundingBox() const { return m_strokeAndMarkerBoundingBox; }
     void setStrokeAndMarkerBoundingBox(FloatRect rect) { m_strokeAndMarkerBoundingBox = rect; }
-    virtual bool shapeDependentStrokeContains(const FloatPoint&) const;
+    virtual bool shapeDependentStrokeContains(const FloatPoint&);
     virtual bool shapeDependentFillContains(const FloatPoint&, const WindRule) const;
     float strokeWidth() const;
-    void setIsPaintingFallback(bool isFallback) { m_fillFallback = isFallback; }
     bool hasPath() const { return m_path.get(); }
     bool hasNonScalingStroke() const { return style()->svgStyle()->vectorEffect() == VE_NON_SCALING_STROKE; }
+    bool hasSmoothStroke() const;
 
 private:
     // Hit-detection separated for the fill and the stroke
@@ -148,7 +147,6 @@ private:
     bool m_needsBoundariesUpdate : 1;
     bool m_needsShapeUpdate : 1;
     bool m_needsTransformUpdate : 1;
-    bool m_fillFallback : 1;
 };
 
 inline RenderSVGShape* toRenderSVGShape(RenderObject* object)