Proactive SkPixelRef invalidation

src/core/SkBitmap.cpp

 
 /*
  * Copyright 2008 The Android Open Source Project
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 
 #include "SkBitmap.h"
@@ skipping 1038 matching lines @@
             subset.fRight = subset.fLeft + fWidth;
             subset.fBottom = subset.fTop + fHeight;
             if (fPixelRef->readPixels(&tmpSrc, &subset)) {
                 SkASSERT(tmpSrc.width() == this->width());
                 SkASSERT(tmpSrc.height() == this->height());
 
                 // did we get lucky and we can just return tmpSrc?
                 if (tmpSrc.config() == dstConfig && NULL == alloc) {
                     dst->swap(tmpSrc);
                     if (dst->pixelRef() && this->config() == dstConfig) {
-                        dst->pixelRef()->fGenerationID = fPixelRef->getGenerationID();
+                        dst->pixelRef()->cloneGenID(*fPixelRef);

[scroggo, 2013/10/22 21:04:02]

So this patch does not address the bug we discussed earlier that the ID gets
cloned even if we're looking at a subset? Can you add a FIXME that we know
there's a bug (or fix it)?

[bsalomon, 2013/10/23 13:52:57]

I'd prefer keeping a fix for that as a separate issue.

[mtklein, 2013/10/23 15:28:10]

Added skia:1742.

[scroggo, 2013/10/23 15:39:19]

I still think it would be nice to document this in the code. Thanks for adding
skia:1742; I think it should be referenced in a comment. The bug references line
1058 may be less meaningful in the future if the code moves.

[mtklein, 2013/10/23 16:07:10]

Done.

                     }
                     return true;
                 }
 
                 // fall through to the raster case
                 src = &tmpSrc;
             }
         }
     }
 
@@ skipping 19 matching lines @@
         // allocator/lock failed
         return false;
     }
 
     /* do memcpy for the same configs cases, else use drawing
     */
     if (src->config() == dstConfig) {
         if (tmpDst.getSize() == src->getSize()) {
             memcpy(tmpDst.getPixels(), src->getPixels(), src->getSafeSize());
             SkPixelRef* pixelRef = tmpDst.pixelRef();
-            if (pixelRef != NULL) {
-                pixelRef->fGenerationID = this->getGenerationID();
+            if (pixelRef != NULL && fPixelRef != NULL) {

[bsalomon, 2013/10/23 13:52:57]

ubernbit: NULL != pixelRef

[mtklein, 2013/10/23 15:28:10]

Done.

+                pixelRef->cloneGenID(*fPixelRef);
             }
         } else {
             const char* srcP = reinterpret_cast<const char*>(src->getPixels());
             char* dstP = reinterpret_cast<char*>(tmpDst.getPixels());
             // to be sure we don't read too much, only copy our logical pixels
             size_t bytesToCopy = tmpDst.width() * tmpDst.bytesPerPixel();
             for (int y = 0; y < tmpDst.height(); y++) {
                 memcpy(dstP, srcP, bytesToCopy);
                 srcP += src->rowBytes();
                 dstP += tmpDst.rowBytes();
@@ skipping 33 matching lines @@
         return false;
     }
 
     // If we have a PixelRef, and it supports deep copy, use it.
     // Currently supported only by texture-backed bitmaps.
     if (fPixelRef) {
         SkPixelRef* pixelRef = fPixelRef->deepCopy(dstConfig);
         if (pixelRef) {
             uint32_t rowBytes;
             if (dstConfig == fConfig) {
-                pixelRef->fGenerationID = fPixelRef->getGenerationID();
+                pixelRef->cloneGenID(*fPixelRef);
                 // Use the same rowBytes as the original.
                 rowBytes = fRowBytes;
             } else {
                 // With the new config, an appropriate fRowBytes will be computed by setConfig.
                 rowBytes = 0;
             }
             dst->setConfig(dstConfig, fWidth, fHeight, rowBytes);
 
             size_t pixelRefOffset;
             if (0 == fPixelRefOffset || dstConfig == fConfig) {
@@ skipping 531 matching lines @@
         if (NULL != uri) {
             str->appendf(" uri:\"%s\"", uri);
         } else {
             str->appendf(" pixelref:%p", pr);
         }
     }
 
     str->append(")");
 }
 #endif