Avoid extra bitmap copies in SkColorSpaceXformCanvas

Before:
ColorCanvasDrawBitmap_sRGB_to_sRGB     5.09us
ColorCanvasDrawBitmap_AdobeRGB_to_sRGB 50.7us

After:
ColorCanvasDrawBitmap_sRGB_to_sRGB     2.43us
ColorCanvasDrawBitmap_AdobeRGB_to_sRGB 37.1us

BUG=skia:6456
Change-Id: Ie382936c36fd347b59485882cf8f27f315a5d35f

Change-Id: Ie382936c36fd347b59485882cf8f27f315a5d35f
Reviewed-on: https://skia-review.googlesource.com/11040
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Matt Sarett <msarett@google.com>

diff --git a/bench/ColorCanvasDrawBitmapBench.cpp b/bench/ColorCanvasDrawBitmapBench.cpp
new file mode 100644 (file)
index 0000000..de13ba2
--- /dev/null
+++ b/bench/ColorCanvasDrawBitmapBench.cpp
@@ -0,0 +1,57 @@
+/*
+* 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 "Benchmark.h"
+#include "SkCanvas.h"
+#include "SkColorSpaceXformCanvas.h"
+#include "SkString.h"
+
+class ColorCanvasDrawBitmap : public Benchmark {
+public:
+    ColorCanvasDrawBitmap(sk_sp<SkColorSpace> src, sk_sp<SkColorSpace> dst, const char* name)
+        : fDst(dst)
+        , fName(SkStringPrintf("ColorCanvasDrawBitmap_%s", name))
+    {
+        fBitmap.allocPixels(SkImageInfo::MakeN32(100, 100, kOpaque_SkAlphaType, src));
+        fBitmap.eraseColor(SK_ColorBLUE);
+    }
+
+    const char* onGetName() override {
+        return fName.c_str();
+    }
+
+    SkIPoint onGetSize() override {
+        return SkIPoint::Make(100, 100);
+    }
+
+    bool isSuitableFor(Backend backend) override {
+        return kRaster_Backend == backend || kGPU_Backend == backend;
+    }
+
+    void onDraw(int n, SkCanvas* canvas) override {
+        // This simulates an Android app that draws bitmaps to a software canvas.
+        // Chrome and the Android framework both use SkImages exclusively.
+        std::unique_ptr<SkCanvas> colorCanvas = SkCreateColorSpaceXformCanvas(canvas, fDst);
+        for (int i = 0; i < n; i++) {
+            colorCanvas->drawBitmap(fBitmap, 0, 0, nullptr);
+        }
+    }
+
+private:
+    sk_sp<SkColorSpace> fDst;
+    SkString            fName;
+    SkBitmap            fBitmap;
+
+    typedef Benchmark INHERITED;
+};
+
+DEF_BENCH(return new ColorCanvasDrawBitmap(nullptr, SkColorSpace::MakeSRGB(), "null_to_sRGB");)
+DEF_BENCH(return new ColorCanvasDrawBitmap(SkColorSpace::MakeSRGB(), SkColorSpace::MakeSRGB(),
+        "sRGB_to_sRGB");)
+DEF_BENCH(return new ColorCanvasDrawBitmap(
+        SkColorSpace::MakeRGB(SkColorSpace::kSRGB_RenderTargetGamma, SkColorSpace::kAdobeRGB_Gamut),
+        SkColorSpace::MakeSRGB(), "AdobeRGB_to_sRGB");)

diff --git a/gn/bench.gni b/gn/bench.gni
index d2d233f..5aff91b 100644 (file)
--- a/gn/bench.gni
+++ b/gn/bench.gni
@@ -29,6 +29,7 @@ bench_sources = [
   "$_bench/ChromeBench.cpp",
   "$_bench/CmapBench.cpp",
   "$_bench/CodecBench.cpp",
+  "$_bench/ColorCanvasDrawBitmapBench.cpp",
   "$_bench/ColorCodecBench.cpp",
   "$_bench/ColorFilterBench.cpp",
   "$_bench/ColorPrivBench.cpp",

diff --git a/src/core/SkColorSpaceXformCanvas.cpp b/src/core/SkColorSpaceXformCanvas.cpp
index 5756905..f701d12 100644 (file)
--- a/src/core/SkColorSpaceXformCanvas.cpp
+++ b/src/core/SkColorSpaceXformCanvas.cpp
@@ -10,16 +10,18 @@
 #include "SkColorSpaceXformer.h"
 #include "SkGradientShader.h"
 #include "SkImage_Base.h"
+#include "SkImagePriv.h"
 #include "SkMakeUnique.h"
 #include "SkNoDrawCanvas.h"
 #include "SkSurface.h"
 
 class SkColorSpaceXformCanvas : public SkNoDrawCanvas {
 public:
-    SkColorSpaceXformCanvas(SkCanvas* target,
+    SkColorSpaceXformCanvas(SkCanvas* target, sk_sp<SkColorSpace> targetCS,
                             std::unique_ptr<SkColorSpaceXformer> xformer)
         : SkNoDrawCanvas(SkIRect::MakeSize(target->getBaseLayerSize()))
         , fTarget(target)
+        , fTargetCS(targetCS)
         , fXformer(std::move(xformer))
     {
         // Set the matrix and clip to match |fTarget|.  Otherwise, we'll answer queries for
@@ -131,7 +133,7 @@ public:
                          const SkRect* src, const SkRect& dst,
                          const SkPaint* paint, SrcRectConstraint constraint) override {
         fTarget->drawImageRect(fXformer->apply(img).get(),
-                               src ? *src : dst, dst,
+                               src ? *src : SkRect::MakeIWH(img->width(), img->height()), dst,
                                fXformer->apply(paint), constraint);
     }
     void onDrawImageNine(const SkImage* img,
@@ -165,30 +167,45 @@ public:
     void onDrawBitmap(const SkBitmap& bitmap,
                       SkScalar l, SkScalar t,
                       const SkPaint* paint) override {
-        if (auto image = SkImage::MakeFromBitmap(bitmap)) {
-            this->onDrawImage(image.get(), l, t, paint);
+        if (this->skipXform(bitmap)) {
+            return fTarget->drawBitmap(bitmap, l, t, fXformer->apply(paint));
         }
+
+        fTarget->drawImage(fXformer->apply(bitmap).get(), l, t, fXformer->apply(paint));
     }
     void onDrawBitmapRect(const SkBitmap& bitmap,
                           const SkRect* src, const SkRect& dst,
                           const SkPaint* paint, SrcRectConstraint constraint) override {
-        if (auto image = SkImage::MakeFromBitmap(bitmap)) {
-            this->onDrawImageRect(image.get(), src, dst, paint, constraint);
+        if (this->skipXform(bitmap)) {
+            return fTarget->drawBitmapRect(bitmap,
+                    src ? *src : SkRect::MakeIWH(bitmap.width(), bitmap.height()), dst,
+                    fXformer->apply(paint), constraint);
         }
+
+        fTarget->drawImageRect(fXformer->apply(bitmap).get(),
+                               src ? *src : SkRect::MakeIWH(bitmap.width(), bitmap.height()), dst,
+                               fXformer->apply(paint), constraint);
     }
     void onDrawBitmapNine(const SkBitmap& bitmap,
                           const SkIRect& center, const SkRect& dst,
                           const SkPaint* paint) override {
-        if (auto image = SkImage::MakeFromBitmap(bitmap)) {
-            this->onDrawImageNine(image.get(), center, dst, paint);
+        if (this->skipXform(bitmap)) {
+            return fTarget->drawBitmapNine(bitmap, center, dst, fXformer->apply(paint));
         }
+
+        fTarget->drawImageNine(fXformer->apply(bitmap).get(), center, dst, fXformer->apply(paint));
+
     }
     void onDrawBitmapLattice(const SkBitmap& bitmap,
                              const Lattice& lattice, const SkRect& dst,
                              const SkPaint* paint) override {
-        if (auto image = SkImage::MakeFromBitmap(bitmap)) {
-            this->onDrawImageLattice(image.get(), lattice, dst, paint);
+        if (this->skipXform(bitmap)) {
+            return fTarget->drawBitmapLattice(bitmap, lattice, dst, fXformer->apply(paint));
         }
+
+
+        fTarget->drawImageLattice(fXformer->apply(bitmap).get(), lattice, dst,
+                                  fXformer->apply(paint));
     }
 
     // TODO: May not be ideal to unfurl pictures.
@@ -265,16 +282,24 @@ public:
     void onFlush() override { return fTarget->flush(); }
 
 private:
+    bool skipXform(const SkBitmap& bitmap) {
+        return (!bitmap.colorSpace() && fTargetCS->isSRGB()) ||
+               (SkColorSpace::Equals(bitmap.colorSpace(), fTargetCS.get())) ||
+               (kAlpha_8_SkColorType == bitmap.colorType());
+    }
+
     SkCanvas*                            fTarget;
+    sk_sp<SkColorSpace>                  fTargetCS;
     std::unique_ptr<SkColorSpaceXformer> fXformer;
 };
 
 std::unique_ptr<SkCanvas> SkCreateColorSpaceXformCanvas(SkCanvas* target,
                                                         sk_sp<SkColorSpace> targetCS) {
-    std::unique_ptr<SkColorSpaceXformer> xformer = SkColorSpaceXformer::Make(std::move(targetCS));
+    std::unique_ptr<SkColorSpaceXformer> xformer = SkColorSpaceXformer::Make(targetCS);
     if (!xformer) {
         return nullptr;
     }
 
-    return skstd::make_unique<SkColorSpaceXformCanvas>(target, std::move(xformer));
+    return skstd::make_unique<SkColorSpaceXformCanvas>(target, std::move(targetCS),
+                                                       std::move(xformer));
 }

diff --git a/src/core/SkColorSpaceXformer.cpp b/src/core/SkColorSpaceXformer.cpp
index e983756..4d93fae 100644 (file)
--- a/src/core/SkColorSpaceXformer.cpp
+++ b/src/core/SkColorSpaceXformer.cpp
@@ -11,6 +11,7 @@
 #include "SkDrawLooper.h"
 #include "SkGradientShader.h"
 #include "SkImage_Base.h"
+#include "SkImagePriv.h"
 #include "SkMakeUnique.h"
 
 std::unique_ptr<SkColorSpaceXformer> SkColorSpaceXformer::Make(sk_sp<SkColorSpace> dst) {
@@ -30,6 +31,18 @@ sk_sp<SkImage> SkColorSpaceXformer::apply(const SkImage* src) {
     return as_IB(src)->makeColorSpace(fDst);
 }
 
+sk_sp<SkImage> SkColorSpaceXformer::apply(const SkBitmap& src) {
+    sk_sp<SkImage> image = SkMakeImageFromRasterBitmap(src, kNever_SkCopyPixelsMode);
+    if (!image) {
+        return nullptr;
+    }
+
+    sk_sp<SkImage> xformed = as_IB(image)->makeColorSpace(fDst);
+    // We want to be sure we don't let the kNever_SkCopyPixelsMode image escape this stack frame.
+    SkASSERT(xformed != image);
+    return xformed;
+}
+
 void SkColorSpaceXformer::apply(SkColor* xformed, const SkColor* srgb, int n) {
     SkAssertResult(fFromSRGB->apply(SkColorSpaceXform::kBGRA_8888_ColorFormat, xformed,
                                     SkColorSpaceXform::kBGRA_8888_ColorFormat, srgb,

diff --git a/src/core/SkColorSpaceXformer.h b/src/core/SkColorSpaceXformer.h
index b89fd8a..020051b 100644 (file)
--- a/src/core/SkColorSpaceXformer.h
+++ b/src/core/SkColorSpaceXformer.h
@@ -17,6 +17,7 @@ public:
     static std::unique_ptr<SkColorSpaceXformer> Make(sk_sp<SkColorSpace> dst);
 
     sk_sp<SkImage> apply(const SkImage* src);
+    sk_sp<SkImage> apply(const SkBitmap& bitmap);
     const SkPaint* apply(const SkPaint* src);
     const SkPaint& apply(const SkPaint& src);
     void apply(SkColor dst[], const SkColor src[], int n);