SkBitmap::setPixelRef(): less reference churn
BUG=skia:
Change-Id: I9657e4af5fcc808f9175ff336155374ccc77999d
Reviewed-on: https://skia-review.googlesource.com/5461
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Hal Canary <halcanary@google.com>
diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index 24ca72a..f0ade70 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -30,18 +30,48 @@
return false;
}
-SkBitmap::SkBitmap() {
- sk_bzero(this, sizeof(*this));
-}
+SkBitmap::SkBitmap()
+ : fPixelLockCount(0)
+ , fPixels (nullptr)
+ , fColorTable (nullptr)
+ , fPixelRefOrigin{0, 0}
+ , fRowBytes (0)
+ , fFlags (0) {}
-SkBitmap::SkBitmap(const SkBitmap& src) {
+// copy pixelref, but don't copy lock.
+SkBitmap::SkBitmap(const SkBitmap& src)
+ : fPixelRef (src.fPixelRef)
+ , fPixelLockCount(0)
+ , fPixels (nullptr)
+ , fColorTable (nullptr)
+ , fPixelRefOrigin(src.fPixelRefOrigin)
+ , fInfo (src.fInfo)
+ , fRowBytes (src.fRowBytes)
+ , fFlags (src.fFlags)
+{
SkDEBUGCODE(src.validate();)
- sk_bzero(this, sizeof(*this));
- *this = src;
SkDEBUGCODE(this->validate();)
}
-SkBitmap::SkBitmap(SkBitmap&& other) : SkBitmap() { this->swap(other); }
+// take lock and lockcount from other.
+SkBitmap::SkBitmap(SkBitmap&& other)
+ : fPixelRef (std::move(other.fPixelRef))
+ , fPixelLockCount (other.fPixelLockCount)
+ , fPixels (other.fPixels)
+ , fColorTable (other.fColorTable)
+ , fPixelRefOrigin (other.fPixelRefOrigin)
+ , fInfo (std::move(other.fInfo))
+ , fRowBytes (other.fRowBytes)
+ , fFlags (other.fFlags) {
+ SkASSERT(!other.fPixelRef);
+ other.fInfo.reset();
+ other.fPixelLockCount = 0;
+ other.fPixels = nullptr;
+ other.fColorTable = nullptr;
+ other.fPixelRefOrigin = SkIPoint{0, 0};
+ other.fRowBytes = 0;
+ other.fFlags = 0;
+}
SkBitmap::~SkBitmap() {
SkDEBUGCODE(this->validate();)
@@ -51,46 +81,47 @@
SkBitmap& SkBitmap::operator=(const SkBitmap& src) {
if (this != &src) {
this->freePixels();
- this->fPixelRef = SkSafeRef(src.fPixelRef);
- if (this->fPixelRef) {
- // ignore the values if we have a pixelRef
- this->fPixels = nullptr;
- this->fColorTable = nullptr;
- } else {
- this->fPixels = src.fPixels;
- this->fColorTable = src.fColorTable;
- }
- // we reset our locks if we get blown away
- this->fPixelLockCount = 0;
-
- this->fPixelRefOrigin = src.fPixelRefOrigin;
- this->fInfo = src.fInfo;
- this->fRowBytes = src.fRowBytes;
- this->fFlags = src.fFlags;
+ SkASSERT(!fPixels);
+ SkASSERT(!fColorTable);
+ SkASSERT(!fPixelLockCount);
+ fPixelRef = src.fPixelRef;
+ fPixelRefOrigin = src.fPixelRefOrigin;
+ fInfo = src.fInfo;
+ fRowBytes = src.fRowBytes;
+ fFlags = src.fFlags;
}
-
SkDEBUGCODE(this->validate();)
return *this;
}
SkBitmap& SkBitmap::operator=(SkBitmap&& other) {
if (this != &other) {
- this->swap(other);
- other.reset();
+ this->freePixels();
+ SkASSERT(!fPixels);
+ SkASSERT(!fColorTable);
+ SkASSERT(!fPixelLockCount);
+ fPixelRef = std::move(other.fPixelRef);
+ fInfo = std::move(other.fInfo);
+ fPixelLockCount = other.fPixelLockCount;
+ fPixels = other.fPixels;
+ fColorTable = other.fColorTable;
+ fPixelRefOrigin = other.fPixelRefOrigin;
+ fRowBytes = other.fRowBytes;
+ fFlags = other.fFlags;
+ SkASSERT(!other.fPixelRef);
+ other.fInfo.reset();
+ other.fPixelLockCount = 0;
+ other.fPixels = nullptr;
+ other.fColorTable = nullptr;
+ other.fPixelRefOrigin = SkIPoint{0, 0};
+ other.fRowBytes = 0;
+ other.fFlags = 0;
}
return *this;
}
void SkBitmap::swap(SkBitmap& other) {
- SkTSwap(fColorTable, other.fColorTable);
- SkTSwap(fPixelRef, other.fPixelRef);
- SkTSwap(fPixelRefOrigin, other.fPixelRefOrigin);
- SkTSwap(fPixelLockCount, other.fPixelLockCount);
- SkTSwap(fPixels, other.fPixels);
- SkTSwap(fInfo, other.fInfo);
- SkTSwap(fRowBytes, other.fRowBytes);
- SkTSwap(fFlags, other.fFlags);
-
+ SkTSwap(*this, other);
SkDEBUGCODE(this->validate();)
}
@@ -182,7 +213,14 @@
}
}
+#ifdef SK_SUPPORT_LEGACY_BITMAP_SETPIXELREF
SkPixelRef* SkBitmap::setPixelRef(SkPixelRef* pr, int dx, int dy) {
+ this->setPixelRef(sk_ref_sp(pr), dx, dy);
+ return pr;
+}
+#endif
+
+void SkBitmap::setPixelRef(sk_sp<SkPixelRef> pr, int dx, int dy) {
#ifdef SK_DEBUG
if (pr) {
if (kUnknown_SkColorType != fInfo.colorType()) {
@@ -218,15 +256,13 @@
if (fPixelRef != pr) {
this->freePixels();
- SkASSERT(nullptr == fPixelRef);
+ SkASSERT(!fPixelRef);
- SkSafeRef(pr);
- fPixelRef = pr;
+ fPixelRef = std::move(pr);
this->updatePixelsFromRef();
}
SkDEBUGCODE(this->validate();)
- return pr;
}
void SkBitmap::lockPixels() const {
@@ -238,7 +274,7 @@
}
void SkBitmap::unlockPixels() const {
- SkASSERT(nullptr == fPixelRef || fPixelLockCount > 0);
+ SkASSERT(!fPixelRef || fPixelLockCount > 0);
if (fPixelRef && 1 == sk_atomic_dec(&fPixelLockCount)) {
fPixelRef->unlockPixels();
@@ -248,28 +284,25 @@
}
bool SkBitmap::lockPixelsAreWritable() const {
- return (fPixelRef) ? fPixelRef->lockPixelsAreWritable() : false;
+ return fPixelRef ? fPixelRef->lockPixelsAreWritable() : false;
}
void SkBitmap::setPixels(void* p, SkColorTable* ctable) {
if (nullptr == p) {
- this->setPixelRef(nullptr);
+ this->setPixelRef(nullptr, 0, 0);
return;
}
if (kUnknown_SkColorType == fInfo.colorType()) {
- this->setPixelRef(nullptr);
+ this->setPixelRef(nullptr, 0, 0);
return;
}
- SkPixelRef* pr = SkMallocPixelRef::NewDirect(fInfo, p, fRowBytes, ctable);
- if (nullptr == pr) {
- this->setPixelRef(nullptr);
+ sk_sp<SkPixelRef> pr(SkMallocPixelRef::NewDirect(fInfo, p, fRowBytes, ctable));
+ this->setPixelRef(std::move(pr), 0, 0);
+ if (!fPixelRef) {
return;
}
-
- this->setPixelRef(pr)->unref();
-
// since we're already allocated, we lockPixels right away
this->lockPixels();
SkDEBUGCODE(this->validate();)
@@ -301,11 +334,11 @@
SkMallocPixelRef::PRFactory defaultFactory;
- SkPixelRef* pr = defaultFactory.create(correctedInfo, rowBytes, nullptr);
- if (nullptr == pr) {
+ sk_sp<SkPixelRef> pr(defaultFactory.create(correctedInfo, rowBytes, nullptr));
+ if (!pr) {
return reset_return_false(this);
}
- this->setPixelRef(pr)->unref();
+ this->setPixelRef(std::move(pr), 0, 0);
// TODO: lockPixels could/should return bool or void*/nullptr
this->lockPixels();
@@ -332,11 +365,11 @@
factory = &defaultFactory;
}
- SkPixelRef* pr = factory->create(correctedInfo, correctedInfo.minRowBytes(), ctable);
- if (nullptr == pr) {
+ sk_sp<SkPixelRef> pr(factory->create(correctedInfo, correctedInfo.minRowBytes(), ctable));
+ if (!pr) {
return reset_return_false(this);
}
- this->setPixelRef(pr)->unref();
+ this->setPixelRef(std::move(pr), 0, 0);
// TODO: lockPixels could/should return bool or void*/nullptr
this->lockPixels();
@@ -368,14 +401,14 @@
// setInfo may have corrected info (e.g. 565 is always opaque).
const SkImageInfo& correctedInfo = this->info();
- SkPixelRef* pr = SkMallocPixelRef::NewWithProc(correctedInfo, rb, ct, pixels, releaseProc,
- context);
+ sk_sp<SkPixelRef> pr(SkMallocPixelRef::NewWithProc(correctedInfo, rb, ct, pixels, releaseProc,
+ context));
if (!pr) {
this->reset();
return false;
}
- this->setPixelRef(pr)->unref();
+ this->setPixelRef(std::move(pr), 0, 0);
// since we're already allocated, we lockPixels right away
this->lockPixels();
@@ -406,7 +439,6 @@
if (fPixelLockCount > 0) {
fPixelRef->unlockPixels();
}
- fPixelRef->unref();
fPixelRef = nullptr;
fPixelRefOrigin.setZero();
}
@@ -416,7 +448,7 @@
}
uint32_t SkBitmap::getGenerationID() const {
- return (fPixelRef) ? fPixelRef->getGenerationID() : 0;
+ return fPixelRef ? fPixelRef->getGenerationID() : 0;
}
void SkBitmap::notifyPixelsChanged() const {
@@ -439,12 +471,12 @@
return false;
}
- SkPixelRef* pr = SkMallocPixelRef::NewAllocate(info, dst->rowBytes(), ctable);
- if (nullptr == pr) {
+ sk_sp<SkPixelRef> pr(SkMallocPixelRef::NewAllocate(info, dst->rowBytes(), ctable));
+ if (!pr) {
return false;
}
- dst->setPixelRef(pr)->unref();
+ dst->setPixelRef(std::move(pr), 0, 0);
// since we're already allocated, we lockPixels right away
dst->lockPixels();
return true;
@@ -597,7 +629,7 @@
bool SkBitmap::extractSubset(SkBitmap* result, const SkIRect& subset) const {
SkDEBUGCODE(this->validate();)
- if (nullptr == result || nullptr == fPixelRef) {
+ if (nullptr == result || !fPixelRef) {
return false; // no src pixels
}
@@ -621,7 +653,7 @@
origin.fX += r.fLeft;
origin.fY += r.fTop;
// share the pixelref with a custom offset
- dst.setPixelRef(fPixelRef, origin);
+ dst.setPixelRef(fPixelRef, origin.x(), origin.y());
}
SkDEBUGCODE(dst.validate();)
@@ -707,8 +739,9 @@
if (tmpSrc.colorType() == dstColorType && nullptr == alloc) {
dst->swap(tmpSrc);
// If the result is an exact copy, clone the gen ID.
- if (dst->pixelRef() && dst->pixelRef()->info() == fPixelRef->info()) {
- dst->pixelRef()->cloneGenID(*fPixelRef);
+ SkPixelRef* dstPixelRef = dst->pixelRef();
+ if (!dstPixelRef && dstPixelRef->info() == fPixelRef->info()) {
+ dstPixelRef->cloneGenID(*fPixelRef);
}
return true;
}
@@ -895,7 +928,7 @@
void SkBitmap::WriteRawPixels(SkWriteBuffer* buffer, const SkBitmap& bitmap) {
const SkImageInfo info = bitmap.info();
- if (0 == info.width() || 0 == info.height() || nullptr == bitmap.pixelRef()) {
+ if (0 == info.width() || 0 == info.height() || bitmap.isNull()) {
buffer->writeUInt(0); // instead of snugRB, signaling no pixels
return;
}
@@ -981,7 +1014,7 @@
return false;
}
bitmap->setInfo(pr->info());
- bitmap->setPixelRef(pr.get(), 0, 0);
+ bitmap->setPixelRef(std::move(pr), 0, 0);
return true;
}
@@ -1083,7 +1116,7 @@
bool SkBitmap::requestLock(SkAutoPixmapUnlock* result) const {
SkASSERT(result);
- SkPixelRef* pr = fPixelRef;
+ SkPixelRef* pr = fPixelRef.get();
if (nullptr == pr) {
return false;
}
diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index f8069ee..6a825b5 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -876,7 +876,7 @@
}
if (weAllocated) {
- bitmap->setPixelRef(nullptr);
+ bitmap->setPixelRef(nullptr, 0, 0);
}
return false;
}
diff --git a/src/core/SkResourceCache.cpp b/src/core/SkResourceCache.cpp
index 4bdc8dd..47d9f6a 100644
--- a/src/core/SkResourceCache.cpp
+++ b/src/core/SkResourceCache.cpp
@@ -189,8 +189,8 @@
}
SkImageInfo info = bitmap->info();
- bitmap->setPixelRef(new SkOneShotDiscardablePixelRef(info, dm, bitmap->rowBytes(),
- ctable))->unref();
+ bitmap->setPixelRef(
+ sk_make_sp<SkOneShotDiscardablePixelRef>(info, dm, bitmap->rowBytes(), ctable), 0, 0);
bitmap->lockPixels();
return bitmap->readyToDraw();
}
diff --git a/src/core/SkSpecialSurface.cpp b/src/core/SkSpecialSurface.cpp
index 956300c..e6206a0 100644
--- a/src/core/SkSpecialSurface.cpp
+++ b/src/core/SkSpecialSurface.cpp
@@ -63,14 +63,14 @@
class SkSpecialSurface_Raster : public SkSpecialSurface_Base {
public:
- SkSpecialSurface_Raster(SkPixelRef* pr,
+ SkSpecialSurface_Raster(sk_sp<SkPixelRef> pr,
const SkIRect& subset,
const SkSurfaceProps* props)
: INHERITED(subset, props) {
const SkImageInfo& info = pr->info();
fBitmap.setInfo(info, info.minRowBytes());
- fBitmap.setPixelRef(pr);
+ fBitmap.setPixelRef(std::move(pr), 0, 0);
fCanvas.reset(new SkCanvas(fBitmap, this->props()));
fCanvas->clipRect(SkRect::Make(subset));
@@ -93,7 +93,7 @@
sk_sp<SkSpecialSurface> SkSpecialSurface::MakeFromBitmap(const SkIRect& subset, SkBitmap& bm,
const SkSurfaceProps* props) {
- return sk_make_sp<SkSpecialSurface_Raster>(bm.pixelRef(), subset, props);
+ return sk_make_sp<SkSpecialSurface_Raster>(sk_ref_sp(bm.pixelRef()), subset, props);
}
sk_sp<SkSpecialSurface> SkSpecialSurface::MakeRaster(const SkImageInfo& info,
@@ -105,7 +105,7 @@
const SkIRect subset = SkIRect::MakeWH(pr->info().width(), pr->info().height());
- return sk_make_sp<SkSpecialSurface_Raster>(pr.get(), subset, props);
+ return sk_make_sp<SkSpecialSurface_Raster>(std::move(pr), subset, props);
}
#if SK_SUPPORT_GPU
diff --git a/src/effects/gradients/SkGradientShader.cpp b/src/effects/gradients/SkGradientShader.cpp
index df78da9..ca64cb7 100644
--- a/src/effects/gradients/SkGradientShader.cpp
+++ b/src/effects/gradients/SkGradientShader.cpp
@@ -730,7 +730,7 @@
// force our cache32pixelref to be built
(void)cache->getCache32();
bitmap->setInfo(SkImageInfo::MakeN32Premul(kCache32Count, 1));
- bitmap->setPixelRef(cache->getCache32PixelRef());
+ bitmap->setPixelRef(sk_ref_sp(cache->getCache32PixelRef()), 0, 0);
} else {
// For these cases we use the bitmap cache, but not the GradientShaderCache. So just
// allocate and populate the bitmap's data directly.
diff --git a/src/image/SkImage_Raster.cpp b/src/image/SkImage_Raster.cpp
index b14a06e..afd5f90 100644
--- a/src/image/SkImage_Raster.cpp
+++ b/src/image/SkImage_Raster.cpp
@@ -158,7 +158,7 @@
: INHERITED(info.width(), info.height(), pr->getGenerationID())
{
fBitmap.setInfo(info, rowBytes);
- fBitmap.setPixelRef(pr, pixelRefOrigin);
+ fBitmap.setPixelRef(sk_ref_sp(pr), pixelRefOrigin.x(), pixelRefOrigin.y());
fBitmap.lockPixels();
SkASSERT(fBitmap.isImmutable());
}
@@ -364,7 +364,9 @@
// (thus changing our state).
if (fBitmap.isImmutable()) {
bitmap->setInfo(fBitmap.info(), fBitmap.rowBytes());
- bitmap->setPixelRef(fBitmap.pixelRef(), fBitmap.pixelRefOrigin());
+ bitmap->setPixelRef(sk_ref_sp(fBitmap.pixelRef()),
+ fBitmap.pixelRefOrigin().x(),
+ fBitmap.pixelRefOrigin().y());
return true;
}
}
diff --git a/src/image/SkSurface_Raster.cpp b/src/image/SkSurface_Raster.cpp
index 987ff2b..308b072 100644
--- a/src/image/SkSurface_Raster.cpp
+++ b/src/image/SkSurface_Raster.cpp
@@ -20,7 +20,7 @@
SkSurface_Raster(const SkImageInfo&, void*, size_t rb,
void (*releaseProc)(void* pixels, void* context), void* context,
const SkSurfaceProps*);
- SkSurface_Raster(SkPixelRef*, const SkSurfaceProps*);
+ SkSurface_Raster(sk_sp<SkPixelRef>, const SkSurfaceProps*);
SkCanvas* onNewCanvas() override;
sk_sp<SkSurface> onNewSurface(const SkImageInfo&) override;
@@ -108,14 +108,14 @@
fWeOwnThePixels = false; // We are "Direct"
}
-SkSurface_Raster::SkSurface_Raster(SkPixelRef* pr, const SkSurfaceProps* props)
+SkSurface_Raster::SkSurface_Raster(sk_sp<SkPixelRef> pr, const SkSurfaceProps* props)
: INHERITED(pr->info().width(), pr->info().height(), props)
{
const SkImageInfo& info = pr->info();
fBitmap.setInfo(info, pr->rowBytes());
- fBitmap.setPixelRef(pr);
fRowBytes = pr->rowBytes(); // we track this, so that subsequent re-allocs will match
+ fBitmap.setPixelRef(std::move(pr), 0, 0);
fWeOwnThePixels = true;
}
@@ -215,5 +215,5 @@
if (rowBytes) {
SkASSERT(pr->rowBytes() == rowBytes);
}
- return sk_make_sp<SkSurface_Raster>(pr.get(), props);
+ return sk_make_sp<SkSurface_Raster>(std::move(pr), props);
}
diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp
index 4f21489..005ce67 100644
--- a/src/pdf/SkPDFDevice.cpp
+++ b/src/pdf/SkPDFDevice.cpp
@@ -91,7 +91,7 @@
SkASSERT(bitmap.pixelRef());
SkBitmap tmp;
tmp.setInfo(bitmap.pixelRef()->info(), bitmap.rowBytes());
- tmp.setPixelRef(bitmap.pixelRef());
+ tmp.setPixelRef(sk_ref_sp(bitmap.pixelRef()), 0, 0);
tmp.lockPixels();
auto img = SkImage::MakeFromBitmap(tmp);
if (img) {