Add SkCopyPixelsMode argument to SkMakeImageInColorSpace

Deferred rendering in Android will need the ability to use this in
kIfMutable mode.

Bug: skia:
Change-Id: I5194f2c50f9d17351fdab49373ca9bc1e80cf586
Reviewed-on: https://skia-review.googlesource.com/20157
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Matt Sarett <msarett@google.com>

src/core/SkColorSpaceXformImageGenerator.h

@@ -37,7 +37,8 @@ private:
             const SkBitmap& src, sk_sp<SkColorSpace> dst, SkCopyPixelsMode, uint32_t id);
     SkColorSpaceXformImageGenerator(const SkBitmap& src, sk_sp<SkColorSpace> dst, uint32_t id);
 
-    friend sk_sp<SkImage> SkMakeImageInColorSpace(const SkBitmap&, sk_sp<SkColorSpace>, uint32_t);
+    friend sk_sp<SkImage> SkMakeImageInColorSpace(const SkBitmap&, sk_sp<SkColorSpace>, uint32_t,
+                                                  SkCopyPixelsMode);
     typedef SkImageGenerator INHERITED;
 };
 

src/core/SkImagePriv.h

@@ -47,7 +47,6 @@ extern sk_sp<SkImage> SkMakeImageFromRasterBitmap(const SkBitmap&, SkCopyPixelsM
 
 /**
  *  Similar to SkMakeImageFromRasterBitmap, this wraps a |src| bitmap in an image.
- *  This always operates in kNever_SkCopyPixelsMode.
  *
  *  It also promises to transform the bitmap into the |dst| color space before it
  *  is drawn.  The transform will happen lazily.
@@ -56,7 +55,8 @@ extern sk_sp<SkImage> SkMakeImageFromRasterBitmap(const SkBitmap&, SkCopyPixelsM
  *  it will generate a new id.
  */
 extern sk_sp<SkImage> SkMakeImageInColorSpace(const SkBitmap& src, sk_sp<SkColorSpace> dst,
-                                              uint32_t id);
+                                              uint32_t id,
+                                              SkCopyPixelsMode = kNever_SkCopyPixelsMode);
 
 // Given an image created from SkNewImageFromBitmap, return its pixelref. This
 // may be called to see if the surface and the image share the same pixelref,

src/image/SkImage_Raster.cpp

@@ -78,7 +78,8 @@ public:
         return true;
     }
 
-    SkImage_Raster(const SkImageInfo&, sk_sp<SkData>, size_t rb, SkColorTable*);
+    SkImage_Raster(const SkImageInfo&, sk_sp<SkData>, size_t rb, SkColorTable*,
+                   uint32_t id = kNeedNewImageUniqueID);
     ~SkImage_Raster() override;
 
     SkImageInfo onImageInfo() const override {
@@ -145,8 +146,8 @@ static void release_data(void* addr, void* context) {
 }
 
 SkImage_Raster::SkImage_Raster(const Info& info, sk_sp<SkData> data, size_t rowBytes,
-                               SkColorTable* ctable)
+                               SkColorTable* ctable, uint32_t id)
-    : INHERITED(info.width(), info.height(), kNeedNewImageUniqueID)
+    : INHERITED(info.width(), info.height(), id)
 {
     void* addr = const_cast<void*>(data->data());
 
@@ -261,18 +262,22 @@ sk_sp<SkImage> SkImage_Raster::onMakeSubset(const SkIRect& subset) const {
 
 ///////////////////////////////////////////////////////////////////////////////
 
-sk_sp<SkImage> SkImage::MakeRasterCopy(const SkPixmap& pmap) {
+sk_sp<SkImage> MakeRasterCopyPriv(const SkPixmap& pmap, uint32_t id) {
     size_t size;
     if (!SkImage_Raster::ValidArgs(pmap.info(), pmap.rowBytes(),
-                                   pmap.ctable() != nullptr, &size) || !pmap.addr()) {
+        pmap.ctable() != nullptr, &size) || !pmap.addr()) {
         return nullptr;
     }
 
     // Here we actually make a copy of the caller's pixel data
     sk_sp<SkData> data(SkData::MakeWithCopy(pmap.addr(), size));
-    return sk_make_sp<SkImage_Raster>(pmap.info(), std::move(data), pmap.rowBytes(), pmap.ctable());
+    return sk_make_sp<SkImage_Raster>(pmap.info(), std::move(data), pmap.rowBytes(), pmap.ctable(),
+                                      id);
 }
 
+sk_sp<SkImage> SkImage::MakeRasterCopy(const SkPixmap& pmap) {
+    return MakeRasterCopyPriv(pmap, kNeedNewImageUniqueID);
+}
 
 sk_sp<SkImage> SkImage::MakeRasterData(const SkImageInfo& info, sk_sp<SkData> data,
                                        size_t rowBytes) {
@@ -303,11 +308,12 @@ sk_sp<SkImage> SkImage::MakeFromRaster(const SkPixmap& pmap, RasterReleaseProc p
     return sk_make_sp<SkImage_Raster>(pmap.info(), std::move(data), pmap.rowBytes(), pmap.ctable());
 }
 
-sk_sp<SkImage> SkMakeImageFromRasterBitmapPriv(const SkBitmap& bm, SkCopyPixelsMode cpm) {
+sk_sp<SkImage> SkMakeImageFromRasterBitmapPriv(const SkBitmap& bm, SkCopyPixelsMode cpm,
+                                               uint32_t idForCopy) {
     if (kAlways_SkCopyPixelsMode == cpm || (!bm.isImmutable() && kNever_SkCopyPixelsMode != cpm)) {
         SkPixmap pmap;
         if (bm.peekPixels(&pmap)) {
-            return SkImage::MakeRasterCopy(pmap);
+            return MakeRasterCopyPriv(pmap, idForCopy);
         } else {
             return sk_sp<SkImage>();
         }
@@ -321,10 +327,11 @@ sk_sp<SkImage> SkMakeImageFromRasterBitmap(const SkBitmap& bm, SkCopyPixelsMode
         return nullptr;
     }
 
-    return SkMakeImageFromRasterBitmapPriv(bm, cpm);
+    return SkMakeImageFromRasterBitmapPriv(bm, cpm, kNeedNewImageUniqueID);
 }
 
-sk_sp<SkImage> SkMakeImageInColorSpace(const SkBitmap& bm, sk_sp<SkColorSpace> dstCS, uint32_t id) {
+sk_sp<SkImage> SkMakeImageInColorSpace(const SkBitmap& bm, sk_sp<SkColorSpace> dstCS, uint32_t id,
+                                       SkCopyPixelsMode cpm) {
     if (!SkImageInfoIsValidAllowNumericalCS(bm.info()) || !bm.getPixels() ||
             bm.rowBytes() < bm.info().minRowBytes() || !dstCS) {
         return nullptr;
@@ -336,14 +343,20 @@ sk_sp<SkImage> SkMakeImageInColorSpace(const SkBitmap& bm, sk_sp<SkColorSpace> d
         srcCS = SkColorSpace::MakeSRGB();
     }
 
+    sk_sp<SkImage> image = nullptr;
+
     // For the Android use case, this is very likely to be true.
     if (SkColorSpace::Equals(srcCS.get(), dstCS.get())) {
-        SkASSERT(0 == id || bm.getGenerationID() == id);
+        SkASSERT(kNeedNewImageUniqueID == id || bm.getGenerationID() == id);
-        return SkMakeImageFromRasterBitmapPriv(bm, kNever_SkCopyPixelsMode);
+        image = SkMakeImageFromRasterBitmapPriv(bm, cpm, id);
+    } else {
+        image = SkImage::MakeFromGenerator(SkColorSpaceXformImageGenerator::Make(bm, dstCS, cpm,
+                                                                                 id));
     }
 
-    return SkImage::MakeFromGenerator(SkColorSpaceXformImageGenerator::Make(
+    // If the caller suplied an id, we must propagate that to the image we return
-            bm, dstCS, kNever_SkCopyPixelsMode, id));
+    SkASSERT(kNeedNewImageUniqueID == id || image->uniqueID() == id);
+    return image;
 }
 
 const SkPixelRef* SkBitmapImageGetPixelRef(const SkImage* image) {

tests/ImageGeneratorTest.cpp

@@ -113,7 +113,7 @@ DEF_TEST(PictureImageGenerator, reporter) {
 #include "SkImagePriv.h"
 
 DEF_TEST(ColorXformGenerator, r) {
-    SkBitmap a, b, c, d;
+    SkBitmap a, b, c, d, e;
     SkImageInfo info = SkImageInfo::MakeS32(1, 1, kPremul_SkAlphaType);
     a.allocPixels(info);
     b.allocPixels(info.makeColorSpace(nullptr));
@@ -121,16 +121,20 @@ DEF_TEST(ColorXformGenerator, r) {
                                                             SkColorSpace::kRec2020_Gamut)));
     d.allocPixels(info.makeColorSpace(SkColorSpace::MakeRGB(SkColorSpace::kSRGB_RenderTargetGamma,
                                                             SkColorSpace::kAdobeRGB_Gamut)));
+    e.allocPixels(info);
     a.eraseColor(0);
     b.eraseColor(1);
     c.eraseColor(2);
     d.eraseColor(3);
+    e.eraseColor(4);
 
     sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
     sk_sp<SkImage> ia = SkMakeImageInColorSpace(a, srgb, 0);
     sk_sp<SkImage> ib = SkMakeImageInColorSpace(b, srgb, b.getGenerationID());
     sk_sp<SkImage> ic = SkMakeImageInColorSpace(c, srgb, c.getGenerationID());
     sk_sp<SkImage> id = SkMakeImageInColorSpace(d, srgb, 0);
+    sk_sp<SkImage> ie = SkMakeImageInColorSpace(e, srgb, e.getGenerationID(),
+                                                kAlways_SkCopyPixelsMode);
 
     // Equal because sRGB->sRGB is a no-op.
     REPORTER_ASSERT(r, ia->uniqueID() == a.getGenerationID());
@@ -145,4 +149,7 @@ DEF_TEST(ColorXformGenerator, r) {
 
     // Not equal because sRGB->Adobe is not a no-op and we do not pass an explicit id.
     REPORTER_ASSERT(r, id->uniqueID() != d.getGenerationID());
+
+    // Equal because we pass in an explicit id. Forcing a copy, but still want the id respected.
+    REPORTER_ASSERT(r, ie->uniqueID() == e.getGenerationID());
 }