Add subsets to SkImageGenerator and SkImageCacherator .

Add subsets to SkImageGenerator and SkImageCacherator
... to support subsets in SkImage!

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/935d6cfaa78b6be75c9fcc596805f0f9b8da972e

[reed1, 2015-08-17 22:04:59]

reed@google.com changed reviewers:
+ bsalomon@google.com, robertphillips@google.com, scroggo@google.com

[reed1, 2015-08-17 22:05:08]

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

[commit-bot: I haz the power, 2015-08-17 22:05:27]

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

[reed1, 2015-08-17 22:05:59]

ptal

[commit-bot: I haz the power, 2015-08-17 22:13:34]

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

[commit-bot: I haz the power, 2015-08-17 22:13:35]

Dry run: Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared-Trybot on client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)

[reed1, 2015-08-17 22:16:55]

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

[commit-bot: I haz the power, 2015-08-17 22:17:08]

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

[commit-bot: I haz the power, 2015-08-17 22:23:24]

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

[commit-bot: I haz the power, 2015-08-17 22:23:25]

Dry run: This issue passed the CQ dry run.

[bsalomon, 2015-08-18 13:09:07]

On 2015/08/17 22:23:25, commit-bot: I haz the power wrote:
> Dry run: This issue passed the CQ dry run.

Is picture the only generator class right now? Just expected to see a spot where
we gen a subset from something that is already a texture... but maybe there is
no such generator class currently?

[reed1, 2015-08-18 14:05:15]

generateTexture is so new, there are no subclasses before this one (picture).
Chrome has a video provider that subclasses generator, but only its raster
entry-point at the moment.

[bsalomon, 2015-08-18 14:11:51]

On 2015/08/18 14:05:15, reed1 wrote:
> generateTexture is so new, there are no subclasses before this one (picture).
> Chrome has a video provider that subclasses generator, but only its raster
> entry-point at the moment.

lgtm, but as you suggested, we should have a test case for a generator that's
natively a texture.

[reed1, 2015-08-18 17:48:37]

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

[commit-bot: I haz the power, 2015-08-18 17:48:45]

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

[reed1, 2015-08-18 17:48:52]

now w/ tests for other generators

[commit-bot: I haz the power, 2015-08-18 17:50:45]

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

[commit-bot: I haz the power, 2015-08-18 17:50:45]

Dry run: Try jobs failed on following builders:
  Build-Mac10.8-Clang-x86_64-Release-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Mac10.8-Clang-...)

[reed1, 2015-08-18 18:04:44]

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

[commit-bot: I haz the power, 2015-08-18 18:05:01]

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

[commit-bot: I haz the power, 2015-08-18 18:11:14]

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

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

Dry run: This issue passed the CQ dry run.

[reed1, 2015-08-18 18:15:27]

The CQ bit was checked by reed@google.com

[reed1, 2015-08-18 18:15:27]

The patchset sent to the CQ was uploaded after l-g-t-m from bsalomon@google.com
Link to the patchset: https://codereview.chromium.org/1301633002/#ps100001
(title: "fix release-build warning")

[commit-bot: I haz the power, 2015-08-18 18:15:44]

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

[commit-bot: I haz the power, 2015-08-18 18:16:12]

Committed patchset #6 (id:100001) as
https://skia.googlesource.com/skia/+/935d6cfaa78b6be75c9fcc596805f0f9b8da972e

[bsalomon, 2015-08-18 18:16:41]

lgtm

[scroggo, 2015-08-19 20:58:20]

lgtm

https://codereview.chromium.org/1301633002/diff/100001/src/core/SkImageCacher...
File src/core/SkImageCacherator.cpp (right):

https://codereview.chromium.org/1301633002/diff/100001/src/core/SkImageCacher...
src/core/SkImageCacherator.cpp:90: // need to handle subsetting
Should this assert that the dimensions are actually a subset?

https://codereview.chromium.org/1301633002/diff/100001/src/core/SkImageCacher...
src/core/SkImageCacherator.cpp:99: full.readPixels(bitmap->info(),
bitmap->getPixels(), bitmap->rowBytes(),
Is there a reason you did not call extractSubset? I suppose you'd rather do the
copy in this situation and hang on to less memory?

[reed1, 2015-08-19 21:04:09]

https://codereview.chromium.org/1301633002/diff/100001/src/core/SkImageCacher...
File src/core/SkImageCacherator.cpp (right):

https://codereview.chromium.org/1301633002/diff/100001/src/core/SkImageCacher...
src/core/SkImageCacherator.cpp:90: // need to handle subsetting
On 2015/08/19 20:58:20, scroggo wrote:
> Should this assert that the dimensions are actually a subset?

I think the caller has done that, but it'd be fine to also assert it here.

https://codereview.chromium.org/1301633002/diff/100001/src/core/SkImageCacher...
src/core/SkImageCacherator.cpp:99: full.readPixels(bitmap->info(),
bitmap->getPixels(), bitmap->rowBytes(),
On 2015/08/19 20:58:20, scroggo wrote:
> Is there a reason you did not call extractSubset? I suppose you'd rather do
the
> copy in this situation and hang on to less memory?

Definitely. This is a generator, not a cacher, and our caller will not know that
it can reuse other parts if it gets a request for a diff subset, so it would be
wasted memory. I have a bug into you/matt/me to expose subsetting directly in
SkImageGenerator, and in that way this code would not make a copy (though some
subclasses of generator may still have to).