SkPDF: subset drawImageRect while still deduping
- Replace SkImageBitmap with SkImageSubset
- SkBitmapKey becomes trivial for simplicity.
- SkPDFCanvas::onDraw(Bitmap|Image)Rect now clip and call
SkCanvas::onDraw(Bitmap|Image)Rect.
- SkPDFDevice::draw(Bitmap|BitmapRect|Sprite) now convert bitmap
into SkImageSubset via make_image_subset function.
- SkPDFDevice::draw(Image|Bitmap)Rect now implemented again.
- SkPDFDevice::internalDrawImage now performs image subsetting
as needed, while still deduping properly.
BUG=633528
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2785
Change-Id: I063346d12b0e9c6b6c0c4943ee25400c88aa1a44
Reviewed-on: https://skia-review.googlesource.com/2785
Reviewed-by: Ben Wagner <bungeman@google.com>
diff --git a/src/pdf/SkBitmapKey.h b/src/pdf/SkBitmapKey.h
index 53d4d64..a640468 100644
--- a/src/pdf/SkBitmapKey.h
+++ b/src/pdf/SkBitmapKey.h
@@ -11,57 +11,67 @@
#include "SkImage.h"
#include "SkCanvas.h"
-class SkBitmapKey {
-public:
- SkBitmapKey() : fSubset(SkIRect::MakeEmpty()), fID(0) {}
- explicit SkBitmapKey(const SkBitmap& bm)
- : fSubset(bm.getSubset()), fID(bm.getGenerationID()) {}
- explicit SkBitmapKey(const SkImage* img)
- : fSubset(img ? img->bounds() : SkIRect::MakeEmpty())
- , fID(img ? img->uniqueID() : 0) {}
- explicit SkBitmapKey(const sk_sp<SkImage> img)
- : fSubset(img->bounds()), fID(img->uniqueID()) {}
+struct SkBitmapKey {
+ SkIRect fSubset;
+ uint32_t fID;
bool operator==(const SkBitmapKey& rhs) const {
return fID == rhs.fID && fSubset == rhs.fSubset;
}
bool operator!=(const SkBitmapKey& rhs) const { return !(*this == rhs); }
- uint32_t id() const { return fID; }
-
-private:
- SkIRect fSubset;
- uint32_t fID;
};
/**
- This wraps a thing that could either be a bitmap or a image and
- abstracts out some common tasks.
+ This class has all the advantages of SkBitmaps and SkImages.
*/
-class SkImageBitmap {
+class SkImageSubset {
public:
- explicit SkImageBitmap(const SkBitmap& b) : fBitmap(b), fImage(nullptr) {}
- explicit SkImageBitmap(SkImage* i) : fImage(i) { SkASSERT(fImage); }
- SkIRect bounds() const { return fImage ? fImage->bounds() : fBitmap.bounds(); }
- SkISize dimensions() const {
- return fImage ? fImage->dimensions() : fBitmap.dimensions();
- }
- sk_sp<SkImage> makeImage() const {
- return fImage ? sk_ref_sp(fImage) : SkImage::MakeFromBitmap(fBitmap);
- }
- SkBitmapKey getKey() const {
- return fImage ? SkBitmapKey(fImage) : SkBitmapKey(fBitmap);
- }
- void draw(SkCanvas* canvas, SkPaint* paint) const {
- if (fImage) {
- canvas->drawImage(fImage, 0, 0, paint);
+ SkImageSubset(sk_sp<SkImage> i, SkIRect subset = {0, 0, 0, 0})
+ : fImage(std::move(i)) {
+ if (!fImage) {
+ fSubset = {0, 0, 0, 0};
+ fID = 0;
+ return;
+ }
+ fID = fImage->uniqueID();
+ if (subset.isEmpty()) {
+ fSubset = fImage->bounds();
+ // SkImage always has a non-zero dimensions.
+ SkASSERT(!fSubset.isEmpty());
} else {
- canvas->drawBitmap(fBitmap, 0, 0, paint);
+ fSubset = subset;
+ if (!fSubset.intersect(fImage->bounds())) {
+ fImage = nullptr;
+ fSubset = {0, 0, 0, 0};
+ fID = 0;
+ }
}
}
+ void setID(uint32_t id) { fID = id; }
+
+ bool isValid() const { return fImage != nullptr; }
+
+ SkIRect bounds() const { return SkIRect::MakeSize(this->dimensions()); }
+
+ SkISize dimensions() const { return fSubset.size(); }
+
+ sk_sp<SkImage> makeImage() const {
+ return fSubset == fImage->bounds() ? fImage : fImage->makeSubset(fSubset);
+ }
+
+ SkBitmapKey getKey() const { return SkBitmapKey{fSubset, fID}; }
+
+ void draw(SkCanvas* canvas, SkPaint* paint) const {
+ SkASSERT(this->isValid());
+ SkRect src = SkRect::Make(fSubset),
+ dst = SkRect::Make(this->bounds());
+ canvas->drawImageRect(fImage.get(), src, dst, paint);
+ }
+
private:
- SkBitmap fBitmap;
- SkImage* fImage; // non-owning; when drawImage starts passing a sk_sp<>,
- // we can take a const ref to that sk_sp<>.
+ SkIRect fSubset;
+ sk_sp<SkImage> fImage;
+ uint32_t fID;
};
#endif // SkBitmapKey_DEFINED
diff --git a/src/pdf/SkPDFCanvas.cpp b/src/pdf/SkPDFCanvas.cpp
index 0fc242b..13fcb55 100644
--- a/src/pdf/SkPDFCanvas.cpp
+++ b/src/pdf/SkPDFCanvas.cpp
@@ -54,35 +54,23 @@
}
void SkPDFCanvas::onDrawImageRect(const SkImage* image,
- const SkRect* srcPtr,
+ const SkRect* src,
const SkRect& dst,
const SkPaint* paint,
SkCanvas::SrcRectConstraint constraint) {
- SkRect bounds = SkRect::Make(image->bounds());
- SkRect src = srcPtr ? *srcPtr : bounds;
SkAutoCanvasRestore autoCanvasRestore(this, true);
- if (src != bounds) {
- this->clipRect(dst);
- }
- this->concat(SkMatrix::MakeRectToRect(src, dst,
- SkMatrix::kFill_ScaleToFit));
- this->drawImage(image, 0, 0, paint);
+ this->clipRect(dst);
+ this->SkCanvas::onDrawImageRect(image, src, dst, paint, constraint);
}
void SkPDFCanvas::onDrawBitmapRect(const SkBitmap& bitmap,
- const SkRect* srcPtr,
+ const SkRect* src,
const SkRect& dst,
const SkPaint* paint,
SkCanvas::SrcRectConstraint constraint) {
- SkRect bounds = SkRect::Make(bitmap.bounds());
- SkRect src = srcPtr ? *srcPtr : bounds;
SkAutoCanvasRestore autoCanvasRestore(this, true);
- if (src != bounds) {
- this->clipRect(dst);
- }
- this->concat(SkMatrix::MakeRectToRect(src, dst,
- SkMatrix::kFill_ScaleToFit));
- this->drawBitmap(bitmap, 0, 0, paint);
+ this->clipRect(dst);
+ this->SkCanvas::onDrawBitmapRect(bitmap, src, dst, paint, constraint);
}
void SkPDFCanvas::onDrawImageLattice(const SkImage* image,
diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp
index eed4992..4065235 100644
--- a/src/pdf/SkPDFDevice.cpp
+++ b/src/pdf/SkPDFDevice.cpp
@@ -31,6 +31,7 @@
#include "SkPDFShader.h"
#include "SkPDFTypes.h"
#include "SkPDFUtils.h"
+#include "SkPixelRef.h"
#include "SkRasterClip.h"
#include "SkRRect.h"
#include "SkScopeExit.h"
@@ -83,6 +84,27 @@
return result;
}
+static SkImageSubset make_image_subset(const SkBitmap& bitmap) {
+ SkASSERT(!bitmap.drawsNothing());
+ SkIRect subset = bitmap.getSubset();
+ SkAutoLockPixels autoLockPixels(bitmap);
+ SkASSERT(bitmap.pixelRef());
+ SkBitmap tmp;
+ tmp.setInfo(bitmap.pixelRef()->info(), bitmap.rowBytes());
+ tmp.setPixelRef(bitmap.pixelRef());
+ tmp.lockPixels();
+ auto img = SkImage::MakeFromBitmap(tmp);
+ if (img) {
+ SkASSERT(!bitmap.isImmutable() || img->uniqueID() == bitmap.getGenerationID());
+ SkASSERT(img->bounds().contains(subset));
+ }
+ SkImageSubset imageSubset(std::move(img), subset);
+ // SkImage::MakeFromBitmap only preserves genID for immutable
+ // bitmaps. Use the bitmap's original ID for de-duping.
+ imageSubset.setID(bitmap.getGenerationID());
+ return imageSubset;
+}
+
SkPDFDevice::GraphicStateEntry::GraphicStateEntry()
: fColor(SK_ColorBLACK)
, fTextScaleX(SK_Scalar1)
@@ -772,33 +794,98 @@
&content.entry()->fContent);
}
-void SkPDFDevice::drawBitmapRect(const SkDraw& draw,
+
+void SkPDFDevice::drawImageRect(const SkDraw& d,
+ const SkImage* image,
+ const SkRect* src,
+ const SkRect& dst,
+ const SkPaint& srcPaint,
+ SkCanvas::SrcRectConstraint) {
+ if (!image) {
+ return;
+ }
+ SkIRect bounds = image->bounds();
+ SkPaint paint = srcPaint;
+ if (image->isOpaque()) {
+ replace_srcmode_on_opaque_paint(&paint);
+ }
+ SkRect srcRect = src ? *src : SkRect::Make(bounds);
+ SkMatrix transform;
+ transform.setRectToRect(srcRect, dst, SkMatrix::kFill_ScaleToFit);
+ if (src) {
+ if (!srcRect.intersect(SkRect::Make(bounds))) {
+ return;
+ }
+ srcRect.roundOut(&bounds);
+ transform.preTranslate(SkIntToScalar(bounds.x()),
+ SkIntToScalar(bounds.y()));
+ }
+ SkImageSubset imageSubset(sk_ref_sp(const_cast<SkImage*>(image)), bounds);
+ if (!imageSubset.isValid()) {
+ return;
+ }
+ transform.postConcat(*d.fMatrix);
+ this->internalDrawImage(transform, d.fClipStack, d.fRC->bwRgn(),
+ std::move(imageSubset), paint);
+}
+
+void SkPDFDevice::drawBitmapRect(const SkDraw& d,
const SkBitmap& bitmap,
const SkRect* src,
const SkRect& dst,
- const SkPaint& srcPaint,
- SkCanvas::SrcRectConstraint constraint) {
- SkASSERT(false);
+ const SkPaint& srcPaint,
+ SkCanvas::SrcRectConstraint) {
+ if (bitmap.drawsNothing()) {
+ return;
+ }
+ SkIRect bounds = bitmap.bounds();
+ SkPaint paint = srcPaint;
+ if (bitmap.isOpaque()) {
+ replace_srcmode_on_opaque_paint(&paint);
+ }
+ SkRect srcRect = src ? *src : SkRect::Make(bounds);
+ SkMatrix transform;
+ transform.setRectToRect(srcRect, dst, SkMatrix::kFill_ScaleToFit);
+ if (src) {
+ if (!srcRect.intersect(SkRect::Make(bounds))) {
+ return;
+ }
+ srcRect.roundOut(&bounds);
+ transform.preTranslate(SkIntToScalar(bounds.x()),
+ SkIntToScalar(bounds.y()));
+ }
+ SkBitmap bitmapSubset;
+ if (!bitmap.extractSubset(&bitmapSubset, bounds)) {
+ return;
+ }
+ SkImageSubset imageSubset = make_image_subset(bitmapSubset);
+ if (!imageSubset.isValid()) {
+ return;
+ }
+ transform.postConcat(*d.fMatrix);
+ this->internalDrawImage(transform, d.fClipStack, d.fRC->bwRgn(),
+ std::move(imageSubset), paint);
}
void SkPDFDevice::drawBitmap(const SkDraw& d,
const SkBitmap& bitmap,
const SkMatrix& matrix,
const SkPaint& srcPaint) {
+ if (bitmap.drawsNothing() || d.fRC->isEmpty()) {
+ return;
+ }
SkPaint paint = srcPaint;
if (bitmap.isOpaque()) {
replace_srcmode_on_opaque_paint(&paint);
}
-
- if (d.fRC->isEmpty()) {
+ SkImageSubset imageSubset = make_image_subset(bitmap);
+ if (!imageSubset.isValid()) {
return;
}
-
SkMatrix transform = matrix;
transform.postConcat(*d.fMatrix);
- SkImageBitmap imageBitmap(bitmap);
this->internalDrawImage(
- transform, d.fClipStack, d.fRC->bwRgn(), imageBitmap, paint);
+ transform, d.fClipStack, d.fRC->bwRgn(), std::move(imageSubset), paint);
}
void SkPDFDevice::drawSprite(const SkDraw& d,
@@ -806,20 +893,20 @@
int x,
int y,
const SkPaint& srcPaint) {
+ if (bitmap.drawsNothing() || d.fRC->isEmpty()) {
+ return;
+ }
SkPaint paint = srcPaint;
if (bitmap.isOpaque()) {
replace_srcmode_on_opaque_paint(&paint);
}
-
- if (d.fRC->isEmpty()) {
+ SkImageSubset imageSubset = make_image_subset(bitmap);
+ if (!imageSubset.isValid()) {
return;
}
-
- SkMatrix matrix;
- matrix.setTranslate(SkIntToScalar(x), SkIntToScalar(y));
- SkImageBitmap imageBitmap(bitmap);
+ SkMatrix transform = SkMatrix::MakeTrans(SkIntToScalar(x), SkIntToScalar(y));
this->internalDrawImage(
- matrix, d.fClipStack, d.fRC->bwRgn(), imageBitmap, paint);
+ transform, d.fClipStack, d.fRC->bwRgn(), std::move(imageSubset), paint);
}
void SkPDFDevice::drawImage(const SkDraw& draw,
@@ -837,20 +924,14 @@
if (draw.fRC->isEmpty()) {
return;
}
+ SkImageSubset imageSubset(sk_ref_sp(const_cast<SkImage*>(image)));
+ if (!imageSubset.isValid()) {
+ return;
+ }
SkMatrix transform = SkMatrix::MakeTrans(x, y);
transform.postConcat(*draw.fMatrix);
- SkImageBitmap imageBitmap(const_cast<SkImage*>(image));
this->internalDrawImage(
- transform, draw.fClipStack, draw.fRC->bwRgn(), imageBitmap, paint);
-}
-
-void SkPDFDevice::drawImageRect(const SkDraw& draw,
- const SkImage* image,
- const SkRect* src,
- const SkRect& dst,
- const SkPaint& srcPaint,
- SkCanvas::SrcRectConstraint constraint) {
- SkASSERT(false);
+ transform, draw.fClipStack, draw.fRC->bwRgn(), std::move(imageSubset), paint);
}
namespace {
@@ -2109,16 +2190,16 @@
return SkSize::Make(r.width(), r.height());
}
-static sk_sp<SkImage> color_filter(const SkImageBitmap& imageBitmap,
+static sk_sp<SkImage> color_filter(const SkImageSubset& imageSubset,
SkColorFilter* colorFilter) {
auto surface =
- SkSurface::MakeRaster(SkImageInfo::MakeN32Premul(imageBitmap.dimensions()));
+ SkSurface::MakeRaster(SkImageInfo::MakeN32Premul(imageSubset.dimensions()));
SkASSERT(surface);
SkCanvas* canvas = surface->getCanvas();
canvas->clear(SK_ColorTRANSPARENT);
SkPaint paint;
paint.setColorFilter(sk_ref_sp(colorFilter));
- imageBitmap.draw(canvas, &paint);
+ imageSubset.draw(canvas, &paint);
canvas->flush();
return surface->makeImageSnapshot();
}
@@ -2127,9 +2208,9 @@
void SkPDFDevice::internalDrawImage(const SkMatrix& origMatrix,
const SkClipStack* clipStack,
const SkRegion& origClipRegion,
- SkImageBitmap imageBitmap,
+ SkImageSubset imageSubset,
const SkPaint& paint) {
- if (imageBitmap.dimensions().isZero()) {
+ if (imageSubset.dimensions().isZero()) {
return;
}
#ifdef SK_PDF_IMAGE_STATS
@@ -2138,7 +2219,6 @@
SkMatrix matrix = origMatrix;
SkRegion perspectiveBounds;
const SkRegion* clipRegion = &origClipRegion;
- sk_sp<SkImage> autoImageUnref;
// Rasterize the bitmap using perspective in a new bitmap.
if (origMatrix.hasPerspective()) {
@@ -2148,7 +2228,7 @@
// Transform the bitmap in the new space, without taking into
// account the initial transform.
SkPath perspectiveOutline;
- SkRect imageBounds = SkRect::Make(imageBitmap.bounds());
+ SkRect imageBounds = SkRect::Make(imageSubset.bounds());
perspectiveOutline.addRect(imageBounds);
perspectiveOutline.transform(origMatrix);
@@ -2182,7 +2262,7 @@
SkISize wh = rect_to_size(physicalPerspectiveBounds).toCeil();
- auto surface(SkSurface::MakeRaster(SkImageInfo::MakeN32Premul(wh)));
+ auto surface = SkSurface::MakeRaster(SkImageInfo::MakeN32Premul(wh));
if (!surface) {
return;
}
@@ -2199,7 +2279,7 @@
// Translate the draw in the new canvas, so we perfectly fit the
// shape in the bitmap.
canvas->setMatrix(offsetMatrix);
- imageBitmap.draw(canvas, nullptr);
+ imageSubset.draw(canvas, nullptr);
// Make sure the final bits are in the bitmap.
canvas->flush();
@@ -2211,8 +2291,7 @@
perspectiveBounds.setRect(bounds.roundOut());
clipRegion = &perspectiveBounds;
- autoImageUnref = surface->makeImageSnapshot();
- imageBitmap = SkImageBitmap(autoImageUnref.get());
+ imageSubset = SkImageSubset(surface->makeImageSnapshot());
}
SkMatrix scaled;
@@ -2220,9 +2299,9 @@
scaled.setScale(SK_Scalar1, -SK_Scalar1);
scaled.postTranslate(0, SK_Scalar1);
// Scale the image up from 1x1 to WxH.
- SkIRect subset = imageBitmap.bounds();
- scaled.postScale(SkIntToScalar(imageBitmap.dimensions().width()),
- SkIntToScalar(imageBitmap.dimensions().height()));
+ SkIRect subset = imageSubset.bounds();
+ scaled.postScale(SkIntToScalar(imageSubset.dimensions().width()),
+ SkIntToScalar(imageSubset.dimensions().height()));
scaled.postConcat(matrix);
ScopedContentEntry content(this, clipStack, *clipRegion, scaled, paint);
if (!content.entry()) {
@@ -2244,16 +2323,16 @@
// drawBitmap*()/drawImage*() calls amd ImageFilters (which
// rasterize a layer on this backend). Fortuanely, this seems
// to be how Chromium impements most color-filters.
- autoImageUnref = color_filter(imageBitmap, colorFilter);
- imageBitmap = SkImageBitmap(autoImageUnref.get());
+ sk_sp<SkImage> img = color_filter(imageSubset, colorFilter);
+ imageSubset = SkImageSubset(std::move(img));
// TODO(halcanary): de-dupe this by caching filtered images.
// (maybe in the resource cache?)
}
- SkBitmapKey key = imageBitmap.getKey();
+ SkBitmapKey key = imageSubset.getKey();
sk_sp<SkPDFObject> pdfimage = fDocument->canon()->findPDFBitmap(key);
if (!pdfimage) {
- sk_sp<SkImage> img = imageBitmap.makeImage();
+ sk_sp<SkImage> img = imageSubset.makeImage();
if (!img) {
return;
}
diff --git a/src/pdf/SkPDFDevice.h b/src/pdf/SkPDFDevice.h
index db6f9b2..be0a277 100644
--- a/src/pdf/SkPDFDevice.h
+++ b/src/pdf/SkPDFDevice.h
@@ -21,7 +21,7 @@
#include "SkTDArray.h"
#include "SkTextBlob.h"
-class SkImageBitmap;
+class SkImageSubset;
class SkPath;
class SkPDFArray;
class SkPDFCanon;
@@ -285,7 +285,7 @@
void internalDrawImage(const SkMatrix& origMatrix,
const SkClipStack* clipStack,
const SkRegion& origClipRegion,
- SkImageBitmap imageBitmap,
+ SkImageSubset imageSubset,
const SkPaint& paint);
bool handleInversePath(const SkDraw& d, const SkPath& origPath,
diff --git a/src/pdf/SkPDFShader.cpp b/src/pdf/SkPDFShader.cpp
index d44f10b..82b5b34 100644
--- a/src/pdf/SkPDFShader.cpp
+++ b/src/pdf/SkPDFShader.cpp
@@ -958,7 +958,8 @@
SkScalar dpi,
const SkPDFShader::State& state,
SkBitmap image) {
- SkASSERT(state.fBitmapKey == SkBitmapKey(image));
+ SkASSERT(state.fBitmapKey ==
+ (SkBitmapKey{image.getSubset(), image.getGenerationID()}));
SkAutoLockPixels SkAutoLockPixels(image);
// The image shader pattern cell will be drawn into a separate device
@@ -1167,7 +1168,7 @@
if (fType == SkShader::kNone_GradientType) {
if (fBitmapKey != b.fBitmapKey ||
- fBitmapKey.id() == 0 ||
+ fBitmapKey.fID == 0 ||
fImageTileModes[0] != b.fImageTileModes[0] ||
fImageTileModes[1] != b.fImageTileModes[1]) {
return false;
@@ -1223,6 +1224,7 @@
fType = shader->asAGradient(&fInfo);
if (fType != SkShader::kNone_GradientType) {
+ fBitmapKey = SkBitmapKey{{0, 0, 0, 0}, 0};
fShaderTransform = shader->getLocalMatrix();
this->allocateGradientInfoStorage();
shader->asAGradient(&fInfo);
@@ -1231,7 +1233,7 @@
if (SkImage* skimg = shader->isAImage(&fShaderTransform, fImageTileModes)) {
// TODO(halcanary): delay converting to bitmap.
if (skimg->asLegacyBitmap(imageDst, SkImage::kRO_LegacyBitmapMode)) {
- fBitmapKey = SkBitmapKey(*imageDst);
+ fBitmapKey = SkBitmapKey{imageDst->getSubset(), imageDst->getGenerationID()};
return;
}
}
@@ -1276,7 +1278,7 @@
fShaderTransform.setTranslate(shaderRect.x(), shaderRect.y());
fShaderTransform.preScale(1 / scale.width(), 1 / scale.height());
- fBitmapKey = SkBitmapKey(*imageDst);
+ fBitmapKey = SkBitmapKey{imageDst->getSubset(), imageDst->getGenerationID()};
}
SkPDFShader::State::State(const SkPDFShader::State& other)
@@ -1305,7 +1307,7 @@
* Only valid for gradient states.
*/
SkPDFShader::State SkPDFShader::State::MakeAlphaToLuminosityState() const {
- SkASSERT(fBitmapKey == SkBitmapKey());
+ SkASSERT(fBitmapKey == (SkBitmapKey{{0, 0, 0, 0}, 0}));
SkASSERT(fType != SkShader::kNone_GradientType);
SkPDFShader::State newState(*this);
@@ -1323,7 +1325,7 @@
* Only valid for gradient states.
*/
SkPDFShader::State SkPDFShader::State::MakeOpaqueState() const {
- SkASSERT(fBitmapKey == SkBitmapKey());
+ SkASSERT(fBitmapKey == (SkBitmapKey{{0, 0, 0, 0}, 0}));
SkASSERT(fType != SkShader::kNone_GradientType);
SkPDFShader::State newState(*this);