diff --git a/src/core/SkBitmapDevice.cpp b/src/core/SkBitmapDevice.cpp index fd2d1aa0e7..09105878ef 100644 --- a/src/core/SkBitmapDevice.cpp +++ b/src/core/SkBitmapDevice.cpp @@ -628,7 +628,7 @@ void SkBitmapDevice::drawSpecial(SkSpecialImage* src, int x, int y, const SkPain SkImageFilter_Base::Context ctx(matrix, clipBounds, cache.get(), fBitmap.colorType(), fBitmap.colorSpace(), src); - filteredImage = as_IFB(filter)->filterImage(ctx, &offset); + filteredImage = as_IFB(filter)->filterImage(ctx).imageAndOffset(&offset); if (!filteredImage) { return; } diff --git a/src/core/SkImageFilter.cpp b/src/core/SkImageFilter.cpp index 36e9db9151..4a76e5d545 100644 --- a/src/core/SkImageFilter.cpp +++ b/src/core/SkImageFilter.cpp @@ -129,6 +129,8 @@ sk_sp SkImageFilter::makeWithLocalMatrix(const SkMatrix& matrix) // SkImageFilter_Base /////////////////////////////////////////////////////////////////////////////////////////////////// +SK_USE_FLUENT_IMAGE_FILTER_TYPES + static int32_t next_image_filter_unique_id() { static std::atomic nextID{1}; @@ -199,41 +201,54 @@ void SkImageFilter_Base::flatten(SkWriteBuffer& buffer) const { buffer.writeUInt(fCropRect.flags()); } -sk_sp SkImageFilter_Base::filterImage(const Context& context, - SkIPoint* offset) const { - SkASSERT(offset); +skif::FilterResult SkImageFilter_Base::filterImage(const skif::Context& context) const { + // TODO (michaelludwig) - Old filters have an implicit assumption that the source image + // (originally passed separately) has an origin of (0, 0). SkComposeImageFilter makes an effort + // to ensure that remains the case. Once everyone uses the new type systems for bounds, non + // (0, 0) source origins will be easy to support. + SkASSERT(context.source().origin().fX == 0 && context.source().origin().fY == 0); + + skif::FilterResult result; if (!context.isValid()) { - return nullptr; + return result; } uint32_t srcGenID = fUsesSrcInput ? context.sourceImage()->uniqueID() : 0; const SkIRect srcSubset = fUsesSrcInput ? context.sourceImage()->subset() : SkIRect::MakeWH(0, 0); - SkImageFilterCacheKey key(fUniqueID, context.ctm(), context.clipBounds(), srcGenID, srcSubset); - if (context.cache()) { - sk_sp result = context.cache()->get(key, offset); - if (result) { - return result; - } + + SkImageFilterCacheKey key(fUniqueID, context.layerMatrix(), context.clipBounds(), srcGenID, + srcSubset); + if (context.cache() && context.cache()->get(key, &result)) { + return result; } - sk_sp result(this->onFilterImage(context, offset)); + result = this->onFilterImage(context); #if SK_SUPPORT_GPU - if (context.gpuBacked() && result && !result->isTextureBacked()) { + if (context.gpuBacked() && result.image() && !result.image()->isTextureBacked()) { // Keep the result on the GPU - this is still required for some // image filters that don't support GPU in all cases - result = result->makeTextureImage(context.getContext()); + auto asTexture = result.image()->makeTextureImage(context.getContext()); + result = skif::FilterResult(std::move(asTexture), result.origin()); } #endif - if (result && context.cache()) { - context.cache()->set(key, result.get(), *offset, this); + if (context.cache()) { + context.cache()->set(key, this, result); } return result; } +// TODO (michaelludwig) - Default to using the old onFilterImage, as filters are updated one by one. +// Once the old function is gone, this onFilterImage() will be made a pure virtual. +skif::FilterResult SkImageFilter_Base::onFilterImage(const skif::Context& context) const { + SkIPoint origin; + auto image = this->onFilterImage(context, &origin); + return skif::FilterResult(std::move(image), origin); +} + bool SkImageFilter_Base::canHandleComplexCTM() const { // CropRects need to apply in the source coordinate system, but are not aware of complex CTMs // when performing clipping. For a simple fix, any filter with a crop rect set cannot support @@ -380,20 +395,29 @@ SkIRect SkImageFilter_Base::onFilterNodeBounds(const SkIRect& src, const SkMatri return src; } -sk_sp SkImageFilter_Base::filterInput(int index, - const Context& ctx, - SkIPoint* offset) const { +template +skif::FilterResult SkImageFilter_Base::filterInput(int index, const skif::Context& ctx) const { + // Sanity checks for the index-specific input usages + SkASSERT(kU != skif::Usage::kInput0 || index == 0); + SkASSERT(kU != skif::Usage::kInput1 || index == 1); + const SkImageFilter* input = this->getInput(index); if (!input) { - return sk_ref_sp(ctx.sourceImage()); + // Convert from the generic kInput of the source image to kU + return static_cast>(ctx.source()); } - sk_sp result(as_IFB(input)->filterImage(this->mapContext(ctx), offset)); + skif::FilterResult result = as_IFB(input)->filterImage(this->mapContext(ctx)); + SkASSERT(!result.image() || ctx.gpuBacked() == result.image()->isTextureBacked()); - SkASSERT(!result || ctx.gpuBacked() == result->isTextureBacked()); - - return result; + // Map the output result of the input image filter to the input usage requested for this filter + return static_cast>(std::move(result)); } +// Instantiate filterInput() for kInput, kInput0, and kInput1. This does not provide a definition +// for kOutput, which should never be used anyways, and this way the linker will fail for us then. +template skif::FilterResult SkImageFilter_Base::filterInput(int, const skif::Context&) const; +template skif::FilterResult SkImageFilter_Base::filterInput(int, const skif::Context&) const; +template skif::FilterResult SkImageFilter_Base::filterInput(int, const skif::Context&) const; SkImageFilter_Base::Context SkImageFilter_Base::mapContext(const Context& ctx) const { SkIRect clipBounds = this->onFilterNodeBounds(ctx.clipBounds(), ctx.ctm(), diff --git a/src/core/SkImageFilterCache.cpp b/src/core/SkImageFilterCache.cpp index 7cdc595f60..c92338b457 100644 --- a/src/core/SkImageFilterCache.cpp +++ b/src/core/SkImageFilterCache.cpp @@ -41,12 +41,12 @@ public: } } struct Value { - Value(const Key& key, SkSpecialImage* image, const SkIPoint& offset, const SkImageFilter* filter) - : fKey(key), fImage(SkRef(image)), fOffset(offset), fFilter(filter) {} + Value(const Key& key, const skif::FilterResult& image, + const SkImageFilter* filter) + : fKey(key), fImage(image), fFilter(filter) {} Key fKey; - sk_sp fImage; - SkIPoint fOffset; + skif::FilterResult fImage; const SkImageFilter* fFilter; static const Key& GetKey(const Value& v) { return v.fKey; @@ -57,28 +57,32 @@ public: SK_DECLARE_INTERNAL_LLIST_INTERFACE(Value); }; - sk_sp get(const Key& key, SkIPoint* offset) const override { + bool get(const Key& key, skif::FilterResult* result) const override { + SkASSERT(result); + SkAutoMutexExclusive mutex(fMutex); if (Value* v = fLookup.find(key)) { - *offset = v->fOffset; if (v != fLRU.head()) { fLRU.remove(v); fLRU.addToHead(v); } - return v->fImage; + + *result = v->fImage; + return true; } - return nullptr; + return false; } - void set(const Key& key, SkSpecialImage* image, const SkIPoint& offset, const SkImageFilter* filter) override { + void set(const Key& key, const SkImageFilter* filter, + const skif::FilterResult& result) override { SkAutoMutexExclusive mutex(fMutex); if (Value* v = fLookup.find(key)) { this->removeInternal(v); } - Value* v = new Value(key, image, offset, filter); + Value* v = new Value(key, result, filter); fLookup.add(v); fLRU.addToHead(v); - fCurrentBytes += image->getSize(); + fCurrentBytes += result.image() ? result.image()->getSize() : 0; if (auto* values = fImageFilterValues.find(filter)) { values->push_back(v); } else { @@ -122,7 +126,6 @@ public: SkDEBUGCODE(int count() const override { return fLookup.count(); }) private: void removeInternal(Value* v) { - SkASSERT(v->fImage); if (v->fFilter) { if (auto* values = fImageFilterValues.find(v->fFilter)) { if (values->size() == 1 && (*values)[0] == v) { @@ -137,7 +140,7 @@ private: } } } - fCurrentBytes -= v->fImage->getSize(); + fCurrentBytes -= v->fImage.image() ? v->fImage.image()->getSize() : 0; fLRU.remove(v); fLookup.remove(v->fKey); delete v; diff --git a/src/core/SkImageFilterCache.h b/src/core/SkImageFilterCache.h index 11d8f7ed06..b82f4daf29 100644 --- a/src/core/SkImageFilterCache.h +++ b/src/core/SkImageFilterCache.h @@ -10,10 +10,10 @@ #include "include/core/SkMatrix.h" #include "include/core/SkRefCnt.h" +#include "src/core/SkImageFilterTypes.h" struct SkIPoint; class SkImageFilter; -class SkSpecialImage; struct SkImageFilterCacheKey { SkImageFilterCacheKey(const uint32_t uniqueID, const SkMatrix& matrix, @@ -46,18 +46,27 @@ struct SkImageFilterCacheKey { } }; -// This cache maps from (filter's unique ID + CTM + clipBounds + src bitmap generation ID) to -// (result, offset). +// This cache maps from (filter's unique ID + CTM + clipBounds + src bitmap generation ID) to result +// NOTE: this is the _specific_ unique ID of the image filter, so refiltering the same image with a +// copy of the image filter (with exactly the same parameters) will not yield a cache hit. class SkImageFilterCache : public SkRefCnt { public: + SK_USE_FLUENT_IMAGE_FILTER_TYPES_IN_CLASS + enum { kDefaultTransientSize = 32 * 1024 * 1024 }; virtual ~SkImageFilterCache() {} static SkImageFilterCache* Create(size_t maxBytes); static SkImageFilterCache* Get(); - virtual sk_sp get(const SkImageFilterCacheKey& key, SkIPoint* offset) const = 0; - virtual void set(const SkImageFilterCacheKey& key, SkSpecialImage* image, - const SkIPoint& offset, const SkImageFilter* filter) = 0; + + // Returns true on cache hit and updates 'result' to be the cached result. Returns false when + // not in the cache, in which case 'result' is not modified. + virtual bool get(const SkImageFilterCacheKey& key, + skif::FilterResult* result) const = 0; + // 'filter' is included in the caching to allow the purging of all of an image filter's cached + // results when it is destroyed. + virtual void set(const SkImageFilterCacheKey& key, const SkImageFilter* filter, + const skif::FilterResult& result) = 0; virtual void purge() = 0; virtual void purgeByImageFilter(const SkImageFilter*) = 0; SkDEBUGCODE(virtual int count() const = 0;) diff --git a/src/core/SkImageFilterTypes.h b/src/core/SkImageFilterTypes.h index 049f232e40..85cf77591f 100644 --- a/src/core/SkImageFilterTypes.h +++ b/src/core/SkImageFilterTypes.h @@ -23,13 +23,141 @@ class SkSurfaceProps; // strongly encouraged. namespace skif { +// Usage is a template tag to improve the readability of filter implementations. It is attached to +// images and geometry to group a collection of related variables and ensure that moving from one +// use case to another is explicit. +// NOTE: This can be aliased as 'For' when using the fluent type names. +enum class Usage { + // Designates the semantic purpose of the bounds, coordinate, or image as being an input + // to the image filter calculations. When this usage is used, it denotes a generic input, + // such as the current input in a dynamic loop, or some aggregate of all inputs. Because + // most image filters consume 1 or 2 filters only, the related kInput0 and kInput1 are + // also defined. + kInput, + // A more specific version of kInput, this marks the tagged variable as attached to the + // image filter of SkImageFilter_Base::getInput(0). + kInput0, + // A more specific version of kInput, this marks the tagged variable as attached to the + // image filter of SkImageFilter_Base::getInput(1). + kInput1, + // Designates the purpose of the bounds, coordinate, or image as being the output of the + // current image filter calculation. There is only ever one output for an image filter. + kOutput, +}; + +// Convenience macros to add 'using' declarations that rename the above enums to provide a more +// fluent and readable API. This should only be used in a private or local scope to prevent leakage +// of the names. Use the IN_CLASS variant at the start of a class declaration in those scenarios. +// These macros enable the following simpler type names: +// skif::Image -> Image +#define SK_USE_FLUENT_IMAGE_FILTER_TYPES \ + using For = skif::Usage; + +#define SK_USE_FLUENT_IMAGE_FILTER_TYPES_IN_CLASS \ + protected: SK_USE_FLUENT_IMAGE_FILTER_TYPES public: + +// Wraps an SkSpecialImage and tags it with a corresponding usage, either as generic input (e.g. the +// source image), or a specific input image from a filter's connected inputs. It also includes the +// origin of the image in the layer space. This origin is used to draw the image in the correct +// location. The 'layerBounds' rectangle of the filtered image is the layer-space bounding box of +// the image. It has its top left corner at 'origin' and has the same dimensions as the underlying +// special image subset. Transforming 'layerBounds' by the Context's layer matrix and painting it +// with the subset rectangle will display the filtered results in the appropriate device-space +// region. +// +// When filter implementations are processing intermediate FilterResult results, it can be assumed +// that all FilterResult' layerBounds are in the same layer coordinate space defined by the shared +// skif::Context. +// +// NOTE: This is named FilterResult since most instances will represent the output of an image +// filter (even if that is then cast to be the input to the next filter). The main exception is the +// source input used when an input filter is null, but from a data-standpoint it is the same since +// it is equivalent to the result of an identity filter. +template +class FilterResult { +public: + FilterResult() : fImage(nullptr), fOrigin({0, 0}) {} + + FilterResult(sk_sp image, const SkIPoint& origin) + : fImage(std::move(image)) + , fOrigin(origin) {} + + // Allow explicit moves/copies in order to cast from one use type to another, except kInput0 + // and kInput1 can only be cast to kOutput (e.g. as part of a noop image filter). + template + explicit FilterResult(FilterResult&& image) + : fImage(std::move(image.fImage)) + , fOrigin(std::move(image.fOrigin)) { + static_assert((kU != Usage::kInput) || (kI != Usage::kInput0 && kI != Usage::kInput1), + "kInput0 and kInput1 cannot be moved to more generic kInput usage."); + static_assert((kU != Usage::kInput0 && kU != Usage::kInput1) || + (kI == kU || kI == Usage::kInput || kI == Usage::kOutput), + "Can only move to specific input from the generic kInput or kOutput usage."); + } + + template + explicit FilterResult(const FilterResult& image) + : fImage(image.fImage) + , fOrigin(image.fOrigin) { + static_assert((kU != Usage::kInput) || (kI != Usage::kInput0 && kI != Usage::kInput1), + "kInput0 and kInput1 cannot be copied to more generic kInput usage."); + static_assert((kU != Usage::kInput0 && kU != Usage::kInput1) || + (kI == kU || kI == Usage::kInput || kI == Usage::kOutput), + "Can only copy to specific input from the generic kInput usage."); + } + + const SkSpecialImage* image() const { return fImage.get(); } + sk_sp refImage() const { return fImage; } + + // TODO (michaelludwig) - the geometry types will be updated to tag the coordinate system + // associated with each one. + + // Get the subset bounds of this image within its backing proxy. This will have the same + // dimensions as the image. + const SkIRect& subset() const { return fImage->subset(); } + + // Get the layer-space bounds of this image. This will have the same dimensions as the + // image and its top left corner will be 'origin()'. + const SkIRect& layerBounds() const { + return SkIRect::MakeXYWH(fOrigin.x(), fOrigin.y(), fImage->width(), fImage->height()); + } + + // Get the layer-space coordinate of this image's top left pixel. + const SkIPoint& origin() const { return fOrigin; } + + // Extract image and origin, safely when the image is null. + // TODO (michaelludwig) - This is intended for convenience until all call sites of + // SkImageFilter_Base::filterImage() have been updated to work in the new type system + // (which comes later as SkDevice, SkCanvas, etc. need to be modified, and coordinate space + // tagging needs to be added). + sk_sp imageAndOffset(SkIPoint* offset) const { + if (fImage) { + *offset = fOrigin; + return fImage; + } else { + *offset = {0, 0}; + return nullptr; + } + } + +private: + // Allow all FilterResult templates access to each others members + template + friend class FilterResult; + + sk_sp fImage; + SkIPoint fOrigin; +}; + // The context contains all necessary information to describe how the image filter should be -// computed (i.e. the current layer matrix and clip), and the color information of the output of -// a filter DAG. For now, this is just the color space (of the original requesting device). This -// is used when constructing intermediate rendering surfaces, so that we ensure we land in a -// surface that's similar/compatible to the final consumer of the DAG's output. +// computed (i.e. the current layer matrix and clip), and the color information of the output of a +// filter DAG. For now, this is just the color space (of the original requesting device). This is +// used when constructing intermediate rendering surfaces, so that we ensure we land in a surface +// that's similar/compatible to the final consumer of the DAG's output. class Context { public: + SK_USE_FLUENT_IMAGE_FILTER_TYPES_IN_CLASS + // Creates a context with the given layer matrix and destination clip, reading from 'source' // with an origin of (0,0). Context(const SkMatrix& layerMatrix, const SkIRect& clipBounds, SkImageFilterCache* cache, @@ -39,6 +167,16 @@ public: , fCache(cache) , fColorType(colorType) , fColorSpace(colorSpace) + , fSource(sk_ref_sp(source), {0, 0}) {} + + Context(const SkMatrix& layerMatrix, const SkIRect& clipBounds, SkImageFilterCache* cache, + SkColorType colorType, SkColorSpace* colorSpace, + const FilterResult& source) + : fLayerMatrix(layerMatrix) + , fClipBounds(clipBounds) + , fCache(cache) + , fColorType(colorType) + , fColorSpace(colorSpace) , fSource(source) {} // The transformation from the local parameter space of the filters to the layer space where @@ -68,19 +206,21 @@ public: SkColorSpace* colorSpace() const { return fColorSpace; } sk_sp refColorSpace() const { return sk_ref_sp(fColorSpace); } // The default surface properties to use when making transient surfaces during filtering. - const SkSurfaceProps* surfaceProps() const { return &fSource->props(); } + const SkSurfaceProps* surfaceProps() const { return &fSource.image()->props(); } // This is the image to use whenever an expected input filter has been set to null. In the // majority of cases, this is the original source image for the image filter DAG so it comes // from the SkDevice that holds either the saveLayer or the temporary rendered result. The // exception is composing two image filters (via SkImageFilters::Compose), which must use // the output of the inner DAG as the "source" for the outer DAG. - const SkSpecialImage* sourceImage() const { return fSource; } + const FilterResult& source() const { return fSource; } + // DEPRECATED: Use source() instead to get both the image and its origin. + const SkSpecialImage* sourceImage() const { return fSource.image(); } // True if image filtering should occur on the GPU if possible. - bool gpuBacked() const { return fSource->isTextureBacked(); } + bool gpuBacked() const { return fSource.image()->isTextureBacked(); } // The recording context to use when computing the filter with the GPU. - GrRecordingContext* getContext() const { return fSource->getContext(); } + GrRecordingContext* getContext() const { return fSource.image()->getContext(); } /** * Since a context can be built directly, its constructor has no chance to "return null" if @@ -90,14 +230,15 @@ public: * The SkImageFilterCache Key, for example, requires a finite ctm (no infinities or NaN), * so that test is part of isValid. */ - bool isValid() const { return fSource && fLayerMatrix.isFinite(); } + bool isValid() const { return fSource.image() != nullptr && fLayerMatrix.isFinite(); } // Create a surface of the given size, that matches the context's color type and color space // as closely as possible, and uses the same backend of the device that produced the source // image. sk_sp makeSurface(const SkISize& size, const SkSurfaceProps* props = nullptr) const { - return fSource->makeSurface(fColorType, fColorSpace, size, kPremul_SkAlphaType, props); + return fSource.image()->makeSurface(fColorType, fColorSpace, size, + kPremul_SkAlphaType, props); } // Create a new context that matches this context, but with an overridden layer CTM matrix. @@ -114,10 +255,10 @@ private: SkIRect fClipBounds; SkImageFilterCache* fCache; SkColorType fColorType; - // These pointers are owned by the device controlling the filter process, and our - // lifetime is bounded by the device, so they can be bare pointers. + // The pointed-to object is owned by the device controlling the filter process, and our lifetime + // is bounded by the device, so this can be a bare pointer. SkColorSpace* fColorSpace; - const SkSpecialImage* fSource; + FilterResult fSource; }; } // end namespace skif diff --git a/src/core/SkImageFilter_Base.h b/src/core/SkImageFilter_Base.h index 98eb2d6885..f04d694a0b 100644 --- a/src/core/SkImageFilter_Base.h +++ b/src/core/SkImageFilter_Base.h @@ -22,26 +22,23 @@ class GrRecordingContext; // actual API surface that Skia will use to compute the filtered images. class SkImageFilter_Base : public SkImageFilter { public: + SK_USE_FLUENT_IMAGE_FILTER_TYPES_IN_CLASS + // DEPRECATED - Use skif::Context directly. using Context = skif::Context; /** - * Request a new filtered image to be created from the src image. + * Request a new filtered image to be created from the src image. The returned skif::Image + * provides both the pixel data and the origin point that it should be drawn at, relative to + * the layer space defined by the provided context. * - * The context contains the environment in which the filter is occurring. - * It includes the clip bounds, CTM and cache. - * - * Offset is the amount to translate the resulting image relative to the - * src when it is drawn. This is an out-param. - * - * If the result image cannot be created, or the result would be - * transparent black, return null, in which case the offset parameter - * should be ignored by the caller. + * If the result image cannot be created, or the result would be transparent black, returns + * a skif::Image that has a null special image, in which its origin should be ignored. * * TODO: Right now the imagefilters sometimes return empty result bitmaps/ * specialimages. That doesn't seem quite right. */ - sk_sp filterImage(const skif::Context& context, SkIPoint* offset) const; + skif::FilterResult filterImage(const skif::Context& context) const; /** * Returns whether any edges of the crop rect have been set. The crop @@ -128,28 +125,7 @@ protected: virtual bool affectsTransparentBlack() const { return false; } - /** - * This is the virtual which should be overridden by the derived class - * to perform image filtering. - * - * src is the original primitive bitmap. If the filter has a connected - * input, it should recurse on that input and use that in place of src. - * - * The matrix is the matrix used to draw the geometry into the current - * layer that produced the 'src' image. This may be the total canvas' - * matrix, or part of its decomposition (depending on what the filter DAG - * is able to support). - * - * Offset is the amount to translate the resulting image relative to the - * src when it is drawn. This is an out-param. - * - * If the result image cannot be created (either because of error or if, say, the result - * is entirely clipped out), this should return nullptr. - * Callers that affect transparent black should explicitly handle nullptr - * results and press on. In the error case this behavior will produce a better result - * than nothing and is necessary for the clipped out case. - * If the return value is nullptr then offset should be ignored. - */ + // DEPRECATED - Use the private context-only variant virtual sk_sp onFilterImage(const Context&, SkIPoint* offset) const = 0; /** @@ -188,13 +164,6 @@ protected: virtual SkIRect onFilterNodeBounds(const SkIRect&, const SkMatrix& ctm, MapDirection, const SkIRect* inputRect) const; - // Helper function which invokes filter processing on the input at the specified "index". If the - // input is null, it returns the Context's source image "src" and leaves "offset" untouched. If - // the input is non-null, it calls filterImage() on that input, and returns the result. - sk_sp filterInput(int index, - const Context&, - SkIPoint* offset) const; - /** * Return true (and returns a ref'd colorfilter) if this node in the DAG is just a * colorfilter w/o CropRect constraints. @@ -209,6 +178,31 @@ protected: */ virtual bool onCanHandleComplexCTM() const { return false; } + // DEPRECRATED - Call the Context-only getInputFilteredImage() + sk_sp filterInput(int index, const Context& ctx, SkIPoint* offset) const { + return this->getInputFilteredImage(index, ctx).imageAndOffset(offset); + } + + // Helper function to help with recursing through the filter DAG. It invokes filter processing + // set to null, it returns the dynamic source image on the Context instead. + // + // Implementations must handle cases when the input filter was unable to compute an image and + // the returned skif::Image has a null SkSpecialImage. If the filter affect transparent black + // should explicitly handle nullptr results and press on. In the error case this behavior will + // produce a better result than nothing and is necessary for the clipped out case. + skif::FilterResult getInputFilteredImage(int index, + const skif::Context& context) const { + return this->filterInput(index, context); + } + // Convenience that calls filterInput with index = 0 and the most specific usage. + skif::FilterResult getInputFilteredImage0(const skif::Context& context) const { + return this->filterInput(0, context); + } + // Convenience that calls filterInput with index = 1 and the most specific usage. + skif::FilterResult getInputFilteredImage1(const skif::Context& context) const { + return this->filterInput(1, context); + } + const CropRect* getCropRectIfSet() const { return this->cropRectIsSet() ? &fCropRect : nullptr; } @@ -279,6 +273,31 @@ private: void init(sk_sp const* inputs, int inputCount, const CropRect* cropRect); + // Configuration points for the filter implementation, marked private since they should not + // need to be invoked by the subclasses. These refer to the node's specific behavior and are + // not responsible for aggregating the behavior of the entire filter DAG. + + /** + * This is the virtual which should be overridden by the derived class to perform image + * filtering. Subclasses are responsible for recursing to their input filters, although the + * getFilteredInputX() functions are provided to handle all necessary details of this. If the + * filter has a fixed number of inputs, the getFilterInput0() and getFilteredInput1() functions + * ensure the returned filtered Images have the most specific input usage. + * + * If the image cannot be created (either because of an error or if the result would be empty + * because it was clipped out), this should return a filtered Image with a null SkSpecialImage. + * In these situations, callers that do not affect transparent black can end early, since the + * "transparent" implicit image would be unchanged. Callers that affect transparent black need + * to safely handle these null and empty images and return an image filling the context's clip + * bounds as if its input filtered image were transparent black. + */ + virtual skif::FilterResult onFilterImage(const skif::Context& context) const; + + // The actual implementation of the protected getFilterInputX() functions, but don't expose the + // flexible templating to subclasses so it can't be abused. + template + skif::FilterResult filterInput(int index, const skif::Context& ctx) const; + SkAutoSTArray<2, sk_sp> fInputs; bool fUsesSrcInput; diff --git a/src/core/SkSpecialImage.cpp b/src/core/SkSpecialImage.cpp index 56d69edc2f..ad17fbb845 100644 --- a/src/core/SkSpecialImage.cpp +++ b/src/core/SkSpecialImage.cpp @@ -86,13 +86,13 @@ SkSpecialImage::SkSpecialImage(const SkIRect& subset, , fUniqueID(kNeedNewImageUniqueID_SpecialImage == uniqueID ? SkNextID::ImageID() : uniqueID) { } -sk_sp SkSpecialImage::makeTextureImage(GrRecordingContext* context) { +sk_sp SkSpecialImage::makeTextureImage(GrRecordingContext* context) const { #if SK_SUPPORT_GPU if (!context) { return nullptr; } if (GrRecordingContext* curContext = as_SIB(this)->onGetContext()) { - return curContext->priv().matches(context) ? sk_sp(SkRef(this)) : nullptr; + return curContext->priv().matches(context) ? sk_ref_sp(this) : nullptr; } auto proxyProvider = context->priv().proxyProvider(); diff --git a/src/core/SkSpecialImage.h b/src/core/SkSpecialImage.h index 1c35ba912f..753a3170ff 100644 --- a/src/core/SkSpecialImage.h +++ b/src/core/SkSpecialImage.h @@ -60,7 +60,7 @@ public: * If no transformation is required, the returned image may be the same as this special image. * If this special image is from a different GrRecordingContext, this will fail. */ - sk_sp makeTextureImage(GrRecordingContext*); + sk_sp makeTextureImage(GrRecordingContext*) const; /** * Draw this SpecialImage into the canvas, automatically taking into account the image's subset diff --git a/src/effects/imagefilters/SkDisplacementMapEffect.cpp b/src/effects/imagefilters/SkDisplacementMapEffect.cpp index 595fa894c7..9b23a57dc0 100644 --- a/src/effects/imagefilters/SkDisplacementMapEffect.cpp +++ b/src/effects/imagefilters/SkDisplacementMapEffect.cpp @@ -294,8 +294,8 @@ sk_sp SkDisplacementMapEffectImpl::onFilterImage(const Context& // With a more complex DAG attached to this input, it's not clear that working in ANY specific // color space makes sense, so we ignore color spaces (and gamma) entirely. This may not be // ideal, but it's at least consistent and predictable. - Context displContext(ctx.ctm(), ctx.clipBounds(), ctx.cache(), kN32_SkColorType, nullptr, - ctx.sourceImage()); + Context displContext(ctx.ctm(), ctx.clipBounds(), ctx.cache(), + kN32_SkColorType, nullptr, ctx.source()); sk_sp displ(this->filterInput(0, displContext, &displOffset)); if (!displ) { return nullptr; diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp index 817fe5da3e..32ddda9b87 100644 --- a/src/gpu/SkGpuDevice.cpp +++ b/src/gpu/SkGpuDevice.cpp @@ -169,7 +169,7 @@ sk_sp SkGpuDevice::filterTexture(SkSpecialImage* srcImg, SkImageFilter_Base::Context ctx(matrix, clipBounds, cache.get(), colorType, fRenderTargetContext->colorSpaceInfo().colorSpace(), srcImg); - return as_IFB(filter)->filterImage(ctx, offset); + return as_IFB(filter)->filterImage(ctx).imageAndOffset(offset); } /////////////////////////////////////////////////////////////////////////////// diff --git a/src/image/SkImage.cpp b/src/image/SkImage.cpp index 0adcc8f363..de6729b97e 100644 --- a/src/image/SkImage.cpp +++ b/src/image/SkImage.cpp @@ -293,7 +293,7 @@ sk_sp SkImage::makeWithFilter(GrContext* grContext, cache.get(), fInfo.colorType(), fInfo.colorSpace(), srcSpecialImage.get()); - sk_sp result = as_IFB(filter)->filterImage(context, offset); + sk_sp result = as_IFB(filter)->filterImage(context).imageAndOffset(offset); if (!result) { return nullptr; } diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp index 48fe13e326..5c032c08eb 100644 --- a/src/pdf/SkPDFDevice.cpp +++ b/src/pdf/SkPDFDevice.cpp @@ -1732,7 +1732,7 @@ void SkPDFDevice::drawSpecial(SkSpecialImage* srcImg, int x, int y, const SkPain SkImageFilter_Base::Context ctx(matrix, clipBounds, cache.get(), kN32_SkColorType, srcImg->getColorSpace(), srcImg); - sk_sp resultImg(as_IFB(filter)->filterImage(ctx, &offset)); + sk_sp resultImg(as_IFB(filter)->filterImage(ctx).imageAndOffset(&offset)); if (resultImg) { SkPaint tmpUnfiltered(paint); tmpUnfiltered.setImageFilter(nullptr); diff --git a/tests/ImageFilterCacheTest.cpp b/tests/ImageFilterCacheTest.cpp index 20c8da5858..1c1b48dac3 100644 --- a/tests/ImageFilterCacheTest.cpp +++ b/tests/ImageFilterCacheTest.cpp @@ -16,6 +16,8 @@ #include "src/core/SkImageFilterCache.h" #include "src/core/SkSpecialImage.h" +SK_USE_FLUENT_IMAGE_FILTER_TYPES + static const int kSmallerSize = 10; static const int kPad = 3; static const int kFullSize = kSmallerSize + 2 * kPad; @@ -28,8 +30,7 @@ static SkBitmap create_bm() { } static sk_sp make_filter() { - sk_sp filter(SkColorFilters::Blend(SK_ColorBLUE, - SkBlendMode::kSrcIn)); + sk_sp filter(SkColorFilters::Blend(SK_ColorBLUE, SkBlendMode::kSrcIn)); return SkImageFilters::ColorFilter(std::move(filter), nullptr, nullptr); } @@ -46,15 +47,13 @@ static void test_find_existing(skiatest::Reporter* reporter, SkIPoint offset = SkIPoint::Make(3, 4); auto filter = make_filter(); - cache->set(key1, image.get(), offset, filter.get()); + cache->set(key1, filter.get(), skif::FilterResult(image, offset)); - SkIPoint foundOffset; + skif::FilterResult foundImage; + REPORTER_ASSERT(reporter, cache->get(key1, &foundImage)); + REPORTER_ASSERT(reporter, offset == foundImage.origin()); - sk_sp foundImage = cache->get(key1, &foundOffset); - REPORTER_ASSERT(reporter, foundImage); - REPORTER_ASSERT(reporter, offset == foundOffset); - - REPORTER_ASSERT(reporter, !cache->get(key2, &foundOffset)); + REPORTER_ASSERT(reporter, !cache->get(key2, &foundImage)); } // If either id is different or the clip or the matrix are different the @@ -76,13 +75,13 @@ static void test_dont_find_if_diff_key(skiatest::Reporter* reporter, SkIPoint offset = SkIPoint::Make(3, 4); auto filter = make_filter(); - cache->set(key0, image.get(), offset, filter.get()); + cache->set(key0, filter.get(), skif::FilterResult(image, offset)); - SkIPoint foundOffset; - REPORTER_ASSERT(reporter, !cache->get(key1, &foundOffset)); - REPORTER_ASSERT(reporter, !cache->get(key2, &foundOffset)); - REPORTER_ASSERT(reporter, !cache->get(key3, &foundOffset)); - REPORTER_ASSERT(reporter, !cache->get(key4, &foundOffset)); + skif::FilterResult foundImage; + REPORTER_ASSERT(reporter, !cache->get(key1, &foundImage)); + REPORTER_ASSERT(reporter, !cache->get(key2, &foundImage)); + REPORTER_ASSERT(reporter, !cache->get(key3, &foundImage)); + REPORTER_ASSERT(reporter, !cache->get(key4, &foundImage)); } // Test purging when the max cache size is exceeded @@ -97,18 +96,17 @@ static void test_internal_purge(skiatest::Reporter* reporter, const sk_spset(key1, image.get(), offset, filter1.get()); + cache->set(key1, filter1.get(), skif::FilterResult(image, offset)); - SkIPoint foundOffset; - - REPORTER_ASSERT(reporter, cache->get(key1, &foundOffset)); + skif::FilterResult foundImage; + REPORTER_ASSERT(reporter, cache->get(key1, &foundImage)); // This should knock the first one out of the cache auto filter2 = make_filter(); - cache->set(key2, image.get(), offset, filter2.get()); + cache->set(key2, filter2.get(), skif::FilterResult(image, offset)); - REPORTER_ASSERT(reporter, cache->get(key2, &foundOffset)); - REPORTER_ASSERT(reporter, !cache->get(key1, &foundOffset)); + REPORTER_ASSERT(reporter, cache->get(key2, &foundImage)); + REPORTER_ASSERT(reporter, !cache->get(key1, &foundImage)); } // Exercise the purgeByKey and purge methods @@ -125,26 +123,25 @@ static void test_explicit_purging(skiatest::Reporter* reporter, SkIPoint offset = SkIPoint::Make(3, 4); auto filter1 = make_filter(); auto filter2 = make_filter(); - cache->set(key1, image.get(), offset, filter1.get()); - cache->set(key2, image.get(), offset, filter2.get()); + cache->set(key1, filter1.get(), skif::FilterResult(image, offset)); + cache->set(key2, filter2.get(), skif::FilterResult(image, offset)); SkDEBUGCODE(REPORTER_ASSERT(reporter, 2 == cache->count());) - SkIPoint foundOffset; - - REPORTER_ASSERT(reporter, cache->get(key1, &foundOffset)); - REPORTER_ASSERT(reporter, cache->get(key2, &foundOffset)); + skif::FilterResult foundImage; + REPORTER_ASSERT(reporter, cache->get(key1, &foundImage)); + REPORTER_ASSERT(reporter, cache->get(key2, &foundImage)); cache->purgeByImageFilter(filter1.get()); SkDEBUGCODE(REPORTER_ASSERT(reporter, 1 == cache->count());) - REPORTER_ASSERT(reporter, !cache->get(key1, &foundOffset)); - REPORTER_ASSERT(reporter, cache->get(key2, &foundOffset)); + REPORTER_ASSERT(reporter, !cache->get(key1, &foundImage)); + REPORTER_ASSERT(reporter, cache->get(key2, &foundImage)); cache->purge(); SkDEBUGCODE(REPORTER_ASSERT(reporter, 0 == cache->count());) - REPORTER_ASSERT(reporter, !cache->get(key1, &foundOffset)); - REPORTER_ASSERT(reporter, !cache->get(key2, &foundOffset)); + REPORTER_ASSERT(reporter, !cache->get(key1, &foundImage)); + REPORTER_ASSERT(reporter, !cache->get(key2, &foundImage)); } DEF_TEST(ImageFilterCache_RasterBacked, reporter) { diff --git a/tests/ImageFilterTest.cpp b/tests/ImageFilterTest.cpp index 5cc091597d..dd284e9074 100644 --- a/tests/ImageFilterTest.cpp +++ b/tests/ImageFilterTest.cpp @@ -485,7 +485,7 @@ static void test_cropRects(skiatest::Reporter* reporter, GrContext* context) { SkIPoint offset; SkImageFilter_Base::Context ctx(SkMatrix::I(), SkIRect::MakeWH(100, 100), nullptr, kN32_SkColorType, nullptr, srcImg.get()); - sk_sp resultImg(as_IFB(filter)->filterImage(ctx, &offset)); + sk_sp resultImg(as_IFB(filter)->filterImage(ctx).imageAndOffset(&offset)); REPORTER_ASSERT(reporter, resultImg, filters.getName(i)); REPORTER_ASSERT(reporter, offset.fX == 20 && offset.fY == 30, filters.getName(i)); } @@ -509,11 +509,11 @@ static void test_negative_blur_sigma(skiatest::Reporter* reporter, GrContext* co kN32_SkColorType, nullptr, imgSrc.get()); sk_sp positiveResult1( - as_IFB(positiveFilter)->filterImage(ctx, &offset)); + as_IFB(positiveFilter)->filterImage(ctx).imageAndOffset(&offset)); REPORTER_ASSERT(reporter, positiveResult1); sk_sp negativeResult1( - as_IFB(negativeFilter)->filterImage(ctx, &offset)); + as_IFB(negativeFilter)->filterImage(ctx).imageAndOffset(&offset)); REPORTER_ASSERT(reporter, negativeResult1); SkMatrix negativeScale; @@ -522,11 +522,11 @@ static void test_negative_blur_sigma(skiatest::Reporter* reporter, GrContext* co kN32_SkColorType, nullptr, imgSrc.get()); sk_sp negativeResult2( - as_IFB(positiveFilter)->filterImage(negativeCTX, &offset)); + as_IFB(positiveFilter)->filterImage(negativeCTX).imageAndOffset(&offset)); REPORTER_ASSERT(reporter, negativeResult2); sk_sp positiveResult2( - as_IFB(negativeFilter)->filterImage(negativeCTX, &offset)); + as_IFB(negativeFilter)->filterImage(negativeCTX).imageAndOffset(&offset)); REPORTER_ASSERT(reporter, positiveResult2); @@ -585,7 +585,7 @@ static void test_zero_blur_sigma(skiatest::Reporter* reporter, GrContext* contex SkImageFilter_Base::Context ctx(SkMatrix::I(), SkIRect::MakeWH(32, 32), nullptr, kN32_SkColorType, nullptr, image.get()); - sk_sp result(as_IFB(filter)->filterImage(ctx, &offset)); + sk_sp result(as_IFB(filter)->filterImage(ctx).imageAndOffset(&offset)); REPORTER_ASSERT(reporter, offset.fX == 5 && offset.fY == 0); REPORTER_ASSERT(reporter, result); REPORTER_ASSERT(reporter, result->width() == 5 && result->height() == 10); @@ -626,7 +626,7 @@ static void test_fail_affects_transparent_black(skiatest::Reporter* reporter, Gr sk_sp greenFilter(SkImageFilters::ColorFilter(std::move(green), std::move(failFilter))); SkIPoint offset; - sk_sp result(as_IFB(greenFilter)->filterImage(ctx, &offset)); + sk_sp result(as_IFB(greenFilter)->filterImage(ctx).imageAndOffset(&offset)); REPORTER_ASSERT(reporter, nullptr != result.get()); if (result.get()) { SkBitmap resultBM; @@ -907,7 +907,7 @@ static void test_imagefilter_merge_result_size(skiatest::Reporter* reporter, GrC kN32_SkColorType, nullptr, srcImg.get()); SkIPoint offset; - sk_sp resultImg(as_IFB(merge)->filterImage(ctx, &offset)); + sk_sp resultImg(as_IFB(merge)->filterImage(ctx).imageAndOffset(&offset)); REPORTER_ASSERT(reporter, resultImg); REPORTER_ASSERT(reporter, resultImg->width() == 20 && resultImg->height() == 20); @@ -1064,7 +1064,7 @@ static void test_big_kernel(skiatest::Reporter* reporter, GrContext* context) { SkIPoint offset; SkImageFilter_Base::Context ctx(SkMatrix::I(), SkIRect::MakeWH(100, 100), nullptr, kN32_SkColorType, nullptr, srcImg.get()); - sk_sp resultImg(as_IFB(filter)->filterImage(ctx, &offset)); + sk_sp resultImg(as_IFB(filter)->filterImage(ctx).imageAndOffset(&offset)); REPORTER_ASSERT(reporter, resultImg); REPORTER_ASSERT(reporter, SkToBool(context) == resultImg->isTextureBacked()); REPORTER_ASSERT(reporter, resultImg->width() == 100 && resultImg->height() == 100); @@ -1137,7 +1137,8 @@ static void test_clipped_picture_imagefilter(skiatest::Reporter* reporter, GrCon SkImageFilter_Base::Context ctx(SkMatrix::I(), SkIRect::MakeXYWH(1, 1, 1, 1), nullptr, kN32_SkColorType, nullptr, srcImg.get()); - sk_sp resultImage(as_IFB(imageFilter)->filterImage(ctx, &offset)); + sk_sp resultImage( + as_IFB(imageFilter)->filterImage(ctx).imageAndOffset(&offset)); REPORTER_ASSERT(reporter, !resultImage); } @@ -1160,8 +1161,7 @@ DEF_TEST(ImageFilterEmptySaveLayer, reporter) { SkRTreeFactory factory; SkPictureRecorder recorder; - sk_sp green(SkColorFilters::Blend(SK_ColorGREEN, - SkBlendMode::kSrc)); + sk_sp green(SkColorFilters::Blend(SK_ColorGREEN, SkBlendMode::kSrc)); sk_sp imageFilter(SkImageFilters::ColorFilter(green, nullptr)); SkPaint imageFilterPaint; imageFilterPaint.setImageFilter(std::move(imageFilter)); @@ -1364,7 +1364,7 @@ static void test_composed_imagefilter_offset(skiatest::Reporter* reporter, GrCon kN32_SkColorType, nullptr, srcImg.get()); sk_sp resultImg( - as_IFB(composedFilter)->filterImage(ctx, &offset)); + as_IFB(composedFilter)->filterImage(ctx).imageAndOffset(&offset)); REPORTER_ASSERT(reporter, resultImg); REPORTER_ASSERT(reporter, offset.fX == 1 && offset.fY == 0); } @@ -1400,7 +1400,7 @@ static void test_composed_imagefilter_bounds(skiatest::Reporter* reporter, GrCon kN32_SkColorType, nullptr, sourceImage.get()); SkIPoint offset; sk_sp result( - as_IFB(composedFilter)->filterImage(ctx, &offset)); + as_IFB(composedFilter)->filterImage(ctx).imageAndOffset(&offset)); REPORTER_ASSERT(reporter, offset.isZero()); REPORTER_ASSERT(reporter, result); REPORTER_ASSERT(reporter, result->subset().size() == SkISize::Make(100, 100));