Back-buffer to front-buffer copy fails for some buffer formats
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 20 Nov 2015 20:05:05 +0000 (20:05 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 20 Nov 2015 20:05:05 +0000 (20:05 +0000)
https://bugs.webkit.org/show_bug.cgi?id=151475
rdar://problem/23617899

Reviewed by Tim Horton.

Source/WebCore:

Fix some fo the bitsPerComponent/bitsPerPixel options in IOSurface::ensurePlatformContext()
for RGB10 buffers. Fix IOSurface::format() to return the new formats.

Implement IOSurface::copyToSurface(), which does a synchronous copy between
surfaces.

* platform/graphics/cocoa/IOSurface.h:
* platform/graphics/cocoa/IOSurface.mm:
(IOSurface::create):
(IOSurface::ensurePlatformContext):
(IOSurface::format):
(IOSurface::copyToSurface):

Source/WebKit2:

When displaying RemoteLayerBackingStore, we copy the back buffer to the front
buffer before painting the updated regions. For buffers using Format::RGB10A8,
the CGImage-based copy fails, so in this case, use IOSurface::copyToSurface().

Reorganized RemoteLayerBackingStore::drawInContext() to make this a bit easier
to understand. First, we either copy the entire surface over, or paint the backImage.
Then we clip to the dirty rects, and clear them, then paint the layer contents.

* Shared/mac/RemoteLayerBackingStore.mm:
(WebKit::RemoteLayerBackingStore::display):
(WebKit::RemoteLayerBackingStore::drawInContext):

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

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/cocoa/IOSurface.h
Source/WebCore/platform/graphics/cocoa/IOSurface.mm
Source/WebKit2/ChangeLog
Source/WebKit2/Shared/mac/RemoteLayerBackingStore.mm

index 3ef586a..81cdbc3 100644 (file)
@@ -1,3 +1,24 @@
+2015-11-19  Simon Fraser  <simon.fraser@apple.com>
+
+        Back-buffer to front-buffer copy fails for some buffer formats
+        https://bugs.webkit.org/show_bug.cgi?id=151475
+        rdar://problem/23617899
+
+        Reviewed by Tim Horton.
+        
+        Fix some fo the bitsPerComponent/bitsPerPixel options in IOSurface::ensurePlatformContext()
+        for RGB10 buffers. Fix IOSurface::format() to return the new formats.
+        
+        Implement IOSurface::copyToSurface(), which does a synchronous copy between
+        surfaces.
+
+        * platform/graphics/cocoa/IOSurface.h:
+        * platform/graphics/cocoa/IOSurface.mm:
+        (IOSurface::create):
+        (IOSurface::ensurePlatformContext):
+        (IOSurface::format):
+        (IOSurface::copyToSurface):
+
 2015-11-20  Zalan Bujtas  <zalan@apple.com>
 
         Simple line layout: Add text-indent support.
index 6b8fbe0..8ad1831 100644 (file)
@@ -80,7 +80,7 @@ public:
     IntSize size() const { return m_size; }
     size_t totalBytes() const { return m_totalBytes; }
     ColorSpace colorSpace() const { return m_colorSpace; }
-    Format format() const;
+    WEBCORE_EXPORT Format format() const;
 
     WEBCORE_EXPORT bool isInUse() const;
 
@@ -89,6 +89,7 @@ public:
     WEBCORE_EXPORT void releaseGraphicsContext();
 
 #if PLATFORM(IOS)
+    WEBCORE_EXPORT void copyToSurface(IOSurface&);
     WEBCORE_EXPORT static void convertToFormat(std::unique_ptr<WebCore::IOSurface>&& inSurface, Format, std::function<void(std::unique_ptr<WebCore::IOSurface>)>);
 #endif
 
index 5c07653..4189580 100644 (file)
@@ -54,6 +54,7 @@ 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;
@@ -259,8 +260,27 @@ CGContextRef IOSurface::ensurePlatformContext()
         return m_cgContext.get();
 
     CGBitmapInfo bitmapInfo = kCGImageAlphaPremultipliedFirst | kCGBitmapByteOrder32Host;
+
     size_t bitsPerComponent = 8;
     size_t bitsPerPixel = 32;
+    
+    switch (format()) {
+    case Format::RGBA:
+        break;
+    case Format::RGB10:
+        bitsPerComponent = 10;
+        bitsPerPixel = 32;
+        break;
+    case Format::RGB10A8:
+        // FIXME: This doesn't take the two-plane format into account.
+        bitsPerComponent = 10;
+        bitsPerPixel = 32;
+        break;
+    case Format::YUV422:
+        ASSERT_NOT_REACHED();
+        break;
+    }
+    
     m_cgContext = adoptCF(CGIOSurfaceContextCreate(m_surface.get(), m_contextSize.width(), m_contextSize.height(), bitsPerComponent, bitsPerPixel, cachedCGColorSpace(m_colorSpace), bitmapInfo));
 
     return m_cgContext.get();
@@ -310,6 +330,13 @@ IOSurface::Format IOSurface::format() const
     unsigned pixelFormat = IOSurfaceGetPixelFormat(m_surface.get());
     if (pixelFormat == 'BGRA')
         return Format::RGBA;
+
+    if (pixelFormat == 'w30r')
+        return Format::RGB10;
+
+    if (pixelFormat == 'b3a8')
+        return Format::RGB10A8;
+
     if (pixelFormat == 'yuvf')
         return Format::YUV422;
 
@@ -329,6 +356,27 @@ void IOSurface::releaseGraphicsContext()
 }
 
 #if PLATFORM(IOS)
+WEBCORE_EXPORT void IOSurface::copyToSurface(IOSurface& destSurface)
+{
+    if (destSurface.format() != format()) {
+        WTFLogAlways("Trying to copy IOSurface to another surface with a different format");
+        return;
+    }
+
+    if (destSurface.size() != size()) {
+        WTFLogAlways("Trying to copy IOSurface to another surface with a different size");
+        return;
+    }
+
+    static IOSurfaceAcceleratorRef accelerator;
+    if (!accelerator)
+        IOSurfaceAcceleratorCreate(nullptr, nullptr, &accelerator);
+
+    IOReturn ret = IOSurfaceAcceleratorTransformSurface(accelerator, m_surface.get(), destSurface.surface(), nullptr, nullptr, nullptr, nullptr, nullptr);
+    if (ret)
+        WTFLogAlways("IOSurfaceAcceleratorTransformSurface %p to %p failed with error %d", m_surface.get(), destSurface.surface(), ret);
+}
+
 void IOSurface::convertToFormat(std::unique_ptr<WebCore::IOSurface>&& inSurface, Format format, std::function<void(std::unique_ptr<WebCore::IOSurface>)> callback)
 {
     static IOSurfaceAcceleratorRef accelerator;
index 499a995..de09705 100644 (file)
@@ -1,3 +1,23 @@
+2015-11-19  Simon Fraser  <simon.fraser@apple.com>
+
+        Back-buffer to front-buffer copy fails for some buffer formats
+        https://bugs.webkit.org/show_bug.cgi?id=151475
+        rdar://problem/23617899
+
+        Reviewed by Tim Horton.
+        
+        When displaying RemoteLayerBackingStore, we copy the back buffer to the front
+        buffer before painting the updated regions. For buffers using Format::RGB10A8,
+        the CGImage-based copy fails, so in this case, use IOSurface::copyToSurface().
+        
+        Reorganized RemoteLayerBackingStore::drawInContext() to make this a bit easier
+        to understand. First, we either copy the entire surface over, or paint the backImage.
+        Then we clip to the dirty rects, and clear them, then paint the layer contents.
+
+        * Shared/mac/RemoteLayerBackingStore.mm:
+        (WebKit::RemoteLayerBackingStore::display):
+        (WebKit::RemoteLayerBackingStore::drawInContext):
+
 2015-11-19  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r192667 and r192668.
index 431c046..8addac5 100644 (file)
@@ -252,8 +252,15 @@ bool RemoteLayerBackingStore::display()
 #if USE(IOSURFACE)
     if (m_acceleratesDrawing) {
         RetainPtr<CGImageRef> backImage;
-        if (m_backBuffer.surface && !willPaintEntireBackingStore)
-            backImage = m_backBuffer.surface->createImage();
+        if (m_backBuffer.surface && !willPaintEntireBackingStore) {
+#if PLATFORM(IOS)
+            if (m_backBuffer.surface->format() == WebCore::IOSurface::Format::RGB10A8) {
+                // FIXME: remove this when rdar://problem/23623670 is fixed.
+                m_backBuffer.surface->copyToSurface(*m_frontBuffer.surface);
+            } else
+#endif
+                backImage = m_backBuffer.surface->createImage();
+        }
 
         GraphicsContext& context = m_frontBuffer.surface->ensureGraphicsContext();
 
@@ -286,16 +293,6 @@ void RemoteLayerBackingStore::drawInContext(GraphicsContext& context, CGImageRef
     scaledSize.scale(m_scale);
     IntRect scaledLayerBounds(IntPoint(), roundedIntSize(scaledSize));
 
-    if (!m_isOpaque)
-        context.clearRect(scaledLayerBounds);
-
-#ifndef NDEBUG
-    if (m_isOpaque)
-        context.fillRect(scaledLayerBounds, Color(255, 0, 0));
-#endif
-
-    CGContextRef cgContext = context.platformContext();
-
     // If we have less than webLayerMaxRectsToPaint rects to paint and they cover less
     // than webLayerWastedSpaceThreshold of the total dirty area, we'll repaint each rect separately.
     // Otherwise, repaint the entire bounding box of the dirty region.
@@ -323,22 +320,26 @@ void RemoteLayerBackingStore::drawInContext(GraphicsContext& context, CGImageRef
         cgPaintingRects[i] = scaledPaintingRect;
     }
 
+    CGContextRef cgContext = context.platformContext();
+
     if (backImage) {
-        CGContextSaveGState(cgContext);
+        CGContextStateSaver stateSaver(cgContext);
         CGContextSetBlendMode(cgContext, kCGBlendModeCopy);
-
-        CGContextAddRect(cgContext, CGRectInfinite);
-        CGContextAddRects(cgContext, cgPaintingRects, m_paintingRects.size());
-        CGContextEOClip(cgContext);
-
         CGContextTranslateCTM(cgContext, 0, scaledLayerBounds.height());
         CGContextScaleCTM(cgContext, 1, -1);
         CGContextDrawImage(cgContext, scaledLayerBounds, backImage);
-        CGContextRestoreGState(cgContext);
     }
 
     CGContextClipToRects(cgContext, cgPaintingRects, m_paintingRects.size());
 
+    if (!m_isOpaque)
+        context.clearRect(scaledLayerBounds);
+
+#ifndef NDEBUG
+    if (m_isOpaque)
+        context.fillRect(scaledLayerBounds, Color(255, 0, 0));
+#endif
+
     context.scale(FloatSize(m_scale, m_scale));
 
     // FIXME: This should be moved to PlatformCALayerRemote for better layering.