Consistently fail readPixels when rowbytes not a multiple of bpp
Previously this would report success but may have rounded the
rowbytes down to a multiple of bpp prior to writing the dst.
On GPU it could trigger an assert in a debug build.
Bug: chromium:1163061
Change-Id: I19709f4cdb71139732998a4dd2e14476099f0ba8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/363782
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Mike Reed <reed@google.com>
diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index b870215..cd72d46 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -484,7 +484,10 @@
void* dstPixels = this->getAddr(rec.fX, rec.fY);
const SkImageInfo dstInfo = this->info().makeDimensions(rec.fInfo.dimensions());
- SkConvertPixels(dstInfo, dstPixels, this->rowBytes(), rec.fInfo, rec.fPixels, rec.fRowBytes);
+ if (!SkConvertPixels(dstInfo, dstPixels, this->rowBytes(),
+ rec.fInfo, rec.fPixels, rec.fRowBytes)) {
+ return false;
+ }
this->notifyPixelsChanged();
return true;
}
@@ -503,9 +506,8 @@
}
return false;
}
- SkConvertPixels(SkImageInfo::MakeA8(pmap.width(), pmap.height()), alpha, alphaRowBytes,
- pmap.info(), pmap.addr(), pmap.rowBytes());
- return true;
+ return SkConvertPixels(SkImageInfo::MakeA8(pmap.width(), pmap.height()), alpha, alphaRowBytes,
+ pmap.info(), pmap.addr(), pmap.rowBytes());
}
#include "include/core/SkMaskFilter.h"
diff --git a/src/core/SkConvertPixels.cpp b/src/core/SkConvertPixels.cpp
index c3dd1d2..42b2aa0 100644
--- a/src/core/SkConvertPixels.cpp
+++ b/src/core/SkConvertPixels.cpp
@@ -202,12 +202,11 @@
}
// Default: Use the pipeline.
-static void convert_with_pipeline(const SkImageInfo& dstInfo, void* dstRow, size_t dstRB,
- const SkImageInfo& srcInfo, const void* srcRow, size_t srcRB,
+static void convert_with_pipeline(const SkImageInfo& dstInfo, void* dstRow, int dstStride,
+ const SkImageInfo& srcInfo, const void* srcRow, int srcStride,
const SkColorSpaceXformSteps& steps) {
-
- SkRasterPipeline_MemoryCtx src = { (void*)srcRow, (int)(srcRB / srcInfo.bytesPerPixel()) },
- dst = { (void*)dstRow, (int)(dstRB / dstInfo.bytesPerPixel()) };
+ SkRasterPipeline_MemoryCtx src = { (void*)srcRow, srcStride },
+ dst = { (void*)dstRow, dstStride };
SkRasterPipeline_<256> pipeline;
pipeline.append_load(srcInfo.colorType(), &src);
@@ -219,18 +218,26 @@
pipeline.run(0,0, srcInfo.width(), srcInfo.height());
}
-void SkConvertPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRB,
+bool SkConvertPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRB,
const SkImageInfo& srcInfo, const void* srcPixels, size_t srcRB) {
SkASSERT(dstInfo.dimensions() == srcInfo.dimensions());
SkASSERT(SkImageInfoValidConversion(dstInfo, srcInfo));
+ int srcStride = (int)(srcRB / srcInfo.bytesPerPixel());
+ int dstStride = (int)(dstRB / dstInfo.bytesPerPixel());
+ if ((size_t)srcStride * srcInfo.bytesPerPixel() != srcRB ||
+ (size_t)dstStride * dstInfo.bytesPerPixel() != dstRB) {
+ return false;
+ }
+
SkColorSpaceXformSteps steps{srcInfo.colorSpace(), srcInfo.alphaType(),
dstInfo.colorSpace(), dstInfo.alphaType()};
for (auto fn : {rect_memcpy, swizzle_or_premul, convert_to_alpha8}) {
if (fn(dstInfo, dstPixels, dstRB, srcInfo, srcPixels, srcRB, steps)) {
- return;
+ return true;
}
}
- convert_with_pipeline(dstInfo, dstPixels, dstRB, srcInfo, srcPixels, srcRB, steps);
+ convert_with_pipeline(dstInfo, dstPixels, dstStride, srcInfo, srcPixels, srcStride, steps);
+ return true;
}
diff --git a/src/core/SkConvertPixels.h b/src/core/SkConvertPixels.h
index 99d6a01..af507c5 100644
--- a/src/core/SkConvertPixels.h
+++ b/src/core/SkConvertPixels.h
@@ -13,8 +13,9 @@
class SkColorTable;
-void SkConvertPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRowBytes,
- const SkImageInfo& srcInfo, const void* srcPixels, size_t srcRowBytes);
+bool SK_WARN_UNUSED_RESULT SkConvertPixels(
+ const SkImageInfo& dstInfo, void* dstPixels, size_t dstRowBytes,
+ const SkImageInfo& srcInfo, const void* srcPixels, size_t srcRowBytes);
static inline void SkRectMemcpy(void* dst, size_t dstRB, const void* src, size_t srcRB,
size_t trimRowBytes, int rowCount) {
diff --git a/src/core/SkDraw_vertices.cpp b/src/core/SkDraw_vertices.cpp
index 1b21424..432f24d 100644
--- a/src/core/SkDraw_vertices.cpp
+++ b/src/core/SkDraw_vertices.cpp
@@ -220,7 +220,7 @@
kUnpremul_SkAlphaType, SkColorSpace::MakeSRGB());
SkImageInfo dstInfo = SkImageInfo::Make(count, 1, kRGBA_F32_SkColorType,
kPremul_SkAlphaType, sk_ref_sp(deviceCS));
- SkConvertPixels(dstInfo, dst, 0, srcInfo, src, 0);
+ SkAssertResult(SkConvertPixels(dstInfo, dst, 0, srcInfo, src, 0));
return dst;
}
diff --git a/src/core/SkPixmap.cpp b/src/core/SkPixmap.cpp
index 2343352..02cc354 100644
--- a/src/core/SkPixmap.cpp
+++ b/src/core/SkPixmap.cpp
@@ -168,8 +168,8 @@
const void* srcPixels = this->addr(rec.fX, rec.fY);
const SkImageInfo srcInfo = fInfo.makeDimensions(rec.fInfo.dimensions());
- SkConvertPixels(rec.fInfo, rec.fPixels, rec.fRowBytes, srcInfo, srcPixels, this->rowBytes());
- return true;
+ return SkConvertPixels(rec.fInfo, rec.fPixels, rec.fRowBytes, srcInfo, srcPixels,
+ this->rowBytes());
}
bool SkPixmap::erase(SkColor color, const SkIRect& subset) const {
diff --git a/src/gpu/GrSurfaceContext.cpp b/src/gpu/GrSurfaceContext.cpp
index d67fa0f..8be9309 100644
--- a/src/gpu/GrSurfaceContext.cpp
+++ b/src/gpu/GrSurfaceContext.cpp
@@ -177,6 +177,10 @@
return false;
}
+ if (dst.rowBytes() % dst.info().bpp()) {
+ return false;
+ }
+
dst = dst.clip(this->dimensions(), &pt);
if (!dst.hasPixels()) {
return false;
diff --git a/src/shaders/gradients/SkGradientShader.cpp b/src/shaders/gradients/SkGradientShader.cpp
index 6e2fbe5..dce7230 100644
--- a/src/shaders/gradients/SkGradientShader.cpp
+++ b/src/shaders/gradients/SkGradientShader.cpp
@@ -473,8 +473,8 @@
}
std::vector<float> rgba(4*fColorCount); // TODO: SkSTArray?
- SkConvertPixels(dst, rgba.data(), dst.minRowBytes(),
- src, fOrigColors4f, src.minRowBytes());
+ SkAssertResult(SkConvertPixels(dst, rgba.data(), dst.minRowBytes(),
+ src, fOrigColors4f, src.minRowBytes()));
// Transform our colors into a scale factor f and bias b such that for
// any t between stops i and i+1, the color we want is mad(t, f[i], b[i]).
@@ -637,8 +637,10 @@
auto info = SkImageInfo::Make(colorCount,1, kRGBA_F32_SkColorType, kUnpremul_SkAlphaType);
- SkConvertPixels(info.makeColorSpace(sk_ref_sp(dst)), fStorage.begin(), info.minRowBytes(),
- info.makeColorSpace(sk_ref_sp(src)), fColors , info.minRowBytes());
+ auto dstInfo = info.makeColorSpace(sk_ref_sp(dst));
+ auto srcInfo = info.makeColorSpace(sk_ref_sp(src));
+ SkAssertResult(SkConvertPixels(dstInfo, fStorage.begin(), info.minRowBytes(),
+ srcInfo, fColors , info.minRowBytes()));
fColors = fStorage.begin();
}
diff --git a/src/utils/SkPatchUtils.cpp b/src/utils/SkPatchUtils.cpp
index 9ca2633..009c3c6 100644
--- a/src/utils/SkPatchUtils.cpp
+++ b/src/utils/SkPatchUtils.cpp
@@ -223,7 +223,7 @@
kUnpremul_SkAlphaType, SkColorSpace::MakeSRGB());
SkImageInfo dstInfo = SkImageInfo::Make(count, 1, kRGBA_F32_SkColorType,
kPremul_SkAlphaType, sk_ref_sp(dstCS));
- SkConvertPixels(dstInfo, dst, 0, srcInfo, src, 0);
+ SkAssertResult(SkConvertPixels(dstInfo, dst, 0, srcInfo, src, 0));
}
static void float_to_skcolor(SkColor* dst, const SkPMColor4f* src, int count, SkColorSpace* srcCS) {
@@ -231,7 +231,7 @@
kPremul_SkAlphaType, sk_ref_sp(srcCS));
SkImageInfo dstInfo = SkImageInfo::Make(count, 1, kBGRA_8888_SkColorType,
kUnpremul_SkAlphaType, SkColorSpace::MakeSRGB());
- SkConvertPixels(dstInfo, dst, 0, srcInfo, src, 0);
+ SkAssertResult(SkConvertPixels(dstInfo, dst, 0, srcInfo, src, 0));
}
sk_sp<SkVertices> SkPatchUtils::MakeVertices(const SkPoint cubics[12], const SkColor srcColors[4],