Commit Graph

16 Commits

Author SHA1 Message Date
Herb Derby
c3cc5fa6de Move from SkChunkAlloc to SkArenaAlloc for PathOps
Attempt two. Remove ~SkOpContour because it is handled by the SkArenaAlloc.

Change-Id: Id3049db97aebcc1009d403a031f2fac219f58f2f
Reviewed-on: https://skia-review.googlesource.com/9381
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Herb Derby <herb@google.com>
2017-03-07 17:56:41 +00:00
Derek Sollenberger
c0a4c4f969 Revert "Move from SkChunkAlloc to SkArenaAlloc for PathOps"
This reverts commit 38c6018024.

Reason for revert: breaking ASAN run in TAP build

Original change's description:
> Move from SkChunkAlloc to SkArenaAlloc for PathOps
> 
> Change-Id: Iab111a4ebcae4e896b1fdfe285def9ef0ae2ab6b
> Reviewed-on: https://skia-review.googlesource.com/7314
> Reviewed-by: Cary Clark <caryclark@google.com>
> Commit-Queue: Herb Derby <herb@google.com>
> 

TBR=herb@google.com,caryclark@google.com,reviews@skia.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Change-Id: I6364254571bb1617a9f45ed08f2af4a59f9d5841
Reviewed-on: https://skia-review.googlesource.com/9335
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Derek Sollenberger <djsollen@google.com>
2017-03-07 11:15:10 +00:00
Herb Derby
38c6018024 Move from SkChunkAlloc to SkArenaAlloc for PathOps
Change-Id: Iab111a4ebcae4e896b1fdfe285def9ef0ae2ab6b
Reviewed-on: https://skia-review.googlesource.com/7314
Reviewed-by: Cary Clark <caryclark@google.com>
Commit-Queue: Herb Derby <herb@google.com>
2017-03-06 19:48:56 +00:00
Hal Canary
18fa6421e9 chmod -x *.cpp *.h
Change-Id: I5b37499505e26e0837921292c1aea0056e4d5ecd
Reviewed-on: https://skia-review.googlesource.com/4035
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Hal Canary <halcanary@google.com>
2016-10-27 16:41:14 +00:00
caryclark
a35ab3e6e0 fix fuzzers
Many old pathops-related fuzz failures have built up while
the codebase was under a state a flux. Now that the code
is stable, address these failures.

Most of the CL plumbs the debug global state to downstream
routines so that, if the data is not trusted (ala fuzzed)
the function can safely exit without asserting.

TBR=reed@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2426173002

Review-Url: https://chromiumcodereview.appspot.com/2426173002
2016-10-20 08:32:18 -07:00
caryclark
55888e4417 pathops coincidence and security rewrite
Most changes stem from working on an examples bracketed
by #if DEBUG_UNDER_DEVELOPMENT  // tiger
These exposed many problems with coincident curves,
as well as errors throughout the code.

Fixing these errors also fixed a number of fuzzer-inspired
bug reports.

* Line/Curve Intersections
Check to see if the end of the line nearly intersects
the curve. This was a FIXME in the old code.

* Performance
Use a central chunk allocator.
Plumb the allocator into the global variable state
so that it can be shared. (Note that 'SkGlobalState'
is allocated on the stack and is visible to children
functions but not other threads.)

* Refactor
Let SkOpAngle grow up from a structure to a class.
Let SkCoincidentSpans grow up from a structure to a class.
Rename enum Alias to AliasMatch.

* Coincidence Rewrite
Add more debugging to coincidence detection.
Parallel debugging routines have read-only logic to report
the current coincidence state so that steps through the
logic can expose whether things got better or worse.

More functions can error-out and cause the pathops
engine to non-destructively exit.

* Accuracy
Remove code that adjusted point locations. Instead,
offset the curve part so that sorted curves all use
the same origin.
Reduce the size (and influence) of magic numbers.

* Testing
The debug suite with verify and the full release suite
./out/Debug/pathops_unittest -v -V
./out/Release/pathops_unittest -v -V -x
expose one error. That error is captured as cubics_d3.
This error exists in the checked in code as well.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2128633003

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2128633003

Review-Url: https://codereview.chromium.org/2128633003
2016-07-18 10:01:36 -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
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
624637cc8e Path ops formerly found the topmost unprocessed edge and determined its angle sort order to initialize the winding. This never worked correctly with cubics and was flaky with paths consisting mostly of vertical edges.
This replacement shoots axis-aligned rays through all intersecting edges to find the outermost one either horizontally or vertically. The resulting code is smaller and twice as fast.

To support this, most of the horizontal / vertical intersection code was rewritten and standardized, and old code supporting the top-directed winding was deleted.

Contours were pointed to by an SkTDArray. Instead, put them in a linked list, and designate the list head with its own class to ensure that methods that take lists of contours start at the top. This change removed a large percentage of memory allocations used by path ops.

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

Review URL: https://codereview.chromium.org/1111333002
2015-05-11 07:21:28 -07:00
caryclark
1049f1246e Now, path ops natively intersect conics, quads, and cubics in any combination. There are still a class of cubic tests that fail and a handful of undiagnosed failures from skps and fuzz tests, but things are much better overall.
Extended tests (150M+) run to completion in release in about 6 minutes; the standard test suite exceeds 100K and finishes in a few seconds on desktops.

TBR=reed
BUG=skia:3588

Review URL: https://codereview.chromium.org/1037953004
2015-04-20 08:31:59 -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
dac1d17027 Enabling the canvas bit to turn the clip stack into a flat replace exposed around 100 failures when testing the 800K skp set generated from the top 1M web sites.
This fixes all but one of those failures.

Major changes include:
- Replace angle indices with angle pointers. This was motivated by the need to add angles later but not renumber existing angles.
- Aggressive segment chase. When the winding is known on a segment, more aggressively passing that winding to adjacent segments allows fragmented data sets to succeed.
- Line segments with ends nearly the same are treated as coincident first.
- Transfer partial coincidence by observing that if segment A is partially coincident to B and C then B and C may be partially coincident.

TBR=reed

Author: caryclark@google.com

Review URL: https://codereview.chromium.org/272153002
2014-06-17 05:15:38 -07:00
commit-bot@chromium.org
4431e7757c Mike R: please sanity check SkPostConfig.h
Mike K: please sanity check Test.cpp and skia_test.cpp

Feel free to look at the rest, but I don't expect any in depth review of path ops innards.

Path Ops first iteration used QuickSort to order segments radiating from an intersection to compute the winding rule.

This revision uses a circular sort instead. Breaking out the circular sort into its own long-lived structure (SkOpAngle) allows doing less work and provides a home for caching additional sorting data.

The circle sort is more stable than the former sort, has a robust ordering and fewer exceptions. It finds unsortable ordering less often. It is less reliant on the initial curve  tangent, using convex hulls instead whenever it can.

Additional debug validation makes sure that the computed structures are self-consistent. A new visualization tool helps verify that the angle ordering is correct.

The 70+M tests pass with this change on Windows, Mac, Linux 32 and Linux 64 in debug and release.

R=mtklein@google.com, reed@google.com

Author: caryclark@google.com

Review URL: https://codereview.chromium.org/131103009

git-svn-id: http://skia.googlecode.com/svn/trunk@14183 2bbb7eff-a529-9590-31e7-b0007b416f81
2014-04-14 17:08:59 +00:00