aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGravatar senorblanco <senorblanco@chromium.org>2015-12-04 13:57:30 -0800
committerGravatar Commit bot <commit-bot@chromium.org>2015-12-04 13:57:31 -0800
commit8705ec80518ef551994b82ca5ccaeb0241d6adec (patch)
tree205344722622c58258b8bf48cb6e3faf97d18c66
parent95f53fbc5ddf14ddb1e51c7ebced2b80fb069f81 (diff)
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: Review URL: https://codereview.chromium.org/1500923004
-rw-r--r--gm/matrixconvolution.cpp23
-rw-r--r--include/core/SkImageFilter.h17
-rw-r--r--include/effects/SkColorFilterImageFilter.h2
-rw-r--r--include/effects/SkLightingImageFilter.h2
-rw-r--r--include/effects/SkMatrixConvolutionImageFilter.h2
-rw-r--r--include/effects/SkRectShaderImageFilter.h2
-rw-r--r--src/core/SkImageFilter.cpp15
-rw-r--r--src/effects/SkColorFilterImageFilter.cpp7
-rw-r--r--src/effects/SkMatrixConvolutionImageFilter.cpp6
-rw-r--r--src/effects/SkRectShaderImageFilter.cpp7
10 files changed, 42 insertions, 41 deletions
diff --git a/gm/matrixconvolution.cpp b/gm/matrixconvolution.cpp
index 6d16f8dfa7..3b159f455b 100644
--- a/gm/matrixconvolution.cpp
+++ b/gm/matrixconvolution.cpp
@@ -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);
-
- 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);
+ 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, &rect);
+ this->draw(canvas, 410, 110, kernelOffset, MCIF::kClampToBlack_TileMode, false, &rect);
+ this->draw(canvas, 410, 210, kernelOffset, MCIF::kRepeat_TileMode, false, &rect);
}
private:
diff --git a/include/core/SkImageFilter.h b/include/core/SkImageFilter.h
index d90df29c8b..909a2f8284 100644
--- a/include/core/SkImageFilter.h
+++ b/include/core/SkImageFilter.h
@@ -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();
diff --git a/include/effects/SkColorFilterImageFilter.h b/include/effects/SkColorFilterImageFilter.h
index 200ec86500..db5d842c2f 100644
--- a/include/effects/SkColorFilterImageFilter.h
+++ b/include/effects/SkColorFilterImageFilter.h
@@ -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,
diff --git a/include/effects/SkLightingImageFilter.h b/include/effects/SkLightingImageFilter.h
index fb356c52e4..33cfceccb2 100644
--- a/include/effects/SkLightingImageFilter.h
+++ b/include/effects/SkLightingImageFilter.h
@@ -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;
diff --git a/include/effects/SkMatrixConvolutionImageFilter.h b/include/effects/SkMatrixConvolutionImageFilter.h
index 76349f430c..ef5ff72494 100644
--- a/include/effects/SkMatrixConvolutionImageFilter.h
+++ b/include/effects/SkMatrixConvolutionImageFilter.h
@@ -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&,
diff --git a/include/effects/SkRectShaderImageFilter.h b/include/effects/SkRectShaderImageFilter.h
index 9798af2d64..9ac116ae5f 100644
--- a/include/effects/SkRectShaderImageFilter.h
+++ b/include/effects/SkRectShaderImageFilter.h
@@ -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)
diff --git a/src/core/SkImageFilter.cpp b/src/core/SkImageFilter.cpp
index 6a3286ed67..3f33bc3876 100644
--- a/src/core/SkImageFilter.cpp
+++ b/src/core/SkImageFilter.cpp
@@ -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,14 @@ 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 {
diff --git a/src/effects/SkColorFilterImageFilter.cpp b/src/effects/SkColorFilterImageFilter.cpp
index 8d394aa17b..b0e47505a7 100644
--- a/src/effects/SkColorFilterImageFilter.cpp
+++ b/src/effects/SkColorFilterImageFilter.cpp
@@ -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
diff --git a/src/effects/SkMatrixConvolutionImageFilter.cpp b/src/effects/SkMatrixConvolutionImageFilter.cpp
index e58eec10f4..7c5dd8368f 100644
--- a/src/effects/SkMatrixConvolutionImageFilter.cpp
+++ b/src/effects/SkMatrixConvolutionImageFilter.cpp
@@ -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(
diff --git a/src/effects/SkRectShaderImageFilter.cpp b/src/effects/SkRectShaderImageFilter.cpp
index 14837d02b1..00964b5ffb 100644
--- a/src/effects/SkRectShaderImageFilter.cpp
+++ b/src/effects/SkRectShaderImageFilter.cpp
@@ -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