From d4a338f4d0a0cdc08d7d3668931c60997f0fa971 Mon Sep 17 00:00:00 2001 From: Matt Sarett Date: Thu, 27 Apr 2017 12:45:45 -0400 Subject: Delete copyTo(Allocator), hide copyTo() behind flag Replace uses of copyTo() in Skia. Bug: skia:6464 Change-Id: I921dc53a1c29a5176d18f05741f7c0b5a008e548 Reviewed-on: https://skia-review.googlesource.com/14502 Commit-Queue: Matt Sarett Reviewed-by: Mike Reed --- src/core/SkBitmap.cpp | 81 ++---------------------------------- src/core/SkPixelRef.cpp | 14 ------- src/core/SkSpecialImage.cpp | 8 ++-- src/pdf/SkPDFBitmap.cpp | 3 +- src/ports/SkImageEncoder_CG.cpp | 7 ++-- src/ports/SkImageEncoder_WIC.cpp | 4 +- src/utils/mac/SkCreateCGImageRef.cpp | 5 ++- 7 files changed, 21 insertions(+), 101 deletions(-) (limited to 'src') diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp index 8f131d15f5..1ba2968466 100644 --- a/src/core/SkBitmap.cpp +++ b/src/core/SkBitmap.cpp @@ -549,7 +549,6 @@ bool SkBitmap::canCopyTo(SkColorType dstCT) const { case kRGB_565_SkColorType: case kRGBA_8888_SkColorType: case kBGRA_8888_SkColorType: - case kRGBA_F16_SkColorType: break; case kGray_8_SkColorType: if (!sameConfigs) { @@ -595,7 +594,8 @@ bool SkBitmap::writePixels(const SkPixmap& src, int dstX, int dstY, return true; } -bool SkBitmap::internalCopyTo(SkBitmap* dst, SkColorType dstColorType, Allocator* alloc) const { +#ifdef SK_SUPPORT_LEGACY_BITMAP_COPYTO +bool SkBitmap::copyTo(SkBitmap* dst, SkColorType dstColorType) const { if (!this->canCopyTo(dstColorType)) { return false; } @@ -605,36 +605,8 @@ bool SkBitmap::internalCopyTo(SkBitmap* dst, SkColorType dstColorType, Allocator return false; } - // Various Android specific compatibility modes. - // TODO: - // Move the logic of this entire function into the framework, then call readPixels() directly. - SkImageInfo dstInfo = srcPM.info().makeColorType(dstColorType); - switch (dstColorType) { - case kRGB_565_SkColorType: - // copyTo() is not strict on alpha type. Here we set the src to opaque to allow - // the call to readPixels() to succeed and preserve this lenient behavior. - if (kOpaque_SkAlphaType != srcPM.alphaType()) { - srcPM = SkPixmap(srcPM.info().makeAlphaType(kOpaque_SkAlphaType), srcPM.addr(), - srcPM.rowBytes(), srcPM.ctable()); - dstInfo = dstInfo.makeAlphaType(kOpaque_SkAlphaType); - } - break; - case kRGBA_F16_SkColorType: - // The caller does not have an opportunity to pass a dst color space. Assume that - // they want linear sRGB. - dstInfo = dstInfo.makeColorSpace(SkColorSpace::MakeSRGBLinear()); - - if (!srcPM.colorSpace()) { - // We can't do a sane conversion to F16 without a dst color space. Guess sRGB - // in this case. - srcPM.setColorSpace(SkColorSpace::MakeSRGB()); - } - break; - default: - break; - } - SkBitmap tmpDst; + SkImageInfo dstInfo = srcPM.info().makeColorType(dstColorType); if (!tmpDst.setInfo(dstInfo)) { return false; } @@ -644,7 +616,7 @@ bool SkBitmap::internalCopyTo(SkBitmap* dst, SkColorType dstColorType, Allocator if (dstColorType == kIndex_8_SkColorType) { ctable.reset(SkRef(srcPM.ctable())); } - if (!tmpDst.tryAllocPixels(alloc, ctable.get())) { + if (!tmpDst.tryAllocPixels(ctable.get())) { return false; } @@ -653,60 +625,15 @@ bool SkBitmap::internalCopyTo(SkBitmap* dst, SkColorType dstColorType, Allocator return false; } - // We can't do a sane conversion from F16 without a src color space. Guess sRGB in this case. - if (kRGBA_F16_SkColorType == srcPM.colorType() && !dstPM.colorSpace()) { - dstPM.setColorSpace(SkColorSpace::MakeSRGB()); - } - - // readPixels does not yet support color spaces with parametric transfer functions. This - // works around that restriction when the color spaces are equal. - if (kRGBA_F16_SkColorType != dstColorType && kRGBA_F16_SkColorType != srcPM.colorType() && - dstPM.colorSpace() == srcPM.colorSpace()) { - dstPM.setColorSpace(nullptr); - srcPM.setColorSpace(nullptr); - } - if (!srcPM.readPixels(dstPM)) { return false; } - // (for BitmapHeap) Clone the pixelref genID even though we have a new pixelref. - // The old copyTo impl did this, so we continue it for now. - // - // TODO: should we ignore rowbytes (i.e. getSize)? Then it could just be - // if (src_pixelref->info == dst_pixelref->info) - // - if (srcPM.colorType() == dstColorType && tmpDst.getSize() == srcPM.getSize64()) { - SkPixelRef* dstPixelRef = tmpDst.pixelRef(); - if (dstPixelRef->info() == fPixelRef->info()) { - dstPixelRef->cloneGenID(*fPixelRef); - } - } - dst->swap(tmpDst); return true; } - -bool SkBitmap::copyTo(SkBitmap* dst, SkColorType ct) const { - return this->internalCopyTo(dst, ct, nullptr); -} - -#ifdef SK_BUILD_FOR_ANDROID -bool SkBitmap::copyTo(SkBitmap* dst, SkColorType ct, Allocator* alloc) const { - return this->internalCopyTo(dst, ct, alloc); -} #endif -// TODO: can we merge this with copyTo? -bool SkBitmap::deepCopyTo(SkBitmap* dst) const { - const SkColorType dstCT = this->colorType(); - - if (!this->canCopyTo(dstCT)) { - return false; - } - return this->copyTo(dst, dstCT); -} - /////////////////////////////////////////////////////////////////////////////// static bool GetBitmapAlpha(const SkBitmap& src, uint8_t* SK_RESTRICT alpha, int alphaRowBytes) { diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp index 5834b66936..c6d59eab19 100644 --- a/src/core/SkPixelRef.cpp +++ b/src/core/SkPixelRef.cpp @@ -96,20 +96,6 @@ void SkPixelRef::needsNewGenID() { SkASSERT(!this->genIDIsUnique()); // This method isn't threadsafe, so the assert should be fine. } -void SkPixelRef::cloneGenID(const SkPixelRef& that) { - // This is subtle. We must call that.getGenerationID() to make sure its genID isn't 0. - uint32_t genID = that.getGenerationID(); - - // Neither ID is unique any more. - // (These & ~1u are actually redundant. that.getGenerationID() just did it for us.) - this->fTaggedGenID.store(genID & ~1u); - that. fTaggedGenID.store(genID & ~1u); - - // This method isn't threadsafe, so these asserts should be fine. - SkASSERT(!this->genIDIsUnique()); - SkASSERT(!that. genIDIsUnique()); -} - uint32_t SkPixelRef::getGenerationID() const { uint32_t id = fTaggedGenID.load(); if (0 == id) { diff --git a/src/core/SkSpecialImage.cpp b/src/core/SkSpecialImage.cpp index a142576c2e..b22335bb75 100644 --- a/src/core/SkSpecialImage.cpp +++ b/src/core/SkSpecialImage.cpp @@ -327,13 +327,15 @@ sk_sp SkSpecialImage::MakeFromRaster(const SkIRect& subset, } const SkBitmap* srcBM = &bm; - SkBitmap tmpStorage; + SkBitmap tmp; // ImageFilters only handle N32 at the moment, so force our src to be that if (!valid_for_imagefilters(bm.info())) { - if (!bm.copyTo(&tmpStorage, kN32_SkColorType)) { + if (!tmp.tryAllocPixels(bm.info().makeColorType(kN32_SkColorType)) || + !bm.readPixels(tmp.info(), tmp.getPixels(), tmp.rowBytes(), 0, 0)) + { return nullptr; } - srcBM = &tmpStorage; + srcBM = &tmp; } return sk_make_sp(subset, *srcBM, props); } diff --git a/src/pdf/SkPDFBitmap.cpp b/src/pdf/SkPDFBitmap.cpp index 19460a44f0..b58aaf48a5 100644 --- a/src/pdf/SkPDFBitmap.cpp +++ b/src/pdf/SkPDFBitmap.cpp @@ -147,7 +147,8 @@ static const SkBitmap& not4444(const SkBitmap& input, SkBitmap* copy) { return input; } // ARGB_4444 is rarely used, so we can do a wasteful tmp copy. - SkAssertResult(input.copyTo(copy, kN32_SkColorType)); + copy->allocPixels(input.info().makeColorType(kN32_SkColorType)); + SkAssertResult(input.readPixels(copy->info(), copy->getPixels(), copy->rowBytes(), 0, 0)); copy->setImmutable(); return *copy; } diff --git a/src/ports/SkImageEncoder_CG.cpp b/src/ports/SkImageEncoder_CG.cpp index b3fd243324..8c2a542fc3 100644 --- a/src/ports/SkImageEncoder_CG.cpp +++ b/src/ports/SkImageEncoder_CG.cpp @@ -89,9 +89,10 @@ bool SkEncodeImageWithCG(SkWStream* stream, const SkPixmap& pixmap, SkEncodedIma // : CGImageDestinationFinalize image destination does not have enough images // So instead we copy to 8888. if (bm.colorType() == kARGB_4444_SkColorType) { - SkBitmap bitmap8888; - bm.copyTo(&bitmap8888, kN32_SkColorType); - bm.swap(bitmap8888); + SkBitmap bitmapN32; + bitmapN32.allocPixels(bm.info().makeColorType(kN32_SkColorType)); + bm.readPixels(bitmapN32.info(), bitmapN32.getPixels(), bitmapN32.rowBytes(), 0, 0); + bm.swap(bitmapN32); } type = kUTTypePNG; break; diff --git a/src/ports/SkImageEncoder_WIC.cpp b/src/ports/SkImageEncoder_WIC.cpp index 6d355c1d47..1ae9257395 100644 --- a/src/ports/SkImageEncoder_WIC.cpp +++ b/src/ports/SkImageEncoder_WIC.cpp @@ -70,7 +70,9 @@ bool SkEncodeImageWithWIC(SkWStream* stream, const SkPixmap& pixmap, // First convert to BGRA if necessary. SkBitmap bitmap; - if (!bitmapOrig.copyTo(&bitmap, kBGRA_8888_SkColorType)) { + if (!bitmap.tryAllocPixels(bitmapOrig.info().makeColorType(kBGRA_8888_SkColorType)) || + !bitmapOrig.readPixels(bitmap.info(), bitmap.getPixels(), bitmap.rowBytes(), 0, 0)) + { return false; } diff --git a/src/utils/mac/SkCreateCGImageRef.cpp b/src/utils/mac/SkCreateCGImageRef.cpp index 880c566c73..b5df42364b 100644 --- a/src/utils/mac/SkCreateCGImageRef.cpp +++ b/src/utils/mac/SkCreateCGImageRef.cpp @@ -110,9 +110,10 @@ static SkBitmap* prepareForImageRef(const SkBitmap& bm, SkBitmap* copy; if (upscaleTo32) { copy = new SkBitmap; - // here we make a ceep copy of the pixels, since CG won't take our + // here we make a deep copy of the pixels, since CG won't take our // 565 directly - bm.copyTo(copy, kN32_SkColorType); + copy->allocPixels(bm.info().makeColorType(kN32_SkColorType)); + bm.readPixels(copy->info(), copy->getPixels(), copy->rowBytes(), 0, 0); } else { copy = new SkBitmap(bm); } -- cgit v1.2.3