Deferred image generator subsetting

src/core/SkImageCacherator.cpp

Index: src/core/SkImageCacherator.cpp
diff --git a/src/core/SkImageCacherator.cpp b/src/core/SkImageCacherator.cpp
index 75f8734e838004ac63b3bc393524b2223d69603f..c2bc266458067ad65667ac30bed1ae1870d48b3e 100644
--- a/src/core/SkImageCacherator.cpp
+++ b/src/core/SkImageCacherator.cpp
@@ -10,7 +10,6 @@
 #include "SkImage_Base.h"
 #include "SkImageCacherator.h"
 #include "SkMallocPixelRef.h"
-#include "SkMutex.h"
 #include "SkNextID.h"
 #include "SkPixelRef.h"
 #include "SkResourceCache.h"
@@ -33,24 +32,6 @@
 // see skbug.com/ 4971, 5128, ...
 //#define SK_SUPPORT_COMPRESSED_TEXTURES_IN_CACHERATOR
 
-// Ref-counted tuple(SkImageGenerator, SkMutex) which allows sharing of one generator
-// among several cacherators.
-class SkImageCacherator::SharedGenerator final : public SkNVRefCnt<SharedGenerator> {
-public:
-    static sk_sp<SharedGenerator> Make(SkImageGenerator* gen) {
-        return gen ? sk_sp<SharedGenerator>(new SharedGenerator(gen)) : nullptr;
-    }
-
-private:
-    explicit SharedGenerator(SkImageGenerator* gen) : fGenerator(gen) { SkASSERT(gen); }
-
-    friend class ScopedGenerator;
-
-    std::unique_ptr<SkImageGenerator> fGenerator;
-    SkMutex                           fMutex;
-};
-
-
 // Helper for exclusive access to a shared generator.
 class SkImageCacherator::ScopedGenerator {
 public:
@@ -73,22 +54,20 @@ private:
     SkAutoExclusive               fAutoAquire;
 };
 
-SkImageCacherator::Validator::Validator(SkImageGenerator* gen, const SkIRect* subset)
-    // We are required to take ownership of gen, regardless of whether we instantiate a cacherator
-    // or not.  On instantiation, the client is responsible for transferring ownership.
-    : fSharedGenerator(SkImageCacherator::SharedGenerator::Make(gen)) {
+SkImageCacherator::Validator::Validator(sk_sp<SharedGenerator> gen, const SkIRect* subset)
+    : fSharedGenerator(std::move(gen)) {
 
     if (!fSharedGenerator) {
         return;
     }
 
-    const SkImageInfo& info = gen->getInfo();
+    const SkImageInfo& info = fSharedGenerator->fGenerator->getInfo();

[reed1, 2016/11/04 17:55:29]

I presume its "safe" to access the generator w/o grabbing the shared mutex?

[f(malita), 2016/11/04 18:10:16]

Added comment, will follow up with cleanup CL.

     if (info.isEmpty()) {
         fSharedGenerator.reset();
         return;
     }
 
-    fUniqueID = gen->uniqueID();
+    fUniqueID = fSharedGenerator->fGenerator->uniqueID();
     const SkIRect bounds = SkIRect::MakeWH(info.width(), info.height());
     if (subset) {
         if (!bounds.contains(*subset)) {
@@ -107,11 +86,9 @@ SkImageCacherator::Validator::Validator(SkImageGenerator* gen, const SkIRect* su
     fOrigin = SkIPoint::Make(subset->x(), subset->y());
 }
 
-SkImageCacherator::Validator::~Validator() {}
-
 SkImageCacherator* SkImageCacherator::NewFromGenerator(SkImageGenerator* gen,
                                                        const SkIRect* subset) {
-    Validator validator(gen, subset);
+    Validator validator(SharedGenerator::Make(gen), subset);
 
     return validator ? new SkImageCacherator(&validator) : nullptr;
 }