Matrix convolution bounds fix; affectsTransparentBlack fixes.

Because the convolution kernel is (currently) applied in device space,
there's no way to know which object-space pixels will be touched. So
return false from canComputeFastBounds().

The results from the matrixconvolution GM were actually wrong, since
they were showing edge differences on the clip boundaries, where they
should really only show on crop boundaries. I added a crop to the GM
to keep the results the same (which are useful to test the different
convolution tile modes).

While I was at it, SkImageFilter::affectsTransparentBlack() was
inapplicable on most things except color filters, and its use on
leaf nodes was confusing. So I removed it, and made
SkImageFilter::canComputeFastBounds() virtual instead.

BUG=skia:4630

Review URL: https://codereview.chromium.org/1500923004
This commit is contained in:
senorblanco 2015-12-07 07:48:34 -08:00 committed by Commit bot
parent d7a2c1f5fd
commit a544eda5dd
10 changed files with 45 additions and 40 deletions

View File

@ -86,21 +86,22 @@ protected:
void onDraw(SkCanvas* canvas) override {
canvas->clear(SK_ColorBLACK);
SkIPoint kernelOffset = SkIPoint::Make(1, 0);
SkImageFilter::CropRect rect(SkRect::Make(fBitmap.bounds()));
for (int x = 10; x < 310; x += 100) {
this->draw(canvas, x, 10, kernelOffset, MCIF::kClamp_TileMode, true);
this->draw(canvas, x, 110, kernelOffset, MCIF::kClampToBlack_TileMode, true);
this->draw(canvas, x, 210, kernelOffset, MCIF::kRepeat_TileMode, true);
this->draw(canvas, x, 10, kernelOffset, MCIF::kClamp_TileMode, true, &rect);
this->draw(canvas, x, 110, kernelOffset, MCIF::kClampToBlack_TileMode, true, &rect);
this->draw(canvas, x, 210, kernelOffset, MCIF::kRepeat_TileMode, true, &rect);
kernelOffset.fY++;
}
kernelOffset.fY = 1;
SkImageFilter::CropRect rect(SkRect::MakeXYWH(10, 5, 60, 60));
this->draw(canvas, 310, 10, kernelOffset, MCIF::kClamp_TileMode, true, &rect);
this->draw(canvas, 310, 110, kernelOffset, MCIF::kClampToBlack_TileMode, true, &rect);
this->draw(canvas, 310, 210, kernelOffset, MCIF::kRepeat_TileMode, true, &rect);
SkImageFilter::CropRect smallRect(SkRect::MakeXYWH(10, 5, 60, 60));
this->draw(canvas, 310, 10, kernelOffset, MCIF::kClamp_TileMode, true, &smallRect);
this->draw(canvas, 310, 110, kernelOffset, MCIF::kClampToBlack_TileMode, true, &smallRect);
this->draw(canvas, 310, 210, kernelOffset, MCIF::kRepeat_TileMode, true, &smallRect);
this->draw(canvas, 410, 10, kernelOffset, MCIF::kClamp_TileMode, false);
this->draw(canvas, 410, 110, kernelOffset, MCIF::kClampToBlack_TileMode, false);
this->draw(canvas, 410, 210, kernelOffset, MCIF::kRepeat_TileMode, false);
this->draw(canvas, 410, 10, kernelOffset, MCIF::kClamp_TileMode, false, &rect);
this->draw(canvas, 410, 110, kernelOffset, MCIF::kClampToBlack_TileMode, false, &rect);
this->draw(canvas, 410, 210, kernelOffset, MCIF::kRepeat_TileMode, false, &rect);
}
private:

View File

@ -199,12 +199,7 @@ public:
* replaced by the returned colorfilter. i.e. the two effects will affect drawing in the
* same way.
*/
bool asAColorFilter(SkColorFilter** filterPtr) const {
return this->countInputs() > 0 &&
NULL == this->getInput(0) &&
!this->affectsTransparentBlack() &&
this->isColorFilterNode(filterPtr);
}
bool asAColorFilter(SkColorFilter** filterPtr) const;
/**
* Returns the number of inputs this filter will accept (some inputs can
@ -240,7 +235,7 @@ public:
virtual void computeFastBounds(const SkRect&, SkRect*) const;
// Can this filter DAG compute the resulting bounds of an object-space rectangle?
bool canComputeFastBounds() const;
virtual bool canComputeFastBounds() const;
/**
* If this filter can be represented by another filter + a localMatrix, return that filter,
@ -410,14 +405,6 @@ protected:
virtual bool asFragmentProcessor(GrFragmentProcessor**, GrTexture*, const SkMatrix&,
const SkIRect& bounds) const;
/**
* Returns true if this filter can cause transparent black pixels to become
* visible (ie., alpha > 0). The default implementation returns false. This
* function is non-recursive, i.e., only queries this filter and not its
* inputs.
*/
virtual bool affectsTransparentBlack() const;
private:
friend class SkGraphics;
static void PurgeCache();

View File

@ -25,7 +25,7 @@ protected:
bool onFilterImage(Proxy*, const SkBitmap& src, const Context&, SkBitmap* result,
SkIPoint* loc) const override;
bool onIsColorFilterNode(SkColorFilter**) const override;
bool affectsTransparentBlack() const override;
bool canComputeFastBounds() const override;
private:
SkColorFilterImageFilter(SkColorFilter* cf,

View File

@ -49,7 +49,7 @@ protected:
void flatten(SkWriteBuffer&) const override;
const SkImageFilterLight* light() const { return fLight.get(); }
SkScalar surfaceScale() const { return fSurfaceScale; }
bool affectsTransparentBlack() const override { return true; }
bool canComputeFastBounds() const override { return false; }
private:
typedef SkImageFilter INHERITED;

View File

@ -80,7 +80,7 @@ protected:
bool onFilterImage(Proxy*, const SkBitmap& src, const Context&,
SkBitmap* result, SkIPoint* loc) const override;
bool onFilterBounds(const SkIRect&, const SkMatrix&, SkIRect*) const override;
bool canComputeFastBounds() const override;
#if SK_SUPPORT_GPU
bool asFragmentProcessor(GrFragmentProcessor**, GrTexture*, const SkMatrix&,

View File

@ -29,7 +29,7 @@ public:
static SkImageFilter* Create(SkShader* s, const SkRect& rect);
static SkImageFilter* Create(SkShader* s, const CropRect* rect = NULL);
bool affectsTransparentBlack() const override;
bool canComputeFastBounds() const override;
SK_TO_STRING_OVERRIDE()
SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(SkRectShaderImageFilter)

View File

@ -309,9 +309,6 @@ void SkImageFilter::computeFastBounds(const SkRect& src, SkRect* dst) const {
}
bool SkImageFilter::canComputeFastBounds() const {
if (this->affectsTransparentBlack()) {
return false;
}
for (int i = 0; i < fInputCount; i++) {
SkImageFilter* input = this->getInput(i);
if (input && !input->canComputeFastBounds()) {
@ -321,10 +318,6 @@ bool SkImageFilter::canComputeFastBounds() const {
return true;
}
bool SkImageFilter::affectsTransparentBlack() const {
return false;
}
bool SkImageFilter::onFilterImage(Proxy*, const SkBitmap&, const Context&,
SkBitmap*, SkIPoint*) const {
return false;
@ -391,6 +384,18 @@ bool SkImageFilter::filterImageGPU(Proxy* proxy, const SkBitmap& src, const Cont
return false;
}
bool SkImageFilter::asAColorFilter(SkColorFilter** filterPtr) const {
SkASSERT(nullptr != filterPtr);
if (!this->isColorFilterNode(filterPtr)) {
return false;
}
if (nullptr != this->getInput(0) || (*filterPtr)->affectsTransparentBlack()) {
(*filterPtr)->unref();
return false;
}
return true;
}
bool SkImageFilter::applyCropRect(const Context& ctx, const SkBitmap& src,
const SkIPoint& srcOffset, SkIRect* dstBounds,
SkIRect* srcBounds) const {

View File

@ -99,8 +99,11 @@ bool SkColorFilterImageFilter::onIsColorFilterNode(SkColorFilter** filter) const
return false;
}
bool SkColorFilterImageFilter::affectsTransparentBlack() const {
return fColorFilter->affectsTransparentBlack();
bool SkColorFilterImageFilter::canComputeFastBounds() const {
if (fColorFilter->affectsTransparentBlack()) {
return false;
}
return INHERITED::canComputeFastBounds();
}
#ifndef SK_IGNORE_TO_STRING

View File

@ -335,6 +335,12 @@ bool SkMatrixConvolutionImageFilter::onFilterBounds(const SkIRect& src, const Sk
return true;
}
bool SkMatrixConvolutionImageFilter::canComputeFastBounds() const {
// Because the kernel is applied in device-space, we have no idea what
// pixels it will affect in object-space.
return false;
}
#if SK_SUPPORT_GPU
static GrTextureDomain::Mode convert_tilemodes(

View File

@ -79,8 +79,11 @@ bool SkRectShaderImageFilter::onFilterImage(Proxy* proxy,
return true;
}
bool SkRectShaderImageFilter::affectsTransparentBlack() const {
return true;
bool SkRectShaderImageFilter::canComputeFastBounds() const {
// http:skbug.com/4627: "make computeFastBounds and onFilterBounds() CropRect-aware"
// computeFastBounds() doesn't currently take the crop rect into account,
// so we can't compute it. If a full crop rect is set, we should return true here.
return false;
}
#ifndef SK_IGNORE_TO_STRING