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
Review URL: https://codereview.chromium.org/
1005173004
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;
///////////////////////////////////////////////////////////////////////////
// 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;
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()
};
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 {
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;
virtual void lockPixels() {}
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
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:
int x, int y,
const SkPaint& paint) SK_OVERRIDE;
- bool allowImageFilter(const SkImageFilter*) SK_OVERRIDE;
-
private:
class TypefaceUse : ::SkNoncopyable {
public:
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&);
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);
}
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;
}
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;
}
}
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
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);
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.");
}
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;
}
// anyway to document logically what is going on.
//
fLeakyProperties->setPixelGeometry(CreateInfo::AdjustGeometry(this->imageInfo(),
- kGeneral_Usage,
+ kPossible_TileUsage,
geo));
}
: 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);
"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) {
"Could not create canvas for layer.");
}
-bool SkXPSDevice::allowImageFilter(const SkImageFilter*) {
- return false;
-}
///////////////////////////////////////////////////////////////////////////////
-SkBaseDevice* SkGpuDevice::onCreateCompatibleDevice(const CreateInfo& cinfo) {
+SkBaseDevice* SkGpuDevice::onCreateDevice(const CreateInfo& cinfo, const SkPaint*) {
GrSurfaceDesc desc;
desc.fConfig = fRenderTarget->config();
desc.fFlags = kRenderTarget_GrSurfaceFlag;
// 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));
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);
}
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;
}
}
-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());
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);
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;
void lockPixels() SK_OVERRIDE {}
void unlockPixels() SK_OVERRIDE {}
- bool allowImageFilter(const SkImageFilter*) SK_OVERRIDE {
- return false;
- }
bool canHandleImageFilter(const SkImageFilter*) SK_OVERRIDE {
return false;
}
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) {