SkCanvas::drawPatch param SkPoint[12]

SkCanvas::drawPatch param SkPoint[12]

drawPatch now receives as parameter const SkPoint cubics[12]

Adjusted derived classes and serialization.

Ajusted GM's and benches that take into account combinations of optional
parameters, the scale of the patch and 4 different types of patches.

Planning on adding the extra functionality of SkPatch in another CL. 

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/b3c9d1c33caf325aada244204215eb790c228c12

[reed1, 2014-08-11 13:50:23]

https://codereview.chromium.org/463493002/diff/1/include/core/SkCanvas.h
File include/core/SkCanvas.h (right):

https://codereview.chromium.org/463493002/diff/1/include/core/SkCanvas.h#newc...
include/core/SkCanvas.h:1038: virtual void drawPatch(const SkPoint cubics[12],
const SkColor colors[4],
I like.

nit: lets make this version non-virtual, and add a protected virtual
onDrawPatch, so we can inject some shared debugging/checks in the non-virtual
version.

[dandov, 2014-08-11 14:45:07]

https://codereview.chromium.org/463493002/diff/1/include/core/SkCanvas.h
File include/core/SkCanvas.h (right):

https://codereview.chromium.org/463493002/diff/1/include/core/SkCanvas.h#newc...
include/core/SkCanvas.h:1038: virtual void drawPatch(const SkPoint cubics[12],
const SkColor colors[4],
On 2014/08/11 13:50:23, reed1 wrote:
> I like.
> 
> nit: lets make this version non-virtual, and add a protected virtual
> onDrawPatch, so we can inject some shared debugging/checks in the non-virtual
> version.

I did this in 2 patch sets in the first I just added the virtual call and
adjusted all the 
other ones and in the second one I understood the shared/debugging checks and 
put them in drawPatch and removed them from the derived onDrawPatch.

[reed1, 2014-08-11 15:38:44]

There is a lot in the public header SkPatch.h, and we don't take it in our
public API. Can it either be trimmed to just what the public needs, or move to
utils for now (or private)?

https://codereview.chromium.org/463493002/diff/20001/include/core/SkPatch.h
File include/core/SkPatch.h (right):

https://codereview.chromium.org/463493002/diff/20001/include/core/SkPatch.h#n...
include/core/SkPatch.h:108: if (NULL == points) {
Hmmm, why is this check here? I can't think of a reason for this to be called
*other* than to retrieve the pts.

[dandov, 2014-08-11 18:07:52]

Removed the skpatch file because now it's not used, will add it again when i get
some time to implement the SkPatchRef

https://codereview.chromium.org/463493002/diff/20001/include/core/SkPatch.h
File include/core/SkPatch.h (right):

https://codereview.chromium.org/463493002/diff/20001/include/core/SkPatch.h#n...
include/core/SkPatch.h:108: if (NULL == points) {
On 2014/08/11 15:38:44, reed1 wrote:
> Hmmm, why is this check here? I can't think of a reason for this to be called
> *other* than to retrieve the pts.

I added them to avoid crashing but I see now that this could lead to some
unexpected behavior.

[reed1, 2014-08-11 18:15:36]

On 2014/08/11 18:07:52, dandov wrote:
> Removed the skpatch file because now it's not used, will add it again when i
get
> some time to implement the SkPatchRef
> 
> https://codereview.chromium.org/463493002/diff/20001/include/core/SkPatch.h
> File include/core/SkPatch.h (right):
> 
>
https://codereview.chromium.org/463493002/diff/20001/include/core/SkPatch.h#n...
> include/core/SkPatch.h:108: if (NULL == points) {
> On 2014/08/11 15:38:44, reed1 wrote:
> > Hmmm, why is this check here? I can't think of a reason for this to be
called
> > *other* than to retrieve the pts.
> 
> I added them to avoid crashing but I see now that this could lead to some
> unexpected behavior.

yea, if we're crashing there, the caller needs to not send us a null.

[dandov, 2014-08-12 13:12:52]

Had to rebase my branch because there was a big change in SkPictureRecord
yesterday and I was having problems when the bots tried to apply a patch.

[reed1, 2014-08-12 13:33:02]

lgtm

https://codereview.chromium.org/463493002/diff/100001/include/core/SkCanvas.h
File include/core/SkCanvas.h (right):

https://codereview.chromium.org/463493002/diff/100001/include/core/SkCanvas.h...
include/core/SkCanvas.h:16: #include "SkPatchUtils.h"
Does SkCanvas.h need this header?

[commit-bot: I haz the power, 2014-08-12 13:33:19]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/dandov@google.com/463493002/100001

[commit-bot: I haz the power, 2014-08-12 13:41:51]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  Build-Win-VS2013-x86-Debug-Trybot on tryserver.skia
(http://108.170.220.76:10117/builders/Build-Win-VS2013-x86-Debug-Trybot/builds...)

[dandov, 2014-08-12 14:59:02]

The CQ bit was checked by dandov@google.com

[commit-bot: I haz the power, 2014-08-12 14:59:51]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/dandov@google.com/463493002/160001

[commit-bot: I haz the power, 2014-08-12 15:02:58]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  Test-Ubuntu13.10-GCE-NoGPU-x86_64-Debug-Trybot on tryserver.skia
(http://108.170.220.120:10117/builders/Test-Ubuntu13.10-GCE-NoGPU-x86_64-Debug...)
  Test-Ubuntu13.10-GCE-NoGPU-x86_64-Release-Shared-Trybot on tryserver.skia
(http://108.170.220.120:10117/builders/Test-Ubuntu13.10-GCE-NoGPU-x86_64-Relea...)

[commit-bot: I haz the power, 2014-08-12 15:04:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-12 15:04:49]

Try jobs failed on following builders:
  Test-Ubuntu13.10-GCE-NoGPU-x86_64-Debug-Trybot on tryserver.skia
(http://108.170.220.120:10117/builders/Test-Ubuntu13.10-GCE-NoGPU-x86_64-Debug...)

[dandov, 2014-08-12 15:21:31]

The CQ bit was checked by dandov@google.com

[commit-bot: I haz the power, 2014-08-12 15:21:47]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/dandov@google.com/463493002/180001

[dandov, 2014-08-12 15:23:31]

Did some changes to the CL:
*Fixed loss of precision warning in GM. 
*Removed the SkPatchUtils.h header from SkCanvas.h
*Removed the only GPU stuff from the patch GM
*Fixed things that crashed after those 2 changes.

https://codereview.chromium.org/463493002/diff/100001/include/core/SkCanvas.h
File include/core/SkCanvas.h (right):

https://codereview.chromium.org/463493002/diff/100001/include/core/SkCanvas.h...
include/core/SkCanvas.h:16: #include "SkPatchUtils.h"
On 2014/08/12 13:33:02, reed1 wrote:
> Does SkCanvas.h need this header?

Removed header from SkCanvas.h and add it where it is needed.

[commit-bot: I haz the power, 2014-08-12 15:34:35]

Change committed as b3c9d1c33caf325aada244204215eb790c228c12