From 4b1e17edc78c0313d5cb8a415f816f654cdfa417 Mon Sep 17 00:00:00 2001 From: halcanary Date: Wed, 27 Jul 2016 14:49:46 -0700 Subject: [PATCH] SkPdf: SkPDFFormXObject de-class-ified. We don't need an object, just a few standard fields on the base class; the change lets us get rid of a bunch of boilerplate code. I think this also reduces the cognitive load of the SkPDF internals. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2185803003 Review-Url: https://codereview.chromium.org/2185803003 --- src/pdf/SkPDFDevice.cpp | 41 ++++++++++---------- src/pdf/SkPDFDevice.h | 10 ++--- src/pdf/SkPDFFormXObject.cpp | 71 +++++++---------------------------- src/pdf/SkPDFFormXObject.h | 44 +++++----------------- src/pdf/SkPDFGraphicState.cpp | 2 +- src/pdf/SkPDFGraphicState.h | 3 +- src/pdf/SkPDFShader.cpp | 7 ++-- 7 files changed, 56 insertions(+), 122 deletions(-) diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp index 4181defc5d..6972f8b01f 100644 --- a/src/pdf/SkPDFDevice.cpp +++ b/src/pdf/SkPDFDevice.cpp @@ -627,7 +627,7 @@ private: SkPDFDevice* fDevice; SkPDFDevice::ContentEntry* fContentEntry; SkXfermode::Mode fXfermode; - SkPDFFormXObject* fDstFormXObject; + SkPDFObject* fDstFormXObject; SkPath fShape; void init(const SkClipStack* clipStack, const SkRegion& clipRegion, @@ -835,7 +835,7 @@ static sk_sp create_link_annotation(const SkRect& translatedRect) { } static sk_sp create_link_to_url(const SkData* urlData, const SkRect& r) { - auto annotation = create_link_annotation(r); + sk_sp annotation = create_link_annotation(r); SkString url(static_cast(urlData->data()), urlData->size() - 1); auto action = sk_make_sp("Action"); @@ -847,7 +847,7 @@ static sk_sp create_link_to_url(const SkData* urlData, const SkRect& static sk_sp create_link_named_dest(const SkData* nameData, const SkRect& r) { - auto annotation = create_link_annotation(r); + sk_sp annotation = create_link_annotation(r); SkString name(static_cast(nameData->data()), nameData->size() - 1); annotation->insertName("Dest", name); @@ -1394,7 +1394,7 @@ void SkPDFDevice::drawDevice(const SkDraw& d, SkBaseDevice* device, return; } - auto xObject = sk_make_sp(pdfDevice); + sk_sp xObject = pdfDevice->makeFormXObjectFromDevice(); SkPDFUtils::DrawFormXObject(this->addXObjectResource(xObject.get()), &content.entry()->fContent); } @@ -1610,18 +1610,20 @@ void SkPDFDevice::appendDestinations(SkPDFDict* dict, SkPDFObject* page) const { } } -SkPDFFormXObject* SkPDFDevice::createFormXObjectFromDevice() { - SkPDFFormXObject* xobject = new SkPDFFormXObject(this); +sk_sp SkPDFDevice::makeFormXObjectFromDevice() { + sk_sp xobject = + SkPDFMakeFormXObject(this->content(), this->copyMediaBox(), + this->makeResourceDict(), nullptr); // We always draw the form xobjects that we create back into the device, so // we simply preserve the font usage instead of pulling it out and merging // it back in later. - cleanUp(); // Reset this device to have no content. - init(); + this->cleanUp(); // Reset this device to have no content. + this->init(); return xobject; } void SkPDFDevice::drawFormXObjectWithMask(int xObjectIndex, - SkPDFFormXObject* mask, + SkPDFObject* mask, const SkClipStack* clipStack, const SkRegion& clipRegion, SkXfermode::Mode mode, @@ -1630,7 +1632,7 @@ void SkPDFDevice::drawFormXObjectWithMask(int xObjectIndex, return; } - auto sMaskGS = SkPDFGraphicState::GetSMaskGraphicState( + sk_sp sMaskGS = SkPDFGraphicState::GetSMaskGraphicState( mask, invertClip, SkPDFGraphicState::kAlpha_SMaskMode, fDocument->canon()); SkMatrix identity; @@ -1658,7 +1660,7 @@ SkPDFDevice::ContentEntry* SkPDFDevice::setUpContentEntry(const SkClipStack* cli const SkMatrix& matrix, const SkPaint& paint, bool hasText, - SkPDFFormXObject** dst) { + SkPDFObject** dst) { *dst = nullptr; if (clipRegion.isEmpty()) { return nullptr; @@ -1699,7 +1701,8 @@ SkPDFDevice::ContentEntry* SkPDFDevice::setUpContentEntry(const SkClipStack* cli xfermode == SkXfermode::kDstATop_Mode || xfermode == SkXfermode::kModulate_Mode) { if (!isContentEmpty()) { - *dst = createFormXObjectFromDevice(); + // TODO(halcanary): make this safer. + *dst = this->makeFormXObjectFromDevice().release(); SkASSERT(isContentEmpty()); } else if (xfermode != SkXfermode::kSrc_Mode && xfermode != SkXfermode::kSrcOut_Mode) { @@ -1730,7 +1733,7 @@ SkPDFDevice::ContentEntry* SkPDFDevice::setUpContentEntry(const SkClipStack* cli } void SkPDFDevice::finishContentEntry(SkXfermode::Mode xfermode, - SkPDFFormXObject* dst, + SkPDFObject* dst, SkPath* shape) { if (xfermode != SkXfermode::kClear_Mode && xfermode != SkXfermode::kSrc_Mode && @@ -1775,7 +1778,7 @@ void SkPDFDevice::finishContentEntry(SkXfermode::Mode xfermode, identity.reset(); SkPaint stockPaint; - sk_sp srcFormXObject; + sk_sp srcFormXObject; if (isContentEmpty()) { // If nothing was drawn and there's no shape, then the draw was a // no-op, but dst needs to be restored for that to be true. @@ -1795,7 +1798,7 @@ void SkPDFDevice::finishContentEntry(SkXfermode::Mode xfermode, } } else { SkASSERT(fContentEntries.count() == 1); - srcFormXObject.reset(createFormXObjectFromDevice()); + srcFormXObject = this->makeFormXObjectFromDevice(); } // TODO(vandebo) srcFormXObject may contain alpha, but here we want it @@ -1809,8 +1812,8 @@ void SkPDFDevice::finishContentEntry(SkXfermode::Mode xfermode, &fExistingClipStack, fExistingClipRegion, SkXfermode::kSrcOver_Mode, true); } else { - sk_sp dstMaskStorage; - SkPDFFormXObject* dstMask = srcFormXObject.get(); + sk_sp dstMaskStorage; + SkPDFObject* dstMask = srcFormXObject.get(); if (shape != nullptr) { // Draw shape into a form-xobject. SkRasterClip rc(clipRegion); @@ -1823,7 +1826,7 @@ void SkPDFDevice::finishContentEntry(SkXfermode::Mode xfermode, filledPaint.setStyle(SkPaint::kFill_Style); this->drawPath(d, *shape, filledPaint, nullptr, true); - dstMaskStorage.reset(createFormXObjectFromDevice()); + dstMaskStorage = this->makeFormXObjectFromDevice(); dstMask = dstMaskStorage.get(); } drawFormXObjectWithMask(addXObjectResource(dst), dstMask, @@ -2184,7 +2187,7 @@ void SkPDFDevice::internalDrawImage(const SkMatrix& origMatrix, SkBitmapKey key = imageBitmap.getKey(); sk_sp pdfimage = fDocument->canon()->findPDFBitmap(key); if (!pdfimage) { - auto img = imageBitmap.makeImage(); + sk_sp img = imageBitmap.makeImage(); if (!img) { return; } diff --git a/src/pdf/SkPDFDevice.h b/src/pdf/SkPDFDevice.h index 6a167d979a..d5d52f98e3 100644 --- a/src/pdf/SkPDFDevice.h +++ b/src/pdf/SkPDFDevice.h @@ -29,8 +29,8 @@ class SkPDFDevice; class SkPDFDocument; class SkPDFDict; class SkPDFFont; -class SkPDFFormXObject; class SkPDFObject; +class SkPDFStream; class SkRRect; /** \class SkPDFDevice @@ -268,10 +268,10 @@ private: void init(); void cleanUp(); - SkPDFFormXObject* createFormXObjectFromDevice(); + sk_sp makeFormXObjectFromDevice(); void drawFormXObjectWithMask(int xObjectIndex, - SkPDFFormXObject* mask, + SkPDFObject* mask, const SkClipStack* clipStack, const SkRegion& clipRegion, SkXfermode::Mode mode, @@ -286,9 +286,9 @@ private: const SkMatrix& matrix, const SkPaint& paint, bool hasText, - SkPDFFormXObject** dst); + SkPDFObject** dst); void finishContentEntry(SkXfermode::Mode xfermode, - SkPDFFormXObject* dst, + SkPDFObject* dst, SkPath* shape); bool isContentEmpty(); diff --git a/src/pdf/SkPDFFormXObject.cpp b/src/pdf/SkPDFFormXObject.cpp index ba49df8ea8..1ab391c346 100644 --- a/src/pdf/SkPDFFormXObject.cpp +++ b/src/pdf/SkPDFFormXObject.cpp @@ -8,69 +8,26 @@ #include "SkPDFFormXObject.h" -#include "SkMatrix.h" -#include "SkPDFDevice.h" -#include "SkPDFUtils.h" -#include "SkStream.h" -#include "SkTypes.h" +sk_sp SkPDFMakeFormXObject(std::unique_ptr content, + sk_sp mediaBox, + sk_sp resourceDict, + const char* colorSpace) { + auto form = sk_make_sp(std::move(content)); + form->insertName("Type", "XObject"); + form->insertName("Subtype", "Form"); + form->insertObject("Resources", std::move(resourceDict)); + form->insertObject("BBox", std::move(mediaBox)); -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). - auto resourceDict = device->makeResourceDict(); - - this->setData(device->content()); - - sk_sp bboxArray(device->copyMediaBox()); - this->init(nullptr, resourceDict.get(), bboxArray.get()); - - // We invert the initial transform and apply that to the xobject so that - // it doesn't get applied twice. We can't just undo it because it's - // embedded in things like shaders and images. - if (!device->initialTransform().isIdentity()) { - SkMatrix inverse; - if (!device->initialTransform().invert(&inverse)) { - // The initial transform should be invertible. - SkASSERT(false); - inverse.reset(); - } - this->insertObject("Matrix", SkPDFUtils::MatrixToArray(inverse)); - } -} - -/** - * Creates a FormXObject from a content stream and associated resources. - */ -SkPDFFormXObject::SkPDFFormXObject(std::unique_ptr content, - SkRect bbox, - SkPDFDict* resourceDict) { - this->setData(std::move(content)); - sk_sp bboxArray(SkPDFUtils::RectToArray(bbox)); - this->init("DeviceRGB", resourceDict, bboxArray.get()); -} - -/** - * Common initialization code. - * Note that bbox is unreferenced here, so calling code does not need worry. - */ -void SkPDFFormXObject::init(const char* colorSpace, - SkPDFDict* resourceDict, SkPDFArray* bbox) { - this->insertName("Type", "XObject"); - this->insertName("Subtype", "Form"); - this->insertObject("Resources", sk_ref_sp(resourceDict)); - this->insertObject("BBox", sk_ref_sp(bbox)); - - // Right now SkPDFFormXObject is only used for saveLayer, which implies + // Right now FormXObject is only used for saveLayer, which implies // isolated blending. Do this conditionally if that changes. + // TODO(halcanary): Is this comment obsolete, since we use it for + // alpha masks? auto group = sk_make_sp("Group"); group->insertName("S", "Transparency"); - if (colorSpace != nullptr) { group->insertName("CS", colorSpace); } group->insertBool("I", true); // Isolated. - this->insertObject("Group", std::move(group)); + form->insertObject("Group", std::move(group)); + return form; } - -SkPDFFormXObject::~SkPDFFormXObject() {} diff --git a/src/pdf/SkPDFFormXObject.h b/src/pdf/SkPDFFormXObject.h index a483a60466..6ce8b87bc9 100644 --- a/src/pdf/SkPDFFormXObject.h +++ b/src/pdf/SkPDFFormXObject.h @@ -10,41 +10,15 @@ #define SkPDFFormXObject_DEFINED #include "SkPDFStream.h" +#include "SkPDFDevice.h" -class SkPDFArray; -class SkPDFDevice; -class SkPDFDict; -struct SkRect; - -/** \class SkPDFFormXObject - - A form XObject; a self contained description of graphics objects. A form - XObject is basically a page object with slightly different syntax, that - can be drawn onto a page. +/** A form XObject is a self contained description of a graphics + object. A form XObject is a page object with slightly different + syntax, that can be drawn into a page content stream, just like a + bitmap XObject can be drawn into a page content stream. */ - -// The caller could keep track of the form XObjects it creates and -// canonicalize them, but the Skia API doesn't provide enough context to -// automatically do it (trivially). -class SkPDFFormXObject final : public SkPDFStream { -public: - /** Create a PDF form XObject. Entries for the dictionary entries are - * automatically added. - * @param device The set of graphical elements on this form. - */ - explicit SkPDFFormXObject(SkPDFDevice* device); - /** - * Create a PDF form XObject from a raw content stream and associated - * resources. - */ - explicit SkPDFFormXObject(std::unique_ptr content, - SkRect bbox, - SkPDFDict* resourceDict); - virtual ~SkPDFFormXObject(); - -private: - void init(const char* colorSpace, - SkPDFDict* resourceDict, SkPDFArray* bbox); -}; - +sk_sp SkPDFMakeFormXObject(std::unique_ptr content, + sk_sp mediaBox, + sk_sp resourceDict, + const char* colorSpace); #endif diff --git a/src/pdf/SkPDFGraphicState.cpp b/src/pdf/SkPDFGraphicState.cpp index 17129bb79d..3edf7458da 100644 --- a/src/pdf/SkPDFGraphicState.cpp +++ b/src/pdf/SkPDFGraphicState.cpp @@ -144,7 +144,7 @@ sk_sp SkPDFGraphicState::MakeInvertFunction() { } sk_sp SkPDFGraphicState::GetSMaskGraphicState( - SkPDFFormXObject* sMask, + SkPDFObject* sMask, bool invert, SkPDFSMaskMode sMaskMode, SkPDFCanon* canon) { diff --git a/src/pdf/SkPDFGraphicState.h b/src/pdf/SkPDFGraphicState.h index 569f9da7e6..70b63d4260 100644 --- a/src/pdf/SkPDFGraphicState.h +++ b/src/pdf/SkPDFGraphicState.h @@ -14,7 +14,6 @@ class SkPaint; class SkPDFCanon; -class SkPDFFormXObject; /** \class SkPDFGraphicState SkPaint objects roughly correspond to graphic state dictionaries that can @@ -52,7 +51,7 @@ public: * * These are not de-duped. */ - static sk_sp GetSMaskGraphicState(SkPDFFormXObject* sMask, + static sk_sp GetSMaskGraphicState(SkPDFObject* sMask, bool invert, SkPDFSMaskMode sMaskMode, SkPDFCanon* canon); diff --git a/src/pdf/SkPDFShader.cpp b/src/pdf/SkPDFShader.cpp index 13b4479dec..51d7442d96 100644 --- a/src/pdf/SkPDFShader.cpp +++ b/src/pdf/SkPDFShader.cpp @@ -703,9 +703,10 @@ static sk_sp create_smask_graphic_state( auto resources = get_gradient_resource_dict(luminosityShader.get(), nullptr); - sk_sp alphaMask( - new SkPDFFormXObject(std::move(alphaStream), bbox, resources.get())); - + auto alphaMask = SkPDFMakeFormXObject(std::move(alphaStream), + SkPDFUtils::RectToArray(bbox), + std::move(resources), + "DeviceRGB"); return SkPDFGraphicState::GetSMaskGraphicState( alphaMask.get(), false, SkPDFGraphicState::kLuminosity_SMaskMode, doc->canon());