Change GPU read/write pixels API to support color space conversion

GrContext still doesn't convert, but it has the source and destination
color spaces, and call sites are supplying appropriate values where it
makes sense.

BUG=skia:

Change-Id: Ia88733125b8090776cfc9b0dc8030cce365b0b8b
Reviewed-on: https://skia-review.googlesource.com/6400
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>

diff --git a/include/gpu/GrContext.h b/include/gpu/GrContext.h
index 8d3b4f5..6664db9 100644 (file)
--- a/include/gpu/GrContext.h
+++ b/include/gpu/GrContext.h
@@ -260,11 +260,13 @@ public:
     /**
      * Reads a rectangle of pixels from a surface.
      * @param surface       the surface to read from.
+     * @param srcColorSpace color space of the surface
      * @param left          left edge of the rectangle to read (inclusive)
      * @param top           top edge of the rectangle to read (inclusive)
      * @param width         width of rectangle to read in pixels.
      * @param height        height of rectangle to read in pixels.
      * @param config        the pixel config of the destination buffer
+     * @param dstColorSpace color space of the destination buffer
      * @param buffer        memory to read the rectangle into.
      * @param rowBytes      number of bytes bewtween consecutive rows. Zero means rows are tightly
      *                      packed.
@@ -273,20 +275,22 @@ public:
      * @return true if the read succeeded, false if not. The read can fail because of an unsupported
      *         pixel configs
      */
-    bool readSurfacePixels(GrSurface* surface,
+    bool readSurfacePixels(GrSurface* surface, SkColorSpace* srcColorSpace,
                            int left, int top, int width, int height,
-                           GrPixelConfig config, void* buffer,
+                           GrPixelConfig config, SkColorSpace* dstColorSpace, void* buffer,
                            size_t rowBytes = 0,
                            uint32_t pixelOpsFlags = 0);
 
     /**
      * Writes a rectangle of pixels to a surface.
      * @param surface       the surface to write to.
+     * @param dstColorSpace color space of the surface
      * @param left          left edge of the rectangle to write (inclusive)
      * @param top           top edge of the rectangle to write (inclusive)
      * @param width         width of rectangle to write in pixels.
      * @param height        height of rectangle to write in pixels.
      * @param config        the pixel config of the source buffer
+     * @param srcColorSpace color space of the source buffer
      * @param buffer        memory to read pixels from
      * @param rowBytes      number of bytes between consecutive rows. Zero
      *                      means rows are tightly packed.
@@ -294,9 +298,9 @@ public:
      * @return true if the write succeeded, false if not. The write can fail because of an
      *         unsupported combination of surface and src configs.
      */
-    bool writeSurfacePixels(GrSurface* surface,
+    bool writeSurfacePixels(GrSurface* surface, SkColorSpace* dstColorSpace,
                             int left, int top, int width, int height,
-                            GrPixelConfig config, const void* buffer,
+                            GrPixelConfig config, SkColorSpace* srcColorSpace, const void* buffer,
                             size_t rowBytes,
                             uint32_t pixelOpsFlags = 0);
 

diff --git a/include/gpu/GrSurface.h b/include/gpu/GrSurface.h
index 2205bd8..7bfed3c 100644 (file)
--- a/include/gpu/GrSurface.h
+++ b/include/gpu/GrSurface.h
@@ -67,12 +67,14 @@ public:
     virtual const GrRenderTarget* asRenderTarget() const { return NULL; }
 
     /**
-     * Reads a rectangle of pixels from the surface.
+     * Reads a rectangle of pixels from the surface, possibly performing color space conversion.
+     * @param srcColorSpace color space of the source data (this surface)
      * @param left          left edge of the rectangle to read (inclusive)
      * @param top           top edge of the rectangle to read (inclusive)
      * @param width         width of rectangle to read in pixels.
      * @param height        height of rectangle to read in pixels.
      * @param config        the pixel config of the destination buffer
+     * @param dstColorSpace color space of the destination buffer
      * @param buffer        memory to read the rectangle into.
      * @param rowBytes      number of bytes between consecutive rows. Zero means rows are tightly
      *                      packed.
@@ -81,20 +83,48 @@ public:
      * @return true if the read succeeded, false if not. The read can fail because of an unsupported
      *              pixel config.
      */
-    bool readPixels(int left, int top, int width, int height,
+    bool readPixels(SkColorSpace* srcColorSpace,
+                    int left, int top, int width, int height,
                     GrPixelConfig config,
+                    SkColorSpace* dstColorSpace,
                     void* buffer,
                     size_t rowBytes = 0,
                     uint32_t pixelOpsFlags = 0);
 
     /**
+     * Reads a rectangle of pixels from the surface. Does not perform any color space conversion.
+     * @param left          left edge of the rectangle to read (inclusive)
+     * @param top           top edge of the rectangle to read (inclusive)
+     * @param width         width of rectangle to read in pixels.
+     * @param height        height of rectangle to read in pixels.
+     * @param config        the pixel config of the destination buffer
+     * @param buffer        memory to read the rectangle into.
+     * @param rowBytes      number of bytes between consecutive rows. Zero means rows are tightly
+     *                      packed.
+     * @param pixelOpsFlags See the GrContext::PixelOpsFlags enum.
+     *
+     * @return true if the read succeeded, false if not. The read can fail because of an unsupported
+     *              pixel config.
+     */
+    bool readPixels(int left, int top, int width, int height,
+                    GrPixelConfig config,
+                    void* buffer,
+                    size_t rowBytes = 0,
+                    uint32_t pixelOpsFlags = 0) {
+        return this->readPixels(nullptr, left, top, width, height, config, nullptr, buffer,
+                                rowBytes, pixelOpsFlags);
+    }
+
+    /**
      * Copy the src pixels [buffer, rowbytes, pixelconfig] into the surface at the specified
-     * rectangle.
+     * rectangle, possibly performing color space conversion.
+     * @param dstColorSpace color space of the destination (this surface)
      * @param left          left edge of the rectangle to write (inclusive)
      * @param top           top edge of the rectangle to write (inclusive)
      * @param width         width of rectangle to write in pixels.
      * @param height        height of rectangle to write in pixels.
      * @param config        the pixel config of the source buffer
+     * @param srcColorSpace color space of the source buffer
      * @param buffer        memory to read the rectangle from.
      * @param rowBytes      number of bytes between consecutive rows. Zero means rows are tightly
      *                      packed.
@@ -103,13 +133,40 @@ public:
      * @return true if the write succeeded, false if not. The write can fail because of an
      *              unsupported pixel config.
      */
-    bool writePixels(int left, int top, int width, int height,
+    bool writePixels(SkColorSpace* dstColorSpace,
+                     int left, int top, int width, int height,
                      GrPixelConfig config,
+                     SkColorSpace* srcColorSpace,
                      const void* buffer,
                      size_t rowBytes = 0,
                      uint32_t pixelOpsFlags = 0);
 
     /**
+     * Copy the src pixels [buffer, rowbytes, pixelconfig] into the surface at the specified
+     * rectangle. Does not perform any color space conversion.
+     * @param left          left edge of the rectangle to write (inclusive)
+     * @param top           top edge of the rectangle to write (inclusive)
+     * @param width         width of rectangle to write in pixels.
+     * @param height        height of rectangle to write in pixels.
+     * @param config        the pixel config of the source buffer
+     * @param buffer        memory to read the rectangle from.
+     * @param rowBytes      number of bytes between consecutive rows. Zero means rows are tightly
+     *                      packed.
+     * @param pixelOpsFlags See the GrContext::PixelOpsFlags enum.
+     *
+     * @return true if the write succeeded, false if not. The write can fail because of an
+     *              unsupported pixel config.
+     */
+    bool writePixels(int left, int top, int width, int height,
+                     GrPixelConfig config,
+                     const void* buffer,
+                     size_t rowBytes = 0,
+                     uint32_t pixelOpsFlags = 0) {
+        return this->writePixels(nullptr, left, top, width, height, config, nullptr, buffer,
+                                 rowBytes, pixelOpsFlags);
+    }
+
+    /**
      * After this returns any pending writes to the surface will be issued to the backend 3D API.
      */
     void flushWrites();

diff --git a/src/core/SkImageCacherator.cpp b/src/core/SkImageCacherator.cpp
index 21b112f..ed36d66 100644 (file)
--- a/src/core/SkImageCacherator.cpp
+++ b/src/core/SkImageCacherator.cpp
@@ -246,9 +246,10 @@ bool SkImageCacherator::lockAsBitmap(SkBitmap* bitmap, const SkImage* client,
     }
 
     const uint32_t pixelOpsFlags = 0;
-    if (!tex->readPixels(0, 0, bitmap->width(), bitmap->height(),
+    if (!tex->readPixels(fInfo.colorSpace(), 0, 0, bitmap->width(), bitmap->height(),
                          SkImageInfo2GrPixelConfig(cacheInfo, *tex->getContext()->caps()),
-                         bitmap->getPixels(), bitmap->rowBytes(), pixelOpsFlags)) {
+                         cacheInfo.colorSpace(), bitmap->getPixels(), bitmap->rowBytes(),
+                         pixelOpsFlags)) {
         bitmap->reset();
         return false;
     }

diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp
index 8cbe701..30f596f 100644 (file)
--- a/src/gpu/GrContext.cpp
+++ b/src/gpu/GrContext.cpp
@@ -244,10 +244,12 @@ bool sw_convert_to_premul(GrPixelConfig srcConfig, int width, int height, size_t
     return srcPI.convertPixelsTo(&dstPI, width, height);
 }
 
-bool GrContext::writeSurfacePixels(GrSurface* surface,
+bool GrContext::writeSurfacePixels(GrSurface* surface, SkColorSpace* dstColorSpace,
                                    int left, int top, int width, int height,
-                                   GrPixelConfig srcConfig, const void* buffer, size_t rowBytes,
-                                   uint32_t pixelOpsFlags) {
+                                   GrPixelConfig srcConfig, SkColorSpace* srcColorSpace,
+                                   const void* buffer, size_t rowBytes, uint32_t pixelOpsFlags) {
+    // TODO: Color space conversion
+
     ASSERT_SINGLE_OWNER
     RETURN_FALSE_IF_ABANDONED
     ASSERT_OWNED_RESOURCE(surface);
@@ -389,10 +391,12 @@ bool GrContext::writeSurfacePixels(GrSurface* surface,
     return true;
 }
 
-bool GrContext::readSurfacePixels(GrSurface* src,
+bool GrContext::readSurfacePixels(GrSurface* src, SkColorSpace* srcColorSpace,
                                   int left, int top, int width, int height,
-                                  GrPixelConfig dstConfig, void* buffer, size_t rowBytes,
-                                  uint32_t flags) {
+                                  GrPixelConfig dstConfig, SkColorSpace* dstColorSpace,
+                                  void* buffer, size_t rowBytes, uint32_t flags) {
+    // TODO: Color space conversion
+
     ASSERT_SINGLE_OWNER
     RETURN_FALSE_IF_ABANDONED
     ASSERT_OWNED_RESOURCE(src);

diff --git a/src/gpu/GrRenderTargetContext.cpp b/src/gpu/GrRenderTargetContext.cpp
index 10881de..dcb1d0b 100644 (file)
--- a/src/gpu/GrRenderTargetContext.cpp
+++ b/src/gpu/GrRenderTargetContext.cpp
@@ -1286,8 +1286,8 @@ bool GrRenderTargetContext::readPixels(const SkImageInfo& dstInfo, void* dstBuff
         return false;
     }
 
-    return rt->readPixels(x, y, dstInfo.width(), dstInfo.height(),
-                          config, dstBuffer, dstRowBytes, flags);
+    return rt->readPixels(this->getColorSpace(), x, y, dstInfo.width(), dstInfo.height(),
+                          config, dstInfo.colorSpace(), dstBuffer, dstRowBytes, flags);
 }
 
 bool GrRenderTargetContext::writePixels(const SkImageInfo& srcInfo, const void* srcBuffer,
@@ -1309,8 +1309,8 @@ bool GrRenderTargetContext::writePixels(const SkImageInfo& srcInfo, const void*
         return false;
     }
 
-    return rt->writePixels(x, y, srcInfo.width(), srcInfo.height(),
-                           config, srcBuffer, srcRowBytes, flags);
+    return rt->writePixels(this->getColorSpace(), x, y, srcInfo.width(), srcInfo.height(),
+                           config, srcInfo.colorSpace(), srcBuffer, srcRowBytes, flags);
 }
 
 // Can 'path' be drawn as a pair of filled nested rectangles?

diff --git a/src/gpu/GrSurface.cpp b/src/gpu/GrSurface.cpp
index b978789..c3ac224 100644 (file)
--- a/src/gpu/GrSurface.cpp
+++ b/src/gpu/GrSurface.cpp
@@ -138,28 +138,28 @@ bool GrSurfacePriv::AdjustWritePixelParams(int surfaceWidth,
 
 //////////////////////////////////////////////////////////////////////////////
 
-bool GrSurface::writePixels(int left, int top, int width, int height,
-                            GrPixelConfig config, const void* buffer, size_t rowBytes,
-                            uint32_t pixelOpsFlags) {
+bool GrSurface::writePixels(SkColorSpace* dstColorSpace, int left, int top, int width, int height,
+                            GrPixelConfig config, SkColorSpace* srcColorSpace, const void* buffer,
+                            size_t rowBytes, uint32_t pixelOpsFlags) {
     // go through context so that all necessary flushing occurs
     GrContext* context = this->getContext();
     if (nullptr == context) {
         return false;
     }
-    return context->writeSurfacePixels(this, left, top, width, height, config, buffer,
-                                       rowBytes, pixelOpsFlags);
+    return context->writeSurfacePixels(this, dstColorSpace, left, top, width, height, config,
+                                       srcColorSpace, buffer, rowBytes, pixelOpsFlags);
 }
 
-bool GrSurface::readPixels(int left, int top, int width, int height,
-                           GrPixelConfig config, void* buffer, size_t rowBytes,
-                           uint32_t pixelOpsFlags) {
+bool GrSurface::readPixels(SkColorSpace* srcColorSpace, int left, int top, int width, int height,
+                           GrPixelConfig config, SkColorSpace* dstColorSpace, void* buffer,
+                           size_t rowBytes, uint32_t pixelOpsFlags) {
     // go through context so that all necessary flushing occurs
     GrContext* context = this->getContext();
     if (nullptr == context) {
         return false;
     }
-    return context->readSurfacePixels(this, left, top, width, height, config, buffer,
-                                      rowBytes, pixelOpsFlags);
+    return context->readSurfacePixels(this, srcColorSpace, left, top, width, height, config,
+                                      dstColorSpace, buffer, rowBytes, pixelOpsFlags);
 }
 
 void GrSurface::flushWrites() {

diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp
index c347a1f..cdbdd7c 100644 (file)
--- a/src/image/SkImage_Gpu.cpp
+++ b/src/image/SkImage_Gpu.cpp
@@ -130,8 +130,8 @@ bool SkImage_Gpu::onReadPixels(const SkImageInfo& info, void* pixels, size_t row
         // let the GPU perform this transformation for us
         flags = GrContext::kUnpremul_PixelOpsFlag;
     }
-    if (!fTexture->readPixels(srcX, srcY, info.width(), info.height(), config,
-                              pixels, rowBytes, flags)) {
+    if (!fTexture->readPixels(fColorSpace.get(), srcX, srcY, info.width(), info.height(), config,
+                              info.colorSpace(), pixels, rowBytes, flags)) {
         return false;
     }
     // do we have to manually fix-up the alpha channel?