Fixes for SkAlphaTypes.

src/core/SkBitmap.cpp

Index: src/core/SkBitmap.cpp
diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index 07f6f177deb16bc5df445381ae61d78e019fc404..922858f7a9a256ff7953302bb8b894c988ce7735 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -422,12 +422,15 @@ bool SkBitmap::allocPixels(const SkImageInfo& info, SkPixelRefFactory* factory,
         return reset_return_false(this);
     }
 
+    // setInfo may have corrected info (e.g. 565 is always opaque).
+    const SkImageInfo& correctedInfo = this->info();
+
     SkMallocPixelRef::PRFactory defaultFactory;
     if (NULL == factory) {
         factory = &defaultFactory;
     }
 
-    SkPixelRef* pr = factory->create(info, ctable);
+    SkPixelRef* pr = factory->create(correctedInfo, ctable);
     if (NULL == pr) {
         return reset_return_false(this);
     }
@@ -448,7 +451,11 @@ bool SkBitmap::installPixels(const SkImageInfo& info, void* pixels, size_t rb, S
         return false;
     }
 
-    SkPixelRef* pr = SkMallocPixelRef::NewWithProc(info, rb, ct, pixels, releaseProc, context);
+    // setInfo may have corrected info (e.g. 565 is always opaque).
+    const SkImageInfo& correctedInfo = this->info();
+
+    SkPixelRef* pr = SkMallocPixelRef::NewWithProc(correctedInfo, rb, ct, pixels, releaseProc,
+                                                   context);
     if (!pr) {
         this->reset();
         return false;
@@ -962,6 +969,11 @@ bool SkBitmap::copyTo(SkBitmap* dst, SkColorType dstColorType,
         subset.setXYWH(fPixelRefOrigin.fX, fPixelRefOrigin.fY,
                        fInfo.width(), fInfo.height());
         if (fPixelRef->readPixels(&tmpSrc, &subset)) {
+            if (fPixelRef->info().alphaType() == kUnpremul_SkAlphaType) {

[scroggo, 2014/06/04 20:28:27]

Brian,

I've cc'ed you mainly to make sure I'm doing the right thing in this case.

[bsalomon, 2014/06/04 20:39:32]

I suppose... I can't think of any better way to handle it.

[reed1, 2014/06/05 12:38:45]

Is this because pixelref::readPixels is poorly defined, or is it defined that it
must return 32bit-premul?

[scroggo, 2014/06/05 15:33:29]

I suppose the former. readPixels was written before we had any notion of
unpremul, and does not take it into account. The implementation specifically
creates a premultiplied bitmap.

+                // FIXME: The only meaningful implementation of readPixels
+                // (GrPixelRef) assumes premultiplied pixels.
+                return false;
+            }
             SkASSERT(tmpSrc.width() == this->width());
             SkASSERT(tmpSrc.height() == this->height());
 
@@ -1021,23 +1033,10 @@ bool SkBitmap::copyTo(SkBitmap* dst, SkColorType dstColorType,
     if (src->colorType() == dstColorType) {
         if (tmpDst.getSize() == src->getSize()) {
             memcpy(tmpDst.getPixels(), src->getPixels(), src->getSafeSize());
-            SkPixelRef* pixelRef = tmpDst.pixelRef();
-
-            // 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);
+
+            SkPixelRef* dstPixelRef = tmpDst.pixelRef();
+            if (dstPixelRef->info() == fPixelRef->info()) {
+                dstPixelRef->cloneGenID(*fPixelRef);
             }
         } else {
             const char* srcP = reinterpret_cast<const char*>(src->getPixels());
@@ -1052,6 +1051,10 @@ bool SkBitmap::copyTo(SkBitmap* dst, SkColorType dstColorType,
         }
     } else if (kARGB_4444_SkColorType == dstColorType
                && kN32_SkColorType == src->colorType()) {
+        if (src->alphaType() == kUnpremul_SkAlphaType) {
+            // Our method for converting to 4444 assumes premultiplied.
+            return false;
+        }
         SkASSERT(src->height() == tmpDst.height());
         SkASSERT(src->width() == tmpDst.width());
         for (int y = 0; y < src->height(); ++y) {
@@ -1064,6 +1067,11 @@ bool SkBitmap::copyTo(SkBitmap* dst, SkColorType dstColorType,
             }
         }
     } else {
+        if (tmpDst.alphaType() == kUnpremul_SkAlphaType) {
+            // We do not support drawing to unpremultiplied bitmaps.
+            return false;
+        }
+
         // Always clear the dest in case one of the blitters accesses it
         // TODO: switch the allocation of tmpDst to call sk_calloc_throw
         tmpDst.eraseColor(SK_ColorTRANSPARENT);