SkPDF: simplify drawImage/Bitmap/Sprite code.

  - SkImageSubset becomes SkKeyedImage
  - SkPDFCanvas::onDraw{Bitmap, Image} go away
  - Remove SkPDFCanvas: base classes now do the right thing.
  - SkPDFDevice::draw{Bitmap,Image}{Rect,}() simplified
  - 244 fewer SLOC.

All but a few PDFs are identical, those rasterize almost the same.

Change-Id: I3ceb3b8935c689719cedf1ad544b0407b5c1733e
Reviewed-on: https://skia-review.googlesource.com/22218
Commit-Queue: Hal Canary <halcanary@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>

10 files changed, 215 insertions, 459 deletions

diff --git a/gn/pdf.gni b/gn/pdf.gni
index 6b140e0839..355c27a4a8 100644
--- a/gn/pdf.gni
+++ b/gn/pdf.gni
@@ -12,12 +12,12 @@ skia_pdf_sources = [
   "$_src/pdf/SkDeflate.h",
   "$_src/pdf/SkJpegInfo.cpp",
   "$_src/pdf/SkJpegInfo.h",
+  "$_src/pdf/SkKeyedImage.cpp",
+  "$_src/pdf/SkKeyedImage.h",
   "$_src/pdf/SkPDFBitmap.cpp",
   "$_src/pdf/SkPDFBitmap.h",
   "$_src/pdf/SkPDFCanon.cpp",
   "$_src/pdf/SkPDFCanon.h",
-  "$_src/pdf/SkPDFCanvas.cpp",
-  "$_src/pdf/SkPDFCanvas.h",
   "$_src/pdf/SkPDFConvertType1FontStream.cpp",
   "$_src/pdf/SkPDFConvertType1FontStream.h",
   "$_src/pdf/SkPDFDevice.cpp",
@@ -46,4 +46,5 @@ skia_pdf_sources = [
   "$_src/pdf/SkPDFTypes.h",
   "$_src/pdf/SkPDFUtils.cpp",
   "$_src/pdf/SkPDFUtils.h",
+  "$_src/pdf/SkScopeExit.h",
 ]
diff --git a/site/dev/design/pdftheory.md b/site/dev/design/pdftheory.md
index e07f014665..a5521ba864 100644
--- a/site/dev/design/pdftheory.md
+++ b/site/dev/design/pdftheory.md
@@ -42,7 +42,7 @@ to use SkPDF as a client calling Skia's public API.
 -----------------------------------------------------------------------------------
 
 SkPDFDevice is the main interface to the PDF backend. This child of
-SkDevice can be set on an SkPDFCanvas and drawn to.  Once drawing to
+SkDevice can be set on an SkCanvas and drawn to.  Once drawing to
 the canvas is complete (SkDocument::onEndPage() is called), the
 device's content and resouces are added to the SkPDFDocument that owns
 the device.  A new SkPDFDevice should be created for each page or
diff --git a/src/pdf/SkBitmapKey.h b/src/pdf/SkBitmapKey.h
index a640468d62..99d88a3b86 100644
--- a/src/pdf/SkBitmapKey.h
+++ b/src/pdf/SkBitmapKey.h
@@ -7,9 +7,7 @@
 #ifndef SkBitmapKey_DEFINED
 #define SkBitmapKey_DEFINED
 
-#include "SkBitmap.h"
-#include "SkImage.h"
-#include "SkCanvas.h"
+#include "SkRect.h"
 
 struct SkBitmapKey {
     SkIRect fSubset;
@@ -20,58 +18,5 @@ struct SkBitmapKey {
     bool operator!=(const SkBitmapKey& rhs) const { return !(*this == rhs); }
 };
 
-/**
-   This class has all the advantages of SkBitmaps and SkImages.
- */
-class SkImageSubset {
-public:
-    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 {
-            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:
-    SkIRect fSubset;
-    sk_sp<SkImage> fImage;
-    uint32_t fID;
-};
 
 #endif  // SkBitmapKey_DEFINED
diff --git a/src/pdf/SkKeyedImage.cpp b/src/pdf/SkKeyedImage.cpp
new file mode 100644
index 0000000000..bf196323af
--- /dev/null
+++ b/src/pdf/SkKeyedImage.cpp
@@ -0,0 +1,45 @@
+/*
+ * Copyright 2017 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "SkKeyedImage.h"
+
+#include "SkImage_Base.h"
+
+SkKeyedImage::SkKeyedImage(sk_sp<SkImage> i) : fImage(std::move(i)) {
+    if (fImage) {
+        if (const SkBitmap* bm = as_IB(fImage.get())->onPeekBitmap()) {
+            SkIPoint o = bm->pixelRefOrigin();
+            fKey = {fImage->bounds().makeOffset(o.fX, o.fY), bm->getGenerationID()};
+        } else {
+            fKey = {fImage->bounds(), fImage->uniqueID()};
+        }
+    }
+}
+
+SkKeyedImage::SkKeyedImage(const SkBitmap& bm) : fImage(SkImage::MakeFromBitmap(bm)) {
+    if (fImage) {
+        fKey = {bm.getSubset(), bm.getGenerationID()};
+    }
+}
+
+SkKeyedImage SkKeyedImage::subset(SkIRect subset) const {
+    SkKeyedImage img;
+    if (fImage && subset.intersect(fImage->bounds())) {
+        img.fImage = fImage->makeSubset(subset);
+        if (img.fImage) {
+            img.fKey = {subset.makeOffset(fKey.fSubset.x(), fKey.fSubset.y()), fKey.fID};
+        }
+    }
+    return img;
+}
+
+sk_sp<SkImage> SkKeyedImage::release() {
+    sk_sp<SkImage> image = std::move(fImage);
+    SkASSERT(nullptr == fImage);
+    fKey = {{0, 0, 0, 0}, 0};
+    return image;
+}
diff --git a/src/pdf/SkKeyedImage.h b/src/pdf/SkKeyedImage.h
new file mode 100644
index 0000000000..f74ec5c170
--- /dev/null
+++ b/src/pdf/SkKeyedImage.h
@@ -0,0 +1,40 @@
+/*
+ * 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 SkKeyedImage_DEFINED
+#define SkKeyedImage_DEFINED
+
+#include "SkBitmap.h"
+#include "SkBitmapKey.h"
+#include "SkImage.h"
+
+/**
+   This class has all the advantages of SkBitmaps and SkImages.
+
+   The SkImage holds on to encoded data.  The SkBitmapKey properly de-dups subsets.
+ */
+class SkKeyedImage {
+public:
+    SkKeyedImage() {}
+    SkKeyedImage(sk_sp<SkImage>);
+    SkKeyedImage(const SkBitmap&);
+    SkKeyedImage(SkKeyedImage&&) = default;
+    SkKeyedImage(const SkKeyedImage&) = default;
+
+    SkKeyedImage& operator=(SkKeyedImage&&) = default;
+    SkKeyedImage& operator=(const SkKeyedImage&) = default;
+
+    explicit operator bool() const { return fImage; }
+    const SkBitmapKey& key() const { return fKey; }
+    const sk_sp<SkImage>& image() const { return fImage; }
+    sk_sp<SkImage> release();
+    SkKeyedImage subset(SkIRect subset) const;
+
+private:
+    sk_sp<SkImage> fImage;
+    SkBitmapKey fKey = {{0, 0, 0, 0}, 0};
+};
+#endif  // SkKeyedImage_DEFINED
diff --git a/src/pdf/SkPDFCanvas.cpp b/src/pdf/SkPDFCanvas.cpp
deleted file mode 100644
index 720161ecf5..0000000000
--- a/src/pdf/SkPDFCanvas.cpp
+++ /dev/null
@@ -1,171 +0,0 @@
-/*
- * Copyright 2016 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-#include "SkImage.h"
-#include "SkLatticeIter.h"
-#include "SkPDFCanvas.h"
-#include "SkPDFDevice.h"
-
-SkPDFCanvas::SkPDFCanvas(const sk_sp<SkPDFDevice>& dev)
-    : SkCanvas(dev.get()) {}
-
-SkPDFCanvas::~SkPDFCanvas() {}
-
-/*
- *  PDF's impl sometimes wants to access the raster clip as a SkRegion. To keep this valid,
- *  we intercept all clip calls to ensure that the clip stays BW (i.e. never antialiased), since
- *  an antialiased clip won't build a SkRegion (it builds SkAAClip).
- */
-void SkPDFCanvas::onClipRect(const SkRect& rect, SkClipOp op, ClipEdgeStyle edgeStyle) {
-    this->INHERITED::onClipRect(rect, op, kHard_ClipEdgeStyle);
-}
-
-void SkPDFCanvas::onClipRRect(const SkRRect& rrect, SkClipOp op, ClipEdgeStyle edgeStyle) {
-    this->INHERITED::onClipRRect(rrect, op, kHard_ClipEdgeStyle);
-}
-
-void SkPDFCanvas::onClipPath(const SkPath& path, SkClipOp op, ClipEdgeStyle edgeStyle) {
-    this->INHERITED::onClipPath(path, op, kHard_ClipEdgeStyle);
-}
-
-void SkPDFCanvas::onDrawBitmapNine(const SkBitmap& bitmap,
-                                   const SkIRect& center,
-                                   const SkRect& dst,
-                                   const SkPaint* paint) {
-    SkLatticeIter iter(bitmap.width(), bitmap.height(), center, dst);
-    SkRect srcR, dstR;
-    while (iter.next(&srcR, &dstR)) {
-        this->drawBitmapRect(bitmap, srcR, dstR, paint);
-    }
-}
-
-void SkPDFCanvas::onDrawImageNine(const SkImage* image,
-                                  const SkIRect& center,
-                                  const SkRect& dst,
-                     const SkPaint* paint) {
-    SkLatticeIter iter(image->width(), image->height(), center, dst);
-    SkRect srcR, dstR;
-    while (iter.next(&srcR, &dstR)) {
-        this->drawImageRect(image, srcR, dstR, paint);
-    }
-}
-
-void SkPDFCanvas::onDrawImage(const SkImage* image,
-                              SkScalar x,
-                              SkScalar y,
-                              const SkPaint* paint) {
-    SkASSERT(image);
-    if (paint && paint->getMaskFilter()) {
-        SkPaint paintCopy(*paint);
-        SkMatrix m = SkMatrix::MakeTrans(x, y);
-        paintCopy.setShader(image->makeShader(&m));
-        this->drawRect(SkRect::MakeXYWH(x, y, image->width(), image->height()), paintCopy);
-        return;
-    }
-    this->SkCanvas::onDrawImage(image, x, y, paint);
-}
-
-void SkPDFCanvas::onDrawBitmap(const SkBitmap& bitmap,
-                              SkScalar x,
-                              SkScalar y,
-                              const SkPaint* paint) {
-    if (paint && paint->getMaskFilter()) {
-        if (sk_sp<SkImage> img = SkImage::MakeFromBitmap(bitmap)) {
-            this->onDrawImage(img.get(), x, y, paint);
-        }
-        return;
-    }
-    this->SkCanvas::onDrawBitmap(bitmap, x, y, paint);
-}
-
-
-static bool is_integer(SkScalar x) {
-    return x == SkScalarTruncToScalar(x);
-}
-
-static bool is_integral(const SkRect& r) {
-    return is_integer(r.left()) &&
-           is_integer(r.top()) &&
-           is_integer(r.right()) &&
-           is_integer(r.bottom());
-}
-
-void SkPDFCanvas::onDrawImageRect(const SkImage* image,
-                                  const SkRect* src,
-                                  const SkRect& dst,
-                                  const SkPaint* paint,
-                                  SkCanvas::SrcRectConstraint constraint) {
-    SkASSERT(src);
-    SkASSERT(image);
-    if (paint && paint->getMaskFilter()) {
-        SkPaint paintCopy(*paint);
-        paintCopy.setAntiAlias(true);
-        SkRect srcRect = src ? *src : SkRect::Make(image->bounds());
-        SkMatrix m = SkMatrix::MakeRectToRect(srcRect, dst, SkMatrix::kFill_ScaleToFit);
-        if (!src || *src == SkRect::Make(image->bounds()) ||
-                SkCanvas::kFast_SrcRectConstraint == constraint) {
-            paintCopy.setShader(image->makeShader(&m));
-        } else {
-            SkIRect subset = src->roundOut();
-            m.preTranslate(subset.x(), subset.y());
-            auto si = image->makeSubset(subset);
-            if (!si) { return; }
-            paintCopy.setShader(si->makeShader(&m));
-        }
-        this->drawRect(dst, paintCopy);
-        return;
-    }
-    SkAutoCanvasRestore autoCanvasRestore(this, false);
-    if (src && !is_integral(*src)) {
-        this->save();
-        this->clipRect(dst);
-    }
-    this->SkCanvas::onDrawImageRect(image, src, dst, paint, constraint);
-}
-
-void SkPDFCanvas::onDrawBitmapRect(const SkBitmap& bitmap,
-                                   const SkRect* src,
-                                   const SkRect& dst,
-                                   const SkPaint* paint,
-                                   SkCanvas::SrcRectConstraint constraint) {
-    SkASSERT(src);
-    if (paint && paint->getMaskFilter()) {
-        if (sk_sp<SkImage> img = SkImage::MakeFromBitmap(bitmap)) {
-            this->onDrawImageRect(img.get(), src, dst, paint, constraint);
-        }
-        return;
-    }
-    SkAutoCanvasRestore autoCanvasRestore(this, false);
-    if (src && !is_integral(*src)) {
-        this->save();
-        this->clipRect(dst);
-    }
-    this->SkCanvas::onDrawBitmapRect(bitmap, src, dst, paint, constraint);
-}
-
-void SkPDFCanvas::onDrawImageLattice(const SkImage* image,
-                                     const Lattice& lattice,
-                                     const SkRect& dst,
-                                     const SkPaint* paint) {
-    SkLatticeIter iter(lattice, dst);
-    SkRect srcR, dstR;
-    while (iter.next(&srcR, &dstR)) {
-        this->drawImageRect(image, srcR, dstR, paint);
-    }
-}
-
-void SkPDFCanvas::onDrawBitmapLattice(const SkBitmap& bitmap,
-                                      const Lattice& lattice,
-                                      const SkRect& dst,
-                                      const SkPaint* paint) {
-    SkLatticeIter iter(lattice, dst);
-    SkRect srcR, dstR;
-    while (iter.next(&srcR, &dstR)) {
-        this->drawBitmapRect(bitmap, srcR, dstR, paint);
-    }
-}
-
diff --git a/src/pdf/SkPDFCanvas.h b/src/pdf/SkPDFCanvas.h
deleted file mode 100644
index 117056664a..0000000000
--- a/src/pdf/SkPDFCanvas.h
+++ /dev/null
@@ -1,59 +0,0 @@
-/*
- * Copyright 2016 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-#ifndef SkPDFCanvas_DEFINED
-#define SkPDFCanvas_DEFINED
-
-#include "SkCanvas.h"
-
-class SkPDFDevice;
-
-class SkPDFCanvas : public SkCanvas {
-public:
-    SkPDFCanvas(const sk_sp<SkPDFDevice>&);
-    ~SkPDFCanvas() override;
-
-protected:
-    void onClipRect(const SkRect&, SkClipOp, ClipEdgeStyle) override;
-    void onClipRRect(const SkRRect&, SkClipOp, ClipEdgeStyle) override;
-    void onClipPath(const SkPath&, SkClipOp, ClipEdgeStyle) override;
-
-    void onDrawImage(const SkImage*, SkScalar, SkScalar, const SkPaint*) override;
-    void onDrawBitmap(const SkBitmap&, SkScalar, SkScalar, const SkPaint*) override;
-
-    void onDrawBitmapNine(const SkBitmap&, const SkIRect&, const SkRect&,
-                          const SkPaint*) override;
-
-    void onDrawImageNine(const SkImage*, const SkIRect&, const SkRect&,
-                         const SkPaint*) override;
-
-    void onDrawImageRect(const SkImage*,
-                         const SkRect*,
-                         const SkRect&,
-                         const SkPaint*,
-                         SkCanvas::SrcRectConstraint) override;
-
-    void onDrawBitmapRect(const SkBitmap&,
-                          const SkRect*,
-                          const SkRect&,
-                          const SkPaint*,
-                          SkCanvas::SrcRectConstraint) override;
-
-    void onDrawImageLattice(const SkImage*,
-                            const Lattice&,
-                            const SkRect&,
-                            const SkPaint*) override;
-
-    void onDrawBitmapLattice(const SkBitmap&,
-                             const Lattice&,
-                             const SkRect&,
-                             const SkPaint*) override;
-
-private:
-    typedef SkCanvas INHERITED;
-};
-
-#endif  // SkPDFCanvas_DEFINED
diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp
index b744a524f6..3125904797 100644
--- a/src/pdf/SkPDFDevice.cpp
+++ b/src/pdf/SkPDFDevice.cpp
@@ -11,6 +11,7 @@
 #include "SkAnnotationKeys.h"
 #include "SkBitmapDevice.h"
 #include "SkBitmapKey.h"
+#include "SkCanvas.h"
 #include "SkClipOpPriv.h"
 #include "SkColor.h"
 #include "SkColorFilter.h"
@@ -23,7 +24,6 @@
 #include "SkMaskFilter.h"
 #include "SkPDFBitmap.h"
 #include "SkPDFCanon.h"
-#include "SkPDFCanvas.h"
 #include "SkPDFDocument.h"
 #include "SkPDFFont.h"
 #include "SkPDFFormXObject.h"
@@ -157,26 +157,6 @@ static SkPaint calculate_text_paint(const SkPaint& paint) {
     return result;
 }
 
-static SkImageSubset make_image_subset(const SkBitmap& bitmap) {
-    SkASSERT(!bitmap.drawsNothing());
-    SkIRect subset = bitmap.getSubset();
-    SkASSERT(bitmap.pixelRef());
-    SkBitmap tmp;
-    SkImageInfo pixelRefInfo =
-            bitmap.info().makeWH(bitmap.pixelRef()->width(), bitmap.pixelRef()->height());
-    tmp.setInfo(pixelRefInfo, bitmap.rowBytes());
-    tmp.setPixelRef(sk_ref_sp(bitmap.pixelRef()), 0, 0);
-    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;
-}
 
 // If the paint has a color filter, apply the color filter to the shader or the
 // paint color.  Remove the color filter.
@@ -864,7 +844,7 @@ void SkPDFDevice::internalDrawPathWithFilter(const SkClipStack& clipStack,
     // Must mask with a Form XObject.
     sk_sp<SkPDFDevice> maskDevice = this->makeCongruentDevice();
     {
-        SkPDFCanvas canvas(maskDevice);
+        SkCanvas canvas(maskDevice.get());
         canvas.drawImage(mask, dstMaskBounds.x(), dstMaskBounds.y());
     }
     if (!ctm.isIdentity() && paint->getShader()) {
@@ -977,134 +957,48 @@ void SkPDFDevice::internalDrawPath(const SkClipStack& clipStack,
     SkPDFUtils::PaintPath(paint.getStyle(), pathPtr->getFillType(), content.stream());
 }
 
+////////////////////////////////////////////////////////////////////////////////
 
 void SkPDFDevice::drawImageRect(const SkImage* image,
                                 const SkRect* src,
                                 const SkRect& dst,
-                                const SkPaint& srcPaint,
+                                const SkPaint& paint,
                                 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(this->ctm());
-    this->internalDrawImage(transform, this->cs(), std::move(imageSubset), paint);
+    SkASSERT(image);
+    this->internalDrawImageRect(SkKeyedImage(sk_ref_sp(const_cast<SkImage*>(image))),
+                                src, dst, paint, this->ctm());
 }
 
-void SkPDFDevice::drawBitmapRect(const SkBitmap& bitmap,
+void SkPDFDevice::drawBitmapRect(const SkBitmap& bm,
                                  const SkRect* src,
                                  const SkRect& dst,
-                                 const SkPaint& srcPaint,
+                                 const SkPaint& paint,
                                  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(this->ctm());
-    this->internalDrawImage(transform, this->cs(), std::move(imageSubset), paint);
+    SkASSERT(!bm.drawsNothing());
+    this->internalDrawImageRect(SkKeyedImage(bm), src, dst, paint, this->ctm());
 }
 
-void SkPDFDevice::drawBitmap(const SkBitmap& bitmap,
-                             SkScalar x,
-                             SkScalar y,
-                             const SkPaint& srcPaint) {
-    if (bitmap.drawsNothing() || this->cs().isEmpty(size(*this))) {
-        return;
-    }
-    SkPaint paint = srcPaint;
-    if (bitmap.isOpaque()) {
-        replace_srcmode_on_opaque_paint(&paint);
-    }
-    SkImageSubset imageSubset = make_image_subset(bitmap);
-    if (!imageSubset.isValid()) {
-        return;
-    }
-    SkMatrix transform = SkMatrix::MakeTrans(x, y);
-    transform.postConcat(this->ctm());
-    this->internalDrawImage(transform, this->cs(), std::move(imageSubset), paint);
+void SkPDFDevice::drawBitmap(const SkBitmap& bm, SkScalar x, SkScalar y, const SkPaint& paint) {
+    SkASSERT(!bm.drawsNothing());
+    auto r = SkRect::MakeXYWH(x, y, bm.width(), bm.height());
+    this->internalDrawImageRect(SkKeyedImage(bm), nullptr, r, paint, this->ctm());
 }
 
-void SkPDFDevice::drawSprite(const SkBitmap& bitmap,
-                             int x,
-                             int y,
-                             const SkPaint& srcPaint) {
-    if (bitmap.drawsNothing() || this->cs().isEmpty(size(*this))) {
-        return;
-    }
-    SkPaint paint = srcPaint;
-    if (bitmap.isOpaque()) {
-        replace_srcmode_on_opaque_paint(&paint);
-    }
-    SkImageSubset imageSubset = make_image_subset(bitmap);
-    if (!imageSubset.isValid()) {
-        return;
-    }
-    SkMatrix transform = SkMatrix::MakeTrans(SkIntToScalar(x), SkIntToScalar(y));
-    this->internalDrawImage(transform, this->cs(), std::move(imageSubset), paint);
+void SkPDFDevice::drawSprite(const SkBitmap& bm, int x, int y, const SkPaint& paint) {
+    SkASSERT(!bm.drawsNothing());
+    auto r = SkRect::MakeXYWH(x, y, bm.width(), bm.height());
+    this->internalDrawImageRect(SkKeyedImage(bm), nullptr, r, paint, SkMatrix::I());
 }
 
-void SkPDFDevice::drawImage(const SkImage* image,
-                            SkScalar x,
-                            SkScalar y,
-                            const SkPaint& srcPaint) {
-    SkPaint paint = srcPaint;
-    if (!image) {
-        return;
-    }
-    if (image->isOpaque()) {
-        replace_srcmode_on_opaque_paint(&paint);
-    }
-    SkImageSubset imageSubset(sk_ref_sp(const_cast<SkImage*>(image)));
-    if (!imageSubset.isValid()) {
-        return;
-    }
-    SkMatrix transform = SkMatrix::MakeTrans(x, y);
-    transform.postConcat(this->ctm());
-    this->internalDrawImage(transform, this->cs(), std::move(imageSubset), paint);
+void SkPDFDevice::drawImage(const SkImage* image, SkScalar x, SkScalar y, const SkPaint& paint) {
+    SkASSERT(image);
+    auto r = SkRect::MakeXYWH(x, y, image->width(), image->height());
+    this->internalDrawImageRect(SkKeyedImage(sk_ref_sp(const_cast<SkImage*>(image))),
+                                nullptr, r, paint, this->ctm());
 }
 
+////////////////////////////////////////////////////////////////////////////////
+
 namespace {
 class GlyphPositioner {
 public:
@@ -2249,42 +2143,96 @@ int SkPDFDevice::getFontResourceIndex(SkTypeface* typeface, uint16_t glyphID) {
 
 static SkSize rect_to_size(const SkRect& r) { return {r.width(), r.height()}; }
 
-static sk_sp<SkImage> color_filter(const SkImageSubset& imageSubset,
+static sk_sp<SkImage> color_filter(const SkImage* image,
                                    SkColorFilter* colorFilter) {
     auto surface =
-        SkSurface::MakeRaster(SkImageInfo::MakeN32Premul(imageSubset.dimensions()));
+        SkSurface::MakeRaster(SkImageInfo::MakeN32Premul(image->dimensions()));
     SkASSERT(surface);
     SkCanvas* canvas = surface->getCanvas();
     canvas->clear(SK_ColorTRANSPARENT);
     SkPaint paint;
     paint.setColorFilter(sk_ref_sp(colorFilter));
-    imageSubset.draw(canvas, &paint);
-    canvas->flush();
+    canvas->drawImage(image, 0, 0, &paint);
     return surface->makeImageSnapshot();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-void SkPDFDevice::internalDrawImage(const SkMatrix& origMatrix,
-                                    const SkClipStack& clipStack,
-                                    SkImageSubset imageSubset,
-                                    const SkPaint& paint) {
-    if (imageSubset.dimensions().isZero()) {
+
+static bool is_integer(SkScalar x) {
+    return x == SkScalarTruncToScalar(x);
+}
+
+static bool is_integral(const SkRect& r) {
+    return is_integer(r.left()) &&
+           is_integer(r.top()) &&
+           is_integer(r.right()) &&
+           is_integer(r.bottom());
+}
+
+void SkPDFDevice::internalDrawImageRect(SkKeyedImage imageSubset,
+                                        const SkRect* src,
+                                        const SkRect& dst,
+                                        const SkPaint& srcPaint,
+                                        const SkMatrix& ctm) {
+    if (!imageSubset) {
+        return;
+    }
+
+    SkIRect bounds = imageSubset.image()->bounds();
+    SkPaint paint = srcPaint;
+    if (imageSubset.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 && *src != SkRect::Make(bounds)) {
+        if (!srcRect.intersect(SkRect::Make(bounds))) {
+            return;
+        }
+        srcRect.roundOut(&bounds);
+        transform.preTranslate(SkIntToScalar(bounds.x()),
+                               SkIntToScalar(bounds.y()));
+        if (bounds != imageSubset.image()->bounds()) {
+            imageSubset = imageSubset.subset(bounds);
+        }
+        if (!imageSubset) {
+            return;
+        }
+    }
+
+    if (paint.getMaskFilter()) {
+        paint.setShader(imageSubset.image()->makeShader(&transform));
+        SkPath path;
+        path.addRect(SkRect::Make(imageSubset.image()->bounds()));
+        this->internalDrawPath(this->cs(), this->ctm(), path, paint, &transform, true);
         return;
     }
+    transform.postConcat(ctm);
+
+    bool needToRestore = false;
+    if (src && !is_integral(*src)) {
+        // Need sub-pixel clipping to fix https://bug.skia.org/4374
+        this->cs().save();
+        this->cs().clipRect(dst, ctm, SkClipOp::kIntersect, true);
+        needToRestore = true;
+    }
+    SK_AT_SCOPE_EXIT(if (needToRestore) { this->cs().restore(); });
+
     #ifdef SK_PDF_IMAGE_STATS
     gDrawImageCalls.fetch_add(1);
     #endif
-    SkMatrix matrix = origMatrix;
+    SkMatrix matrix = transform;
 
     // Rasterize the bitmap using perspective in a new bitmap.
-    if (origMatrix.hasPerspective()) {
+    if (transform.hasPerspective()) {
         SkASSERT(fDocument->rasterDpi() > 0);
         // Transform the bitmap in the new space, without taking into
         // account the initial transform.
         SkPath perspectiveOutline;
-        SkRect imageBounds = SkRect::Make(imageSubset.bounds());
+        SkRect imageBounds = SkRect::Make(imageSubset.image()->bounds());
         perspectiveOutline.addRect(imageBounds);
-        perspectiveOutline.transform(origMatrix);
+        perspectiveOutline.transform(transform);
 
         // TODO(edisonn): perf - use current clip too.
         // Retrieve the bounds of the new shape.
@@ -2292,7 +2240,7 @@ void SkPDFDevice::internalDrawImage(const SkMatrix& origMatrix,
 
         // Transform the bitmap in the new space, taking into
         // account the initial transform.
-        SkMatrix total = origMatrix;
+        SkMatrix total = transform;
         total.postConcat(fInitialTransform);
         SkScalar dpiScale = SkIntToScalar(fDocument->rasterDpi()) /
                             SkIntToScalar(SkPDFUtils::kDpiForRasterScaleOne);
@@ -2326,14 +2274,14 @@ void SkPDFDevice::internalDrawImage(const SkMatrix& origMatrix,
         SkScalar deltaX = bounds.left();
         SkScalar deltaY = bounds.top();
 
-        SkMatrix offsetMatrix = origMatrix;
+        SkMatrix offsetMatrix = transform;
         offsetMatrix.postTranslate(-deltaX, -deltaY);
         offsetMatrix.postScale(scaleX, scaleY);
 
         // Translate the draw in the new canvas, so we perfectly fit the
         // shape in the bitmap.
         canvas->setMatrix(offsetMatrix);
-        imageSubset.draw(canvas, nullptr);
+        canvas->drawImage(imageSubset.image(), 0, 0);
         // Make sure the final bits are in the bitmap.
         canvas->flush();
 
@@ -2342,7 +2290,10 @@ void SkPDFDevice::internalDrawImage(const SkMatrix& origMatrix,
         matrix.setScale(1 / scaleX, 1 / scaleY);
         matrix.postTranslate(deltaX, deltaY);
 
-        imageSubset = SkImageSubset(surface->makeImageSnapshot());
+        imageSubset = SkKeyedImage(surface->makeImageSnapshot());
+        if (!imageSubset) {
+            return;
+        }
     }
 
     SkMatrix scaled;
@@ -2350,11 +2301,11 @@ void SkPDFDevice::internalDrawImage(const SkMatrix& origMatrix,
     scaled.setScale(SK_Scalar1, -SK_Scalar1);
     scaled.postTranslate(0, SK_Scalar1);
     // Scale the image up from 1x1 to WxH.
-    SkIRect subset = imageSubset.bounds();
-    scaled.postScale(SkIntToScalar(imageSubset.dimensions().width()),
-                     SkIntToScalar(imageSubset.dimensions().height()));
+    SkIRect subset = imageSubset.image()->bounds();
+    scaled.postScale(SkIntToScalar(subset.width()),
+                     SkIntToScalar(subset.height()));
     scaled.postConcat(matrix);
-    ScopedContentEntry content(this, clipStack, scaled, paint);
+    ScopedContentEntry content(this, this->cs(), scaled, paint);
     if (!content.entry()) {
         return;
     }
@@ -2374,26 +2325,27 @@ void SkPDFDevice::internalDrawImage(const SkMatrix& origMatrix,
         // drawBitmap*()/drawImage*() calls amd ImageFilters (which
         // rasterize a layer on this backend).  Fortuanely, this seems
         // to be how Chromium impements most color-filters.
-        sk_sp<SkImage> img = color_filter(imageSubset, colorFilter);
-        imageSubset = SkImageSubset(std::move(img));
+        sk_sp<SkImage> img = color_filter(imageSubset.image().get(), colorFilter);
+        imageSubset = SkKeyedImage(std::move(img));
+        if (!imageSubset) {
+            return;
+        }
         // TODO(halcanary): de-dupe this by caching filtered images.
         // (maybe in the resource cache?)
     }
 
-    SkBitmapKey key = imageSubset.getKey();
+    SkBitmapKey key = imageSubset.key();
     sk_sp<SkPDFObject>* pdfimagePtr = fDocument->canon()->fPDFBitmapMap.find(key);
     sk_sp<SkPDFObject> pdfimage = pdfimagePtr ? *pdfimagePtr : nullptr;
     if (!pdfimage) {
-        sk_sp<SkImage> img = imageSubset.makeImage();
-        if (!img) {
-            return;
-        }
-        pdfimage =
-                SkPDFCreateBitmapObject(std::move(img), fDocument->canon()->fPixelSerializer.get());
+        SkASSERT(imageSubset);
+        pdfimage = SkPDFCreateBitmapObject(imageSubset.release(),
+                                           fDocument->canon()->fPixelSerializer.get());
         if (!pdfimage) {
             return;
         }
         fDocument->serialize(pdfimage);  // serialize images early.
+        SkASSERT((key != SkBitmapKey{{0, 0, 0, 0}, 0}));
         fDocument->canon()->fPDFBitmapMap.set(key, pdfimage);
     }
     // TODO(halcanary): addXObjectResource() should take a sk_sp<SkPDFObject>
diff --git a/src/pdf/SkPDFDevice.h b/src/pdf/SkPDFDevice.h
index 8d2c511be0..6df9d6dba2 100644
--- a/src/pdf/SkPDFDevice.h
+++ b/src/pdf/SkPDFDevice.h
@@ -20,8 +20,9 @@
 #include "SkStream.h"
 #include "SkTDArray.h"
 #include "SkTextBlob.h"
+#include "SkKeyedImage.h"
 
-class SkImageSubset;
+class SkKeyedImage;
 class SkPath;
 class SkPDFArray;
 class SkPDFCanon;
@@ -246,10 +247,11 @@ private:
 
     void internalDrawPaint(const SkPaint& paint, ContentEntry* contentEntry);
 
-    void internalDrawImage(const SkMatrix& origMatrix,
-                           const SkClipStack& clipStack,
-                           SkImageSubset imageSubset,
-                           const SkPaint& paint);
+    void internalDrawImageRect(SkKeyedImage,
+                               const SkRect* src,
+                               const SkRect& dst,
+                               const SkPaint&,
+                               const SkMatrix& canvasTransformationMatrix);
 
     void internalDrawPath(const SkClipStack&,
                           const SkMatrix&,
diff --git a/src/pdf/SkPDFDocument.cpp b/src/pdf/SkPDFDocument.cpp
index ee009651b6..9ed335678b 100644
--- a/src/pdf/SkPDFDocument.cpp
+++ b/src/pdf/SkPDFDocument.cpp
@@ -5,11 +5,12 @@
  * found in the LICENSE file.
  */
 
+#include "SkPDFDocument.h"
+
+#include "SkCanvas.h"
 #include "SkMakeUnique.h"
 #include "SkPDFCanon.h"
-#include "SkPDFCanvas.h"
 #include "SkPDFDevice.h"
-#include "SkPDFDocument.h"
 #include "SkPDFUtils.h"
 #include "SkStream.h"
 
@@ -216,7 +217,7 @@ SkCanvas* SkPDFDocument::onBeginPage(SkScalar width, SkScalar height) {
             SkScalarRoundToInt(width), SkScalarRoundToInt(height));
     fPageDevice = sk_make_sp<SkPDFDevice>(pageSize, this);
     fPageDevice->setFlip();  // Only the top-level device needs to be flipped.
-    fCanvas.reset(new SkPDFCanvas(fPageDevice));
+    fCanvas.reset(new SkCanvas(fPageDevice.get()));
     return fCanvas.get();
 }