Revert of Make the canvas draw looper setup update the canvas save count (patchset #1 id:1 of https://codereview.chromium.org/1034033004/)
Reason for revert:
makes internalSave and internalSaveLayer inconsistent. Need to find a different solution.
Original issue's description:
> 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
>
> Committed: https://skia.googlesource.com/skia/+/fd3a91e1fc4de69611b5297f624a1cd65db4ced1
TBR=kkinnunen@nvidia.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:
Review URL: https://codereview.chromium.org/1037653004
diff --git a/gyp/tests.gypi b/gyp/tests.gypi
index d358ac8..049b4b2 100644
--- a/gyp/tests.gypi
+++ b/gyp/tests.gypi
@@ -90,8 +90,7 @@
'../tests/DocumentTest.cpp',
'../tests/DrawBitmapRectTest.cpp',
'../tests/DrawPathTest.cpp',
- '../tests/DrawTextTest.cpp',
- '../tests/DrawLooperTest.cpp',
+ '../tests/DrawTextTest.cpp',
'../tests/DynamicHashTest.cpp',
'../tests/EmptyPathTest.cpp',
'../tests/ErrorTest.cpp',
diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index ac909de..8426f09 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -795,6 +795,7 @@
if (fMCStack.count() > 1) {
this->willRestore();
SkASSERT(fSaveCount > 1);
+ fSaveCount -= 1;
this->internalRestore();
this->didRestore();
}
@@ -878,6 +879,7 @@
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;
}
@@ -887,6 +889,7 @@
bounds = NULL;
}
SaveLayerStrategy strategy = this->willSaveLayer(bounds, paint, flags);
+ fSaveCount += 1;
this->internalSaveLayer(bounds, paint, flags, strategy);
return this->getSaveCount() - 1;
}
@@ -897,8 +900,6 @@
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();
@@ -977,8 +978,6 @@
void SkCanvas::internalRestore() {
SkASSERT(fMCStack.count() != 0);
- fSaveCount -= 1;
-
fDeviceCMDirty = true;
fCachedLocalClipBoundsDirty = true;
diff --git a/tests/DrawLooperTest.cpp b/tests/DrawLooperTest.cpp
deleted file mode 100644
index 77b4828..0000000
--- a/tests/DrawLooperTest.cpp
+++ /dev/null
@@ -1,92 +0,0 @@
-/*
- * 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);
-}