Make the canvas draw looper setup update the canvas save count
Image filter in a paint would leave save count in wrong state
for normal draws. This could be observed through the canvas
references during the draw call. An example of this is
inspecting the canvas during a draw looper.
patch from issue 993863002 at patchset 20001 (http://crrev.com/993863002#ps20001)
BUG=skia:
TBR=kkinnunen@nvidia.com
Review URL: https://codereview.chromium.org/1034033004
diff --git a/gyp/tests.gypi b/gyp/tests.gypi
index 049b4b2..d358ac8 100644
--- a/gyp/tests.gypi
+++ b/gyp/tests.gypi
@@ -90,7 +90,8 @@
'../tests/DocumentTest.cpp',
'../tests/DrawBitmapRectTest.cpp',
'../tests/DrawPathTest.cpp',
- '../tests/DrawTextTest.cpp',
+ '../tests/DrawTextTest.cpp',
+ '../tests/DrawLooperTest.cpp',
'../tests/DynamicHashTest.cpp',
'../tests/EmptyPathTest.cpp',
'../tests/ErrorTest.cpp',
diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index 8426f09..ac909de 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -795,7 +795,6 @@
if (fMCStack.count() > 1) {
this->willRestore();
SkASSERT(fSaveCount > 1);
- fSaveCount -= 1;
this->internalRestore();
this->didRestore();
}
@@ -879,7 +878,6 @@
bounds = NULL;
}
SaveLayerStrategy strategy = this->willSaveLayer(bounds, paint, kARGB_ClipLayer_SaveFlag);
- fSaveCount += 1;
this->internalSaveLayer(bounds, paint, kARGB_ClipLayer_SaveFlag, strategy);
return this->getSaveCount() - 1;
}
@@ -889,7 +887,6 @@
bounds = NULL;
}
SaveLayerStrategy strategy = this->willSaveLayer(bounds, paint, flags);
- fSaveCount += 1;
this->internalSaveLayer(bounds, paint, flags, strategy);
return this->getSaveCount() - 1;
}
@@ -900,6 +897,8 @@
flags |= kClipToLayer_SaveFlag;
#endif
+ fSaveCount += 1;
+
// do this before we create the layer. We don't call the public save() since
// that would invoke a possibly overridden virtual
this->internalSave();
@@ -978,6 +977,8 @@
void SkCanvas::internalRestore() {
SkASSERT(fMCStack.count() != 0);
+ fSaveCount -= 1;
+
fDeviceCMDirty = true;
fCachedLocalClipBoundsDirty = true;
diff --git a/tests/DrawLooperTest.cpp b/tests/DrawLooperTest.cpp
new file mode 100644
index 0000000..77b4828
--- /dev/null
+++ b/tests/DrawLooperTest.cpp
@@ -0,0 +1,92 @@
+/*
+ * Copyright 2015 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "SkCanvas.h"
+#include "SkColorFilter.h"
+#include "SkColorFilterImageFilter.h"
+#include "SkDrawLooper.h"
+#include "Test.h"
+
+/* Tests for SkDrawLooper -related APIs and implementations. */
+
+namespace {
+/*
+ * Subclass that caused an assert at the time of writing.
+ */
+class GetSaveCountAssertLooper : public SkDrawLooper {
+public:
+
+ SkDrawLooper::Context* createContext(SkCanvas*, void* storage) const override {
+ return SkNEW_PLACEMENT(storage, GetSaveCountAssertLooperContext);
+ }
+
+ size_t contextSize() const override { return sizeof(GetSaveCountAssertLooperContext); }
+
+#ifndef SK_IGNORE_TO_STRING
+ void toString(SkString* str) const override {
+ str->append("GetSaveCountAssertLooper:");
+ }
+#endif
+
+ SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(GetSaveCountAssertLooper);
+
+private:
+ class GetSaveCountAssertLooperContext : public SkDrawLooper::Context {
+ public:
+ GetSaveCountAssertLooperContext() : fOnce(0) {}
+ bool next(SkCanvas* canvas, SkPaint* p) override {
+ // Getting the save count would assert in SkCanvas at the time of writing.
+ canvas->getSaveCount();
+
+ SkASSERT(p->getColor() == SK_ColorRED);
+ // Set the color green so the test knows payload was run. We use this color in order to
+ // try to express the expectation that Skia can not away the color filter. Due to
+ // non-pm-to-pm-to-non-pm conversions, this probably is not exactly correct.
+ p->setColor(SkColorSetARGB(255, 0, 254, 0));
+ return fOnce++ < 1;
+ }
+ private:
+ int fOnce;
+ };
+};
+
+SkFlattenable* GetSaveCountAssertLooper::CreateProc(SkReadBuffer&) {
+ return SkNEW(GetSaveCountAssertLooper);
+}
+
+}
+
+DEF_TEST(SkCanvas_GetSaveCountInDrawLooperAssert, reporter) {
+ SkBitmap dst;
+ dst.allocN32Pixels(10, 10);
+ dst.eraseColor(SK_ColorTRANSPARENT);
+
+ SkCanvas canvas(dst);
+ SkPaint paint;
+ {
+ SkAutoTUnref<SkColorFilter> addGreenCF(
+ SkColorFilter::CreateModeFilter(SkColorSetARGB(64, 0, 4, 0), SkXfermode::kPlus_Mode));
+ SkAutoTUnref<SkImageFilter> addGreenIF(
+ SkColorFilterImageFilter::Create(addGreenCF));
+
+ // This would trigger the assert upon a draw. It is a color filter that adds (roughly) 1 to
+ // the green component.
+ paint.setImageFilter(addGreenIF);
+
+ SkAutoTUnref<SkDrawLooper> looper(SkNEW(GetSaveCountAssertLooper));
+ paint.setLooper(looper);
+ }
+ paint.setColor(SK_ColorRED);
+ paint.setStrokeWidth(1);
+ paint.setStyle(SkPaint::kStroke_Style);
+ canvas.drawPoint(0, 0, paint);
+
+ uint32_t pixel = 0;
+ SkImageInfo info = SkImageInfo::Make(1, 1, kBGRA_8888_SkColorType, kUnpremul_SkAlphaType);
+ canvas.readPixels(info, &pixel, 4, 0, 0);
+ REPORTER_ASSERT(reporter, pixel == SK_ColorGREEN);
+}