SkPDF: refactor font subset: fewer copies

src/pdf/SkPDFFont.cpp

 /*
  * Copyright 2011 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include <ctype.h>
 
 #include "SkData.h"
@@ skipping 566 matching lines @@
         uint16_t lastGlyphID) {
     SkDynamicMemoryWStream cmap;
     if (multiByteGlyphs) {
         append_tounicode_header(&cmap, firstGlyphID, lastGlyphID);
     } else {
         append_tounicode_header(&cmap, 1, lastGlyphID - firstGlyphID + 1);
     }
     append_cmap_sections(glyphToUnicode, subset, &cmap, multiByteGlyphs,
                          firstGlyphID, lastGlyphID);
     append_cmap_footer(&cmap);
-    sk_sp<SkData> cmapData(cmap.copyToData());
-    return sk_make_sp<SkPDFStream>(cmapData.get());
+    return sk_make_sp<SkPDFStream>(
+            std::unique_ptr<SkStreamAsset>(cmap.detachAsStream()));
 }
 
-#if defined (SK_SFNTLY_SUBSETTER)
-static void sk_delete_array(const void* ptr, void*) {
-    // Use C-style cast to cast away const and cast type simultaneously.
-    delete[] (unsigned char*)ptr;
-}
-#endif
-
-#if defined(SK_SFNTLY_SUBSETTER)
-static size_t get_subset_font_stream(const char* fontName,
-                                     const SkTypeface* typeface,
-                                     const SkTDArray<uint32_t>& subset,
-                                     SkPDFStream** fontStream) {
-    int ttcIndex;
-    std::unique_ptr<SkStreamAsset> fontData(typeface->openStream(&ttcIndex));
-    SkASSERT(fontData);
-    if (!fontData) {
-        return 0;
-    }
-
-    size_t fontSize = fontData->getLength();
-
-    // Read font into buffer.
-    SkPDFStream* subsetFontStream = nullptr;
-    SkTDArray<unsigned char> originalFont;
-    originalFont.setCount(SkToInt(fontSize));
-    if (fontData->read(originalFont.begin(), fontSize) == fontSize) {
-        unsigned char* subsetFont = nullptr;
-        // sfntly requires unsigned int* to be passed in, as far as we know,
-        // unsigned int is equivalent to uint32_t on all platforms.
-        static_assert(sizeof(unsigned int) == sizeof(uint32_t), "unsigned_int_not_32_bits");
-        int subsetFontSize = SfntlyWrapper::SubsetFont(fontName,
-                                                       originalFont.begin(),
-                                                       fontSize,
-                                                       subset.begin(),
-                                                       subset.count(),
-                                                       &subsetFont);
-        if (subsetFontSize > 0 && subsetFont != nullptr) {
-            SkAutoDataUnref data(SkData::NewWithProc(subsetFont,
-                                                     subsetFontSize,
-                                                     sk_delete_array,
-                                                     nullptr));
-            subsetFontStream = new SkPDFStream(data.get());
-            fontSize = subsetFontSize;
-        }
-    }
-    if (subsetFontStream) {
-        *fontStream = subsetFontStream;
-        return fontSize;
-    }
-    fontData->rewind();
-
-    // Fail over: just embed the whole font.
-    *fontStream = new SkPDFStream(std::move(fontData));
-    return fontSize;
-}
-#endif
-
 ///////////////////////////////////////////////////////////////////////////////
 // class SkPDFGlyphSet
 ///////////////////////////////////////////////////////////////////////////////
 
 SkPDFGlyphSet::SkPDFGlyphSet() : fBitSet(SK_MaxU16 + 1) {
 }
 
 void SkPDFGlyphSet::set(const uint16_t* glyphIDs, int numGlyphs) {
     for (int i = 0; i < numGlyphs; ++i) {
         fBitSet.setBit(glyphIDs[i], true);
     }
 }
 
 bool SkPDFGlyphSet::has(uint16_t glyphID) const {
     return fBitSet.isBitSet(glyphID);
 }
 
 void SkPDFGlyphSet::exportTo(SkTDArray<unsigned int>* glyphIDs) const {
     fBitSet.exportTo(glyphIDs);
 }
- 
+
 ///////////////////////////////////////////////////////////////////////////////
 // class SkPDFGlyphSetMap
 ///////////////////////////////////////////////////////////////////////////////
 
 SkPDFGlyphSetMap::SkPDFGlyphSetMap() {}
 
 SkPDFGlyphSetMap::~SkPDFGlyphSetMap() {
     fMap.reset();
 }
 
@@ skipping 329 matching lines @@
 
 SkPDFCIDFont::SkPDFCIDFont(const SkAdvancedTypefaceMetrics* info,
                            SkTypeface* typeface,
                            const SkPDFGlyphSet* subset)
     : SkPDFFont(info, typeface, nullptr) {
     this->populate(subset);
 }
 
 SkPDFCIDFont::~SkPDFCIDFont() {}
 
+#ifdef SK_SFNTLY_SUBSETTER
+// if possible, make no copy.
+static sk_sp<SkData> stream_to_data(std::unique_ptr<SkStreamAsset> stream) {
+    SkASSERT(stream);
+    (void)stream->rewind();
+    SkASSERT(stream->hasLength());
+    size_t size = stream->getLength();
+    if (const void* base = stream->getMemoryBase()) {
+        SkData::ReleaseProc proc =
+            [](const void*, void* ctx) { delete (SkStream*)ctx; };
+        return SkData::MakeWithProc(base, size, proc, stream.release());
+    }
+    return SkData::MakeFromStream(stream.get(), size);
+}
+
+static sk_sp<SkPDFObject> get_subset_font_stream(
+        std::unique_ptr<SkStreamAsset> fontAsset,
+        const SkTDArray<uint32_t>& subset,
+        const char* fontName) {
+    sk_sp<SkData> fontData(stream_to_data(std::move(fontAsset)));
+    unsigned char* subsetFont = nullptr;
+    // sfntly requires unsigned int* to be passed in,
+    // as far as we know, unsigned int is equivalent
+    // to uint32_t on all platforms.
+    static_assert(sizeof(unsigned) == sizeof(uint32_t), "");
+    // TODO(halcanary): Use ttcIndex, not fontName.
+    int subsetFontSize =
+        SfntlyWrapper::SubsetFont(fontName,
+                                  fontData->bytes(),
+                                  fontData->size(),
+                                  subset.begin(),
+                                  subset.count(),
+                                  &subsetFont);
+    fontData = nullptr;  // Release memory if necessary.

[bungeman-skia, 2016/07/27 20:15:59]

Instead of doing this, can this be like

unsigned char* subsetFont = nullptr;
int subsetFontSize = 0;
{
    sk_sp<SkData> fontData(...);
    subsetFontSize = ...
}

or even something roughly like

unsigned char* subsetFont = nullptr;
int subsetFontSize = [&]() {
    sk_sp<SkData> fontData(...);
    return SfntlyWrapper::SubsetFont( ... );
}

so that fontData just naturally goes out of scope.

[hal.canary, 2016/07/27 20:23:04]

Done.

+    SkASSERT(subsetFontSize > 0 || subsetFont == nullptr);
+    if (subsetFontSize > 0) {

[bungeman-skia, 2016/07/27 20:15:59]

Can we reverse this so that less code goes into the block.

if (subsetFontSize < 1) {
    return nullptr;
}

...

Then the code that's currently in the block is less indented, so it can fit more
per line. Also, the 'successful' 'return subset;' really wants to be on the last
line of this function.

[hal.canary, 2016/07/27 20:23:04]

Done.

+        SkASSERT(subsetFont != nullptr);
+        auto subset = sk_make_sp<SkPDFStream>(
+                SkData::MakeWithProc(
+                        subsetFont, subsetFontSize,
+                        [](const void* p, void*) {
+                            delete[] (unsigned char*)p;
+                        }, nullptr));
+        subset->insertInt("Length1", subsetFontSize);
+        return subset;
+    }
+    return nullptr;
+}
+#endif  // SK_SFNTLY_SUBSETTER
+
 bool SkPDFCIDFont::addFontDescriptor(int16_t defaultWidth,
                                      const SkTDArray<uint32_t>* subset) {
     auto descriptor = sk_make_sp<SkPDFDict>("FontDescriptor");
     setFontDescriptor(descriptor.get());
     if (!addCommonFontDescriptorEntries(defaultWidth)) {
         this->insertObjRef("FontDescriptor", std::move(descriptor));
         return false;
     }
     SkASSERT(this->canEmbed());
 
     switch (getType()) {
         case SkAdvancedTypefaceMetrics::kTrueType_Font: {
-            size_t fontSize = 0;
-#if defined(SK_SFNTLY_SUBSETTER)
-            if (this->canSubset()) {
-                sk_sp<SkPDFStream> fontStream;
-                SkPDFStream* rawStream = nullptr;
-                fontSize = get_subset_font_stream(fontInfo()->fFontName.c_str(),
-                                                  typeface(),
-                                                  *subset,
-                                                  &rawStream);
-                if (0 == fontSize) {
-                    return false;
-                }
-                if (rawStream) {
-                    fontStream.reset(rawStream);
-                    fontStream->insertInt("Length1", fontSize);
-                    descriptor->insertObjRef("FontFile2", std::move(fontStream));
+            int ttcIndex;
+            std::unique_ptr<SkStreamAsset> fontAsset(
+                    this->typeface()->openStream(&ttcIndex));
+            SkASSERT(fontAsset);
+            if (!fontAsset) {
+                return false;
+            }
+            size_t fontSize = fontAsset->getLength();
+            SkASSERT(fontSize > 0);
+            if (fontSize == 0) {
+                return false;
+            }
+
+            #ifdef SK_SFNTLY_SUBSETTER
+            if (this->canSubset() && subset) {
+                sk_sp<SkPDFObject> subsetStream = get_subset_font_stream(
+                        std::move(fontAsset), *subset, fontInfo()->fFontName.c_str());
+                if (subsetStream) {
+                    descriptor->insertObjRef("FontFile2", std::move(subsetStream));
                     break;
                 }
+                // If subsetting fails, fall back to original font data.
+                fontAsset.reset(this->typeface()->openStream(&ttcIndex));
             }
-#endif
-            sk_sp<SkPDFSharedStream> fontStream;
-            std::unique_ptr<SkStreamAsset> fontData(
-                    this->typeface()->openStream(nullptr));
-            SkASSERT(fontData);
-            if (!fontData || 0 == fontData->getLength()) {
-                return false;
-            }
-            fontSize = fontData->getLength();
-            SkASSERT(fontSize > 0);
-            fontStream.reset(new SkPDFSharedStream(fontData.release()));
+            #endif  // SK_SFNTLY_SUBSETTER
+            auto fontStream = sk_make_sp<SkPDFSharedStream>(fontAsset.release());
             fontStream->dict()->insertInt("Length1", fontSize);
             descriptor->insertObjRef("FontFile2", std::move(fontStream));
             break;
         }
         case SkAdvancedTypefaceMetrics::kCFF_Font:
         case SkAdvancedTypefaceMetrics::kType1CID_Font: {
             std::unique_ptr<SkStreamAsset> fontData(
                     this->typeface()->openStream(nullptr));
             SkASSERT(fontData);
             SkASSERT(fontData->getLength() > 0);
@@ skipping 130 matching lines @@
     SkASSERT(rawFontData->getLength() > 0);
     if (!rawFontData || 0 == rawFontData->getLength()) {
         return false;
     }
     sk_sp<SkData> fontData(handle_type1_stream(rawFontData.get(), &header,
                                                       &data, &trailer));
     if (fontData.get() == nullptr) {
         return false;
     }
     SkASSERT(this->canEmbed());
-    auto fontStream = sk_make_sp<SkPDFStream>(fontData.get());
+    auto fontStream = sk_make_sp<SkPDFStream>(std::move(fontData));
     fontStream->insertInt("Length1", header);
     fontStream->insertInt("Length2", data);
     fontStream->insertInt("Length3", trailer);
     descriptor->insertObjRef("FontFile", std::move(fontStream));
 
     this->insertObjRef("FontDescriptor", std::move(descriptor));
 
     return addCommonFontDescriptorEntries(defaultWidth);
 }
 
@@ skipping 194 matching lines @@
     }
     return *canon->fCanEmbedTypeface.set(id, canEmbed);
 }
 
 void SkPDFFont::drop() {
     fTypeface = nullptr;
     fFontInfo = nullptr;
     fDescriptor = nullptr;
     this->SkPDFDict::drop();
 }