Reland "Fix genID cloning bugs."

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 975 matching lines @@
     if (fPixelRef) {
         SkIRect subset;
         subset.setXYWH(fPixelRefOrigin.fX, fPixelRefOrigin.fY, fWidth, 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) {
-                    // TODO(scroggo): fix issue 1742
+                // If the result is an exact copy, clone the gen ID.
+                if (dst->pixelRef() && dst->pixelRef()->info() == fPixelRef->info()) {
                     dst->pixelRef()->cloneGenID(*fPixelRef);
                 }
                 return true;
             }
 
             // fall through to the raster case
             src = &tmpSrc;
         }
     }
 
     // we lock this now, since we may need its colortable
     SkAutoLockPixels srclock(*src);
     if (!src->readyToDraw()) {
         return false;
     }
 
+    // The only way to be readyToDraw is if fPixelRef is non NULL.
+    SkASSERT(fPixelRef != NULL);
+
     SkBitmap tmpDst;
     tmpDst.setConfig(dstConfig, src->width(), src->height(), 0,
                      src->alphaType());
 
     // allocate colortable if srcConfig == kIndex8_Config
     SkColorTable* ctable = (dstConfig == kIndex8_Config) ?
     new SkColorTable(*src->getColorTable()) : NULL;
     SkAutoUnref au(ctable);
     if (!tmpDst.allocPixels(alloc, ctable)) {
         return false;
     }
 
     if (!tmpDst.readyToDraw()) {
         // allocator/lock failed
         return false;
     }
 
+    // pixelRef must be non NULL or tmpDst.readyToDraw() would have
+    // returned false.
+    SkASSERT(tmpDst.pixelRef() != NULL);
+
     /* 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 (NULL != pixelRef && NULL != fPixelRef) {
-                // TODO(scroggo): fix issue 1742
+
+            // In order to reach this point, we know that the width, config and
+            // rowbytes of the SkPixelRefs are the same, but it is possible for
+            // the heights to differ, if this SkBitmap's height is a subset of
+            // fPixelRef. Only if the SkPixelRefs' heights match are we
+            // guaranteed that this is an exact copy, meaning we should clone
+            // the genID.
+            if (pixelRef->info().fHeight == fPixelRef->info().fHeight) {
+                // TODO: what to do if the two infos match, BUT
+                // fPixelRef is premul and pixelRef is opaque?
+                // skipping assert for now
+                // https://code.google.com/p/skia/issues/detail?id=2012
+//                SkASSERT(pixelRef->info() == fPixelRef->info());
+                SkASSERT(pixelRef->info().fWidth == fPixelRef->info().fWidth);
+                SkASSERT(pixelRef->info().fColorType == fPixelRef->info().fColorType);
                 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();
@@ skipping 34 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) {
-                // TODO(scroggo): fix issue 1742
+                // Since there is no subset to pass to deepCopy, and deepCopy
+                // succeeded, the new pixel ref must be identical.
+                SkASSERT(fPixelRef->info() == pixelRef->info());
                 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);
             dst->setPixelRef(pixelRef, fPixelRefOrigin)->unref();
             return true;
@@ skipping 532 matching lines @@
         if (NULL != uri) {
             str->appendf(" uri:\"%s\"", uri);
         } else {
             str->appendf(" pixelref:%p", pr);
         }
     }
 
     str->append(")");
 }
 #endif