Initial implementation of GPU no filter NinePatch

Initial implementation of GPU no filter NinePatch

TBR=bsalomon@google.com
BUG=skia:

Committed: https://skia.googlesource.com/skia/+/33a5fce6126dc5d3927a71fdc6c35af6f5893fd5

[joshualitt, 2015-11-17 22:05:29]

joshualitt@google.com changed reviewers:
+ bsalomon@google.com, joshualitt@google.com, robertphillips@google.com

[joshualitt, 2015-11-17 22:05:29]

This is mostly about the GrNinePatch class itself right now, I'm going to clean
up some of the logic in SkGpuDevice tomorrow morning.  I also need to write gms.

[joshualitt, 2015-11-17 22:10:58]

On 2015/11/17 22:05:29, joshualitt wrote:
> This is mostly about the GrNinePatch class itself right now, I'm going to
clean
> up some of the logic in SkGpuDevice tomorrow morning.  I also need to write
gms.

okay look at SkGpuDevice.  I don't really know what I'm doing there(as is
obvious).

[bsalomon, 2015-11-17 22:26:09]

I think you might have forgotten to git add files

https://codereview.chromium.org/1454933002/diff/60001/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

https://codereview.chromium.org/1454933002/diff/60001/src/gpu/SkGpuDevice.cpp...
src/gpu/SkGpuDevice.cpp:1735: if (!wrap_as_bm(this->context(), image, &bitmap))
{
Let's do a real impl here... this has a malloc hidden in it. Maybe could try
sharing impl with a template impl?

https://codereview.chromium.org/1454933002/diff/60001/src/gpu/SkGpuDevice.cpp...
src/gpu/SkGpuDevice.cpp:1748: 
Let's just fail if the bitmap is texture backed. Don't want to support it ever.

https://codereview.chromium.org/1454933002/diff/60001/src/gpu/SkGpuDevice.cpp...
src/gpu/SkGpuDevice.cpp:1775: AutoBitmapTexture abt(fContext, bitmap, params,
&texture);
Let's not expand the use of this. It used to do a lot more, now it does almost
nothing.

https://codereview.chromium.org/1454933002/diff/60001/src/gpu/SkGpuDevice.cpp...
src/gpu/SkGpuDevice.cpp:1781: SkMatrix texMatrix = SkMatrix::I();
SkMatrix texMatrix;
texMatrix.setIDiv(texture->width(), texture->height());

https://codereview.chromium.org/1454933002/diff/60001/src/gpu/SkGpuDevice.cpp...
src/gpu/SkGpuDevice.cpp:1791: if (kAlpha_8_SkColorType == bitmap.colorType()) {
we do this logic in a bunch of places.. maybe this can share the impl in
skgpudevice_drawtexture.cpp?

[joshualitt, 2015-11-18 14:35:12]

On 2015/11/17 22:26:09, bsalomon wrote:
> I think you might have forgotten to git add files
> 
> https://codereview.chromium.org/1454933002/diff/60001/src/gpu/SkGpuDevice.cpp
> File src/gpu/SkGpuDevice.cpp (right):
> 
>
https://codereview.chromium.org/1454933002/diff/60001/src/gpu/SkGpuDevice.cpp...
> src/gpu/SkGpuDevice.cpp:1735: if (!wrap_as_bm(this->context(), image,
&bitmap))
> {
> Let's do a real impl here... this has a malloc hidden in it. Maybe could try
> sharing impl with a template impl?
> 
>
https://codereview.chromium.org/1454933002/diff/60001/src/gpu/SkGpuDevice.cpp...
> src/gpu/SkGpuDevice.cpp:1748: 
> Let's just fail if the bitmap is texture backed. Don't want to support it
ever.
> 
>
https://codereview.chromium.org/1454933002/diff/60001/src/gpu/SkGpuDevice.cpp...
> src/gpu/SkGpuDevice.cpp:1775: AutoBitmapTexture abt(fContext, bitmap, params,
> &texture);
> Let's not expand the use of this. It used to do a lot more, now it does almost
> nothing.
> 
>
https://codereview.chromium.org/1454933002/diff/60001/src/gpu/SkGpuDevice.cpp...
> src/gpu/SkGpuDevice.cpp:1781: SkMatrix texMatrix = SkMatrix::I();
> SkMatrix texMatrix;
> texMatrix.setIDiv(texture->width(), texture->height());
> 
>
https://codereview.chromium.org/1454933002/diff/60001/src/gpu/SkGpuDevice.cpp...
> src/gpu/SkGpuDevice.cpp:1791: if (kAlpha_8_SkColorType == bitmap.colorType())
{
> we do this logic in a bunch of places.. maybe this can share the impl in
> skgpudevice_drawtexture.cpp?

okay ninepatch added.  I'll incorporate the feedback in a follow on

[joshualitt, 2015-11-18 17:46:15]

feedback inc

https://codereview.chromium.org/1454933002/diff/60001/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

https://codereview.chromium.org/1454933002/diff/60001/src/gpu/SkGpuDevice.cpp...
src/gpu/SkGpuDevice.cpp:1735: if (!wrap_as_bm(this->context(), image, &bitmap))
{
On 2015/11/17 22:26:09, bsalomon wrote:
> Let's do a real impl here... this has a malloc hidden in it. Maybe could try
> sharing impl with a template impl?

per conversation, lets defer until you finish what you are working on.

https://codereview.chromium.org/1454933002/diff/60001/src/gpu/SkGpuDevice.cpp...
src/gpu/SkGpuDevice.cpp:1748: 
On 2015/11/17 22:26:09, bsalomon wrote:
> Let's just fail if the bitmap is texture backed. Don't want to support it
ever.

we can't because it seems that wrap_as_bm seems to trigger this.

https://codereview.chromium.org/1454933002/diff/60001/src/gpu/SkGpuDevice.cpp...
src/gpu/SkGpuDevice.cpp:1775: AutoBitmapTexture abt(fContext, bitmap, params,
&texture);
On 2015/11/17 22:26:09, bsalomon wrote:
> Let's not expand the use of this. It used to do a lot more, now it does almost
> nothing.

Acknowledged.

https://codereview.chromium.org/1454933002/diff/60001/src/gpu/SkGpuDevice.cpp...
src/gpu/SkGpuDevice.cpp:1781: SkMatrix texMatrix = SkMatrix::I();
On 2015/11/17 22:26:09, bsalomon wrote:
> SkMatrix texMatrix;
> texMatrix.setIDiv(texture->width(), texture->height());

Acknowledged.

https://codereview.chromium.org/1454933002/diff/60001/src/gpu/SkGpuDevice.cpp...
src/gpu/SkGpuDevice.cpp:1791: if (kAlpha_8_SkColorType == bitmap.colorType()) {
On 2015/11/17 22:26:09, bsalomon wrote:
> we do this logic in a bunch of places.. maybe this can share the impl in
> skgpudevice_drawtexture.cpp?

Acknowledged.

[bsalomon, 2015-11-18 17:57:36]

https://codereview.chromium.org/1454933002/diff/120001/src/gpu/SkGrPriv.h
File src/gpu/SkGrPriv.h (right):

https://codereview.chromium.org/1454933002/diff/120001/src/gpu/SkGrPriv.h#new...
src/gpu/SkGrPriv.h:87: bool SkPaintToGrPaintWithTexture(GrContext* context,
Comment explaining what this does? may or may not look at the shader...

[robertphillips, 2015-11-18 18:12:07]

https://codereview.chromium.org/1454933002/diff/120001/gm/ninepatchstretch.cpp
File gm/ninepatchstretch.cpp (right):

https://codereview.chromium.org/1454933002/diff/120001/gm/ninepatchstretch.cp...
gm/ninepatchstretch.cpp:104: }
'+' ?

https://codereview.chromium.org/1454933002/diff/120001/include/gpu/GrDrawCont...
File include/gpu/GrDrawContext.h (right):

https://codereview.chromium.org/1454933002/diff/120001/include/gpu/GrDrawCont...
include/gpu/GrDrawContext.h:28: class GrTextContext;
Do we need this guy ?

https://codereview.chromium.org/1454933002/diff/120001/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

https://codereview.chromium.org/1454933002/diff/120001/src/gpu/SkGpuDevice.cp...
src/gpu/SkGpuDevice.cpp:1727: 
This comment seems wrong ...

https://codereview.chromium.org/1454933002/diff/120001/src/gpu/batches/GrNine...
File src/gpu/batches/GrNinePatch.cpp (right):

https://codereview.chromium.org/1454933002/diff/120001/src/gpu/batches/GrNine...
src/gpu/batches/GrNinePatch.cpp:29: 
kVertsPerRect, kIndicesPerRect ?

[joshualitt, 2015-11-18 19:07:07]

feedback incorporated

https://codereview.chromium.org/1454933002/diff/120001/include/gpu/GrDrawCont...
File include/gpu/GrDrawContext.h (right):

https://codereview.chromium.org/1454933002/diff/120001/include/gpu/GrDrawCont...
include/gpu/GrDrawContext.h:28: class GrTextContext;
On 2015/11/18 18:12:06, robertphillips wrote:
> Do we need this guy ?

Acknowledged.

https://codereview.chromium.org/1454933002/diff/120001/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

https://codereview.chromium.org/1454933002/diff/120001/src/gpu/SkGpuDevice.cp...
src/gpu/SkGpuDevice.cpp:1727: 
On 2015/11/18 18:12:07, robertphillips wrote:
> This comment seems wrong ...

Acknowledged.

https://codereview.chromium.org/1454933002/diff/120001/src/gpu/SkGrPriv.h
File src/gpu/SkGrPriv.h (right):

https://codereview.chromium.org/1454933002/diff/120001/src/gpu/SkGrPriv.h#new...
src/gpu/SkGrPriv.h:87: bool SkPaintToGrPaintWithTexture(GrContext* context,
On 2015/11/18 17:57:35, bsalomon wrote:
> Comment explaining what this does? may or may not look at the shader...

Acknowledged.

https://codereview.chromium.org/1454933002/diff/120001/src/gpu/batches/GrNine...
File src/gpu/batches/GrNinePatch.cpp (right):

https://codereview.chromium.org/1454933002/diff/120001/src/gpu/batches/GrNine...
src/gpu/batches/GrNinePatch.cpp:29: 
On 2015/11/18 18:12:07, robertphillips wrote:
> kVertsPerRect, kIndicesPerRect ?

Acknowledged.

[robertphillips, 2015-11-18 19:09:52]

lgtm

[joshualitt, 2015-11-18 20:48:14]

The CQ bit was checked by joshualitt@google.com

[commit-bot: I haz the power, 2015-11-18 20:48:23]

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

[commit-bot: I haz the power, 2015-11-18 20:49:21]

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

[commit-bot: I haz the power, 2015-11-18 20:49:22]

Try jobs failed on following builders:
  Build-Ubuntu-GCC-Arm7-Debug-Android-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Arm...)
  Build-Ubuntu-GCC-Mips-Debug-Android-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Mip...)

[joshualitt, 2015-11-18 20:59:03]

The CQ bit was checked by joshualitt@google.com

[joshualitt, 2015-11-18 20:59:03]

The patchset sent to the CQ was uploaded after l-g-t-m from
robertphillips@google.com
Link to the patchset: https://codereview.chromium.org/1454933002/#ps180001
(title: "rebase")

[commit-bot: I haz the power, 2015-11-18 20:59:12]

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

[commit-bot: I haz the power, 2015-11-18 21:00:35]

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

[commit-bot: I haz the power, 2015-11-18 21:00:36]

Try jobs failed on following builders:
  skia_presubmit-Trybot on client.skia.fyi (JOB_FAILED,
http://build.chromium.org/p/client.skia.fyi/builders/skia_presubmit-Trybot/bu...)

[joshualitt, 2015-11-18 21:15:03]

Description was changed from

==========
Initial implementation of GPU no filter NinePatch

BUG=skia:
==========

to

==========
Initial implementation of GPU no filter NinePatch

TBR=bsalomon@google.com
BUG=skia:
==========

[joshualitt, 2015-11-18 21:15:06]

The CQ bit was checked by joshualitt@google.com

[joshualitt, 2015-11-18 21:15:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
robertphillips@google.com
Link to the patchset: https://codereview.chromium.org/1454933002/#ps200001
(title: "tweaks")

[commit-bot: I haz the power, 2015-11-18 21:15:11]

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

[commit-bot: I haz the power, 2015-11-18 21:28:55]

Committed patchset #11 (id:200001) as
https://skia.googlesource.com/skia/+/33a5fce6126dc5d3927a71fdc6c35af6f5893fd5