SVGPathElement should cache the built-up Path of its non animating pathByteStream()
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 21 Oct 2017 02:33:53 +0000 (02:33 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 21 Oct 2017 02:33:53 +0000 (02:33 +0000)
https://bugs.webkit.org/show_bug.cgi?id=178248

Patch by Said Abou-Hallawa <sabouhallawa@apple.com> on 2017-10-20
Reviewed by Simon Fraser.

Instead of creating a Path object from the non animating pathByteStream()
every time we need to updatePathFromPathElement(), the Path object can be
cached once it is created and used for later calls.

* html/canvas/Path2D.h: buildPathFromString() now returns a Path.

* platform/graphics/Path.h:
* platform/graphics/cairo/PathCairo.cpp:
(WebCore::Path::Path):
(WebCore::Path::operator=):
* platform/graphics/cg/PathCG.cpp:
(WebCore::Path::Path):
(WebCore::Path::operator=):
* platform/graphics/win/PathDirect2D.cpp:
(WebCore::Path::Path):
(WebCore::Path::operator=):
Define the move constructor and the move assignment operator for the the
Path class so a statement like "Path path = buildPathFromString()" won't
go through the copy constructor and the copy assignment operator.

* rendering/style/BasicShapes.cpp:
(WebCore::SVGPathTranslatedByteStream::path const):
* rendering/svg/RenderSVGResourceClipper.cpp:
(WebCore::RenderSVGResourceClipper::pathOnlyClipping):
* rendering/svg/RenderSVGShape.cpp:
(WebCore::RenderSVGShape::updateShapeFromElement):
* rendering/svg/RenderSVGTextPath.cpp:
(WebCore::RenderSVGTextPath::layoutPath const):
* rendering/svg/SVGPathData.cpp:
(WebCore::pathFromCircleElement):
(WebCore::pathFromEllipseElement):
(WebCore::pathFromLineElement):
(WebCore::pathFromPathElement):
(WebCore::pathFromPolygonElement):
(WebCore::pathFromPolylineElement):
(WebCore::pathFromRectElement):
(WebCore::pathFromGraphicsElement):
(WebCore::updatePathFromCircleElement): Deleted.
(WebCore::updatePathFromEllipseElement): Deleted.
(WebCore::updatePathFromLineElement): Deleted.
(WebCore::updatePathFromPathElement): Deleted.
(WebCore::updatePathFromPolygonElement): Deleted.
(WebCore::updatePathFromPolylineElement): Deleted.
(WebCore::updatePathFromRectElement): Deleted.
(WebCore::updatePathFromGraphicsElement): Deleted.
* rendering/svg/SVGPathData.h:
* svg/SVGAnimateMotionElement.cpp:
(WebCore::SVGAnimateMotionElement::parseAttribute):
(WebCore::SVGAnimateMotionElement::updateAnimationPath):
* svg/SVGGraphicsElement.cpp:
(WebCore::SVGGraphicsElement::toClipPath):
* svg/SVGGraphicsElement.h:
Rename updatePathFromElement() to pathFromGraphicsElement().

* svg/SVGPathElement.cpp:
(WebCore::SVGPathElement::parseAttribute): Clear the cache when
m_pathByteStream changes.
(WebCore::SVGPathElement::pathForByteStream const): Caches the m_cachedPath
if it is null.
(WebCore::SVGPathElement::pathSegListChanged): Clear the cache when
m_pathByteStream changes.

* svg/SVGPathElement.h:
* svg/SVGPathUtilities.cpp:
(WebCore::buildPathFromString):
(WebCore::buildPathFromByteStream):
* svg/SVGPathUtilities.h:
Make thes buildPathFromString() and buildPathFromByteStream() return Paths.

* svg/SVGUseElement.cpp:
(WebCore::SVGUseElement::toClipPath):
* svg/SVGUseElement.h:
Make these toClipPath() return Path.

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

21 files changed:
Source/WebCore/ChangeLog
Source/WebCore/html/canvas/Path2D.h
Source/WebCore/platform/graphics/Path.h
Source/WebCore/platform/graphics/cairo/PathCairo.cpp
Source/WebCore/platform/graphics/cg/PathCG.cpp
Source/WebCore/platform/graphics/win/PathDirect2D.cpp
Source/WebCore/rendering/style/BasicShapes.cpp
Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp
Source/WebCore/rendering/svg/RenderSVGShape.cpp
Source/WebCore/rendering/svg/RenderSVGTextPath.cpp
Source/WebCore/rendering/svg/SVGPathData.cpp
Source/WebCore/rendering/svg/SVGPathData.h
Source/WebCore/svg/SVGAnimateMotionElement.cpp
Source/WebCore/svg/SVGGraphicsElement.cpp
Source/WebCore/svg/SVGGraphicsElement.h
Source/WebCore/svg/SVGPathElement.cpp
Source/WebCore/svg/SVGPathElement.h
Source/WebCore/svg/SVGPathUtilities.cpp
Source/WebCore/svg/SVGPathUtilities.h
Source/WebCore/svg/SVGUseElement.cpp
Source/WebCore/svg/SVGUseElement.h

index 9f1ba83..0e319f8 100644 (file)
@@ -1,3 +1,84 @@
+2017-10-20  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        SVGPathElement should cache the built-up Path of its non animating pathByteStream()
+        https://bugs.webkit.org/show_bug.cgi?id=178248
+
+        Reviewed by Simon Fraser.
+
+        Instead of creating a Path object from the non animating pathByteStream()
+        every time we need to updatePathFromPathElement(), the Path object can be
+        cached once it is created and used for later calls.
+
+        * html/canvas/Path2D.h: buildPathFromString() now returns a Path.
+
+        * platform/graphics/Path.h:
+        * platform/graphics/cairo/PathCairo.cpp:
+        (WebCore::Path::Path):
+        (WebCore::Path::operator=):
+        * platform/graphics/cg/PathCG.cpp:
+        (WebCore::Path::Path):
+        (WebCore::Path::operator=):
+        * platform/graphics/win/PathDirect2D.cpp:
+        (WebCore::Path::Path):
+        (WebCore::Path::operator=):
+        Define the move constructor and the move assignment operator for the the
+        Path class so a statement like "Path path = buildPathFromString()" won't 
+        go through the copy constructor and the copy assignment operator.
+
+        * rendering/style/BasicShapes.cpp:
+        (WebCore::SVGPathTranslatedByteStream::path const):
+        * rendering/svg/RenderSVGResourceClipper.cpp:
+        (WebCore::RenderSVGResourceClipper::pathOnlyClipping):
+        * rendering/svg/RenderSVGShape.cpp:
+        (WebCore::RenderSVGShape::updateShapeFromElement):
+        * rendering/svg/RenderSVGTextPath.cpp:
+        (WebCore::RenderSVGTextPath::layoutPath const):
+        * rendering/svg/SVGPathData.cpp:
+        (WebCore::pathFromCircleElement):
+        (WebCore::pathFromEllipseElement):
+        (WebCore::pathFromLineElement):
+        (WebCore::pathFromPathElement):
+        (WebCore::pathFromPolygonElement):
+        (WebCore::pathFromPolylineElement):
+        (WebCore::pathFromRectElement):
+        (WebCore::pathFromGraphicsElement):
+        (WebCore::updatePathFromCircleElement): Deleted.
+        (WebCore::updatePathFromEllipseElement): Deleted.
+        (WebCore::updatePathFromLineElement): Deleted.
+        (WebCore::updatePathFromPathElement): Deleted.
+        (WebCore::updatePathFromPolygonElement): Deleted.
+        (WebCore::updatePathFromPolylineElement): Deleted.
+        (WebCore::updatePathFromRectElement): Deleted.
+        (WebCore::updatePathFromGraphicsElement): Deleted.
+        * rendering/svg/SVGPathData.h:
+        * svg/SVGAnimateMotionElement.cpp:
+        (WebCore::SVGAnimateMotionElement::parseAttribute):
+        (WebCore::SVGAnimateMotionElement::updateAnimationPath):
+        * svg/SVGGraphicsElement.cpp:
+        (WebCore::SVGGraphicsElement::toClipPath):
+        * svg/SVGGraphicsElement.h:
+        Rename updatePathFromElement() to pathFromGraphicsElement().
+
+        * svg/SVGPathElement.cpp:
+        (WebCore::SVGPathElement::parseAttribute): Clear the cache when
+        m_pathByteStream changes.
+        (WebCore::SVGPathElement::pathForByteStream const): Caches the m_cachedPath
+        if it is null.
+        (WebCore::SVGPathElement::pathSegListChanged): Clear the cache when
+        m_pathByteStream changes.
+
+        * svg/SVGPathElement.h:
+        * svg/SVGPathUtilities.cpp:
+        (WebCore::buildPathFromString):
+        (WebCore::buildPathFromByteStream):
+        * svg/SVGPathUtilities.h:
+        Make thes buildPathFromString() and buildPathFromByteStream() return Paths.
+
+        * svg/SVGUseElement.cpp:
+        (WebCore::SVGUseElement::toClipPath):
+        * svg/SVGUseElement.h:
+        Make these toClipPath() return Path.
+
 2017-10-20  Ryosuke Niwa  <rniwa@webkit.org>
 
         Rename insertedInto and removedFrom to insertedIntoAncestor and removedFromAncestor
index 182d5d9..78fe018 100644 (file)
@@ -58,9 +58,7 @@ public:
 
     static Ref<Path2D> create(const String& pathData)
     {
-        Path path;
-        buildPathFromString(pathData, path);
-        return create(path);
+        return create(buildPathFromString(pathData));
     }
 
     ExceptionOr<void> addPath(Path2D&, DOMMatrix2DInit&&);
index 92f6f25..047c948 100644 (file)
@@ -115,7 +115,9 @@ namespace WebCore {
         WEBCORE_EXPORT ~Path();
 
         WEBCORE_EXPORT Path(const Path&);
+        WEBCORE_EXPORT Path(Path&&);
         WEBCORE_EXPORT Path& operator=(const Path&);
+        WEBCORE_EXPORT Path& operator=(Path&&);
         
         static Path polygonPathFromPoints(const Vector<FloatPoint>&);
 
index 82ffc97..7d0c472 100644 (file)
@@ -61,6 +61,12 @@ Path::Path(const Path& other)
     cairo_append_path(cr, pathCopy);
     cairo_path_destroy(pathCopy);
 }
+    
+Path::Path(Path&& other)
+{
+    m_path = other.m_path;
+    other.m_path = nullptr;
+}
 
 PlatformPathPtr Path::ensurePlatformPath()
 {
@@ -89,6 +95,17 @@ Path& Path::operator=(const Path& other)
 
     return *this;
 }
+    
+Path& Path::operator=(Path&& other)
+{
+    if (this == &other)
+        return *this;
+    if (m_path)
+        delete m_path;
+    m_path = other.m_path;
+    other.m_path = nullptr;
+    return *this;
+}
 
 void Path::clear()
 {
index aa494bf..e959abf 100644 (file)
@@ -113,12 +113,30 @@ Path::Path(const Path& other)
     m_path = other.m_path ? CGPathCreateMutableCopy(other.m_path) : 0;
 }
 
+Path::Path(Path&& other)
+{
+    m_path = other.m_path;
+    other.m_path = nullptr;
+}
+    
 Path& Path::operator=(const Path& other)
 {
-    CGMutablePathRef path = other.m_path ? CGPathCreateMutableCopy(other.m_path) : 0;
+    if (this == &other)
+        return *this;
     if (m_path)
         CGPathRelease(m_path);
-    m_path = path;
+    m_path = other.m_path ? CGPathCreateMutableCopy(other.m_path) : nullptr;
+    return *this;
+}
+
+Path& Path::operator=(Path&& other)
+{
+    if (this == &other)
+        return *this;
+    if (m_path)
+        CGPathRelease(m_path);
+    m_path = other.m_path;
+    other.m_path = nullptr;
     return *this;
 }
 
index 1b88fe7..ec4fe86 100644 (file)
@@ -158,13 +158,35 @@ Path::Path(const Path& other)
             entry->Release();
     }
 }
+    
+Path::Path(Path&& other)
+{
+    m_path = other.m_path;
+    m_activePath = other.m_activePath;
+    m_activePathGeometry = other.m_activePathGeometry;
+    other.m_path = nullptr;
+    other.m_activePath = nullptr;
+    other.m_activePathGeometry = nullptr;
+}
 
 Path& Path::operator=(const Path& other)
 {
     m_path = other.m_path;
     m_activePath = other.m_activePath;
     m_activePathGeometry = other.m_activePathGeometry;
+    return *this;
+}
 
+Path& Path::operator=(Path&& other)
+{
+    if (this == &other)
+        return *this;
+    m_path = other.m_path;
+    m_activePath = other.m_activePath;
+    m_activePathGeometry = other.m_activePathGeometry;
+    other.m_path = nullptr;
+    other.m_activePath = nullptr;
+    other.m_activePathGeometry = nullptr;
     return *this;
 }
 
index 86963a9..d574e82 100644 (file)
@@ -73,8 +73,7 @@ struct SVGPathTranslatedByteStream {
 
     Path path() const
     {
-        Path path;
-        buildPathFromByteStream(m_rawStream, path);
+        Path path = buildPathFromByteStream(m_rawStream);
         path.translate(toFloatSize(m_offset));
         return path;
     }
index 53bd921..ebd8c1f 100644 (file)
@@ -102,7 +102,7 @@ bool RenderSVGResourceClipper::pathOnlyClipping(GraphicsContext& context, const
             return false;
         // Fallback to masking, if there is more than one clipping path.
         if (clipPath.isEmpty()) {
-            styled.toClipPath(clipPath);
+            clipPath = styled.toClipPath();
             clipRule = svgStyle.clipRule();
         } else
             return false;
index 32f49de..676782b 100644 (file)
@@ -75,10 +75,7 @@ RenderSVGShape::~RenderSVGShape() = default;
 
 void RenderSVGShape::updateShapeFromElement()
 {
-    m_path = std::make_unique<Path>();
-    ASSERT(RenderSVGShape::isEmpty());
-
-    updatePathFromGraphicsElement(&graphicsElement(), path());
+    m_path = std::make_unique<Path>(pathFromGraphicsElement(&graphicsElement()));
     processMarkerPositions();
 
     m_fillBoundingBox = calculateObjectBoundingBox();
index 3d5f365..9c18e9e 100644 (file)
@@ -49,16 +49,15 @@ Path RenderSVGTextPath::layoutPath() const
     
     SVGPathElement& pathElement = downcast<SVGPathElement>(*targetElement);
     
-    Path pathData;
-    updatePathFromGraphicsElement(&pathElement, pathData);
+    Path path = pathFromGraphicsElement(&pathElement);
 
     // Spec:  The transform attribute on the referenced 'path' element represents a
     // supplemental transformation relative to the current user coordinate system for
     // the current 'text' element, including any adjustments to the current user coordinate
     // system due to a possible transform attribute on the current 'text' element.
     // http://www.w3.org/TR/SVG/text.html#TextPathElement
-    pathData.transform(pathElement.animatedLocalTransform());
-    return pathData;
+    path.transform(pathElement.animatedLocalTransform());
+    return path;
 }
 
 float RenderSVGTextPath::startOffset() const
index 6eeb547..9aa9e1b 100644 (file)
 
 namespace WebCore {
 
-static void updatePathFromCircleElement(SVGElement* element, Path& path)
+static Path pathFromCircleElement(SVGElement& element)
 {
     ASSERT(is<SVGCircleElement>(element));
 
-    SVGLengthContext lengthContext(element);
-    RenderElement* renderer = element->renderer();
+    RenderElement* renderer = element.renderer();
     if (!renderer)
-        return;
+        return { };
+
+    Path path;
     auto& style = renderer->style();
+    SVGLengthContext lengthContext(&element);
     float r = lengthContext.valueForLength(style.svgStyle().r());
     if (r > 0) {
         float cx = lengthContext.valueForLength(style.svgStyle().cx(), LengthModeWidth);
         float cy = lengthContext.valueForLength(style.svgStyle().cy(), LengthModeHeight);
         path.addEllipse(FloatRect(cx - r, cy - r, r * 2, r * 2));
     }
+    return path;
 }
 
-static void updatePathFromEllipseElement(SVGElement* element, Path& path)
+static Path pathFromEllipseElement(SVGElement& element)
 {
-    RenderElement* renderer = element->renderer();
+    RenderElement* renderer = element.renderer();
     if (!renderer)
-        return;
+        return { };
+
     auto& style = renderer->style();
-    SVGLengthContext lengthContext(element);
+    SVGLengthContext lengthContext(&element);
     float rx = lengthContext.valueForLength(style.svgStyle().rx(), LengthModeWidth);
     if (rx <= 0)
-        return;
+        return { };
+
     float ry = lengthContext.valueForLength(style.svgStyle().ry(), LengthModeHeight);
     if (ry <= 0)
-        return;
+        return { };
+
+    Path path;
     float cx = lengthContext.valueForLength(style.svgStyle().cx(), LengthModeWidth);
     float cy = lengthContext.valueForLength(style.svgStyle().cy(), LengthModeHeight);
     path.addEllipse(FloatRect(cx - rx, cy - ry, rx * 2, ry * 2));
+    return path;
 }
 
-static void updatePathFromLineElement(SVGElement* element, Path& path)
+static Path pathFromLineElement(SVGElement& element)
 {
-    SVGLineElement* line = downcast<SVGLineElement>(element);
+    Path path;
+    const auto& line = downcast<SVGLineElement>(element);
 
-    SVGLengthContext lengthContext(element);
-    path.moveTo(FloatPoint(line->x1().value(lengthContext), line->y1().value(lengthContext)));
-    path.addLineTo(FloatPoint(line->x2().value(lengthContext), line->y2().value(lengthContext)));
+    SVGLengthContext lengthContext(&element);
+    path.moveTo(FloatPoint(line.x1().value(lengthContext), line.y1().value(lengthContext)));
+    path.addLineTo(FloatPoint(line.x2().value(lengthContext), line.y2().value(lengthContext)));
+    return path;
 }
 
-static void updatePathFromPathElement(SVGElement* element, Path& path)
+static Path pathFromPathElement(SVGElement& element)
 {
-    buildPathFromByteStream(downcast<SVGPathElement>(element)->pathByteStream(), path);
+    return downcast<SVGPathElement>(element).pathForByteStream();
 }
 
-static void updatePathFromPolygonElement(SVGElement* element, Path& path)
+static Path pathFromPolygonElement(SVGElement& element)
 {
-    auto& points = downcast<SVGPolygonElement>(element)->animatedPoints()->values();
+    auto& points = downcast<SVGPolygonElement>(element).animatedPoints()->values();
     if (points.isEmpty())
-        return;
+        return { };
 
+    Path path;
     path.moveTo(points.first());
 
     unsigned size = points.size();
@@ -101,35 +112,41 @@ static void updatePathFromPolygonElement(SVGElement* element, Path& path)
         path.addLineTo(points.at(i));
 
     path.closeSubpath();
+    return path;
 }
 
-static void updatePathFromPolylineElement(SVGElement* element, Path& path)
+static Path pathFromPolylineElement(SVGElement& element)
 {
-    auto& points = downcast<SVGPolylineElement>(element)->animatedPoints()->values();
+    auto& points = downcast<SVGPolylineElement>(element).animatedPoints()->values();
     if (points.isEmpty())
-        return;
+        return { };
 
+    Path path;
     path.moveTo(points.first());
 
     unsigned size = points.size();
     for (unsigned i = 1; i < size; ++i)
         path.addLineTo(points.at(i));
+    return path;
 }
 
-static void updatePathFromRectElement(SVGElement* element, Path& path)
+static Path pathFromRectElement(SVGElement& element)
 {
-    RenderElement* renderer = element->renderer();
+    RenderElement* renderer = element.renderer();
     if (!renderer)
-        return;
+        return { };
 
     auto& style = renderer->style();
-    SVGLengthContext lengthContext(element);
+    SVGLengthContext lengthContext(&element);
     float width = lengthContext.valueForLength(style.width(), LengthModeWidth);
     if (width <= 0)
-        return;
+        return { };
+
     float height = lengthContext.valueForLength(style.height(), LengthModeHeight);
     if (height <= 0)
-        return;
+        return { };
+
+    Path path;
     float x = lengthContext.valueForLength(style.svgStyle().x(), LengthModeWidth);
     float y = lengthContext.valueForLength(style.svgStyle().y(), LengthModeHeight);
     float rx = lengthContext.valueForLength(style.svgStyle().rx(), LengthModeWidth);
@@ -145,32 +162,34 @@ static void updatePathFromRectElement(SVGElement* element, Path& path)
         // the native method uses a different line dash origin, causing svg/custom/dashOrigin.svg to fail.
         // See bug https://bugs.webkit.org/show_bug.cgi?id=79932 which tracks this issue.
         path.addRoundedRect(FloatRect(x, y, width, height), FloatSize(rx, ry), Path::PreferBezierRoundedRect);
-        return;
+        return path;
     }
 
     path.addRect(FloatRect(x, y, width, height));
+    return path;
 }
 
-void updatePathFromGraphicsElement(SVGElement* element, Path& path)
+Path pathFromGraphicsElement(SVGElement* element)
 {
     ASSERT(element);
-    ASSERT(path.isEmpty());
 
-    typedef void (*PathUpdateFunction)(SVGElement*, Path&);
-    static HashMap<AtomicStringImpl*, PathUpdateFunction>* map = 0;
+    typedef Path (*PathFromFunction)(SVGElement&);
+    static HashMap<AtomicStringImpl*, PathFromFunction>* map = 0;
     if (!map) {
-        map = new HashMap<AtomicStringImpl*, PathUpdateFunction>;
-        map->set(SVGNames::circleTag.localName().impl(), updatePathFromCircleElement);
-        map->set(SVGNames::ellipseTag.localName().impl(), updatePathFromEllipseElement);
-        map->set(SVGNames::lineTag.localName().impl(), updatePathFromLineElement);
-        map->set(SVGNames::pathTag.localName().impl(), updatePathFromPathElement);
-        map->set(SVGNames::polygonTag.localName().impl(), updatePathFromPolygonElement);
-        map->set(SVGNames::polylineTag.localName().impl(), updatePathFromPolylineElement);
-        map->set(SVGNames::rectTag.localName().impl(), updatePathFromRectElement);
+        map = new HashMap<AtomicStringImpl*, PathFromFunction>;
+        map->set(SVGNames::circleTag.localName().impl(), pathFromCircleElement);
+        map->set(SVGNames::ellipseTag.localName().impl(), pathFromEllipseElement);
+        map->set(SVGNames::lineTag.localName().impl(), pathFromLineElement);
+        map->set(SVGNames::pathTag.localName().impl(), pathFromPathElement);
+        map->set(SVGNames::polygonTag.localName().impl(), pathFromPolygonElement);
+        map->set(SVGNames::polylineTag.localName().impl(), pathFromPolylineElement);
+        map->set(SVGNames::rectTag.localName().impl(), pathFromRectElement);
     }
 
-    if (PathUpdateFunction pathUpdateFunction = map->get(element->localName().impl()))
-        (*pathUpdateFunction)(element, path);
+    if (PathFromFunction pathFromFunction = map->get(element->localName().impl()))
+        return (*pathFromFunction)(*element);
+    
+    return { };
 }
 
 } // namespace WebCore
index 9da6bad..62dd819 100644 (file)
@@ -24,6 +24,6 @@ namespace WebCore {
 class SVGElement;
 class Path;
 
-void updatePathFromGraphicsElement(SVGElement*, Path&);
+Path pathFromGraphicsElement(SVGElement*);
 
 } // namespace WebCore
index 29aa054..0c7416d 100644 (file)
@@ -96,8 +96,7 @@ bool SVGAnimateMotionElement::hasValidAttributeName()
 void SVGAnimateMotionElement::parseAttribute(const QualifiedName& name, const AtomicString& value)
 {
     if (name == SVGNames::pathAttr) {
-        m_path = Path();
-        buildPathFromString(value, m_path);
+        m_path = buildPathFromString(value);
         updateAnimationPath();
         return;
     }
@@ -125,7 +124,7 @@ void SVGAnimateMotionElement::updateAnimationPath()
     for (auto& mPath : childrenOfType<SVGMPathElement>(*this)) {
         SVGPathElement* pathElement = mPath.pathElement();
         if (pathElement) {
-            updatePathFromGraphicsElement(pathElement, m_animationPath);
+            m_animationPath = pathFromGraphicsElement(pathElement);
             foundMPath = true;
             break;
         }
index db8717c..495d011 100644 (file)
@@ -199,11 +199,12 @@ RenderPtr<RenderElement> SVGGraphicsElement::createElementRenderer(RenderStyle&&
     return createRenderer<RenderSVGPath>(*this, WTFMove(style));
 }
 
-void SVGGraphicsElement::toClipPath(Path& path)
+Path SVGGraphicsElement::toClipPath()
 {
-    updatePathFromGraphicsElement(this, path);
+    Path path = pathFromGraphicsElement(this);
     // FIXME: How do we know the element has done a layout?
     path.transform(animatedLocalTransform());
+    return path;
 }
 
 Ref<SVGStringList> SVGGraphicsElement::requiredFeatures()
index 7523cfc..065d0d3 100644 (file)
@@ -56,7 +56,7 @@ public:
     void setShouldIsolateBlending(bool isolate) { m_shouldIsolateBlending = isolate; }
 
     // "base class" methods for all the elements which render as paths
-    virtual void toClipPath(Path&);
+    virtual Path toClipPath();
     RenderPtr<RenderElement> createElementRenderer(RenderStyle&&, const RenderTreePosition&) override;
 
     size_t approximateMemoryCost() const override { return sizeof(*this); }
index 40a0ca7..c2dd849 100644 (file)
@@ -225,6 +225,7 @@ void SVGPathElement::parseAttribute(const QualifiedName& name, const AtomicStrin
     if (name == SVGNames::dAttr) {
         if (!buildSVGPathByteStreamFromString(value, m_pathByteStream, UnalteredParsing))
             document().accessSVGExtensions().reportError("Problem parsing d=\"" + value + "\"");
+        m_cachedPath = std::nullopt;
         return;
     }
 
@@ -304,6 +305,19 @@ const SVGPathByteStream& SVGPathElement::pathByteStream() const
 
     return *animatedPathByteStream;
 }
+    
+Path SVGPathElement::pathForByteStream() const
+{
+    const auto& pathByteStreamToUse = pathByteStream();
+
+    if (&pathByteStreamToUse == &m_pathByteStream) {
+        if (!m_cachedPath)
+            m_cachedPath = buildPathFromByteStream(m_pathByteStream);
+        return *m_cachedPath;
+    }
+    
+    return buildPathFromByteStream(pathByteStreamToUse);
+}
 
 Ref<SVGAnimatedProperty> SVGPathElement::lookupOrCreateDWrapper(SVGElement* contextElement)
 {
@@ -382,6 +396,7 @@ void SVGPathElement::pathSegListChanged(SVGPathSegRole role, ListModification li
             appendSVGPathByteStreamFromSVGPathSeg(m_pathSegList.value.last().copyRef(), m_pathByteStream, UnalteredParsing);
         } else
             buildSVGPathByteStreamFromSVGPathSegListValues(m_pathSegList.value, m_pathByteStream, UnalteredParsing);
+        m_cachedPath = std::nullopt;
         break;
     case PathSegUndefinedRole:
         return;
index 647c7d4..f2981aa 100644 (file)
@@ -20,6 +20,7 @@
 
 #pragma once
 
+#include "Path.h"
 #include "SVGAnimatedBoolean.h"
 #include "SVGAnimatedNumber.h"
 #include "SVGExternalResourcesRequired.h"
@@ -52,8 +53,7 @@ class SVGPathSegCurvetoQuadraticSmoothRel;
 class SVGPathSegList;
 class SVGPoint;
 
-class SVGPathElement final : public SVGGraphicsElement,
-                             public SVGExternalResourcesRequired {
+class SVGPathElement final : public SVGGraphicsElement, public SVGExternalResourcesRequired {
 public:
     static Ref<SVGPathElement> create(const QualifiedName&, Document&);
     
@@ -88,6 +88,7 @@ public:
     RefPtr<SVGPathSegList> animatedNormalizedPathSegList();
 
     const SVGPathByteStream& pathByteStream() const;
+    Path pathForByteStream() const;
 
     void pathSegListChanged(SVGPathSegRole, ListModification = ListModificationUnknown);
 
@@ -131,6 +132,7 @@ private:
 
 private:
     SVGPathByteStream m_pathByteStream;
+    mutable std::optional<Path> m_cachedPath;
     mutable SVGSynchronizableAnimatedProperty<SVGPathSegListValues> m_pathSegList;
     WeakPtrFactory<SVGPathElement> m_weakPtrFactory;
     bool m_isAnimValObserved;
index 22194af..ce4613a 100644 (file)
 
 namespace WebCore {
 
-bool buildPathFromString(const String& d, Path& result)
+Path buildPathFromString(const String& d)
 {
     if (d.isEmpty())
-        return true;
+        return { };
 
-    SVGPathBuilder builder(result);
+    Path path;
+    SVGPathBuilder builder(path);
     SVGPathStringSource source(d);
-    return SVGPathParser::parse(source, builder);
+    SVGPathParser::parse(source, builder);
+    return path;
 }
 
 String buildStringFromPath(const Path& path)
@@ -130,14 +132,16 @@ bool appendSVGPathByteStreamFromSVGPathSeg(RefPtr<SVGPathSeg>&& pathSeg, SVGPath
     return ok;
 }
 
-bool buildPathFromByteStream(const SVGPathByteStream& stream, Path& result)
+Path buildPathFromByteStream(const SVGPathByteStream& stream)
 {
     if (stream.isEmpty())
-        return true;
+        return { };
 
-    SVGPathBuilder builder(result);
+    Path path;
+    SVGPathBuilder builder(path);
     SVGPathByteStreamSource source(stream);
-    return SVGPathParser::parse(source, builder);
+    SVGPathParser::parse(source, builder);
+    return path;
 }
 
 bool buildSVGPathSegListValuesFromByteStream(const SVGPathByteStream& stream, SVGPathElement& element, SVGPathSegListValues& result, PathParsingMode parsingMode)
index df94e27..216766e 100644 (file)
@@ -33,8 +33,8 @@ class SVGPathSeg;
 class SVGPathSegListValues;
 
 // String/SVGPathByteStream -> Path
-bool buildPathFromString(const String&, Path&);
-bool buildPathFromByteStream(const SVGPathByteStream&, Path&);
+Path buildPathFromString(const String&);
+Path buildPathFromByteStream(const SVGPathByteStream&);
 
 // Path -> String
 String buildStringFromPath(const Path&);
index 8554240..f0f6f3f 100644 (file)
@@ -285,25 +285,24 @@ static bool isDirectReference(const SVGElement& element)
         || element.hasTagName(textTag);
 }
 
-void SVGUseElement::toClipPath(Path& path)
+Path SVGUseElement::toClipPath()
 {
-    ASSERT(path.isEmpty());
-
     auto* targetClone = this->targetClone();
     if (!is<SVGGraphicsElement>(targetClone))
-        return;
+        return { };
 
     if (!isDirectReference(*targetClone)) {
         // Spec: Indirect references are an error (14.3.5)
         document().accessSVGExtensions().reportError(ASCIILiteral("Not allowed to use indirect reference in <clip-path>"));
-        return;
+        return { };
     }
 
-    downcast<SVGGraphicsElement>(*targetClone).toClipPath(path);
+    Path path = downcast<SVGGraphicsElement>(*targetClone).toClipPath();
     SVGLengthContext lengthContext(this);
     // FIXME: Find a way to do this without manual resolution of x/y here. It's potentially incorrect.
     path.translate(FloatSize(x().value(lengthContext), y().value(lengthContext)));
     path.transform(animatedLocalTransform());
+    return path;
 }
 
 RenderElement* SVGUseElement::rendererClipChild() const
index c5c93a9..0bcd1bb 100644 (file)
@@ -64,7 +64,7 @@ private:
     void svgAttributeChanged(const QualifiedName&) override;
     void willRecalcStyle(Style::Change) override;
     RenderPtr<RenderElement> createElementRenderer(RenderStyle&&, const RenderTreePosition&) override;
-    void toClipPath(Path&) override;
+    Path toClipPath() override;
     bool haveLoadedRequiredResources() override;
     void finishParsingChildren() override;
     bool selfHasRelativeLengths() const override;