Use SkPicture::ExtractBitmap callback in pdf too.

src/pdf/SkPDFImage.cpp

 /*
  * Copyright 2010 The Android Open Source Project
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkPDFImage.h"
 
 #include "SkBitmap.h"
@@ skipping 321 matching lines @@
         buf[2] = SkGetPackedB32(color);
         index.append(buf, 3);
     }
     result->append(new SkPDFString(index))->unref();
     return result;
 }
 
 // static
 SkPDFImage* SkPDFImage::CreateImage(const SkBitmap& bitmap,
                                     const SkIRect& srcRect,
-                                    EncodeToDCTStream encoder) {
+                                    SkPicture::EncodeBitmap encoder) {
     if (bitmap.getConfig() == SkBitmap::kNo_Config) {
         return NULL;
     }
 
     bool isTransparent = false;
     SkAutoTUnref<SkStream> alphaData;
     if (!bitmap.isOpaque()) {
         // Note that isOpaque is not guaranteed to return false for bitmaps
         // with alpha support but a completely opaque alpha channel,
         // so alphaData may still be NULL if we have a completely opaque
@@ skipping 30 matching lines @@
 
 void SkPDFImage::getResources(const SkTSet<SkPDFObject*>& knownResourceObjects,
                               SkTSet<SkPDFObject*>* newResourceObjects) {
     GetResourcesHelper(&fResources, knownResourceObjects, newResourceObjects);
 }
 
 SkPDFImage::SkPDFImage(SkStream* stream,
                        const SkBitmap& bitmap,
                        bool isAlpha,
                        const SkIRect& srcRect,
-                       EncodeToDCTStream encoder)
+                       SkPicture::EncodeBitmap encoder)
     : fIsAlpha(isAlpha),
       fSrcRect(srcRect),
       fEncoder(encoder) {
 
     if (bitmap.isImmutable()) {
         fBitmap = bitmap;
     } else {
         bitmap.deepCopyTo(&fBitmap, bitmap.config());
         fBitmap.setImmutable();
     }
@@ skipping 71 matching lines @@
     // Nothing to do here - the image params are already copied in SkPDFStream's
     // constructor, and the bitmap will be regenerated and encoded in
     // populate.
 }
 
 bool SkPDFImage::populate(SkPDFCatalog* catalog) {
     if (getState() == kUnused_State) {
         // Initializing image data for the first time.
         SkDynamicMemoryWStream dctCompressedWStream;
         if (!skip_compression(catalog) && fEncoder &&
-                get_uncompressed_size(fBitmap, fSrcRect) > 1 &&
-                fEncoder(&dctCompressedWStream, fBitmap, fSrcRect) &&
-                dctCompressedWStream.getOffset() <
-                            get_uncompressed_size(fBitmap, fSrcRect)) {
-            SkAutoTUnref<SkData> data(dctCompressedWStream.copyToData());
-            SkAutoTUnref<SkStream> stream(SkNEW_ARGS(SkMemoryStream, (data)));
-            setData(stream.get());
+                get_uncompressed_size(fBitmap, fSrcRect) > 1) {
+            SkBitmap subset;
+            // Extract subset

[vandebo (ex-Chrome), 2013/09/27 16:06:13]

It's unfortunate that we have to do this without giving the caller a chance to
make a decision first.

+            if (!fBitmap.extractSubset(&subset, fSrcRect)) {

[vandebo (ex-Chrome), 2013/09/27 16:06:13]

extractSubset doesn't have any special cases for fSrcRect beign the same size as
the image, so you might want to do that here.

[reed1, 2013/09/27 16:15:29]

What is the concern with always calling extractSubset()?

[vandebo (ex-Chrome), 2013/09/27 16:16:37]

Just that it's unnecessary computational overhead when the subset == the full
image.

+                // If Extract subset fails, make a deep copy.
+                if (!fBitmap.copyTo(&subset, fBitmap.config())) {

[vandebo (ex-Chrome), 2013/09/27 16:06:13]

Don't you still need to subset the image after this?

[edisonn, 2013/09/27 17:57:20]

removed. extractSubset will not fork for A1, if that is a mojor missing feature,
We will implement it later.

+                    return false;
+                }
+            }
+            size_t pixelRefOffset = 0;
+            SkAutoTUnref<SkData> data(fEncoder(&pixelRefOffset, subset));
+            if (data.get() && data->size() < get_uncompressed_size(fBitmap, fSrcRect)) {

[vandebo (ex-Chrome), 2013/09/27 16:06:13]

This file is basically 80-column clean, so please keep it that way (so says the
style guide).

[edisonn, 2013/09/27 17:57:20]

Done.

+                SkAutoTUnref<SkStream> stream(SkNEW_ARGS(SkMemoryStream, (data)));
+                setData(stream.get());
 
-            insertName("Filter", "DCTDecode");
-            insertInt("ColorTransform", kNoColorTransform);
-            insertInt("Length", getData()->getLength());
-            setState(kCompressed_State);
-            return true;
+                insertName("Filter", "DCTDecode");
+                insertInt("ColorTransform", kNoColorTransform);
+                insertInt("Length", getData()->getLength());
+                setState(kCompressed_State);
+                return true;
+            }
         }
         // Fallback method
         if (!fStreamValid) {
             SkAutoTUnref<SkStream> stream(
                     extract_image_data(fBitmap, fSrcRect, fIsAlpha, NULL));
             setData(stream);
             fStreamValid = true;
         }
         return INHERITED::populate(catalog);
     } else if (getState() == kNoCompression_State &&
             !skip_compression(catalog) &&
             (SkFlate::HaveFlate() || fEncoder)) {
         // Compression has not been requested when the stream was first created,
         // but the new catalog wants it compressed.
         if (!getSubstitute()) {
             SkPDFStream* substitute = SkNEW_ARGS(SkPDFImage, (*this));
             setSubstitute(substitute);
             catalog->setSubstitute(this, substitute);
         }
         return false;
     }
     return true;
 }