Fixes for SkAlphaTypes.

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 395 matching lines @@
     HeapAllocator stdalloc;
 
     if (NULL == allocator) {
         allocator = &stdalloc;
     }
     return allocator->allocPixelRef(this, ctable);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 bool SkBitmap::allocPixels(const SkImageInfo& info, SkPixelRefFactory* factory,

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

perhaps rename this to origInfo or requestedInfo, to make your point even
clearer.

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

Done.

                            SkColorTable* ctable) {
     if (kIndex_8_SkColorType == info.fColorType && NULL == ctable) {
         return reset_return_false(this);
     }
     if (!this->setInfo(info)) {
         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);
     }
     this->setPixelRef(pr)->unref();
 
     // TODO: lockPixels could/should return bool or void*/NULL
     this->lockPixels();
     if (NULL == this->getPixels()) {
         return reset_return_false(this);
     }
     return true;
 }
 
 bool SkBitmap::installPixels(const SkImageInfo& info, void* pixels, size_t rb, SkColorTable* ct,

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

ditto about renaming this too.

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

Done.

                              void (*releaseProc)(void* addr, void* context), void* context) {
     if (!this->setInfo(info, rb)) {
         this->reset();
         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;
     }
 
     this->setPixelRef(pr)->unref();
 
     // since we're already allocated, we lockPixels right away
     this->lockPixels();
     SkDEBUGCODE(this->validate();)
@@ skipping 493 matching lines @@
 
     // if we have a texture, first get those pixels
     SkBitmap tmpSrc;
     const SkBitmap* src = this;
 
     if (fPixelRef) {
         SkIRect subset;
         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());
 
             // did we get lucky and we can just return tmpSrc?
             if (tmpSrc.colorType() == dstColorType && NULL == alloc) {
                 dst->swap(tmpSrc);
                 // If the result is an exact copy, clone the gen ID.
                 if (dst->pixelRef() && dst->pixelRef()->info() == fPixelRef->info()) {
                     dst->pixelRef()->cloneGenID(*fPixelRef);
                 }
@@ skipping 39 matching lines @@
 
     // 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->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());
             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();
             }
         }
     } 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) {
             SkPMColor16* SK_RESTRICT dstRow = (SkPMColor16*) tmpDst.getAddr16(0, y);
             SkPMColor* SK_RESTRICT srcRow = (SkPMColor*) src->getAddr32(0, y);
             DITHER_4444_SCAN(y);
             for (int x = 0; x < src->width(); ++x) {
                 dstRow[x] = SkDitherARGB32To4444(srcRow[x],
                                                  DITHER_VALUE(x));
             }
         }
     } 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);
 
         SkCanvas canvas(tmpDst);
         SkPaint  paint;
 
         paint.setDither(true);
         canvas.drawBitmap(*src, 0, 0, &paint);
     }
@@ skipping 399 matching lines @@
 ///////////////////////////////////////////////////////////////////////////////
 
 #ifdef SK_DEBUG
 void SkImageInfo::validate() const {
     SkASSERT(fWidth >= 0);
     SkASSERT(fHeight >= 0);
     SkASSERT(SkColorTypeIsValid(fColorType));
     SkASSERT(SkAlphaTypeIsValid(fAlphaType));
 }
 #endif