unify pixelref and image ID space, so we can share IDs when we share pixels
I view this as a performance opportunity, not a feature or bug fix per-se.
BUG=skia:
Review URL: https://codereview.chromium.org/1266883002
diff --git a/include/core/SkImage.h b/include/core/SkImage.h
index 2b759b8..e89ffa5 100644
--- a/include/core/SkImage.h
+++ b/include/core/SkImage.h
@@ -283,22 +283,13 @@
bool asLegacyBitmap(SkBitmap*, LegacyBitmapMode) const;
protected:
- SkImage(int width, int height) :
- fWidth(width),
- fHeight(height),
- fUniqueID(NextUniqueID()) {
-
- SkASSERT(width > 0);
- SkASSERT(height > 0);
- }
+ SkImage(int width, int height, uint32_t uniqueID);
private:
const int fWidth;
const int fHeight;
const uint32_t fUniqueID;
- static uint32_t NextUniqueID();
-
typedef SkRefCnt INHERITED;
};
diff --git a/src/core/SkNextID.h b/src/core/SkNextID.h
new file mode 100644
index 0000000..570fd94
--- /dev/null
+++ b/src/core/SkNextID.h
@@ -0,0 +1,21 @@
+/*
+ * Copyright 2015 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef SkNextID_DEFINED
+#define SkNextID_DEFINED
+
+#include "SkTypes.h"
+
+class SkNextID {
+public:
+ /**
+ * Shared between SkPixelRef's generationID and SkImage's uniqueID
+ */
+ static uint32_t ImageID();
+};
+
+#endif
diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp
index 8e0f212..0edad83 100644
--- a/src/core/SkPixelRef.cpp
+++ b/src/core/SkPixelRef.cpp
@@ -59,15 +59,16 @@
}
///////////////////////////////////////////////////////////////////////////////
+#include "SkNextID.h"
-static uint32_t next_gen_id() {
- static uint32_t gNextGenID = 0;
- uint32_t genID;
+uint32_t SkNextID::ImageID() {
+ static uint32_t gID = 0;
+ uint32_t id;
// Loop in case our global wraps around, as we never want to return a 0.
do {
- genID = sk_atomic_fetch_add(&gNextGenID, 2u) + 2; // Never set the low bit.
- } while (0 == genID);
- return genID;
+ id = sk_atomic_fetch_add(&gID, 2u) + 2; // Never set the low bit.
+ } while (0 == id);
+ return id;
}
///////////////////////////////////////////////////////////////////////////////
@@ -300,7 +301,7 @@
uint32_t SkPixelRef::getGenerationID() const {
uint32_t id = fTaggedGenID.load();
if (0 == id) {
- uint32_t next = next_gen_id() | 1u;
+ uint32_t next = SkNextID::ImageID() | 1u;
if (fTaggedGenID.compare_exchange(&id, next)) {
id = next; // There was no race or we won the race. fTaggedGenID is next now.
} else {
diff --git a/src/image/SkImage.cpp b/src/image/SkImage.cpp
index 081785f..a17f336 100644
--- a/src/image/SkImage.cpp
+++ b/src/image/SkImage.cpp
@@ -11,6 +11,7 @@
#include "SkImageGenerator.h"
#include "SkImagePriv.h"
#include "SkImage_Base.h"
+#include "SkNextID.h"
#include "SkPixelRef.h"
#include "SkReadPixelsRec.h"
#include "SkString.h"
@@ -22,15 +23,13 @@
#include "SkImage_Gpu.h"
#endif
-uint32_t SkImage::NextUniqueID() {
- static int32_t gUniqueID;
-
- // never return 0;
- uint32_t id;
- do {
- id = sk_atomic_inc(&gUniqueID) + 1;
- } while (0 == id);
- return id;
+SkImage::SkImage(int width, int height, uint32_t uniqueID)
+ : fWidth(width)
+ , fHeight(height)
+ , fUniqueID(kNeedNewImageUniqueID == uniqueID ? SkNextID::ImageID() : uniqueID)
+{
+ SkASSERT(width > 0);
+ SkASSERT(height > 0);
}
const void* SkImage::peekPixels(SkImageInfo* info, size_t* rowBytes) const {
@@ -248,8 +247,8 @@
unrefCopy.reset(tex);
}
const SkImageInfo info = bm.info();
- return SkNEW_ARGS(SkImage_Gpu, (info.width(), info.height(), info.alphaType(),
- tex, 0, SkSurface::kNo_Budgeted));
+ return SkNEW_ARGS(SkImage_Gpu, (info.width(), info.height(), bm.getGenerationID(),
+ info.alphaType(), tex, 0, SkSurface::kNo_Budgeted));
}
#endif
diff --git a/src/image/SkImage_Base.h b/src/image/SkImage_Base.h
index b3661d9..37dcb40 100644
--- a/src/image/SkImage_Base.h
+++ b/src/image/SkImage_Base.h
@@ -11,14 +11,18 @@
#include "SkImage.h"
#include "SkSurface.h"
+enum {
+ kNeedNewImageUniqueID = 0
+};
+
static SkSurfaceProps copy_or_safe_defaults(const SkSurfaceProps* props) {
return props ? *props : SkSurfaceProps(0, kUnknown_SkPixelGeometry);
}
class SkImage_Base : public SkImage {
public:
- SkImage_Base(int width, int height, const SkSurfaceProps* props)
- : INHERITED(width, height)
+ SkImage_Base(int width, int height, uint32_t uniqueID, const SkSurfaceProps* props)
+ : INHERITED(width, height, uniqueID)
, fProps(copy_or_safe_defaults(props))
{}
diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp
index 31ae5a9..7ce6831 100644
--- a/src/image/SkImage_Gpu.cpp
+++ b/src/image/SkImage_Gpu.cpp
@@ -13,9 +13,9 @@
#include "SkGpuDevice.h"
-SkImage_Gpu::SkImage_Gpu(int w, int h, SkAlphaType at, GrTexture* tex,
+SkImage_Gpu::SkImage_Gpu(int w, int h, uint32_t uniqueID, SkAlphaType at, GrTexture* tex,
int sampleCountForNewSurfaces, SkSurface::Budgeted budgeted)
- : INHERITED(w, h, NULL)
+ : INHERITED(w, h, uniqueID, NULL)
, fTexture(SkRef(tex))
, fSampleCountForNewSurfaces(sampleCountForNewSurfaces)
, fAlphaType(at)
@@ -130,7 +130,8 @@
}
const SkSurface::Budgeted budgeted = SkSurface::kNo_Budgeted;
- return SkNEW_ARGS(SkImage_Gpu, (desc.fWidth, desc.fHeight, at, tex, 0, budgeted));
+ return SkNEW_ARGS(SkImage_Gpu,
+ (desc.fWidth, desc.fHeight, kNeedNewImageUniqueID, at, tex, 0, budgeted));
}
@@ -164,7 +165,8 @@
const SkSurface::Budgeted budgeted = SkSurface::kYes_Budgeted;
const int sampleCount = 0; // todo: make this an explicit parameter to newSurface()?
- return SkNEW_ARGS(SkImage_Gpu, (desc.fWidth, desc.fHeight, at, dst, sampleCount, budgeted));
+ return SkNEW_ARGS(SkImage_Gpu, (desc.fWidth, desc.fHeight, kNeedNewImageUniqueID,
+ at, dst, sampleCount, budgeted));
}
SkImage* SkImage::NewFromYUVTexturesCopy(GrContext* ctx , SkYUVColorSpace colorSpace,
@@ -238,8 +240,8 @@
GrDrawContext* drawContext = ctx->drawContext();
drawContext->drawRect(dst->asRenderTarget(), GrClip::WideOpen(), paint, SkMatrix::I(), rect);
ctx->flushSurfaceWrites(dst);
- return SkNEW_ARGS(SkImage_Gpu, (dstDesc.fWidth, dstDesc.fHeight, kOpaque_SkAlphaType, dst, 0,
- budgeted));
+ return SkNEW_ARGS(SkImage_Gpu, (dstDesc.fWidth, dstDesc.fHeight, kNeedNewImageUniqueID,
+ kOpaque_SkAlphaType, dst, 0, budgeted));
}
///////////////////////////////////////////////////////////////////////////////////////////////////
diff --git a/src/image/SkImage_Gpu.h b/src/image/SkImage_Gpu.h
index df26f82..4c7ebd6 100644
--- a/src/image/SkImage_Gpu.h
+++ b/src/image/SkImage_Gpu.h
@@ -23,8 +23,8 @@
* An "image" can be a subset/window into a larger texture, so we explicit take the
* width and height.
*/
- SkImage_Gpu(int w, int h, SkAlphaType, GrTexture*, int sampleCountForNewSurfaces,
- SkSurface::Budgeted);
+ SkImage_Gpu(int w, int h, uint32_t uniqueID, SkAlphaType, GrTexture*,
+ int sampleCountForNewSurfaces, SkSurface::Budgeted);
void applyBudgetDecision() const {
GrTexture* tex = this->getTexture();
diff --git a/src/image/SkImage_Raster.cpp b/src/image/SkImage_Raster.cpp
index bde4c34..7b5cf9b 100644
--- a/src/image/SkImage_Raster.cpp
+++ b/src/image/SkImage_Raster.cpp
@@ -82,7 +82,7 @@
bool onAsLegacyBitmap(SkBitmap*, LegacyBitmapMode) const override;
SkImage_Raster(const SkBitmap& bm, const SkSurfaceProps* props, bool lockPixels = false)
- : INHERITED(bm.width(), bm.height(), props)
+ : INHERITED(bm.width(), bm.height(), bm.getGenerationID(), props)
, fBitmap(bm) {
if (lockPixels) {
fBitmap.lockPixels();
@@ -91,7 +91,7 @@
}
private:
- SkImage_Raster() : INHERITED(0, 0, NULL) {
+ SkImage_Raster() : INHERITED(0, 0, fBitmap.getGenerationID(), NULL) {
fBitmap.setImmutable();
}
@@ -109,7 +109,7 @@
SkImage_Raster::SkImage_Raster(const Info& info, SkData* data, size_t rowBytes,
SkColorTable* ctable, const SkSurfaceProps* props)
- : INHERITED(info.width(), info.height(), props)
+ : INHERITED(info.width(), info.height(), kNeedNewImageUniqueID, props)
{
data->ref();
void* addr = const_cast<void*>(data->data());
@@ -121,7 +121,7 @@
SkImage_Raster::SkImage_Raster(const Info& info, SkPixelRef* pr, const SkIPoint& pixelRefOrigin,
size_t rowBytes, const SkSurfaceProps* props)
- : INHERITED(info.width(), info.height(), props)
+ : INHERITED(info.width(), info.height(), pr->getGenerationID(), props)
{
fBitmap.setInfo(info, rowBytes);
fBitmap.setPixelRef(pr, pixelRefOrigin);
diff --git a/src/image/SkSurface_Gpu.cpp b/src/image/SkSurface_Gpu.cpp
index 67f54d5..dbe82f8 100644
--- a/src/image/SkSurface_Gpu.cpp
+++ b/src/image/SkSurface_Gpu.cpp
@@ -83,7 +83,7 @@
GrTexture* tex = fDevice->accessRenderTarget()->asTexture();
if (tex) {
image = SkNEW_ARGS(SkImage_Gpu,
- (info.width(), info.height(), info.alphaType(),
+ (info.width(), info.height(), kNeedNewImageUniqueID, info.alphaType(),
tex, sampleCount, budgeted));
}
if (image) {
diff --git a/tests/ImageTest.cpp b/tests/ImageTest.cpp
index d93815b..5da8342 100644
--- a/tests/ImageTest.cpp
+++ b/tests/ImageTest.cpp
@@ -165,3 +165,89 @@
REPORTER_ASSERT(reporter, pixels[2] == green);
REPORTER_ASSERT(reporter, pixels[3] == red);
}
+
+/////////////////////////////////////////////////////////////////////////////////////////////////
+#include "SkImageGenerator.h"
+#include "SkData.h"
+
+const uint8_t tiny_png[] = {
+ 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d,
+ 0x49, 0x48, 0x44, 0x52, 0x00, 0x00, 0x00, 0x64, 0x00, 0x00, 0x00, 0x64,
+ 0x08, 0x06, 0x00, 0x00, 0x00, 0x70, 0xe2, 0x95, 0x54, 0x00, 0x00, 0x00,
+ 0x01, 0x73, 0x52, 0x47, 0x42, 0x00, 0xae, 0xce, 0x1c, 0xe9, 0x00, 0x00,
+ 0x01, 0x6b, 0x49, 0x44, 0x41, 0x54, 0x78, 0x01, 0xed, 0xd3, 0x41, 0x11,
+ 0x00, 0x20, 0x0c, 0xc4, 0x40, 0xc0, 0xbf, 0xe7, 0xc2, 0xa0, 0x22, 0x8f,
+ 0xad, 0x82, 0x4c, 0xd2, 0xdb, 0xf3, 0x6e, 0xb9, 0x8c, 0x81, 0x93, 0x21,
+ 0x01, 0xf2, 0x0d, 0x08, 0x12, 0x7b, 0x04, 0x41, 0x04, 0x89, 0x19, 0x88,
+ 0xe1, 0x58, 0x88, 0x20, 0x31, 0x03, 0x31, 0x1c, 0x0b, 0x11, 0x24, 0x66,
+ 0x20, 0x86, 0x63, 0x21, 0x82, 0xc4, 0x0c, 0xc4, 0x70, 0x2c, 0x44, 0x90,
+ 0x98, 0x81, 0x18, 0x8e, 0x85, 0x08, 0x12, 0x33, 0x10, 0xc3, 0xb1, 0x10,
+ 0x41, 0x62, 0x06, 0x62, 0x38, 0x16, 0x22, 0x48, 0xcc, 0x40, 0x0c, 0xc7,
+ 0x42, 0x04, 0x89, 0x19, 0x88, 0xe1, 0x58, 0x88, 0x20, 0x31, 0x03, 0x31,
+ 0x1c, 0x0b, 0x11, 0x24, 0x66, 0x20, 0x86, 0x63, 0x21, 0x82, 0xc4, 0x0c,
+ 0xc4, 0x70, 0x2c, 0x44, 0x90, 0x98, 0x81, 0x18, 0x8e, 0x85, 0x08, 0x12,
+ 0x33, 0x10, 0xc3, 0xb1, 0x10, 0x41, 0x62, 0x06, 0x62, 0x38, 0x16, 0x22,
+ 0x48, 0xcc, 0x40, 0x0c, 0xc7, 0x42, 0x04, 0x89, 0x19, 0x88, 0xe1, 0x58,
+ 0x88, 0x20, 0x31, 0x03, 0x31, 0x1c, 0x0b, 0x11, 0x24, 0x66, 0x20, 0x86,
+ 0x63, 0x21, 0x82, 0xc4, 0x0c, 0xc4, 0x70, 0x2c, 0x44, 0x90, 0x98, 0x81,
+ 0x18, 0x8e, 0x85, 0x08, 0x12, 0x33, 0x10, 0xc3, 0xb1, 0x10, 0x41, 0x62,
+ 0x06, 0x62, 0x38, 0x16, 0x22, 0x48, 0xcc, 0x40, 0x0c, 0xc7, 0x42, 0x04,
+ 0x89, 0x19, 0x88, 0xe1, 0x58, 0x88, 0x20, 0x31, 0x03, 0x31, 0x1c, 0x0b,
+ 0x11, 0x24, 0x66, 0x20, 0x86, 0x63, 0x21, 0x82, 0xc4, 0x0c, 0xc4, 0x70,
+ 0x2c, 0x44, 0x90, 0x98, 0x81, 0x18, 0x8e, 0x85, 0x08, 0x12, 0x33, 0x10,
+ 0xc3, 0xb1, 0x10, 0x41, 0x62, 0x06, 0x62, 0x38, 0x16, 0x22, 0x48, 0xcc,
+ 0x40, 0x0c, 0xc7, 0x42, 0x04, 0x89, 0x19, 0x88, 0xe1, 0x58, 0x88, 0x20,
+ 0x31, 0x03, 0x31, 0x1c, 0x0b, 0x11, 0x24, 0x66, 0x20, 0x86, 0x63, 0x21,
+ 0x82, 0xc4, 0x0c, 0xc4, 0x70, 0x2c, 0x44, 0x90, 0x98, 0x81, 0x18, 0x8e,
+ 0x85, 0x08, 0x12, 0x33, 0x10, 0xc3, 0xb1, 0x10, 0x41, 0x62, 0x06, 0x62,
+ 0x38, 0x16, 0x22, 0x48, 0xcc, 0x40, 0x0c, 0xc7, 0x42, 0x04, 0x89, 0x19,
+ 0x88, 0xe1, 0x58, 0x88, 0x20, 0x31, 0x03, 0x31, 0x1c, 0x0b, 0x11, 0x24,
+ 0x66, 0x20, 0x86, 0x63, 0x21, 0x82, 0xc4, 0x0c, 0xc4, 0x70, 0x2c, 0x44,
+ 0x90, 0x98, 0x81, 0x18, 0x8e, 0x85, 0x08, 0x12, 0x33, 0x10, 0xc3, 0xb1,
+ 0x10, 0x41, 0x62, 0x06, 0x62, 0x38, 0x16, 0x22, 0x48, 0xcc, 0x40, 0x0c,
+ 0xc7, 0x42, 0x62, 0x41, 0x2e, 0x08, 0x60, 0x04, 0xc4, 0x4c, 0x5d, 0x6e,
+ 0xf2, 0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4e, 0x44, 0xae, 0x42, 0x60,
+ 0x82
+};
+
+static void make_bitmap_lazy(SkBitmap* bm) {
+ SkAutoTUnref<SkData> data(SkData::NewWithoutCopy(tiny_png, sizeof(tiny_png)));
+ SkInstallDiscardablePixelRef(data, bm);
+}
+
+static void make_bitmap_mutable(SkBitmap* bm) {
+ bm->allocN32Pixels(10, 10);
+}
+
+static void make_bitmap_immutable(SkBitmap* bm) {
+ bm->allocN32Pixels(10, 10);
+ bm->setImmutable();
+}
+
+DEF_TEST(image_newfrombitmap, reporter) {
+ const struct {
+ void (*fMakeProc)(SkBitmap*);
+ bool fExpectPeekSuccess;
+ bool fExpectSharedID;
+ } rec[] = {
+ { make_bitmap_lazy, false, true },
+ { make_bitmap_mutable, true, false },
+ { make_bitmap_immutable, true, true },
+ };
+
+ for (size_t i = 0; i < SK_ARRAY_COUNT(rec); ++i) {
+ SkBitmap bm;
+ rec[i].fMakeProc(&bm);
+
+ SkAutoTUnref<SkImage> image(SkImage::NewFromBitmap(bm));
+ SkPixmap pmap;
+
+ const bool sharedID = (image->uniqueID() == bm.getGenerationID());
+ REPORTER_ASSERT(reporter, sharedID == rec[i].fExpectSharedID);
+
+#if 0 // TODO: fix so that peek will succeed in the immutable case
+ const bool peekSuccess = image->peekPixels(&pmap);
+ REPORTER_ASSERT(reporter, peekSuccess == rec[i].fExpectPeekSuccess);
+#endif
+ }
+}