SkPDF: draw{Image,Bitmap} always serializes early

SkPDF: draw{Image,Bitmap} always serializes early

Before this change, the PDFCanon held a map from BitmapKeys
to SkImages for de-duping bitmaps.  Even if the PDFDocument
serialized images early, the Canon still held a ref to that
image inside the map.  With this change, the Canon holds a
single map from BitmapKeys to PDFObjects.  Now, Images are
only held by the PDFObject, which the document serializes
and drops early.

This change also:

-   Moves SkBitmapKey into its own header (for possible
    reuse); it now can operate with images as well as
    bitmaps.

-   Creates SkImageBitmap, which wraps a pointer to a bitmap
    or an image and abstracts out some common tasks so that
    drawBitmap and drawImage behave the same.

-   Modifies SkPDFCreateBitmapObject to take and return a
    sk_sp<T>, not a T*.

-   Refactors SkPDFDevice::internalDrawImage to use bitmaps
    or images (via a SkImageBitmap).

-   Turns on pre-serialization of all images.

BUG=skia:5087
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1829693002

Committed: https://skia.googlesource.com/skia/+/a50151dcb5a0b8bfdef383e363e519c91d2d2c7a

[hal.canary, 2016-03-23 12:56:11]

Description was changed from

==========
SkPDF: image/bitmap draw no longer hold images

Move SkBitmapKey into its own header (for possible reuse) and can
operate with images as well as bitmaps.

create SkImageBitmap, which wraps a pointer to a bitmap or an image
and abstracts out some common tasks so that drawBitmap and drawImage
behave the same.

SkPDFCreateBitmapObject	takes and returns a sk_sp<T>, not a T*.

In SkPDFCanon, findPDFBitmap and bitmapToImage are combined into a
single funtion what works with BitmapKeys.

In SkPDFCanon, ownership in fPDFBitmapMap is made explcicit by using
SkTHashMap<K, sk_sp<V>> rather than SkTHashMap<K, V*>.

SkPDFDevice::internalDrawImage refactored to use bitmaps or images.

Pre-serialize images.

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

to

==========
SkPDF: image/bitmap draw no longer hold images

Move SkBitmapKey into its own header (for possible reuse) and can
operate with images as well as bitmaps.

create SkImageBitmap, which wraps a pointer to a bitmap or an image
and abstracts out some common tasks so that drawBitmap and drawImage
behave the same.

SkPDFCreateBitmapObject	takes and returns a sk_sp<T>, not a T*.

In SkPDFCanon, findPDFBitmap and bitmapToImage are combined into a
single funtion what works with BitmapKeys.

In SkPDFCanon, ownership in fPDFBitmapMap is made explcicit by using
SkTHashMap<K, sk_sp<V>> rather than SkTHashMap<K, V*>.

SkPDFDevice::internalDrawImage refactored to use bitmaps or images.

Pre-serialize images.

BUG=skia:5087
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[hal.canary, 2016-03-23 12:56:39]

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

[commit-bot: I haz the power, 2016-03-23 12:56:49]

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

[commit-bot: I haz the power, 2016-03-23 12:57:59]

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

[commit-bot: I haz the power, 2016-03-23 12:58:00]

Dry run: Try jobs failed on following builders:
  Build-Mac-Clang-x86_64-Release-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Mac-Clang-x86_...)
  Build-Ubuntu-Clang-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-Clang-x...)
  Build-Ubuntu-GCC-Arm64-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-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...)
  Build-Ubuntu-GCC-x86_64-Release-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-x86...)

[hal.canary, 2016-03-23 13:01:20]

Patchset #1 (id:1) has been deleted

[hal.canary, 2016-03-23 13:11:42]

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

[commit-bot: I haz the power, 2016-03-23 13:11:46]

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

[commit-bot: I haz the power, 2016-03-23 13:12:56]

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

[commit-bot: I haz the power, 2016-03-23 13:12:57]

Dry run: Try jobs failed on following builders:
  Build-Mac-Clang-x86_64-Release-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Mac-Clang-x86_...)
  Build-Ubuntu-Clang-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-Clang-x...)
  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...)

[hal.canary, 2016-03-23 13:16:37]

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

[hal.canary, 2016-03-23 13:16:46]

Patchset #2 (id:40001) has been deleted

[commit-bot: I haz the power, 2016-03-23 13:16:48]

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

[hal.canary, 2016-03-23 13:16:54]

Patchset #1 (id:20001) has been deleted

[commit-bot: I haz the power, 2016-03-23 13:27:16]

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

[commit-bot: I haz the power, 2016-03-23 13:27:17]

Dry run: This issue passed the CQ dry run.

[hal.canary, 2016-03-23 13:40:17]

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

[hal.canary, 2016-03-23 13:40:18]

PTAL.

This flips the switch.

[tomhudson, 2016-03-23 13:45:33]

Once again your change description tells me what you did but not why. (And what
you did is mostly at a low per-file level.)

[hal.canary, 2016-03-23 14:52:42]

Description was changed from

==========
SkPDF: image/bitmap draw no longer hold images

Move SkBitmapKey into its own header (for possible reuse) and can
operate with images as well as bitmaps.

create SkImageBitmap, which wraps a pointer to a bitmap or an image
and abstracts out some common tasks so that drawBitmap and drawImage
behave the same.

SkPDFCreateBitmapObject	takes and returns a sk_sp<T>, not a T*.

In SkPDFCanon, findPDFBitmap and bitmapToImage are combined into a
single funtion what works with BitmapKeys.

In SkPDFCanon, ownership in fPDFBitmapMap is made explcicit by using
SkTHashMap<K, sk_sp<V>> rather than SkTHashMap<K, V*>.

SkPDFDevice::internalDrawImage refactored to use bitmaps or images.

Pre-serialize images.

BUG=skia:5087
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
SkPDF: draw{Image,Bitmap} always serizlizes early

Previously, we had a map from BitmapKeys to SkImages for de-duping
bitmaps.  Even if we serizlized images early, we still held a ref to
that image inside the map.  Now we hold a single map from
Images-or-Bitmaps to PDFObjects.  Now, Images are only held by the
PDFObject, which the document serialized early.

Move SkBitmapKey into its own header (for possible reuse); it now can
operate with images as well as bitmaps.

Create SkImageBitmap, which wraps a pointer to a bitmap or an image
and abstracts out some common tasks so that drawBitmap and drawImage
behave the same.

SkPDFCreateBitmapObject takes and returns a sk_sp<T>, not a T*.

In SkPDFCanon, findPDFBitmap and bitmapToImage are combined into a
single funtion what works with BitmapKeys.

In SkPDFCanon, ownership in fPDFBitmapMap is made explcicit by using
SkTHashMap<K, sk_sp<V>> rather than SkTHashMap<K, V*>.

SkPDFDevice::internalDrawImage refactored to use bitmaps or images
(via a SkImageBitmap).

Pre-serialize images.

BUG=skia:5087
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[hal.canary, 2016-03-23 14:53:17]

On 2016/03/23 13:45:33, tomhudson wrote:
> Once again your change description tells me what you did but not why. (And
what
> you did is mostly at a low per-file level.)

How is this:

    Previously, we had a map from BitmapKeys to SkImages for
    de-duping bitmaps.  Even if we serizlized images early, we
    still held a ref to that image inside the map.  Now we hold a
    single map from Images-or-Bitmaps to PDFObjects.  Now, Images
    are only held by the PDFObject, which the document serialized
    early.

[tomhudson, 2016-03-23 15:01:25]

On 2016/03/23 14:53:17, Hal Canary wrote:
> On 2016/03/23 13:45:33, tomhudson wrote:
> > Once again your change description tells me what you did but not why. (And
> what
> > you did is mostly at a low per-file level.)
> 
> How is this:
> 
>     Previously, we had a map from BitmapKeys to SkImages for
>     de-duping bitmaps.  Even if we serizlized images early, we
>     still held a ref to that image inside the map.  Now we hold a
>     single map from Images-or-Bitmaps to PDFObjects.  Now, Images
>     are only held by the PDFObject, which the document serialized
>     early.

Needs wordsmithing, but that does a better job of conveying the purpose of the
change.

[hal.canary, 2016-03-23 16:05:43]

Patchset #2 (id:80001) has been deleted

[hal.canary, 2016-03-23 16:05:56]

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

[commit-bot: I haz the power, 2016-03-23 16:06:00]

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

[hal.canary, 2016-03-23 16:06:18]

Description was changed from

==========
SkPDF: draw{Image,Bitmap} always serizlizes early

Previously, we had a map from BitmapKeys to SkImages for de-duping
bitmaps.  Even if we serizlized images early, we still held a ref to
that image inside the map.  Now we hold a single map from
Images-or-Bitmaps to PDFObjects.  Now, Images are only held by the
PDFObject, which the document serialized early.

Move SkBitmapKey into its own header (for possible reuse); it now can
operate with images as well as bitmaps.

Create SkImageBitmap, which wraps a pointer to a bitmap or an image
and abstracts out some common tasks so that drawBitmap and drawImage
behave the same.

SkPDFCreateBitmapObject takes and returns a sk_sp<T>, not a T*.

In SkPDFCanon, findPDFBitmap and bitmapToImage are combined into a
single funtion what works with BitmapKeys.

In SkPDFCanon, ownership in fPDFBitmapMap is made explcicit by using
SkTHashMap<K, sk_sp<V>> rather than SkTHashMap<K, V*>.

SkPDFDevice::internalDrawImage refactored to use bitmaps or images
(via a SkImageBitmap).

Pre-serialize images.

BUG=skia:5087
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
SkPDF: draw{Image,Bitmap} always serizlizes early

Previously, we had a map from BitmapKeys to SkImages for de-duping
bitmaps.  Even if we serizlized images early, we still held a ref to
that image inside the map.  Now we hold a single map from
Images-or-Bitmaps to PDFObjects.  Now, Images are only held by the
PDFObject, which the document serialized early.

Move SkBitmapKey into its own header (for possible reuse); it now can
operate with images as well as bitmaps.

Create SkImageBitmap, which wraps a pointer to a bitmap or an image
and abstracts out some common tasks so that drawBitmap and drawImage
behave the same.

SkPDFCreateBitmapObject takes and returns a sk_sp<T>, not a T*.

In SkPDFCanon, findPDFBitmap and bitmapToImage are combined into a
single funtion what works with BitmapKeys.

SkPDFDevice::internalDrawImage refactored to use bitmaps or images
(via a SkImageBitmap).

Pre-serialize images.

BUG=skia:5087
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[commit-bot: I haz the power, 2016-03-23 16:17:10]

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

[commit-bot: I haz the power, 2016-03-23 16:17:11]

Dry run: This issue passed the CQ dry run.

[hal.canary, 2016-03-24 15:13:17]

Description was changed from

==========
SkPDF: draw{Image,Bitmap} always serizlizes early

Previously, we had a map from BitmapKeys to SkImages for de-duping
bitmaps.  Even if we serizlized images early, we still held a ref to
that image inside the map.  Now we hold a single map from
Images-or-Bitmaps to PDFObjects.  Now, Images are only held by the
PDFObject, which the document serialized early.

Move SkBitmapKey into its own header (for possible reuse); it now can
operate with images as well as bitmaps.

Create SkImageBitmap, which wraps a pointer to a bitmap or an image
and abstracts out some common tasks so that drawBitmap and drawImage
behave the same.

SkPDFCreateBitmapObject takes and returns a sk_sp<T>, not a T*.

In SkPDFCanon, findPDFBitmap and bitmapToImage are combined into a
single funtion what works with BitmapKeys.

SkPDFDevice::internalDrawImage refactored to use bitmaps or images
(via a SkImageBitmap).

Pre-serialize images.

BUG=skia:5087
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
SkPDF: draw{Image,Bitmap} always serializes early

Before this change, the PDFCanon held a map from BitmapKeys
to SkImages for de-duping bitmaps.  Even if the PDFDocument
serialized images early, the Canon still held a ref to that
image inside the map.  With this change, the Canon holds a
single map from BitmapKeys to PDFObjects.  Now, Images are
only held by the PDFObject, which the document serializes
and drops early.

This change also:

-   Moves SkBitmapKey into its own header (for possible
    reuse); it now can operate with images as well as
    bitmaps.

-   Creates SkImageBitmap, which wraps a pointer to a bitmap
    or an image and abstracts out some common tasks so that
    drawBitmap and drawImage behave the same.

-   Modifies SkPDFCreateBitmapObject to take and return a
    sk_sp<T>, not a T*.

-   Refactors SkPDFDevice::internalDrawImage to use bitmaps
    or images (via a SkImageBitmap).

-   Turns on pre-serialization of all images.

BUG=skia:5087

GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[hal.canary, 2016-03-24 15:13:42]

On 2016/03/23 15:01:25, tomhudson wrote:
> Needs wordsmithing,...

done

[tomhudson, 2016-03-25 17:38:48]

lgtm

https://codereview.chromium.org/1829693002/diff/100001/src/pdf/SkBitmapKey.h
File src/pdf/SkBitmapKey.h (right):

https://codereview.chromium.org/1829693002/diff/100001/src/pdf/SkBitmapKey.h#...
src/pdf/SkBitmapKey.h:41: explicit SkImageBitmap(SkImage* i) : fImage(i) {
SkASSERT(fImage); }
Given how often you're calling sk_sp<>::get() when invoking these constructors,
would it make sense to at least sometimes *take* the smart pointer here?

[hal.canary, 2016-03-25 18:25:21]

https://codereview.chromium.org/1829693002/diff/100001/src/pdf/SkBitmapKey.h
File src/pdf/SkBitmapKey.h (right):

https://codereview.chromium.org/1829693002/diff/100001/src/pdf/SkBitmapKey.h#...
src/pdf/SkBitmapKey.h:41: explicit SkImageBitmap(SkImage* i) : fImage(i) {
SkASSERT(fImage); }
On 2016/03/25 17:38:47, tomhudson wrote:
> Given how often you're calling sk_sp<>::get() when invoking these
constructors,
> would it make sense to at least sometimes *take* the smart pointer here?

I think it makes more sense to do that when I can *always* take it.

[hal.canary, 2016-03-25 18:25:29]

The CQ bit was checked by halcanary@google.com

[commit-bot: I haz the power, 2016-03-25 18:25:41]

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

[commit-bot: I haz the power, 2016-03-25 18:26:51]

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

[commit-bot: I haz the power, 2016-03-25 18:26:52]

Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-Trybot on client.skia (JOB_FAILED,
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_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)
  Build-Mac-Clang-x86_64-Release-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Mac-Clang-x86_...)
  Build-Ubuntu-Clang-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-Clang-x...)
  Build-Ubuntu-GCC-Arm64-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-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...)
  Build-Ubuntu-GCC-x86_64-Release-CMake-Trybot on client.skia.compile
(JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-x86...)
  Build-Ubuntu-GCC-x86_64-Release-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-x86...)

[hal.canary, 2016-03-25 18:36:47]

The CQ bit was checked by halcanary@google.com

[hal.canary, 2016-03-25 18:36:47]

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

[commit-bot: I haz the power, 2016-03-25 18:36:51]

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

[commit-bot: I haz the power, 2016-03-25 18:57:51]

Description was changed from

==========
SkPDF: draw{Image,Bitmap} always serializes early

Before this change, the PDFCanon held a map from BitmapKeys
to SkImages for de-duping bitmaps.  Even if the PDFDocument
serialized images early, the Canon still held a ref to that
image inside the map.  With this change, the Canon holds a
single map from BitmapKeys to PDFObjects.  Now, Images are
only held by the PDFObject, which the document serializes
and drops early.

This change also:

-   Moves SkBitmapKey into its own header (for possible
    reuse); it now can operate with images as well as
    bitmaps.

-   Creates SkImageBitmap, which wraps a pointer to a bitmap
    or an image and abstracts out some common tasks so that
    drawBitmap and drawImage behave the same.

-   Modifies SkPDFCreateBitmapObject to take and return a
    sk_sp<T>, not a T*.

-   Refactors SkPDFDevice::internalDrawImage to use bitmaps
    or images (via a SkImageBitmap).

-   Turns on pre-serialization of all images.

BUG=skia:5087

GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
SkPDF: draw{Image,Bitmap} always serializes early

Before this change, the PDFCanon held a map from BitmapKeys
to SkImages for de-duping bitmaps.  Even if the PDFDocument
serialized images early, the Canon still held a ref to that
image inside the map.  With this change, the Canon holds a
single map from BitmapKeys to PDFObjects.  Now, Images are
only held by the PDFObject, which the document serializes
and drops early.

This change also:

-   Moves SkBitmapKey into its own header (for possible
    reuse); it now can operate with images as well as
    bitmaps.

-   Creates SkImageBitmap, which wraps a pointer to a bitmap
    or an image and abstracts out some common tasks so that
    drawBitmap and drawImage behave the same.

-   Modifies SkPDFCreateBitmapObject to take and return a
    sk_sp<T>, not a T*.

-   Refactors SkPDFDevice::internalDrawImage to use bitmaps
    or images (via a SkImageBitmap).

-   Turns on pre-serialization of all images.

BUG=skia:5087

GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/a50151dcb5a0b8bfdef383e363e519c91d2d2c7a
==========

[commit-bot: I haz the power, 2016-03-25 18:57:53]

Committed patchset #3 (id:120001) as
https://skia.googlesource.com/skia/+/a50151dcb5a0b8bfdef383e363e519c91d2d2c7a