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

Issue 1517883002: fix SkPath::contains() for points on edge, conics (Closed)

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

Description

If a point is on a path edge, it's in the path, at least for all cases where the path edge is not canceled with another edge through coincidence. Add test cases for edges and conics, and make sure it all works. R=reed@google.com BUG=skia:4669, 4265 Committed: https://skia.googlesource.com/skia/+/9aacd9029c7076d5c0f0e62338b82ce91de68ef9

Patch Set 1 #

Patch Set 2 : wip coin #

Patch Set 3 : wip; more coin #

Patch Set 4 : wip; coincident test works #

Patch Set 5 : wip; on edge working for all types #

Patch Set 6 : wip; quad on edge works #

Patch Set 7 : all tests work #

Patch Set 8 : add comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+394 lines, -100 lines) Patch
M src/core/SkCubicClipper.h View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M src/core/SkCubicClipper.cpp View 1 2 3 4 5 6 3 chunks +3 lines, -3 lines 0 comments Download
M src/core/SkPath.cpp View 1 2 3 4 5 6 7 11 chunks +316 lines, -86 lines 0 comments Download
M tests/PathTest.cpp View 1 2 3 4 5 6 6 chunks +74 lines, -11 lines 0 comments Download

Messages

Total messages: 16 (7 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/1517883002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1517883002/1
5 years ago (2015-12-10 22:40:58 UTC) #2
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years ago (2015-12-10 22:54:12 UTC) #4
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1517883002/110001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1517883002/110001
5 years ago (2015-12-11 22:03:19 UTC) #7
caryclark
first cut; PTAL
5 years ago (2015-12-11 22:08:47 UTC) #8
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years ago (2015-12-11 22:21:53 UTC) #10
reed1
lgtm
5 years ago (2015-12-14 13:54:20 UTC) #11
reed1
lets file a bug/issue to track looking to consolidate/optimize our cubic chopper(s)
5 years ago (2015-12-14 13:54:48 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1517883002/130001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1517883002/130001
5 years ago (2015-12-14 13:57:13 UTC) #14
commit-bot: I haz the power
5 years ago (2015-12-14 16:38:13 UTC) #16
Message was sent while issue was closed.
Committed patchset #8 (id:130001) as
https://skia.googlesource.com/skia/+/9aacd9029c7076d5c0f0e62338b82ce91de68ef9

Powered by Google App Engine
This is Rietveld 408576698