From f1d415188ffb4c34e2886c2cfceb363a148333f1 Mon Sep 17 00:00:00 2001 From: caryclark Date: Tue, 9 Feb 2016 10:30:22 -0800 Subject: [PATCH] Add unit test to feed valid SVG sequences to make sure that path strings can be parsed without returning an error. Draw the output through Skia and SVG to make sure they are parsed correctly. R=fmalita@chromium.org BUG=skia:4549 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1675053002 Review URL: https://codereview.chromium.org/1675053002 --- gm/arcto.cpp | 83 +++++++++++++++++++++++++++++++ gyp/tools.gyp | 1 + src/core/SkPath.cpp | 1 + src/utils/SkParsePath.cpp | 49 ++++++++++--------- tests/ParsePathTest.cpp | 17 +++++++ tools/random_parse_path.cpp | 97 +++++++++++++++++++++++++++++++++++++ tools/random_parse_path.h | 17 +++++++ 7 files changed, 242 insertions(+), 23 deletions(-) create mode 100644 tools/random_parse_path.cpp create mode 100644 tools/random_parse_path.h diff --git a/gm/arcto.cpp b/gm/arcto.cpp index aac9bde1b3..432b889fb8 100644 --- a/gm/arcto.cpp +++ b/gm/arcto.cpp @@ -102,3 +102,86 @@ DEF_SIMPLE_GM(arcto, canvas, 500, 600) { path.arcTo(80, 80, 0, SkPath::kLarge_ArcSize, SkPath::kCW_Direction, 200, 100); canvas->drawPath(path, paint); } + +#include "random_parse_path.h" +#include "SkRandom.h" + +/* The test below generates a reference image using SVG. To compare the result for correctness, + enable the define below and then view the generated SVG in a browser. + */ +#define GENERATE_SVG_REFERENCE 0 + +#if GENERATE_SVG_REFERENCE +#include "SkOSFile.h" +#endif + +enum { + kParsePathTestDimension = 500 +}; + +DEF_SIMPLE_GM(parsedpaths, canvas, kParsePathTestDimension, kParsePathTestDimension) { +#if GENERATE_SVG_REFERENCE + FILE* file = sk_fopen("svgout.htm", kWrite_SkFILE_Flag); + SkString str; + str.printf("\n", kParsePathTestDimension, + kParsePathTestDimension); + sk_fwrite(str.c_str(), str.size(), file); +#endif + SkRandom rand; + SkPaint paint; + paint.setAntiAlias(true); + for (int xStart = 0; xStart < kParsePathTestDimension; xStart += 100) { + canvas->save(); + for (int yStart = 0; yStart < kParsePathTestDimension; yStart += 100) { +#if GENERATE_SVG_REFERENCE + str.printf("\n", xStart, yStart, + 1, 1); + sk_fwrite(str.c_str(), str.size(), file); + str.printf("\n", xStart, yStart); + sk_fwrite(str.c_str(), str.size(), file); + str.printf("\n"); + sk_fwrite(str.c_str(), str.size(), file); + str.printf("\n"); + sk_fwrite(str.c_str(), str.size(), file); +#endif + int count = 3; + do { + SkPath path; + SkString spec; + spec.printf("M %d,%d\n", rand.nextRangeU(30, 70), rand.nextRangeU(30, 70)); + uint32_t count = rand.nextRangeU(0, 10); + for (uint32_t i = 0; i < count; ++i) { + spec.append(MakeRandomParsePathPiece(&rand)); + } + SkAssertResult(SkParsePath::FromSVGString(spec.c_str(), &path)); + paint.setColor(rand.nextU()); + canvas->save(); + canvas->clipRect(SkRect::MakeIWH(100, 100)); + canvas->drawPath(path, paint); + canvas->restore(); +#if GENERATE_SVG_REFERENCE + str.printf("\n", xStart, yStart); + sk_fwrite(str.c_str(), str.size(), file); +#endif + } while (--count > 0); +#if GENERATE_SVG_REFERENCE + str.printf("\n"); + sk_fwrite(str.c_str(), str.size(), file); +#endif + canvas->translate(0, 100); + } + canvas->restore(); + canvas->translate(100, 0); + } +#if GENERATE_SVG_REFERENCE + const char trailer[] = "\n"; + sk_fwrite(trailer, sizeof(trailer) - 1, file); + sk_fclose(file); +#endif +} diff --git a/gyp/tools.gyp b/gyp/tools.gyp index 9bcda35543..e428076bb9 100644 --- a/gyp/tools.gyp +++ b/gyp/tools.gyp @@ -136,6 +136,7 @@ 'sources': [ '../tools/sk_tool_utils.cpp', '../tools/sk_tool_utils_font.cpp', + '../tools/random_parse_path.cpp', ], 'include_dirs': [ '../include/private', diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp index f3c2628c4f..dcc3e0202d 100644 --- a/src/core/SkPath.cpp +++ b/src/core/SkPath.cpp @@ -1265,6 +1265,7 @@ void SkPath::arcTo(const SkRect& oval, SkScalar startAngle, SkScalar sweepAngle, // Note that arcSweep bool value is flipped from the original implementation. void SkPath::arcTo(SkScalar rx, SkScalar ry, SkScalar angle, SkPath::ArcSize arcLarge, SkPath::Direction arcSweep, SkScalar x, SkScalar y) { + this->injectMoveToIfNeeded(); SkPoint srcPts[2]; this->getLastPt(&srcPts[0]); // If rx = 0 or ry = 0 then this arc is treated as a straight line segment (a "lineto") diff --git a/src/utils/SkParsePath.cpp b/src/utils/SkParsePath.cpp index c0f39aa06f..8baa6611ee 100644 --- a/src/utils/SkParsePath.cpp +++ b/src/utils/SkParsePath.cpp @@ -40,7 +40,9 @@ static const char* skip_ws(const char str[]) { } static const char* skip_sep(const char str[]) { - SkASSERT(str); + if (!str) { + return nullptr; + } while (is_sep(*str)) str++; return str; @@ -61,6 +63,9 @@ static const char* find_points(const char str[], SkPoint value[], int count, static const char* find_scalar(const char str[], SkScalar* value, bool isRelative, SkScalar relative) { str = SkParse::FindScalar(str, value); + if (!str) { + return nullptr; + } if (isRelative) { *value += relative; } @@ -70,7 +75,7 @@ static const char* find_scalar(const char str[], SkScalar* value, bool SkParsePath::FromSVGString(const char data[], SkPath* result) { SkPath path; - SkPoint f = {0, 0}; + SkPoint first = {0, 0}; SkPoint c = {0, 0}; SkPoint lastc = {0, 0}; SkPoint points[3]; @@ -107,6 +112,7 @@ bool SkParsePath::FromSVGString(const char data[], SkPath* result) { case 'M': data = find_points(data, points, 1, relative, &c); path.moveTo(points[0]); + previousOp = '\0'; op = 'L'; c = points[0]; break; @@ -147,10 +153,10 @@ bool SkParsePath::FromSVGString(const char data[], SkPath* result) { goto quadraticCommon; case 'T': data = find_points(data, &points[1], 1, relative, &c); - points[0] = points[1]; + points[0] = c; if (previousOp == 'Q' || previousOp == 'T') { - points[0].fX = c.fX * 2 - lastc.fX; - points[0].fY = c.fY * 2 - lastc.fY; + points[0].fX -= lastc.fX - c.fX; + points[0].fY -= lastc.fY - c.fY; } quadraticCommon: path.quadTo(points[0], points[1]); @@ -159,27 +165,24 @@ bool SkParsePath::FromSVGString(const char data[], SkPath* result) { break; case 'A': { SkPoint radii; - data = find_points(data, &radii, 1, false, nullptr); SkScalar angle, largeArc, sweep; - data = find_scalar(data, &angle, false, 0); - data = find_scalar(data, &largeArc, false, 0); - data = find_scalar(data, &sweep, false, 0); - data = find_points(data, &points[0], 1, relative, &c); - path.arcTo(radii, angle, (SkPath::ArcSize) SkToBool(largeArc), - (SkPath::Direction) !SkToBool(sweep), points[0]); + if ((data = find_points(data, &radii, 1, false, nullptr)) + && (data = skip_sep(data)) + && (data = find_scalar(data, &angle, false, 0)) + && (data = skip_sep(data)) + && (data = find_scalar(data, &largeArc, false, 0)) + && (data = skip_sep(data)) + && (data = find_scalar(data, &sweep, false, 0)) + && (data = skip_sep(data)) + && (data = find_points(data, &points[0], 1, relative, &c))) { + path.arcTo(radii, angle, (SkPath::ArcSize) SkToBool(largeArc), + (SkPath::Direction) !SkToBool(sweep), points[0]); + path.getLastPt(&c); + } } break; case 'Z': path.close(); -#if 0 // !!! still a bug? - if (fPath.isEmpty() && (f.fX != 0 || f.fY != 0)) { - c.fX -= SkScalar.Epsilon; // !!! enough? - fPath.moveTo(c); - fPath.lineTo(f); - fPath.close(); - } -#endif - c = f; - op = '\0'; + c = first; break; case '~': { SkPoint args[2]; @@ -191,7 +194,7 @@ bool SkParsePath::FromSVGString(const char data[], SkPath* result) { return false; } if (previousOp == 0) { - f = c; + first = c; } previousOp = op; } diff --git a/tests/ParsePathTest.cpp b/tests/ParsePathTest.cpp index 561eed04b5..fa239c2b3d 100644 --- a/tests/ParsePathTest.cpp +++ b/tests/ParsePathTest.cpp @@ -71,3 +71,20 @@ DEF_TEST(ParsePath_invalid, r) { bool success = SkParsePath::FromSVGString("M 5", &path); REPORTER_ASSERT(r, !success); } + +#include "random_parse_path.h" +#include "SkRandom.h" + +DEF_TEST(ParsePathRandom, r) { + SkRandom rand; + for (int index = 0; index < 1000; ++index) { + SkPath path, path2; + SkString spec; + uint32_t count = rand.nextRangeU(0, 10); + for (uint32_t i = 0; i < count; ++i) { + spec.append(MakeRandomParsePathPiece(&rand)); + } + bool success = SkParsePath::FromSVGString(spec.c_str(), &path); + REPORTER_ASSERT(r, success); + } +} diff --git a/tools/random_parse_path.cpp b/tools/random_parse_path.cpp new file mode 100644 index 0000000000..ffc4b58ef6 --- /dev/null +++ b/tools/random_parse_path.cpp @@ -0,0 +1,97 @@ +/* + * Copyright 2016 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include "SkRandom.h" +#include "random_parse_path.h" + +const struct Legal { + char fSymbol; + int fScalars; +} gLegal[] = { + { 'M', 2 }, + { 'H', 1 }, + { 'V', 1 }, + { 'L', 2 }, + { 'Q', 4 }, + { 'T', 2 }, + { 'C', 6 }, + { 'S', 4 }, + { 'A', 4 }, + { 'Z', 0 }, +}; + +bool gEasy = false; // set to true while debugging to suppress unusual whitespace + +// mostly do nothing, then bias towards spaces +const char gWhiteSpace[] = { 0, 0, 0, 0, 0, 0, 0, 0, ' ', ' ', ' ', ' ', 0x09, 0x0D, 0x0A }; + +static void add_white(SkRandom* rand, SkString* atom) { + if (gEasy) { + atom->append(" "); + return; + } + int reps = rand->nextRangeU(0, 2); + for (int rep = 0; rep < reps; ++rep) { + int index = rand->nextRangeU(0, (int) SK_ARRAY_COUNT(gWhiteSpace) - 1); + if (gWhiteSpace[index]) { + atom->append(&gWhiteSpace[index], 1); + } + } +} + +static void add_comma(SkRandom* rand, SkString* atom) { + if (gEasy) { + atom->append(","); + return; + } + size_t count = atom->size(); + add_white(rand, atom); + if (rand->nextBool()) { + atom->append(","); + } + do { + add_white(rand, atom); + } while (count == atom->size()); +} + +static void add_some_white(SkRandom* rand, SkString* atom) { + size_t count = atom->size(); + do { + add_white(rand, atom); + } while (count == atom->size()); +} + +SkString MakeRandomParsePathPiece(SkRandom* rand) { + SkString atom; + int index = rand->nextRangeU(0, (int) SK_ARRAY_COUNT(gLegal) - 1); + const Legal& legal = gLegal[index]; + gEasy ? atom.append("\n") : add_white(rand, &atom); + char symbol = legal.fSymbol | (rand->nextBool() ? 0x20 : 0); + atom.append(&symbol, 1); + int reps = rand->nextRangeU(1, 3); + for (int rep = 0; rep < reps; ++rep) { + for (int index = 0; index < legal.fScalars; ++index) { + SkScalar coord = rand->nextRangeF(0, 100); + add_white(rand, &atom); + atom.appendScalar(coord); + if (rep < reps - 1 && index < legal.fScalars - 1) { + add_comma(rand, &atom); + } else { + add_some_white(rand, &atom); + } + if ('A' == legal.fSymbol && 1 == index) { + atom.appendScalar(rand->nextRangeF(-720, 720)); + add_comma(rand, &atom); + atom.appendU32(rand->nextRangeU(0, 1)); + add_comma(rand, &atom); + atom.appendU32(rand->nextRangeU(0, 1)); + add_comma(rand, &atom); + } + } + } + return atom; +} diff --git a/tools/random_parse_path.h b/tools/random_parse_path.h new file mode 100644 index 0000000000..4dfa5015cc --- /dev/null +++ b/tools/random_parse_path.h @@ -0,0 +1,17 @@ +/* + * Copyright 2016 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#ifndef random_parse_path_DEFINED +#define random_parse_path_DEFINED + +#include "SkString.h" + +class SkRandom; + +SkString MakeRandomParsePathPiece(SkRandom* rand); + +#endif