pdf: write only ToUnicode mappings needed by the font, trimming anything out of [firstChar, lastCha…

src/pdf/SkPDFFont.cpp

 /*
  * Copyright 2011 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include <ctype.h>
 
 #include "SkData.h"
@@ skipping 324 matching lines @@
                 appendAdvance(advanceInfo->fAdvance[0], emSize, result);
                 break;
             }
         }
     }
     return result;
 }
 
 }  // namespace
 
-static void append_tounicode_header(SkDynamicMemoryWStream* cmap) {
+static void append_tounicode_header(SkDynamicMemoryWStream* cmap,
+                                    uint16_t firstGlyphID,
+                                    uint16_t lastGlyphID) {
     // 12 dict begin: 12 is an Adobe-suggested value. Shall not change.
     // It's there to prevent old version Adobe Readers from malfunctioning.
     const char* kHeader =
         "/CIDInit /ProcSet findresource begin\n"
         "12 dict begin\n"
         "begincmap\n";
     cmap->writeText(kHeader);
 
     // The /CIDSystemInfo must be consistent to the one in
     // SkPDFFont::populateCIDFont().
     // We can not pass over the system info object here because the format is
     // different. This is not a reference object.
     const char* kSysInfo =
         "/CIDSystemInfo\n"
         "<<  /Registry (Adobe)\n"
         "/Ordering (UCS)\n"
         "/Supplement 0\n"
         ">> def\n";
     cmap->writeText(kSysInfo);
 
     // The CMapName must be consistent to /CIDSystemInfo above.
     // /CMapType 2 means ToUnicode.
-    // We specify codespacerange from 0x0000 to 0xFFFF because we convert our
-    // code table from unsigned short (16-bits). Codespace range just tells the
-    // PDF processor the valid range. It does not matter whether a complete
-    // mapping is provided or not.
-    const char* kTypeInfo =
+    // Codespace range just tells the PDF processor the valid range.
+    const char* kTypeInfoHeader =
         "/CMapName /Adobe-Identity-UCS def\n"
         "/CMapType 2 def\n"
-        "1 begincodespacerange\n"
-        "<0000> <FFFF>\n"
-        "endcodespacerange\n";
-    cmap->writeText(kTypeInfo);
+        "1 begincodespacerange\n";
+    cmap->writeText(kTypeInfoHeader);
+    
+    // e.g.     "<0000> <FFFF>\n"
+    SkString range;
+    range.appendf("<%04X> <%04X>\n", firstGlyphID, lastGlyphID);
+    cmap->writeText(range.c_str());
+    
+    const char* kTypeInfoFooter = "endcodespacerange\n";
+    cmap->writeText(kTypeInfoFooter);
 }
 
 static void append_cmap_footer(SkDynamicMemoryWStream* cmap) {
     const char* kFooter =
         "endcmap\n"
         "CMapName currentdict /CMap defineresource pop\n"
         "end\n"
         "end";
     cmap->writeText(kFooter);
 }
@@ skipping 73 matching lines @@
 // the spec does not mention how will this kind of conflict being resolved.
 //
 // For the worst case (having 65536 continuous unicode and we use every other
 // one of them), the possible savings by aggressive optimization is 416KB
 // pre-compressed and does not provide enough motivation for implementation.
 
 // FIXME: this should be in a header so that it is separately testable
 // ( see caller in tests/ToUnicode.cpp )
 void append_cmap_sections(const SkTDArray<SkUnichar>& glyphToUnicode,
                           const SkPDFGlyphSet* subset,
-                          SkDynamicMemoryWStream* cmap);
+                          SkDynamicMemoryWStream* cmap,
+                          uint16_t firstGlyphID,
+                          uint16_t lastGlyphID);
 
 void append_cmap_sections(const SkTDArray<SkUnichar>& glyphToUnicode,
                           const SkPDFGlyphSet* subset,
-                          SkDynamicMemoryWStream* cmap) {
+                          SkDynamicMemoryWStream* cmap,
+                          uint16_t firstGlyphID,
+                          uint16_t lastGlyphID) {
     if (glyphToUnicode.isEmpty()) {
         return;
     }
 
     SkTDArray<BFChar> bfcharEntries;
     SkTDArray<BFRange> bfrangeEntries;
 
     BFRange currentRangeEntry = {0, 0, 0};
     bool rangeEmpty = true;
     const int count = glyphToUnicode.count();
 
-    for (int i = 0; i < count + 1; ++i) {
+    for (int i = SkMax32(firstGlyphID - 1, 0);
+          i < SkMax32(count, lastGlyphID) + 1; ++i) {
         bool inSubset = i < count && (subset == NULL || subset->has(i));
         if (!rangeEmpty) {
             // PDF spec requires bfrange not changing the higher byte,
             // e.g. <1035> <10FF> <2222> is ok, but
             //      <1035> <1100> <2222> is no good
             bool inRange =
                 i == currentRangeEntry.fEnd + 1 &&
                 i >> 8 == currentRangeEntry.fStart >> 8 &&
                 i < count &&

[vandebo (ex-Chrome), 2013/09/09 16:04:50]

I think this line is meant to terminate the range on the last iteration of the
loop.  Probably easiest/best to set count to SkMax32(count, lastGlyphID) and
maybe rename it.

[edisonn, 2013/09/09 16:48:41]

in this case i < count is to protect on OutOfBounds the next line. Of I rename
and change cunt, code does not get simpler, and the loop continues to iterate
anyway, because we do not break.

[vandebo (ex-Chrome), 2013/09/09 17:13:11]

But why do you think the loop goes to count + 1?  The last unicode data is at
count.  The loop goes around once more to close the range.  Both inRange and
inSubset have checks for i < count, so they will both be false in that last
loop.  If a range is open, it will be closed, if not a new one will not open
because of the values of inRange and inSubset.

[edisonn, 2013/09/09 17:32:11]

if (i < count && foo[i]) woks

if (i < max(count, bar) && foo[i]) will assert if bar > count

count = max(count, bar)
if (i < count && foo[i]) asserts too.

                 glyphToUnicode[i] == currentRangeEntry.fUnicode + i -
                                          currentRangeEntry.fStart;
             if (!inSubset || !inRange) {
-                if (currentRangeEntry.fEnd > currentRangeEntry.fStart) {
-                    bfrangeEntries.push(currentRangeEntry);
-                } else {
+                BFRange range = {(uint16_t)SkMax32(firstGlyphID,

[vandebo (ex-Chrome), 2013/09/09 16:04:50]

With the loop changed, I don't think you need any of the changes here.

[edisonn, 2013/09/09 16:48:41]

I need it, I start from firstGlypthID - 1

[vandebo (ex-Chrome), 2013/09/09 17:13:11]

I don't see why you start at first - 1, please explain.

[edisonn, 2013/09/09 17:32:11]

Correct, it is not needed.

+                                                   currentRangeEntry.fStart),
+                                 (uint16_t)SkMin32(lastGlyphID,
+                                                   currentRangeEntry.fEnd),
+                                 currentRangeEntry.fUnicode};
+                if (range.fEnd > range.fStart) {
+                    bfrangeEntries.push(range);
+                } else if (range.fStart == range.fEnd &&
+                           firstGlyphID <= range.fStart &&
+                           range.fEnd <= lastGlyphID) {
                     BFChar* entry = bfcharEntries.append();
-                    entry->fGlyphId = currentRangeEntry.fStart;
-                    entry->fUnicode = currentRangeEntry.fUnicode;
+                    entry->fGlyphId = range.fStart;
+                    entry->fUnicode = range.fUnicode;
                 }
                 rangeEmpty = true;
+            } else if (i > lastGlyphID && !inSubset && !inRange) {

[vandebo (ex-Chrome), 2013/09/09 16:04:50]

or here

[edisonn, 2013/09/09 16:48:41]

This one i need, we go to max(lastGlyphID, count)

[vandebo (ex-Chrome), 2013/09/09 17:13:11]

Isn't this break redundant with the loop condition?

[edisonn, 2013/09/09 17:32:11]

no, the loop goes to max(last, count), while this if breaks if we exceeded last,
and there is no rand not subset

Without the if, the loop with continue to cycle doing nothing until max(count,
last)

+                break;
             }
         }
         if (inSubset) {
             currentRangeEntry.fEnd = i;
             if (rangeEmpty) {
               currentRangeEntry.fStart = i;
               currentRangeEntry.fUnicode = glyphToUnicode[i];
               rangeEmpty = false;
             }
         }
     }
 
     // The spec requires all bfchar entries for a font must come before bfrange
     // entries.
     append_bfchar_section(bfcharEntries, cmap);
     append_bfrange_section(bfrangeEntries, cmap);
 }
 
 static SkPDFStream* generate_tounicode_cmap(
         const SkTDArray<SkUnichar>& glyphToUnicode,
-        const SkPDFGlyphSet* subset) {
+        const SkPDFGlyphSet* subset,
+        uint16_t firstGlyphID,
+        uint16_t lastGlyphID) {
     SkDynamicMemoryWStream cmap;
-    append_tounicode_header(&cmap);
-    append_cmap_sections(glyphToUnicode, subset, &cmap);
+    append_tounicode_header(&cmap, firstGlyphID, lastGlyphID);
+    append_cmap_sections(glyphToUnicode, subset, &cmap,
+                         firstGlyphID, lastGlyphID);
     append_cmap_footer(&cmap);
     SkAutoTUnref<SkMemoryStream> cmapStream(new SkMemoryStream());
     cmapStream->setData(cmap.copyToData())->unref();
     return new SkPDFStream(cmapStream.get());
 }
 
 #if defined (SK_SFNTLY_SUBSETTER)
 static void sk_delete_array(const void* ptr, size_t, void*) {
     // Use C-style cast to cast away const and cast type simultaneously.
     delete[] (unsigned char*)ptr;
@@ skipping 465 matching lines @@
     : fFont(font),
       fFontID(fontID),
       fGlyphID(glyphID) {
 }
 
 void SkPDFFont::populateToUnicodeTable(const SkPDFGlyphSet* subset) {
     if (fFontInfo == NULL || fFontInfo->fGlyphToUnicode.begin() == NULL) {
         return;
     }
     SkAutoTUnref<SkPDFStream> pdfCmap(
-        generate_tounicode_cmap(fFontInfo->fGlyphToUnicode, subset));
+        generate_tounicode_cmap(fFontInfo->fGlyphToUnicode, subset,
+                                firstGlyphID(), lastGlyphID()));
     addResource(pdfCmap.get());
     insert("ToUnicode", new SkPDFObjRef(pdfCmap.get()))->unref();
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // class SkPDFType0Font
 ///////////////////////////////////////////////////////////////////////////////
 
 SkPDFType0Font::SkPDFType0Font(SkAdvancedTypefaceMetrics* info,
                                SkTypeface* typeface)
@@ skipping 380 matching lines @@
 
     insert("FontBBox", makeFontBBox(bbox, 1000))->unref();
     insertInt("FirstChar", firstGlyphID());
     insertInt("LastChar", lastGlyphID());
     insert("Widths", widthArray.get());
     insertName("CIDToGIDMap", "Identity");
 
     populateToUnicodeTable(NULL);
     return true;
 }