Make SkColorSpaceXformer::apply(SkPaint) safe to call recursively
Before, we would stomp on the original paint in the recursive call.
This fixes 4 gbr-8888 gms. Currently, this only affects loopers
and SkPaintImageFilter.
Bug: skia:6516
Change-Id: Ic47d637a912370c0a1ae8ef3282ad7d15d9902e3
Reviewed-on: https://skia-review.googlesource.com/14182
Commit-Queue: Matt Sarett <msarett@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>
diff --git a/src/core/SkColorSpaceXformCanvas.cpp b/src/core/SkColorSpaceXformCanvas.cpp
index 9486a4e..f026c6d 100644
--- a/src/core/SkColorSpaceXformCanvas.cpp
+++ b/src/core/SkColorSpaceXformCanvas.cpp
@@ -15,6 +15,18 @@
#include "SkMakeUnique.h"
#include "SkNoDrawCanvas.h"
#include "SkSurface.h"
+#include "SkTLazy.h"
+
+namespace {
+ struct MaybePaint {
+ SkTLazy<SkPaint> fStorage;
+ const SkPaint* fPaint = nullptr;
+ MaybePaint(const SkPaint* p, SkColorSpaceXformer* xformer) {
+ if (p) { fPaint = fStorage.set(xformer->apply(*p)); }
+ }
+ operator const SkPaint*() const { return fPaint; }
+ };
+};
class SkColorSpaceXformCanvas : public SkNoDrawCanvas {
public:
@@ -126,30 +138,26 @@
void onDrawImage(const SkImage* img,
SkScalar l, SkScalar t,
const SkPaint* paint) override {
- fTarget->drawImage(fXformer->apply(img).get(),
- l, t,
- fXformer->apply(paint));
+ fTarget->drawImage(fXformer->apply(img).get(), l, t, MaybePaint(paint, fXformer.get()));
}
void onDrawImageRect(const SkImage* img,
const SkRect* src, const SkRect& dst,
const SkPaint* paint, SrcRectConstraint constraint) override {
fTarget->drawImageRect(fXformer->apply(img).get(),
src ? *src : SkRect::MakeIWH(img->width(), img->height()), dst,
- fXformer->apply(paint), constraint);
+ MaybePaint(paint, fXformer.get()), constraint);
}
void onDrawImageNine(const SkImage* img,
const SkIRect& center, const SkRect& dst,
const SkPaint* paint) override {
- fTarget->drawImageNine(fXformer->apply(img).get(),
- center, dst,
- fXformer->apply(paint));
+ fTarget->drawImageNine(fXformer->apply(img).get(), center, dst,
+ MaybePaint(paint, fXformer.get()));
}
void onDrawImageLattice(const SkImage* img,
const Lattice& lattice, const SkRect& dst,
const SkPaint* paint) override {
- fTarget->drawImageLattice(fXformer->apply(img).get(),
- lattice, dst,
- fXformer->apply(paint));
+ fTarget->drawImageLattice(fXformer->apply(img).get(), lattice, dst,
+ MaybePaint(paint, fXformer.get()));
}
void onDrawAtlas(const SkImage* atlas, const SkRSXform* xforms, const SkRect* tex,
const SkColor* colors, int count, SkBlendMode mode,
@@ -160,19 +168,18 @@
fXformer->apply(xformed.begin(), colors, count);
colors = xformed.begin();
}
-
fTarget->drawAtlas(fXformer->apply(atlas).get(), xforms, tex, colors, count, mode, cull,
- fXformer->apply(paint));
+ MaybePaint(paint, fXformer.get()));
}
void onDrawBitmap(const SkBitmap& bitmap,
SkScalar l, SkScalar t,
const SkPaint* paint) override {
if (this->skipXform(bitmap)) {
- return fTarget->drawBitmap(bitmap, l, t, fXformer->apply(paint));
+ return fTarget->drawBitmap(bitmap, l, t, MaybePaint(paint, fXformer.get()));
}
- fTarget->drawImage(fXformer->apply(bitmap).get(), l, t, fXformer->apply(paint));
+ fTarget->drawImage(fXformer->apply(bitmap).get(), l, t, MaybePaint(paint, fXformer.get()));
}
void onDrawBitmapRect(const SkBitmap& bitmap,
const SkRect* src, const SkRect& dst,
@@ -180,39 +187,41 @@
if (this->skipXform(bitmap)) {
return fTarget->drawBitmapRect(bitmap,
src ? *src : SkRect::MakeIWH(bitmap.width(), bitmap.height()), dst,
- fXformer->apply(paint), constraint);
+ MaybePaint(paint, fXformer.get()), constraint);
}
fTarget->drawImageRect(fXformer->apply(bitmap).get(),
src ? *src : SkRect::MakeIWH(bitmap.width(), bitmap.height()), dst,
- fXformer->apply(paint), constraint);
+ MaybePaint(paint, fXformer.get()), constraint);
}
void onDrawBitmapNine(const SkBitmap& bitmap,
const SkIRect& center, const SkRect& dst,
const SkPaint* paint) override {
if (this->skipXform(bitmap)) {
- return fTarget->drawBitmapNine(bitmap, center, dst, fXformer->apply(paint));
+ return fTarget->drawBitmapNine(bitmap, center, dst, MaybePaint(paint, fXformer.get()));
}
- fTarget->drawImageNine(fXformer->apply(bitmap).get(), center, dst, fXformer->apply(paint));
+ fTarget->drawImageNine(fXformer->apply(bitmap).get(), center, dst,
+ MaybePaint(paint, fXformer.get()));
}
void onDrawBitmapLattice(const SkBitmap& bitmap,
const Lattice& lattice, const SkRect& dst,
const SkPaint* paint) override {
if (this->skipXform(bitmap)) {
- return fTarget->drawBitmapLattice(bitmap, lattice, dst, fXformer->apply(paint));
+ return fTarget->drawBitmapLattice(bitmap, lattice, dst,
+ MaybePaint(paint, fXformer.get()));
}
fTarget->drawImageLattice(fXformer->apply(bitmap).get(), lattice, dst,
- fXformer->apply(paint));
+ MaybePaint(paint, fXformer.get()));
}
void onDrawPicture(const SkPicture* pic,
const SkMatrix* matrix,
const SkPaint* paint) override {
- SkCanvas::onDrawPicture(pic, matrix, fXformer->apply(paint));
+ SkCanvas::onDrawPicture(pic, matrix, MaybePaint(paint, fXformer.get()));
}
void onDrawDrawable(SkDrawable* drawable, const SkMatrix* matrix) override {
SkCanvas::onDrawDrawable(drawable, matrix);
@@ -222,7 +231,7 @@
sk_sp<SkImageFilter> backdrop = rec.fBackdrop ? fXformer->apply(rec.fBackdrop) : nullptr;
fTarget->saveLayer({
rec.fBounds,
- fXformer->apply(rec.fPaint),
+ MaybePaint(rec.fPaint, fXformer.get()),
backdrop.get(),
rec.fSaveLayerFlags,
});
diff --git a/src/core/SkColorSpaceXformer.cpp b/src/core/SkColorSpaceXformer.cpp
index 0db303b..ce6fff6 100644
--- a/src/core/SkColorSpaceXformer.cpp
+++ b/src/core/SkColorSpaceXformer.cpp
@@ -79,52 +79,29 @@
return xformed;
}
-const SkPaint& SkColorSpaceXformer::apply(const SkPaint& src) {
- const SkPaint* result = &src;
- auto get_dst = [&] {
- if (result == &src) {
- fDstPaint = src;
- result = &fDstPaint;
- }
- return &fDstPaint;
- };
+SkPaint SkColorSpaceXformer::apply(const SkPaint& src) {
+ SkPaint dst = src;
// All SkColorSpaces have the same black point.
if (src.getColor() & 0xffffff) {
- get_dst()->setColor(this->apply(src.getColor()));
+ dst.setColor(this->apply(src.getColor()));
}
if (auto shader = src.getShader()) {
- auto replacement = shader->makeColorSpace(this);
- if (replacement.get() != shader) {
- get_dst()->setShader(std::move(replacement));
- }
+ dst.setShader(shader->makeColorSpace(this));
}
if (auto cf = src.getColorFilter()) {
- auto replacement = this->apply(cf);
- if (replacement.get() != cf) {
- get_dst()->setColorFilter(std::move(replacement));
- }
+ dst.setColorFilter(this->apply(cf));
}
if (auto looper = src.getDrawLooper()) {
- auto replacement = looper->makeColorSpace(this);
- if (replacement.get() != looper) {
- get_dst()->setDrawLooper(std::move(replacement));
- }
+ dst.setDrawLooper(looper->makeColorSpace(this));
}
if (auto imageFilter = src.getImageFilter()) {
- auto replacement = this->apply(imageFilter);
- if (replacement.get() != imageFilter) {
- get_dst()->setImageFilter(std::move(replacement));
- }
+ dst.setImageFilter(this->apply(imageFilter));
}
- return *result;
-}
-
-const SkPaint* SkColorSpaceXformer::apply(const SkPaint* src) {
- return src ? &this->apply(*src) : nullptr;
+ return dst;
}
diff --git a/src/core/SkColorSpaceXformer.h b/src/core/SkColorSpaceXformer.h
index d357743..914e839 100644
--- a/src/core/SkColorSpaceXformer.h
+++ b/src/core/SkColorSpaceXformer.h
@@ -20,8 +20,7 @@
sk_sp<SkImage> apply(const SkBitmap& bitmap);
sk_sp<SkColorFilter> apply(const SkColorFilter* filter);
sk_sp<SkImageFilter> apply(const SkImageFilter* filter);
- const SkPaint* apply(const SkPaint* src);
- const SkPaint& apply(const SkPaint& src);
+ SkPaint apply(const SkPaint& src);
void apply(SkColor dst[], const SkColor src[], int n);
SkColor apply(SkColor srgb);
@@ -32,7 +31,6 @@
sk_sp<SkColorSpace> fDst;
std::unique_ptr<SkColorSpaceXform> fFromSRGB;
- SkPaint fDstPaint;
};
#endif