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

Unified Diff: src/core/SkBitmap.cpp

Issue 377303002: add readPixels() to SkBitmap (Closed) Base URL: https://skia.googlesource.com/skia.git@master
Patch Set: Created 6 years, 5 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
Index: src/core/SkBitmap.cpp
diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index 651152e703bd5c2e0bf9c31b6f97f9d33abba532..b64c02913d13e5b4aa818828e906684f829639a9 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -821,11 +821,13 @@ bool SkBitmap::extractSubset(SkBitmap* result, const SkIRect& subset) const {
#include "SkPaint.h"
bool SkBitmap::canCopyTo(SkColorType dstColorType) const {
- if (this->colorType() == kUnknown_SkColorType) {
+ const SkColorType srcCT = this->colorType();
+
+ if (srcCT == kUnknown_SkColorType) {
return false;
}
- bool sameConfigs = (this->colorType() == dstColorType);
+ bool sameConfigs = (srcCT == dstColorType);
switch (dstColorType) {
case kAlpha_8_SkColorType:
case kRGB_565_SkColorType:
@@ -838,13 +840,65 @@ bool SkBitmap::canCopyTo(SkColorType dstColorType) const {
}
break;
case kARGB_4444_SkColorType:
- return sameConfigs || kN32_SkColorType == this->colorType();
+ return sameConfigs || kN32_SkColorType == srcCT || kIndex_8_SkColorType == srcCT;
default:
return false;
}
return true;
}
+#include "SkConfig8888.h"
+
+bool SkBitmap::readPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRB,
+ int x, int y) const {
+ if (kUnknown_SkColorType == dstInfo.colorType()) {
+ return false;
+ }
+ if (NULL == dstPixels || dstRB < dstInfo.minRowBytes()) {
+ return false;
+ }
+ if (0 == dstInfo.width() || 0 == dstInfo.height()) {
scroggo 2014/07/10 15:09:48 Arguably this could return true.
reed1 2014/07/10 15:32:16 Possibly. Below we return false if the srcR doesn'
+ return false;
+ }
+
+ SkIRect srcR = SkIRect::MakeXYWH(x, y, dstInfo.width(), dstInfo.height());
+ if (!srcR.intersect(0, 0, this->width(), this->height())) {
+ return false;
+ }
+
+ SkImageInfo info = dstInfo;
+ // the intersect may have shrunk info's logical size
+ info.fWidth = srcR.width();
+ info.fHeight = srcR.height();
scroggo 2014/07/10 15:09:48 You never use info after this (except to check byt
reed1 2014/07/10 15:32:16 Good catch.
+
+ // if x or y are negative, then we have to adjust pixels
+ if (x > 0) {
+ x = 0;
+ }
+ if (y > 0) {
+ y = 0;
+ }
+ // here x,y are either 0 or negative
+ dstPixels = ((char*)dstPixels - y * dstRB - x * info.bytesPerPixel());
+
+ //////////////
+
+ SkAutoLockPixels alp(*this);
+
+ // since we don't stop creating un-pixeled devices yet, check for no pixels here
+ if (NULL == this->getPixels()) {
+ return false;
+ }
+
+ SkImageInfo srcInfo = this->info();
+ srcInfo.fWidth = dstInfo.width();
scroggo 2014/07/10 15:09:48 shouldn't this be: srcInfo.fWidth = info.widt
reed1 2014/07/10 15:32:15 Agreed. I will make the suggested change above, an
+ srcInfo.fHeight = dstInfo.height();
+
+ const void* srcPixels = this->getAddr(srcR.x(), srcR.y());
+ return SkPixelInfo::CopyPixels(dstInfo, dstPixels, dstRB, srcInfo, srcPixels, this->rowBytes(),
+ this->getColorTable());
+}
+
bool SkBitmap::copyTo(SkBitmap* dst, SkColorType dstColorType,
Allocator* alloc) const {
if (!this->canCopyTo(dstColorType)) {
@@ -919,59 +973,18 @@ bool SkBitmap::copyTo(SkBitmap* dst, SkColorType dstColorType,
// 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());
+ if (!src->readPixels(tmpDst.info(), tmpDst.getPixels(), tmpDst.rowBytes(), 0, 0)) {
+ return false;
+ }
- 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();
- }
+ // Hack (for BitmapHeap) -- clone the pixelref genID even though we have a new pixelref.
+ // The old copyTo impl did this, so we continue it for now.
scroggo 2014/07/10 15:09:48 I don't agree that this is a hack. The point of th
reed1 2014/07/10 15:32:16 I don't agree with the hack either, but BitmapHeap
+ //
+ if (src->colorType() == dstColorType && tmpDst.getSize() == src->getSize()) {
+ SkPixelRef* dstPixelRef = tmpDst.pixelRef();
+ if (dstPixelRef->info() == fPixelRef->info()) {
+ dstPixelRef->cloneGenID(*fPixelRef);
}
- } 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);
}
dst->swap(tmpDst);

Powered by Google App Engine
This is Rietveld 408576698