Path::applyElements() should not trigger creation of a platform path …

…for simple segments https://bugs.webkit.org/show_bug.cgi?id=259795 rdar://113355740 Reviewed by Simon Fraser. Allow enumrating PathStream if it consist only of Move/Line/Quadratic/Cubic/Close segments without creating the platform path. Transform single segment and PathStream-based paths in place when applyElements() is called, to avoid the overhead of generating a platform path. If other segment types are in the path, continue to convert to a platform path first. * Source/WebCore/platform/graphics/Path.cpp: (WebCore::Path::closeSubpath): (WebCore::Path::applyElements const): * Source/WebCore/platform/graphics/PathImpl.h: * Source/WebCore/platform/graphics/PathSegment.cpp: (WebCore::PathSegment::canApplyElements const): (WebCore::PathSegment::applyElements const): * Source/WebCore/platform/graphics/PathSegment.h: * Source/WebCore/platform/graphics/PathSegmentData.h: (WebCore::PathArcTo::applyElements const): Deleted. (WebCore::PathArc::applyElements const): Deleted. (WebCore::PathEllipse::applyElements const): Deleted. (WebCore::PathEllipseInRect::applyElements const): Deleted. (WebCore::PathRect::applyElements const): Deleted. (WebCore::PathRoundedRect::applyElements const): Deleted. (WebCore::PathDataArc::applyElements const): Deleted. * Source/WebCore/platform/graphics/PathStream.cpp: (WebCore::PathStream::applyElements const): * Source/WebCore/platform/graphics/PathStream.h: * Source/WebCore/platform/graphics/cairo/PathCairo.cpp: (WebCore::PathCairo::applyElements const): * Source/WebCore/platform/graphics/cairo/PathCairo.h: * Source/WebCore/platform/graphics/cg/PathCG.cpp: (WebCore::PathCG::applyElements const): * Source/WebCore/platform/graphics/cg/PathCG.h: Canonical link: https://commits.webkit.org/266564@main
WebKit · Aug 4, 2023 · c9b0bf1 · c9b0bf1
1 parent eb66cb9
commit c9b0bf1
Show file tree

Hide file tree

Showing 11 changed files with 78 additions and 23 deletions.
diff --git a/Source/WebCore/platform/graphics/Path.cpp b/Source/WebCore/platform/graphics/Path.cpp
@@ -299,7 +299,7 @@ void Path::addRoundedRect(const RoundedRect& rect)
 
 void Path::closeSubpath()
 {
-    if (isEmpty())
+    if (isEmpty() || isClosed())
         return;
 
     ensureImpl().closeSubpath();
@@ -323,6 +323,17 @@ void Path::applySegments(const PathSegmentApplier& applier) const
 
 void Path::applyElements(const PathElementApplier& applier) const
 {
+    if (isEmpty())
+        return;
+
+    auto segment = asSingle();
+    if (segment && segment->applyElements(applier))
+        return;
+
+    auto impl = asImpl();
+    if (impl && impl->applyElements(applier))
+        return;
+
     const_cast<Path&>(*this).ensurePlatformPathImpl().applyElements(applier);
 }
 

diff --git a/Source/WebCore/platform/graphics/PathImpl.h b/Source/WebCore/platform/graphics/PathImpl.h
@@ -70,7 +70,7 @@ class PathImpl {
     void addBeziersForRoundedRect(const FloatRoundedRect&);
 
     virtual void applySegments(const PathSegmentApplier&) const = 0;
-    virtual void applyElements(const PathElementApplier&) const = 0;
+    virtual bool applyElements(const PathElementApplier&) const = 0;
 
     virtual std::optional<PathSegment> singleSegment() const { return std::nullopt; }
     virtual std::optional<PathDataLine> singleDataLine() const { return std::nullopt; }

diff --git a/Source/WebCore/platform/graphics/PathSegment.cpp b/Source/WebCore/platform/graphics/PathSegment.cpp
@@ -65,10 +65,21 @@ void PathSegment::addToImpl(PathImpl& impl) const
     });
 }
 
-void PathSegment::applyElements(const PathElementApplier& applier) const
+bool PathSegment::canApplyElements() const
 {
-    WTF::switchOn(m_data, [&](auto& data) {
-        data.applyElements(applier);
+    return WTF::switchOn(m_data, [&](auto& data) {
+        return data.canApplyElements;
+    });
+}
+
+bool PathSegment::applyElements(const PathElementApplier& applier) const
+{
+    return WTF::switchOn(m_data, [&](auto& data) -> bool {
+        if constexpr (std::decay_t<decltype(data)>::canApplyElements) {
+            data.applyElements(applier);
+            return true;
+        }
+        return false;
     });
 }
 

diff --git a/Source/WebCore/platform/graphics/PathSegment.h b/Source/WebCore/platform/graphics/PathSegment.h
@@ -66,7 +66,9 @@ class PathSegment {
     void extendBoundingRect(const FloatPoint& currentPoint, const FloatPoint& lastMoveToPoint, FloatRect& boundingRect) const;
 
     void addToImpl(PathImpl&) const;
-    void applyElements(const PathElementApplier&) const;
+
+    bool canApplyElements() const;
+    bool applyElements(const PathElementApplier&) const;
 
 private:
     Data m_data;

diff --git a/Source/WebCore/platform/graphics/PathSegmentData.h b/Source/WebCore/platform/graphics/PathSegmentData.h
@@ -41,6 +41,8 @@ class PathImpl;
 struct PathMoveTo {
     FloatPoint point;
 
+    static constexpr bool canApplyElements = true;
+
     bool operator==(const PathMoveTo&) const = default;
 
     FloatPoint calculateEndPoint(const FloatPoint& currentPoint, FloatPoint& lastMoveToPoint) const;
@@ -57,6 +59,8 @@ WEBCORE_EXPORT WTF::TextStream& operator<<(WTF::TextStream&, const PathMoveTo&);
 struct PathLineTo {
     FloatPoint point;
 
+    static constexpr bool canApplyElements = true;
+
     bool operator==(const PathLineTo&) const = default;
 
     FloatPoint calculateEndPoint(const FloatPoint& currentPoint, FloatPoint& lastMoveToPoint) const;
@@ -74,6 +78,8 @@ struct PathQuadCurveTo {
     FloatPoint controlPoint;
     FloatPoint endPoint;
 
+    static constexpr bool canApplyElements = true;
+
     bool operator==(const PathQuadCurveTo&) const = default;
 
     FloatPoint calculateEndPoint(const FloatPoint& currentPoint, FloatPoint& lastMoveToPoint) const;
@@ -92,6 +98,8 @@ struct PathBezierCurveTo {
     FloatPoint controlPoint2;
     FloatPoint endPoint;
 
+    static constexpr bool canApplyElements = true;
+
     bool operator==(const PathBezierCurveTo&) const = default;
 
     FloatPoint calculateEndPoint(const FloatPoint& currentPoint, FloatPoint& lastMoveToPoint) const;
@@ -110,6 +118,8 @@ struct PathArcTo {
     FloatPoint controlPoint2;
     float radius;
 
+    static constexpr bool canApplyElements = false;
+
     bool operator==(const PathArcTo&) const = default;
 
     FloatPoint calculateEndPoint(const FloatPoint& currentPoint, FloatPoint& lastMoveToPoint) const;
@@ -118,7 +128,6 @@ struct PathArcTo {
     void extendBoundingRect(const FloatPoint& currentPoint, const FloatPoint& lastMoveToPoint, FloatRect& boundingRect) const;
 
     void addToImpl(PathImpl&) const;
-    [[noreturn]] void applyElements(const PathElementApplier&) const { RELEASE_ASSERT_NOT_REACHED(); }
 };
 
 WEBCORE_EXPORT WTF::TextStream& operator<<(WTF::TextStream&, const PathArcTo&);
@@ -130,6 +139,8 @@ struct PathArc {
     float endAngle;
     RotationDirection direction;
 
+    static constexpr bool canApplyElements = false;
+
     bool operator==(const PathArc&) const = default;
 
     FloatPoint calculateEndPoint(const FloatPoint& currentPoint, FloatPoint& lastMoveToPoint) const;
@@ -138,7 +149,6 @@ struct PathArc {
     void extendBoundingRect(const FloatPoint& currentPoint, const FloatPoint& lastMoveToPoint, FloatRect& boundingRect) const;
 
     void addToImpl(PathImpl&) const;
-    [[noreturn]] void applyElements(const PathElementApplier&) const { RELEASE_ASSERT_NOT_REACHED(); }
 };
 
 WEBCORE_EXPORT WTF::TextStream& operator<<(WTF::TextStream&, const PathArc&);
@@ -152,6 +162,8 @@ struct PathEllipse {
     float endAngle;
     RotationDirection direction;
 
+    static constexpr bool canApplyElements = false;
+
     bool operator==(const PathEllipse&) const = default;
 
     FloatPoint calculateEndPoint(const FloatPoint& currentPoint, FloatPoint& lastMoveToPoint) const;
@@ -160,14 +172,15 @@ struct PathEllipse {
     void extendBoundingRect(const FloatPoint& currentPoint, const FloatPoint& lastMoveToPoint, FloatRect& boundingRect) const;
 
     void addToImpl(PathImpl&) const;
-    [[noreturn]] void applyElements(const PathElementApplier&) const { RELEASE_ASSERT_NOT_REACHED(); }
 };
 
 WEBCORE_EXPORT WTF::TextStream& operator<<(WTF::TextStream&, const PathEllipse&);
 
 struct PathEllipseInRect {
     FloatRect rect;
 
+    static constexpr bool canApplyElements = false;
+
     bool operator==(const PathEllipseInRect&) const = default;
 
     FloatPoint calculateEndPoint(const FloatPoint& currentPoint, FloatPoint& lastMoveToPoint) const;
@@ -176,14 +189,15 @@ struct PathEllipseInRect {
     void extendBoundingRect(const FloatPoint& currentPoint, const FloatPoint& lastMoveToPoint, FloatRect& boundingRect) const;
 
     void addToImpl(PathImpl&) const;
-    [[noreturn]] void applyElements(const PathElementApplier&) const { RELEASE_ASSERT_NOT_REACHED(); }
 };
 
 WEBCORE_EXPORT WTF::TextStream& operator<<(WTF::TextStream&, const PathEllipseInRect&);
 
 struct PathRect {
     FloatRect rect;
 
+    static constexpr bool canApplyElements = false;
+
     bool operator==(const PathRect&) const = default;
 
     FloatPoint calculateEndPoint(const FloatPoint& currentPoint, FloatPoint& lastMoveToPoint) const;
@@ -192,7 +206,6 @@ struct PathRect {
     void extendBoundingRect(const FloatPoint& currentPoint, const FloatPoint& lastMoveToPoint, FloatRect& boundingRect) const;
 
     void addToImpl(PathImpl&) const;
-    [[noreturn]] void applyElements(const PathElementApplier&) const { RELEASE_ASSERT_NOT_REACHED(); }
 };
 
 WEBCORE_EXPORT WTF::TextStream& operator<<(WTF::TextStream&, const PathRect&);
@@ -206,6 +219,8 @@ struct PathRoundedRect {
     FloatRoundedRect roundedRect;
     Strategy strategy;
 
+    static constexpr bool canApplyElements = false;
+
     bool operator==(const PathRoundedRect&) const = default;
 
     FloatPoint calculateEndPoint(const FloatPoint& currentPoint, FloatPoint& lastMoveToPoint) const;
@@ -214,7 +229,6 @@ struct PathRoundedRect {
     void extendBoundingRect(const FloatPoint& currentPoint, const FloatPoint& lastMoveToPoint, FloatRect& boundingRect) const;
 
     void addToImpl(PathImpl&) const;
-    [[noreturn]] void applyElements(const PathElementApplier&) const { RELEASE_ASSERT_NOT_REACHED(); }
 };
 
 WEBCORE_EXPORT WTF::TextStream& operator<<(WTF::TextStream&, const PathRoundedRect&);
@@ -223,6 +237,8 @@ struct PathDataLine {
     FloatPoint start;
     FloatPoint end;
 
+    static constexpr bool canApplyElements = true;
+
     bool operator==(const PathDataLine&) const = default;
 
     FloatPoint calculateEndPoint(const FloatPoint& currentPoint, FloatPoint& lastMoveToPoint) const;
@@ -241,6 +257,8 @@ struct PathDataQuadCurve {
     FloatPoint controlPoint;
     FloatPoint endPoint;
 
+    static constexpr bool canApplyElements = true;
+
     bool operator==(const PathDataQuadCurve&) const = default;
 
     FloatPoint calculateEndPoint(const FloatPoint& currentPoint, FloatPoint& lastMoveToPoint) const;
@@ -260,6 +278,8 @@ struct PathDataBezierCurve {
     FloatPoint controlPoint2;
     FloatPoint endPoint;
 
+    static constexpr bool canApplyElements = true;
+
     bool operator==(const PathDataBezierCurve&) const = default;
 
     FloatPoint calculateEndPoint(const FloatPoint& currentPoint, FloatPoint& lastMoveToPoint) const;
@@ -279,6 +299,8 @@ struct PathDataArc {
     FloatPoint controlPoint2;
     float radius;
 
+    static constexpr bool canApplyElements = false;
+
     bool operator==(const PathDataArc&) const = default;
 
     FloatPoint calculateEndPoint(const FloatPoint& currentPoint, FloatPoint& lastMoveToPoint) const;
@@ -287,12 +309,13 @@ struct PathDataArc {
     void extendBoundingRect(const FloatPoint& currentPoint, const FloatPoint& lastMoveToPoint, FloatRect& boundingRect) const;
 
     void addToImpl(PathImpl&) const;
-    [[noreturn]] void applyElements(const PathElementApplier&) const { RELEASE_ASSERT_NOT_REACHED(); }
 };
 
 WEBCORE_EXPORT WTF::TextStream& operator<<(WTF::TextStream&, const PathDataArc&);
 
 struct PathCloseSubpath {
+    static constexpr bool canApplyElements = true;
+
     bool operator==(const PathCloseSubpath&) const = default;
 
     FloatPoint calculateEndPoint(const FloatPoint& currentPoint, FloatPoint& lastMoveToPoint) const;

diff --git a/Source/WebCore/platform/graphics/PathStream.cpp b/Source/WebCore/platform/graphics/PathStream.cpp
@@ -193,10 +193,17 @@ void PathStream::applySegments(const PathSegmentApplier& applier) const
         applier(segment);
 }
 
-void PathStream::applyElements(const PathElementApplier& applier) const
+bool PathStream::applyElements(const PathElementApplier& applier) const
 {
+    for (auto& segment : m_segmentsData->segments) {
+        if (!segment.canApplyElements())
+            return false;
+    }
+
     for (auto& segment : m_segmentsData->segments)
         segment.applyElements(applier);
+
+    return true;
 }
 
 std::optional<PathSegment> PathStream::singleSegment() const

diff --git a/Source/WebCore/platform/graphics/PathStream.h b/Source/WebCore/platform/graphics/PathStream.h
@@ -68,7 +68,7 @@ class PathStream final : public PathImpl {
     WEBCORE_EXPORT const Vector<PathSegment>& segments() const;
 
     void applySegments(const PathSegmentApplier&) const final;
-    void applyElements(const PathElementApplier&) const final;
+    bool applyElements(const PathElementApplier&) const final;
 
     FloatRect fastBoundingRect() const final;
     FloatRect boundingRect() const final;

diff --git a/Source/WebCore/platform/graphics/cairo/PathCairo.cpp b/Source/WebCore/platform/graphics/cairo/PathCairo.cpp
@@ -347,12 +347,10 @@ void PathCairo::applySegments(const PathSegmentApplier& applier) const
     });
 }
 
-void PathCairo::applyElements(const PathElementApplier& applier) const
+bool PathCairo::applyElements(const PathElementApplier& applier) const
 {
-    if (m_elementsStream) {
-        m_elementsStream->applyElements(applier);
-        return;
-    }
+    if (m_elementsStream && m_elementsStream->applyElements(applier))
+        return true;
 
     CairoUniquePtr<cairo_path_t> pathCopy(cairo_copy_path(platformPath()));
     cairo_path_data_t* data;
@@ -387,6 +385,8 @@ void PathCairo::applyElements(const PathElementApplier& applier) const
             break;
         }
     }
+
+    return true;
 }
 
 bool PathCairo::isEmpty() const

diff --git a/Source/WebCore/platform/graphics/cairo/PathCairo.h b/Source/WebCore/platform/graphics/cairo/PathCairo.h
@@ -52,7 +52,7 @@ class PathCairo final : public PathImpl {
 
     void addPath(const PathCairo&, const AffineTransform&);
 
-    void applyElements(const PathElementApplier&) const;
+    bool applyElements(const PathElementApplier&) const final;
 
     void transform(const AffineTransform&);
 

diff --git a/Source/WebCore/platform/graphics/cg/PathCG.cpp b/Source/WebCore/platform/graphics/cg/PathCG.cpp
@@ -290,9 +290,10 @@ static void pathElementApplierCallback(void* info, const CGPathElement* element)
     }
 }
 
-void PathCG::applyElements(const PathElementApplier& applier) const
+bool PathCG::applyElements(const PathElementApplier& applier) const
 {
     CGPathApply(platformPath(), (void*)&applier, pathElementApplierCallback);
+    return true;
 }
 
 bool PathCG::isEmpty() const

diff --git a/Source/WebCore/platform/graphics/cg/PathCG.h b/Source/WebCore/platform/graphics/cg/PathCG.h
@@ -49,7 +49,7 @@ class PathCG final : public PathImpl {
 
     void addPath(const PathCG&, const AffineTransform&);
 
-    void applyElements(const PathElementApplier&) const final;
+    bool applyElements(const PathElementApplier&) const final;
 
     void transform(const AffineTransform&);