Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(129)

Unified Diff: src/core/SkBitmap.cpp

Issue 317053003: Fixes for SkAlphaTypes. (Closed) Base URL: https://skia.googlesource.com/skia.git@master
Patch Set: Created 6 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 doi
bsalomon 2014/06/04 20:39:32 I suppose... I can't think of any better way to ha
reed1 2014/06/05 12:38:45 Is this because pixelref::readPixels is poorly def
scroggo 2014/06/05 15:33:29 I suppose the former. readPixels was written befor
+ // 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);
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698