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
This commit is contained in:
reed 2015-03-26 13:40:09 -07:00 committed by Commit bot
parent fd3a91e1fc
commit a644116c33
3 changed files with 4 additions and 98 deletions

View File

@ -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',

View File

@ -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;

View File

@ -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);
}