Add getCurrentClipPath() to SkCanvas.

Add getCurrentClipPath() to SkCanvas.

BUG=skia:

[zino, 2014-06-17 07:58:54]

Please take a look.
It is necessary for hit regions feature in Blink.
(win_blink_compile_dbg)

[reed1, 2014-06-17 13:49:43]

I'm not sure we want to have clients use ClipStack, as it may need to go away.

What is the blink use-case? Perhaps there are other, more defensive, ways Skia
can help...

[zino, 2014-06-17 13:57:29]

On 2014/06/17 13:49:43, reed1 wrote:
> I'm not sure we want to have clients use ClipStack, as it may need to go away.
> 
> What is the blink use-case? Perhaps there are other, more defensive, ways Skia
> can help...

Thank you for review.

The use-case is a new feature (hit regions) on canvas2d.
https://codereview.chromium.org/300223009/

The feature requires a current clip path.
How can we get the clip path? Please help me :)

Thank you.

[zino, 2014-06-17 13:59:08]

On 2014/06/17 13:57:29, zino wrote:
> On 2014/06/17 13:49:43, reed1 wrote:
> > I'm not sure we want to have clients use ClipStack, as it may need to go
away.
> > 
> > What is the blink use-case? Perhaps there are other, more defensive, ways
Skia
> > can help...
> 
> Thank you for review.
> 
> The use-case is a new feature (hit regions) on canvas2d.
> https://codereview.chromium.org/300223009/
> 
> The feature requires a current clip path.
> How can we get the clip path? Please help me :)
> 
> Thank you.

Please refer to GraphicsContext::getCurrentClipPath() in the CL.

[Justin Novosad, 2014-06-17 14:35:04]

On 2014/06/17 13:59:08, zino wrote:
> On 2014/06/17 13:57:29, zino wrote:
> > On 2014/06/17 13:49:43, reed1 wrote:
> > > I'm not sure we want to have clients use ClipStack, as it may need to go
> away.
> > > 
> > > What is the blink use-case? Perhaps there are other, more defensive, ways
> Skia
> > > can help...
> > 
> > Thank you for review.
> > 
> > The use-case is a new feature (hit regions) on canvas2d.
> > https://codereview.chromium.org/300223009/
> > 
> > The feature requires a current clip path.
> > How can we get the clip path? Please help me :)
> > 
> > Thank you.
> 
> Please refer to GraphicsContext::getCurrentClipPath() in the CL.

Perhaps the right solution would be to have an SkCanvas::getCurrentClipPath()
method.

[zino, 2014-06-18 02:31:34]

On 2014/06/17 14:35:04, junov wrote:
> On 2014/06/17 13:59:08, zino wrote:
> > On 2014/06/17 13:57:29, zino wrote:
> > > On 2014/06/17 13:49:43, reed1 wrote:
> > > > I'm not sure we want to have clients use ClipStack, as it may need to go
> > away.
> > > > 
> > > > What is the blink use-case? Perhaps there are other, more defensive,
ways
> > Skia
> > > > can help...
> > > 
> > > Thank you for review.
> > > 
> > > The use-case is a new feature (hit regions) on canvas2d.
> > > https://codereview.chromium.org/300223009/
> > > 
> > > The feature requires a current clip path.
> > > How can we get the clip path? Please help me :)
> > > 
> > > Thank you.
> > 
> > Please refer to GraphicsContext::getCurrentClipPath() in the CL.
> 
> Perhaps the right solution would be to have an SkCanvas::getCurrentClipPath()
> method.

Dear all,

I agree with Justin's comment. (So, I uploaded a new patch set)
What do you think of the patch set 2?

[reed1, 2014-06-18 12:49:43]

Why are we adding this? Is this different than florin's idea for hit-regions?

[zino, 2014-06-18 13:09:59]

On 2014/06/18 12:49:43, reed1 wrote:
> Why are we adding this? Is this different than florin's idea for hit-regions?

Yes, I think it is different.

Please refer to my comment(ctrl c v):
---------------------------------------
Thank you for review and good information :)

If it only has rejection case, your idea seems good to me.
I'm not sure because I'm not familiar with skia side codes.
But I think we still need a current clip path.

If you have another good ideas, please share to me.

Thank you.

https://codereview.chromium.org/300223009/diff/470001/Source/core/html/canvas...
File Source/core/html/canvas/CanvasRenderingContext2D.cpp (right):

https://codereview.chromium.org/300223009/diff/470001/Source/core/html/canvas...
Source/core/html/canvas/CanvasRenderingContext2D.cpp:2394: ||
!specifiedPath.intersectPath(context->getCurrentClipPath())
The method sets the intersection path between two path to lvalue(specifiedPath).
(Note: the method doesn't return whether or not to intersect)

As a result, if specifiedPath is not empty (clipPath is not empty),
we should pass it and should save it to HitRegion class.
Also, the path region will be used continuously while hit region exists.
----------------------------------------

[f(malita), 2014-06-18 13:14:30]

On 2014/06/18 12:49:43, reed1 wrote:
> Is this different than florin's idea for hit-regions?

Looks like empty-clip rejection is not the only reason for it. As spec'd, hit
regions need an intersect-this-path-with-the-current-clip logical op.

[zino, 2014-06-19 08:49:24]

On 2014/06/18 13:14:30, Florin Malita wrote:
> On 2014/06/18 12:49:43, reed1 wrote:
> > Is this different than florin's idea for hit-regions?
> 
> Looks like empty-clip rejection is not the only reason for it. As spec'd, hit
> regions need an intersect-this-path-with-the-current-clip logical op.

IMHO, getCurrentClipPath() will still be needed even if there wasn't
path-intersection op.
We should be able to check whether a hit point falls in the clipping path. (if
hit region takes a clipping int account)

So, could you please review this?

[reed1, 2014-06-19 13:12:44]

On 2014/06/19 08:49:24, zino wrote:
> On 2014/06/18 13:14:30, Florin Malita wrote:
> > On 2014/06/18 12:49:43, reed1 wrote:
> > > Is this different than florin's idea for hit-regions?
> > 
> > Looks like empty-clip rejection is not the only reason for it. As spec'd,
hit
> > regions need an intersect-this-path-with-the-current-clip logical op.
> 
> IMHO, getCurrentClipPath() will still be needed even if there wasn't
> path-intersection op.
> We should be able to check whether a hit point falls in the clipping path. (if
> hit region takes a clipping int account)
> 
> So, could you please review this?

I think this is exposing more than we want to. I am following the other thread
on the spec itself (for hittest region), and looking for a more conservative way
to either change the spec to be more performance friendly, or to find some other
way to support it.