From c2f9ec1f5e4e8e98489cd5ea9356771cf4d8ce13 Mon Sep 17 00:00:00 2001 From: halcanary Date: Mon, 12 Sep 2016 08:55:29 -0700 Subject: [PATCH] SkPDF: refactor & code cleanup ahead of https://crrev.com/2322403002 SkPDFDevice::GraphicStateEntry: remove unnecessary fFont and fTextSize. SkPDFDevice::updateFont(): replace with update_font() and inlined code. De-duplicate this block of code. SkPDFResourceDict::GetResourceTypePrefix function made public: removes need for temporary SkString returned by SkPDFResourceDict::getResourceName() GlyphPositioner: delay writing intial matrix until first glyph. Assert that widechars is a constant. SkPDFFont::FontType(): make public so that PDFDevice can know about multibyte status. SkPDFFont::countStretch() removed, and the stretch loop flattened. *no changes in PDF output* BUG=skia:5434 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2327953002 Review-Url: https://codereview.chromium.org/2327953002 --- src/pdf/SkPDFDevice.cpp | 168 +++++++++++++--------------------- src/pdf/SkPDFDevice.h | 12 --- src/pdf/SkPDFFont.cpp | 4 +- src/pdf/SkPDFFont.h | 20 +--- src/pdf/SkPDFResourceDict.cpp | 6 +- src/pdf/SkPDFResourceDict.h | 2 + 6 files changed, 71 insertions(+), 141 deletions(-) diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp index 9635f54367..608d284bf2 100644 --- a/src/pdf/SkPDFDevice.cpp +++ b/src/pdf/SkPDFDevice.cpp @@ -85,9 +85,7 @@ SkPDFDevice::GraphicStateEntry::GraphicStateEntry() , fTextScaleX(SK_Scalar1) , fTextFill(SkPaint::kFill_Style) , fShaderIndex(-1) - , fGraphicStateIndex(-1) - , fFont(nullptr) - , fTextSize(SK_ScalarNaN) { + , fGraphicStateIndex(-1) { fMatrix.reset(); } @@ -861,20 +859,10 @@ public: bool defaultPositioning, SkPoint origin) : fContent(content) - , fCurrentMatrixOrigin{0.0f, 0.0f} - , fXAdvance(0.0f) + , fCurrentMatrixOrigin(origin) + , fTextSkewX(textSkewX) , fWideChars(wideChars) - , fInText(false) , fDefaultPositioning(defaultPositioning) { - // Flip the text about the x-axis to account for origin swap and include - // the passed parameters. - fContent->writeText("1 0 "); - SkPDFUtils::AppendScalar(0 - textSkewX, fContent); - fContent->writeText(" -1 "); - SkPDFUtils::AppendScalar(origin.x(), fContent); - fContent->writeText(" "); - SkPDFUtils::AppendScalar(origin.y(), fContent); - fContent->writeText(" Tm\n"); } ~GlyphPositioner() { this->flush(); } void flush() { @@ -883,16 +871,22 @@ public: fInText = false; } } - void setWideChars(bool wideChars) { - if (fWideChars != wideChars) { - SkASSERT(!fInText); - SkASSERT(fWideChars == wideChars); - fWideChars = wideChars; - } - } void writeGlyph(SkPoint xy, SkScalar advanceWidth, uint16_t glyph) { + if (!fInitialized) { + // Flip the text about the x-axis to account for origin swap and include + // the passed parameters. + fContent->writeText("1 0 "); + SkPDFUtils::AppendScalar(-fTextSkewX, fContent); + fContent->writeText(" -1 "); + SkPDFUtils::AppendScalar(fCurrentMatrixOrigin.x(), fContent); + fContent->writeText(" "); + SkPDFUtils::AppendScalar(fCurrentMatrixOrigin.y(), fContent); + fContent->writeText(" Tm\n"); + fCurrentMatrixOrigin.set(0.0f, 0.0f); + fInitialized = true; + } if (!fDefaultPositioning) { SkPoint position = xy - fCurrentMatrixOrigin; if (position != SkPoint{fXAdvance, 0}) { @@ -921,9 +915,11 @@ public: private: SkDynamicMemoryWStream* fContent; SkPoint fCurrentMatrixOrigin; - SkScalar fXAdvance; + SkScalar fXAdvance = 0.0f; + SkScalar fTextSkewX; bool fWideChars; - bool fInText; + bool fInText = false; + bool fInitialized = false; const bool fDefaultPositioning; }; } // namespace @@ -969,6 +965,16 @@ static void draw_transparent_text(SkPDFDevice* device, } } +static void update_font(SkWStream* wStream, int fontIndex, SkScalar textSize) { + wStream->writeText("/"); + char prefix = SkPDFResourceDict::GetResourceTypePrefix(SkPDFResourceDict::kFont_ResourceType); + wStream->write(&prefix, 1); + wStream->writeDecAsText(fontIndex); + wStream->writeText(" "); + SkPDFUtils::AppendScalar(textSize, wStream); + wStream->writeText(" Tf\n"); +} + void SkPDFDevice::internalDrawText( const SkDraw& d, const void* sourceText, size_t sourceByteCount, const SkScalar pos[], SkTextBlob::GlyphPositioning positioning, @@ -1067,74 +1073,50 @@ void SkPDFDevice::internalDrawText( SkDynamicMemoryWStream* out = &content.entry()->fContent; SkScalar textSize = paint.getTextSize(); - int index = 0; - while (glyphs[index] > maxGlyphID) { // Invalid glyphID for this font. - ++index; // Skip this glyphID - if (index == glyphCount) { - return; // all glyphIDs were bad. - } - } - out->writeText("BT\n"); SK_AT_SCOPE_EXIT(out->writeText("ET\n")); - SkPDFFont* font = this->updateFont( - typeface, textSize, glyphs[index], content.entry()); - SkASSERT(font); // All preconditions for SkPDFFont::GetFontResource are met. - if (!font) { return; } - + bool multiByteGlyphs = SkPDFFont::IsMultiByte(SkPDFFont::FontType(*metrics)); GlyphPositioner glyphPositioner(out, paint.getTextSkewX(), - font->multiByteGlyphs(), + multiByteGlyphs, defaultPositioning, offset); - - while (index < glyphCount) { - int stretch = font->countStretch(&glyphs[index], glyphCount - index, maxGlyphID); - SkASSERT(index + stretch <= glyphCount); - if (stretch < 1) { - // The current pdf font cannot encode the next glyph. - // Try to get a pdf font which can encode the next glyph. + SkPDFFont* font = nullptr; + for (int index = 0; index < glyphCount; ++index) { + SkGlyphID gid = glyphs[index]; + if (gid > maxGlyphID) { + continue; // Skip this invalid glyphID. + } + if (!font || !font->hasGlyph(gid)) { + // Either this is the first loop iteration or the current + // PDFFont cannot encode this glyph. glyphPositioner.flush(); - // first, validate the next glyph - while (glyphs[index] > maxGlyphID) { - ++index; // Skip this glyphID - if (index == glyphCount) { - return; // all remainng glyphIDs were bad. - } - } - SkASSERT(index < glyphCount); - font = this->updateFont(typeface, textSize, glyphs[index], content.entry()); - SkASSERT(font); // preconditions for SkPDFFont::GetFontResource met. + // Try to get a font which can encode the glyph. + int fontIndex = this->getFontResourceIndex(typeface, gid); + SkASSERT(fontIndex >= 0); + if (fontIndex < 0) { return; } + update_font(out, fontIndex, textSize); + font = fFontResources[fontIndex]; + SkASSERT(font); // All preconditions for SkPDFFont::GetFontResource are met. if (!font) { return; } - glyphPositioner.setWideChars(font->multiByteGlyphs()); - // Get stretch for this new font. - stretch = font->countStretch(&glyphs[index], glyphCount - index, maxGlyphID); - if (stretch < 1) { - SkDEBUGFAIL("PDF could not encode glyph."); - return; + SkASSERT(font->multiByteGlyphs() == multiByteGlyphs); + } + font->noteGlyphUsage(gid); + SkScalar advance{0.0f}; + SkPoint xy{0.0f, 0.0f}; + if (!defaultPositioning) { + advance = glyphCache->getGlyphIDAdvance(gid).fAdvanceX; + xy = SkTextBlob::kFull_Positioning == positioning + ? SkPoint{pos[2 * index], pos[2 * index + 1]} + : SkPoint{pos[index], 0}; + if (alignment != SkPaint::kLeft_Align) { + xy.offset(alignmentFactor * advance, 0); } } - while (stretch-- > 0) { - SkGlyphID gid = glyphs[index]; - if (gid <= maxGlyphID) { - font->noteGlyphUsage(gid); - SkGlyphID encodedGlyph = font->glyphToPDFFontEncoding(gid); - if (defaultPositioning) { - glyphPositioner.writeGlyph(SkPoint{0, 0}, 0, encodedGlyph); - } else { - SkScalar advance = glyphCache->getGlyphIDAdvance(gid).fAdvanceX; - SkPoint xy = SkTextBlob::kFull_Positioning == positioning - ? SkPoint{pos[2 * index], pos[2 * index + 1]} - : SkPoint{pos[index], 0}; - if (alignment != SkPaint::kLeft_Align) { - xy.offset(alignmentFactor * advance, 0); - } - glyphPositioner.writeGlyph(xy, advance, encodedGlyph); - } - } - ++index; - } + SkGlyphID encodedGlyph = + multiByteGlyphs ? gid : font->glyphToPDFFontEncoding(gid); + glyphPositioner.writeGlyph(xy, advance, encodedGlyph); } } @@ -1837,30 +1819,6 @@ int SkPDFDevice::addXObjectResource(SkPDFObject* xObject) { return result; } -SkPDFFont* SkPDFDevice::updateFont(SkTypeface* typeface, - SkScalar textSize, - uint16_t glyphID, - SkPDFDevice::ContentEntry* contentEntry) { - if (contentEntry->fState.fFont == nullptr || - contentEntry->fState.fTextSize != textSize || - !contentEntry->fState.fFont->hasGlyph(glyphID)) { - int fontIndex = getFontResourceIndex(typeface, glyphID); - if (fontIndex < 0) { - SkDebugf("SkPDF: Font error."); - return nullptr; - } - contentEntry->fContent.writeText("/"); - contentEntry->fContent.writeText(SkPDFResourceDict::getResourceName( - SkPDFResourceDict::kFont_ResourceType, - fontIndex).c_str()); - contentEntry->fContent.writeText(" "); - SkPDFUtils::AppendScalar(textSize, &contentEntry->fContent); - contentEntry->fContent.writeText(" Tf\n"); - contentEntry->fState.fFont = fFontResources[fontIndex]; - } - return contentEntry->fState.fFont; -} - int SkPDFDevice::getFontResourceIndex(SkTypeface* typeface, uint16_t glyphID) { sk_sp newFont( SkPDFFont::GetFontResource(fDocument->canon(), typeface, glyphID)); diff --git a/src/pdf/SkPDFDevice.h b/src/pdf/SkPDFDevice.h index 344fcb5e46..dba5d06818 100644 --- a/src/pdf/SkPDFDevice.h +++ b/src/pdf/SkPDFDevice.h @@ -170,13 +170,6 @@ public: SkPaint::Style fTextFill; // Only if TextScaleX is non-zero. int fShaderIndex; int fGraphicStateIndex; - - // We may change the font (i.e. for Type1 support) within a - // ContentEntry. This is the one currently in effect, or nullptr if none. - SkPDFFont* fFont; - // In PDF, text size has no default value. It is only valid if fFont is - // not nullptr. - SkScalar fTextSize; }; protected: @@ -279,11 +272,6 @@ private: int addGraphicStateResource(SkPDFObject* gs); int addXObjectResource(SkPDFObject* xObject); - // returns nullptr when a valid SkFont can not be produced - SkPDFFont* updateFont(SkTypeface* typeface, - SkScalar textSize, - uint16_t glyphID, - ContentEntry* contentEntry); int getFontResourceIndex(SkTypeface* typeface, uint16_t glyphID); diff --git a/src/pdf/SkPDFFont.cpp b/src/pdf/SkPDFFont.cpp index 6c37e4cde2..89c7951f63 100644 --- a/src/pdf/SkPDFFont.cpp +++ b/src/pdf/SkPDFFont.cpp @@ -161,7 +161,7 @@ const SkAdvancedTypefaceMetrics* SkPDFFont::GetMetrics(SkTypeface* typeface, return *canon->fTypefaceMetrics.set(id, metrics.release()); } -SkAdvancedTypefaceMetrics::FontType font_type(const SkAdvancedTypefaceMetrics& metrics) { +SkAdvancedTypefaceMetrics::FontType SkPDFFont::FontType(const SkAdvancedTypefaceMetrics& metrics) { if (SkToBool(metrics.fFlags & SkAdvancedTypefaceMetrics::kMultiMaster_FontFlag)) { // force Type3 fallback. return SkAdvancedTypefaceMetrics::kOther_Font; @@ -182,7 +182,7 @@ SkPDFFont* SkPDFFont::GetFontResource(SkPDFCanon* canon, SkASSERT(fontMetrics); // SkPDFDevice::internalDrawText ensures the typeface is good. // GetMetrics only returns null to signify a bad typeface. const SkAdvancedTypefaceMetrics& metrics = *fontMetrics; - SkAdvancedTypefaceMetrics::FontType type = font_type(metrics); + SkAdvancedTypefaceMetrics::FontType type = SkPDFFont::FontType(metrics); bool multibyte = SkPDFFont::IsMultiByte(type); SkGlyphID subsetCode = multibyte ? 0 : first_nonzero_glyph_for_single_byte_encoding(glyphID); uint64_t fontID = (SkTypeface::UniqueID(face) << 16) | subsetCode; diff --git a/src/pdf/SkPDFFont.h b/src/pdf/SkPDFFont.h index 9f33819ed8..a14ae63572 100644 --- a/src/pdf/SkPDFFont.h +++ b/src/pdf/SkPDFFont.h @@ -40,6 +40,8 @@ public: */ SkAdvancedTypefaceMetrics::FontType getType() const { return fFontType; } + static SkAdvancedTypefaceMetrics::FontType FontType(const SkAdvancedTypefaceMetrics&); + static bool IsMultiByte(SkAdvancedTypefaceMetrics::FontType type) { return type == SkAdvancedTypefaceMetrics::kType1CID_Font || type == SkAdvancedTypefaceMetrics::kTrueType_Font; @@ -65,24 +67,6 @@ public: return gid - fFirstGlyphID + 1; } - /** Count the number of glyphIDs that can be encoded with this font. - * glyphIDs > maxGlyphID are considered okay. */ - int countStretch(const SkGlyphID* glyphIDs, - int numGlyphs, - SkGlyphID maxGlyphID) const { - if (this->multiByteGlyphs()) { - return numGlyphs; - } - for (int i = 0; i < numGlyphs; i++) { - SkGlyphID gid = glyphIDs[i]; - if (gid != 0 && gid <= maxGlyphID && - (gid < fFirstGlyphID || gid > fLastGlyphID)) { - return i; - } - } - return numGlyphs; - } - void noteGlyphUsage(SkGlyphID glyph) { SkASSERT(this->hasGlyph(glyph)); fGlyphUsage.set(glyph); diff --git a/src/pdf/SkPDFResourceDict.cpp b/src/pdf/SkPDFResourceDict.cpp index b24f8a5877..67e81b6c7b 100644 --- a/src/pdf/SkPDFResourceDict.cpp +++ b/src/pdf/SkPDFResourceDict.cpp @@ -32,7 +32,7 @@ static const char* resource_type_names[] = { "Font" }; -static char get_resource_type_prefix( +char SkPDFResourceDict::GetResourceTypePrefix( SkPDFResourceDict::SkPDFResourceType type) { SkASSERT(type >= 0); SkASSERT(type < SkPDFResourceDict::kResourceTypeCount); @@ -50,9 +50,7 @@ static const char* get_resource_type_name( SkString SkPDFResourceDict::getResourceName( SkPDFResourceDict::SkPDFResourceType type, int key) { - SkString keyString; - keyString.printf("%c%d", get_resource_type_prefix(type), key); - return keyString; + return SkStringPrintf("%c%d", SkPDFResourceDict::GetResourceTypePrefix(type), key); } static void add_subdict( diff --git a/src/pdf/SkPDFResourceDict.h b/src/pdf/SkPDFResourceDict.h index 53932eea91..a9618b2423 100644 --- a/src/pdf/SkPDFResourceDict.h +++ b/src/pdf/SkPDFResourceDict.h @@ -32,6 +32,8 @@ public: kResourceTypeCount }; + static char GetResourceTypePrefix(SkPDFResourceDict::SkPDFResourceType type); + /** Create a PDF resource dictionary. * The full set of ProcSet entries is automatically created for backwards * compatibility, as recommended by the PDF spec.