From f361b714390422a5c2a8b1dacb8e67502d0e40bb Mon Sep 17 00:00:00 2001 From: halcanary Date: Tue, 13 Jan 2015 07:12:57 -0800 Subject: [PATCH] In SkPDFDocument::emitPDF(), stop pre-calculating file offsets. * Add Streamer utility class which measures the current pdf offset by calling SkWStream::bytesWritten(). Calls SkPDFCatalog::setFileOffset() and SkPDFObject::emit() at the same time to guarantee that everything works out. * SkPDFCatalog::setFileOffset() no longer calculates the object's size. * SkPDFCatalog::setSubstituteResourcesOffsets() removed. * SkPDFCatalog::emitSubstituteResources() removed and getSubstituteList() made public in its place. * Remove SkPDFPage::getPageSize and SkPDFPage::emitPage. Replace with SkPDFPage::getContentStream(). * SkPDFObject::getOutputSize no longer virtual, only used in unit tests. All SkPDFObject subclasses getOutputSize() overrides removed. * SkPDFObject::getIndirectOutputSize removed. * PDFPrimitivesTest updated for new functions. Review URL: https://codereview.chromium.org/846023003 --- src/pdf/SkPDFCatalog.cpp | 21 +---------- src/pdf/SkPDFCatalog.h | 18 +++------- src/pdf/SkPDFDocument.cpp | 83 +++++++++++++++++++++++-------------------- src/pdf/SkPDFGraphicState.cpp | 6 ---- src/pdf/SkPDFGraphicState.h | 5 ++- src/pdf/SkPDFPage.cpp | 15 +++----- src/pdf/SkPDFPage.h | 17 ++------- src/pdf/SkPDFStream.cpp | 13 ------- src/pdf/SkPDFStream.h | 3 +- src/pdf/SkPDFTypes.cpp | 63 -------------------------------- src/pdf/SkPDFTypes.h | 11 ++---- tests/PDFPrimitivesTest.cpp | 14 +++++--- 12 files changed, 71 insertions(+), 198 deletions(-) diff --git a/src/pdf/SkPDFCatalog.cpp b/src/pdf/SkPDFCatalog.cpp index adb466c..5fd0f76 100644 --- a/src/pdf/SkPDFCatalog.cpp +++ b/src/pdf/SkPDFCatalog.cpp @@ -38,13 +38,11 @@ SkPDFObject* SkPDFCatalog::addObject(SkPDFObject* obj, bool onFirstPage) { return obj; } -size_t SkPDFCatalog::setFileOffset(SkPDFObject* obj, off_t offset) { +void SkPDFCatalog::setFileOffset(SkPDFObject* obj, off_t offset) { int objIndex = assignObjNum(obj) - 1; SkASSERT(fCatalog[objIndex].fObjNumAssigned); SkASSERT(fCatalog[objIndex].fFileOffset == 0); fCatalog[objIndex].fFileOffset = offset; - - return getSubstituteObject(obj)->getOutputSize(this, true); } void SkPDFCatalog::emitObjectNumber(SkWStream* stream, SkPDFObject* obj) { @@ -192,23 +190,6 @@ SkPDFObject* SkPDFCatalog::getSubstituteObject(SkPDFObject* object) { return object; } -off_t SkPDFCatalog::setSubstituteResourcesOffsets(off_t fileOffset, - bool firstPage) { - SkTSet* targetSet = getSubstituteList(firstPage); - off_t offsetSum = fileOffset; - for (int i = 0; i < targetSet->count(); ++i) { - offsetSum += SkToOffT(setFileOffset((*targetSet)[i], offsetSum)); - } - return offsetSum - fileOffset; -} - -void SkPDFCatalog::emitSubstituteResources(SkWStream *stream, bool firstPage) { - SkTSet* targetSet = getSubstituteList(firstPage); - for (int i = 0; i < targetSet->count(); ++i) { - (*targetSet)[i]->emit(stream, this, true); - } -} - SkTSet* SkPDFCatalog::getSubstituteList(bool firstPage) { return firstPage ? &fSubstituteResourcesFirstPage : &fSubstituteResourcesRemaining; diff --git a/src/pdf/SkPDFCatalog.h b/src/pdf/SkPDFCatalog.h index c7c6d6e..ecefd0a 100644 --- a/src/pdf/SkPDFCatalog.h +++ b/src/pdf/SkPDFCatalog.h @@ -37,12 +37,11 @@ public: SkPDFObject* addObject(SkPDFObject* obj, bool onFirstPage); /** Inform the catalog of the object's position in the final stream. - * The object should already have been added to the catalog. Returns - * the object's size. + * The object should already have been added to the catalog. * @param obj The object to add. * @param offset The byte offset in the output stream of this object. */ - size_t setFileOffset(SkPDFObject* obj, off_t offset); + void setFileOffset(SkPDFObject* obj, off_t offset); /** Output the object number for the passed object. * @param obj The object of interest. @@ -77,16 +76,9 @@ public: */ SkPDFObject* getSubstituteObject(SkPDFObject* object); - /** Set file offsets for the resources of substitute objects. - * @param fileOffset Accumulated offset of current document. - * @param firstPage Indicate whether this is for the first page only. - * @return Total size of resources of substitute objects. + /** get the resources of substitute objects. */ - off_t setSubstituteResourcesOffsets(off_t fileOffset, bool firstPage); - - /** Emit the resources of substitute objects. - */ - void emitSubstituteResources(SkWStream* stream, bool firstPage); + SkTSet* getSubstituteList(bool firstPage); private: struct Rec { @@ -130,8 +122,6 @@ private: int findObjectIndex(SkPDFObject* obj) const; int assignObjNum(SkPDFObject* obj); - - SkTSet* getSubstituteList(bool firstPage); }; #endif diff --git a/src/pdf/SkPDFDocument.cpp b/src/pdf/SkPDFDocument.cpp index 632cbcf..c4a5d44 100644 --- a/src/pdf/SkPDFDocument.cpp +++ b/src/pdf/SkPDFDocument.cpp @@ -82,6 +82,29 @@ SkPDFDocument::~SkPDFDocument() { SkDELETE(fOtherPageResources); } +namespace { +class Streamer { +public: + Streamer(SkPDFCatalog* cat, SkWStream* out) + : fCat(cat), fOut(out), fBaseOffset(SkToOffT(out->bytesWritten())) { + } + + void stream(SkPDFObject* obj) { + fCat->setFileOffset(obj, this->offset()); + obj->emit(fOut, fCat, true); + } + + off_t offset() { + return SkToOffT(fOut->bytesWritten()) - fBaseOffset; + } + +private: + SkPDFCatalog* const fCat; + SkWStream* const fOut; + const off_t fBaseOffset; +}; +} // namespace + bool SkPDFDocument::emitPDF(SkWStream* stream) { if (fPages.isEmpty()) { return false; @@ -158,46 +181,24 @@ bool SkPDFDocument::emitPDF(SkWStream* stream) { // Build font subsetting info before proceeding. perform_font_subsetting(fCatalog.get(), fPages, &fSubstitutes); + } - // Figure out the size of things and inform the catalog of file offsets. - off_t fileOffset = SkToOffT(this->headerSize()); - fileOffset += SkToOffT(fCatalog->setFileOffset(fDocCatalog, fileOffset)); - fileOffset += SkToOffT(fCatalog->setFileOffset(fPages[0], fileOffset)); - fileOffset += fPages[0]->getPageSize(fCatalog.get(), fileOffset); - for (int i = 0; i < fFirstPageResources->count(); i++) { - fileOffset += SkToOffT(fCatalog->setFileOffset((*fFirstPageResources)[i], fileOffset)); - } - // Add the size of resources of substitute objects used on page 1. - fileOffset += fCatalog->setSubstituteResourcesOffsets(fileOffset, true); - if (fPages.count() > 1) { - // TODO(vandebo): For linearized format, save the start of the - // first page xref table and calculate the size. - } - - for (int i = 0; i < fPageTree.count(); i++) { - fileOffset += SkToOffT(fCatalog->setFileOffset(fPageTree[i], fileOffset)); - } - - for (int i = 1; i < fPages.count(); i++) { - fileOffset += fPages[i]->getPageSize(fCatalog.get(), fileOffset); - } + Streamer out(fCatalog, stream); + emitHeader(stream); - for (int i = 0; i < fOtherPageResources->count(); i++) { - fileOffset += SkToOffT(fCatalog->setFileOffset((*fOtherPageResources)[i], fileOffset)); - } + out.stream(fDocCatalog); + out.stream(fPages[0]); + out.stream(fPages[0]->getContentStream()); - fileOffset += fCatalog->setSubstituteResourcesOffsets(fileOffset, false); - fXRefFileOffset = fileOffset; + for (int i = 0; i < fFirstPageResources->count(); i++) { + out.stream((*fFirstPageResources)[i]); } - emitHeader(stream); - fDocCatalog->emitIndirectObject(stream, fCatalog.get()); - fPages[0]->emitIndirectObject(stream, fCatalog.get()); - fPages[0]->emitPage(stream, fCatalog.get()); - for (int i = 0; i < fFirstPageResources->count(); i++) { - (*fFirstPageResources)[i]->emit(stream, fCatalog.get(), true); + SkTSet* firstPageSubstituteResources = + fCatalog->getSubstituteList(true); + for (int i = 0; i < firstPageSubstituteResources->count(); ++i) { + out.stream((*firstPageSubstituteResources)[i]); } - fCatalog->emitSubstituteResources(stream, true); // TODO(vandebo): Support linearized format // if (fPages.size() > 1) { // // TODO(vandebo): Save the file offset for the first page xref table. @@ -205,18 +206,24 @@ bool SkPDFDocument::emitPDF(SkWStream* stream) { // } for (int i = 0; i < fPageTree.count(); i++) { - fPageTree[i]->emitIndirectObject(stream, fCatalog.get()); + out.stream(fPageTree[i]); } for (int i = 1; i < fPages.count(); i++) { - fPages[i]->emitPage(stream, fCatalog.get()); + out.stream(fPages[i]->getContentStream()); } for (int i = 0; i < fOtherPageResources->count(); i++) { - (*fOtherPageResources)[i]->emit(stream, fCatalog.get(), true); + out.stream((*fOtherPageResources)[i]); + } + + SkTSet* otherSubstituteResources = + fCatalog->getSubstituteList(false); + for (int i = 0; i < otherSubstituteResources->count(); ++i) { + out.stream((*otherSubstituteResources)[i]); } - fCatalog->emitSubstituteResources(stream, false); + fXRefFileOffset = out.offset(); int64_t objCount = fCatalog->emitXrefTable(stream, fPages.count() > 1); emitFooter(stream, objCount); return true; diff --git a/src/pdf/SkPDFGraphicState.cpp b/src/pdf/SkPDFGraphicState.cpp index b330fb4..f4cb1b2 100644 --- a/src/pdf/SkPDFGraphicState.cpp +++ b/src/pdf/SkPDFGraphicState.cpp @@ -75,12 +75,6 @@ void SkPDFGraphicState::emitObject(SkWStream* stream, SkPDFCatalog* catalog) { } // static -size_t SkPDFGraphicState::getOutputSize(SkPDFCatalog* catalog, bool indirect) { - populateDict(); - return SkPDFDict::getOutputSize(catalog, indirect); -} - -// static SkTDArray& SkPDFGraphicState::CanonicalPaints() { CanonicalPaintsMutex().assertHeld(); static SkTDArray gCanonicalPaints; diff --git a/src/pdf/SkPDFGraphicState.h b/src/pdf/SkPDFGraphicState.h index 28686e3..246f1db 100644 --- a/src/pdf/SkPDFGraphicState.h +++ b/src/pdf/SkPDFGraphicState.h @@ -39,10 +39,9 @@ public: virtual void getResources(const SkTSet& knownResourceObjects, SkTSet* newResourceObjects); - // Override emitObject and getOutputSize so that we can populate - // the dictionary on demand. + // Override emitObject so that we can populate the dictionary on + // demand. virtual void emitObject(SkWStream* stream, SkPDFCatalog* catalog); - virtual size_t getOutputSize(SkPDFCatalog* catalog, bool indirect); /** Get the graphic state for the passed SkPaint. The reference count of * the object is incremented and it is the caller's responsibility to diff --git a/src/pdf/SkPDFPage.cpp b/src/pdf/SkPDFPage.cpp index 35fc460..cb2fe20 100644 --- a/src/pdf/SkPDFPage.cpp +++ b/src/pdf/SkPDFPage.cpp @@ -46,17 +46,6 @@ void SkPDFPage::finalizePage(SkPDFCatalog* catalog, bool firstPage, true); } -off_t SkPDFPage::getPageSize(SkPDFCatalog* catalog, off_t fileOffset) { - SkASSERT(fContentStream.get() != NULL); - catalog->setFileOffset(fContentStream.get(), fileOffset); - return SkToOffT(fContentStream->getOutputSize(catalog, true)); -} - -void SkPDFPage::emitPage(SkWStream* stream, SkPDFCatalog* catalog) { - SkASSERT(fContentStream.get() != NULL); - fContentStream->emitIndirectObject(stream, catalog); -} - // static void SkPDFPage::GeneratePageTree(const SkTDArray& pages, SkPDFCatalog* catalog, @@ -156,3 +145,7 @@ const SkPDFGlyphSetMap& SkPDFPage::getFontGlyphUsage() const { void SkPDFPage::appendDestinations(SkPDFDict* dict) { fDevice->appendDestinations(dict, this); } + +SkPDFObject* SkPDFPage::getContentStream() const { + return fContentStream.get(); +} diff --git a/src/pdf/SkPDFPage.h b/src/pdf/SkPDFPage.h index 47573c7..38cc5c4 100644 --- a/src/pdf/SkPDFPage.h +++ b/src/pdf/SkPDFPage.h @@ -56,21 +56,6 @@ public: */ void appendDestinations(SkPDFDict* dict); - /** Determine the size of the page content and store to the catalog - * the offsets of all nonresource-indirect objects that make up the page - * content. This must be called before emitPage(), but after finalizePage. - * @param catalog The catalog to add the object offsets to. - * @param fileOffset The file offset where the page content will be - * emitted. - */ - off_t getPageSize(SkPDFCatalog* catalog, off_t fileOffset); - - /** Output the page content to the passed stream. - * @param stream The writable output stream to send the content to. - * @param catalog The active object catalog. - */ - void emitPage(SkWStream* stream, SkPDFCatalog* catalog); - /** Generate a page tree for the passed vector of pages. New objects are * added to the catalog. The pageTree vector is populated with all of * the 'Pages' dictionaries as well as the 'Page' objects. Page trees @@ -97,6 +82,8 @@ public: */ const SkPDFGlyphSetMap& getFontGlyphUsage() const; + SkPDFObject* getContentStream() const; + private: // Multiple pages may reference the content. SkAutoTUnref fDevice; diff --git a/src/pdf/SkPDFStream.cpp b/src/pdf/SkPDFStream.cpp index 4b55226..837de7a 100644 --- a/src/pdf/SkPDFStream.cpp +++ b/src/pdf/SkPDFStream.cpp @@ -58,19 +58,6 @@ void SkPDFStream::emitObject(SkWStream* stream, SkPDFCatalog* catalog) { stream->writeText("\nendstream"); } -size_t SkPDFStream::getOutputSize(SkPDFCatalog* catalog, bool indirect) { - if (indirect) { - return getIndirectOutputSize(catalog); - } - SkAutoMutexAcquire lock(fMutex); // multiple threads could be calling emit - if (!this->populate(catalog)) { - return fSubstitute->getOutputSize(catalog, indirect); - } - - return this->INHERITED::getOutputSize(catalog, false) + - strlen(" stream\n\nendstream") + this->dataSize(); -} - SkPDFStream::SkPDFStream() : fState(kUnused_State) {} void SkPDFStream::setData(SkData* data) { diff --git a/src/pdf/SkPDFStream.h b/src/pdf/SkPDFStream.h index 7f33c16..586b2e8 100644 --- a/src/pdf/SkPDFStream.h +++ b/src/pdf/SkPDFStream.h @@ -39,10 +39,9 @@ public: virtual ~SkPDFStream(); - // The SkPDFObject interface. These two methods use a mutex to + // The SkPDFObject interface. This two method uses a mutex to // allow multiple threads to call at the same time. virtual void emitObject(SkWStream* stream, SkPDFCatalog* catalog) SK_OVERRIDE; - virtual size_t getOutputSize(SkPDFCatalog* catalog, bool indirect); protected: enum State { diff --git a/src/pdf/SkPDFTypes.cpp b/src/pdf/SkPDFTypes.cpp index 7562528..4a84876 100644 --- a/src/pdf/SkPDFTypes.cpp +++ b/src/pdf/SkPDFTypes.cpp @@ -45,11 +45,6 @@ void SkPDFObject::emitIndirectObject(SkWStream* stream, SkPDFCatalog* catalog) { stream->writeText("\nendobj\n"); } -size_t SkPDFObject::getIndirectOutputSize(SkPDFCatalog* catalog) { - return catalog->getObjectNumberSize(this) + strlen(" obj\n") + - this->getOutputSize(catalog, false) + strlen("\nendobj\n"); -} - void SkPDFObject::AddResourceHelper(SkPDFObject* resource, SkTDArray* list) { list->push(resource); @@ -86,11 +81,6 @@ void SkPDFObjRef::emitObject(SkWStream* stream, SkPDFCatalog* catalog) { stream->writeText(" R"); } -size_t SkPDFObjRef::getOutputSize(SkPDFCatalog* catalog, bool indirect) { - SkASSERT(!indirect); - return catalog->getObjectNumberSize(fObj.get()) + strlen(" R"); -} - SkPDFInt::SkPDFInt(int32_t value) : fValue(value) {} SkPDFInt::~SkPDFInt() {} @@ -109,14 +99,6 @@ void SkPDFBool::emitObject(SkWStream* stream, SkPDFCatalog* catalog) { } } -size_t SkPDFBool::getOutputSize(SkPDFCatalog* catalog, bool indirect) { - SkASSERT(!indirect); - if (fValue) { - return strlen("true"); - } - return strlen("false"); -} - SkPDFScalar::SkPDFScalar(SkScalar value) : fValue(value) {} SkPDFScalar::~SkPDFScalar() {} @@ -191,12 +173,6 @@ void SkPDFString::emitObject(SkWStream* stream, SkPDFCatalog* catalog) { stream->write(fValue.c_str(), fValue.size()); } -size_t SkPDFString::getOutputSize(SkPDFCatalog* catalog, bool indirect) { - if (indirect) - return getIndirectOutputSize(catalog); - return fValue.size(); -} - // static SkString SkPDFString::FormatString(const char* input, size_t len) { return DoFormatString(input, len, false, false); @@ -274,11 +250,6 @@ void SkPDFName::emitObject(SkWStream* stream, SkPDFCatalog* catalog) { stream->write(fValue.c_str(), fValue.size()); } -size_t SkPDFName::getOutputSize(SkPDFCatalog* catalog, bool indirect) { - SkASSERT(!indirect); - return fValue.size(); -} - // static SkString SkPDFName::FormatName(const SkString& input) { SkASSERT(input.size() <= kMaxLen); @@ -315,21 +286,6 @@ void SkPDFArray::emitObject(SkWStream* stream, SkPDFCatalog* catalog) { stream->writeText("]"); } -size_t SkPDFArray::getOutputSize(SkPDFCatalog* catalog, bool indirect) { - if (indirect) { - return getIndirectOutputSize(catalog); - } - - size_t result = strlen("[]"); - if (fValue.count()) { - result += fValue.count() - 1; - } - for (int i = 0; i < fValue.count(); i++) { - result += fValue[i]->getOutputSize(catalog, false); - } - return result; -} - void SkPDFArray::reserve(int length) { SkASSERT(length <= kMaxLen); fValue.setReserve(length); @@ -400,25 +356,6 @@ void SkPDFDict::emitObject(SkWStream* stream, SkPDFCatalog* catalog) { stream->writeText(">>"); } -size_t SkPDFDict::getOutputSize(SkPDFCatalog* catalog, bool indirect) { - if (indirect) { - return getIndirectOutputSize(catalog); - } - - SkAutoMutexAcquire lock(fMutex); // If another thread triggers a - // resize while this thread is in - // the for-loop, we can be left - // with a bad fValue[i] reference. - size_t result = strlen("<<>>") + (fValue.count() * 2); - for (int i = 0; i < fValue.count(); i++) { - SkASSERT(fValue[i].key); - SkASSERT(fValue[i].value); - result += fValue[i].key->getOutputSize(catalog, false); - result += fValue[i].value->getOutputSize(catalog, false); - } - return result; -} - SkPDFObject* SkPDFDict::append(SkPDFName* key, SkPDFObject* value) { SkASSERT(key); SkASSERT(value); diff --git a/src/pdf/SkPDFTypes.h b/src/pdf/SkPDFTypes.h index 75989c0..ce1ca86 100644 --- a/src/pdf/SkPDFTypes.h +++ b/src/pdf/SkPDFTypes.h @@ -31,12 +31,11 @@ public: SK_DECLARE_INST_COUNT(SkPDFObject) /** Return the size (number of bytes) of this object in the final output - * file. Compound objects or objects that are computationally intensive - * to output should override this method. + * file. Only used for testing. * @param catalog The object catalog to use. * @param indirect If true, output an object identifier with the object. */ - virtual size_t getOutputSize(SkPDFCatalog* catalog, bool indirect); + size_t getOutputSize(SkPDFCatalog* catalog, bool indirect); /** For non-primitive objects (i.e. objects defined outside this file), * this method will add to newResourceObjects any objects that this method @@ -116,7 +115,6 @@ public: // The SkPDFObject interface. virtual void emitObject(SkWStream* stream, SkPDFCatalog* catalog) SK_OVERRIDE; - virtual size_t getOutputSize(SkPDFCatalog* catalog, bool indirect); private: SkAutoTUnref fObj; @@ -163,7 +161,6 @@ public: // The SkPDFObject interface. virtual void emitObject(SkWStream* stream, SkPDFCatalog* catalog) SK_OVERRIDE; - virtual size_t getOutputSize(SkPDFCatalog* catalog, bool indirect); private: bool fValue; @@ -221,7 +218,6 @@ public: // The SkPDFObject interface. virtual void emitObject(SkWStream* stream, SkPDFCatalog* catalog) SK_OVERRIDE; - virtual size_t getOutputSize(SkPDFCatalog* catalog, bool indirect); static SkString FormatString(const char* input, size_t len); static SkString FormatString(const uint16_t* input, size_t len, @@ -256,7 +252,6 @@ public: // The SkPDFObject interface. virtual void emitObject(SkWStream* stream, SkPDFCatalog* catalog) SK_OVERRIDE; - virtual size_t getOutputSize(SkPDFCatalog* catalog, bool indirect); private: static const size_t kMaxLen = 127; @@ -283,7 +278,6 @@ public: // The SkPDFObject interface. virtual void emitObject(SkWStream* stream, SkPDFCatalog* catalog) SK_OVERRIDE; - virtual size_t getOutputSize(SkPDFCatalog* catalog, bool indirect); /** The size of the array. */ @@ -355,7 +349,6 @@ public: // The SkPDFObject interface. virtual void emitObject(SkWStream* stream, SkPDFCatalog* catalog) SK_OVERRIDE; - virtual size_t getOutputSize(SkPDFCatalog* catalog, bool indirect); /** The size of the dictionary. */ diff --git a/tests/PDFPrimitivesTest.cpp b/tests/PDFPrimitivesTest.cpp index 9421fc9..3a4c6f6 100644 --- a/tests/PDFPrimitivesTest.cpp +++ b/tests/PDFPrimitivesTest.cpp @@ -228,12 +228,18 @@ static void TestSubstitute(skiatest::Reporter* reporter) { SkDynamicMemoryWStream buffer; proxy->emit(&buffer, &catalog, false); - catalog.emitSubstituteResources(&buffer, false); + SkTSet* substituteResources = + catalog.getSubstituteList(false); + for (int i = 0; i < substituteResources->count(); ++i) { + (*substituteResources)[i]->emit(&buffer, &catalog, true); + } char objectResult[] = "2 0 obj\n<>\nendobj\n"; - REPORTER_ASSERT( - reporter, - catalog.setFileOffset(proxy.get(), 0) == strlen(objectResult)); + catalog.setFileOffset(proxy.get(), 0); + + size_t outputSize = catalog.getSubstituteObject(proxy.get()) + ->getOutputSize(&catalog, true); + REPORTER_ASSERT(reporter, outputSize == strlen(objectResult)); char expectedResult[] = "<>1 0 obj\n<>\nendobj\n"; -- 2.7.4