[webkit-changes] [195970] trunk/Source

Mon, 01 Feb 2016 11:45:10 -0800

Title: [195970] trunk/Source
Revision
195970
Author
[email protected]
Date
2016-02-01 11:45:18 -0800 (Mon, 01 Feb 2016)

Log Message

Cache the Path instead of creating it every time it is required
https://bugs.webkit.org/show_bug.cgi?id=152939

Patch by Said Abou-Hallawa <[email protected]> on 2016-02-01
Reviewed by Darin Adler.

Source/WebCore:

Instead of creating the Path object every time it is required, we should
cache it in an LRU cache. TinyLRUCache returns a reference to the cached
entry so we do not have to pay the cost of copying it either.

* platform/graphics/FloatRoundedRect.h:
(WebCore::operator!=):
Implement the inequality operator for FloatRoundedRect since it is
called by TinyLRUCache.

* rendering/ClipPathOperation.h:
Return a reference to the path in the cache since instead of creating a
new copy.

* rendering/style/BasicShapes.cpp:
(WebCore::SVGPathTranslatedByteStream::SVGPathTranslatedByteStream):
(WebCore::SVGPathTranslatedByteStream::operator==):
(WebCore::SVGPathTranslatedByteStream::operator!=):
(WebCore::SVGPathTranslatedByteStream::isEmpty):
(WebCore::SVGPathTranslatedByteStream::path):
This struct holds an offset and an SVGPathByteStream. It is the key of
the LRU cache for the the translated SVGPathByteStream.

(WebCore::EllipsePathPolicy::isKeyNull):
(WebCore::EllipsePathPolicy::createValueForKey):
(WebCore::RoundedRectPathPolicy::isKeyNull):
(WebCore::RoundedRectPathPolicy::createValueForKey):
(WebCore::PolygonPathPolicy::isKeyNull):
(WebCore::PolygonPathPolicy::createValueForKey):
(WebCore::TranslatedByteStreamPathPolicy::isKeyNull):
(WebCore::TranslatedByteStreamPathPolicy::createValueForKey):
Inherit from the LRU cache policy template, so have a specific name for
the desired path contents and pass this class explicitly to the LRU cache
template.

(WebCore::cachedEllipsePath):
(WebCore::cachedRoundedRectPath):
(WebCore::cachedPolygonPath):
(WebCore::cachedTranslatedByteStreamPath):
Return a cached path object for specific path contents.

(WebCore::BasicShapeCircle::path):
(WebCore::BasicShapeEllipse::path):
(WebCore::BasicShapePolygon::path):
(WebCore::BasicShapePath::path):
(WebCore::BasicShapeInset::path):
Get the Path object from the cache; create a new one if it does not exist.

* rendering/style/BasicShapes.h:
Change the prototype of the path() function to return a reference to the
path in the cache instead of a having to copying it.

* svg/SVGPathByteStream.h:
(WebCore::SVGPathByteStream::operator!=):
Implement the inequality operator for SVGPathByteStream because it is
called by TinyLRUCache.

Source/WTF:

If the key type of an LRU cache can't to be strongly tided to a specific
data type; e.g. FloatRect -> Path, we need to be able to pass the policy
type to the TinyLRUCache template instead of just specializing it. This
will make the code more readable and will allow different caches for the
same key type.

* wtf/TinyLRUCache.h:
(WebCore::TinyLRUCache::get):

Modified Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (195969 => 195970)


--- trunk/Source/WTF/ChangeLog	2016-02-01 19:42:14 UTC (rev 195969)
+++ trunk/Source/WTF/ChangeLog	2016-02-01 19:45:18 UTC (rev 195970)
@@ -1,3 +1,19 @@
+2016-02-01  Said Abou-Hallawa  <[email protected]>
+
+        Cache the Path instead of creating it every time it is required
+        https://bugs.webkit.org/show_bug.cgi?id=152939
+
+        Reviewed by Darin Adler.
+
+        If the key type of an LRU cache can't to be strongly tided to a specific
+        data type; e.g. FloatRect -> Path, we need to be able to pass the policy
+        type to the TinyLRUCache template instead of just specializing it. This
+        will make the code more readable and will allow different caches for the
+        same key type.
+
+        * wtf/TinyLRUCache.h:
+        (WebCore::TinyLRUCache::get):
+
 2016-02-01  Alex Christensen  <[email protected]>
 
         [Win] WTFHeaderDetection.h no longer needed

Modified: trunk/Source/WTF/wtf/TinyLRUCache.h (195969 => 195970)


--- trunk/Source/WTF/wtf/TinyLRUCache.h	2016-02-01 19:42:14 UTC (rev 195969)
+++ trunk/Source/WTF/wtf/TinyLRUCache.h	2016-02-01 19:45:18 UTC (rev 195970)
@@ -38,13 +38,13 @@
     static ValueType createValueForKey(const KeyType&) { return { }; }
 };
 
-template<typename KeyType, typename ValueType, size_t capacity = 4>
+template<typename KeyType, typename ValueType, size_t capacity = 4, typename Policy = TinyLRUCachePolicy<KeyType, ValueType>>
 class TinyLRUCache {
 public:
     const ValueType& get(const KeyType& key)
     {
-        if (TinyLRUCachePolicy<KeyType, ValueType>::isKeyNull(key)) {
-            static NeverDestroyed<ValueType> valueForNull = TinyLRUCachePolicy<KeyType, ValueType>::createValueForNullKey();
+        if (Policy::isKeyNull(key)) {
+            static NeverDestroyed<ValueType> valueForNull = Policy::createValueForNullKey();
             return valueForNull;
         }
 
@@ -66,7 +66,7 @@
         if (m_cache.size() == capacity)
             m_cache.remove(0);
 
-        m_cache.append(std::make_pair(key, TinyLRUCachePolicy<KeyType, ValueType>::createValueForKey(key)));
+        m_cache.append(std::make_pair(key, Policy::createValueForKey(key)));
         return m_cache.last().second;
     }

Modified: trunk/Source/WebCore/ChangeLog (195969 => 195970)


--- trunk/Source/WebCore/ChangeLog	2016-02-01 19:42:14 UTC (rev 195969)
+++ trunk/Source/WebCore/ChangeLog	2016-02-01 19:45:18 UTC (rev 195970)
@@ -1,3 +1,66 @@
+2016-02-01  Said Abou-Hallawa  <[email protected]>
+
+        Cache the Path instead of creating it every time it is required
+        https://bugs.webkit.org/show_bug.cgi?id=152939
+
+        Reviewed by Darin Adler.
+
+        Instead of creating the Path object every time it is required, we should 
+        cache it in an LRU cache. TinyLRUCache returns a reference to the cached
+        entry so we do not have to pay the cost of copying it either.
+
+        * platform/graphics/FloatRoundedRect.h:
+        (WebCore::operator!=):
+        Implement the inequality operator for FloatRoundedRect since it is
+        called by TinyLRUCache.
+        
+        * rendering/ClipPathOperation.h:
+        Return a reference to the path in the cache since instead of creating a
+        new copy.
+        
+        * rendering/style/BasicShapes.cpp:
+        (WebCore::SVGPathTranslatedByteStream::SVGPathTranslatedByteStream):
+        (WebCore::SVGPathTranslatedByteStream::operator==):
+        (WebCore::SVGPathTranslatedByteStream::operator!=):
+        (WebCore::SVGPathTranslatedByteStream::isEmpty):
+        (WebCore::SVGPathTranslatedByteStream::path):
+        This struct holds an offset and an SVGPathByteStream. It is the key of 
+        the LRU cache for the the translated SVGPathByteStream.
+        
+        (WebCore::EllipsePathPolicy::isKeyNull):
+        (WebCore::EllipsePathPolicy::createValueForKey):
+        (WebCore::RoundedRectPathPolicy::isKeyNull):
+        (WebCore::RoundedRectPathPolicy::createValueForKey):
+        (WebCore::PolygonPathPolicy::isKeyNull):
+        (WebCore::PolygonPathPolicy::createValueForKey):
+        (WebCore::TranslatedByteStreamPathPolicy::isKeyNull):
+        (WebCore::TranslatedByteStreamPathPolicy::createValueForKey):
+        Inherit from the LRU cache policy template, so have a specific name for
+        the desired path contents and pass this class explicitly to the LRU cache
+        template.
+        
+        (WebCore::cachedEllipsePath):
+        (WebCore::cachedRoundedRectPath):
+        (WebCore::cachedPolygonPath):
+        (WebCore::cachedTranslatedByteStreamPath):
+        Return a cached path object for specific path contents.
+        
+        (WebCore::BasicShapeCircle::path):
+        (WebCore::BasicShapeEllipse::path):
+        (WebCore::BasicShapePolygon::path):
+        (WebCore::BasicShapePath::path):
+        (WebCore::BasicShapeInset::path):
+        Get the Path object from the cache; create a new one if it does not exist.
+        
+        * rendering/style/BasicShapes.h:
+        Change the prototype of the path() function to return a reference to the
+        path in the cache instead of a having to copying it.
+        
+        * svg/SVGPathByteStream.h:
+        (WebCore::SVGPathByteStream::operator!=):
+        Implement the inequality operator for SVGPathByteStream because it is
+        called by TinyLRUCache.
+
 2016-02-01  Chris Dumez  <[email protected]>
 
         Move properties that use custom bindings to the prototype

Modified: trunk/Source/WebCore/platform/graphics/FloatRoundedRect.h (195969 => 195970)


--- trunk/Source/WebCore/platform/graphics/FloatRoundedRect.h	2016-02-01 19:42:14 UTC (rev 195969)
+++ trunk/Source/WebCore/platform/graphics/FloatRoundedRect.h	2016-02-01 19:45:18 UTC (rev 195970)
@@ -141,11 +141,21 @@
     return a.topLeft() == b.topLeft() && a.topRight() == b.topRight() && a.bottomLeft() == b.bottomLeft() && a.bottomRight() == b.bottomRight();
 }
 
+inline bool operator!=(const FloatRoundedRect::Radii& a, const FloatRoundedRect::Radii& b)
+{
+    return !(a == b);
+}
+
 inline bool operator==(const FloatRoundedRect& a, const FloatRoundedRect& b)
 {
     return a.rect() == b.rect() && a.radii() == b.radii();
 }
 
+inline bool operator!=(const FloatRoundedRect& a, const FloatRoundedRect& b)
+{
+    return !(a == b);
+}
+
 inline float calcBorderRadiiConstraintScaleFor(const FloatRect& rect, const FloatRoundedRect::Radii& radii)
 {
     // Constrain corner radii using CSS3 rules:

Modified: trunk/Source/WebCore/rendering/ClipPathOperation.h (195969 => 195970)


--- trunk/Source/WebCore/rendering/ClipPathOperation.h	2016-02-01 19:42:14 UTC (rev 195969)
+++ trunk/Source/WebCore/rendering/ClipPathOperation.h	2016-02-01 19:45:18 UTC (rev 195970)
@@ -103,12 +103,7 @@
 
     const BasicShape& basicShape() const { return m_shape; }
     WindRule windRule() const { return m_shape.get().windRule(); }
-    const Path pathForReferenceRect(const FloatRect& boundingRect)
-    {
-        Path path;
-        m_shape.get().path(path, boundingRect);
-        return path;
-    }
+    const Path& pathForReferenceRect(const FloatRect& boundingRect) { return m_shape.get().path(boundingRect); }
 
     void setReferenceBox(CSSBoxType referenceBox) { m_referenceBox = referenceBox; }
     CSSBoxType referenceBox() const { return m_referenceBox; }

Modified: trunk/Source/WebCore/rendering/style/BasicShapes.cpp (195969 => 195970)


--- trunk/Source/WebCore/rendering/style/BasicShapes.cpp	2016-02-01 19:42:14 UTC (rev 195969)
+++ trunk/Source/WebCore/rendering/style/BasicShapes.cpp	2016-02-01 19:45:18 UTC (rev 195970)
@@ -41,6 +41,9 @@
 #include "SVGPathByteStream.h"
 #include "SVGPathUtilities.h"
 
+#include <wtf/NeverDestroyed.h>
+#include <wtf/TinyLRUCache.h>
+
 namespace WebCore {
 
 void BasicShapeCenterCoordinate::updateComputedLength()
@@ -60,6 +63,90 @@
     m_computedLength = Length(CalculationValue::create(WTFMove(op), CalculationRangeAll));
 }
 
+struct SVGPathTranslatedByteStream {
+    SVGPathTranslatedByteStream(const FloatPoint& offset, const SVGPathByteStream& rawStream)
+        : m_offset(offset)
+        , m_rawStream(rawStream)
+    { }
+
+    bool operator==(const SVGPathTranslatedByteStream& other) const { return other.m_offset == m_offset && other.m_rawStream == m_rawStream; }
+    bool operator!=(const SVGPathTranslatedByteStream& other) const { return !(*this == other); }
+    bool isEmpty() const { return m_rawStream.isEmpty(); }
+
+    Path path() const
+    {
+        Path path;
+        buildPathFromByteStream(m_rawStream, path);
+        path.translate(toFloatSize(m_offset));
+        return path;
+    }
+    
+    FloatPoint m_offset;
+    SVGPathByteStream m_rawStream;
+};
+
+struct EllipsePathPolicy : public TinyLRUCachePolicy<FloatRect, Path> {
+public:
+    static bool isKeyNull(const FloatRect& rect) { return rect.isEmpty(); }
+
+    static Path createValueForKey(const FloatRect& rect)
+    {
+        Path path;
+        path.addEllipse(rect);
+        return path;
+    }
+};
+
+struct RoundedRectPathPolicy : public TinyLRUCachePolicy<FloatRoundedRect, Path> {
+public:
+    static bool isKeyNull(const FloatRoundedRect& rect) { return rect.isEmpty(); }
+
+    static Path createValueForKey(const FloatRoundedRect& rect)
+    {
+        Path path;
+        path.addRoundedRect(rect);
+        return path;
+    }
+};
+
+struct PolygonPathPolicy : public TinyLRUCachePolicy<Vector<FloatPoint>, Path> {
+public:
+    static bool isKeyNull(const Vector<FloatPoint>& points) { return !points.size(); }
+
+    static Path createValueForKey(const Vector<FloatPoint>& points) { return Path::polygonPathFromPoints(points); }
+};
+
+struct TranslatedByteStreamPathPolicy : public TinyLRUCachePolicy<SVGPathTranslatedByteStream, Path> {
+public:
+    static bool isKeyNull(const SVGPathTranslatedByteStream& stream) { return stream.isEmpty(); }
+
+    static Path createValueForKey(const SVGPathTranslatedByteStream& stream) { return stream.path(); }
+};
+
+static const Path& cachedEllipsePath(const FloatRect& rect)
+{
+    static NeverDestroyed<TinyLRUCache<FloatRect, Path, 4, EllipsePathPolicy>> cache;
+    return cache.get().get(rect);
+}
+
+static const Path& cachedRoundedRectPath(const FloatRoundedRect& rect)
+{
+    static NeverDestroyed<TinyLRUCache<FloatRoundedRect, Path, 4, RoundedRectPathPolicy>> cache;
+    return cache.get().get(rect);
+}
+
+static const Path& cachedPolygonPath(const Vector<FloatPoint>& points)
+{
+    static NeverDestroyed<TinyLRUCache<Vector<FloatPoint>, Path, 4, PolygonPathPolicy>> cache;
+    return cache.get().get(points);
+}
+
+static const Path& cachedTranslatedByteStreamPath(const SVGPathByteStream& stream, const FloatPoint& offset)
+{
+    static NeverDestroyed<TinyLRUCache<SVGPathTranslatedByteStream, Path, 4, TranslatedByteStreamPathPolicy>> cache;
+    return cache.get().get(SVGPathTranslatedByteStream(offset, stream));
+}
+
 bool BasicShapeCircle::operator==(const BasicShape& other) const
 {
     if (type() != other.type())
@@ -88,19 +175,13 @@
     return std::max(std::max(std::abs(centerX), widthDelta), std::max(std::abs(centerY), heightDelta));
 }
 
-void BasicShapeCircle::path(Path& path, const FloatRect& boundingBox)
+const Path& BasicShapeCircle::path(const FloatRect& boundingBox)
 {
-    ASSERT(path.isEmpty());
-
     float centerX = floatValueForCenterCoordinate(m_centerX, boundingBox.width());
     float centerY = floatValueForCenterCoordinate(m_centerY, boundingBox.height());
     float radius = floatValueForRadiusInBox(boundingBox.width(), boundingBox.height());
-    path.addEllipse(FloatRect(
-        centerX - radius + boundingBox.x(),
-        centerY - radius + boundingBox.y(),
-        radius * 2,
-        radius * 2
-    ));
+
+    return cachedEllipsePath(FloatRect(centerX - radius + boundingBox.x(), centerY - radius + boundingBox.y(), radius * 2, radius * 2));
 }
 
 bool BasicShapeCircle::canBlend(const BasicShape& other) const
@@ -148,19 +229,14 @@
     return std::max(std::abs(center), widthOrHeightDelta);
 }
 
-void BasicShapeEllipse::path(Path& path, const FloatRect& boundingBox)
+const Path& BasicShapeEllipse::path(const FloatRect& boundingBox)
 {
-    ASSERT(path.isEmpty());
-
     float centerX = floatValueForCenterCoordinate(m_centerX, boundingBox.width());
     float centerY = floatValueForCenterCoordinate(m_centerY, boundingBox.height());
     float radiusX = floatValueForRadiusInBox(m_radiusX, centerX, boundingBox.width());
     float radiusY = floatValueForRadiusInBox(m_radiusY, centerY, boundingBox.height());
-    path.addEllipse(FloatRect(
-        centerX - radiusX + boundingBox.x(),
-        centerY - radiusY + boundingBox.y(),
-        radiusX * 2,
-        radiusY * 2));
+
+    return cachedEllipsePath(FloatRect(centerX - radiusX + boundingBox.x(), centerY - radiusY + boundingBox.y(), radiusX * 2, radiusY * 2));
 }
 
 bool BasicShapeEllipse::canBlend(const BasicShape& other) const
@@ -204,22 +280,18 @@
         && m_values == otherPolygon.m_values;
 }
 
-void BasicShapePolygon::path(Path& path, const FloatRect& boundingBox)
+const Path& BasicShapePolygon::path(const FloatRect& boundingBox)
 {
-    ASSERT(path.isEmpty());
     ASSERT(!(m_values.size() % 2));
     size_t length = m_values.size();
-    
-    if (!length)
-        return;
 
-    path.moveTo(FloatPoint(floatValueForLength(m_values.at(0), boundingBox.width()) + boundingBox.x(),
-        floatValueForLength(m_values.at(1), boundingBox.height()) + boundingBox.y()));
-    for (size_t i = 2; i < length; i = i + 2) {
-        path.addLineTo(FloatPoint(floatValueForLength(m_values.at(i), boundingBox.width()) + boundingBox.x(),
-            floatValueForLength(m_values.at(i + 1), boundingBox.height()) + boundingBox.y()));
+    Vector<FloatPoint> points(length / 2);
+    for (size_t i = 0; i < points.size(); ++i) {
+        points[i].setX(floatValueForLength(m_values.at(i * 2), boundingBox.width()) + boundingBox.x());
+        points[i].setY(floatValueForLength(m_values.at(i * 2 + 1), boundingBox.height()) + boundingBox.y());
     }
-    path.closeSubpath();
+
+    return cachedPolygonPath(points);
 }
 
 bool BasicShapePolygon::canBlend(const BasicShape& other) const
@@ -259,11 +331,9 @@
 {
 }
 
-void BasicShapePath::path(Path& path, const FloatRect& boundingBox)
+const Path& BasicShapePath::path(const FloatRect& boundingBox)
 {
-    ASSERT(path.isEmpty());
-    buildPathFromByteStream(*m_byteStream, path);
-    path.translate(toFloatSize(boundingBox.location()));
+    return cachedTranslatedByteStreamPath(*m_byteStream, boundingBox.location());
 }
 
 bool BasicShapePath::operator==(const BasicShape& other) const
@@ -320,9 +390,8 @@
         floatValueForLength(lengthSize.height(), boundingBox.height()));
 }
 
-void BasicShapeInset::path(Path& path, const FloatRect& boundingBox)
+const Path& BasicShapeInset::path(const FloatRect& boundingBox)
 {
-    ASSERT(path.isEmpty());
     float left = floatValueForLength(m_left, boundingBox.width());
     float top = floatValueForLength(m_top, boundingBox.height());
     auto rect = FloatRect(left + boundingBox.x(), top + boundingBox.y(),
@@ -333,7 +402,8 @@
         floatSizeForLengthSize(m_bottomLeftRadius, boundingBox),
         floatSizeForLengthSize(m_bottomRightRadius, boundingBox));
     radii.scale(calcBorderRadiiConstraintScaleFor(rect, radii));
-    path.addRoundedRect(FloatRoundedRect(rect, radii));
+
+    return cachedRoundedRectPath(FloatRoundedRect(rect, radii));
 }
 
 bool BasicShapeInset::canBlend(const BasicShape& other) const

Modified: trunk/Source/WebCore/rendering/style/BasicShapes.h (195969 => 195970)


--- trunk/Source/WebCore/rendering/style/BasicShapes.h	2016-02-01 19:42:14 UTC (rev 195969)
+++ trunk/Source/WebCore/rendering/style/BasicShapes.h	2016-02-01 19:45:18 UTC (rev 195970)
@@ -60,7 +60,7 @@
 
     virtual Type type() const = 0;
 
-    virtual void path(Path&, const FloatRect&) = 0;
+    virtual const Path& path(const FloatRect&) = 0;
     virtual WindRule windRule() const { return RULE_NONZERO; }
 
     virtual bool canBlend(const BasicShape&) const = 0;
@@ -192,7 +192,7 @@
 
     virtual Type type() const override { return BasicShapeCircleType; }
 
-    virtual void path(Path&, const FloatRect&) override;
+    virtual const Path& path(const FloatRect&) override;
 
     virtual bool canBlend(const BasicShape&) const override;
     virtual Ref<BasicShape> blend(const BasicShape&, double) const override;
@@ -224,7 +224,7 @@
 
     virtual Type type() const override { return BasicShapeEllipseType; }
 
-    virtual void path(Path&, const FloatRect&) override;
+    virtual const Path& path(const FloatRect&) override;
 
     virtual bool canBlend(const BasicShape&) const override;
     virtual Ref<BasicShape> blend(const BasicShape&, double) const override;
@@ -255,7 +255,7 @@
 
     virtual Type type() const override { return BasicShapePolygonType; }
 
-    virtual void path(Path&, const FloatRect&) override;
+    virtual const Path& path(const FloatRect&) override;
 
     virtual bool canBlend(const BasicShape&) const override;
     virtual Ref<BasicShape> blend(const BasicShape&, double) const override;
@@ -283,7 +283,7 @@
 
     virtual Type type() const override { return BasicShapePathType; }
 
-    virtual void path(Path&, const FloatRect&) override;
+    virtual const Path& path(const FloatRect&) override;
 
     virtual bool canBlend(const BasicShape&) const override;
     virtual Ref<BasicShape> blend(const BasicShape&, double) const override;
@@ -323,7 +323,7 @@
 
     virtual Type type() const override { return BasicShapeInsetType; }
 
-    virtual void path(Path&, const FloatRect&) override;
+    virtual const Path& path(const FloatRect&) override;
 
     virtual bool canBlend(const BasicShape&) const override;
     virtual Ref<BasicShape> blend(const BasicShape&, double) const override;

Modified: trunk/Source/WebCore/svg/SVGPathByteStream.h (195969 => 195970)


--- trunk/Source/WebCore/svg/SVGPathByteStream.h	2016-02-01 19:42:14 UTC (rev 195969)
+++ trunk/Source/WebCore/svg/SVGPathByteStream.h	2016-02-01 19:45:18 UTC (rev 195970)
@@ -55,6 +55,11 @@
         return m_data == other.m_data;
     }
 
+    bool operator!=(const SVGPathByteStream& other) const
+    {
+        return !(*this == other);
+    }
+
     std::unique_ptr<SVGPathByteStream> copy() const
     {
         return std::make_unique<SVGPathByteStream>(m_data);

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to