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

include/core/SkImage.h

@@ -283,22 +283,13 @@ public:
     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;
 };
 

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

src/core/SkPixelRef.cpp

@@ -59,15 +59,16 @@ static SkBaseMutex* get_default_mutex() {
 }
 
 ///////////////////////////////////////////////////////////////////////////////
+#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 @@ bool SkPixelRef::onLockPixelsAreWritable() const {
 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 {

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 @@ SkImage* SkImage::NewFromBitmap(const SkBitmap& bm) {
             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
 

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))
     {}
 

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 @@ static SkImage* new_wrapped_texture_common(GrContext* ctx, const GrBackendTextur
     }
 
     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 @@ SkImage* SkImage::NewFromTextureCopy(GrContext* ctx, const GrBackendTextureDesc&
 
     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 @@ SkImage* SkImage::NewFromYUVTexturesCopy(GrContext* ctx , SkYUVColorSpace colorS
     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));
 }
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////

src/image/SkImage_Gpu.h

@@ -23,8 +23,8 @@ public:
      *  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();

src/image/SkImage_Raster.cpp

@@ -82,7 +82,7 @@ public:
     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 @@ public:
     }
 
 private:
-    SkImage_Raster() : INHERITED(0, 0, NULL) {
+    SkImage_Raster() : INHERITED(0, 0, fBitmap.getGenerationID(), NULL) {
         fBitmap.setImmutable();
     }
 
@@ -109,7 +109,7 @@ static void release_data(void* addr, void* context) {
 
 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, SkData* data, size_t rowBytes,
 
 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);

src/image/SkSurface_Gpu.cpp

@@ -83,7 +83,7 @@ SkImage* SkSurface_Gpu::onNewImageSnapshot(Budgeted budgeted) {
     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) {

tests/ImageTest.cpp

@@ -165,3 +165,89 @@ DEF_TEST(Image_RetainSnapshot, reporter) {
     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
+    }
+}