SkPDF: SkPDFDocument reorganized so that some objects can be serialized early.

src/pdf/SkPDFDocument.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 "SkPDFCanon.h"
 #include "SkPDFDevice.h"
 #include "SkPDFDocument.h"
 #include "SkPDFFont.h"
-#include "SkPDFMetadata.h"
 #include "SkPDFStream.h"
 #include "SkPDFTypes.h"
 #include "SkPDFUtils.h"
 #include "SkStream.h"
 
-static void emit_pdf_header(SkWStream* stream) {
-    stream->writeText("%PDF-1.4\n%");
-    // The PDF spec recommends including a comment with four bytes, all
-    // with their high bits set.  This is "Skia" with the high bits set.
-    stream->write32(0xD3EBE9E1);
-    stream->writeText("\n");
+
+SkPDFObjectSerializer::SkPDFObjectSerializer() : fBaseOffset(0), fNextToBeSerialized(0) {}
+
+template <class T> static void renew(T* t) { t->~T(); new (t) T; }
+
+void SkPDFObjectSerializer::addObjectRecursively(const sk_sp<SkPDFObject>& object) {
+    fObjNumMap.addObjectRecursively(object.get(), fSubstituteMap);
 }
 
-static void emit_pdf_footer(SkWStream* stream,
-                            const SkPDFObjNumMap& objNumMap,
-                            const SkPDFSubstituteMap& substitutes,
-                            SkPDFObject* docCatalog,
-                            int64_t objCount,
-                            int32_t xRefFileOffset,
-                            sk_sp<SkPDFObject> info,
-                            sk_sp<SkPDFObject> id) {
+void SkPDFObjectSerializer::serializeHeader(SkWStream* wStream, const SkPDFMetadata& md) {
+    fBaseOffset = wStream->bytesWritten();
+    static const char kHeader[] = "%PDF-1.4\n%\xE1\xE9\xEB\xD3\n";
+    wStream->write(kHeader, strlen(kHeader));
+    // The PDF spec recommends including a comment with four
+    // bytes, all with their high bits set.  "\xD3\xEB\xE9\xE1" is
+    // "Skia" with the high bits set.
+    fInfoDict.reset(md.createDocumentInformationDict());
+    this->addObjectRecursively(fInfoDict);
+    this->serializeObjects(wStream);
+}
+
+// Serialize all objects in the fObjNumMap that have not yet been serialized;
+void SkPDFObjectSerializer::serializeObjects(SkWStream* wStream) {
+    const auto& objects = fObjNumMap.objects();

[tomhudson, 2016/03/21 15:55:11]

Still trying to stop worrying and love the auto; it seems to me that this is
landing on both the pro and con side of
https://google.github.io/styleguide/cppguide.html#auto. If fObjNumMap.objects()
were transparently typed that might be OK, but having to call get() is a
surprise that tells me that perhaps this is a more complicated than we'd intuit
from just seeing a bare auto?

[hal.canary, 2016/03/21 18:49:18]

Done.

+    while (fNextToBeSerialized < objects.count()) {
+        SkPDFObject* object = objects[fNextToBeSerialized].get();
+        int32_t index = fNextToBeSerialized + 1;  // Skip object 0.

[tomhudson, 2016/03/21 15:55:11]

I can see equivalent code in the previous version (lines 234-235), but I'd like
to know *why* you aren't writing object 0. Is PDF's array 1-based? Am I missing
a note about this somewhere else in our codebase?

[hal.canary, 2016/03/21 18:49:18]

This is a PDF thing.  Object zero is special.

"The first entry in the table (object number 0) is always free and has a
generation number of 65,535; it is the head of the linked list of free objects."

+        SkASSERT(fOffsets.count() == fNextToBeSerialized);
+        fOffsets.push(this->offset(wStream));
+        SkASSERT(object == fSubstituteMap.getSubstitute(object));
+        wStream->writeDecAsText(index);
+        wStream->writeText(" 0 obj\n");  // Generation number is always 0.
+        object->emitObject(wStream, fObjNumMap, fSubstituteMap);
+        wStream->writeText("\nendobj\n");
+        object->dump();
+        ++fNextToBeSerialized;
+    }
+}
+
+// Xref table and footer
+void SkPDFObjectSerializer::serializeFooter(SkWStream* wStream,
+                                            const sk_sp<SkPDFObject> docCatalog,
+                                            sk_sp<SkPDFObject> id) {
+    this->serializeObjects(wStream);
+    int32_t xRefFileOffset = this->offset(wStream);
+    // Include the zeroth object in the count.

[tomhudson, 2016/03/21 15:55:12]

Yeah, confusing after we previously skipped object zero.

[hal.canary, 2016/03/21 18:49:18]

Acknowledged.

+    int32_t objCount = SkToS32(fOffsets.count() + 1);

[tomhudson, 2016/03/21 15:55:12]

Switched from int64_t to int32_t?

[hal.canary, 2016/03/21 18:49:17]

For whatever reason, `writeDecAsText` takes an int32_t.

+    wStream->writeText("xref\n0 ");
+    wStream->writeDecAsText(objCount);
+    wStream->writeText("\n0000000000 65535 f \n");
+    for (int i = 0; i < fOffsets.count(); i++) {
+        wStream->writeBigDecAsText(fOffsets[i], 10);
+        wStream->writeText(" 00000 n \n");
+    }
     SkPDFDict trailerDict;
-    // TODO(http://crbug.com/80908): Linearized format will take a
-    //                               Prev entry too.
-    trailerDict.insertInt("Size", int(objCount));
-    trailerDict.insertObjRef("Root", sk_ref_sp(docCatalog));
-    SkASSERT(info);
-    trailerDict.insertObjRef("Info", std::move(info));
+    trailerDict.insertInt("Size", objCount);
+    SkASSERT(docCatalog);
+    trailerDict.insertObjRef("Root", docCatalog);

[tomhudson, 2016/03/21 15:55:12]

In the old code we explicitly ref'd this here?

[hal.canary, 2016/03/21 18:49:18]

we switched to using sk_sp<T>  no need.

+    SkASSERT(fInfoDict);
+    trailerDict.insertObjRef("Info", std::move(fInfoDict));
     if (id) {
         trailerDict.insertObject("ID", std::move(id));
     }
-    stream->writeText("trailer\n");
-    trailerDict.emitObject(stream, objNumMap, substitutes);
-    stream->writeText("\nstartxref\n");
-    stream->writeBigDecAsText(xRefFileOffset);
-    stream->writeText("\n%%EOF");
+    wStream->writeText("trailer\n");
+    trailerDict.emitObject(wStream, fObjNumMap, fSubstituteMap);
+    wStream->writeText("\nstartxref\n");
+    wStream->writeBigDecAsText(xRefFileOffset);
+    wStream->writeText("\n%%EOF");
+}
+
+int32_t SkPDFObjectSerializer::offset(SkWStream* wStream) {
+    size_t offset = wStream->bytesWritten();
+    SkASSERT(offset > fBaseOffset);
+    return SkToS32(offset - fBaseOffset);
 }
 
 static void perform_font_subsetting(
         const SkTArray<sk_sp<const SkPDFDevice>>& pageDevices,
         SkPDFSubstituteMap* substituteMap) {
     SkASSERT(substituteMap);
 
     SkPDFGlyphSetMap usage;
     for (const sk_sp<const SkPDFDevice>& pageDevice : pageDevices) {
         usage.merge(pageDevice->getFontGlyphUsage());
@@ skipping 93 matching lines @@
 
         curNodes = nextRoundNodes;
         nextRoundNodes.rewind();
         treeCapacity *= kNodeSize;
     } while (curNodes.count() > 1);
 
     pageTree->push(curNodes[0]);  // Transfer reference.
     return sk_ref_sp(curNodes[0]);
 }
 
-static bool emit_pdf_document(const SkTArray<sk_sp<const SkPDFDevice>>& pageDevices,
-                              const SkPDFMetadata& metadata,
-                              SkWStream* stream) {
-    if (pageDevices.empty()) {
-        return false;
-    }
-
-    SkTDArray<SkPDFDict*> pages;  // TODO: SkTArray<sk_sp<SkPDFDict>>
-    auto dests = sk_make_sp<SkPDFDict>();
-
-    for (const sk_sp<const SkPDFDevice>& pageDevice : pageDevices) {
-        SkASSERT(pageDevice);
-        SkASSERT(pageDevices[0]->getCanon() == pageDevice->getCanon());
-        sk_sp<SkPDFDict> page(create_pdf_page(pageDevice.get()));
-        pageDevice->appendDestinations(dests.get(), page.get());
-        pages.push(page.release());
-    }
-
-    auto docCatalog = sk_make_sp<SkPDFDict>("Catalog");
-
-    sk_sp<SkPDFObject> infoDict(metadata.createDocumentInformationDict());
-
-    sk_sp<SkPDFObject> id, xmp;
-#ifdef SK_PDF_GENERATE_PDFA
-    SkPDFMetadata::UUID uuid = metadata.uuid();
-    // We use the same UUID for Document ID and Instance ID since this
-    // is the first revision of this document (and Skia does not
-    // support revising existing PDF documents).
-    // If we are not in PDF/A mode, don't use a UUID since testing
-    // works best with reproducible outputs.
-    id.reset(SkPDFMetadata::CreatePdfId(uuid, uuid));
-    xmp.reset(metadata.createXMPObject(uuid, uuid));
-    docCatalog->insertObjRef("Metadata", std::move(xmp));
-
-    // sRGB is specified by HTML, CSS, and SVG.
-    auto outputIntent = sk_make_sp<SkPDFDict>("OutputIntent");
-    outputIntent->insertName("S", "GTS_PDFA1");
-    outputIntent->insertString("RegistryName", "http://www.color.org");
-    outputIntent->insertString("OutputConditionIdentifier",
-                               "sRGB IEC61966-2.1");
-    auto intentArray = sk_make_sp<SkPDFArray>();
-    intentArray->appendObject(std::move(outputIntent));
-    // Don't specify OutputIntents if we are not in PDF/A mode since
-    // no one has ever asked for this feature.
-    docCatalog->insertObject("OutputIntents", std::move(intentArray));
-#endif
-
-    SkTDArray<SkPDFDict*> pageTree;
-    docCatalog->insertObjRef("Pages", generate_page_tree(pages, &pageTree));
-
-    if (dests->size() > 0) {
-        docCatalog->insertObjRef("Dests", std::move(dests));
-    }
-
-    // Build font subsetting info before proceeding.
-    SkPDFSubstituteMap substitutes;
-    perform_font_subsetting(pageDevices, &substitutes);
-
-    SkPDFObjNumMap objNumMap;
-    objNumMap.addObjectRecursively(infoDict.get(), substitutes);
-    objNumMap.addObjectRecursively(docCatalog.get(), substitutes);
-    size_t baseOffset = stream->bytesWritten();
-    emit_pdf_header(stream);
-    SkTDArray<int32_t> offsets;
-    for (int i = 0; i < objNumMap.objects().count(); ++i) {
-        SkPDFObject* object = objNumMap.objects()[i].get();
-        size_t offset = stream->bytesWritten();
-        // This assert checks that size(pdf_header) > 0 and that
-        // the output stream correctly reports bytesWritten().
-        SkASSERT(offset > baseOffset);
-        offsets.push(SkToS32(offset - baseOffset));
-        SkASSERT(object == substitutes.getSubstitute(object));
-        SkASSERT(objNumMap.getObjectNumber(object) == i + 1);
-        stream->writeDecAsText(i + 1);
-        stream->writeText(" 0 obj\n");  // Generation number is always 0.
-        object->emitObject(stream, objNumMap, substitutes);
-        stream->writeText("\nendobj\n");
-        object->dump();
-    }
-    int32_t xRefFileOffset = SkToS32(stream->bytesWritten() - baseOffset);
-
-    // Include the zeroth object in the count.
-    int32_t objCount = SkToS32(offsets.count() + 1);
-
-    stream->writeText("xref\n0 ");
-    stream->writeDecAsText(objCount);
-    stream->writeText("\n0000000000 65535 f \n");
-    for (int i = 0; i < offsets.count(); i++) {
-        SkASSERT(offsets[i] > 0);
-        stream->writeBigDecAsText(offsets[i], 10);
-        stream->writeText(" 00000 n \n");
-    }
-    emit_pdf_footer(stream, objNumMap, substitutes, docCatalog.get(), objCount,
-                    xRefFileOffset, std::move(infoDict), std::move(id));
-
-    // The page tree has both child and parent pointers, so it creates a
-    // reference cycle.  We must clear that cycle to properly reclaim memory.
-    for (int i = 0; i < pageTree.count(); i++) {
-        pageTree[i]->dump();
-    }
-    pageTree.safeUnrefAll();
-    pages.unrefAll();
-    return true;
-}
-
 #if 0
 // TODO(halcanary): expose notEmbeddableCount in SkDocument
 void GetCountOfFontTypes(
         const SkTDArray<SkPDFDevice*>& pageDevices,
         int counts[SkAdvancedTypefaceMetrics::kOther_Font + 1],
         int* notSubsettableCount,
         int* notEmbeddableCount) {
     sk_bzero(counts, sizeof(int) *
                      (SkAdvancedTypefaceMetrics::kOther_Font + 1));
     SkTDArray<SkFontID> seenFonts;
@@ skipping 23 matching lines @@
     }
     if (notEmbeddableCount) {
         *notEmbeddableCount = notEmbeddable;
     }
 }
 #endif
 
 template <typename T> static T* clone(const T* o) { return o ? new T(*o) : nullptr; }
 ////////////////////////////////////////////////////////////////////////////////
 
-namespace {
-class SkPDFDocument : public SkDocument {
-public:
-    SkPDFDocument(SkWStream* stream,
-                   void (*doneProc)(SkWStream*, bool),
-                   SkScalar rasterDpi,
-                   SkPixelSerializer* jpegEncoder)
-        : SkDocument(stream, doneProc)
-        , fRasterDpi(rasterDpi) {
-        fCanon.setPixelSerializer(SkSafeRef(jpegEncoder));
+SkPDFDocument::SkPDFDocument(SkWStream* stream,
+                             void (*doneProc)(SkWStream*, bool),
+                             SkScalar rasterDpi,
+                             SkPixelSerializer* jpegEncoder)
+    : SkDocument(stream, doneProc)
+    , fRasterDpi(rasterDpi) {
+    fCanon.setPixelSerializer(SkSafeRef(jpegEncoder));
+}
+
+SkPDFDocument::~SkPDFDocument() {
+    // subclasses must call close() in their destructors

[tomhudson, 2016/03/21 15:55:11]

subclasses of SkDocument, or subclasses of SkPDFDocument?
(If the latter, won't this automatically be invoked as subclass destructors fall
through to superclass destructors? The SkDocument header comment says that
doesn't work, because SkDocument::close() isn't virtual, but I don't see why
that's necessarily true?)

[hal.canary, 2016/03/21 18:49:18]

 // subclasses of SkDocument 

+    this->close();
+}
+
+void SkPDFDocument::serialize(const sk_sp<SkPDFObject>& object) {

[tomhudson, 2016/03/21 15:55:11]

Name reads like it's going to serialize a single object, but implementation
reads like it's going to serialize the entire document, including the new
object?

[hal.canary, 2016/03/21 18:49:18]

new comments:

    /**
       Serialize the object, as well as any other objects it
       indirectly refers to.  If any any other objects have been added
       to the SkPDFObjNumMap without serializing them, they will be
       serialized as well. 

       It might go without saying that objects should not be changed
       after calling serialize, since those changes will be too late.
       The same goes for changes to the SkPDFSubstituteMap that effect
       the object or its dependencies.
     */

+    fObjectSerializer.addObjectRecursively(object);
+    fObjectSerializer.serializeObjects(this->getStream());
+}
+
+SkCanvas* SkPDFDocument::onBeginPage(SkScalar width, SkScalar height,
+                                     const SkRect& trimBox) {
+    SkASSERT(!fCanvas.get());  // end page was called.

[tomhudson, 2016/03/21 15:55:12]

Nit: This comment unclear.
Reading through the code further it becomes understandable, but I'd either
expand so it can be grasped completely here, or remove.

[hal.canary, 2016/03/21 18:49:18]

Done.

+    if (fPageDevices.empty()) {  // if first page
+        fObjectSerializer.serializeHeader(this->getStream(), fMetadata);
+    }
+    SkISize pageSize = SkISize::Make(
+            SkScalarRoundToInt(width), SkScalarRoundToInt(height));
+    sk_sp<SkPDFDevice> device(
+            SkPDFDevice::Create(pageSize, fRasterDpi, &fCanon));
+    fCanvas.reset(new SkCanvas(device.get()));
+    fPageDevices.push_back(std::move(device));
+    fCanvas->clipRect(trimBox);
+    fCanvas->translate(trimBox.x(), trimBox.y());
+    return fCanvas.get();
+}
+
+void SkPDFDocument::onEndPage() {
+    SkASSERT(fCanvas.get());
+    fCanvas->flush();
+    fCanvas.reset(nullptr);
+}
+
+void SkPDFDocument::onAbort() { 
+    fPageDevices.reset();
+    fCanon.reset();
+    renew(&fObjectSerializer);

[tomhudson, 2016/03/21 15:55:11]

Should we be resetting fCanvas?

[hal.canary, 2016/03/21 18:49:18]

Done.

+}
+
+void SkPDFDocument::setMetadata(const SkDocument::Attribute info[],
+                                int infoCount,
+                                const SkTime::DateTime* creationDate,
+                                const SkTime::DateTime* modifiedDate) {
+    fMetadata.fInfo.reset(info, infoCount);
+    fMetadata.fCreation.reset(clone(creationDate));
+    fMetadata.fModified.reset(clone(modifiedDate));
+}
+
+bool SkPDFDocument::onClose(SkWStream* stream) {
+    SkASSERT(!fCanvas.get());
+    if (fPageDevices.empty()) {
+        fPageDevices.reset();
+        fCanon.reset();
+        renew(&fObjectSerializer);
+        return false;
+    }
+    SkTDArray<SkPDFDict*> pages;  // TODO: SkTArray<sk_sp<SkPDFDict>>
+    auto dests = sk_make_sp<SkPDFDict>();
+
+    for (const sk_sp<const SkPDFDevice>& pageDevice : fPageDevices) {
+        SkASSERT(pageDevice);
+        SkASSERT(fPageDevices[0]->getCanon() == pageDevice->getCanon());
+        sk_sp<SkPDFDict> page(create_pdf_page(pageDevice.get()));
+        pageDevice->appendDestinations(dests.get(), page.get());
+        pages.push(page.release());
     }
 
-    virtual ~SkPDFDocument() {
-        // subclasses must call close() in their destructors
-        this->close();
+    auto docCatalog = sk_make_sp<SkPDFDict>("Catalog");
+
+    sk_sp<SkPDFObject> id, xmp;
+    #ifdef SK_PDF_GENERATE_PDFA
+        SkPDFMetadata::UUID uuid = metadata.uuid();
+        // We use the same UUID for Document ID and Instance ID since this
+        // is the first revision of this document (and Skia does not
+        // support revising existing PDF documents).
+        // If we are not in PDF/A mode, don't use a UUID since testing
+        // works best with reproducible outputs.
+        id.reset(SkPDFMetadata::CreatePdfId(uuid, uuid));
+        xmp.reset(metadata.createXMPObject(uuid, uuid));
+        docCatalog->insertObjRef("Metadata", std::move(xmp));
+
+        // sRGB is specified by HTML, CSS, and SVG.
+        auto outputIntent = sk_make_sp<SkPDFDict>("OutputIntent");
+        outputIntent->insertName("S", "GTS_PDFA1");
+        outputIntent->insertString("RegistryName", "http://www.color.org");
+        outputIntent->insertString("OutputConditionIdentifier",
+                                   "sRGB IEC61966-2.1");
+        auto intentArray = sk_make_sp<SkPDFArray>();
+        intentArray->appendObject(std::move(outputIntent));
+        // Don't specify OutputIntents if we are not in PDF/A mode since
+        // no one has ever asked for this feature.
+        docCatalog->insertObject("OutputIntents", std::move(intentArray));
+    #endif
+
+    SkTDArray<SkPDFDict*> pageTree;
+    docCatalog->insertObjRef("Pages", generate_page_tree(pages, &pageTree));
+
+    if (dests->size() > 0) {
+        docCatalog->insertObjRef("Dests", std::move(dests));
     }
 
-protected:
-    SkCanvas* onBeginPage(SkScalar width, SkScalar height,
-                          const SkRect& trimBox) override {
-        SkASSERT(!fCanvas.get());
+    // Build font subsetting info before proceeding.

[tomhudson, 2016/03/21 15:55:12]

Nit: unnecessary comment?

[hal.canary, 2016/03/21 18:49:18]

    // Build font subsetting info before calling addObjectRecursively()

+    perform_font_subsetting(fPageDevices, &fObjectSerializer.fSubstituteMap);
 
-        SkISize pageSize = SkISize::Make(
-                SkScalarRoundToInt(width), SkScalarRoundToInt(height));
-        sk_sp<SkPDFDevice> device(
-                SkPDFDevice::Create(pageSize, fRasterDpi, &fCanon));
-        fCanvas.reset(new SkCanvas(device.get()));
-        fPageDevices.push_back(std::move(device));
-        fCanvas->clipRect(trimBox);
-        fCanvas->translate(trimBox.x(), trimBox.y());
-        return fCanvas.get();
-    }
+    fObjectSerializer.addObjectRecursively(docCatalog);
+    fObjectSerializer.serializeObjects(this->getStream());
+    fObjectSerializer.serializeFooter(this->getStream(), docCatalog, std::move(id));
+    // // The page tree has both child and parent pointers, so it creates a
+    // // reference cycle.  We must clear that cycle to properly reclaim memory.
+    // for (int i = 0; i < pageTree.count(); i++) {
+    //    pageTree[i]->dump();
+    // }

[tomhudson, 2016/03/21 15:55:11]

Nit: Delete before committing?

[hal.canary, 2016/03/21 18:49:18]

Done.

+    pageTree.unrefAll();  // TODO: necesary?
+    pages.unrefAll();
+    fPageDevices.reset();
+    fCanon.reset();
+    renew(&fObjectSerializer);
+    return true;
+}
 
-    void onEndPage() override {
-        SkASSERT(fCanvas.get());
-        fCanvas->flush();
-        fCanvas.reset(nullptr);
-    }
-
-    bool onClose(SkWStream* stream) override {
-        SkASSERT(!fCanvas.get());
-
-        bool success = emit_pdf_document(fPageDevices, fMetadata, stream);
-        fPageDevices.reset();
-        fCanon.reset();
-        return success;
-    }
-
-    void onAbort() override {
-        fPageDevices.reset();
-        fCanon.reset();
-    }
-
-    void setMetadata(const SkDocument::Attribute info[],
-                     int infoCount,
-                     const SkTime::DateTime* creationDate,
-                     const SkTime::DateTime* modifiedDate) override {
-        fMetadata.fInfo.reset(info, infoCount);
-        fMetadata.fCreation.reset(clone(creationDate));
-        fMetadata.fModified.reset(clone(modifiedDate));
-    }
-
-private:
-    SkPDFCanon fCanon;
-    SkTArray<sk_sp<const SkPDFDevice>> fPageDevices;
-    sk_sp<SkCanvas> fCanvas;
-    SkScalar fRasterDpi;
-    SkPDFMetadata fMetadata;
-};
-}  // namespace
 ///////////////////////////////////////////////////////////////////////////////
 
 sk_sp<SkDocument> SkPDFMakeDocument(SkWStream* stream,
                                     void (*proc)(SkWStream*, bool),
                                     SkScalar dpi,
                                     SkPixelSerializer* jpeg) {
     return stream ? sk_make_sp<SkPDFDocument>(stream, proc, dpi, jpeg) : nullptr;
 }
 
 SkDocument* SkDocument::CreatePDF(SkWStream* stream, SkScalar dpi) {
     return SkPDFMakeDocument(stream, nullptr, dpi, nullptr).release();
 }
 
 SkDocument* SkDocument::CreatePDF(SkWStream* stream,
                                   SkScalar dpi,
                                   SkPixelSerializer* jpegEncoder) {
     return SkPDFMakeDocument(stream, nullptr, dpi, jpegEncoder).release();
 }
 
 SkDocument* SkDocument::CreatePDF(const char path[], SkScalar dpi) {
     auto delete_wstream = [](SkWStream* stream, bool) { delete stream; };
     SkAutoTDelete<SkFILEWStream> stream(new SkFILEWStream(path));
     return stream->isValid()
         ? SkPDFMakeDocument(stream.detach(), delete_wstream, dpi, nullptr).release()
         : nullptr;
 }