cache private readback for gpu-images

Does not try to cache calls to readPixels at the moment:
- not triggered by drawing
- not clear if we want to perform any pixel transformations (that readPixels allows) on the GPU or CPU

Can consider that another time.

BUG=513695

Review URL: https://codereview.chromium.org/1262923003

diff --git a/include/core/SkPixelRef.h b/include/core/SkPixelRef.h
index 1dc01f79abb1f650dd0688bb84367d2c3f4d38fa..0ed3099126de3481c384996cc9c55cd0d0306891 100644 (file)
--- a/include/core/SkPixelRef.h
+++ b/include/core/SkPixelRef.h
@@ -379,6 +379,9 @@ private:
     friend class SkBitmap;  // only for cloneGenID
     void cloneGenID(const SkPixelRef&);
 
+    void setImmutableWithID(uint32_t genID);
+    friend class SkImage_Gpu;
+
     typedef SkRefCnt INHERITED;
 };
 

diff --git a/src/core/SkBitmapCache.cpp b/src/core/SkBitmapCache.cpp
index aabf87ad5ba7ee14919ab6fc901c185d35e87cf2..3f1feac7bb8c1ef28b8e5ae91f1f1bcc86b59d88 100644 (file)
--- a/src/core/SkBitmapCache.cpp
+++ b/src/core/SkBitmapCache.cpp
@@ -142,6 +142,20 @@ bool SkBitmapCache::Add(SkPixelRef* pr, const SkIRect& subset, const SkBitmap& r
     }
 }
 
+bool SkBitmapCache::Find(uint32_t genID, SkBitmap* result, SkResourceCache* localCache) {
+    BitmapKey key(genID, SK_Scalar1, SK_Scalar1, SkIRect::MakeEmpty());
+
+    return CHECK_LOCAL(localCache, find, Find, key, BitmapRec::Finder, result);
+}
+
+void SkBitmapCache::Add(uint32_t genID, const SkBitmap& result, SkResourceCache* localCache) {
+    SkASSERT(result.isImmutable());
+
+    BitmapRec* rec = SkNEW_ARGS(BitmapRec, (genID, 1, 1, SkIRect::MakeEmpty(), result));
+
+    CHECK_LOCAL(localCache, add, Add, rec);
+}
+
 //////////////////////////////////////////////////////////////////////////////////////////
 //////////////////////////////////////////////////////////////////////////////////////////
 

diff --git a/src/core/SkBitmapCache.h b/src/core/SkBitmapCache.h
index de50aabe1e4d7eb28e0cd1149a6cdd4d724fcdba..c75894208072a100362429199be1b6e047045bac 100644 (file)
--- a/src/core/SkBitmapCache.h
+++ b/src/core/SkBitmapCache.h
@@ -52,6 +52,10 @@ public:
      */
     static bool Add(SkPixelRef*, const SkIRect& subset, const SkBitmap& result,
                     SkResourceCache* localCache = NULL);
+
+    static bool Find(uint32_t genID, SkBitmap* result, SkResourceCache* localCache = NULL);
+    // todo: eliminate the need to specify ID, since it should == the bitmap's
+    static void Add(uint32_t genID, const SkBitmap&, SkResourceCache* localCache = NULL);
 };
 
 class SkMipMapCache {

diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp
index e79dfba27ceebf729ec553cd83bce750c7be6d74..2a46385c2416f545b88d3e2da7a0c25315bcf920 100644 (file)
--- a/src/core/SkPixelRef.cpp
+++ b/src/core/SkPixelRef.cpp
@@ -356,6 +356,18 @@ void SkPixelRef::changeAlphaType(SkAlphaType at) {
 void SkPixelRef::setImmutable() {
     fMutability = kImmutable;
 }
+
+void SkPixelRef::setImmutableWithID(uint32_t genID) {
+    /*
+     *  We are forcing the genID to match an external value. The caller must ensure that this
+     *  value does not conflict with other content.
+     *
+     *  One use is to force this pixelref's id to match an SkImage's id
+     */
+    fMutability = kImmutable;
+    fTaggedGenID.store(genID);
+}
+
 void SkPixelRef::setTemporarilyImmutable() {
     SkASSERT(fMutability != kImmutable);
     fMutability = kTemporarilyImmutable;

diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp
index c89c08de6e3d11cb20a9660f92f4f6a66b254383..d410de81b86f6baea7dd4ade9ae172f604978a95 100644 (file)
--- a/src/image/SkImage_Gpu.cpp
+++ b/src/image/SkImage_Gpu.cpp
@@ -5,13 +5,14 @@
  * found in the LICENSE file.
  */
 
+#include "SkBitmapCache.h"
 #include "SkImage_Gpu.h"
 #include "GrContext.h"
 #include "GrDrawContext.h"
 #include "effects/GrYUVtoRGBEffect.h"
 #include "SkCanvas.h"
 #include "SkGpuDevice.h"
-
+#include "SkPixelRef.h"
 
 SkImage_Gpu::SkImage_Gpu(int w, int h, uint32_t uniqueID, SkAlphaType at, GrTexture* tex,
                          int sampleCountForNewSurfaces, SkSurface::Budgeted budgeted)
@@ -22,6 +23,12 @@ SkImage_Gpu::SkImage_Gpu(int w, int h, uint32_t uniqueID, SkAlphaType at, GrText
     , fBudgeted(budgeted)
     {}
 
+SkImage_Gpu::~SkImage_Gpu() {
+    if (fAddedRasterVersionToCache.load()) {
+        SkNotifyBitmapGenIDIsStale(this->uniqueID());
+    }
+}
+
 SkSurface* SkImage_Gpu::onNewSurface(const SkImageInfo& info, const SkSurfaceProps& props) const {
     GrTexture* tex = this->getTexture();
     SkASSERT(tex);
@@ -49,6 +56,13 @@ SkShader* SkImage_Gpu::onNewShader(SkShader::TileMode tileX, SkShader::TileMode
 }
 
 bool SkImage_Gpu::getROPixels(SkBitmap* dst) const {
+    if (SkBitmapCache::Find(this->uniqueID(), dst)) {
+        SkASSERT(dst->getGenerationID() == this->uniqueID());
+        SkASSERT(dst->isImmutable());
+        SkASSERT(dst->getPixels());
+        return true;
+    }
+
     SkAlphaType at = this->isOpaque() ? kOpaque_SkAlphaType : kPremul_SkAlphaType;
     if (!dst->tryAllocPixels(SkImageInfo::MakeN32(this->width(), this->height(), at))) {
         return false;
@@ -57,6 +71,10 @@ bool SkImage_Gpu::getROPixels(SkBitmap* dst) const {
                               dst->getPixels(), dst->rowBytes())) {
         return false;
     }
+
+    dst->pixelRef()->setImmutableWithID(this->uniqueID());
+    SkBitmapCache::Add(this->uniqueID(), *dst);
+    fAddedRasterVersionToCache.store(true);
     return true;
 }
 

diff --git a/src/image/SkImage_Gpu.h b/src/image/SkImage_Gpu.h
index 4c7ebd6f7185aef5c4d36c5157ba18982c48943f..9481690714c9619b6b17369b549f6c1b580f21c5 100644 (file)
--- a/src/image/SkImage_Gpu.h
+++ b/src/image/SkImage_Gpu.h
@@ -8,6 +8,7 @@
 #ifndef SkImage_Gpu_DEFINED
 #define SkImage_Gpu_DEFINED
 
+#include "SkAtomics.h"
 #include "GrTexture.h"
 #include "GrGpuResourcePriv.h"
 #include "SkBitmap.h"
@@ -17,14 +18,13 @@
 
 class SkImage_Gpu : public SkImage_Base {
 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, uint32_t uniqueID, SkAlphaType, GrTexture*,
                 int sampleCountForNewSurfaces, SkSurface::Budgeted);
+    ~SkImage_Gpu() override;
 
     void applyBudgetDecision() const {
         GrTexture* tex = this->getTexture();
@@ -47,10 +47,12 @@ public:
                       int srcX, int srcY) const override;
 
 private:
-    SkAutoTUnref<GrTexture> fTexture;
-    const int               fSampleCountForNewSurfaces;   // 0 if we don't know
-    const SkAlphaType       fAlphaType;
-    SkSurface::Budgeted     fBudgeted;
+    SkAutoTUnref<GrTexture>     fTexture;
+    const int                   fSampleCountForNewSurfaces;   // 0 if we don't know
+    const SkAlphaType           fAlphaType;
+    const SkSurface::Budgeted   fBudgeted;
+    mutable SkAtomic<bool>      fAddedRasterVersionToCache;
+
 
     typedef SkImage_Base INHERITED;
 };

diff --git a/tests/ImageTest.cpp b/tests/ImageTest.cpp
index ba071bb1f75ca6f878eba49e728fcc3edcb454ea..f5e1abc0a4ad34a61eaf12e1982c4d4a75a80531 100644 (file)
--- a/tests/ImageTest.cpp
+++ b/tests/ImageTest.cpp
@@ -249,3 +249,65 @@ DEF_TEST(image_newfrombitmap, reporter) {
         REPORTER_ASSERT(reporter, peekSuccess == rec[i].fExpectPeekSuccess);
     }
 }
+
+///////////////////////////////////////////////////////////////////////////////////////////////////
+#if SK_SUPPORT_GPU
+
+static SkImage* make_gpu_image(GrContext* ctx, const SkImageInfo& info, SkColor color) {
+    const SkSurface::Budgeted budgeted = SkSurface::kNo_Budgeted;
+    SkAutoTUnref<SkSurface> surface(SkSurface::NewRenderTarget(ctx, budgeted, info, 0));
+    surface->getCanvas()->drawColor(color);
+    return surface->newImageSnapshot();
+}
+
+#include "SkBitmapCache.h"
+
+/*
+ *  This tests the caching (and preemptive purge) of the raster equivalent of a gpu-image.
+ *  We cache it for performance when drawing into a raster surface.
+ *
+ *  A cleaner test would know if each drawImage call triggered a read-back from the gpu,
+ *  but we don't have that facility (at the moment) so we use a little internal knowledge
+ *  of *how* the raster version is cached, and look for that.
+ */
+DEF_GPUTEST(SkImage_Gpu2Cpu, reporter, factory) {
+    GrContext* ctx = factory->get(GrContextFactory::kNative_GLContextType);
+    if (!ctx) {
+        REPORTER_ASSERT(reporter, false);
+        return;
+    }
+
+    const SkImageInfo info = SkImageInfo::MakeN32Premul(10, 10);
+    SkAutoTUnref<SkImage> image(make_gpu_image(ctx, info, SK_ColorRED));
+    const uint32_t uniqueID = image->uniqueID();
+
+    SkAutoTUnref<SkSurface> surface(SkSurface::NewRaster(info));
+
+    // now we can test drawing a gpu-backed image into a cpu-backed surface
+
+    {
+        SkBitmap cachedBitmap;
+        REPORTER_ASSERT(reporter, !SkBitmapCache::Find(uniqueID, &cachedBitmap));
+    }
+
+    surface->getCanvas()->drawImage(image, 0, 0);
+    {
+        SkBitmap cachedBitmap;
+        if (SkBitmapCache::Find(uniqueID, &cachedBitmap)) {
+            REPORTER_ASSERT(reporter, cachedBitmap.getGenerationID() == uniqueID);
+            REPORTER_ASSERT(reporter, cachedBitmap.isImmutable());
+            REPORTER_ASSERT(reporter, cachedBitmap.getPixels());
+        } else {
+            // unexpected, but not really a bug, since the cache is global and this test may be
+            // run w/ other threads competing for its budget.
+            SkDebugf("SkImage_Gpu2Cpu : cachedBitmap was already purged\n");
+        }
+    }
+
+    image.reset(nullptr);
+    {
+        SkBitmap cachedBitmap;
+        REPORTER_ASSERT(reporter, !SkBitmapCache::Find(uniqueID, &cachedBitmap));
+    }
+}
+#endif