From 024855ee91ecf1e1ff821f54153e7579e582374a Mon Sep 17 00:00:00 2001 From: Kenta Ishizaki Date: Sun, 22 Mar 2026 15:28:01 +0900 Subject: [PATCH] Fix CFF CID-keyed font subsetting producing corrupt glyph data When subsetting CID-keyed CFF fonts (e.g., NotoSerifCJK.ttc), the encoded CFF data contains structural errors that cause certain glyphs to not render in PDF viewers. Three bugs are fixed: 1. CharstringsIndex#encode_items omits the .notdef charstring when the charmap has no mapping for GID 0. The CFF spec requires .notdef at index 0; its absence shifts all charstring indices by one. 2. FdSelector#encode similarly omits the .notdef entry, causing a mismatch between charstring indices and Font Dict assignments. Glyphs end up referencing the wrong Font Dict's local subroutines, producing corrupt outlines. 3. Charset#encode passes unsorted SIDs to BinUtils.rangify, which assumes sorted input. In CID-keyed fonts SIDs ordered by new GID are not necessarily ascending, so rangify merges unrelated SIDs into incorrect ranges. The fix falls back to array format when SIDs are not sorted. Tested with NotoSerifCJK.ttc (65,535 glyphs, 18 Font Dicts). Verified correct rendering and CFF structure with fonttools. Ref: prawnpdf/prawn#1105 --- lib/ttfunk/table/cff/charset.rb | 38 +++++++++++-------- lib/ttfunk/table/cff/charstrings_index.rb | 11 +++++- lib/ttfunk/table/cff/fd_selector.rb | 8 ++++ spec/ttfunk/table/cff/charset_spec.rb | 24 ++++++++++++ .../table/cff/charstrings_index_spec.rb | 27 +++++++++++++ spec/ttfunk/table/cff/fd_selector_spec.rb | 15 +++++++- 6 files changed, 106 insertions(+), 17 deletions(-) diff --git a/lib/ttfunk/table/cff/charset.rb b/lib/ttfunk/table/cff/charset.rb index c4a87ee..a508a5a 100644 --- a/lib/ttfunk/table/cff/charset.rb +++ b/lib/ttfunk/table/cff/charset.rb @@ -158,22 +158,30 @@ def encode(charmap) .sort_by { |mapping| mapping[:new] } .map { |mapping| sid_for(mapping[:old]) } - ranges = TTFunk::BinUtils.rangify(sids) - range_max = ranges.map(&:last).max - - range_bytes = - if range_max.positive? - (Math.log2(range_max) / 8).floor + 1 - else - # for cases when there are no sequences at all - Float::INFINITY - end + # BinUtils.rangify assumes a sorted sequence of values. In + # CID-keyed fonts, SIDs may not be in ascending order when + # sorted by new GID, which causes rangify to produce incorrect + # ranges. Fall back to array format if SIDs are not sorted. + sids_sorted = sids.each_cons(2).all? { |a, b| a <= b } + + if sids_sorted + ranges = TTFunk::BinUtils.rangify(sids) + range_max = ranges.map(&:last).max + + range_bytes = + if range_max.positive? + (Math.log2(range_max) / 8).floor + 1 + else + Float::INFINITY + end - # calculate whether storing the charset as a series of ranges is - # more efficient (i.e. takes up less space) vs storing it as an - # array of SID values - total_range_size = (2 * ranges.size) + (range_bytes * ranges.size) - total_array_size = sids.size * element_width(:array_format) + total_range_size = (2 * ranges.size) + (range_bytes * ranges.size) + total_array_size = sids.size * element_width(:array_format) + else + # Force array format when SIDs are not sorted + total_range_size = Float::INFINITY + total_array_size = 0 + end if total_array_size <= total_range_size ([format_int(:array_format)] + sids).pack('Cn*') diff --git a/lib/ttfunk/table/cff/charstrings_index.rb b/lib/ttfunk/table/cff/charstrings_index.rb index be8c3e4..754d86d 100644 --- a/lib/ttfunk/table/cff/charstrings_index.rb +++ b/lib/ttfunk/table/cff/charstrings_index.rb @@ -26,10 +26,19 @@ def decode_item(index, _offset, _length) end def encode_items(charmap) - charmap + new_items = charmap .reject { |code, mapping| mapping[:new].zero? && !code.zero? } .sort_by { |_code, mapping| mapping[:new] } .map { |(_code, mapping)| items[mapping[:old]] } + + # Ensure .notdef (GID 0) is included in the CharstringsIndex. + # The charmap may not contain a mapping for .notdef, but the + # CFF spec requires .notdef to always be present at index 0. + unless charmap.any? { |code, mapping| mapping[:new].zero? && code.zero? } + new_items.unshift(items[0]) + end + + new_items end def font_dict_for(index) diff --git a/lib/ttfunk/table/cff/fd_selector.rb b/lib/ttfunk/table/cff/fd_selector.rb index b2f01a8..3b17be0 100644 --- a/lib/ttfunk/table/cff/fd_selector.rb +++ b/lib/ttfunk/table/cff/fd_selector.rb @@ -99,6 +99,14 @@ def encode(charmap) .sort_by { |_code, mapping| mapping[:new] } .map { |(_code, mapping)| [mapping[:new], self[mapping[:old]]] } + # Ensure .notdef (GID 0) is included in the FD selector. + # The charmap may not contain a mapping for .notdef, but the + # CharstringsIndex always includes .notdef at index 0, so the + # FD selector must have a corresponding entry for it. + unless new_indices.any? { |gid, _| gid.zero? } + new_indices.unshift([0, self[0]]) + end + ranges = rangify_gids(new_indices) total_range_size = ranges.size * RANGE_ENTRY_SIZE total_array_size = new_indices.size * ARRAY_ENTRY_SIZE diff --git a/spec/ttfunk/table/cff/charset_spec.rb b/spec/ttfunk/table/cff/charset_spec.rb index 7879b94..2f2131e 100644 --- a/spec/ttfunk/table/cff/charset_spec.rb +++ b/spec/ttfunk/table/cff/charset_spec.rb @@ -162,5 +162,29 @@ expect(encoded.bytes).to eq([0x02, 0x00, 0x01, 0x03, 0xFF]) end end + + context 'when SIDs are not in ascending order' do + let(:charmap) do + # Simulate a CID-keyed font where GID order differs from SID order. + # SIDs in new GID order: [1, 10, 5, 8, 3] - not sorted. + { + 0x20 => { old: 1, new: 1 }, + 0x21 => { old: 10, new: 2 }, + 0x22 => { old: 5, new: 3 }, + 0x23 => { old: 8, new: 4 }, + 0x24 => { old: 3, new: 5 }, + } + end + + it 'falls back to array format' do + expect(encoded.bytes[0]).to eq(0) # array format + end + + it 'preserves the correct SID for each new GID' do + sids = encoded.bytes.drop(1).each_slice(2).map { |hi, lo| (hi << 8) | lo } + # SIDs should be in new GID order: [1, 10, 5, 8, 3] + expect(sids).to eq([1, 10, 5, 8, 3]) + end + end end end diff --git a/spec/ttfunk/table/cff/charstrings_index_spec.rb b/spec/ttfunk/table/cff/charstrings_index_spec.rb index 7dd35c8..45e2b62 100644 --- a/spec/ttfunk/table/cff/charstrings_index_spec.rb +++ b/spec/ttfunk/table/cff/charstrings_index_spec.rb @@ -174,4 +174,31 @@ ], ) end + + describe '#encode' do + let(:font_path) { test_font('NotoSansCJKsc-Thin', :otf) } + + it 'includes .notdef at index 0 when charmap lacks GID 0' do + charmap = { + 0x20 => { old: 1, new: 1 }, + 0x21 => { old: 2, new: 2 }, + } + + items = charstrings_index.__send__(:items) + encoded_items = charstrings_index.__send__(:encode_items, charmap) + + # Should have 3 items: .notdef + 2 from charmap + expect(encoded_items.length).to eq(3) + + # First item should be .notdef (items[0]) + expect(encoded_items[0]).to eq(items[0]) + end + + it 'does not duplicate .notdef when charmap includes GID 0' do + charmap = (0..5).to_h { |i| [i, { old: i, new: i }] } + encoded_items = charstrings_index.__send__(:encode_items, charmap) + + expect(encoded_items.length).to eq(6) + end + end end diff --git a/spec/ttfunk/table/cff/fd_selector_spec.rb b/spec/ttfunk/table/cff/fd_selector_spec.rb index 63b45fa..91b0a31 100644 --- a/spec/ttfunk/table/cff/fd_selector_spec.rb +++ b/spec/ttfunk/table/cff/fd_selector_spec.rb @@ -49,7 +49,20 @@ 0x22 => { old: 3, new: 3 }, 0x24 => { old: 5, new: 5 }, } - expect(fd_selector.encode(charmap)).to eq("\x00\x02\x04\x06") + expect(fd_selector.encode(charmap)).to eq("\x00\x01\x02\x04\x06") + end + + it 'includes .notdef FD entry at index 0 when charmap lacks GID 0' do + charmap = { + 0x20 => { old: 1, new: 1 }, + 0x22 => { old: 3, new: 3 }, + } + result = fd_selector.encode(charmap) + # Format byte (0x00 = array) followed by FD indices. + # GID 0 (.notdef) should map to FD index of original GID 0, + # which is entries[0] = 1 in the test fixture. + expect(result.bytes[0]).to eq(0) # array format + expect(result.bytes[1]).to eq(1) # .notdef FD index end end