aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
-rw-r--r--src/core/SkBitmap.cpp17
-rw-r--r--src/core/SkCanvas.cpp61
-rw-r--r--src/core/SkConfig8888.cpp2
-rw-r--r--src/core/SkDevice.cpp20
-rw-r--r--src/core/SkImageInfo.cpp39
-rw-r--r--src/core/SkImageInfoPriv.h5
-rw-r--r--src/core/SkPixmap.cpp34
-rw-r--r--src/core/SkWritePixelsRec.h41
-rw-r--r--src/gpu/SkGpuDevice.cpp16
-rw-r--r--src/image/SkImage.cpp6
-rw-r--r--src/image/SkImage_Gpu.cpp23
-rw-r--r--tests/BitmapCopyTest.cpp5
12 files changed, 149 insertions, 120 deletions
diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index 00b2f8e503..f54488d0a5 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -12,6 +12,7 @@
#include "SkData.h"
#include "SkFilterQuality.h"
#include "SkHalf.h"
+#include "SkImageInfoPriv.h"
#include "SkMallocPixelRef.h"
#include "SkMask.h"
#include "SkMath.h"
@@ -22,6 +23,7 @@
#include "SkTemplates.h"
#include "SkUnPreMultiply.h"
#include "SkWriteBuffer.h"
+#include "SkWritePixelsRec.h"
#include <string.h>
@@ -682,7 +684,6 @@ bool SkBitmap::canCopyTo(SkColorType dstCT) const {
case kBGRA_8888_SkColorType:
break;
case kGray_8_SkColorType:
- case kIndex_8_SkColorType:
if (!sameConfigs) {
return false;
}
@@ -714,13 +715,19 @@ bool SkBitmap::writePixels(const SkPixmap& src, int dstX, int dstY) {
return false;
}
- SkPixmap subset;
- if (!dst.pixmap().extractSubset(&subset,
- SkIRect::MakeXYWH(dstX, dstY, src.width(), src.height()))) {
+ if (!SkImageInfoValidConversion(fInfo, src.info())) {
+ return false;
+ }
+
+ SkWritePixelsRec rec(src.info(), src.addr(), src.rowBytes(), dstX, dstY);
+ if (!rec.trim(fInfo.width(), fInfo.height())) {
return false;
}
- return src.readPixels(subset);
+ void* dstPixels = this->getAddr(rec.fX, rec.fY);
+ const SkImageInfo dstInfo = fInfo.makeWH(rec.fInfo.width(), rec.fInfo.height());
+ return SkPixelInfo::CopyPixels(dstInfo, dstPixels, this->rowBytes(),
+ rec.fInfo, rec.fPixels, rec.fRowBytes, src.ctable());
}
bool SkBitmap::copyTo(SkBitmap* dst, SkColorType dstColorType, Allocator* alloc) const {
diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index d57b62fa99..7596c3a702 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -30,7 +30,6 @@
#include "SkRadialShadowMapShader.h"
#include "SkRasterClip.h"
#include "SkRasterHandleAllocator.h"
-#include "SkReadPixelsRec.h"
#include "SkRRect.h"
#include "SkShadowPaintFilterCanvas.h"
#include "SkShadowShader.h"
@@ -858,10 +857,6 @@ SkBaseDevice* SkCanvas::getTopDevice() const {
}
bool SkCanvas::readPixels(SkBitmap* bitmap, int x, int y) {
- if (kUnknown_SkColorType == bitmap->colorType()) {
- return false;
- }
-
bool weAllocated = false;
if (nullptr == bitmap->pixelRef()) {
if (!bitmap->tryAllocPixels()) {
@@ -908,15 +903,8 @@ bool SkCanvas::readPixels(const SkImageInfo& dstInfo, void* dstP, size_t rowByte
if (!device) {
return false;
}
- const SkISize size = this->getBaseLayerSize();
- SkReadPixelsRec rec(dstInfo, dstP, rowBytes, x, y);
- if (!rec.trim(size.width(), size.height())) {
- return false;
- }
-
- // The device can assert that the requested area is always contained in its bounds
- return device->readPixels(rec.fInfo, rec.fPixels, rec.fRowBytes, rec.fX, rec.fY);
+ return device->readPixels(dstInfo, dstP, rowBytes, x, y);
}
bool SkCanvas::writePixels(const SkBitmap& bitmap, int x, int y) {
@@ -928,49 +916,30 @@ bool SkCanvas::writePixels(const SkBitmap& bitmap, int x, int y) {
return false;
}
-bool SkCanvas::writePixels(const SkImageInfo& origInfo, const void* pixels, size_t rowBytes,
+bool SkCanvas::writePixels(const SkImageInfo& srcInfo, const void* pixels, size_t rowBytes,
int x, int y) {
- switch (origInfo.colorType()) {
- case kUnknown_SkColorType:
- case kIndex_8_SkColorType:
- return false;
- default:
- break;
- }
- if (nullptr == pixels || rowBytes < origInfo.minRowBytes()) {
- return false;
- }
-
- const SkISize size = this->getBaseLayerSize();
- SkIRect target = SkIRect::MakeXYWH(x, y, origInfo.width(), origInfo.height());
- if (!target.intersect(0, 0, size.width(), size.height())) {
- return false;
- }
-
SkBaseDevice* device = this->getDevice();
if (!device) {
return false;
}
- // the intersect may have shrunk info's logical size
- const SkImageInfo info = origInfo.makeWH(target.width(), target.height());
-
- // if x or y are negative, then we have to adjust pixels
- if (x > 0) {
- x = 0;
- }
- if (y > 0) {
- y = 0;
+ // This check gives us an early out and prevents generation ID churn on the surface.
+ // This is purely optional: it is a subset of the checks performed by SkWritePixelsRec.
+ SkIRect srcRect = SkIRect::MakeXYWH(x, y, srcInfo.width(), srcInfo.height());
+ if (!srcRect.intersect(0, 0, device->width(), device->height())) {
+ return false;
}
- // here x,y are either 0 or negative
- pixels = ((const char*)pixels - y * rowBytes - x * info.bytesPerPixel());
- // Tell our owning surface to bump its generation ID
- const bool completeOverwrite = info.dimensions() == size;
+ // Tell our owning surface to bump its generation ID.
+ const bool completeOverwrite =
+ srcRect.size() == SkISize::Make(device->width(), device->height());
this->predrawNotify(completeOverwrite);
- // The device can assert that the requested area is always contained in its bounds
- return device->writePixels(info, pixels, rowBytes, target.x(), target.y());
+ // This can still fail, most notably in the case of a invalid color type or alpha type
+ // conversion. We could pull those checks into this function and avoid the unnecessary
+ // generation ID bump. But then we would be performing those checks twice, since they
+ // are also necessary at the bitmap/pixmap entry points.
+ return device->writePixels(srcInfo, pixels, rowBytes, x, y);
}
//////////////////////////////////////////////////////////////////////////////
diff --git a/src/core/SkConfig8888.cpp b/src/core/SkConfig8888.cpp
index 98e5b7aced..d75ea27dc6 100644
--- a/src/core/SkConfig8888.cpp
+++ b/src/core/SkConfig8888.cpp
@@ -371,7 +371,7 @@ bool SkPixelInfo::CopyPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t
const int height = srcInfo.height();
// Do the easiest one first : both configs are equal
- if (srcInfo == dstInfo && kIndex_8_SkColorType != srcInfo.colorType()) {
+ if (srcInfo == dstInfo) {
size_t bytes = width * srcInfo.bytesPerPixel();
for (int y = 0; y < height; ++y) {
memcpy(dstPixels, srcPixels, bytes);
diff --git a/src/core/SkDevice.cpp b/src/core/SkDevice.cpp
index 7800c370c1..ab75dbc0bc 100644
--- a/src/core/SkDevice.cpp
+++ b/src/core/SkDevice.cpp
@@ -319,31 +319,11 @@ sk_sp<SkSpecialImage> SkBaseDevice::snapSpecial() { return nullptr; }
///////////////////////////////////////////////////////////////////////////////////////////////////
bool SkBaseDevice::readPixels(const SkImageInfo& info, void* dstP, size_t rowBytes, int x, int y) {
-#ifdef SK_DEBUG
- SkASSERT(info.width() > 0 && info.height() > 0);
- SkASSERT(dstP);
- SkASSERT(rowBytes >= info.minRowBytes());
- SkASSERT(x >= 0 && y >= 0);
-
- const SkImageInfo& srcInfo = this->imageInfo();
- SkASSERT(x + info.width() <= srcInfo.width());
- SkASSERT(y + info.height() <= srcInfo.height());
-#endif
return this->onReadPixels(info, dstP, rowBytes, x, y);
}
bool SkBaseDevice::writePixels(const SkImageInfo& info, const void* pixels, size_t rowBytes,
int x, int y) {
-#ifdef SK_DEBUG
- SkASSERT(info.width() > 0 && info.height() > 0);
- SkASSERT(pixels);
- SkASSERT(rowBytes >= info.minRowBytes());
- SkASSERT(x >= 0 && y >= 0);
-
- const SkImageInfo& dstInfo = this->imageInfo();
- SkASSERT(x + info.width() <= dstInfo.width());
- SkASSERT(y + info.height() <= dstInfo.height());
-#endif
return this->onWritePixels(info, pixels, rowBytes, x, y);
}
diff --git a/src/core/SkImageInfo.cpp b/src/core/SkImageInfo.cpp
index 0453c7b38f..1b7c09b2f9 100644
--- a/src/core/SkImageInfo.cpp
+++ b/src/core/SkImageInfo.cpp
@@ -98,9 +98,6 @@ bool SkColorTypeValidateAlphaType(SkColorType colorType, SkAlphaType alphaType,
#include "SkReadPixelsRec.h"
bool SkReadPixelsRec::trim(int srcWidth, int srcHeight) {
- if (kIndex_8_SkColorType == fInfo.colorType()) {
- return false;
- }
if (nullptr == fPixels || fRowBytes < fInfo.minRowBytes()) {
return false;
}
@@ -131,3 +128,39 @@ bool SkReadPixelsRec::trim(int srcWidth, int srcHeight) {
return true;
}
+
+///////////////////////////////////////////////////////////////////////////////////////////////////
+
+#include "SkWritePixelsRec.h"
+
+bool SkWritePixelsRec::trim(int dstWidth, int dstHeight) {
+ if (nullptr == fPixels || fRowBytes < fInfo.minRowBytes()) {
+ return false;
+ }
+ if (0 >= fInfo.width() || 0 >= fInfo.height()) {
+ return false;
+ }
+
+ int x = fX;
+ int y = fY;
+ SkIRect dstR = SkIRect::MakeXYWH(x, y, fInfo.width(), fInfo.height());
+ if (!dstR.intersect(0, 0, dstWidth, dstHeight)) {
+ return false;
+ }
+
+ // 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
+ fPixels = ((const char*)fPixels - y * fRowBytes - x * fInfo.bytesPerPixel());
+ // the intersect may have shrunk info's logical size
+ fInfo = fInfo.makeWH(dstR.width(), dstR.height());
+ fX = dstR.x();
+ fY = dstR.y();
+
+ return true;
+}
diff --git a/src/core/SkImageInfoPriv.h b/src/core/SkImageInfoPriv.h
index 151de82632..f04acc5484 100644
--- a/src/core/SkImageInfoPriv.h
+++ b/src/core/SkImageInfoPriv.h
@@ -39,8 +39,7 @@ static inline bool SkImageInfoIsValid(const SkImageInfo& info) {
/**
* Returns true if Skia has defined a pixel conversion from the |src| to the |dst|.
* Returns false otherwise. Some discussion of false cases:
- * We will not convert to kIndex8 unless |src| is kIndex8. This is possible only
- * in some cases and likley inefficient.
+ * We will not convert to kIndex8. Do we overwrite the input color table?
* We do not convert to kGray8 when the |src| is not kGray8. We may add this
* feature - it just requires some work to convert to luminance while handling color
* spaces correctly. Currently no one is asking for this.
@@ -57,7 +56,7 @@ static inline bool SkImageInfoValidConversion(const SkImageInfo& dst, const SkIm
return false;
}
- if (kIndex_8_SkColorType == dst.colorType() && kIndex_8_SkColorType != src.colorType()) {
+ if (kIndex_8_SkColorType == dst.colorType()) {
return false;
}
diff --git a/src/core/SkPixmap.cpp b/src/core/SkPixmap.cpp
index 1e24b93a60..c0889cfd84 100644
--- a/src/core/SkPixmap.cpp
+++ b/src/core/SkPixmap.cpp
@@ -16,6 +16,7 @@
#include "SkNx.h"
#include "SkPM4f.h"
#include "SkPixmap.h"
+#include "SkReadPixelsRec.h"
#include "SkSurface.h"
#include "SkUtils.h"
@@ -83,37 +84,20 @@ bool SkPixmap::extractSubset(SkPixmap* result, const SkIRect& subset) const {
return true;
}
-bool SkPixmap::readPixels(const SkImageInfo& requestedDstInfo, void* dstPixels, size_t dstRB,
- int x, int y) const {
- if (!SkImageInfoValidConversion(requestedDstInfo, fInfo)) {
+bool SkPixmap::readPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRB, int x, int y)
+const {
+ if (!SkImageInfoValidConversion(dstInfo, fInfo)) {
return false;
}
- if (nullptr == dstPixels || dstRB < requestedDstInfo.minRowBytes()) {
+ SkReadPixelsRec rec(dstInfo, dstPixels, dstRB, x, y);
+ if (!rec.trim(fInfo.width(), fInfo.height())) {
return false;
}
- SkIRect srcR = SkIRect::MakeXYWH(x, y, requestedDstInfo.width(), requestedDstInfo.height());
- if (!srcR.intersect(0, 0, this->width(), this->height())) {
- return false;
- }
-
- // the intersect may have shrunk info's logical size
- const SkImageInfo dstInfo = requestedDstInfo.makeWH(srcR.width(), srcR.height());
-
- // 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 * dstInfo.bytesPerPixel());
-
- const SkImageInfo srcInfo = this->info().makeWH(dstInfo.width(), dstInfo.height());
- const void* srcPixels = this->addr(srcR.x(), srcR.y());
- return SkPixelInfo::CopyPixels(dstInfo, dstPixels, dstRB,
+ const void* srcPixels = this->addr(rec.fX, rec.fY);
+ const SkImageInfo srcInfo = fInfo.makeWH(rec.fInfo.width(), rec.fInfo.height());
+ return SkPixelInfo::CopyPixels(rec.fInfo, rec.fPixels, rec.fRowBytes,
srcInfo, srcPixels, this->rowBytes(), this->ctable());
}
diff --git a/src/core/SkWritePixelsRec.h b/src/core/SkWritePixelsRec.h
new file mode 100644
index 0000000000..652a13a822
--- /dev/null
+++ b/src/core/SkWritePixelsRec.h
@@ -0,0 +1,41 @@
+/*
+ * Copyright 2017 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef SkWritePixelsRec_DEFINED
+#define SkWritePixelsRec_DEFINED
+
+#include "SkImageInfo.h"
+
+/**
+ * Helper class to package and trim the parameters passed to writePixels()
+ */
+struct SkWritePixelsRec {
+ SkWritePixelsRec(const SkImageInfo& info, const void* pixels, size_t rowBytes, int x, int y)
+ : fPixels(pixels)
+ , fRowBytes(rowBytes)
+ , fInfo(info)
+ , fX(x)
+ , fY(y)
+ {}
+
+ const void* fPixels;
+ size_t fRowBytes;
+ SkImageInfo fInfo;
+ int fX;
+ int fY;
+
+ /*
+ * On true, may have modified its fields (except fRowBytes) to make it a legal subset
+ * of the specified dst width/height.
+ *
+ * On false, leaves self unchanged, but indicates that it does not overlap dst, or
+ * is not valid (e.g. bad fInfo) for writePixels().
+ */
+ bool trim(int dstWidth, int dstHeight);
+};
+
+#endif
diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp
index 85eb671206..c5ade9e7b1 100644
--- a/src/gpu/SkGpuDevice.cpp
+++ b/src/gpu/SkGpuDevice.cpp
@@ -35,6 +35,7 @@
#include "SkPictureData.h"
#include "SkRRect.h"
#include "SkRasterClip.h"
+#include "SkReadPixelsRec.h"
#include "SkRecord.h"
#include "SkSpecialImage.h"
#include "SkStroke.h"
@@ -43,6 +44,7 @@
#include "SkTLazy.h"
#include "SkUtils.h"
#include "SkVertState.h"
+#include "SkWritePixelsRec.h"
#include "effects/GrBicubicEffect.h"
#include "effects/GrSimpleTextureEffect.h"
#include "effects/GrTextureDomain.h"
@@ -199,7 +201,12 @@ bool SkGpuDevice::onReadPixels(const SkImageInfo& dstInfo, void* dstPixels, size
return false;
}
- return fRenderTargetContext->readPixels(dstInfo, dstPixels, dstRowBytes, x, y);
+ SkReadPixelsRec rec(dstInfo, dstPixels, dstRowBytes, x, y);
+ if (!rec.trim(this->width(), this->height())) {
+ return false;
+ }
+
+ return fRenderTargetContext->readPixels(rec.fInfo, rec.fPixels, rec.fRowBytes, rec.fX, rec.fY);
}
bool SkGpuDevice::onWritePixels(const SkImageInfo& srcInfo, const void* srcPixels,
@@ -210,7 +217,12 @@ bool SkGpuDevice::onWritePixels(const SkImageInfo& srcInfo, const void* srcPixel
return false;
}
- return fRenderTargetContext->writePixels(srcInfo, srcPixels, srcRowBytes, x, y);
+ SkWritePixelsRec rec(srcInfo, srcPixels, srcRowBytes, x, y);
+ if (!rec.trim(this->width(), this->height())) {
+ return false;
+ }
+
+ return fRenderTargetContext->writePixels(rec.fInfo, rec.fPixels, rec.fRowBytes, rec.fX, rec.fY);
}
bool SkGpuDevice::onAccessPixels(SkPixmap* pmap) {
diff --git a/src/image/SkImage.cpp b/src/image/SkImage.cpp
index 137f9166ba..8efe91a706 100644
--- a/src/image/SkImage.cpp
+++ b/src/image/SkImage.cpp
@@ -51,11 +51,7 @@ bool SkImage::peekPixels(SkPixmap* pm) const {
bool SkImage::readPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRowBytes,
int srcX, int srcY, CachingHint chint) const {
- SkReadPixelsRec rec(dstInfo, dstPixels, dstRowBytes, srcX, srcY);
- if (!rec.trim(this->width(), this->height())) {
- return false;
- }
- return as_IB(this)->onReadPixels(rec.fInfo, rec.fPixels, rec.fRowBytes, rec.fX, rec.fY, chint);
+ return as_IB(this)->onReadPixels(dstInfo, dstPixels, dstRowBytes, srcX, srcY, chint);
}
bool SkImage::scalePixels(const SkPixmap& dst, SkFilterQuality quality, CachingHint chint) const {
diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp
index 7247ae36fa..fb93350039 100644
--- a/src/image/SkImage_Gpu.cpp
+++ b/src/image/SkImage_Gpu.cpp
@@ -27,6 +27,7 @@
#include "SkImageInfoPriv.h"
#include "SkMipMap.h"
#include "SkPixelRef.h"
+#include "SkReadPixelsRec.h"
SkImage_Gpu::SkImage_Gpu(int w, int h, uint32_t uniqueID, SkAlphaType at, sk_sp<GrTexture> tex,
sk_sp<SkColorSpace> colorSpace, SkBudgeted budgeted)
@@ -128,20 +129,26 @@ static void apply_premul(const SkImageInfo& info, void* pixels, size_t rowBytes)
}
}
-bool SkImage_Gpu::onReadPixels(const SkImageInfo& info, void* pixels, size_t rowBytes,
+bool SkImage_Gpu::onReadPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRB,
int srcX, int srcY, CachingHint) const {
- if (!SkImageInfoValidConversion(info, this->onImageInfo())) {
+ if (!SkImageInfoValidConversion(dstInfo, this->onImageInfo())) {
return false;
}
- GrPixelConfig config = SkImageInfo2GrPixelConfig(info, *fTexture->getContext()->caps());
+ SkReadPixelsRec rec(dstInfo, dstPixels, dstRB, srcX, srcY);
+ if (!rec.trim(this->width(), this->height())) {
+ return false;
+ }
+
+ GrPixelConfig config = SkImageInfo2GrPixelConfig(rec.fInfo, *fTexture->getContext()->caps());
uint32_t flags = 0;
- if (kUnpremul_SkAlphaType == info.alphaType() && kPremul_SkAlphaType == fAlphaType) {
+ if (kUnpremul_SkAlphaType == rec.fInfo.alphaType() && kPremul_SkAlphaType == fAlphaType) {
// let the GPU perform this transformation for us
flags = GrContext::kUnpremul_PixelOpsFlag;
}
- if (!fTexture->readPixels(fColorSpace.get(), srcX, srcY, info.width(), info.height(), config,
- info.colorSpace(), pixels, rowBytes, flags)) {
+ if (!fTexture->readPixels(fColorSpace.get(), rec.fX, rec.fY, rec.fInfo.width(),
+ rec.fInfo.height(), config, rec.fInfo.colorSpace(), rec.fPixels,
+ rec.fRowBytes, flags)) {
return false;
}
// do we have to manually fix-up the alpha channel?
@@ -152,8 +159,8 @@ bool SkImage_Gpu::onReadPixels(const SkImageInfo& info, void* pixels, size_t row
//
// Should this be handled by Ganesh? todo:?
//
- if (kPremul_SkAlphaType == info.alphaType() && kUnpremul_SkAlphaType == fAlphaType) {
- apply_premul(info, pixels, rowBytes);
+ if (kPremul_SkAlphaType == rec.fInfo.alphaType() && kUnpremul_SkAlphaType == fAlphaType) {
+ apply_premul(rec.fInfo, rec.fPixels, rec.fRowBytes);
}
return true;
}
diff --git a/tests/BitmapCopyTest.cpp b/tests/BitmapCopyTest.cpp
index ab0ec30e16..d69f7df76e 100644
--- a/tests/BitmapCopyTest.cpp
+++ b/tests/BitmapCopyTest.cpp
@@ -184,7 +184,7 @@ static void writeCoordPixels(SkBitmap& bm, const Coordinates& coords) {
static const Pair gPairs[] = {
{ kUnknown_SkColorType, "000000" },
{ kAlpha_8_SkColorType, "010000" },
- { kIndex_8_SkColorType, "011111" },
+ { kIndex_8_SkColorType, "010111" },
{ kRGB_565_SkColorType, "010101" },
{ kARGB_4444_SkColorType, "010111" },
{ kN32_SkColorType, "010111" },
@@ -235,7 +235,8 @@ DEF_TEST(BitmapCopy_extractSubset, reporter) {
if (!success) {
// Skip checking that success matches fValid, which is redundant
// with the code below.
- REPORTER_ASSERT(reporter, gPairs[i].fColorType != gPairs[j].fColorType);
+ REPORTER_ASSERT(reporter, kIndex_8_SkColorType == gPairs[i].fColorType ||
+ gPairs[i].fColorType != gPairs[j].fColorType);
continue;
}