From 8dc0ccb8d33d1af7dd13228509e61fe915bc7705 Mon Sep 17 00:00:00 2001 From: reed Date: Fri, 20 Mar 2015 06:32:52 -0700 Subject: [PATCH] disable LCD for layers w/ filters BUG=468311 see also skiabug.com/3567 Review URL: https://codereview.chromium.org/1002603003 --- gm/imagefiltersbase.cpp | 106 ++++++++++++++++++++++++++++++++++++---- src/core/SkCanvas.cpp | 7 +-- src/core/SkDevice.cpp | 3 ++ 3 files changed, 104 insertions(+), 12 deletions(-) diff --git a/gm/imagefiltersbase.cpp b/gm/imagefiltersbase.cpp index ea32769d4d..b21f83d6f1 100644 --- a/gm/imagefiltersbase.cpp +++ b/gm/imagefiltersbase.cpp @@ -36,8 +36,8 @@ public: protected: FailImageFilter() : INHERITED(0, NULL) {} - virtual bool onFilterImage(Proxy*, const SkBitmap& src, const Context&, - SkBitmap* result, SkIPoint* offset) const SK_OVERRIDE { + bool onFilterImage(Proxy*, const SkBitmap& src, const Context&, + SkBitmap* result, SkIPoint* offset) const SK_OVERRIDE { return false; } @@ -78,8 +78,8 @@ public: protected: IdentityImageFilter(SkImageFilter* input) : INHERITED(1, &input) {} - virtual bool onFilterImage(Proxy*, const SkBitmap& src, const Context&, - SkBitmap* result, SkIPoint* offset) const SK_OVERRIDE { + bool onFilterImage(Proxy*, const SkBitmap& src, const Context&, + SkBitmap* result, SkIPoint* offset) const SK_OVERRIDE { *result = src; offset->set(0, 0); return true; @@ -194,11 +194,11 @@ public: ImageFiltersBaseGM () {} protected: - virtual SkString onShortName() { + SkString onShortName() SK_OVERRIDE { return SkString("imagefiltersbase"); } - virtual SkISize onISize() { return SkISize::Make(700, 500); } + SkISize onISize() SK_OVERRIDE { return SkISize::Make(700, 500); } void draw_frame(SkCanvas* canvas, const SkRect& r) { SkPaint paint; @@ -207,7 +207,7 @@ protected: canvas->drawRect(r, paint); } - virtual void onDraw(SkCanvas* canvas) { + void onDraw(SkCanvas* canvas) SK_OVERRIDE { void (*drawProc[])(SkCanvas*, const SkRect&, SkImageFilter*) = { draw_paint, draw_line, draw_rect, draw_path, draw_text, @@ -254,8 +254,96 @@ protected: private: typedef GM INHERITED; }; +DEF_GM( return new ImageFiltersBaseGM; ) /////////////////////////////////////////////////////////////////////////////// -static skiagm::GM* MyFactory(void*) { return new ImageFiltersBaseGM; } -static skiagm::GMRegistry reg(MyFactory); +/* + * Want to test combos of filter and LCD text, to be sure we disable LCD in the presence of + * a filter. + */ +class ImageFiltersTextBaseGM : public skiagm::GM { + SkString fSuffix; +public: + ImageFiltersTextBaseGM(const char suffix[]) : fSuffix(suffix) {} + +protected: + SkString onShortName() SK_OVERRIDE { + SkString name; + name.printf("%s_%s", "textfilter", fSuffix.c_str()); + return name; + } + + SkISize onISize() SK_OVERRIDE { return SkISize::Make(512, 342); } + + void drawWaterfall(SkCanvas* canvas, const SkPaint& origPaint) { + const uint32_t flags[] = { + 0, + SkPaint::kAntiAlias_Flag, + SkPaint::kAntiAlias_Flag | SkPaint::kLCDRenderText_Flag, + }; + SkPaint paint(origPaint); + paint.setTextSize(30); + + SkAutoCanvasRestore acr(canvas, true); + for (size_t i = 0; i < SK_ARRAY_COUNT(flags); ++i) { + paint.setFlags(flags[i]); + canvas->drawText("Hamburgefons", 11, 0, 0, paint); + canvas->translate(0, 40); + } + } + + virtual void installFilter(SkPaint* paint) = 0; + + void onDraw(SkCanvas* canvas) SK_OVERRIDE { + SkPaint paint; + + canvas->translate(20, 40); + + for (int doSaveLayer = 0; doSaveLayer <= 1; ++doSaveLayer) { + SkAutoCanvasRestore acr(canvas, true); + for (int useFilter = 0; useFilter <= 1; ++useFilter) { + SkAutoCanvasRestore acr2(canvas, true); + + SkPaint paint; + if (useFilter) { + this->installFilter(&paint); + } + if (doSaveLayer) { + canvas->saveLayer(NULL, &paint); + paint.setImageFilter(NULL); + } + this->drawWaterfall(canvas, paint); + + acr2.restore(); + canvas->translate(250, 0); + } + acr.restore(); + canvas->translate(0, 200); + } + } + +private: + typedef GM INHERITED; +}; + +class ImageFiltersText_IF : public ImageFiltersTextBaseGM { +public: + ImageFiltersText_IF() : ImageFiltersTextBaseGM("image") {} + + void installFilter(SkPaint* paint) SK_OVERRIDE { + paint->setImageFilter(SkBlurImageFilter::Create(1.5f, 1.5f))->unref(); + } +}; +DEF_GM( return new ImageFiltersText_IF; ) + +class ImageFiltersText_CF : public ImageFiltersTextBaseGM { +public: + ImageFiltersText_CF() : ImageFiltersTextBaseGM("color") {} + + void installFilter(SkPaint* paint) SK_OVERRIDE { + paint->setColorFilter(SkColorFilter::CreateModeFilter(SK_ColorBLUE, SkXfermode::kSrcIn_Mode))->unref(); + } +}; +DEF_GM( return new ImageFiltersText_CF; ) + diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp index 11dc739f4c..8426f090ec 100644 --- a/src/core/SkCanvas.cpp +++ b/src/core/SkCanvas.cpp @@ -918,10 +918,12 @@ void SkCanvas::internalSaveLayer(const SkRect* bounds, const SkPaint* paint, Sav } bool isOpaque = !SkToBool(flags & kHasAlphaLayer_SaveFlag); - if (isOpaque && paint) { + SkPixelGeometry geo = fProps.pixelGeometry(); + if (paint) { // TODO: perhaps add a query to filters so we might preserve opaqueness... if (paint->getImageFilter() || paint->getColorFilter()) { isOpaque = false; + geo = kUnknown_SkPixelGeometry; } } SkImageInfo info = SkImageInfo::MakeN32(ir.width(), ir.height(), @@ -942,8 +944,7 @@ void SkCanvas::internalSaveLayer(const SkRect* bounds, const SkPaint* paint, Sav usage = SkBaseDevice::kPossible_TileUsage; } #endif - device = device->onCreateDevice(SkBaseDevice::CreateInfo(info, usage, fProps.pixelGeometry()), - paint); + device = device->onCreateDevice(SkBaseDevice::CreateInfo(info, usage, geo), paint); if (NULL == device) { SkErrorInternals::SetError( kInternalError_SkError, "Unable to create device for layer."); diff --git a/src/core/SkDevice.cpp b/src/core/SkDevice.cpp index d76a180862..6be9178f23 100644 --- a/src/core/SkDevice.cpp +++ b/src/core/SkDevice.cpp @@ -68,6 +68,9 @@ SkPixelGeometry SkBaseDevice::CreateInfo::AdjustGeometry(const SkImageInfo& info SkPixelGeometry geo) { switch (tileUsage) { case kPossible_TileUsage: + // (we think) for compatibility with old clients, we assume this layer can support LCD + // even though they may not have marked it as opaque... seems like we should update + // our callers (reed/robertphilips). break; case kNever_TileUsage: if (info.alphaType() != kOpaque_SkAlphaType) {