Revert of Make SkPixelRef::isLocked() debug-only, remove related dead code. (patchset...
authorreed <reed@chromium.org>
Fri, 20 Feb 2015 04:00:33 +0000 (20:00 -0800)
committerCommit bot <commit-bot@chromium.org>
Fri, 20 Feb 2015 04:00:33 +0000 (20:00 -0800)
Reason for revert:
Broke callers in chrome

../../skia/ext/platform_canvas_unittest.cc:421:56: error: no member named 'isLocked' in 'SkPixelRef'
  EXPECT_TRUE(platform_bitmap->GetBitmap().pixelRef()->isLocked());

Original issue's description:
> Make SkPixelRef::isLocked() debug-only, remove related dead code.
>
> DM's okay locally with no diffs, no failures.
>
> BUG=skia:
>
> Committed: https://skia.googlesource.com/skia/+/8e65712486c66108677a9b0a55ad3e7ca94db555

TBR=reed@google.com,mtklein@google.com,mtklein@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:

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

include/core/SkPixelRef.h
src/core/SkBitmapProcState.cpp
src/core/SkPixelRef.cpp
src/lazy/SkDiscardablePixelRef.cpp
src/lazy/SkDiscardablePixelRef.h

index f859642..2a5e7ec 100644 (file)
@@ -86,7 +86,11 @@ public:
         }
     };
 
-    SkDEBUGCODE(bool isLocked() const { return fLockCount > 0; })
+    /**
+     *  Returns true if the lockcount > 0
+     */
+    bool isLocked() const { return fLockCount > 0; }
+
     SkDEBUGCODE(int getLockCount() const { return fLockCount; })
 
     /**
@@ -193,6 +197,37 @@ public:
         return this->onRefEncodedData();
     }
 
+    /**
+     *  Experimental -- tells the caller if it is worth it to call decodeInto().
+     *  Just an optimization at this point, to avoid checking the cache first.
+     *  We may remove/change this call in the future.
+     */
+    bool implementsDecodeInto() {
+        return this->onImplementsDecodeInto();
+    }
+
+    /**
+     *  Return a decoded instance of this pixelRef in bitmap. If this cannot be
+     *  done, return false and the bitmap parameter is ignored/unchanged.
+     *
+     *  pow2 is the requeste power-of-two downscale that the caller needs. This
+     *  can be ignored, and the "original" size can be returned, but if the
+     *  underlying codec can efficiently return a smaller size, that should be
+     *  done. Some examples:
+     *
+     *  To request the "base" version (original scale), pass 0 for pow2
+     *  To request 1/2 scale version (1/2 width, 1/2 height), pass 1 for pow2
+     *  To request 1/4 scale version (1/4 width, 1/4 height), pass 2 for pow2
+     *  ...
+     *
+     *  If this returns true, then bitmap must be "locked" such that
+     *  bitmap->getPixels() will return the correct address.
+     */
+    bool decodeInto(int pow2, SkBitmap* bitmap) {
+        SkASSERT(pow2 >= 0);
+        return this->onDecodeInto(pow2, bitmap);
+    }
+
     /** Are we really wrapping a texture instead of a bitmap?
      */
     virtual GrTexture* getTexture() { return NULL; }
@@ -268,6 +303,11 @@ protected:
     /** Default impl returns true */
     virtual bool onLockPixelsAreWritable() const;
 
+    // returns false;
+    virtual bool onImplementsDecodeInto();
+    // returns false;
+    virtual bool onDecodeInto(int pow2, SkBitmap* bitmap);
+
     /**
      *  For pixelrefs that don't have access to their raw pixels, they may be
      *  able to make a copy of them (e.g. if the pixels are on the GPU).
index 335924f..81c14a2 100644 (file)
@@ -241,12 +241,43 @@ void SkBitmapProcState::processMediumRequest() {
     }
 }
 
+static bool get_locked_pixels(const SkBitmap& src, int pow2, SkBitmap* dst) {
+    SkPixelRef* pr = src.pixelRef();
+    if (pr && pr->decodeInto(pow2, dst)) {
+        return true;
+    }
+
+    /*
+     *  If decodeInto() fails, it is possibe that we have an old subclass that
+     *  does not, or cannot, implement that. In that case we fall back to the
+     *  older protocol of having the pixelRef handle the caching for us.
+     */
+    *dst = src;
+    dst->lockPixels();
+    return SkToBool(dst->getPixels());
+}
+
 bool SkBitmapProcState::lockBaseBitmap() {
-    // TODO(reed): use bitmap cache here?
-    fScaledBitmap = fOrigBitmap;
-    fScaledBitmap.lockPixels();
-    if (NULL == fScaledBitmap.getPixels()) {
-        return false;
+    SkPixelRef* pr = fOrigBitmap.pixelRef();
+
+    if (pr->isLocked() || !pr->implementsDecodeInto()) {
+        // fast-case, no need to look in our cache
+        fScaledBitmap = fOrigBitmap;
+        fScaledBitmap.lockPixels();
+        if (NULL == fScaledBitmap.getPixels()) {
+            return false;
+        }
+    } else {
+        if (!SkBitmapCache::Find(fOrigBitmap, 1, 1, &fScaledBitmap)) {
+            if (!get_locked_pixels(fOrigBitmap, 0, &fScaledBitmap)) {
+                return false;
+            }
+
+            // TODO: if fScaled comes back at a different width/height than fOrig,
+            // we need to update the matrix we are using to sample from this guy.
+
+            SkBitmapCache::Add(fOrigBitmap, 1, 1, fScaledBitmap);
+        }
     }
     fBitmap = &fScaledBitmap;
     return true;
@@ -356,7 +387,7 @@ bool SkBitmapProcState::chooseProcs(const SkMatrix& inv, const SkPaint& paint) {
     fShaderProc16 = NULL;
     fSampleProc32 = NULL;
     fSampleProc16 = NULL;
-
+    
     // recompute the triviality of the matrix here because we may have
     // changed it!
 
@@ -530,10 +561,10 @@ bool SkBitmapProcState::chooseScanlineProcs(bool trivialMatrix, bool clampClamp,
             fShaderProc32 = this->chooseShaderProc32();
         }
     }
-
+    
     // see if our platform has any accelerated overrides
     this->platformProcs();
-
+    
     return true;
 }
 
index c73c2f4..f562981 100644 (file)
@@ -197,6 +197,14 @@ bool SkPixelRef::onLockPixelsAreWritable() const {
     return true;
 }
 
+bool SkPixelRef::onImplementsDecodeInto() {
+    return false;
+}
+
+bool SkPixelRef::onDecodeInto(int pow2, SkBitmap* bitmap) {
+    return false;
+}
+
 uint32_t SkPixelRef::getGenerationID() const {
     if (0 == fGenerationID) {
         fGenerationID = SkNextPixelRefGenerationID();
index b6dec1b..5098858 100644 (file)
@@ -18,7 +18,6 @@ SkDiscardablePixelRef::SkDiscardablePixelRef(const SkImageInfo& info,
     , fDMFactory(fact)
     , fRowBytes(rowBytes)
     , fDiscardableMemory(NULL)
-    , fDiscardableMemoryIsLocked(false)
 {
     SkASSERT(fGenerator != NULL);
     SkASSERT(fRowBytes > 0);
@@ -29,9 +28,8 @@ SkDiscardablePixelRef::SkDiscardablePixelRef(const SkImageInfo& info,
 }
 
 SkDiscardablePixelRef::~SkDiscardablePixelRef() {
-    if (fDiscardableMemoryIsLocked) {
+    if (this->isLocked()) {
         fDiscardableMemory->unlock();
-        fDiscardableMemoryIsLocked = false;
     }
     SkDELETE(fDiscardableMemory);
     SkSafeUnref(fDMFactory);
@@ -41,7 +39,6 @@ SkDiscardablePixelRef::~SkDiscardablePixelRef() {
 bool SkDiscardablePixelRef::onNewLockPixels(LockRec* rec) {
     if (fDiscardableMemory != NULL) {
         if (fDiscardableMemory->lock()) {
-            fDiscardableMemoryIsLocked = true;
             rec->fPixels = fDiscardableMemory->data();
             rec->fColorTable = fCTable.get();
             rec->fRowBytes = fRowBytes;
@@ -49,20 +46,16 @@ bool SkDiscardablePixelRef::onNewLockPixels(LockRec* rec) {
         }
         SkDELETE(fDiscardableMemory);
         fDiscardableMemory = NULL;
-        fDiscardableMemoryIsLocked = false;
     }
 
     const size_t size = this->info().getSafeSize(fRowBytes);
 
     if (fDMFactory != NULL) {
         fDiscardableMemory = fDMFactory->create(size);
-        fDiscardableMemoryIsLocked = true;
     } else {
         fDiscardableMemory = SkDiscardableMemory::Create(size);
-        fDiscardableMemoryIsLocked = true;
     }
     if (NULL == fDiscardableMemory) {
-        fDiscardableMemoryIsLocked = false;
         return false;  // Memory allocation failed.
     }
 
@@ -79,7 +72,6 @@ bool SkDiscardablePixelRef::onNewLockPixels(LockRec* rec) {
             break;
         default:
             fDiscardableMemory->unlock();
-            fDiscardableMemoryIsLocked = false;
             SkDELETE(fDiscardableMemory);
             fDiscardableMemory = NULL;
             return false;
@@ -104,7 +96,6 @@ bool SkDiscardablePixelRef::onNewLockPixels(LockRec* rec) {
 
 void SkDiscardablePixelRef::onUnlockPixels() {
     fDiscardableMemory->unlock();
-    fDiscardableMemoryIsLocked = false;
 }
 
 bool SkInstallDiscardablePixelRef(SkImageGenerator* generator, SkBitmap* dst,
index 38ff2c4..448f0ab 100644 (file)
@@ -41,7 +41,6 @@ private:
     // PixelRef, since the SkBitmap doesn't expect them to change.
 
     SkDiscardableMemory* fDiscardableMemory;
-    bool                 fDiscardableMemoryIsLocked;
     SkAutoTUnref<SkColorTable> fCTable;
 
     /* Takes ownership of SkImageGenerator. */