Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(815)

Issue 1405383004: fix path op conic bugs (Closed)

Created:
5 years, 1 month ago by caryclark
Modified:
5 years, 1 month ago
Reviewers:
CC:
reviews_skia.org
Base URL:
https://skia.googlesource.com/skia.git@master
Target Ref:
refs/heads/master
Project:
skia
Visibility:
Public.

Description

More conic-specific tests revealed a few conic-specific bugs. Because javascript / canvas make visualizing conics tricky, new native tools are required. The utility SubsetPath removes parts of a potentially very large path to isolate a minimal test case. SubsetPath is very useful for debugging path ops, but is not path ops specific. PathOpsBuilderConicTest compares the output of the Path Ops Builder, sequential calls to Simplify, and SkRegions for some number of rotated ovals. Some tests caused path ops to hang. It was caught adding a loop of curves because the head was not found by the tail. Even though the root cause has been fixed, SkSegment::addCurveTo callers now abort the path op if the same curve was added twice. The subdivided conic weight was been computed anew. Fortunately, it's a simpler computation that the one it replaces. Some Simplify() subroutines returned false to signal that the results needed assembling. Change these to abort the current operation instead. Coincident curve intersection triggered two small bugs; one where no perpendicular could be found for coincident curves, and one where no coincident curves remain after looping. The SixtyOvals test can be run through multiple processes instead of multiple threads. This strategy allows a 48 core machine to saturate all cores at 100%. The DEBUG_VISUALIZE_CONICS code in PathOpsConicIntersectionTest acknowleges that it is easier to visualize conics with Skia than with script and html canvas. This test also verifies that path ops subdivision matches geometry chopping. TBR=reed@google.com Committed: https://skia.googlesource.com/skia/+/ef784fb7f58c9c021172045a8e0b396c81fdc425

Patch Set 1 #

Patch Set 2 : wip subset path tester #

Patch Set 3 : wip isolate failures with subpaths #

Patch Set 4 : wip zero in on conic intersection bug #

Patch Set 5 : wip conics are all now well behaved #

Patch Set 6 : compute more accurate conic weight; fix conic bugs #

Patch Set 7 : remove change to debugging #

Patch Set 8 : fix compiler nits #

Patch Set 9 : fix w cast #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1517 lines, -320 lines) Patch
M gyp/pathops_unittest.gypi View 1 3 chunks +3 lines, -0 lines 0 comments Download
M src/pathops/SkOpContour.h View 1 1 chunk +2 lines, -2 lines 0 comments Download
M src/pathops/SkOpContour.cpp View 1 2 chunks +2 lines, -2 lines 0 comments Download
M src/pathops/SkOpSegment.h View 1 1 chunk +1 line, -2 lines 0 comments Download
M src/pathops/SkOpSegment.cpp View 1 2 chunks +42 lines, -40 lines 0 comments Download
M src/pathops/SkOpSpan.h View 1 2 chunks +9 lines, -0 lines 0 comments Download
M src/pathops/SkOpSpan.cpp View 1 1 chunk +1 line, -0 lines 0 comments Download
M src/pathops/SkPathOpsConic.cpp View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -7 lines 0 comments Download
M src/pathops/SkPathOpsOp.cpp View 1 3 chunks +9 lines, -3 lines 0 comments Download
M src/pathops/SkPathOpsSimplify.cpp View 1 2 9 chunks +32 lines, -14 lines 0 comments Download
M src/pathops/SkPathOpsTSect.h View 1 3 chunks +9 lines, -5 lines 0 comments Download
A tests/PathOpsBuilderConicTest.cpp View 1 2 3 4 5 6 7 1 chunk +639 lines, -0 lines 0 comments Download
M tests/PathOpsConicIntersectionTest.cpp View 1 2 3 4 1 chunk +276 lines, -0 lines 0 comments Download
A tests/SubsetPath.h View 1 2 3 4 5 6 7 1 chunk +66 lines, -0 lines 0 comments Download
A tests/SubsetPath.cpp View 1 2 3 1 chunk +240 lines, -0 lines 0 comments Download
M tools/pathops_sorter.htm View 1 2 3 4 1 chunk +53 lines, -20 lines 0 comments Download
M tools/pathops_visualizer.htm View 1 2 3 1 chunk +131 lines, -225 lines 0 comments Download

Messages

Total messages: 16 (8 generated)
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1405383004/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1405383004/120001
5 years, 1 month ago (2015-10-30 18:39:23 UTC) #3
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: Build-Ubuntu-Clang-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-Clang-x86_64-Debug-Trybot/builds/3991) Build-Ubuntu-GCC-Arm64-Debug-Android-Trybot on ...
5 years, 1 month ago (2015-10-30 18:40:16 UTC) #5
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1405383004/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1405383004/140001
5 years, 1 month ago (2015-10-30 18:46:39 UTC) #7
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: Build-Ubuntu-Clang-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-Clang-x86_64-Debug-Trybot/builds/3992) Build-Ubuntu-GCC-Arm7-Debug-Android-Trybot on ...
5 years, 1 month ago (2015-10-30 18:47:36 UTC) #9
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1405383004/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1405383004/160001
5 years, 1 month ago (2015-10-30 18:50:29 UTC) #11
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 1 month ago (2015-10-30 19:01:39 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1405383004/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1405383004/160001
5 years, 1 month ago (2015-10-30 19:02:20 UTC) #15
commit-bot: I haz the power
5 years, 1 month ago (2015-10-30 19:03:11 UTC) #16
Message was sent while issue was closed.
Committed patchset #9 (id:160001) as
https://skia.googlesource.com/skia/+/ef784fb7f58c9c021172045a8e0b396c81fdc425

Powered by Google App Engine
This is Rietveld 408576698