Allow more buffer formats in the IOSurface pool

https://bugs.webkit.org/show_bug.cgi?id=151516

Reviewed by Tim Horton.
Source/WebCore:

Previously IOSurface::create was only looking in the pool for RGBA-format surfaces. Change that to
always look in the pool, and to cache all format types. We include format in the criteria used
to pick a surface from the pool.

* platform/graphics/cg/IOSurfacePool.cpp:
(WebCore::surfaceMatchesParameters):
(WebCore::IOSurfacePool::takeSurface):
(WebCore::IOSurfacePool::shouldCacheFormat):
(WebCore::IOSurfacePool::shouldCacheSurface):
(WebCore::IOSurfacePool::addSurface):
* platform/graphics/cg/IOSurfacePool.h:
* platform/graphics/cocoa/IOSurface.h:
* platform/graphics/cocoa/IOSurface.mm:
(IOSurface::surfaceFromPool):
(IOSurface::create):
(IOSurface::IOSurface):

Source/WebKit2:

Have RemoteLayerBackingStore go through a static function on IOSurface to return
a surface to the pool, rather than knowing about IOSurfacePool directly.

* Shared/mac/RemoteLayerBackingStore.mm:
(WebKit::RemoteLayerBackingStore::Buffer::discard):

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

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index e152c4f72b4ce338ee78c2829954bf6cf658e830..aea5d8ea79385d7c810341de3b8d14666f332061 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,27 @@
+2015-11-20  Simon Fraser  <simon.fraser@apple.com>
+
+        Allow more buffer formats in the IOSurface pool
+        https://bugs.webkit.org/show_bug.cgi?id=151516
+
+        Reviewed by Tim Horton.
+
+        Previously IOSurface::create was only looking in the pool for RGBA-format surfaces. Change that to
+        always look in the pool, and to cache all format types. We include format in the criteria used
+        to pick a surface from the pool.
+        
+        * platform/graphics/cg/IOSurfacePool.cpp:
+        (WebCore::surfaceMatchesParameters):
+        (WebCore::IOSurfacePool::takeSurface):
+        (WebCore::IOSurfacePool::shouldCacheFormat):
+        (WebCore::IOSurfacePool::shouldCacheSurface):
+        (WebCore::IOSurfacePool::addSurface):
+        * platform/graphics/cg/IOSurfacePool.h:
+        * platform/graphics/cocoa/IOSurface.h:
+        * platform/graphics/cocoa/IOSurface.mm:
+        (IOSurface::surfaceFromPool):
+        (IOSurface::create):
+        (IOSurface::IOSurface):
+
 2015-11-20  Brent Fulgham  <bfulgham@apple.com>
 
         [Win] Support High DPI drawing with CACFLayers

diff --git a/Source/WebCore/platform/graphics/cg/IOSurfacePool.cpp b/Source/WebCore/platform/graphics/cg/IOSurfacePool.cpp
index ff20ed30baed051162f12eaf9eaa62897fdeff25..3aed713d5eda70eea41bec9408346e94df6ce70e 100644 (file)
--- a/Source/WebCore/platform/graphics/cg/IOSurfacePool.cpp
+++ b/Source/WebCore/platform/graphics/cg/IOSurfacePool.cpp
@@ -29,7 +29,6 @@
 #if USE(IOSURFACE)
 
 #include "GraphicsContextCG.h"
-#include "IOSurface.h"
 #include <CoreGraphics/CoreGraphics.h>
 #include <chrono>
 #include <wtf/NeverDestroyed.h>
@@ -65,12 +64,13 @@ IOSurfacePool& IOSurfacePool::sharedPool()
     return pool;
 }
 
-static bool surfaceMatchesParameters(IOSurface& surface, const IntSize& requestedSize, ColorSpace colorSpace)
+static bool surfaceMatchesParameters(IOSurface& surface, IntSize requestedSize, ColorSpace colorSpace, IOSurface::Format format)
 {
-    IntSize surfaceSize = surface.size();
+    if (format != surface.format())
+        return false;
     if (colorSpace != surface.colorSpace())
         return false;
-    if (surfaceSize != requestedSize)
+    if (requestedSize != surface.size())
         return false;
     return true;
 }
@@ -107,7 +107,7 @@ void IOSurfacePool::didUseSurfaceOfSize(IntSize size)
     m_sizesInPruneOrder.append(size);
 }
 
-std::unique_ptr<IOSurface> IOSurfacePool::takeSurface(IntSize size, ColorSpace colorSpace)
+std::unique_ptr<IOSurface> IOSurfacePool::takeSurface(IntSize size, ColorSpace colorSpace, IOSurface::Format format)
 {
     CachedSurfaceMap::iterator mapIter = m_cachedSurfaces.find(size);
 
@@ -117,7 +117,7 @@ std::unique_ptr<IOSurface> IOSurfacePool::takeSurface(IntSize size, ColorSpace c
     }
 
     for (auto surfaceIter = mapIter->value.begin(); surfaceIter != mapIter->value.end(); ++surfaceIter) {
-        if (!surfaceMatchesParameters(*surfaceIter->get(), size, colorSpace))
+        if (!surfaceMatchesParameters(*surfaceIter->get(), size, colorSpace, format))
             continue;
 
         auto surface = WTF::move(*surfaceIter);
@@ -140,7 +140,7 @@ std::unique_ptr<IOSurface> IOSurfacePool::takeSurface(IntSize size, ColorSpace c
 
     // Some of the in-use surfaces may no longer actually be in-use, but we haven't moved them over yet.
     for (auto surfaceIter = m_inUseSurfaces.begin(); surfaceIter != m_inUseSurfaces.end(); ++surfaceIter) {
-        if (!surfaceMatchesParameters(*surfaceIter->get(), size, colorSpace))
+        if (!surfaceMatchesParameters(*surfaceIter->get(), size, colorSpace, format))
             continue;
         if (surfaceIter->get()->isInUse())
             continue;
@@ -159,14 +159,22 @@ std::unique_ptr<IOSurface> IOSurfacePool::takeSurface(IntSize size, ColorSpace c
     return nullptr;
 }
 
-void IOSurfacePool::addSurface(std::unique_ptr<IOSurface> surface)
+bool IOSurfacePool::shouldCacheSurface(const IOSurface& surface) const
 {
-    if (surface->totalBytes() > m_maximumBytesCached)
-        return;
+    if (surface.totalBytes() > m_maximumBytesCached)
+        return false;
 
     // There's no reason to pool empty surfaces; we should never allocate them in the first place.
     // This also covers isZero(), which would cause trouble when used as the key in m_cachedSurfaces.
-    if (surface->size().isEmpty())
+    if (surface.size().isEmpty())
+        return false;
+
+    return true;
+}
+
+void IOSurfacePool::addSurface(std::unique_ptr<IOSurface> surface)
+{
+    if (!shouldCacheSurface(*surface))
         return;
 
     bool surfaceIsInUse = surface->isInUse();

diff --git a/Source/WebCore/platform/graphics/cg/IOSurfacePool.h b/Source/WebCore/platform/graphics/cg/IOSurfacePool.h
index cf83b821f345b191410cfa871094d351b54689c5..1e172b4799478a07a351863f8d9c1f599e2826a0 100644 (file)
--- a/Source/WebCore/platform/graphics/cg/IOSurfacePool.h
+++ b/Source/WebCore/platform/graphics/cg/IOSurfacePool.h
@@ -27,6 +27,7 @@
 #define IOSurfacePool_h
 
 #include "ColorSpace.h"
+#include "IOSurface.h"
 #include "IntSize.h"
 #include "IntSizeHash.h"
 #include "Timer.h"
@@ -39,8 +40,6 @@
 
 namespace WebCore {
 
-class IOSurface;
-
 class IOSurfacePool {
     WTF_MAKE_NONCOPYABLE(IOSurfacePool);
     WTF_MAKE_FAST_ALLOCATED;
@@ -49,7 +48,7 @@ class IOSurfacePool {
 public:
     WEBCORE_EXPORT static IOSurfacePool& sharedPool();
 
-    std::unique_ptr<IOSurface> takeSurface(IntSize, ColorSpace);
+    std::unique_ptr<IOSurface> takeSurface(IntSize, ColorSpace, IOSurface::Format);
     WEBCORE_EXPORT void addSurface(std::unique_ptr<IOSurface>);
 
     void discardAllSurfaces();
@@ -75,6 +74,8 @@ private:
     typedef Deque<std::unique_ptr<IOSurface>> CachedSurfaceQueue;
     typedef HashMap<IntSize, CachedSurfaceQueue> CachedSurfaceMap;
     typedef HashMap<IOSurface*, CachedSurfaceDetails> CachedSurfaceDetailsMap;
+    
+    bool shouldCacheSurface(const IOSurface&) const;
 
     void willAddSurface(IOSurface&, bool inUse);
     void didRemoveSurface(IOSurface&, bool inUse);

diff --git a/Source/WebCore/platform/graphics/cocoa/IOSurface.h b/Source/WebCore/platform/graphics/cocoa/IOSurface.h
index 8ad18312af14cdb211c6d1fe786976e756f126cd..cbcf56cde2d1e28810588bcecdb5b4da0b4d2b51 100644 (file)
--- a/Source/WebCore/platform/graphics/cocoa/IOSurface.h
+++ b/Source/WebCore/platform/graphics/cocoa/IOSurface.h
@@ -46,11 +46,13 @@ public:
     };
 
     WEBCORE_EXPORT static std::unique_ptr<IOSurface> create(IntSize, ColorSpace, Format = Format::RGBA);
-    WEBCORE_EXPORT static std::unique_ptr<IOSurface> create(IntSize, IntSize contextSize, ColorSpace);
+    WEBCORE_EXPORT static std::unique_ptr<IOSurface> create(IntSize, IntSize contextSize, ColorSpace, Format = Format::RGBA);
     WEBCORE_EXPORT static std::unique_ptr<IOSurface> createFromSendRight(const MachSendRight&, ColorSpace);
     static std::unique_ptr<IOSurface> createFromSurface(IOSurfaceRef, ColorSpace);
     WEBCORE_EXPORT static std::unique_ptr<IOSurface> createFromImage(CGImageRef);
 
+    WEBCORE_EXPORT static void moveToPool(std::unique_ptr<IOSurface>&&);
+
     static IntSize maximumSize();
 
     WEBCORE_EXPORT MachSendRight createSendRight() const;
@@ -95,10 +97,10 @@ public:
 
 private:
     IOSurface(IntSize, ColorSpace, Format);
-    IOSurface(IntSize, IntSize contextSize, ColorSpace);
+    IOSurface(IntSize, IntSize contextSize, ColorSpace, Format);
     IOSurface(IOSurfaceRef, ColorSpace);
 
-    static std::unique_ptr<IOSurface> surfaceFromPool(IntSize, IntSize contextSize, ColorSpace);
+    static std::unique_ptr<IOSurface> surfaceFromPool(IntSize, IntSize contextSize, ColorSpace, Format);
     IntSize contextSize() const { return m_contextSize; }
     void setContextSize(IntSize);
 

diff --git a/Source/WebCore/platform/graphics/cocoa/IOSurface.mm b/Source/WebCore/platform/graphics/cocoa/IOSurface.mm
index 4189580f2a14c23205285d37ed02bdb237acf366..cd4412ef6ea6c2baeb8620ceb48265cad9e12cb3 100644 (file)
--- a/Source/WebCore/platform/graphics/cocoa/IOSurface.mm
+++ b/Source/WebCore/platform/graphics/cocoa/IOSurface.mm
@@ -41,9 +41,9 @@ CGImageRef CGIOSurfaceContextCreateImage(CGContextRef);
 
 using namespace WebCore;
 
-inline std::unique_ptr<IOSurface> IOSurface::surfaceFromPool(IntSize size, IntSize contextSize, ColorSpace colorSpace)
+inline std::unique_ptr<IOSurface> IOSurface::surfaceFromPool(IntSize size, IntSize contextSize, ColorSpace colorSpace, Format pixelFormat)
 {
-    auto cachedSurface = IOSurfacePool::sharedPool().takeSurface(size, colorSpace);
+    auto cachedSurface = IOSurfacePool::sharedPool().takeSurface(size, colorSpace, pixelFormat);
     if (!cachedSurface)
         return nullptr;
 
@@ -53,21 +53,17 @@ inline std::unique_ptr<IOSurface> IOSurface::surfaceFromPool(IntSize size, IntSi
 
 std::unique_ptr<IOSurface> IOSurface::create(IntSize size, ColorSpace colorSpace, Format pixelFormat)
 {
-    // YUV422 IOSurfaces do not go in the pool.
-    // FIXME: Want pooling of RGB10, RGB10A8.
-    if (pixelFormat == Format::RGBA) {
-        if (auto cachedSurface = surfaceFromPool(size, size, colorSpace))
-            return cachedSurface;
-    }
+    if (auto cachedSurface = surfaceFromPool(size, size, colorSpace, pixelFormat))
+        return cachedSurface;
 
     return std::unique_ptr<IOSurface>(new IOSurface(size, colorSpace, pixelFormat));
 }
 
-std::unique_ptr<IOSurface> IOSurface::create(IntSize size, IntSize contextSize, ColorSpace colorSpace)
+std::unique_ptr<IOSurface> IOSurface::create(IntSize size, IntSize contextSize, ColorSpace colorSpace, Format pixelFormat)
 {
-    if (auto cachedSurface = surfaceFromPool(size, contextSize, colorSpace))
+    if (auto cachedSurface = surfaceFromPool(size, contextSize, colorSpace, pixelFormat))
         return cachedSurface;
-    return std::unique_ptr<IOSurface>(new IOSurface(size, contextSize, colorSpace));
+    return std::unique_ptr<IOSurface>(new IOSurface(size, contextSize, colorSpace, pixelFormat));
 }
 
 std::unique_ptr<IOSurface> IOSurface::createFromSendRight(const MachSendRight& sendRight, ColorSpace colorSpace)
@@ -97,6 +93,11 @@ std::unique_ptr<IOSurface> IOSurface::createFromImage(CGImageRef image)
     return surface;
 }
 
+void IOSurface::moveToPool(std::unique_ptr<IOSurface>&& surface)
+{
+    IOSurfacePool::sharedPool().addSurface(WTF::move(surface));
+}
+
 IOSurface::IOSurface(IntSize size, ColorSpace colorSpace, Format format)
     : m_colorSpace(colorSpace)
     , m_size(size)
@@ -212,8 +213,8 @@ IOSurface::IOSurface(IntSize size, ColorSpace colorSpace, Format format)
         NSLog(@"Surface creation failed for options %@", options);
 }
 
-IOSurface::IOSurface(IntSize size, IntSize contextSize, ColorSpace colorSpace)
-    : IOSurface(size, colorSpace, Format::RGBA)
+IOSurface::IOSurface(IntSize size, IntSize contextSize, ColorSpace colorSpace, Format pixelFormat)
+    : IOSurface(size, colorSpace, pixelFormat)
 {
     ASSERT(contextSize.width() <= size.width());
     ASSERT(contextSize.height() <= size.height());

diff --git a/Source/WebKit2/ChangeLog b/Source/WebKit2/ChangeLog
index e9f7b171d066d75f73c782bb7495ffca3482a6e1..ac359c84d8f39c529a9353f5ae0bbfc841dfcc2f 100644 (file)
--- a/Source/WebKit2/ChangeLog
+++ b/Source/WebKit2/ChangeLog
@@ -1,3 +1,16 @@
+2015-11-20  Simon Fraser  <simon.fraser@apple.com>
+
+        Allow more buffer formats in the IOSurface pool
+        https://bugs.webkit.org/show_bug.cgi?id=151516
+
+        Reviewed by Tim Horton.
+        
+        Have RemoteLayerBackingStore go through a static function on IOSurface to return
+        a surface to the pool, rather than knowing about IOSurfacePool directly.
+
+        * Shared/mac/RemoteLayerBackingStore.mm:
+        (WebKit::RemoteLayerBackingStore::Buffer::discard):
+
 2015-11-20  Alex Christensen  <achristensen@webkit.org>
 
         Remove NETWORK_PROCESS compile flag

diff --git a/Source/WebKit2/Shared/mac/RemoteLayerBackingStore.mm b/Source/WebKit2/Shared/mac/RemoteLayerBackingStore.mm
index 8addac57fa1237dd62a5277316c3912fc195905d..d8a9ad52490ec1227b0733275cb5c3ca2ec4174d 100644 (file)
--- a/Source/WebKit2/Shared/mac/RemoteLayerBackingStore.mm
+++ b/Source/WebKit2/Shared/mac/RemoteLayerBackingStore.mm
@@ -36,7 +36,6 @@
 #import <QuartzCore/QuartzCore.h>
 #import <WebCore/GraphicsContextCG.h>
 #import <WebCore/IOSurface.h>
-#import <WebCore/IOSurfacePool.h>
 #import <WebCore/MachSendRight.h>
 #import <WebCore/QuartzCoreSPI.h>
 #import <WebCore/WebLayer.h>
@@ -469,7 +468,7 @@ void RemoteLayerBackingStore::Buffer::discard()
 {
 #if USE(IOSURFACE)
     if (surface)
-        IOSurfacePool::sharedPool().addSurface(WTF::move(surface));
+        IOSurface::moveToPool(WTF::move(surface));
     isVolatile = false;
 #endif
     bitmap = nullptr;