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;
+#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);
 }
 #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 341 matching lines @@
     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<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), "");
+                const char* fontName = fontInfo()->fFontName.c_str();
+                // TODO(halcanary): Use ttcIndex, not fontName.
+                int subsetFontSize =
+                    SfntlyWrapper::SubsetFont(fontName,
+                                              fontData->bytes(),
+                                              fontSize,
+                                              subset->begin(),
+                                              subset->count(),
+                                              &subsetFont);
+                SkASSERT(subsetFontSize > 0 || subsetFont == nullptr);
+                if (subsetFontSize > 0) {
+                    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);
+                    descriptor->insertObjRef("FontFile2", std::move(subset));
                     break;
                 }
+                // If subsetting fails, fall back to original font data.
+                fontAsset.reset(this->typeface()->openStream(&ttcIndex));

[bungeman-skia, 2016/07/27 19:26:44]

Hopefully it doesn't matter too much, but note that this means 'fontData' and
'fontAsset' are both active here, meaning that there may be two copies / mmaps
of the same data in memory (and the the 'subsetFont' data as well). Indeed, the
lifetime of 'fontData' appears to just be around the call to SubsetFont, it
could go away even before making the SkPDFStream.

I know inlining this made some of this more straight forward, but the five
levels of indentation plus indentation for expression continuation makes this
much more difficult to take in.

[hal.canary, 2016/07/27 19:54:26]

Fixed.  now fontData is out of scope (in another fn) before this call.
nope.
done.
fixed.

             }
-#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();
 }