AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Revert of allow one zero length dash (patchset #8 id:140001 of https://codereview.chromium.org/1805963002/ )

Browse Source 
Reason for revert:
Causes the dash bench to crash.

Example crash:
https://build.chromium.org/p/client.skia/builders/Perf-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release/builds/5581/steps/nanobench/logs/stdio

Original issue's description:
> allow one zero length dash
>
> If the constructed stroke that represents a dash has a
> single dash of length zero, and the end cap is square or
> round, draw the cap.
>
> The old code initialized the initial dash length to zero,
> making it ambiguous whether the first length is zero or
> not.
>
> R=robertphillips@google.com
> BUG=583299
> GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1805963002
>
> Committed: https://skia.googlesource.com/skia/+/5e1a24808415df2748822e8082e21a361362cdfe

TBR=robertphillips@google.com,reed@google.com,caryclark@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=583299

Review URL: https://codereview.chromium.org/1808303004
This commit is contained in:
bungeman 2016-03-18 05:10:23 -07:00 committed by Commit bot
parent 5e1a248084
commit 6f0749cfc7
6 changed files with 60 additions and 90 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
gm/arcto.cpp
View File

@ -205,24 +205,3 @@ 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);
}

3
gm/dashing.cpp
View File

@ -21,8 +21,7 @@ static void drawline(SkCanvas* canvas, int on, int off, const SkPaint& paint,
SkIntToScalar(off),
};
SkAutoTUnref<SkPathEffect> effect(SkDashPathEffect::Create(intervals, 2, phase));
p.setPathEffect(effect);
p.setPathEffect(SkDashPathEffect::Create(intervals, 2, phase))->unref();
canvas->drawLine(startX, startY, finalX, finalY, p);
}

14
src/effects/SkDashPathEffect.cpp
View File

@ -14,7 +14,7 @@
SkDashPathEffect::SkDashPathEffect(const SkScalar intervals[], int count, SkScalar phase)
: fPhase(0)
, fInitialDashLength(-1)
, fInitialDashLength(0)
, fInitialDashIndex(0)
, fIntervalLength(0) {
SkASSERT(intervals);
@ -23,6 +23,7 @@ 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];
}
@ -37,7 +38,7 @@ SkDashPathEffect::~SkDashPathEffect() {
bool SkDashPathEffect::filterPath(SkPath* dst, const SkPath& src,
SkStrokeRec* rec, const SkRect* cullRect) const {
return SkDashPath::InternalFilter(dst, src, rec, cullRect, fIntervals, fCount,
return SkDashPath::FilterDashPath(dst, src, rec, cullRect, fIntervals, fCount,
fInitialDashLength, fInitialDashIndex, fIntervalLength);
}
@ -161,7 +162,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 (0 >= rec.getWidth()) {
if (fInitialDashLength < 0 || 0 >= rec.getWidth()) {
return false;
}
@ -387,8 +388,13 @@ void SkDashPathEffect::toString(SkString* str) const {
//////////////////////////////////////////////////////////////////////////////////////////////////
SkPathEffect* SkDashPathEffect::Create(const SkScalar intervals[], int count, SkScalar phase) {
if (!SkDashPath::ValidDashPath(phase, intervals, count)) {
if ((count < 2) || !SkIsAlign2(count)) {
return nullptr;
}
for (int i = 0; i < count; i++) {
if (intervals[i] < 0) {
return nullptr;
}
}
return new SkDashPathEffect(intervals, count, phase);
}

85
src/utils/SkDashPath.cpp
View File

@ -40,35 +40,42 @@ void SkDashPath::CalcDashParameters(SkScalar phase, const SkScalar intervals[],
len += intervals[i];
}
*intervalLength = 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) {
// 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) {
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) {
phase = SkScalarMod(phase, len);
*adjustedPhase = phase;
}
*adjustedPhase = phase;
SkASSERT(phase >= 0 && phase < len);
*initialDashLength = find_first_interval(intervals, phase,
initialDashIndex, count);
SkASSERT(*initialDashLength >= 0);
SkASSERT(*initialDashIndex >= 0 && *initialDashIndex < count);
} else {
*initialDashLength = -1; // signal bad dash intervals
}
SkASSERT(phase >= 0 && phase < len);
*initialDashLength = find_first_interval(intervals, phase,
initialDashIndex, count);
SkASSERT(*initialDashLength >= 0);
SkASSERT(*initialDashIndex >= 0 && *initialDashIndex < count);
}
static void outset_for_stroke(SkRect* rect, const SkStrokeRec& rec) {
@ -213,13 +220,13 @@ private:
};
bool SkDashPath::InternalFilter(SkPath* dst, const SkPath& src, SkStrokeRec* rec,
bool SkDashPath::FilterDashPath(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
if (rec->isFillStyle()) {
// we do nothing if the src wants to be filled, or if our dashlength is 0
if (rec->isFillStyle() || initialDashLength < 0) {
return false;
}
@ -299,7 +306,7 @@ bool SkDashPath::InternalFilter(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;
}
@ -314,29 +321,11 @@ bool SkDashPath::InternalFilter(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 InternalFilter(dst, src, rec, cullRect, info.fIntervals, info.fCount, initialDashLength,
return FilterDashPath(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);
}

16
src/utils/SkDashPathPriv.h
View File

@ -16,25 +16,17 @@ 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

11
tests/DashPathEffectTest.cpp
View File

@ -12,15 +12,20 @@
#include "SkStrokeRec.h"
// crbug.com/348821 was rooted in SkDashPathEffect refusing to flatten and unflatten itself when
// the effect is nonsense. Here we test that it fails when passed nonsense parameters.
// fInitialDashLength < 0 (a signal the effect is nonsense). Here we test that it flattens.
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 a nonsense effect.
SkScalar phase = SK_ScalarInfinity; // Used to force the bad fInitialDashLength = -1 path.
SkAutoTUnref<SkPathEffect> dash(SkDashPathEffect::Create(intervals, count, phase));
REPORTER_ASSERT(r, dash == nullptr);
// 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.
}
// Test out the asPoint culling behavior.