From 188838c20818307fda770ffc395a76ea63c1c8cc Mon Sep 17 00:00:00 2001 From: "vandebo@chromium.org" Date: Fri, 9 Mar 2012 22:16:58 +0000 Subject: [PATCH] [PDF] Fix memory hungry inefficiency in pdf resource tracking. When moving the content of a device into a PDF object like SkPDFFormXObject or SkPDFShader does, we only need the top level resources in the new object's resource list, not the recursive set of objects. Otherwise, when you put a form on a form on form, etc, references to the objects multiply. This fixed http://crbug.com/117321 Review URL: https://codereview.appspot.com/5796048 git-svn-id: http://skia.googlecode.com/svn/trunk@3360 2bbb7eff-a529-9590-31e7-b0007b416f81 --- include/pdf/SkPDFDevice.h | 6 +++++- include/pdf/SkPDFPage.h | 7 ++++--- include/pdf/SkPDFTypes.h | 10 +++++----- src/pdf/SkPDFDevice.cpp | 19 ++++++++++++++----- src/pdf/SkPDFFormXObject.cpp | 2 +- src/pdf/SkPDFPage.cpp | 2 +- src/pdf/SkPDFShader.cpp | 2 +- 7 files changed, 31 insertions(+), 17 deletions(-) diff --git a/include/pdf/SkPDFDevice.h b/include/pdf/SkPDFDevice.h index 9d6b54c..354729d 100644 --- a/include/pdf/SkPDFDevice.h +++ b/include/pdf/SkPDFDevice.h @@ -124,8 +124,12 @@ public: /** Get the list of resources (PDF objects) used on this page. * @param resourceList A list to append the resources to. + * @param recursive If recursive is true, get the resources of the + * device's resources recursively. (Useful for adding + * objects to the catalog.) */ - SK_API void getResources(SkTDArray* resourceList) const; + SK_API void getResources(SkTDArray* resourceList, + bool recursive) const; /** Get the fonts used on this device. */ diff --git a/include/pdf/SkPDFPage.h b/include/pdf/SkPDFPage.h index 38a371a..e726638 100644 --- a/include/pdf/SkPDFPage.h +++ b/include/pdf/SkPDFPage.h @@ -40,9 +40,10 @@ public: * that the page is part of. * @param catalog The catalog to add page content objects to. * @param firstPage Indicate if this is the first page of a document. - * @param resourceObjects The resource objects used on the page are added - * to this array. This gives the caller a chance - * to deduplicate resources across pages. + * @param resourceObjects All the resource objects (recursively) used on + * the page are added to this array. This gives + * the caller a chance to deduplicate resources + * across pages. */ void finalizePage(SkPDFCatalog* catalog, bool firstPage, SkTDArray* resourceObjects); diff --git a/include/pdf/SkPDFTypes.h b/include/pdf/SkPDFTypes.h index 155b826..6334938 100644 --- a/include/pdf/SkPDFTypes.h +++ b/include/pdf/SkPDFTypes.h @@ -40,11 +40,11 @@ public: */ virtual size_t getOutputSize(SkPDFCatalog* catalog, bool indirect); - /** If this object explicitly depends on other objects, add them to the - * end of the list. This only applies to higher level object, where - * the depenency is explicit and introduced by the class. i.e. an - * SkPDFImage added to an SkPDFDevice, but not an SkPDFObjRef added to - * an SkPDFArray. + /** For non-primitive objects (i.e. objects defined outside this file), + * this method will add to resourceList any objects that this method + * depends on. This operates recursively so if this object depends on + * another object and that object depends on two more, all three objects + * will be added. * @param resourceList The list to append dependant resources to. */ virtual void getResources(SkTDArray* resourceList); diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp index ba88956..e7b2433 100644 --- a/src/pdf/SkPDFDevice.cpp +++ b/src/pdf/SkPDFDevice.cpp @@ -1037,7 +1037,8 @@ SkPDFDict* SkPDFDevice::getResourceDict() { return fResourceDict.get(); } -void SkPDFDevice::getResources(SkTDArray* resourceList) const { +void SkPDFDevice::getResources(SkTDArray* resourceList, + bool recursive) const { resourceList->setReserve(resourceList->count() + fGraphicStateResources.count() + fXObjectResources.count() + @@ -1046,22 +1047,30 @@ void SkPDFDevice::getResources(SkTDArray* resourceList) const { for (int i = 0; i < fGraphicStateResources.count(); i++) { resourceList->push(fGraphicStateResources[i]); fGraphicStateResources[i]->ref(); - fGraphicStateResources[i]->getResources(resourceList); + if (recursive) { + fGraphicStateResources[i]->getResources(resourceList); + } } for (int i = 0; i < fXObjectResources.count(); i++) { resourceList->push(fXObjectResources[i]); fXObjectResources[i]->ref(); - fXObjectResources[i]->getResources(resourceList); + if (recursive) { + fXObjectResources[i]->getResources(resourceList); + } } for (int i = 0; i < fFontResources.count(); i++) { resourceList->push(fFontResources[i]); fFontResources[i]->ref(); - fFontResources[i]->getResources(resourceList); + if (recursive) { + fFontResources[i]->getResources(resourceList); + } } for (int i = 0; i < fShaderResources.count(); i++) { resourceList->push(fShaderResources[i]); fShaderResources[i]->ref(); - fShaderResources[i]->getResources(resourceList); + if (recursive) { + fShaderResources[i]->getResources(resourceList); + } } } diff --git a/src/pdf/SkPDFFormXObject.cpp b/src/pdf/SkPDFFormXObject.cpp index 5ac93e4..fb8a915 100644 --- a/src/pdf/SkPDFFormXObject.cpp +++ b/src/pdf/SkPDFFormXObject.cpp @@ -20,7 +20,7 @@ SkPDFFormXObject::SkPDFFormXObject(SkPDFDevice* device) { // We don't want to keep around device because we'd have two copies // of content, so reference or copy everything we need (content and // resources). - device->getResources(&fResources); + device->getResources(&fResources, false); SkRefPtr content = device->content(); content->unref(); // SkRefPtr and content() both took a reference. diff --git a/src/pdf/SkPDFPage.cpp b/src/pdf/SkPDFPage.cpp index 136ef44..3f3dec9 100644 --- a/src/pdf/SkPDFPage.cpp +++ b/src/pdf/SkPDFPage.cpp @@ -32,7 +32,7 @@ void SkPDFPage::finalizePage(SkPDFCatalog* catalog, bool firstPage, insert("Contents", new SkPDFObjRef(fContentStream.get()))->unref(); } catalog->addObject(fContentStream.get(), firstPage); - fDevice->getResources(resourceObjects); + fDevice->getResources(resourceObjects, true); } off_t SkPDFPage::getPageSize(SkPDFCatalog* catalog, off_t fileOffset) { diff --git a/src/pdf/SkPDFShader.cpp b/src/pdf/SkPDFShader.cpp index 72aeb71..15acf67 100644 --- a/src/pdf/SkPDFShader.cpp +++ b/src/pdf/SkPDFShader.cpp @@ -693,7 +693,7 @@ SkPDFImageShader::SkPDFImageShader(SkPDFShader::State* state) : fState(state) { // Put the canvas into the pattern stream (fContent). SkRefPtr content = pattern.content(); content->unref(); // SkRefPtr and content() both took a reference. - pattern.getResources(&fResources); + pattern.getResources(&fResources, false); setData(content.get()); insertName("Type", "Pattern"); -- 2.7.4