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
Review URL: https://codereview.chromium.org/1006923002
diff --git a/include/core/SkBitmapDevice.h b/include/core/SkBitmapDevice.h
index 8ca6a52..caff685 100644
--- a/include/core/SkBitmapDevice.h
+++ b/include/core/SkBitmapDevice.h
@@ -98,11 +98,7 @@
const SkColor colors[], SkXfermode* xmode,
const uint16_t indices[], int indexCount,
const SkPaint& paint) SK_OVERRIDE;
- /** The SkBaseDevice passed will be an SkBaseDevice which was returned by a call to
- onCreateCompatibleDevice on this device with kSaveLayer_Usage.
- */
- virtual void drawDevice(const SkDraw&, SkBaseDevice*, int x, int y,
- const SkPaint&) SK_OVERRIDE;
+ virtual void drawDevice(const SkDraw&, SkBaseDevice*, int x, int y, const SkPaint&) SK_OVERRIDE;
///////////////////////////////////////////////////////////////////////////
@@ -145,7 +141,7 @@
// any clip information.
void replaceBitmapBackendForRasterSurface(const SkBitmap&) SK_OVERRIDE;
- SkBaseDevice* onCreateCompatibleDevice(const CreateInfo&) SK_OVERRIDE;
+ SkBaseDevice* onCreateDevice(const CreateInfo&, const SkPaint*) SK_OVERRIDE;
SkSurface* newSurface(const SkImageInfo&, const SkSurfaceProps&) SK_OVERRIDE;
const void* peekPixels(SkImageInfo*, size_t* rowBytes) SK_OVERRIDE;
diff --git a/include/core/SkCanvas.h b/include/core/SkCanvas.h
index 74bf909c..836f7b1 100644
--- a/include/core/SkCanvas.h
+++ b/include/core/SkCanvas.h
@@ -1321,8 +1321,7 @@
void internalDrawBitmapNine(const SkBitmap& bitmap, const SkIRect& center,
const SkRect& dst, const SkPaint* paint);
void internalDrawPaint(const SkPaint& paint);
- void internalSaveLayer(const SkRect* bounds, const SkPaint* paint,
- SaveFlags, bool justForImageFilter, SaveLayerStrategy strategy);
+ void internalSaveLayer(const SkRect* bounds, const SkPaint*, SaveFlags, SaveLayerStrategy);
void internalDrawDevice(SkBaseDevice*, int x, int y, const SkPaint*);
// shared by save() and saveLayer()
diff --git a/include/core/SkDevice.h b/include/core/SkDevice.h
index c7c5c30..684a084 100644
--- a/include/core/SkDevice.h
+++ b/include/core/SkDevice.h
@@ -125,10 +125,9 @@
};
protected:
- enum Usage {
- kGeneral_Usage,
- kSaveLayer_Usage, // <! internal use only
- kImageFilter_Usage // <! internal use only
+ enum TileUsage {
+ kPossible_TileUsage, //!< the created device may be drawn tiled
+ kNever_TileUsage, //!< the created device will never be drawn tiled
};
struct TextFlags {
@@ -231,7 +230,7 @@
virtual void drawPatch(const SkDraw&, const SkPoint cubics[12], const SkColor colors[4],
const SkPoint texCoords[4], SkXfermode* xmode, const SkPaint& paint);
/** The SkDevice passed will be an SkDevice which was returned by a call to
- onCreateCompatibleDevice on this device with kSaveLayer_Usage.
+ onCreateDevice on this device with kNeverTile_TileExpectation.
*/
virtual void drawDevice(const SkDraw&, SkBaseDevice*, int x, int y,
const SkPaint&) = 0;
@@ -255,14 +254,6 @@
virtual void unlockPixels() {}
/**
- * Returns true if the device allows processing of this imagefilter. If
- * false is returned, then the filter is ignored. This may happen for
- * some subclasses that do not support pixel manipulations after drawing
- * has occurred (e.g. printing). The default implementation returns true.
- */
- virtual bool allowImageFilter(const SkImageFilter*) { return true; }
-
- /**
* Override and return true for filters that the device can handle
* intrinsically. Doing so means that SkCanvas will pass-through this
* filter to drawSprite and drawDevice (and potentially filterImage).
@@ -336,24 +327,39 @@
const SkPaint*);
struct CreateInfo {
- static SkPixelGeometry AdjustGeometry(const SkImageInfo&, Usage, SkPixelGeometry geo);
+ static SkPixelGeometry AdjustGeometry(const SkImageInfo&, TileUsage, SkPixelGeometry);
- // The construct may change the pixel geometry based on usage as needed.
- CreateInfo(const SkImageInfo& info, Usage usage, SkPixelGeometry geo)
+ // The constructor may change the pixel geometry based on other parameters.
+ CreateInfo(const SkImageInfo& info, TileUsage tileUsage, SkPixelGeometry geo)
: fInfo(info)
- , fUsage(usage)
- , fPixelGeometry(AdjustGeometry(info, usage, geo))
+ , fTileUsage(tileUsage)
+ , fPixelGeometry(AdjustGeometry(info, tileUsage, geo))
{}
- const SkImageInfo fInfo;
- const Usage fUsage;
- const SkPixelGeometry fPixelGeometry;
+ const SkImageInfo fInfo;
+ const TileUsage fTileUsage;
+ const SkPixelGeometry fPixelGeometry;
};
+#ifdef SK_SUPPORT_LEGACY_ONCREATECOMPATIBLEDEVICE
+ // legacy method name -- please override onCreateDevice instead
virtual SkBaseDevice* onCreateCompatibleDevice(const CreateInfo&) {
return NULL;
}
+ virtual SkBaseDevice* onCreateDevice(const CreateInfo& cinfo, const SkPaint* layerPaint) {
+ return this->onCreateCompatibleDevice(cinfo);
+ }
+#else
+ /**
+ * Create a new device based on CreateInfo. If the paint is not null, then it represents a
+ * preview of how the new device will be composed with its creator device (this).
+ */
+ virtual SkBaseDevice* onCreateDevice(const CreateInfo&, const SkPaint*) {
+ return NULL;
+ }
+#endif
+
virtual void initForRootLayer(SkPixelGeometry geo);
private:
diff --git a/include/device/xps/SkXPSDevice.h b/include/device/xps/SkXPSDevice.h
index 514259d..10dab81 100644
--- a/include/device/xps/SkXPSDevice.h
+++ b/include/device/xps/SkXPSDevice.h
@@ -135,8 +135,6 @@
int x, int y,
const SkPaint& paint) SK_OVERRIDE;
- bool allowImageFilter(const SkImageFilter*) SK_OVERRIDE;
-
private:
class TypefaceUse : ::SkNoncopyable {
public:
@@ -311,7 +309,7 @@
const SkVector& ppuScale,
IXpsOMPath* shadedPath);
- SkBaseDevice* onCreateCompatibleDevice(const CreateInfo&) SK_OVERRIDE;
+ SkBaseDevice* onCreateDevice(const CreateInfo&, const SkPaint*) SK_OVERRIDE;
// Disable the default copy and assign implementation.
SkXPSDevice(const SkXPSDevice&);
diff --git a/src/core/SkBitmapDevice.cpp b/src/core/SkBitmapDevice.cpp
index 37cbff5..5fae2b5 100644
--- a/src/core/SkBitmapDevice.cpp
+++ b/src/core/SkBitmapDevice.cpp
@@ -110,7 +110,7 @@
fBitmap.lockPixels();
}
-SkBaseDevice* SkBitmapDevice::onCreateCompatibleDevice(const CreateInfo& cinfo) {
+SkBaseDevice* SkBitmapDevice::onCreateDevice(const CreateInfo& cinfo, const SkPaint*) {
SkDeviceProperties leaky(cinfo.fPixelGeometry);
return SkBitmapDevice::Create(cinfo.fInfo, &leaky);
}
diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index a31ded0..11dc739 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -299,7 +299,7 @@
SkPaint tmp;
tmp.setImageFilter(fOrigPaint.getImageFilter());
(void)canvas->internalSaveLayer(bounds, &tmp, SkCanvas::kARGB_ClipLayer_SaveFlag,
- true, SkCanvas::kFullLayer_SaveLayerStrategy);
+ 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 @@
}
SaveLayerStrategy strategy = this->willSaveLayer(bounds, paint, kARGB_ClipLayer_SaveFlag);
fSaveCount += 1;
- this->internalSaveLayer(bounds, paint, kARGB_ClipLayer_SaveFlag, false, strategy);
+ this->internalSaveLayer(bounds, paint, kARGB_ClipLayer_SaveFlag, strategy);
return this->getSaveCount() - 1;
}
@@ -890,12 +890,12 @@
}
SaveLayerStrategy strategy = this->willSaveLayer(bounds, paint, flags);
fSaveCount += 1;
- this->internalSaveLayer(bounds, paint, flags, false, strategy);
+ this->internalSaveLayer(bounds, paint, flags, strategy);
return this->getSaveCount() - 1;
}
void SkCanvas::internalSaveLayer(const SkRect* bounds, const SkPaint* paint, SaveFlags flags,
- bool justForImageFilter, SaveLayerStrategy strategy) {
+ SaveLayerStrategy strategy) {
#ifndef SK_SUPPORT_LEGACY_CLIPTOLAYERFLAG
flags |= kClipToLayer_SaveFlag;
#endif
@@ -917,21 +917,13 @@
return;
}
- // 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);
+ if (isOpaque && paint) {
+ // TODO: perhaps add a query to filters so we might preserve opaqueness...
+ if (paint->getImageFilter() || paint->getColorFilter()) {
+ isOpaque = false;
}
}
-
- bool isOpaque = !SkToBool(flags & kHasAlphaLayer_SaveFlag);
SkImageInfo info = SkImageInfo::MakeN32(ir.width(), ir.height(),
isOpaque ? kOpaque_SkAlphaType : kPremul_SkAlphaType);
@@ -941,12 +933,17 @@
return;
}
- SkBaseDevice::Usage usage = SkBaseDevice::kSaveLayer_Usage;
+ 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 ?
+ //
if (paint && paint->getImageFilter()) {
- usage = SkBaseDevice::kImageFilter_Usage;
+ usage = SkBaseDevice::kPossible_TileUsage;
}
- device = device->onCreateCompatibleDevice(SkBaseDevice::CreateInfo(info, usage,
- fProps.pixelGeometry()));
+#endif
+ device = device->onCreateDevice(SkBaseDevice::CreateInfo(info, usage, fProps.pixelGeometry()),
+ 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 a77e54c..d76a180 100644
--- a/src/core/SkDevice.cpp
+++ b/src/core/SkDevice.cpp
@@ -64,19 +64,16 @@
}
SkPixelGeometry SkBaseDevice::CreateInfo::AdjustGeometry(const SkImageInfo& info,
- Usage usage,
+ TileUsage tileUsage,
SkPixelGeometry geo) {
- switch (usage) {
- case kGeneral_Usage:
+ switch (tileUsage) {
+ case kPossible_TileUsage:
break;
- case kSaveLayer_Usage:
+ case kNever_TileUsage:
if (info.alphaType() != kOpaque_SkAlphaType) {
geo = kUnknown_SkPixelGeometry;
}
break;
- case kImageFilter_Usage:
- geo = kUnknown_SkPixelGeometry;
- break;
}
return geo;
}
@@ -86,7 +83,7 @@
// anyway to document logically what is going on.
//
fLeakyProperties->setPixelGeometry(CreateInfo::AdjustGeometry(this->imageInfo(),
- kGeneral_Usage,
+ kPossible_TileUsage,
geo));
}
diff --git a/src/core/SkDeviceImageFilterProxy.h b/src/core/SkDeviceImageFilterProxy.h
index 0ae686d..11a95ff 100644
--- a/src/core/SkDeviceImageFilterProxy.h
+++ b/src/core/SkDeviceImageFilterProxy.h
@@ -18,15 +18,15 @@
: fDevice(device)
, fProps(props.flags(),
SkBaseDevice::CreateInfo::AdjustGeometry(SkImageInfo(),
- SkBaseDevice::kImageFilter_Usage,
+ SkBaseDevice::kPossible_TileUsage,
props.pixelGeometry()))
{}
SkBaseDevice* createDevice(int w, int h) SK_OVERRIDE {
SkBaseDevice::CreateInfo cinfo(SkImageInfo::MakeN32Premul(w, h),
- SkBaseDevice::kImageFilter_Usage,
+ SkBaseDevice::kPossible_TileUsage,
kUnknown_SkPixelGeometry);
- return fDevice->onCreateCompatibleDevice(cinfo);
+ return fDevice->onCreateDevice(cinfo, NULL);
}
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 2ca799a..88d1182 100644
--- a/src/device/xps/SkXPSDevice.cpp
+++ b/src/device/xps/SkXPSDevice.cpp
@@ -2251,7 +2251,7 @@
"Could not add layer to current visuals.");
}
-SkBaseDevice* SkXPSDevice::onCreateCompatibleDevice(const CreateInfo& info) {
+SkBaseDevice* SkXPSDevice::onCreateDevice(const CreateInfo& info, const SkPaint*) {
//Conditional for bug compatibility with PDF device.
#if 0
if (SkBaseDevice::kGeneral_Usage == info.fUsage) {
@@ -2282,6 +2282,3 @@
"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 fb5fa7c..757dfbf 100644
--- a/src/gpu/SkGpuDevice.cpp
+++ b/src/gpu/SkGpuDevice.cpp
@@ -1862,7 +1862,7 @@
///////////////////////////////////////////////////////////////////////////////
-SkBaseDevice* SkGpuDevice::onCreateCompatibleDevice(const CreateInfo& cinfo) {
+SkBaseDevice* SkGpuDevice::onCreateDevice(const CreateInfo& cinfo, const SkPaint*) {
GrSurfaceDesc desc;
desc.fConfig = fRenderTarget->config();
desc.fFlags = kRenderTarget_GrSurfaceFlag;
@@ -1876,7 +1876,7 @@
// layers are never draw in repeat modes, so we can request an approx
// match and ignore any padding.
- const GrContext::ScratchTexMatch match = (kSaveLayer_Usage == cinfo.fUsage) ?
+ const GrContext::ScratchTexMatch match = (kNever_TileUsage == cinfo.fTileUsage) ?
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 6b520e7..50aa586 100644
--- a/src/gpu/SkGpuDevice.h
+++ b/src/gpu/SkGpuDevice.h
@@ -51,9 +51,9 @@
virtual ~SkGpuDevice();
SkGpuDevice* cloneDevice(const SkSurfaceProps& props) {
- SkBaseDevice* dev = this->onCreateCompatibleDevice(CreateInfo(this->imageInfo(),
- kGeneral_Usage,
- props.pixelGeometry()));
+ SkBaseDevice* dev = this->onCreateDevice(CreateInfo(this->imageInfo(), kPossible_TileUsage,
+ props.pixelGeometry()),
+ NULL);
return static_cast<SkGpuDevice*>(dev);
}
@@ -147,7 +147,7 @@
SkGpuDevice(GrRenderTarget*, const SkSurfaceProps*, unsigned flags);
- SkBaseDevice* onCreateCompatibleDevice(const CreateInfo&) SK_OVERRIDE;
+ SkBaseDevice* onCreateDevice(const CreateInfo&, const SkPaint*) SK_OVERRIDE;
SkSurface* newSurface(const SkImageInfo&, const SkSurfaceProps&) SK_OVERRIDE;
diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp
index 99f3ce1..273b958 100644
--- a/src/pdf/SkPDFDevice.cpp
+++ b/src/pdf/SkPDFDevice.cpp
@@ -566,13 +566,19 @@
}
}
-SkBaseDevice* SkPDFDevice::onCreateCompatibleDevice(const CreateInfo& cinfo) {
+static bool not_supported_for_layers(const SkPaint& layerPaint) {
// 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)
- if (kImageFilter_Usage == cinfo.fUsage) {
+
+ // 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)) {
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 8a88314..a90ea11 100644
--- a/src/pdf/SkPDFDevice.h
+++ b/src/pdf/SkPDFDevice.h
@@ -235,8 +235,7 @@
ContentEntry* getLastContentEntry();
void setLastContentEntry(ContentEntry* contentEntry);
- // override from SkBaseDevice
- SkBaseDevice* onCreateCompatibleDevice(const CreateInfo&) SK_OVERRIDE;
+ SkBaseDevice* onCreateDevice(const CreateInfo&, const SkPaint*) SK_OVERRIDE;
void init();
void cleanUp(bool clearFontUsage);
diff --git a/src/utils/SkDeferredCanvas.cpp b/src/utils/SkDeferredCanvas.cpp
index 8fe9f8a..647105d 100644
--- a/src/utils/SkDeferredCanvas.cpp
+++ b/src/utils/SkDeferredCanvas.cpp
@@ -162,7 +162,7 @@
GrRenderTarget* accessRenderTarget() SK_OVERRIDE;
- SkBaseDevice* onCreateCompatibleDevice(const CreateInfo&) SK_OVERRIDE;
+ SkBaseDevice* onCreateDevice(const CreateInfo&, const SkPaint*) SK_OVERRIDE;
SkSurface* newSurface(const SkImageInfo&, const SkSurfaceProps&) SK_OVERRIDE;
@@ -231,9 +231,6 @@
void lockPixels() SK_OVERRIDE {}
void unlockPixels() SK_OVERRIDE {}
- bool allowImageFilter(const SkImageFilter*) SK_OVERRIDE {
- return false;
- }
bool canHandleImageFilter(const SkImageFilter*) SK_OVERRIDE {
return false;
}
@@ -461,16 +458,13 @@
return immediateDevice()->accessBitmap(false);
}
-SkBaseDevice* SkDeferredDevice::onCreateCompatibleDevice(const CreateInfo& cinfo) {
- // Save layer usage not supported, and not required by SkDeferredCanvas.
- SkASSERT(cinfo.fUsage != kSaveLayer_Usage);
-
+SkBaseDevice* SkDeferredDevice::onCreateDevice(const CreateInfo& cinfo, const SkPaint* layerPaint) {
// 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()->onCreateCompatibleDevice(cinfo);
+ return immediateDevice()->onCreateDevice(cinfo, layerPaint);
}
SkSurface* SkDeferredDevice::newSurface(const SkImageInfo& info, const SkSurfaceProps& props) {