From e53120754b358e950ff417d864f80004697b5a91 Mon Sep 17 00:00:00 2001 From: Brian Osman Date: Tue, 20 Jun 2017 09:35:51 -0400 Subject: [PATCH] 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 Reviewed-by: Matt Sarett --- src/core/SkColorSpaceXformImageGenerator.h | 3 +- src/core/SkImagePriv.h | 4 +-- src/image/SkImage_Raster.cpp | 41 ++++++++++++++-------- tests/ImageGeneratorTest.cpp | 9 ++++- 4 files changed, 39 insertions(+), 18 deletions(-) diff --git a/src/core/SkColorSpaceXformImageGenerator.h b/src/core/SkColorSpaceXformImageGenerator.h index 3bab29f46f..c548fa903f 100644 --- a/src/core/SkColorSpaceXformImageGenerator.h +++ b/src/core/SkColorSpaceXformImageGenerator.h @@ -37,7 +37,8 @@ private: const SkBitmap& src, sk_sp dst, SkCopyPixelsMode, uint32_t id); SkColorSpaceXformImageGenerator(const SkBitmap& src, sk_sp dst, uint32_t id); - friend sk_sp SkMakeImageInColorSpace(const SkBitmap&, sk_sp, uint32_t); + friend sk_sp SkMakeImageInColorSpace(const SkBitmap&, sk_sp, uint32_t, + SkCopyPixelsMode); typedef SkImageGenerator INHERITED; }; diff --git a/src/core/SkImagePriv.h b/src/core/SkImagePriv.h index d4785c69dd..509b7fa34f 100644 --- a/src/core/SkImagePriv.h +++ b/src/core/SkImagePriv.h @@ -47,7 +47,6 @@ extern sk_sp 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 SkMakeImageFromRasterBitmap(const SkBitmap&, SkCopyPixelsM * it will generate a new id. */ extern sk_sp SkMakeImageInColorSpace(const SkBitmap& src, sk_sp 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, diff --git a/src/image/SkImage_Raster.cpp b/src/image/SkImage_Raster.cpp index fcdfce2371..fc2e030ec8 100644 --- a/src/image/SkImage_Raster.cpp +++ b/src/image/SkImage_Raster.cpp @@ -78,7 +78,8 @@ public: return true; } - SkImage_Raster(const SkImageInfo&, sk_sp, size_t rb, SkColorTable*); + SkImage_Raster(const SkImageInfo&, sk_sp, 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 data, size_t rowBytes, - SkColorTable* ctable) - : INHERITED(info.width(), info.height(), kNeedNewImageUniqueID) + SkColorTable* ctable, uint32_t id) + : INHERITED(info.width(), info.height(), id) { void* addr = const_cast(data->data()); @@ -261,18 +262,22 @@ sk_sp SkImage_Raster::onMakeSubset(const SkIRect& subset) const { /////////////////////////////////////////////////////////////////////////////// -sk_sp SkImage::MakeRasterCopy(const SkPixmap& pmap) { +sk_sp 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 data(SkData::MakeWithCopy(pmap.addr(), size)); - return sk_make_sp(pmap.info(), std::move(data), pmap.rowBytes(), pmap.ctable()); + return sk_make_sp(pmap.info(), std::move(data), pmap.rowBytes(), pmap.ctable(), + id); } +sk_sp SkImage::MakeRasterCopy(const SkPixmap& pmap) { + return MakeRasterCopyPriv(pmap, kNeedNewImageUniqueID); +} sk_sp SkImage::MakeRasterData(const SkImageInfo& info, sk_sp data, size_t rowBytes) { @@ -303,11 +308,12 @@ sk_sp SkImage::MakeFromRaster(const SkPixmap& pmap, RasterReleaseProc p return sk_make_sp(pmap.info(), std::move(data), pmap.rowBytes(), pmap.ctable()); } -sk_sp SkMakeImageFromRasterBitmapPriv(const SkBitmap& bm, SkCopyPixelsMode cpm) { +sk_sp 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(); } @@ -321,10 +327,11 @@ sk_sp SkMakeImageFromRasterBitmap(const SkBitmap& bm, SkCopyPixelsMode return nullptr; } - return SkMakeImageFromRasterBitmapPriv(bm, cpm); + return SkMakeImageFromRasterBitmapPriv(bm, cpm, kNeedNewImageUniqueID); } -sk_sp SkMakeImageInColorSpace(const SkBitmap& bm, sk_sp dstCS, uint32_t id) { +sk_sp SkMakeImageInColorSpace(const SkBitmap& bm, sk_sp 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 SkMakeImageInColorSpace(const SkBitmap& bm, sk_sp d srcCS = SkColorSpace::MakeSRGB(); } + sk_sp 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); - return SkMakeImageFromRasterBitmapPriv(bm, kNever_SkCopyPixelsMode); + SkASSERT(kNeedNewImageUniqueID == id || bm.getGenerationID() == id); + image = SkMakeImageFromRasterBitmapPriv(bm, cpm, id); + } else { + image = SkImage::MakeFromGenerator(SkColorSpaceXformImageGenerator::Make(bm, dstCS, cpm, + id)); } - return SkImage::MakeFromGenerator(SkColorSpaceXformImageGenerator::Make( - bm, dstCS, kNever_SkCopyPixelsMode, id)); + // If the caller suplied an id, we must propagate that to the image we return + SkASSERT(kNeedNewImageUniqueID == id || image->uniqueID() == id); + return image; } const SkPixelRef* SkBitmapImageGetPixelRef(const SkImage* image) { diff --git a/tests/ImageGeneratorTest.cpp b/tests/ImageGeneratorTest.cpp index bc9dca4b18..5cfd410ad0 100644 --- a/tests/ImageGeneratorTest.cpp +++ b/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 srgb = SkColorSpace::MakeSRGB(); sk_sp ia = SkMakeImageInColorSpace(a, srgb, 0); sk_sp ib = SkMakeImageInColorSpace(b, srgb, b.getGenerationID()); sk_sp ic = SkMakeImageInColorSpace(c, srgb, c.getGenerationID()); sk_sp id = SkMakeImageInColorSpace(d, srgb, 0); + sk_sp 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()); }