Commit Graph

16 Commits

Author SHA1 Message Date
caryclark
7b33bf1d2a fix builder fuzz
Fix ASAN_heap-use-after-free crash

R=kjlubick
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2168963002

Review-Url: https://codereview.chromium.org/2168963002
2016-07-21 08:53:32 -07:00
caryclark
013e0e6d48 add fail condition to addexpanded
If coincident pairs don't match,
give up rather than deref null.

R=fmalita@chromium.org
BUG=618991
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2058773002

Review-Url: https://codereview.chromium.org/2058773002
2016-06-10 07:59:50 -07:00
caryclark
dae6b97705 fix pathops fuzz bugs
Fail out in a couple of new places when the input data is very
large and exceeds the limits of the pathops machinery.

Most of the change here plumbs in a way to exclude an assert in
one of these exceptional cases. The current SkAddIntersection
implementation and the inner functions it calls has no way to
report an error to the root caller for an early exit, so rather
than add that in, exclude the assert when the test that would
trigger it runs (allowing the test to otherwise ensure that it
properly fails).

TBR=reed@google.com
BUG=617586,617635
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2046713003

Review-Url: https://codereview.chromium.org/2046713003
2016-06-08 04:28:19 -07:00
caryclark
364a0074c3 path ops: check for deleted ends
TBR=fmalita@chromium.org
BUG=569540

Review URL: https://codereview.chromium.org/1524803002
2015-12-14 08:43:21 -08:00
caryclark
580c40ae71 link test to bug
crbug.com/526025 includes a minimized SVG test case.
Translating that test case into native code (fuzzTNG)
did not reproduce the bug. That test case should
have not been included with skia issue 1323813003,
and is deleted here.

Running the minimal test case in a modified version
of chrome isolated the bug. The modified version
generated the test fuzz763_3 with the edit

#define DEBUGGING_PATHOPS_FROM_HOST 1

in src/pathops/SkPathopsOp.cpp line 188.

Rename fuzz763_3 to issue_526025 to associate the test
with the bug. Note that the bug contains the body of the
CL in comment $5.

R=reed@google.com

Review URL: https://codereview.chromium.org/1315503005
2015-09-01 06:22:36 -07:00
caryclark
ae576b733d remove duplicate linked list adjustment
The list of intersection points on a curve segment may have
entries that can be safely removed when nearby points have
nearly the same t value and point value. When a path includes
very large curves as well as small ones, as is the case with
this fuzzer, additional points may lie between the similar
points that do not meet the nearby criteria.

After merging the nearby point with its doppelganger,
SkOpSegment::moveNearby() unnecessarily set the doppelganger's
next pointer to the one following the nearby point. While
this usually has no effect, since the merge already updated
the linked list, the explicit call removes the additional
outlier points from the segment.

TBR=reed@google.com
BUG=526025

Review URL: https://codereview.chromium.org/1323813003
2015-08-31 09:46:20 -07:00
halcanary
96fcdcc219 Style Change: NULL->nullptr
DOCS_PREVIEW= https://skia.org/?cl=1316233002

Review URL: https://codereview.chromium.org/1316233002
2015-08-27 07:41:16 -07:00
caryclark
d4349723fa fix path ops fuzz buster
Mark collapsed segments as done and remove collapsed
segment references from the coincidence array.

Also add test names to global debugging.

R=fmalita@chromium.org
BUG=512592

Review URL: https://codereview.chromium.org/1250293002
2015-07-23 12:40:22 -07:00
caryclark
218f21ac09 don't fix winding for empty paths
A path can be non-empty but become empty when it is simplified.
For instance, a path with the same rectangle, twice, with opposite
windings.

No contours are created for empty paths, so don't try to
fix their winding direction.

Additionally, check for a NULL coincidence since the
OpBuilder assumes that no concidence edges can be present
after the paths are simplified. This code should not get
called, but it's worth the future-proofing to check.

R=fmalita@chromium.org
BUG=502792

Review URL: https://codereview.chromium.org/1218863005
2015-06-29 11:41:52 -07:00
caryclark
4e1a4c9399 fix builder winding again
Record the nesting level when finding the edge winding contribution
so that inner edges can be reversed as needed.

R=fmalita@chromium.org
BUG=skia:3838

Review URL: https://codereview.chromium.org/1140383002
2015-05-18 12:56:58 -07:00
caryclark
5b5ddd73b4 The path ops builder code needs to determine the winding of each contour added, and reverse windings if the contours are nested in other contours.
Cheap (one contour) paths can be evaluated and reversed as needed with a minimum of checking, but multi-contour paths invoke the regular path ops machinery to determine who is contained by whom.

More tests need to be added to verify that all corner cases are considered, but this fixes the cases in the bug thus far.

R=fmalita@chromium.org
TBR=reed@google.com
BUG=skia:3838

Review URL: https://codereview.chromium.org/1129193006
2015-05-18 05:12:56 -07:00
caryclark
fba9da78ac fix path op builder
The rewrite of path ops caused the inner contour direction to be reversed.
This exposed an existing bug in path ops builder, namely that the implicit
winding of the internal sum path could hide inner contours if they ended
up in the wrong direction.

Setting the sum path's fill type to even-odd ensures that the inner
contours aren't discarded.

R=fmalita@chromium.org
BUG=skia:3838

Review URL: https://codereview.chromium.org/1126193004
2015-05-14 14:18:13 -07:00
caryclark
54359294a7 cumulative pathops patch
Replace the implicit curve intersection with a geometric curve intersection. The implicit intersection proved mathematically unstable and took a long time to zero in on an answer.

Use pointers instead of indices to refer to parts of curves. Indices required awkward renumbering.

Unify t and point values so that small intervals can be eliminated in one pass.

Break cubics up front to eliminate loops and cusps.

Make the Simplify and Op code more regular and eliminate arbitrary differences.

Add a builder that takes an array of paths and operators.

Delete unused code.

BUG=skia:3588
R=reed@google.com

Review URL: https://codereview.chromium.org/1037573004
2015-03-26 07:52:43 -07:00
reed
0dc4dd6dda Revert of pathops version two (patchset #16 id:150001 of https://codereview.chromium.org/1002693002/)
Reason for revert:
ASAN investigation

Original issue's description:
> pathops version two
>
> R=reed@google.com
>
> marked 'no commit' to attempt to get trybots to run
>
> TBR=reed@google.com
>
> Committed: https://skia.googlesource.com/skia/+/ccec0f958ffc71a9986d236bc2eb335cb2111119

TBR=caryclark@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Review URL: https://codereview.chromium.org/1029993002
2015-03-24 13:55:33 -07:00
caryclark
ccec0f958f pathops version two
R=reed@google.com

marked 'no commit' to attempt to get trybots to run

TBR=reed@google.com

Review URL: https://codereview.chromium.org/1002693002
2015-03-24 07:28:17 -07:00
caryclark
45fa447460 new files for pathops geometric intersection
There's no gyp references to these new files,
so this should only have the effect of reducing
the size of the commit that turns this code on.

TBR=

Review URL: https://codereview.chromium.org/853223002
2015-01-16 07:04:10 -08:00