Drop SkPathContainsPoint in favor of SkPath::contains

Drop SkPathContainsPoint in favor of SkPath::contains

This will increase the fidelity in some cases when hit-testing.

BUG=373638, 523102
Committed: https://crrev.com/fc2a4e253b958798ef04069083ba19ae79fde296
Cr-Commit-Position: refs/heads/master@{#366141}

[fs, 2015-12-18 15:43:44]

fs@opera.com changed reviewers:
+ fmalita@chromium.org, pdr@chromium.org

[fs, 2015-12-18 15:43:44]

Thanks to some bug fixing by Cary, all layout tests seem to pass when using
SkPath::contains() instead of the "raster-based" solution.

https://codereview.chromium.org/1536803003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/Path.cpp (right):

https://codereview.chromium.org/1536803003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/Path.cpp:83:
const_cast<SkPath&>(m_path).setFillType(WebCoreWindRuleToSkFillType(rule));
Yes, this is pretty horrendous...

[f(malita), 2015-12-18 15:56:22]

Nice!

https://codereview.chromium.org/1536803003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/Path.cpp (right):

https://codereview.chromium.org/1536803003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/Path.cpp:83:
const_cast<SkPath&>(m_path).setFillType(WebCoreWindRuleToSkFillType(rule));
On 2015/12/18 15:43:44, fs wrote:
> Yes, this is pretty horrendous...

How about something like

  if (WebCoreWindRuleToSkFillType(rule) != m_path.getFillType()) {
      SkPath tmp(m_path);
      tmp.setFilType(...);
      return tmp.contains(...);
  }
  return m_path.contains(...);


(the copy should be cheap/shallow)

I bet we could also get rid of this version, but not sure how much effort it'd
take to stop side-caring windrule, so let's not worry about that for now :)

[fs, 2015-12-18 16:48:02]

https://codereview.chromium.org/1536803003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/Path.cpp (right):

https://codereview.chromium.org/1536803003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/Path.cpp:83:
const_cast<SkPath&>(m_path).setFillType(WebCoreWindRuleToSkFillType(rule));
On 2015/12/18 at 15:56:22, f(malita) wrote:
> On 2015/12/18 15:43:44, fs wrote:
> > Yes, this is pretty horrendous...
> 
> How about something like
> 
>   if (WebCoreWindRuleToSkFillType(rule) != m_path.getFillType()) {
>       SkPath tmp(m_path);
>       tmp.setFilType(...);
>       return tmp.contains(...);
>   }
>   return m_path.contains(...);
> 
> 
> (the copy should be cheap/shallow)

Yes, maybe that's a reasonable trade-off - hopefully all the atomic(op)s won't
be too noticeable... Let's try it.
 
> I bet we could also get rid of this version, but not sure how much effort it'd
take to stop side-caring windrule, so let's not worry about that for now :)

I was planning on maybe taking a pass over those once this had settled - some
should definitely be possible to push over to the other overload without to much
effort.

[pdr., 2015-12-18 19:00:06]

On 2015/12/18 at 16:48:02, fs wrote:
>
https://codereview.chromium.org/1536803003/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/platform/graphics/Path.cpp (right):
> 
>
https://codereview.chromium.org/1536803003/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/platform/graphics/Path.cpp:83:
const_cast<SkPath&>(m_path).setFillType(WebCoreWindRuleToSkFillType(rule));
> On 2015/12/18 at 15:56:22, f(malita) wrote:
> > On 2015/12/18 15:43:44, fs wrote:
> > > Yes, this is pretty horrendous...
> > 
> > How about something like
> > 
> >   if (WebCoreWindRuleToSkFillType(rule) != m_path.getFillType()) {
> >       SkPath tmp(m_path);
> >       tmp.setFilType(...);
> >       return tmp.contains(...);
> >   }
> >   return m_path.contains(...);
> > 
> > 
> > (the copy should be cheap/shallow)
> 
> Yes, maybe that's a reasonable trade-off - hopefully all the atomic(op)s won't
be too noticeable... Let's try it.
>  
> > I bet we could also get rid of this version, but not sure how much effort
it'd take to stop side-caring windrule, so let's not worry about that for now :)
> 
> I was planning on maybe taking a pass over those once this had settled - some
should definitely be possible to push over to the other overload without to much
effort.

LGTM 1!11

[f(malita), 2015-12-18 19:10:46]

The CQ bit was checked by fmalita@chromium.org

[f(malita), 2015-12-18 19:10:46]

lgtm

[commit-bot: I haz the power, 2015-12-18 19:11:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1536803003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1536803003/40001

[commit-bot: I haz the power, 2015-12-18 19:17:35]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-12-18 19:18:22]

Description was changed from

==========
Drop SkPathContainsPoint in favor of SkPath::contains

This will increase the fidelity in some cases when hit-testing.

BUG=373638,523102
==========

to

==========
Drop SkPathContainsPoint in favor of SkPath::contains

This will increase the fidelity in some cases when hit-testing.

BUG=373638,523102

Committed: https://crrev.com/fc2a4e253b958798ef04069083ba19ae79fde296
Cr-Commit-Position: refs/heads/master@{#366141}
==========

[commit-bot: I haz the power, 2015-12-18 19:18:25]

Patchset 3 (id:??) landed as
https://crrev.com/fc2a4e253b958798ef04069083ba19ae79fde296
Cr-Commit-Position: refs/heads/master@{#366141}