Fix CFF CID-keyed font subsetting: missing .notdef glyph and incorrect Charset encoding

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*')

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)

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

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

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

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