From e6844834087cf9113c64fb638be3945049cf0f47 Mon Sep 17 00:00:00 2001 From: Matt Sarett Date: Mon, 3 Apr 2017 10:35:42 -0400 Subject: [PATCH] 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 Commit-Queue: Matt Sarett --- bench/ColorCanvasDrawBitmapBench.cpp | 57 ++++++++++++++++++++++++++++++++++++ gn/bench.gni | 1 + src/core/SkColorSpaceXformCanvas.cpp | 49 +++++++++++++++++++++++-------- src/core/SkColorSpaceXformer.cpp | 13 ++++++++ src/core/SkColorSpaceXformer.h | 1 + 5 files changed, 109 insertions(+), 12 deletions(-) create mode 100644 bench/ColorCanvasDrawBitmapBench.cpp diff --git a/bench/ColorCanvasDrawBitmapBench.cpp b/bench/ColorCanvasDrawBitmapBench.cpp new file mode 100644 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 src, sk_sp 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 colorCanvas = SkCreateColorSpaceXformCanvas(canvas, fDst); + for (int i = 0; i < n; i++) { + colorCanvas->drawBitmap(fBitmap, 0, 0, nullptr); + } + } + +private: + sk_sp 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 --- 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 --- 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 targetCS, std::unique_ptr 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 fTargetCS; std::unique_ptr fXformer; }; std::unique_ptr SkCreateColorSpaceXformCanvas(SkCanvas* target, sk_sp targetCS) { - std::unique_ptr xformer = SkColorSpaceXformer::Make(std::move(targetCS)); + std::unique_ptr xformer = SkColorSpaceXformer::Make(targetCS); if (!xformer) { return nullptr; } - return skstd::make_unique(target, std::move(xformer)); + return skstd::make_unique(target, std::move(targetCS), + std::move(xformer)); } diff --git a/src/core/SkColorSpaceXformer.cpp b/src/core/SkColorSpaceXformer.cpp index e983756..4d93fae 100644 --- 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::Make(sk_sp dst) { @@ -30,6 +31,18 @@ sk_sp SkColorSpaceXformer::apply(const SkImage* src) { return as_IB(src)->makeColorSpace(fDst); } +sk_sp SkColorSpaceXformer::apply(const SkBitmap& src) { + sk_sp image = SkMakeImageFromRasterBitmap(src, kNever_SkCopyPixelsMode); + if (!image) { + return nullptr; + } + + sk_sp 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 --- a/src/core/SkColorSpaceXformer.h +++ b/src/core/SkColorSpaceXformer.h @@ -17,6 +17,7 @@ public: static std::unique_ptr Make(sk_sp dst); sk_sp apply(const SkImage* src); + sk_sp 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); -- 2.7.4