From 173e5fe5f4be28272246e5676f5d2e5c4b1f9167 Mon Sep 17 00:00:00 2001 From: reed Date: Fri, 13 Mar 2015 20:05:18 -0700 Subject: Revert of Revert of Revert of Revert of Revert of Change device creation to see the (optional) layer-paint (patchset #1 id:1 of https://codereview.chromium.org/1005173004/) Reason for revert: arrrg. this is a staging nightmare. override required on the chrome side. must revert (again) Original issue's description: > Revert of Revert of Revert of Revert of Change device creation to see the (optional) layer-paint (patchset #1 id:1 of https://codereview.chromium.org/1001423002/) > > Reason for revert: > chrome now has the new virtual, so trying again > > Original issue's description: > > Revert of Revert of Revert of Change device creation to see the (optional) layer-paint (patchset #1 id:1 of https://codereview.chromium.org/1006923002/) > > > > Reason for revert: > > platform_canvas tests failures > > > > skia_unittests (with patch) skia_unittests (with patch) PlatformCanvas.TranslateLayer failed 2 > > Flakiness dashboard > > > > failures: > > PlatformCanvas.TranslateLayer > > PlatformCanvas.FillLayer > > > > Original issue's description: > > > Revert of Revert of Change device creation to see the (optional) layer-paint (patchset #1 id:1 of https://codereview.chromium.org/1008863002/) > > > > > > Reason for revert: > > > guard in chrome has landed > > > > > > Original issue's description: > > > > Revert of Change device creation to see the (optional) layer-paint (patchset #9 id:160001 of https://codereview.chromium.org/988413003/) > > > > > > > > Reason for revert: > > > > need to have chrome opt-in for the older API before this can land (in chrome) > > > > > > > > Original issue's description: > > > > > Change device creation to see the (optional) layer-paint > > > > > > > > > > Motivation: > > > > > > > > > > PDFDevice currently relies on 1) being told that the layer's paint has an imagefilter, and in the case, it creates a rasterdevice. It then relies on (2) canvas itself sniffing the layer's paint and offering to apply-the-imagefilter to call drawSprite instead of drawDevice. > > > > > > > > > > This subtle interchange is fragile, and also does not support other unsupported PDF features like colorfilters. This CL is a step toward making this use-raster-instead-of-native approach to layers more completely in the subclass' hands. > > > > > > > > > > Committed: https://skia.googlesource.com/skia/+/1182d9a96b80bd12183ee7c81325a979a51ee0c0 > > > > > > > > TBR=halcanary@google.com,senorblanco@google.com,robertphillips@google.com > > > > NOPRESUBMIT=true > > > > NOTREECHECKS=true > > > > NOTRY=true > > > > > > > > Committed: https://skia.googlesource.com/skia/+/0e040f7da2fdfeb49aa60d24117306e3b1e6ea90 > > > > > > TBR=halcanary@google.com,senorblanco@google.com,robertphillips@google.com > > > NOPRESUBMIT=true > > > NOTREECHECKS=true > > > NOTRY=true > > > > > > Committed: https://skia.googlesource.com/skia/+/f7076a13e2d4269903b34ef2780e1c84723e4477 > > > > TBR=halcanary@google.com,senorblanco@google.com,robertphillips@google.com > > NOPRESUBMIT=true > > NOTREECHECKS=true > > NOTRY=true > > > > Committed: https://skia.googlesource.com/skia/+/8e14d660b2a434bc708a70180c84210883611683 > > TBR=halcanary@google.com,senorblanco@google.com,robertphillips@google.com,reed@google.com > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > > Committed: https://skia.googlesource.com/skia/+/307d1ed129ff75eb64137dea75df858f9e250b69 TBR=halcanary@google.com,senorblanco@google.com,robertphillips@google.com,reed@google.com NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Review URL: https://codereview.chromium.org/1005183003 --- src/core/SkBitmapDevice.cpp | 2 +- src/core/SkCanvas.cpp | 39 ++++++++++++++++++++----------------- src/core/SkDevice.cpp | 13 ++++++++----- src/core/SkDeviceImageFilterProxy.h | 6 +++--- src/device/xps/SkXPSDevice.cpp | 5 ++++- src/gpu/SkGpuDevice.cpp | 4 ++-- src/gpu/SkGpuDevice.h | 8 ++++---- src/pdf/SkPDFDevice.cpp | 10 ++-------- src/pdf/SkPDFDevice.h | 3 ++- src/utils/SkDeferredCanvas.cpp | 12 +++++++++--- 10 files changed, 56 insertions(+), 46 deletions(-) (limited to 'src') diff --git a/src/core/SkBitmapDevice.cpp b/src/core/SkBitmapDevice.cpp index 5fae2b5862..37cbff50e2 100644 --- a/src/core/SkBitmapDevice.cpp +++ b/src/core/SkBitmapDevice.cpp @@ -110,7 +110,7 @@ void SkBitmapDevice::replaceBitmapBackendForRasterSurface(const SkBitmap& bm) { fBitmap.lockPixels(); } -SkBaseDevice* SkBitmapDevice::onCreateDevice(const CreateInfo& cinfo, const SkPaint*) { +SkBaseDevice* SkBitmapDevice::onCreateCompatibleDevice(const CreateInfo& cinfo) { SkDeviceProperties leaky(cinfo.fPixelGeometry); return SkBitmapDevice::Create(cinfo.fInfo, &leaky); } diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp index 11dc739f4c..a31ded0586 100644 --- a/src/core/SkCanvas.cpp +++ b/src/core/SkCanvas.cpp @@ -299,7 +299,7 @@ public: SkPaint tmp; tmp.setImageFilter(fOrigPaint.getImageFilter()); (void)canvas->internalSaveLayer(bounds, &tmp, SkCanvas::kARGB_ClipLayer_SaveFlag, - SkCanvas::kFullLayer_SaveLayerStrategy); + true, SkCanvas::kFullLayer_SaveLayerStrategy); // we'll clear the imageFilter for the actual draws in next(), so // it will only be applied during the restore(). fDoClearImageFilter = true; @@ -880,7 +880,7 @@ int SkCanvas::saveLayer(const SkRect* bounds, const SkPaint* paint) { } SaveLayerStrategy strategy = this->willSaveLayer(bounds, paint, kARGB_ClipLayer_SaveFlag); fSaveCount += 1; - this->internalSaveLayer(bounds, paint, kARGB_ClipLayer_SaveFlag, strategy); + this->internalSaveLayer(bounds, paint, kARGB_ClipLayer_SaveFlag, false, strategy); return this->getSaveCount() - 1; } @@ -890,12 +890,12 @@ int SkCanvas::saveLayer(const SkRect* bounds, const SkPaint* paint, SaveFlags fl } SaveLayerStrategy strategy = this->willSaveLayer(bounds, paint, flags); fSaveCount += 1; - this->internalSaveLayer(bounds, paint, flags, strategy); + this->internalSaveLayer(bounds, paint, flags, false, strategy); return this->getSaveCount() - 1; } void SkCanvas::internalSaveLayer(const SkRect* bounds, const SkPaint* paint, SaveFlags flags, - SaveLayerStrategy strategy) { + bool justForImageFilter, SaveLayerStrategy strategy) { #ifndef SK_SUPPORT_LEGACY_CLIPTOLAYERFLAG flags |= kClipToLayer_SaveFlag; #endif @@ -917,13 +917,21 @@ void SkCanvas::internalSaveLayer(const SkRect* bounds, const SkPaint* paint, Sav return; } - bool isOpaque = !SkToBool(flags & kHasAlphaLayer_SaveFlag); - if (isOpaque && paint) { - // TODO: perhaps add a query to filters so we might preserve opaqueness... - if (paint->getImageFilter() || paint->getColorFilter()) { - isOpaque = false; + // Kill the imagefilter if our device doesn't allow it + SkLazyPaint lazyP; + if (paint && paint->getImageFilter()) { + if (!this->getTopDevice()->allowImageFilter(paint->getImageFilter())) { + if (justForImageFilter) { + // early exit if the layer was just for the imageFilter + return; + } + SkPaint* p = lazyP.set(*paint); + p->setImageFilter(NULL); + paint = p; } } + + bool isOpaque = !SkToBool(flags & kHasAlphaLayer_SaveFlag); SkImageInfo info = SkImageInfo::MakeN32(ir.width(), ir.height(), isOpaque ? kOpaque_SkAlphaType : kPremul_SkAlphaType); @@ -933,17 +941,12 @@ void SkCanvas::internalSaveLayer(const SkRect* bounds, const SkPaint* paint, Sav return; } - SkBaseDevice::TileUsage usage = SkBaseDevice::kNever_TileUsage; -#if 1 - // this seems needed for current GMs, but makes us draw slower on the GPU - // Related to https://code.google.com/p/skia/issues/detail?id=3519 ? - // + SkBaseDevice::Usage usage = SkBaseDevice::kSaveLayer_Usage; if (paint && paint->getImageFilter()) { - usage = SkBaseDevice::kPossible_TileUsage; + usage = SkBaseDevice::kImageFilter_Usage; } -#endif - device = device->onCreateDevice(SkBaseDevice::CreateInfo(info, usage, fProps.pixelGeometry()), - paint); + device = device->onCreateCompatibleDevice(SkBaseDevice::CreateInfo(info, usage, + fProps.pixelGeometry())); 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..a77e54c2ff 100644 --- a/src/core/SkDevice.cpp +++ b/src/core/SkDevice.cpp @@ -64,16 +64,19 @@ const SkBitmap& SkBaseDevice::accessBitmap(bool changePixels) { } SkPixelGeometry SkBaseDevice::CreateInfo::AdjustGeometry(const SkImageInfo& info, - TileUsage tileUsage, + Usage usage, SkPixelGeometry geo) { - switch (tileUsage) { - case kPossible_TileUsage: + switch (usage) { + case kGeneral_Usage: break; - case kNever_TileUsage: + case kSaveLayer_Usage: if (info.alphaType() != kOpaque_SkAlphaType) { geo = kUnknown_SkPixelGeometry; } break; + case kImageFilter_Usage: + geo = kUnknown_SkPixelGeometry; + break; } return geo; } @@ -83,7 +86,7 @@ void SkBaseDevice::initForRootLayer(SkPixelGeometry geo) { // anyway to document logically what is going on. // fLeakyProperties->setPixelGeometry(CreateInfo::AdjustGeometry(this->imageInfo(), - kPossible_TileUsage, + kGeneral_Usage, geo)); } diff --git a/src/core/SkDeviceImageFilterProxy.h b/src/core/SkDeviceImageFilterProxy.h index 11a95ff31f..0ae686d877 100644 --- a/src/core/SkDeviceImageFilterProxy.h +++ b/src/core/SkDeviceImageFilterProxy.h @@ -18,15 +18,15 @@ public: : fDevice(device) , fProps(props.flags(), SkBaseDevice::CreateInfo::AdjustGeometry(SkImageInfo(), - SkBaseDevice::kPossible_TileUsage, + SkBaseDevice::kImageFilter_Usage, props.pixelGeometry())) {} SkBaseDevice* createDevice(int w, int h) SK_OVERRIDE { SkBaseDevice::CreateInfo cinfo(SkImageInfo::MakeN32Premul(w, h), - SkBaseDevice::kPossible_TileUsage, + SkBaseDevice::kImageFilter_Usage, kUnknown_SkPixelGeometry); - return fDevice->onCreateDevice(cinfo, NULL); + return fDevice->onCreateCompatibleDevice(cinfo); } bool canHandleImageFilter(const SkImageFilter* filter) SK_OVERRIDE { return fDevice->canHandleImageFilter(filter); diff --git a/src/device/xps/SkXPSDevice.cpp b/src/device/xps/SkXPSDevice.cpp index 88d1182526..2ca799ac37 100644 --- a/src/device/xps/SkXPSDevice.cpp +++ b/src/device/xps/SkXPSDevice.cpp @@ -2251,7 +2251,7 @@ void SkXPSDevice::drawDevice(const SkDraw& d, SkBaseDevice* dev, "Could not add layer to current visuals."); } -SkBaseDevice* SkXPSDevice::onCreateDevice(const CreateInfo& info, const SkPaint*) { +SkBaseDevice* SkXPSDevice::onCreateCompatibleDevice(const CreateInfo& info) { //Conditional for bug compatibility with PDF device. #if 0 if (SkBaseDevice::kGeneral_Usage == info.fUsage) { @@ -2282,3 +2282,6 @@ SkXPSDevice::SkXPSDevice(IXpsOMObjectFactory* xpsFactory) "Could not create canvas for layer."); } +bool SkXPSDevice::allowImageFilter(const SkImageFilter*) { + return false; +} diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp index 757dfbf521..fb5fa7cc7a 100644 --- a/src/gpu/SkGpuDevice.cpp +++ b/src/gpu/SkGpuDevice.cpp @@ -1862,7 +1862,7 @@ void SkGpuDevice::flush() { /////////////////////////////////////////////////////////////////////////////// -SkBaseDevice* SkGpuDevice::onCreateDevice(const CreateInfo& cinfo, const SkPaint*) { +SkBaseDevice* SkGpuDevice::onCreateCompatibleDevice(const CreateInfo& cinfo) { GrSurfaceDesc desc; desc.fConfig = fRenderTarget->config(); desc.fFlags = kRenderTarget_GrSurfaceFlag; @@ -1876,7 +1876,7 @@ SkBaseDevice* SkGpuDevice::onCreateDevice(const CreateInfo& cinfo, const SkPaint // layers are never draw in repeat modes, so we can request an approx // match and ignore any padding. - const GrContext::ScratchTexMatch match = (kNever_TileUsage == cinfo.fTileUsage) ? + const GrContext::ScratchTexMatch match = (kSaveLayer_Usage == cinfo.fUsage) ? GrContext::kApprox_ScratchTexMatch : GrContext::kExact_ScratchTexMatch; texture.reset(fContext->refScratchTexture(desc, match)); diff --git a/src/gpu/SkGpuDevice.h b/src/gpu/SkGpuDevice.h index 50aa58685f..6b520e7174 100644 --- a/src/gpu/SkGpuDevice.h +++ b/src/gpu/SkGpuDevice.h @@ -51,9 +51,9 @@ public: virtual ~SkGpuDevice(); SkGpuDevice* cloneDevice(const SkSurfaceProps& props) { - SkBaseDevice* dev = this->onCreateDevice(CreateInfo(this->imageInfo(), kPossible_TileUsage, - props.pixelGeometry()), - NULL); + SkBaseDevice* dev = this->onCreateCompatibleDevice(CreateInfo(this->imageInfo(), + kGeneral_Usage, + props.pixelGeometry())); return static_cast(dev); } @@ -147,7 +147,7 @@ private: SkGpuDevice(GrRenderTarget*, const SkSurfaceProps*, unsigned flags); - SkBaseDevice* onCreateDevice(const CreateInfo&, const SkPaint*) SK_OVERRIDE; + SkBaseDevice* onCreateCompatibleDevice(const CreateInfo&) SK_OVERRIDE; SkSurface* newSurface(const SkImageInfo&, const SkSurfaceProps&) SK_OVERRIDE; diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp index 273b958a8c..99f3ce1188 100644 --- a/src/pdf/SkPDFDevice.cpp +++ b/src/pdf/SkPDFDevice.cpp @@ -566,19 +566,13 @@ void GraphicStackState::updateDrawingState(const GraphicStateEntry& state) { } } -static bool not_supported_for_layers(const SkPaint& layerPaint) { +SkBaseDevice* SkPDFDevice::onCreateCompatibleDevice(const CreateInfo& cinfo) { // PDF does not support image filters, so render them on CPU. // Note that this rendering is done at "screen" resolution (100dpi), not // printer resolution. // FIXME: It may be possible to express some filters natively using PDF // to improve quality and file size (http://skbug.com/3043) - - // TODO: should we return true if there is a colorfilter? - return layerPaint.getImageFilter() != NULL; -} - -SkBaseDevice* SkPDFDevice::onCreateDevice(const CreateInfo& cinfo, const SkPaint* layerPaint) { - if (layerPaint && not_supported_for_layers(*layerPaint)) { + if (kImageFilter_Usage == cinfo.fUsage) { return SkBitmapDevice::Create(cinfo.fInfo); } SkISize size = SkISize::Make(cinfo.fInfo.width(), cinfo.fInfo.height()); diff --git a/src/pdf/SkPDFDevice.h b/src/pdf/SkPDFDevice.h index a90ea115c1..8a88314009 100644 --- a/src/pdf/SkPDFDevice.h +++ b/src/pdf/SkPDFDevice.h @@ -235,7 +235,8 @@ private: ContentEntry* getLastContentEntry(); void setLastContentEntry(ContentEntry* contentEntry); - SkBaseDevice* onCreateDevice(const CreateInfo&, const SkPaint*) SK_OVERRIDE; + // override from SkBaseDevice + SkBaseDevice* onCreateCompatibleDevice(const CreateInfo&) SK_OVERRIDE; void init(); void cleanUp(bool clearFontUsage); diff --git a/src/utils/SkDeferredCanvas.cpp b/src/utils/SkDeferredCanvas.cpp index 647105d757..8fe9f8a4b5 100644 --- a/src/utils/SkDeferredCanvas.cpp +++ b/src/utils/SkDeferredCanvas.cpp @@ -162,7 +162,7 @@ public: GrRenderTarget* accessRenderTarget() SK_OVERRIDE; - SkBaseDevice* onCreateDevice(const CreateInfo&, const SkPaint*) SK_OVERRIDE; + SkBaseDevice* onCreateCompatibleDevice(const CreateInfo&) SK_OVERRIDE; SkSurface* newSurface(const SkImageInfo&, const SkSurfaceProps&) SK_OVERRIDE; @@ -231,6 +231,9 @@ protected: void lockPixels() SK_OVERRIDE {} void unlockPixels() SK_OVERRIDE {} + bool allowImageFilter(const SkImageFilter*) SK_OVERRIDE { + return false; + } bool canHandleImageFilter(const SkImageFilter*) SK_OVERRIDE { return false; } @@ -458,13 +461,16 @@ const SkBitmap& SkDeferredDevice::onAccessBitmap() { return immediateDevice()->accessBitmap(false); } -SkBaseDevice* SkDeferredDevice::onCreateDevice(const CreateInfo& cinfo, const SkPaint* layerPaint) { +SkBaseDevice* SkDeferredDevice::onCreateCompatibleDevice(const CreateInfo& cinfo) { + // Save layer usage not supported, and not required by SkDeferredCanvas. + SkASSERT(cinfo.fUsage != kSaveLayer_Usage); + // Create a compatible non-deferred device. // We do not create a deferred device because we know the new device // will not be used with a deferred canvas (there is no API for that). // And connecting a SkDeferredDevice to non-deferred canvas can result // in unpredictable behavior. - return immediateDevice()->onCreateDevice(cinfo, layerPaint); + return immediateDevice()->onCreateCompatibleDevice(cinfo); } SkSurface* SkDeferredDevice::newSurface(const SkImageInfo& info, const SkSurfaceProps& props) { -- cgit v1.2.3