SkPatchGrid interface (after SkPatch/SkPicture rebase).

Proposal for the mesh gradient interface. Implemented as a grid of
patches and uses 4 private arrays to store the values of the control points and
colors. When it needs a patch at a certain position of the grid it just
builds it using the corresponding values of the array and the
grid coordinates provided. Details on implementation are documented in the corresponding classes' comments.

Also added a gm for mesh gradients.

BUG=skia:

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

[dandov, 2014-08-07 17:41:07]

Added again the interface for the mesh gradient but this issue is already
rebased on the previous cl which fixes problems with SkPicture. 

I added SkMeshGradient to the effects/gradients but I'm not sure it that is OK,
maybe in experimental?

[egdaniel, 2014-08-07 17:58:55]

initial comments, more later

https://codereview.chromium.org/451723003/diff/1/src/effects/gradients/SkMesh...
File src/effects/gradients/SkMeshGradient.h (right):

https://codereview.chromium.org/451723003/diff/1/src/effects/gradients/SkMesh...
src/effects/gradients/SkMeshGradient.h:27: * The array fExists holds bools to
determine if there is an existing patch at a certain location of
Can remove the comments about fExists

https://codereview.chromium.org/451723003/diff/1/src/effects/gradients/SkMesh...
src/effects/gradients/SkMeshGradient.h:91: * exists at the given location and
overwrite was set to false.
what is overwrite here?

https://codereview.chromium.org/451723003/diff/1/src/effects/gradients/SkMesh...
src/effects/gradients/SkMeshGradient.h:109: void resize(int rows, int cols);
Do you have a plan to use this function? My gut instinct tells me we would know
the size of the mesh at start and wouldn't need to change it ever

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

Already fixed the comments, I'll add them after deciding what to do with resize.

https://codereview.chromium.org/451723003/diff/1/src/effects/gradients/SkMesh...
File src/effects/gradients/SkMeshGradient.h (right):

https://codereview.chromium.org/451723003/diff/1/src/effects/gradients/SkMesh...
src/effects/gradients/SkMeshGradient.h:27: * The array fExists holds bools to
determine if there is an existing patch at a certain location of
On 2014/08/07 17:58:55, egdaniel wrote:
> Can remove the comments about fExists

Done.

https://codereview.chromium.org/451723003/diff/1/src/effects/gradients/SkMesh...
src/effects/gradients/SkMeshGradient.h:91: * exists at the given location and
overwrite was set to false.
On 2014/08/07 17:58:55, egdaniel wrote:
> what is overwrite here?

Forgot to fix some of the comments. Overwrite was a flag to keep values of
adjacent patches instead of directly overwriting them, but now that is gone.

https://codereview.chromium.org/451723003/diff/1/src/effects/gradients/SkMesh...
src/effects/gradients/SkMeshGradient.h:109: void resize(int rows, int cols);
On 2014/08/07 17:58:55, egdaniel wrote:
> Do you have a plan to use this function? My gut instinct tells me we would
know
> the size of the mesh at start and wouldn't need to change it ever

I was thinking for a similar case as Illustrator's editor in which you click
inside the mesh and it gets subdivided at that point, maybe a subdivide function
that subdivides the mesh at a point by creating 4 patches based on the previous
corners and the new point? or just remove the function?

[dandov, 2014-08-07 21:11:49]

Renamed to SkPatchGrid, removed resize function and pointer constructor. 

Added SkPatchGrid to core, any suggestions for where should it be?

[egdaniel, 2014-08-08 14:39:04]

Since we have moved away from the mesh gradient name, you probably should also
rename the gm and all references inside the gm to meshgradient

https://codereview.chromium.org/451723003/diff/20001/gm/meshgradient.cpp
File gm/meshgradient.cpp (right):

https://codereview.chromium.org/451723003/diff/20001/gm/meshgradient.cpp#newc...
gm/meshgradient.cpp:118: for (int i = 0; i<2; i++) {
nit: add spaces around <

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

https://codereview.chromium.org/451723003/diff/20001/include/core/SkPatchGrid...
include/core/SkPatchGrid.h:15: * Class that represents a Mesh Gradient. A mesh
gradient is an array of coons patches, adjacent
no longer gradient, fix comment

https://codereview.chromium.org/451723003/diff/20001/include/core/SkPatchGrid...
include/core/SkPatchGrid.h:94: void getPatch(int x, int y, SkPatch* patch)
const;
maybe have this return a bool as well so it is known whether patch was set or
not

https://codereview.chromium.org/451723003/diff/20001/include/core/SkPatchGrid...
include/core/SkPatchGrid.h:98: * this and resize is that resize keeps the
previous values of control points and colors and
no more resize so fix comment

https://codereview.chromium.org/451723003/diff/20001/include/core/SkPatchGrid...
include/core/SkPatchGrid.h:106: void draw(SkCanvas* canvas, SkPaint& paint);
Since we are deciding the draw order of the patches in this version, add a
comment saying what that order is.

Future version maybe should allow the client to specify an order (no need to
implement this now)

[dandov, 2014-08-08 18:31:46]

The last fixes are in the last 2 patch sets. I finished most of them in the
first one but I realized later that I was missing some things.

https://codereview.chromium.org/451723003/diff/20001/gm/meshgradient.cpp
File gm/meshgradient.cpp (right):

https://codereview.chromium.org/451723003/diff/20001/gm/meshgradient.cpp#newc...
gm/meshgradient.cpp:118: for (int i = 0; i<2; i++) {
On 2014/08/08 14:39:04, egdaniel wrote:
> nit: add spaces around <

Done.

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

https://codereview.chromium.org/451723003/diff/20001/include/core/SkPatchGrid...
include/core/SkPatchGrid.h:15: * Class that represents a Mesh Gradient. A mesh
gradient is an array of coons patches, adjacent
On 2014/08/08 14:39:04, egdaniel wrote:
> no longer gradient, fix comment

Done.

https://codereview.chromium.org/451723003/diff/20001/include/core/SkPatchGrid...
include/core/SkPatchGrid.h:94: void getPatch(int x, int y, SkPatch* patch)
const;
On 2014/08/08 14:39:04, egdaniel wrote:
> maybe have this return a bool as well so it is known whether patch was set or
> not

Done. returns false in case the coordinates x,y are out of bounds or if patch is
NULL.

https://codereview.chromium.org/451723003/diff/20001/include/core/SkPatchGrid...
include/core/SkPatchGrid.h:98: * this and resize is that resize keeps the
previous values of control points and colors and
On 2014/08/08 14:39:04, egdaniel wrote:
> no more resize so fix comment

Done.

https://codereview.chromium.org/451723003/diff/20001/include/core/SkPatchGrid...
include/core/SkPatchGrid.h:106: void draw(SkCanvas* canvas, SkPaint& paint);
On 2014/08/08 14:39:04, egdaniel wrote:
> Since we are deciding the draw order of the patches in this version, add a
> comment saying what that order is.
> 
> Future version maybe should allow the client to specify an order (no need to
> implement this now)

Done.

[egdaniel, 2014-08-08 18:45:41]

L-G-T-M + nits (still will need either Mike or Brian to say yes)

https://codereview.chromium.org/451723003/diff/60001/include/core/SkPatchGrid.h
File include/core/SkPatchGrid.h (right):

https://codereview.chromium.org/451723003/diff/60001/include/core/SkPatchGrid...
include/core/SkPatchGrid.h:12: #include "SkCanvas.h"
annoying nit: but alphabetize these please

https://codereview.chromium.org/451723003/diff/60001/src/core/SkPatchGrid.cpp
File src/core/SkPatchGrid.cpp (right):

https://codereview.chromium.org/451723003/diff/60001/src/core/SkPatchGrid.cpp...
src/core/SkPatchGrid.cpp:34: //setup corners and colors
space after //

https://codereview.chromium.org/451723003/diff/60001/src/core/SkPatchGrid.cpp...
src/core/SkPatchGrid.cpp:70: //set the patch by building the array of points and
colors with the corresponding values.
space after //

[dandov, 2014-08-08 19:00:55]

https://codereview.chromium.org/451723003/diff/60001/include/core/SkPatchGrid.h
File include/core/SkPatchGrid.h (right):

https://codereview.chromium.org/451723003/diff/60001/include/core/SkPatchGrid...
include/core/SkPatchGrid.h:12: #include "SkCanvas.h"
On 2014/08/08 18:45:41, egdaniel wrote:
> annoying nit: but alphabetize these please

Done.

https://codereview.chromium.org/451723003/diff/60001/src/core/SkPatchGrid.cpp
File src/core/SkPatchGrid.cpp (right):

https://codereview.chromium.org/451723003/diff/60001/src/core/SkPatchGrid.cpp...
src/core/SkPatchGrid.cpp:34: //setup corners and colors
On 2014/08/08 18:45:41, egdaniel wrote:
> space after //

Done.

https://codereview.chromium.org/451723003/diff/60001/src/core/SkPatchGrid.cpp...
src/core/SkPatchGrid.cpp:70: //set the patch by building the array of points and
colors with the corresponding values.
On 2014/08/08 18:45:41, egdaniel wrote:
> space after //

Done.

[dandov, 2014-08-12 17:59:55]

Rebased the branch to include the changes to SkCanvas::drawPatch and adjusted
SkPatchGrid to follow the same interface.

[reed1, 2014-08-12 19:44:07]

I vote to put at least the header into include/utils for now, til we've used it
more and created a core dependency on it (e.g. added it to canvas).

[dandov, 2014-08-12 20:14:28]

Moved SkPatchGrid.h and .cpp to src/utils

[reed1, 2014-08-12 21:35:37]

lgtm

[dandov, 2014-08-12 21:52:03]

The CQ bit was checked by dandov@google.com

[commit-bot: I haz the power, 2014-08-12 21:53:20]

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

[commit-bot: I haz the power, 2014-08-12 22:01:48]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  Build-Ubuntu13.10-Clang-x86_64-Debug-Trybot on tryserver.skia
(http://108.170.220.76:10117/builders/Build-Ubuntu13.10-Clang-x86_64-Debug-Try...)

[commit-bot: I haz the power, 2014-08-12 22:05:15]

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

[commit-bot: I haz the power, 2014-08-12 22:05:17]

Try jobs failed on following builders:
  Build-Ubuntu13.10-Clang-x86_64-Debug-Trybot on tryserver.skia
(http://108.170.220.76:10117/builders/Build-Ubuntu13.10-Clang-x86_64-Debug-Try...)

[dandov, 2014-08-13 00:04:54]

The CQ bit was checked by dandov@google.com

[commit-bot: I haz the power, 2014-08-13 00:05:47]

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

[commit-bot: I haz the power, 2014-08-13 00:15:12]

Change committed as cc03adb90901d226e8b0252a187b19a68fabcc42