Revert of Matrix convolution bounds fix; affectsTransparentBlack fixes. (patchset #4 id:60001 of https://codereview.chromium.org/1500923004/ )

Reason for revert:
Introduced memory leak; pixel changes in Chrome.

Original issue's description:
> 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:
>
> Committed: https://skia.googlesource.com/skia/+/8705ec80518ef551994b82ca5ccaeb0241d6adec

TBR=reed@google.com,reed@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:

Review URL: https://codereview.chromium.org/1497083005
This commit is contained in:
senorblanco 2015-12-05 05:59:41 -08:00 committed by Commit bot
parent 22a517f71a
commit 50c044b9ad
10 changed files with 40 additions and 41 deletions

View File

@ -86,22 +86,21 @@ 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, &rect);
this->draw(canvas, x, 110, kernelOffset, MCIF::kClampToBlack_TileMode, true, &rect);
this->draw(canvas, x, 210, kernelOffset, MCIF::kRepeat_TileMode, true, &rect);
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);
kernelOffset.fY++;
}
kernelOffset.fY = 1;
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);
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);
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);
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);
}
private:

View File

@ -199,7 +199,12 @@ public:
* replaced by the returned colorfilter. i.e. the two effects will affect drawing in the
* same way.
*/
bool asAColorFilter(SkColorFilter** filterPtr) const;
bool asAColorFilter(SkColorFilter** filterPtr) const {
return this->countInputs() > 0 &&
NULL == this->getInput(0) &&
!this->affectsTransparentBlack() &&
this->isColorFilterNode(filterPtr);
}
/**
* Returns the number of inputs this filter will accept (some inputs can
@ -235,7 +240,7 @@ public:
virtual void computeFastBounds(const SkRect&, SkRect*) const;
// Can this filter DAG compute the resulting bounds of an object-space rectangle?
virtual bool canComputeFastBounds() const;
bool canComputeFastBounds() const;
/**
* If this filter can be represented by another filter + a localMatrix, return that filter,
@ -405,6 +410,14 @@ 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 canComputeFastBounds() const override;
bool affectsTransparentBlack() 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 canComputeFastBounds() const override { return false; }
bool affectsTransparentBlack() const override { return true; }
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 canComputeFastBounds() const override;
bool affectsTransparentBlack() const override;
SK_TO_STRING_OVERRIDE()
SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(SkRectShaderImageFilter)

View File

@ -309,6 +309,9 @@ 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()) {
@ -318,6 +321,10 @@ 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;
@ -384,14 +391,6 @@ bool SkImageFilter::filterImageGPU(Proxy* proxy, const SkBitmap& src, const Cont
return false;
}
bool SkImageFilter::asAColorFilter(SkColorFilter** filterPtr) const {
SkASSERT(nullptr != filterPtr);
return this->countInputs() > 0 &&
NULL == this->getInput(0) &&
this->isColorFilterNode(filterPtr) &&
!(*filterPtr)->affectsTransparentBlack();
}
bool SkImageFilter::applyCropRect(const Context& ctx, const SkBitmap& src,
const SkIPoint& srcOffset, SkIRect* dstBounds,
SkIRect* srcBounds) const {

View File

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

View File

@ -335,12 +335,6 @@ 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,11 +79,8 @@ bool SkRectShaderImageFilter::onFilterImage(Proxy* proxy,
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;
bool SkRectShaderImageFilter::affectsTransparentBlack() const {
return true;
}
#ifndef SK_IGNORE_TO_STRING