Fix int16_t for glyphs, const glyphs, and clarify glyph loop.
Several places in the PDF code are using int16_t for glyphs. With
newer NotoSans fonts, all possible glyph ids are being used, so this
can lead to problems.
The PDF glyphs from text code returns the text for the glyphs if the
encoding is for glyphs. However, it returns this using an unsafe const
cast which is hiding possible bugs and preventing correct use of const
in other places.
The way the glyph loop in SkPDFDevice::drawPosText is written uses a
'--i' in the loop, which makes it appear this can loop forever. I don't
believe it can, but it is an unecessary code folding. We should also at
least assert the forward progress correctness here.
Review URL: https://codereview.chromium.org/626613002
Cherry-pick: 22edc8310cd57ab02155bfa6b2ddaf830556bcaf
Approval: https://code.google.com/p/chromium/issues/detail?id=418939#c30
diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp
index 5946b9a..6ea3914 100644
--- a/src/pdf/SkPDFDevice.cpp
+++ b/src/pdf/SkPDFDevice.cpp
@@ -118,7 +118,7 @@
static int force_glyph_encoding(const SkPaint& paint, const void* text,
size_t len, SkGlyphStorage* storage,
- uint16_t** glyphIDs) {
+ const uint16_t** glyphIDs) {
// Make sure we have a glyph id encoding.
if (paint.getTextEncoding() != SkPaint::kGlyphID_TextEncoding) {
int numGlyphs = paint.textToGlyphs(text, len, NULL);
@@ -131,8 +131,7 @@
// For user supplied glyph ids we need to validate them.
SkASSERT((len & 1) == 0);
int numGlyphs = SkToInt(len / 2);
- const uint16_t* input =
- reinterpret_cast<uint16_t*>(const_cast<void*>((text)));
+ const uint16_t* input = static_cast<const uint16_t*>(text);
int maxGlyphID = max_glyphid_for_typeface(paint.getTypeface());
int validated;
@@ -142,7 +141,7 @@
}
}
if (validated >= numGlyphs) {
- *glyphIDs = reinterpret_cast<uint16_t*>(const_cast<void*>((text)));
+ *glyphIDs = static_cast<const uint16_t*>(text);
return numGlyphs;
}
@@ -1115,7 +1114,7 @@
}
SkGlyphStorage storage(0);
- uint16_t* glyphIDs = NULL;
+ const uint16_t* glyphIDs = NULL;
int numGlyphs = force_glyph_encoding(paint, text, len, &storage, &glyphIDs);
textPaint.setTextEncoding(SkPaint::kGlyphID_TextEncoding);
@@ -1128,8 +1127,9 @@
while (numGlyphs > consumedGlyphCount) {
updateFont(textPaint, glyphIDs[consumedGlyphCount], content.entry());
SkPDFFont* font = content.entry()->fState.fFont;
+ //TODO: the const_cast here is a bug if the encoding started out as glyph encoding.
int availableGlyphs =
- font->glyphsToPDFFontEncoding(glyphIDs + consumedGlyphCount,
+ font->glyphsToPDFFontEncoding(const_cast<uint16_t*>(glyphIDs) + consumedGlyphCount,
numGlyphs - consumedGlyphCount);
fFontGlyphUsage->noteGlyphUsage(font, glyphIDs + consumedGlyphCount,
availableGlyphs);
@@ -1160,9 +1160,8 @@
}
SkGlyphStorage storage(0);
- uint16_t* glyphIDs = NULL;
- size_t numGlyphs = force_glyph_encoding(paint, text, len, &storage,
- &glyphIDs);
+ const uint16_t* glyphIDs = NULL;
+ size_t numGlyphs = force_glyph_encoding(paint, text, len, &storage, &glyphIDs);
textPaint.setTextEncoding(SkPaint::kGlyphID_TextEncoding);
SkDrawCacheProc glyphCacheProc = textPaint.getDrawCacheProc();
@@ -1172,19 +1171,23 @@
SkPDFFont* font = content.entry()->fState.fFont;
uint16_t encodedValue = glyphIDs[i];
if (font->glyphsToPDFFontEncoding(&encodedValue, 1) != 1) {
+ // The current pdf font cannot encode the current glyph.
+ // Try to get a pdf font which can encode the current glyph.
updateFont(textPaint, glyphIDs[i], content.entry());
- i--;
- continue;
+ font = content.entry()->fState.fFont;
+ if (font->glyphsToPDFFontEncoding(&encodedValue, 1) != 1) {
+ SkDEBUGFAIL("PDF could not encode glyph.");
+ continue;
+ }
}
+
fFontGlyphUsage->noteGlyphUsage(font, &encodedValue, 1);
SkScalar x = pos[i * scalarsPerPos];
SkScalar y = scalarsPerPos == 1 ? constY : pos[i * scalarsPerPos + 1];
align_text(glyphCacheProc, textPaint, glyphIDs + i, 1, &x, &y);
- set_text_transform(x, y, textPaint.getTextSkewX(),
- &content.entry()->fContent);
+ set_text_transform(x, y, textPaint.getTextSkewX(), &content.entry()->fContent);
SkString encodedString =
- SkPDFString::FormatString(&encodedValue, 1,
- font->multiByteGlyphs());
+ SkPDFString::FormatString(&encodedValue, 1, font->multiByteGlyphs());
content.entry()->fContent.writeText(encodedString.c_str());
content.entry()->fContent.writeText(" Tj\n");
}
diff --git a/src/pdf/SkPDFFont.cpp b/src/pdf/SkPDFFont.cpp
index 652b56d..9e1bca2 100644
--- a/src/pdf/SkPDFFont.cpp
+++ b/src/pdf/SkPDFFont.cpp
@@ -1028,7 +1028,7 @@
return true;
}
-void SkPDFFont::adjustGlyphRangeForSingleByteEncoding(int16_t glyphID) {
+void SkPDFFont::adjustGlyphRangeForSingleByteEncoding(uint16_t glyphID) {
// Single byte glyph encoding supports a max of 255 glyphs.
fFirstGlyphID = glyphID - (glyphID - 1) % 255;
if (fLastGlyphID > fFirstGlyphID + 255 - 1) {
@@ -1425,7 +1425,7 @@
SkPDFType3Font::~SkPDFType3Font() {}
-bool SkPDFType3Font::populate(int16_t glyphID) {
+bool SkPDFType3Font::populate(uint16_t glyphID) {
SkPaint paint;
paint.setTypeface(typeface());
paint.setTextSize(1000);
diff --git a/src/pdf/SkPDFFont.h b/src/pdf/SkPDFFont.h
index 48d9c30..27f1b5b 100644
--- a/src/pdf/SkPDFFont.h
+++ b/src/pdf/SkPDFFont.h
@@ -164,7 +164,7 @@
/** Set fFirstGlyphID and fLastGlyphID to span at most 255 glyphs,
* including the passed glyphID.
*/
- void adjustGlyphRangeForSingleByteEncoding(int16_t glyphID);
+ void adjustGlyphRangeForSingleByteEncoding(uint16_t glyphID);
// Generate ToUnicode table according to glyph usage subset.
// If subset is NULL, all available glyph ids will be used.
diff --git a/src/pdf/SkPDFFontImpl.h b/src/pdf/SkPDFFontImpl.h
index 99f1941..842e1c9 100644
--- a/src/pdf/SkPDFFontImpl.h
+++ b/src/pdf/SkPDFFontImpl.h
@@ -78,7 +78,7 @@
SkPDFType3Font(const SkAdvancedTypefaceMetrics* info,
SkTypeface* typeface, uint16_t glyphID);
- bool populate(int16_t glyphID);
+ bool populate(uint16_t glyphID);
};
#endif
