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