hide lockpixels api behind flag
guarded by SK_SUPPORT_OBSOLETE_LOCKPIXELS
needs https://codereview.chromium.org/2820873002/# to land first
Bug: skia:6481
Change-Id: I1c39902cbf6fe99f622adfa8192733b95f7fea09
Change-Id: I1c39902cbf6fe99f622adfa8192733b95f7fea09
Reviewed-on: https://skia-review.googlesource.com/13580
Reviewed-by: Florin Malita <fmalita@chromium.org>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Mike Reed <reed@google.com>
diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index 5c6efd4..7e3b2c9 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -478,12 +478,12 @@
break;
}
- SkAutoPixmapUnlock result;
- if (!this->requestLock(&result)) {
+ SkPixmap result;
+ if (!this->peekPixels(&result)) {
return;
}
- if (result.pixmap().erase(c, area)) {
+ if (result.erase(c, area)) {
this->notifyPixelsChanged();
}
}
@@ -566,11 +566,11 @@
bool SkBitmap::readPixels(const SkImageInfo& requestedDstInfo, void* dstPixels, size_t dstRB,
int x, int y) const {
- SkAutoPixmapUnlock src;
- if (!this->requestLock(&src)) {
+ SkPixmap src;
+ if (!this->peekPixels(&src)) {
return false;
}
- return src.pixmap().readPixels(requestedDstInfo, dstPixels, dstRB, x, y);
+ return src.readPixels(requestedDstInfo, dstPixels, dstRB, x, y);
}
bool SkBitmap::readPixels(const SkPixmap& dst, int srcX, int srcY) const {
@@ -579,11 +579,6 @@
bool SkBitmap::writePixels(const SkPixmap& src, int dstX, int dstY,
SkTransferFunctionBehavior behavior) {
- SkAutoPixmapUnlock dst;
- if (!this->requestLock(&dst)) {
- return false;
- }
-
if (!SkImageInfoValidConversion(fInfo, src.info())) {
return false;
}
@@ -605,11 +600,10 @@
return false;
}
- SkAutoPixmapUnlock srcUnlocker;
- if (!this->requestLock(&srcUnlocker)) {
+ SkPixmap srcPM;
+ if (!this->peekPixels(&srcPM)) {
return false;
}
- SkPixmap srcPM = srcUnlocker.pixmap();
// Various Android specific compatibility modes.
// TODO:
@@ -654,13 +648,11 @@
return false;
}
- SkAutoPixmapUnlock dstUnlocker;
- if (!tmpDst.requestLock(&dstUnlocker)) {
+ SkPixmap dstPM;
+ if (!tmpDst.peekPixels(&dstPM)) {
return false;
}
- SkPixmap dstPM = dstUnlocker.pixmap();
-
// 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());
@@ -721,15 +713,14 @@
SkASSERT(alpha != nullptr);
SkASSERT(alphaRowBytes >= src.width());
- SkAutoPixmapUnlock apl;
- if (!src.requestLock(&apl)) {
+ SkPixmap pmap;
+ if (!src.peekPixels(&pmap)) {
for (int y = 0; y < src.height(); ++y) {
memset(alpha, 0, src.width());
alpha += alphaRowBytes;
}
return false;
}
- const SkPixmap& pmap = apl.pixmap();
SkConvertPixels(SkImageInfo::MakeA8(pmap.width(), pmap.height()), alpha, alphaRowBytes,
pmap.info(), pmap.addr(), pmap.rowBytes(), pmap.ctable(),
SkTransferFunctionBehavior::kRespect);
@@ -841,13 +832,13 @@
return;
}
- SkAutoPixmapUnlock result;
- if (!bitmap.requestLock(&result)) {
+ SkPixmap result;
+ if (!bitmap.peekPixels(&result)) {
buffer->writeUInt(0); // instead of snugRB, signaling no pixels
return;
}
- write_raw_pixels(buffer, result.pixmap());
+ write_raw_pixels(buffer, result);
}
bool SkBitmap::ReadRawPixels(SkReadBuffer* buffer, SkBitmap* bitmap) {
@@ -1003,6 +994,7 @@
///////////////////////////////////////////////////////////////////////////////
+#ifdef SK_SUPPORT_OBSOLETE_LOCKPIXELS
bool SkBitmap::requestLock(SkAutoPixmapUnlock* result) const {
SkASSERT(result);
@@ -1031,6 +1023,7 @@
}
return false;
}
+#endif
bool SkBitmap::peekPixels(SkPixmap* pmap) const {
if (fPixels) {
diff --git a/src/core/SkBitmapController.cpp b/src/core/SkBitmapController.cpp
index c72693f..ba287b5 100644
--- a/src/core/SkBitmapController.cpp
+++ b/src/core/SkBitmapController.cpp
@@ -116,7 +116,6 @@
if (fCanShadeHQ) {
fQuality = kHigh_SkFilterQuality;
SkAssertResult(provider.asBitmap(&fResultBitmap));
- fResultBitmap.lockPixels();
return true;
}
@@ -129,15 +128,15 @@
if (!provider.asBitmap(&orig)) {
return false;
}
- SkAutoPixmapUnlock src;
- if (!orig.requestLock(&src)) {
+ SkPixmap src;
+ if (!orig.peekPixels(&src)) {
return false;
}
SkPixmap dst;
SkBitmapCache::RecPtr rec;
const SkImageInfo info = SkImageInfo::MakeN32(desc.fScaledWidth, desc.fScaledHeight,
- src.pixmap().alphaType());
+ src.alphaType());
if (provider.isVolatile()) {
if (!fResultBitmap.tryAllocPixels(info)) {
return false;
@@ -151,7 +150,7 @@
return false;
}
}
- if (!SkBitmapScaler::Resize(dst, src.pixmap(), kHQ_RESIZE_METHOD)) {
+ if (!SkBitmapScaler::Resize(dst, src, kHQ_RESIZE_METHOD)) {
return false; // we failed to create fScaledBitmap
}
if (rec) {
@@ -241,8 +240,6 @@
SkASSERT(fResultBitmap.getPixels());
} else {
(void)provider.asBitmap(&fResultBitmap);
- fResultBitmap.lockPixels();
- // lock may fail to give us pixels
}
SkASSERT(fCanShadeHQ || fQuality <= kLow_SkFilterQuality);
diff --git a/src/core/SkBitmapDevice.cpp b/src/core/SkBitmapDevice.cpp
index fd387ac..09f25fb 100644
--- a/src/core/SkBitmapDevice.cpp
+++ b/src/core/SkBitmapDevice.cpp
@@ -74,7 +74,6 @@
, fRCStack(bitmap.width(), bitmap.height())
{
SkASSERT(valid_for_bitmap_device(bitmap.info(), nullptr));
- fBitmap.lockPixels();
}
SkBitmapDevice* SkBitmapDevice::Create(const SkImageInfo& info) {
@@ -89,7 +88,6 @@
, fRCStack(bitmap.width(), bitmap.height())
{
SkASSERT(valid_for_bitmap_device(bitmap.info(), nullptr));
- fBitmap.lockPixels();
}
SkBitmapDevice* SkBitmapDevice::Create(const SkImageInfo& origInfo,
@@ -134,7 +132,6 @@
SkASSERT(bm.width() == fBitmap.width());
SkASSERT(bm.height() == fBitmap.height());
fBitmap = bm; // intent is to use bm's pixelRef (and rowbytes/config)
- fBitmap.lockPixels();
this->privateResize(fBitmap.info().width(), fBitmap.info().height());
}
diff --git a/src/core/SkBitmapScaler.cpp b/src/core/SkBitmapScaler.cpp
index b4ade85..c803da7 100644
--- a/src/core/SkBitmapScaler.cpp
+++ b/src/core/SkBitmapScaler.cpp
@@ -249,7 +249,6 @@
}
*resultPtr = result;
- resultPtr->lockPixels();
SkASSERT(resultPtr->getPixels());
return true;
}
diff --git a/src/core/SkBlurImageFilter.cpp b/src/core/SkBlurImageFilter.cpp
index 5e7f2a5..320097b 100644
--- a/src/core/SkBlurImageFilter.cpp
+++ b/src/core/SkBlurImageFilter.cpp
@@ -214,8 +214,6 @@
return nullptr;
}
- SkAutoLockPixels inputLock(inputBM), tmpLock(tmp), dstLock(dst);
-
offset->fX = dstBounds.fLeft;
offset->fY = dstBounds.fTop;
SkPMColor* t = tmp.getAddr32(0, 0);
diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index 1b96f49..844de43 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -841,9 +841,8 @@
weAllocated = true;
}
- SkAutoPixmapUnlock unlocker;
- if (bitmap->requestLock(&unlocker)) {
- const SkPixmap& pm = unlocker.pixmap();
+ SkPixmap pm;
+ if (bitmap->peekPixels(&pm)) {
if (this->readPixels(pm.info(), pm.writable_addr(), pm.rowBytes(), x, y)) {
return true;
}
@@ -894,9 +893,8 @@
}
bool SkCanvas::writePixels(const SkBitmap& bitmap, int x, int y) {
- SkAutoPixmapUnlock unlocker;
- if (bitmap.requestLock(&unlocker)) {
- const SkPixmap& pm = unlocker.pixmap();
+ SkPixmap pm;
+ if (bitmap.peekPixels(&pm)) {
return this->writePixels(pm.info(), pm.addr(), pm.rowBytes(), x, y);
}
return false;
diff --git a/src/core/SkDraw.cpp b/src/core/SkDraw.cpp
index 07129f6..1e6c900 100644
--- a/src/core/SkDraw.cpp
+++ b/src/core/SkDraw.cpp
@@ -1170,11 +1170,10 @@
int ix = SkScalarRoundToInt(fMatrix->getTranslateX());
int iy = SkScalarRoundToInt(fMatrix->getTranslateY());
- SkAutoPixmapUnlock result;
- if (!bitmap.requestLock(&result)) {
+ SkPixmap pmap;
+ if (!bitmap.peekPixels(&pmap)) {
return;
}
- const SkPixmap& pmap = result.pixmap();
SkMask mask;
mask.fBounds.set(ix, iy, ix + pmap.width(), iy + pmap.height());
mask.fFormat = SkMask::kA8_Format;
@@ -1290,11 +1289,10 @@
// It is safe to call lock pixels now, since we know the matrix is
// (more or less) identity.
//
- SkAutoPixmapUnlock unlocker;
- if (!bitmap.requestLock(&unlocker)) {
+ SkPixmap pmap;
+ if (!bitmap.peekPixels(&pmap)) {
return;
}
- const SkPixmap& pmap = unlocker.pixmap();
int ix = SkScalarRoundToInt(matrix.getTranslateX());
int iy = SkScalarRoundToInt(matrix.getTranslateY());
if (clipHandlesSprite(*fRC, ix, iy, pmap)) {
@@ -1348,11 +1346,10 @@
SkPaint paint(origPaint);
paint.setStyle(SkPaint::kFill_Style);
- SkAutoPixmapUnlock unlocker;
- if (!bitmap.requestLock(&unlocker)) {
+ SkPixmap pmap;
+ if (!bitmap.peekPixels(&pmap)) {
return;
}
- const SkPixmap& pmap = unlocker.pixmap();
if (nullptr == paint.getColorFilter() && clipHandlesSprite(*fRC, x, y, pmap)) {
// blitter will be owned by the allocator.
diff --git a/src/core/SkImageCacherator.cpp b/src/core/SkImageCacherator.cpp
index a31d31f..c755f5d 100644
--- a/src/core/SkImageCacherator.cpp
+++ b/src/core/SkImageCacherator.cpp
@@ -241,7 +241,6 @@
}
} else {
*bitmap = tmpBitmap;
- bitmap->lockPixels();
bitmap->pixelRef()->setImmutableWithID(uniqueID);
}
return true;
diff --git a/src/core/SkMipMap.cpp b/src/core/SkMipMap.cpp
index d15d2c9..b3766e5 100644
--- a/src/core/SkMipMap.cpp
+++ b/src/core/SkMipMap.cpp
@@ -779,15 +779,10 @@
//
SkMipMap* SkMipMap::Build(const SkBitmap& src, SkDestinationSurfaceColorMode colorMode,
SkDiscardableFactoryProc fact) {
- SkAutoPixmapUnlock srcUnlocker;
- if (!src.requestLock(&srcUnlocker)) {
+ SkPixmap srcPixmap;
+ if (!src.peekPixels(&srcPixmap)) {
return nullptr;
}
- const SkPixmap& srcPixmap = srcUnlocker.pixmap();
- // Try to catch where we might have returned nullptr for src crbug.com/492818
- if (nullptr == srcPixmap.addr()) {
- sk_throw();
- }
return Build(srcPixmap, colorMode, fact);
}
diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp
index 37a450d..3bf20fa 100644
--- a/src/core/SkPixelRef.cpp
+++ b/src/core/SkPixelRef.cpp
@@ -26,9 +26,6 @@
///////////////////////////////////////////////////////////////////////////////
-// just need a > 0 value, so pick a funny one to aid in debugging
-#define SKPIXELREF_PRELOCKED_LOCKCOUNT 123456789
-
static SkImageInfo validate_info(const SkImageInfo& info) {
SkAlphaType newAlphaType = info.alphaType();
SkAssertResult(SkColorTypeValidateAlphaType(info.colorType(), info.alphaType(), &newAlphaType));
@@ -54,6 +51,8 @@
sk_sp<SkColorTable> ctable)
: fInfo(validate_info(info))
, fCTable(std::move(ctable))
+ , fPixels(pixels)
+ , fRowBytes(rowBytes)
#ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK
, fStableID(SkNextID::ImageID())
#endif
@@ -63,13 +62,9 @@
#ifdef SK_TRACE_PIXELREF_LIFETIME
SkDebugf(" pixelref %d\n", sk_atomic_inc(&gInstCounter));
#endif
- fRec.fPixels = pixels;
- fRec.fRowBytes = rowBytes;
- fRec.fColorTable = fCTable.get();
this->needsNewGenID();
fMutability = kMutable;
- fPreLocked = true;
fAddedToCache.store(false);
}
@@ -116,9 +111,15 @@
SkASSERT(!that. genIDIsUnique());
}
+#ifdef SK_SUPPORT_OBSOLETE_LOCKPIXELS
bool SkPixelRef::lockPixels(LockRec* rec) {
- *rec = fRec;
- return true;
+ if (fPixels) {
+ rec->fPixels = fPixels;
+ rec->fRowBytes = fRowBytes;
+ rec->fColorTable = fCTable.get();
+ return true;
+ }
+ return false;
}
bool SkPixelRef::requestLock(const LockRequest& request, LockResult* result) {
@@ -133,12 +134,13 @@
result->fUnlockProc = nullptr;
result->fUnlockContext = nullptr;
- result->fCTable = fRec.fColorTable;
- result->fPixels = fRec.fPixels;
- result->fRowBytes = fRec.fRowBytes;
+ result->fCTable = fCTable.get();
+ result->fPixels = fPixels;
+ result->fRowBytes = fRowBytes;
result->fSize.set(fInfo.width(), fInfo.height());
return true;
}
+#endif
uint32_t SkPixelRef::getGenerationID() const {
uint32_t id = fTaggedGenID.load();
diff --git a/src/core/SkPixmap.cpp b/src/core/SkPixmap.cpp
index 7eac6c4..4d48218 100644
--- a/src/core/SkPixmap.cpp
+++ b/src/core/SkPixmap.cpp
@@ -20,6 +20,7 @@
#include "SkSurface.h"
#include "SkUtils.h"
+#ifdef SK_SUPPORT_OBSOLETE_LOCKPIXELS
void SkAutoPixmapUnlock::reset(const SkPixmap& pm, void (*unlock)(void*), void* ctx) {
SkASSERT(pm.addr() != nullptr);
@@ -29,6 +30,7 @@
fUnlockContext = ctx;
fIsLocked = true;
}
+#endif
/////////////////////////////////////////////////////////////////////////////////////////////////
diff --git a/src/core/SkSpecialImage.cpp b/src/core/SkSpecialImage.cpp
index 0853432..a142576 100644
--- a/src/core/SkSpecialImage.cpp
+++ b/src/core/SkSpecialImage.cpp
@@ -218,11 +218,6 @@
, fBitmap(bm)
{
SkASSERT(bm.pixelRef());
-
- // We have to lock now, while bm is still in scope, since it may have come from our
- // cache, which means we need to keep it locked until we (the special) are done, since
- // we cannot re-generate the cache entry (if bm came from a generator).
- fBitmap.lockPixels();
SkASSERT(fBitmap.getPixels());
}
diff --git a/src/core/SkWriteBuffer.cpp b/src/core/SkWriteBuffer.cpp
index 79422ab..3322e8a 100644
--- a/src/core/SkWriteBuffer.cpp
+++ b/src/core/SkWriteBuffer.cpp
@@ -154,9 +154,9 @@
this->writeBool(false);
// see if the caller wants to manually encode
- SkAutoPixmapUnlock result;
- if (fPixelSerializer && bitmap.requestLock(&result)) {
- sk_sp<SkData> data(fPixelSerializer->encode(result.pixmap()));
+ SkPixmap result;
+ if (fPixelSerializer && bitmap.peekPixels(&result)) {
+ sk_sp<SkData> data(fPixelSerializer->encode(result));
if (data) {
// if we have to "encode" the bitmap, then we assume there is no
// offset to share, since we are effectively creating a new pixelref