aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
Diffstat
-rw-r--r--bench/DashBench.cpp5
-rw-r--r--gm/arcto.cpp21
-rw-r--r--gm/dashing.cpp3
-rw-r--r--src/effects/SkDashPathEffect.cpp14
-rw-r--r--src/utils/SkDashPath.cpp83
-rw-r--r--src/utils/SkDashPathPriv.h16
-rw-r--r--tests/DashPathEffectTest.cpp11
7 files changed, 92 insertions, 61 deletions
diff --git a/bench/DashBench.cpp b/bench/DashBench.cpp
index eb964f6bf1..e3d8364013 100644
--- a/bench/DashBench.cpp
+++ b/bench/DashBench.cpp
@@ -72,8 +72,9 @@ protected:
SkPath path;
this->makePath(&path);
- paint.setPathEffect(SkDashPathEffect::Create(fIntervals.begin(),
- fIntervals.count(), 0))->unref();
+ SkAutoTUnref<SkPathEffect> effect(SkDashPathEffect::Create(fIntervals.begin(),
+ fIntervals.count(), 0));
+ paint.setPathEffect(effect);
if (fDoClip) {
SkRect r = path.getBounds();
diff --git a/gm/arcto.cpp b/gm/arcto.cpp
index cd26eef35a..584fca3c4f 100644
--- a/gm/arcto.cpp
+++ b/gm/arcto.cpp
@@ -205,3 +205,24 @@ DEF_SIMPLE_GM(bug593049, canvas, 300, 300) {
canvas->drawPath(p, paint);
}
+
+#include "SkDashPathEffect.h"
+#include "SkPathMeasure.h"
+
+DEF_SIMPLE_GM(bug583299, canvas, 300, 300) {
+ const char* d="M60,60 A50,50 0 0 0 160,60 A50,50 0 0 0 60,60z";
+ SkPaint p;
+ p.setStyle(SkPaint::kStroke_Style);
+ p.setStrokeWidth(100);
+ p.setAntiAlias(true);
+ p.setColor(0xFF008200);
+ p.setStrokeCap(SkPaint::kSquare_Cap);
+ SkPath path;
+ SkParsePath::FromSVGString(d, &path);
+ SkPathMeasure meas(path, false);
+ SkScalar length = meas.getLength();
+ SkScalar intervals[] = {0, length };
+ int intervalCount = (int) SK_ARRAY_COUNT(intervals);
+ p.setPathEffect(SkDashPathEffect::Create(intervals, intervalCount, 0))->unref();
+ canvas->drawPath(path, p);
+}
diff --git a/gm/dashing.cpp b/gm/dashing.cpp
index dfd1b629c1..c728d15a1a 100644
--- a/gm/dashing.cpp
+++ b/gm/dashing.cpp
@@ -21,7 +21,8 @@ static void drawline(SkCanvas* canvas, int on, int off, const SkPaint& paint,
SkIntToScalar(off),
};
- p.setPathEffect(SkDashPathEffect::Create(intervals, 2, phase))->unref();
+ SkAutoTUnref<SkPathEffect> effect(SkDashPathEffect::Create(intervals, 2, phase));
+ p.setPathEffect(effect);
canvas->drawLine(startX, startY, finalX, finalY, p);
}
diff --git a/src/effects/SkDashPathEffect.cpp b/src/effects/SkDashPathEffect.cpp
index ced0aab69a..3816499915 100644
--- a/src/effects/SkDashPathEffect.cpp
+++ b/src/effects/SkDashPathEffect.cpp
@@ -14,7 +14,7 @@
SkDashPathEffect::SkDashPathEffect(const SkScalar intervals[], int count, SkScalar phase)
: fPhase(0)
- , fInitialDashLength(0)
+ , fInitialDashLength(-1)
, fInitialDashIndex(0)
, fIntervalLength(0) {
SkASSERT(intervals);
@@ -23,7 +23,6 @@ SkDashPathEffect::SkDashPathEffect(const SkScalar intervals[], int count, SkScal
fIntervals = (SkScalar*)sk_malloc_throw(sizeof(SkScalar) * count);
fCount = count;
for (int i = 0; i < count; i++) {
- SkASSERT(intervals[i] >= 0);
fIntervals[i] = intervals[i];
}
@@ -38,7 +37,7 @@ SkDashPathEffect::~SkDashPathEffect() {
bool SkDashPathEffect::filterPath(SkPath* dst, const SkPath& src,
SkStrokeRec* rec, const SkRect* cullRect) const {
- return SkDashPath::FilterDashPath(dst, src, rec, cullRect, fIntervals, fCount,
+ return SkDashPath::InternalFilter(dst, src, rec, cullRect, fIntervals, fCount,
fInitialDashLength, fInitialDashIndex, fIntervalLength);
}
@@ -162,7 +161,7 @@ bool SkDashPathEffect::asPoints(PointData* results,
const SkMatrix& matrix,
const SkRect* cullRect) const {
// width < 0 -> fill && width == 0 -> hairline so requiring width > 0 rules both out
- if (fInitialDashLength < 0 || 0 >= rec.getWidth()) {
+ if (0 >= rec.getWidth()) {
return false;
}
@@ -388,13 +387,8 @@ void SkDashPathEffect::toString(SkString* str) const {
//////////////////////////////////////////////////////////////////////////////////////////////////
SkPathEffect* SkDashPathEffect::Create(const SkScalar intervals[], int count, SkScalar phase) {
- if ((count < 2) || !SkIsAlign2(count)) {
+ if (!SkDashPath::ValidDashPath(phase, intervals, count)) {
return nullptr;
}
- for (int i = 0; i < count; i++) {
- if (intervals[i] < 0) {
- return nullptr;
- }
- }
return new SkDashPathEffect(intervals, count, phase);
}
diff --git a/src/utils/SkDashPath.cpp b/src/utils/SkDashPath.cpp
index 0d2783eba2..d766b0d53e 100644
--- a/src/utils/SkDashPath.cpp
+++ b/src/utils/SkDashPath.cpp
@@ -40,42 +40,35 @@ void SkDashPath::CalcDashParameters(SkScalar phase, const SkScalar intervals[],
len += intervals[i];
}
*intervalLength = len;
-
- // watch out for values that might make us go out of bounds
- if ((len > 0) && SkScalarIsFinite(phase) && SkScalarIsFinite(len)) {
-
- // Adjust phase to be between 0 and len, "flipping" phase if negative.
- // e.g., if len is 100, then phase of -20 (or -120) is equivalent to 80
- if (adjustedPhase) {
- if (phase < 0) {
- phase = -phase;
- if (phase > len) {
- phase = SkScalarMod(phase, len);
- }
- phase = len - phase;
-
- // Due to finite precision, it's possible that phase == len,
- // even after the subtract (if len >>> phase), so fix that here.
- // This fixes http://crbug.com/124652 .
- SkASSERT(phase <= len);
- if (phase == len) {
- phase = 0;
- }
- } else if (phase >= len) {
+ // Adjust phase to be between 0 and len, "flipping" phase if negative.
+ // e.g., if len is 100, then phase of -20 (or -120) is equivalent to 80
+ if (adjustedPhase) {
+ if (phase < 0) {
+ phase = -phase;
+ if (phase > len) {
phase = SkScalarMod(phase, len);
}
- *adjustedPhase = phase;
+ phase = len - phase;
+
+ // Due to finite precision, it's possible that phase == len,
+ // even after the subtract (if len >>> phase), so fix that here.
+ // This fixes http://crbug.com/124652 .
+ SkASSERT(phase <= len);
+ if (phase == len) {
+ phase = 0;
+ }
+ } else if (phase >= len) {
+ phase = SkScalarMod(phase, len);
}
- SkASSERT(phase >= 0 && phase < len);
+ *adjustedPhase = phase;
+ }
+ SkASSERT(phase >= 0 && phase < len);
- *initialDashLength = find_first_interval(intervals, phase,
- initialDashIndex, count);
+ *initialDashLength = find_first_interval(intervals, phase,
+ initialDashIndex, count);
- SkASSERT(*initialDashLength >= 0);
- SkASSERT(*initialDashIndex >= 0 && *initialDashIndex < count);
- } else {
- *initialDashLength = -1; // signal bad dash intervals
- }
+ SkASSERT(*initialDashLength >= 0);
+ SkASSERT(*initialDashIndex >= 0 && *initialDashIndex < count);
}
static void outset_for_stroke(SkRect* rect, const SkStrokeRec& rec) {
@@ -220,13 +213,13 @@ private:
};
-bool SkDashPath::FilterDashPath(SkPath* dst, const SkPath& src, SkStrokeRec* rec,
+bool SkDashPath::InternalFilter(SkPath* dst, const SkPath& src, SkStrokeRec* rec,
const SkRect* cullRect, const SkScalar aIntervals[],
int32_t count, SkScalar initialDashLength, int32_t initialDashIndex,
SkScalar intervalLength) {
- // we do nothing if the src wants to be filled, or if our dashlength is 0
- if (rec->isFillStyle() || initialDashLength < 0) {
+ // we do nothing if the src wants to be filled
+ if (rec->isFillStyle()) {
return false;
}
@@ -306,7 +299,7 @@ bool SkDashPath::FilterDashPath(SkPath* dst, const SkPath& src, SkStrokeRec* rec
// extend if we ended on a segment and we need to join up with the (skipped) initial segment
if (meas.isClosed() && is_even(initialDashIndex) &&
- initialDashLength > 0) {
+ initialDashLength >= 0) {
meas.getSegment(0, initialDashLength, dst, !addedSegment);
++segCount;
}
@@ -321,11 +314,29 @@ bool SkDashPath::FilterDashPath(SkPath* dst, const SkPath& src, SkStrokeRec* rec
bool SkDashPath::FilterDashPath(SkPath* dst, const SkPath& src, SkStrokeRec* rec,
const SkRect* cullRect, const SkPathEffect::DashInfo& info) {
+ if (!ValidDashPath(info.fPhase, info.fIntervals, info.fCount)) {
+ return false;
+ }
SkScalar initialDashLength = 0;
int32_t initialDashIndex = 0;
SkScalar intervalLength = 0;
CalcDashParameters(info.fPhase, info.fIntervals, info.fCount,
&initialDashLength, &initialDashIndex, &intervalLength);
- return FilterDashPath(dst, src, rec, cullRect, info.fIntervals, info.fCount, initialDashLength,
+ return InternalFilter(dst, src, rec, cullRect, info.fIntervals, info.fCount, initialDashLength,
initialDashIndex, intervalLength);
}
+
+bool SkDashPath::ValidDashPath(SkScalar phase, const SkScalar intervals[], int32_t count) {
+ if (count < 2 || !SkIsAlign2(count)) {
+ return false;
+ }
+ SkScalar length = 0;
+ for (int i = 0; i < count; i++) {
+ if (intervals[i] < 0) {
+ return false;
+ }
+ length += intervals[i];
+ }
+ // watch out for values that might make us go out of bounds
+ return length > 0 && SkScalarIsFinite(phase) && SkScalarIsFinite(length);
+}
diff --git a/src/utils/SkDashPathPriv.h b/src/utils/SkDashPathPriv.h
index 7453dcec82..54bf9a4870 100644
--- a/src/utils/SkDashPathPriv.h
+++ b/src/utils/SkDashPathPriv.h
@@ -16,17 +16,25 @@ namespace SkDashPath {
* inputed phase and intervals. If adjustedPhase is passed in, then the phase will be
* adjusted to be between 0 and intervalLength. The result will be stored in adjustedPhase.
* If adjustedPhase is nullptr then it is assumed phase is already between 0 and intervalLength
+ *
+ * Caller should have already used ValidDashPath to exclude invalid data.
*/
void CalcDashParameters(SkScalar phase, const SkScalar intervals[], int32_t count,
SkScalar* initialDashLength, int32_t* initialDashIndex,
SkScalar* intervalLength, SkScalar* adjustedPhase = nullptr);
bool FilterDashPath(SkPath* dst, const SkPath& src, SkStrokeRec*, const SkRect*,
- const SkScalar aIntervals[], int32_t count, SkScalar initialDashLength,
- int32_t initialDashIndex, SkScalar intervalLength);
-
- bool FilterDashPath(SkPath* dst, const SkPath& src, SkStrokeRec*, const SkRect*,
const SkPathEffect::DashInfo& info);
+
+ /*
+ * Caller should have already used ValidDashPath to exclude invalid data.
+ */
+ bool InternalFilter(SkPath* dst, const SkPath& src, SkStrokeRec* rec,
+ const SkRect* cullRect, const SkScalar aIntervals[],
+ int32_t count, SkScalar initialDashLength, int32_t initialDashIndex,
+ SkScalar intervalLength);
+
+ bool ValidDashPath(SkScalar phase, const SkScalar intervals[], int32_t count);
}
#endif
diff --git a/tests/DashPathEffectTest.cpp b/tests/DashPathEffectTest.cpp
index 68fce9a142..fa2395ef75 100644
--- a/tests/DashPathEffectTest.cpp
+++ b/tests/DashPathEffectTest.cpp
@@ -12,20 +12,15 @@
#include "SkStrokeRec.h"
// crbug.com/348821 was rooted in SkDashPathEffect refusing to flatten and unflatten itself when
-// fInitialDashLength < 0 (a signal the effect is nonsense). Here we test that it flattens.
+// the effect is nonsense. Here we test that it fails when passed nonsense parameters.
DEF_TEST(DashPathEffectTest_crbug_348821, r) {
SkScalar intervals[] = { 1.76934361e+36f, 2.80259693e-45f }; // Values from bug.
const int count = 2;
- SkScalar phase = SK_ScalarInfinity; // Used to force the bad fInitialDashLength = -1 path.
+ SkScalar phase = SK_ScalarInfinity; // Used to force a nonsense effect.
SkAutoTUnref<SkPathEffect> dash(SkDashPathEffect::Create(intervals, count, phase));
- // nullptr -> refuses to work with flattening framework.
- REPORTER_ASSERT(r, dash->getFactory() != nullptr);
-
- SkWriteBuffer buffer;
- buffer.writeFlattenable(dash);
- REPORTER_ASSERT(r, buffer.bytesWritten() > 12); // We'd write 12 if broken, >=40 if not.
+ REPORTER_ASSERT(r, dash == nullptr);
}
// Test out the asPoint culling behavior.
generated by cgit v1.2.3 (git 2.39.1) at 2024-05-30 04:14:42 +0000