Fix SkPicture path bound optimization to handle inverse filled paths.

Fix SkPicture path bound optimization to handle inverse filled paths.

BUG=crbug.com/229011
TEST=Picture unit test

Committed: https://code.google.com/p/skia/source/detail?r=9031

[Justin Novosad, 2013-05-06 20:39:33]

PTAL - M28 Release blocker

This patch just corrects one specific use case for
kUsePathBoundsForClip_RecordingFlag. 
Eventually, we should still implement a more global solution to fix all the
caveats of this optimization.(https://codereview.appspot.com/7301054/)

For now, we really need to fix this particular case that is being triggered by
Blink with impl-side painting enabled.

[reed1, 2013-05-06 21:27:19]

lgtm if we are in a rush

https://codereview.chromium.org/14819008/diff/1/src/core/SkPictureRecord.cpp
File src/core/SkPictureRecord.cpp (right):

https://codereview.chromium.org/14819008/diff/1/src/core/SkPictureRecord.cpp#...
src/core/SkPictureRecord.cpp:786: if (!path.isInverseFillType()) {
this seems a little double negatively. Is it correct to say?

if (path.isInverseFillType()) {
    return kEmpty_ClipType != this->getClipType();
} else {
    return this->INHERITED::...
}

That said, is that even correct? I wonder if we need to check if the current
clip is contained inside the path in the inverse case. If it is, then we should
return false...

[Justin Novosad, 2013-05-07 00:40:00]

On 2013/05/06 21:27:19, reed1 wrote:
> lgtm if we are in a rush
> 
> https://codereview.chromium.org/14819008/diff/1/src/core/SkPictureRecord.cpp
> File src/core/SkPictureRecord.cpp (right):
> 
>
https://codereview.chromium.org/14819008/diff/1/src/core/SkPictureRecord.cpp#...
> src/core/SkPictureRecord.cpp:786: if (!path.isInverseFillType()) {
> this seems a little double negatively. Is it correct to say?
> 
> if (path.isInverseFillType()) {
>     return kEmpty_ClipType != this->getClipType();
> } else {
>     return this->INHERITED::...
> }
> 
> That said, is that even correct?

It is conservative, in the sense that the canvas may end up reporting a clip
that is larger that the actual clip. I think this lies well within expectations
when using kUsePathBoundsForClip_RecordingFlag

> I wonder if we need to check if the current
> clip is contained inside the path in the inverse case. If it is, then we
should
> return false...

If we were calculating exect clips, yes. But the point of
kUsePathBoundsForClip_RecordingFlag is to take conservative shortcuts to avoid
clip rasterization overhead at picture record time. There may be trivial cases
that would be reasonable to handle. That should be covered by this other CL that
I should revive at some point: https://codereview.appspot.com/7301054/

[Justin Novosad, 2013-05-07 13:26:22]

Committed patchset #2 manually as r9031 (presubmit successful).

[Justin Novosad, 2013-05-07 19:57:35]

On 2013/05/07 13:26:22, junov wrote:
> Committed patchset #2 manually as r9031 (presubmit successful).

Reverted because broke some GMs