remove unused (by the outside) SkImage::newSurface, and simplify newImage -> newSubset

remove unused (by the outside) SkImage::newSurface, and simplify newImage -> newSubset

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/7b6945bc4e639d7cc4a49b84d492690f8e865566

[reed1, 2015-09-22 19:11:48]

reed@google.com changed reviewers:
+ bsalomon@google.com, fmalita@chromium.org, robertphillips@google.com

[reed1, 2015-09-22 19:11:49]

If we can land this (w/ the guard in chrome), it will simplify other CLs that
want to wrap textures inside generators. Getting rid of newSurface helps this a
lot.

[f(malita), 2015-09-22 19:33:15]

On 2015/09/22 19:11:49, reed1 wrote:
> If we can land this (w/ the guard in chrome), it will simplify other CLs that
> want to wrap textures inside generators. Getting rid of newSurface helps this
a
> lot.


Do we even need a guard?

1) land this CL while preserving a transitional newImage()
2) convert Blink (Chrome?) from newImage -> newSubset
3) remove newImage()

https://codereview.chromium.org/1364443002/diff/1/src/image/SkImage_Generator...
File src/image/SkImage_Generator.cpp (right):

https://codereview.chromium.org/1364443002/diff/1/src/image/SkImage_Generator...
src/image/SkImage_Generator.cpp:81: #include "SkCanvas.h"
Nit: move include to top?

https://codereview.chromium.org/1364443002/diff/1/src/image/SkImage_Raster.cpp
File src/image/SkImage_Raster.cpp (right):

https://codereview.chromium.org/1364443002/diff/1/src/image/SkImage_Raster.cp...
src/image/SkImage_Raster.cpp:212: // TODO : could consider heurist of sharing
pixels, if subset is pretty close to complete
Can't this be implemented similarly to SkBitmap::extractSubset (always share
pixels by adjusting origin/rowBytes)?

[bsalomon, 2015-09-22 19:41:45]

wonder about inheriting the budgeting status of the original, but not sure what
to do about it. lgtm

https://codereview.chromium.org/1364443002/diff/1/src/image/SkImage_Raster.cpp
File src/image/SkImage_Raster.cpp (right):

https://codereview.chromium.org/1364443002/diff/1/src/image/SkImage_Raster.cp...
src/image/SkImage_Raster.cpp:212: // TODO : could consider heurist of sharing
pixels, if subset is pretty close to complete
On 2015/09/22 19:33:14, f(malita) wrote:
> Can't this be implemented similarly to SkBitmap::extractSubset (always share
> pixels by adjusting origin/rowBytes)?

what if the pixels are modified?

[reed1, 2015-09-22 19:42:21]

https://codereview.chromium.org/1364443002/diff/1/src/image/SkImage_Generator...
File src/image/SkImage_Generator.cpp (right):

https://codereview.chromium.org/1364443002/diff/1/src/image/SkImage_Generator...
src/image/SkImage_Generator.cpp:81: #include "SkCanvas.h"
On 2015/09/22 19:33:14, f(malita) wrote:
> Nit: move include to top?

Done.

https://codereview.chromium.org/1364443002/diff/1/src/image/SkImage_Raster.cpp
File src/image/SkImage_Raster.cpp (right):

https://codereview.chromium.org/1364443002/diff/1/src/image/SkImage_Raster.cp...
src/image/SkImage_Raster.cpp:212: // TODO : could consider heurist of sharing
pixels, if subset is pretty close to complete
On 2015/09/22 19:33:14, f(malita) wrote:
> Can't this be implemented similarly to SkBitmap::extractSubset (always share
> pixels by adjusting origin/rowBytes)?

I'm sort of waiting on more wisdom/experience, to know when to copy -vs- share.

[f(malita), 2015-09-22 19:47:22]

lgtm

https://codereview.chromium.org/1364443002/diff/1/src/image/SkImage_Raster.cpp
File src/image/SkImage_Raster.cpp (right):

https://codereview.chromium.org/1364443002/diff/1/src/image/SkImage_Raster.cp...
src/image/SkImage_Raster.cpp:212: // TODO : could consider heurist of sharing
pixels, if subset is pretty close to complete
On 2015/09/22 19:41:45, bsalomon wrote:
> On 2015/09/22 19:33:14, f(malita) wrote:
> > Can't this be implemented similarly to SkBitmap::extractSubset (always share
> > pixels by adjusting origin/rowBytes)?
> 
> what if the pixels are modified?

Aren't SkImages always backed by immutable pixels?

[bsalomon, 2015-09-22 20:08:23]

https://codereview.chromium.org/1364443002/diff/1/src/image/SkImage_Raster.cpp
File src/image/SkImage_Raster.cpp (right):

https://codereview.chromium.org/1364443002/diff/1/src/image/SkImage_Raster.cp...
src/image/SkImage_Raster.cpp:212: // TODO : could consider heurist of sharing
pixels, if subset is pretty close to complete
On 2015/09/22 19:47:22, f(malita) wrote:
> On 2015/09/22 19:41:45, bsalomon wrote:
> > On 2015/09/22 19:33:14, f(malita) wrote:
> > > Can't this be implemented similarly to SkBitmap::extractSubset (always
share
> > > pixels by adjusting origin/rowBytes)?
> > 
> > what if the pixels are modified?
> 
> Aren't SkImages always backed by immutable pixels?

Oh, duh.

[f(malita), 2015-09-22 20:47:33]

https://codereview.chromium.org/1364443002/diff/20001/src/image/SkImage.cpp
File src/image/SkImage.cpp (right):

https://codereview.chromium.org/1364443002/diff/20001/src/image/SkImage.cpp#n...
src/image/SkImage.cpp:137: SkFilterQuality) const {
Found while preparing the Blink cleanup patch: this definition doesn't match the
declaration.  Drop SkFilterQuality?

[reed1, 2015-09-23 10:07:54]

https://codereview.chromium.org/1364443002/diff/20001/src/image/SkImage.cpp
File src/image/SkImage.cpp (right):

https://codereview.chromium.org/1364443002/diff/20001/src/image/SkImage.cpp#n...
src/image/SkImage.cpp:137: SkFilterQuality) const {
On 2015/09/22 20:47:33, f(malita) wrote:
> Found while preparing the Blink cleanup patch: this definition doesn't match
the
> declaration.  Drop SkFilterQuality?

Done.

[reed1, 2015-09-23 10:08:00]

The CQ bit was checked by reed@google.com

[reed1, 2015-09-23 10:08:01]

The patchset sent to the CQ was uploaded after l-g-t-m from bsalomon@google.com,
fmalita@chromium.org
Link to the patchset: https://codereview.chromium.org/1364443002/#ps40001
(title: " ")

[commit-bot: I haz the power, 2015-09-23 10:08:12]

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

[commit-bot: I haz the power, 2015-09-23 10:10:25]

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

[commit-bot: I haz the power, 2015-09-23 10:10:26]

Try jobs failed on following builders:
  Build-Win-MSVC-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86_6...)

[reed1, 2015-09-23 10:27:30]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-09-23 10:27:40]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1364443002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1364443002/60001

[commit-bot: I haz the power, 2015-09-23 10:32:10]

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

[commit-bot: I haz the power, 2015-09-23 10:32:10]

Dry run: This issue passed the CQ dry run.

[reed1, 2015-09-24 07:49:56]

The CQ bit was checked by reed@google.com

[reed1, 2015-09-24 07:49:57]

The patchset sent to the CQ was uploaded after l-g-t-m from bsalomon@google.com,
fmalita@chromium.org
Link to the patchset: https://codereview.chromium.org/1364443002/#ps60001
(title: " ")

[commit-bot: I haz the power, 2015-09-24 07:50:13]

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

[commit-bot: I haz the power, 2015-09-24 07:51:04]

Committed patchset #4 (id:60001) as
https://skia.googlesource.com/skia/+/7b6945bc4e639d7cc4a49b84d492690f8e865566