SkPDF Implement colorfilters on bitmaps

SkPDF Implement colorfilters on bitmaps

BUG=484583
Committed: https://skia.googlesource.com/skia/+/287d22d9999f340d485e1db4dd19930f76f40230

[hal.canary, 2015-09-24 16:55:16]

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

[commit-bot: I haz the power, 2015-09-24 16:55:23]

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

[hal.canary, 2015-09-24 16:56:10]

halcanary@google.com changed reviewers:
+ scroggo@google.com

[hal.canary, 2015-09-24 16:56:10]

PTAL

[commit-bot: I haz the power, 2015-09-24 17:00:13]

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

[commit-bot: I haz the power, 2015-09-24 17:00:14]

Dry run: This issue passed the CQ dry run.

[scroggo, 2015-09-24 17:09:16]

lgtm

https://codereview.chromium.org/1367993002/diff/1/src/pdf/SkPDFDevice.cpp
File src/pdf/SkPDFDevice.cpp (left):

https://codereview.chromium.org/1367993002/diff/1/src/pdf/SkPDFDevice.cpp#old...
src/pdf/SkPDFDevice.cpp:2205: // Should extractSubset be done by the
SkPDFDevice?
I suppose this comment is no longer relevant because your fix for color filters
requires it?

https://codereview.chromium.org/1367993002/diff/1/src/pdf/SkPDFDevice.cpp
File src/pdf/SkPDFDevice.cpp (right):

https://codereview.chromium.org/1367993002/diff/1/src/pdf/SkPDFDevice.cpp#new...
src/pdf/SkPDFDevice.cpp:2210: // TODO(http://skbug.com/4378): implement
colorfilter everywhere.
What do you mean support it everywhere? I suppose this only fixes it for
drawBitmap? The bug does not explain that. The bug just shows a fiddle which
looks similar to the GM you've added here, except without an image filter and
without a saveLayer? Does "everywhere" mean when image filters are not involved?
(It's not clear to me this case handles image filters in particular, though.)

[hal.canary, 2015-09-24 17:15:06]

https://codereview.chromium.org/1367993002/diff/1/src/pdf/SkPDFDevice.cpp
File src/pdf/SkPDFDevice.cpp (left):

https://codereview.chromium.org/1367993002/diff/1/src/pdf/SkPDFDevice.cpp#old...
src/pdf/SkPDFDevice.cpp:2205: // Should extractSubset be done by the
SkPDFDevice?
On 2015/09/24 17:09:16, scroggo wrote:
> I suppose this comment is no longer relevant because your fix for color
filters
> requires it?

It wasn't relevant before, too.

https://codereview.chromium.org/1367993002/diff/1/src/pdf/SkPDFDevice.cpp
File src/pdf/SkPDFDevice.cpp (right):

https://codereview.chromium.org/1367993002/diff/1/src/pdf/SkPDFDevice.cpp#new...
src/pdf/SkPDFDevice.cpp:2210: // TODO(http://skbug.com/4378): implement
colorfilter everywhere.
On 2015/09/24 17:09:16, scroggo wrote:
> What do you mean support it everywhere? I suppose this only fixes it for
> drawBitmap? The bug does not explain that. The bug just shows a fiddle which
> looks similar to the GM you've added here, except without an image filter and
> without a saveLayer? Does "everywhere" mean when image filters are not
involved?
> (It's not clear to me this case handles image filters in particular, though.)

I tried to make it more clear.

[hal.canary, 2015-09-24 17:15:10]

The CQ bit was checked by halcanary@google.com

[hal.canary, 2015-09-24 17:15:10]

The patchset sent to the CQ was uploaded after l-g-t-m from scroggo@google.com
Link to the patchset: https://codereview.chromium.org/1367993002/#ps40001
(title: "2015-09-24 (Thursday) 13:13:56 EDT")

[commit-bot: I haz the power, 2015-09-24 17:15:20]

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

[commit-bot: I haz the power, 2015-09-24 17:20:10]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/287d22d9999f340d485e1db4dd19930f76f40230

[scroggo, 2015-09-24 17:20:26]

https://codereview.chromium.org/1367993002/diff/1/src/pdf/SkPDFDevice.cpp
File src/pdf/SkPDFDevice.cpp (right):

https://codereview.chromium.org/1367993002/diff/1/src/pdf/SkPDFDevice.cpp#new...
src/pdf/SkPDFDevice.cpp:2210: // TODO(http://skbug.com/4378): implement
colorfilter everywhere.
On 2015/09/24 17:15:05, Hal Canary wrote:
> On 2015/09/24 17:09:16, scroggo wrote:
> > What do you mean support it everywhere? I suppose this only fixes it for
> > drawBitmap? The bug does not explain that. The bug just shows a fiddle which
> > looks similar to the GM you've added here, except without an image filter
and
> > without a saveLayer? Does "everywhere" mean when image filters are not
> involved?
> > (It's not clear to me this case handles image filters in particular,
though.)
> 
> I tried to make it more clear.

lgtm

https://codereview.chromium.org/1367993002/diff/40001/src/pdf/SkPDFDevice.cpp
File src/pdf/SkPDFDevice.cpp (right):

https://codereview.chromium.org/1367993002/diff/40001/src/pdf/SkPDFDevice.cpp...
src/pdf/SkPDFDevice.cpp:2213: // backend).  Fortuanely, this seems to be how
Chromium
nit: Fortunately*