SkPDF: Implement drawImage*() properly

SkPDF: Implement drawImage*() properly

drawImage calls now properly embeds the original jpeg.

NOTE: drawBitmap*() calls no longer embed JPEG files when
possible (this is in advance of eliminating bitmaps backed
by encoded data).  Chromium has already moved from
drawBitmap to drawImage.

Comparisons:

control:
    total PDF drawImage/drawBitmap calls: 8010
    total PDF jpeg images: 0
    total PDF regular images: 3581

experiament:
    total PDF drawImage/drawBitmap calls: 8014
    total PDF jpeg images: 271
    total PDF regular images: 3311
    total PDF regular images: 3582 (271 + 3311)

When comparing rendered output there were perceptual
differences in the following four GMs: colorcube, emboss,
colormatrix, and tablecolorfilter.  All of these differences
were improvements (that is, closer to the 8888 rendering)
due fixing a bug with colorfilters and forgetting to call
notifyPixelsChanged.

No SKPs had perceptual differences.

Total PDF size dropped from 133964 kB to 126276 kB, a 5.7%
improvement (mostly due to restoring use of JPG images in
SKPs).

BUG=skia:4370
Committed: https://skia.googlesource.com/skia/+/7a14b310d6c618fa2151d93a43b29f9599adc32a

[hal.canary, 2015-09-26 17:02:33]

halcanary@google.com changed reviewers:
+ mtklein@google.com, reed@google.com, tomhudson@google.com

[hal.canary, 2015-09-26 17:02:34]

reed@, PTAL at big picture
tomhudson@, PTAL at SkPDF details
mtklein@, PTAL at SkTHash

I want this to make into the next Chromium milestone.

reed@, I still owe you numbers on why this change is necessary.

[hal.canary, 2015-09-26 17:29:19]

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

[commit-bot: I haz the power, 2015-09-26 17:29:31]

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

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

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

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

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

[mtklein, 2015-09-27 07:20:33]

https://codereview.chromium.org/1372783003/diff/1/include/private/SkTHash.h
File include/private/SkTHash.h (right):

https://codereview.chromium.org/1372783003/diff/1/include/private/SkTHash.h#n...
include/private/SkTHash.h:247: void foreachValue(Fn&& fn) const {
What value does this add?  Can you not just ignore the key?

[reed1, 2015-09-27 07:48:18]

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

[reed1, 2015-09-27 07:49:33]

Do we have tools to dump stats about a conversion to PDF?

e.g.
- how many images were encoded
- how many images were stored
- etc.

If so, it would be interesting to run that tool before/after this sort of CL.

[hal.canary, 2015-09-27 18:03:44]

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

[commit-bot: I haz the power, 2015-09-27 18:03:56]

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

[commit-bot: I haz the power, 2015-09-27 18:09:55]

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

[commit-bot: I haz the power, 2015-09-27 18:09:55]

Dry run: This issue passed the CQ dry run.

[hal.canary, 2015-09-27 22:45:42]

On 2015/09/27 07:20:33, mtklein wrote:
> https://codereview.chromium.org/1372783003/diff/1/include/private/SkTHash.h
> File include/private/SkTHash.h (right):
> 
>
https://codereview.chromium.org/1372783003/diff/1/include/private/SkTHash.h#n...
> include/private/SkTHash.h:247: void foreachValue(Fn&& fn) const {
> What value does this add?  Can you not just ignore the key?

I kept trying to do the f(K, V) version of foreach, but the compiler wanted the
f(K, V*) version.  I just couldn't get it to compile!  Fixed.

[hal.canary, 2015-09-27 23:27:41]

On 2015/09/27 07:49:33, reed1 wrote:
> Do we have tools to dump stats about a conversion to PDF?
> 
> e.g.
> - how many images were encoded
> - how many images were stored
> - etc.
> 
> If so, it would be interesting to run that tool before/after this sort of CL.

GMs:
    control:
    	total PDF drawImage/drawBitmap calls: 5520
        total PDF jpeg images: 0
        total PDF regular images: 2520
    experiment:
    	total PDF drawImage/drawBitmap calls: 5308
        total PDF jpeg images: 2
        total PDF regular images: 2309

SKP:
    control:
        total PDF drawImage/drawBitmap calls: 1534
        total PDF jpeg images: 0
        total PDF regular images: 1294
    experiment:
    	total PDF drawImage/drawBitmap calls: 1534
        total PDF jpeg images: 269
        total PDF regular images: 1091

[hal.canary, 2015-09-29 18:10:07]

Please take another look.

[hal.canary, 2015-09-29 18:27:30]

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

[commit-bot: I haz the power, 2015-09-29 18:27:47]

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

[commit-bot: I haz the power, 2015-09-29 18:34:33]

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

[commit-bot: I haz the power, 2015-09-29 18:34:34]

Dry run: This issue passed the CQ dry run.

[tomhudson, 2015-09-30 16:10:30]

First question is just a nit, other two should probably be taken care of before
landing.

https://codereview.chromium.org/1372783003/diff/80001/src/pdf/SkPDFBitmap.cpp
File src/pdf/SkPDFBitmap.cpp (right):

https://codereview.chromium.org/1372783003/diff/80001/src/pdf/SkPDFBitmap.cpp...
src/pdf/SkPDFBitmap.cpp:360: void PDFDefaultBitmap::emitObject(SkWStream*
stream,
There's a lot of redundancy between this and PDFAlphaBitmap::emitObject(), just
a few more options here?

https://codereview.chromium.org/1372783003/diff/80001/src/pdf/SkPDFBitmap.cpp...
src/pdf/SkPDFBitmap.cpp:364: image_get_ro_pixels(fImage, &bitmap);     //
TODO(halcanary): test
note TODO?

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

https://codereview.chromium.org/1372783003/diff/80001/src/pdf/SkPDFDevice.cpp...
src/pdf/SkPDFDevice.cpp:1155: // FIXME FIXME FIXME FIXME FIXME FIXME FIXME FIXME
FIXME FIXME FIXME FIXME
Not ready to land?

[hal.canary, 2015-10-01 01:24:55]

https://chromiumcodereview.appspot.com/1372783003/diff/80001/src/pdf/SkPDFBit...
File src/pdf/SkPDFBitmap.cpp (right):

https://chromiumcodereview.appspot.com/1372783003/diff/80001/src/pdf/SkPDFBit...
src/pdf/SkPDFBitmap.cpp:360: void PDFDefaultBitmap::emitObject(SkWStream*
stream,
On 2015/09/30 16:10:29, tomhudson wrote:
> There's a lot of redundancy between this and PDFAlphaBitmap::emitObject(),
just
> a few more options here?

Done.

https://chromiumcodereview.appspot.com/1372783003/diff/80001/src/pdf/SkPDFBit...
src/pdf/SkPDFBitmap.cpp:364: image_get_ro_pixels(fImage, &bitmap);     //
TODO(halcanary): test
On 2015/09/30 16:10:29, tomhudson wrote:
> note TODO?

Acknowledged.

https://chromiumcodereview.appspot.com/1372783003/diff/80001/src/pdf/SkPDFDev...
File src/pdf/SkPDFDevice.cpp (right):

https://chromiumcodereview.appspot.com/1372783003/diff/80001/src/pdf/SkPDFDev...
src/pdf/SkPDFDevice.cpp:1155: // FIXME FIXME FIXME FIXME FIXME FIXME FIXME FIXME
FIXME FIXME FIXME FIXME
On 2015/09/30 16:10:30, tomhudson wrote:
> Not ready to land?

Done.

[tomhudson, 2015-10-01 14:20:28]

lgtm

[tomhudson, 2015-10-01 14:21:23]

The CQ bit was checked by tomhudson@google.com

[commit-bot: I haz the power, 2015-10-01 14:21:43]

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

[commit-bot: I haz the power, 2015-10-01 14:28:17]

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