From a644116c3375b12c642d1b51ee1e5cf4a22c1f5b Mon Sep 17 00:00:00 2001 From: reed Date: Thu, 26 Mar 2015 13:40:09 -0700 Subject: [PATCH] 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 --- gyp/tests.gypi | 3 +- src/core/SkCanvas.cpp | 7 ++- tests/DrawLooperTest.cpp | 92 ---------------------------------------- 3 files changed, 4 insertions(+), 98 deletions(-) delete mode 100644 tests/DrawLooperTest.cpp diff --git a/gyp/tests.gypi b/gyp/tests.gypi index d358ac8a1e..049b4b2361 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 ac909def6a..8426f090ec 100644 --- a/src/core/SkCanvas.cpp +++ b/src/core/SkCanvas.cpp @@ -795,6 +795,7 @@ void SkCanvas::restore() { if (fMCStack.count() > 1) { this->willRestore(); SkASSERT(fSaveCount > 1); + fSaveCount -= 1; this->internalRestore(); this->didRestore(); } @@ -878,6 +879,7 @@ int SkCanvas::saveLayer(const SkRect* bounds, const SkPaint* paint) { 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 @@ int SkCanvas::saveLayer(const SkRect* bounds, const SkPaint* paint, SaveFlags fl 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 @@ void SkCanvas::internalSaveLayer(const SkRect* bounds, const SkPaint* paint, Sav 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 @@ int SkCanvas::saveLayerAlpha(const SkRect* bounds, U8CPU alpha, 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 77b4828e6b..0000000000 --- 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 addGreenCF( - SkColorFilter::CreateModeFilter(SkColorSetARGB(64, 0, 4, 0), SkXfermode::kPlus_Mode)); - SkAutoTUnref 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 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); -}