Implement computeFastBounds for PathEffects
Makes computeFastBounds not part of the public API, it's only accessible
to subclasses of SkPathEffect, GrStyle, and SkPaint. Subclasses can
invoke it other path effects using SkPathEffectPriv::ComputeFastBounds.
Changes the internal function to
bool computeFastBounds(SkRect* bounds) const;
Subclasses of SkPathEffect must implement this, and can choose to return
false when fast bounds aren't computable.
Provides implementations of computeFastBounds() for path effects
bundled with Skia.
Bug: skia:11974
Change-Id: I545ccf99b4e669d3af9df13acfac28573306fab8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/406140
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Mike Reed <reed@google.com>
diff --git a/RELEASE_NOTES.txt b/RELEASE_NOTES.txt
index b236758..ad24826 100644
--- a/RELEASE_NOTES.txt
+++ b/RELEASE_NOTES.txt
@@ -6,6 +6,10 @@
Milestone 92
------------
+ * Hides SkPathEffect::computeFastBounds() from public API; external subclasses of SkPathEffect
+ must implement onComputeFastBounds() but can return false to signal it's not computable.
+ https://review.skia.org/406140
+
* Add SkM44::RectToRect constructor (SkM44's equivalent to SkMatrix::RectToRect)
https://review.skia.org/402957
diff --git a/gn/core.gni b/gn/core.gni
index 8862b8f..d7964d5 100644
--- a/gn/core.gni
+++ b/gn/core.gni
@@ -285,6 +285,7 @@
"$_src/core/SkPath.cpp",
"$_src/core/SkPathBuilder.cpp",
"$_src/core/SkPathEffect.cpp",
+ "$_src/core/SkPathEffectPriv.h",
"$_src/core/SkPathMeasure.cpp",
"$_src/core/SkPathPriv.h",
"$_src/core/SkPathRef.cpp",
diff --git a/include/core/SkPathEffect.h b/include/core/SkPathEffect.h
index 513e7ec..ec7aa4d 100644
--- a/include/core/SkPathEffect.h
+++ b/include/core/SkPathEffect.h
@@ -60,12 +60,6 @@
*/
bool filterPath(SkPath* dst, const SkPath& src, SkStrokeRec*, const SkRect* cullR) const;
- /**
- * Compute a conservative bounds for its effect, given the src bounds.
- * The baseline implementation just assigns src to dst.
- */
- void computeFastBounds(SkRect* dst, const SkRect& src) const;
-
/** \class PointData
PointData aggregates all the information needed to draw the point
@@ -165,9 +159,6 @@
SkPathEffect() {}
virtual bool onFilterPath(SkPath*, const SkPath&, SkStrokeRec*, const SkRect*) const = 0;
- virtual SkRect onComputeFastBounds(const SkRect& src) const {
- return src;
- }
virtual bool onAsPoints(PointData*, const SkPath&, const SkStrokeRec&, const SkMatrix&,
const SkRect*) const {
return false;
@@ -177,6 +168,15 @@
}
private:
+ friend class SkPathEffectPriv;
+
+ // Compute a conservative bounds for its effect, given the bounds of the path. 'bounds' is
+ // both the input and output; if false is returned, fast bounds could not be calculated and
+ // 'bounds' is undefined.
+ //
+ // If 'bounds' is null, performs a dry-run determining if bounds could be computed.
+ virtual bool computeFastBounds(SkRect* bounds) const = 0;
+
// illegal
SkPathEffect(const SkPathEffect&);
SkPathEffect& operator=(const SkPathEffect&);
diff --git a/include/effects/Sk1DPathEffect.h b/include/effects/Sk1DPathEffect.h
index fb29c4d..68b2576 100644
--- a/include/effects/Sk1DPathEffect.h
+++ b/include/effects/Sk1DPathEffect.h
@@ -32,6 +32,9 @@
virtual SkScalar next(SkPath* dst, SkScalar dist, SkPathMeasure&) const = 0;
private:
+ // For simplicity, assume fast bounds cannot be computed
+ bool computeFastBounds(SkRect*) const override { return false; }
+
using INHERITED = SkPathEffect;
};
diff --git a/include/effects/Sk2DPathEffect.h b/include/effects/Sk2DPathEffect.h
index 77aad93..2f02105 100644
--- a/include/effects/Sk2DPathEffect.h
+++ b/include/effects/Sk2DPathEffect.h
@@ -42,6 +42,9 @@
SkMatrix fMatrix, fInverse;
bool fMatrixIsInvertible;
+ // For simplicity, assume fast bounds cannot be computed
+ bool computeFastBounds(SkRect*) const override { return false; }
+
// illegal
Sk2DPathEffect(const Sk2DPathEffect&);
Sk2DPathEffect& operator=(const Sk2DPathEffect&);
diff --git a/include/effects/SkCornerPathEffect.h b/include/effects/SkCornerPathEffect.h
index 76d5315..dbd59a3 100644
--- a/include/effects/SkCornerPathEffect.h
+++ b/include/effects/SkCornerPathEffect.h
@@ -35,6 +35,12 @@
private:
SK_FLATTENABLE_HOOKS(SkCornerPathEffect)
+ bool computeFastBounds(SkRect*) const override {
+ // Rounding sharp corners within a path produces a new path that is still contained within
+ // the original's bounds, so leave 'bounds' unmodified.
+ return true;
+ }
+
SkScalar fRadius;
using INHERITED = SkPathEffect;
diff --git a/include/effects/SkDiscretePathEffect.h b/include/effects/SkDiscretePathEffect.h
index 6f9d4f7..7f60768 100644
--- a/include/effects/SkDiscretePathEffect.h
+++ b/include/effects/SkDiscretePathEffect.h
@@ -42,6 +42,8 @@
private:
SK_FLATTENABLE_HOOKS(SkDiscretePathEffect)
+ bool computeFastBounds(SkRect* bounds) const override;
+
SkScalar fSegLength, fPerterb;
/* Caller-supplied 32 bit seed assist */
diff --git a/samplecode/SampleTextEffects.cpp b/samplecode/SampleTextEffects.cpp
index 10fd89b..5bc1416 100644
--- a/samplecode/SampleTextEffects.cpp
+++ b/samplecode/SampleTextEffects.cpp
@@ -86,6 +86,8 @@
private:
SK_FLATTENABLE_HOOKS(InverseFillPE)
+ bool computeFastBounds(SkRect* bounds) const override { return false; }
+
using INHERITED = SkPathEffect;
};
diff --git a/src/core/SkPaint.cpp b/src/core/SkPaint.cpp
index 84bb966..0255297 100644
--- a/src/core/SkPaint.cpp
+++ b/src/core/SkPaint.cpp
@@ -26,6 +26,7 @@
#include "src/core/SkOpts.h"
#include "src/core/SkPaintDefaults.h"
#include "src/core/SkPaintPriv.h"
+#include "src/core/SkPathEffectPriv.h"
#include "src/core/SkReadBuffer.h"
#include "src/core/SkSafeRange.h"
#include "src/core/SkStringUtils.h"
@@ -370,6 +371,11 @@
if (this->getImageFilter() && !this->getImageFilter()->canComputeFastBounds()) {
return false;
}
+ // Pass nullptr for the bounds to determine if they can be computed
+ if (this->getPathEffect() &&
+ !SkPathEffectPriv::ComputeFastBounds(this->getPathEffect(), nullptr)) {
+ return false;
+ }
return true;
}
@@ -382,7 +388,8 @@
SkRect tmpSrc;
if (this->getPathEffect()) {
- this->getPathEffect()->computeFastBounds(&tmpSrc, origSrc);
+ tmpSrc = origSrc;
+ SkAssertResult(SkPathEffectPriv::ComputeFastBounds(this->getPathEffect(), &tmpSrc));
src = &tmpSrc;
}
diff --git a/src/core/SkPathEffect.cpp b/src/core/SkPathEffect.cpp
index d3625f0..a14c1bf 100644
--- a/src/core/SkPathEffect.cpp
+++ b/src/core/SkPathEffect.cpp
@@ -7,6 +7,7 @@
#include "include/core/SkPath.h"
#include "include/core/SkPathEffect.h"
+#include "src/core/SkPathEffectPriv.h"
#include "src/core/SkReadBuffer.h"
#include "src/core/SkWriteBuffer.h"
@@ -27,10 +28,6 @@
return false;
}
-void SkPathEffect::computeFastBounds(SkRect* dst, const SkRect& src) const {
- *dst = this->onComputeFastBounds(src);
-}
-
bool SkPathEffect::asPoints(PointData* results, const SkPath& src,
const SkStrokeRec& rec, const SkMatrix& mx, const SkRect* rect) const {
return this->onAsPoints(results, src, rec, mx, rect);
@@ -112,6 +109,12 @@
private:
SK_FLATTENABLE_HOOKS(SkComposePathEffect)
+ bool computeFastBounds(SkRect* bounds) const override {
+ // inner (fPE1) is computed first, automatically updating bounds before computing outer.
+ return SkPathEffectPriv::ComputeFastBounds(fPE1.get(), bounds) &&
+ SkPathEffectPriv::ComputeFastBounds(fPE0.get(), bounds);
+ }
+
// illegal
SkComposePathEffect(const SkComposePathEffect&);
SkComposePathEffect& operator=(const SkComposePathEffect&);
@@ -150,8 +153,6 @@
return sk_sp<SkPathEffect>(new SkSumPathEffect(first, second));
}
- SK_FLATTENABLE_HOOKS(SkSumPathEffect)
-
protected:
SkSumPathEffect(sk_sp<SkPathEffect> first, sk_sp<SkPathEffect> second)
: INHERITED(first, second) {}
@@ -164,6 +165,14 @@
}
private:
+ SK_FLATTENABLE_HOOKS(SkSumPathEffect)
+
+ bool computeFastBounds(SkRect* bounds) const override {
+ // Unlike Compose(), PE0 modifies the path first for Sum
+ return SkPathEffectPriv::ComputeFastBounds(fPE0.get(), bounds) &&
+ SkPathEffectPriv::ComputeFastBounds(fPE1.get(), bounds);
+ }
+
// illegal
SkSumPathEffect(const SkSumPathEffect&);
SkSumPathEffect& operator=(const SkSumPathEffect&);
diff --git a/src/core/SkPathEffectPriv.h b/src/core/SkPathEffectPriv.h
new file mode 100644
index 0000000..145e775
--- /dev/null
+++ b/src/core/SkPathEffectPriv.h
@@ -0,0 +1,25 @@
+/*
+ * Copyright 2021 Google LLC
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef SkPathEffectPriv_DEFINED
+#define SkPathEffectPriv_DEFINED
+
+#include "include/core/SkPathEffect.h"
+
+// Provides access to internal SkPathEffect APIs
+class SkPathEffectPriv {
+public:
+
+ static bool ComputeFastBounds(const SkPathEffect* pe, SkRect* bounds) {
+ return pe->computeFastBounds(bounds);
+ }
+
+private:
+ SkPathEffectPriv();
+};
+
+#endif // SkPathEffectPriv_DEFINED
diff --git a/src/effects/SkDashImpl.h b/src/effects/SkDashImpl.h
index 07acd2f..3fe7e2a 100644
--- a/src/effects/SkDashImpl.h
+++ b/src/effects/SkDashImpl.h
@@ -27,6 +27,12 @@
private:
SK_FLATTENABLE_HOOKS(SkDashImpl)
+ bool computeFastBounds(SkRect* bounds) const override {
+ // Dashing a path returns a subset of the input path so just return true and leave
+ // bounds unmodified
+ return true;
+ }
+
SkScalar* fIntervals;
int32_t fCount;
SkScalar fPhase;
diff --git a/src/effects/SkDiscretePathEffect.cpp b/src/effects/SkDiscretePathEffect.cpp
index 08c39d4..342fe2e 100644
--- a/src/effects/SkDiscretePathEffect.cpp
+++ b/src/effects/SkDiscretePathEffect.cpp
@@ -136,6 +136,14 @@
return true;
}
+bool SkDiscretePathEffect::computeFastBounds(SkRect* bounds) const {
+ if (bounds) {
+ SkScalar maxOutset = SkScalarAbs(fPerterb);
+ bounds->outset(maxOutset, maxOutset);
+ }
+ return true;
+}
+
sk_sp<SkFlattenable> SkDiscretePathEffect::CreateProc(SkReadBuffer& buffer) {
SkScalar segLength = buffer.readScalar();
SkScalar perterb = buffer.readScalar();
diff --git a/src/effects/SkOpPE.h b/src/effects/SkOpPE.h
index 6c5c0ac..fef0781 100644
--- a/src/effects/SkOpPE.h
+++ b/src/effects/SkOpPE.h
@@ -22,6 +22,8 @@
private:
SK_FLATTENABLE_HOOKS(SkOpPE)
+ bool computeFastBounds(SkRect* bounds) const override;
+
sk_sp<SkPathEffect> fOne;
sk_sp<SkPathEffect> fTwo;
SkPathOp fOp;
@@ -40,6 +42,13 @@
private:
SK_FLATTENABLE_HOOKS(SkMatrixPE)
+ bool computeFastBounds(SkRect* bounds) const override {
+ if (bounds) {
+ fMatrix.mapRect(bounds);
+ }
+ return true;
+ }
+
SkMatrix fMatrix;
using INHERITED = SkPathEffect;
@@ -52,11 +61,12 @@
protected:
void flatten(SkWriteBuffer&) const override;
bool onFilterPath(SkPath* dst, const SkPath& src, SkStrokeRec*, const SkRect*) const override;
- // TODO: override onComputeFastBounds (I think)
private:
SK_FLATTENABLE_HOOKS(SkStrokePE)
+ bool computeFastBounds(SkRect* bounds) const override;
+
SkScalar fWidth,
fMiter;
SkPaint::Join fJoin;
@@ -72,13 +82,16 @@
protected:
void flatten(SkWriteBuffer&) const override;
bool onFilterPath(SkPath* dst, const SkPath& src, SkStrokeRec*, const SkRect*) const override;
- // TODO: override onComputeFastBounds (I think)
private:
SK_FLATTENABLE_HOOKS(SkStrokeAndFillPE)
+ bool computeFastBounds(SkRect* bounds) const override {
+ // The effect's bounds depend on the StrokeRect that is not yet available
+ return false;
+ }
+
using INHERITED = SkPathEffect;
};
#endif
-
diff --git a/src/effects/SkOpPathEffect.cpp b/src/effects/SkOpPathEffect.cpp
index 985d7ce..e39cb0c 100644
--- a/src/effects/SkOpPathEffect.cpp
+++ b/src/effects/SkOpPathEffect.cpp
@@ -6,7 +6,9 @@
*/
#include "include/core/SkStrokeRec.h"
+#include "src/core/SkPathEffectPriv.h"
#include "src/core/SkReadBuffer.h"
+#include "src/core/SkRectPriv.h"
#include "src/core/SkWriteBuffer.h"
#include "src/effects/SkOpPE.h"
@@ -38,6 +40,44 @@
return Op(one, two, fOp, dst);
}
+bool SkOpPE::computeFastBounds(SkRect* bounds) const {
+ if (!bounds) {
+ return (!SkToBool(fOne) || SkPathEffectPriv::ComputeFastBounds(fOne.get(), nullptr)) &&
+ (!SkToBool(fTwo) || SkPathEffectPriv::ComputeFastBounds(fTwo.get(), nullptr));
+ }
+
+ // bounds will hold the result of the fOne while b2 holds the result of fTwo's fast bounds
+ SkRect b2 = *bounds;
+ if (fOne && !SkPathEffectPriv::ComputeFastBounds(fOne.get(), bounds)) {
+ return false;
+ }
+ if (fTwo && !SkPathEffectPriv::ComputeFastBounds(fTwo.get(), &b2)) {
+ return false;
+ }
+
+ switch (fOp) {
+ case SkPathOp::kIntersect_SkPathOp:
+ if (!bounds->intersect(b2)) {
+ bounds->setEmpty();
+ }
+ break;
+ case SkPathOp::kDifference_SkPathOp:
+ // (one - two) conservatively leaves one's bounds unmodified
+ break;
+ case SkPathOp::kReverseDifference_SkPathOp:
+ // (two - one) conservatively leaves two's bounds unmodified
+ *bounds = b2;
+ break;
+ case SkPathOp::kXOR_SkPathOp:
+ // fall through to union since XOR computes a subset of regular OR
+ case SkPathOp::kUnion_SkPathOp:
+ bounds->join(b2);
+ break;
+ }
+
+ return true;
+}
+
void SkOpPE::flatten(SkWriteBuffer& buffer) const {
buffer.writeFlattenable(fOne.get());
buffer.writeFlattenable(fTwo.get());
@@ -106,6 +146,16 @@
return rec.applyToPath(dst, src);
}
+bool SkStrokePE::computeFastBounds(SkRect* bounds) const {
+ if (bounds) {
+ SkStrokeRec rec(SkStrokeRec::kFill_InitStyle);
+ rec.setStrokeStyle(fWidth);
+ rec.setStrokeParams(fCap, fJoin, fMiter);
+ bounds->outset(rec.getInflationRadius(), rec.getInflationRadius());
+ }
+ return true;
+}
+
void SkStrokePE::flatten(SkWriteBuffer& buffer) const {
buffer.writeScalar(fWidth);
buffer.writeScalar(fMiter);
diff --git a/src/effects/SkTrimPE.h b/src/effects/SkTrimPE.h
index 8849297..3d84c59 100644
--- a/src/effects/SkTrimPE.h
+++ b/src/effects/SkTrimPE.h
@@ -20,9 +20,17 @@
void flatten(SkWriteBuffer&) const override;
bool onFilterPath(SkPath* dst, const SkPath& src, SkStrokeRec*, const SkRect*) const override;
+
+
private:
SK_FLATTENABLE_HOOKS(SkTrimPE)
+ bool computeFastBounds(SkRect* bounds) const override {
+ // Trimming a path returns a subset of the input path so just return true and leave bounds
+ // unmodified
+ return true;
+ }
+
const SkScalar fStartT,
fStopT;
const SkTrimPathEffect::Mode fMode;
diff --git a/src/gpu/GrStyle.h b/src/gpu/GrStyle.h
index f369f64..6a22e13 100644
--- a/src/gpu/GrStyle.h
+++ b/src/gpu/GrStyle.h
@@ -12,6 +12,7 @@
#include "include/core/SkStrokeRec.h"
#include "include/gpu/GrTypes.h"
#include "include/private/SkTemplates.h"
+#include "src/core/SkPathEffectPriv.h"
/**
* Represents the various ways that a GrStyledShape can be styled. It has fill/stroking information
@@ -170,16 +171,16 @@
/** Given bounds of a path compute the bounds of path with the style applied. */
void adjustBounds(SkRect* dst, const SkRect& src) const {
- if (this->pathEffect()) {
- this->pathEffect()->computeFastBounds(dst, src);
- // This may not be the correct SkStrokeRec to use. skbug.com/5299
- // It happens to work for dashing.
- SkScalar radius = fStrokeRec.getInflationRadius();
- dst->outset(radius, radius);
- } else {
- SkScalar radius = fStrokeRec.getInflationRadius();
- *dst = src.makeOutset(radius, radius);
+ *dst = src;
+ if (this->pathEffect() && !SkPathEffectPriv::ComputeFastBounds(this->pathEffect(), dst)) {
+ // Restore dst == src since ComputeFastBounds leaves it undefined when returning false
+ *dst = src;
}
+
+ // This may not be the correct SkStrokeRec to use if there's a path effect: skbug.com/5299
+ // It happens to work for dashing.
+ SkScalar radius = fStrokeRec.getInflationRadius();
+ dst->outset(radius, radius);
}
private:
diff --git a/src/gpu/GrTestUtils.h b/src/gpu/GrTestUtils.h
index b2e4401..77870a1 100644
--- a/src/gpu/GrTestUtils.h
+++ b/src/gpu/GrTestUtils.h
@@ -83,6 +83,8 @@
private:
TestDashPathEffect(const SkScalar* intervals, int count, SkScalar phase);
+ bool computeFastBounds(SkRect* bounds) const override { return true; }
+
int fCount;
SkAutoTArray<SkScalar> fIntervals;
SkScalar fPhase;
diff --git a/tests/GrStyledShapeTest.cpp b/tests/GrStyledShapeTest.cpp
index 625f624..42b7a2c 100644
--- a/tests/GrStyledShapeTest.cpp
+++ b/tests/GrStyledShapeTest.cpp
@@ -1174,8 +1174,11 @@
return true;
}
- SkRect onComputeFastBounds(const SkRect& src) const override {
- return RRect().getBounds();
+ bool computeFastBounds(SkRect* bounds) const override {
+ if (bounds) {
+ *bounds = RRect().getBounds();
+ }
+ return true;
}
private:
@@ -1258,11 +1261,12 @@
}
return true;
}
- SkRect onComputeFastBounds(const SkRect& src) const override {
- SkRect dst = src;
- SkRectPriv::GrowToInclude(&dst, {0, 0});
- SkRectPriv::GrowToInclude(&dst, {100, 100});
- return dst;
+ bool computeFastBounds(SkRect* bounds) const override {
+ if (bounds) {
+ SkRectPriv::GrowToInclude(bounds, {0, 0});
+ SkRectPriv::GrowToInclude(bounds, {100, 100});
+ }
+ return true;
}
private:
AddLineTosPathEffect() {}
@@ -1303,6 +1307,8 @@
return true;
}
private:
+ bool computeFastBounds(SkRect* bounds) const override { return true; }
+
MakeHairlinePathEffect() {}
};
@@ -1386,8 +1392,11 @@
}
return true;
}
- SkRect onComputeFastBounds(const SkRect& src) const override {
- return { 0, 0, 0, 0 };
+ bool computeFastBounds(SkRect* bounds) const override {
+ if (bounds) {
+ *bounds = { 0, 0, 0, 0 };
+ }
+ return true;
}
private:
bool fInvert;
@@ -1469,6 +1478,8 @@
return false;
}
private:
+ bool computeFastBounds(SkRect* bounds) const override { return false; }
+
FailurePathEffect() {}
};
