REGRESSION (r215809): 50% regression 14E305 -> 15A293a in MotionMark Suits test
authordino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Jun 2017 21:06:29 +0000 (21:06 +0000)
committerdino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Jun 2017 21:06:29 +0000 (21:06 +0000)
https://bugs.webkit.org/show_bug.cgi?id=173728
<rdar://problem/32526744>

Reviewed by Tim Horton.

It turns out that CGGradientCreateWithColors is much slower than
CGGradientCreateWithColorComponents, even without colorspace variations.
Update the gradient creation code to only use this slower path
when it has extended colors.

* platform/graphics/Color.h: Add a FIXME about renaming some methods.
* platform/graphics/cg/GradientCG.cpp: Use CGGradientCreateWithColorComponents
if we have stops that are not extended colors.
(WebCore::Gradient::platformGradient):

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

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/Color.h
Source/WebCore/platform/graphics/cg/GradientCG.cpp

index f738ff0..0370b13 100644 (file)
@@ -1,3 +1,21 @@
+2017-06-22  Dean Jackson  <dino@apple.com>
+
+        REGRESSION (r215809): 50% regression 14E305 -> 15A293a in MotionMark Suits test
+        https://bugs.webkit.org/show_bug.cgi?id=173728
+        <rdar://problem/32526744>
+
+        Reviewed by Tim Horton.
+
+        It turns out that CGGradientCreateWithColors is much slower than
+        CGGradientCreateWithColorComponents, even without colorspace variations.
+        Update the gradient creation code to only use this slower path
+        when it has extended colors.
+
+        * platform/graphics/Color.h: Add a FIXME about renaming some methods.
+        * platform/graphics/cg/GradientCG.cpp: Use CGGradientCreateWithColorComponents
+        if we have stops that are not extended colors.
+        (WebCore::Gradient::platformGradient):
+
 2017-06-22  Youenn Fablet  <youenn@apple.com>
 
         Fix memory leak in LibWebRTCMediaEndpoint
index 83b2a39..8914051 100644 (file)
@@ -213,6 +213,8 @@ public:
     // should be identical, since the respective pointer will be different.
     unsigned hash() const { return WTF::intHash(m_colorData.rgbaAndFlags); }
 
+    // FIXME: ExtendedColor - these should be renamed (to be clear about their parameter types, or
+    // replaced with alternative accessors.
     WEBCORE_EXPORT void getRGBA(float& r, float& g, float& b, float& a) const;
     WEBCORE_EXPORT void getRGBA(double& r, double& g, double& b, double& a) const;
     WEBCORE_EXPORT void getHSL(double& h, double& s, double& l) const;
index 675f045..1f214c1 100644 (file)
@@ -49,17 +49,45 @@ CGGradientRef Gradient::platformGradient()
     sortStopsIfNecessary();
 
     auto colorsArray = adoptCF(CFArrayCreateMutable(0, m_stops.size(), &kCFTypeArrayCallBacks));
+    unsigned numStops = m_stops.size();
 
     const int reservedStops = 3;
     Vector<CGFloat, reservedStops> locations;
-    locations.reserveInitialCapacity(m_stops.size());
+    locations.reserveInitialCapacity(numStops);
 
+    Vector<CGFloat, 4 * reservedStops> colorComponents;
+    colorComponents.reserveInitialCapacity(numStops * 4);
+
+    bool hasExtendedColors = false;
     for (const auto& stop : m_stops) {
+
+        // If all the stops are sRGB, it is faster to create a gradient using
+        // components than CGColors.
+        // FIXME: Rather than just check for extended colors, we should check the actual
+        // color space, and whether or not the components are outside [0-1].
+        // <rdar://problem/32926606>
+
+        if (stop.color.isExtended())
+            hasExtendedColors = true;
+
+        float r;
+        float g;
+        float b;
+        float a;
+        stop.color.getRGBA(r, g, b, a);
+        colorComponents.uncheckedAppend(r);
+        colorComponents.uncheckedAppend(g);
+        colorComponents.uncheckedAppend(b);
+        colorComponents.uncheckedAppend(a);
+
         CFArrayAppendValue(colorsArray.get(), cachedCGColor(stop.color));
         locations.uncheckedAppend(stop.offset);
     }
 
-    m_gradient = CGGradientCreateWithColors(extendedSRGBColorSpaceRef(), colorsArray.get(), locations.data());
+    if (hasExtendedColors)
+        m_gradient = CGGradientCreateWithColors(extendedSRGBColorSpaceRef(), colorsArray.get(), locations.data());
+    else
+        m_gradient = CGGradientCreateWithColorComponents(sRGBColorSpaceRef(), colorComponents.data(), locations.data(), numStops);
 
     return m_gradient;
 }