Rename copy indices -> child indices to match spec update proposal.

w3c/IFT#252

Author: garretrieger

MODULE.bazel

@@ -72,10 +72,10 @@ http_archive(
 # Fontations
 http_archive(
     name = "fontations",
-    urls = ["https://github.com/googlefonts/fontations/archive/e01046e776d4539daf9e1856f3d8c9ea02984b1d.zip"],
-    strip_prefix = "fontations-e01046e776d4539daf9e1856f3d8c9ea02984b1d",
+    urls = ["https://github.com/googlefonts/fontations/archive/9063931e9cb9c4e0b40a54c83314486dd36115f8.zip"],
+    strip_prefix = "fontations-9063931e9cb9c4e0b40a54c83314486dd36115f8",
     build_file = "//third_party:fontations.BUILD",
-    integrity = "sha256-S9whGuFK3+vqEdl0Fx4lNpEf7fZ8EwPJirxkGzS6wdY=",
+    integrity = "sha256-aRxRkVF9A6aUE8Fu3y8StkH4tCLnM1iRFha6GcKbgFo=",
 )
 
 
@@ -111,4 +111,4 @@ git_override(
     module_name = "hedron_compile_commands",
     remote = "https://github.com/hedronvision/bazel-compile-commands-extractor.git",
     commit = "f5fbd4cee671d8d908f37c83abaf70fba5928fc7",
-)
+)

fontations/Cargo.lock

@@ -794,16 +794,15 @@ Status Encoder::PopulateGlyphKeyedPatchMap(PatchMap& patch_map) const {
       }
 
       PatchMap::Coverage coverage;
-      coverage.copy_mode_append =
-          false;  // union the group members together (OR).
+      coverage.conjunctive = false;  // ... OR ...
 
       for (uint32_t patch_id : group) {
         auto entry_index = patch_id_to_entry_index.find(patch_id);
         if (entry_index == patch_id_to_entry_index.end()) {
           return absl::InternalError(StrCat("entry for patch_id = ", patch_id,
                                             " was not previously created."));
         }
-        coverage.copy_indices.insert(entry_index->second);
+        coverage.child_indices.insert(entry_index->second);
       }
 
       if (condition->required_groups.size() == 1 &&
@@ -831,15 +830,15 @@ Status Encoder::PopulateGlyphKeyedPatchMap(PatchMap& patch_map) const {
   for (auto condition = remaining_conditions.begin();
        condition != remaining_conditions.end(); condition++) {
     PatchMap::Coverage coverage;
-    coverage.copy_mode_append = true;  // append the groups (AND)
+    coverage.conjunctive = true;  // ... AND ...
 
     for (const auto& group : condition->required_groups) {
       if (group.size() == 1) {
-        coverage.copy_indices.insert(patch_id_to_entry_index[*group.begin()]);
+        coverage.child_indices.insert(patch_id_to_entry_index[*group.begin()]);
         continue;
       }
 
-      coverage.copy_indices.insert(patch_group_to_entry_index[group]);
+      coverage.child_indices.insert(patch_group_to_entry_index[group]);
     }
 
     // TODO(garretrieger): required_features implies f1 AND f2 ..., but

ift/encoder/encoder.cc

@@ -700,6 +700,10 @@ void ClearCompatIdFromFormat2(uint8_t* data) {
   }
 }
 
+// TODO(garretrieger): XXXXXXX test with activation conditions that ensures they
+// are expanded appropriately in the
+//                     initial font.
+
 TEST_F(EncoderTest, Encode_ComplicatedActivationConditions) {
   Encoder encoder;
   hb_face_t* face = font.reference_face();

ift/encoder/encoder_test.cc

@@ -592,6 +592,8 @@ TEST_F(IntegrationTest, MixedMode_OptionalFeatureTags) {
   ASSERT_FALSE(FontHelper::GlyfData(extended_face.get(), chunk6_gid)->empty());
 }
 
+// TODO XXXX test using complex composite activation conditions
+
 TEST_F(IntegrationTest, MixedMode_LocaLenChange) {
   Encoder encoder;
   auto sc = InitEncoderForMixedMode(encoder);

ift/integration_test.cc

@@ -30,7 +30,7 @@ using common::SparseBitSet;
 namespace ift::proto {
 
 constexpr uint8_t features_and_design_space_bit_mask = 1;
-constexpr uint8_t copy_indices_bit_mask = 1 << 1;
+constexpr uint8_t child_indices_bit_mask = 1 << 1;
 constexpr uint8_t index_delta_bit_mask = 1 << 2;
 constexpr uint8_t encoding_bit_mask = 1 << 3;
 constexpr uint8_t codepoint_bit_mask = 0b11 << 4;
@@ -246,7 +246,7 @@ Status EncodeEntry(const PatchMap::Entry& entry, uint32_t last_entry_index,
   bool has_codepoints = !coverage.codepoints.empty();
   bool has_features = !coverage.features.empty();
   bool has_design_space = !coverage.design_space.empty();
-  bool has_copy_indices = !coverage.copy_indices.empty();
+  bool has_child_indices = !coverage.child_indices.empty();
   bool has_features_or_design_space = has_features || has_design_space;
   int64_t delta =
       ((int64_t)entry.patch_index) - ((int64_t)last_entry_index + 1);
@@ -258,10 +258,10 @@ Status EncodeEntry(const PatchMap::Entry& entry, uint32_t last_entry_index,
   // format
   uint8_t format =
       (has_features_or_design_space ? features_and_design_space_bit_mask
-                                    : 0) |              // bit 0
-      (has_copy_indices ? copy_indices_bit_mask : 0) |  // bit 1
-      (has_delta ? index_delta_bit_mask : 0) |          // bit 2
-      (has_patch_encoding ? encoding_bit_mask : 0) |    // bit 3
+                                    : 0) |                // bit 0
+      (has_child_indices ? child_indices_bit_mask : 0) |  // bit 1
+      (has_delta ? index_delta_bit_mask : 0) |            // bit 2
+      (has_patch_encoding ? encoding_bit_mask : 0) |      // bit 3
       (has_codepoints ? codepoint_bit_mask & BiasFormat(bias_bytes)
                       : 0) |                  // bit 4 and 5
       (entry.ignored ? ignore_bit_mask : 0);  // bit 6
@@ -285,20 +285,20 @@ Status EncodeEntry(const PatchMap::Entry& entry, uint32_t last_entry_index,
     }
   }
 
-  if (has_copy_indices) {
-    if (entry.coverage.copy_indices.size() >
+  if (has_child_indices) {
+    if (entry.coverage.child_indices.size() >
         0b01111111) {  // 7 bits are used to store the count.
       return absl::InvalidArgumentError(
-          StrCat("Maximum number of copy indices exceeded: ",
-                 entry.coverage.copy_indices.size(), " > 127."));
+          StrCat("Maximum number of child indices exceeded: ",
+                 entry.coverage.child_indices.size(), " > 127."));
     }
-    uint8_t count = (uint8_t)entry.coverage.copy_indices.size();
-    if (entry.coverage.copy_mode_append) {
+    uint8_t count = (uint8_t)entry.coverage.child_indices.size();
+    if (entry.coverage.conjunctive) {
       // MSB is used to record the append mode bit.
       count |= 0b10000000;
     }
     FontHelper::WriteUInt8(count, out);
-    for (uint32_t index : entry.coverage.copy_indices) {
+    for (uint32_t index : entry.coverage.child_indices) {
       WRITE_UINT24(index, out, "Exceeded max copy index size.");
     }
   }

ift/proto/format_2_patch_map.cc

@@ -90,14 +90,14 @@ TEST_F(Format2PatchMapTest, CopyIndices) {
   ASSERT_TRUE(sc.ok()) << sc;
 
   PatchMap::Coverage union_cov;
-  union_cov.copy_indices.insert(1);
-  union_cov.copy_indices.insert(0);
+  union_cov.child_indices.insert(1);
+  union_cov.child_indices.insert(0);
   sc = map.AddEntry(union_cov, 3, TABLE_KEYED_FULL);
   ASSERT_TRUE(sc.ok()) << sc;
 
   PatchMap::Coverage append_cov;
-  append_cov.copy_indices.insert(2);
-  append_cov.copy_mode_append = true;
+  append_cov.child_indices.insert(2);
+  append_cov.conjunctive = true;
   sc = map.AddEntry(append_cov, 4, TABLE_KEYED_FULL);
   ASSERT_TRUE(sc.ok()) << sc;
 

ift/proto/format_2_patch_map_test.cc

@@ -113,10 +113,10 @@ Span<const PatchMap::Entry> PatchMap::GetEntries() const { return entries_; }
 Status PatchMap::AddEntry(const PatchMap::Coverage& coverage,
                           uint32_t patch_index, PatchEncoding encoding,
                           bool ignored) {
-  // If copy indices are present ensure they refer only to entries prior to this
-  // one.
-  if (!coverage.copy_indices.empty()) {
-    for (uint32_t index : coverage.copy_indices) {
+  // If child indices are present ensure they refer only to entries prior to
+  // this one.
+  if (!coverage.child_indices.empty()) {
+    for (uint32_t index : coverage.child_indices) {
       if (index >= entries_.size()) {
         return absl::InvalidArgumentError(
             absl::StrCat("Invalid copy index. ", index, " is out of bounds."));

ift/proto/patch_map.cc

@@ -56,14 +56,12 @@ class PatchMap {
     absl::btree_set<hb_tag_t> features;
     absl::btree_map<hb_tag_t, common::AxisRange> design_space;
 
-    // If true copy mode
-    // (https://w3c.github.io/IFT/Overview.html#mapping-entry-copymodeandcount)
-    // is "append", other it's "union".
-    bool copy_mode_append = false;
-    // Set of copy indices
-    // (https://w3c.github.io/IFT/Overview.html#mapping-entry-copyindices)
+    // https://w3c.github.io/IFT/Overview.html#mapping-entry-childentrymatchmodeandcount)
+    bool conjunctive = false;
+    // Set of child entry indices
+    // (https://w3c.github.io/IFT/Overview.html#mapping-entry-childentrymatchmodeandcount)
     // values are the indices of previous entries.
-    absl::btree_set<uint32_t> copy_indices;
+    absl::btree_set<uint32_t> child_indices;
   };
 
   struct Entry {

ift/proto/patch_map.h

@@ -136,7 +136,7 @@ TEST_F(PatchMapTest, AddEntry_WithCopyIndices) {
   };
 
   PatchMap::Coverage cov;
-  cov.copy_indices = {0, 1};
+  cov.child_indices = {0, 1};
 
   auto sc = map.AddEntry(cov, 5, TABLE_KEYED_FULL);
   ASSERT_TRUE(sc.ok()) << sc;
@@ -151,7 +151,7 @@ TEST_F(PatchMapTest, AddEntry_InvalidCopyIndices) {
   };
 
   PatchMap::Coverage cov;
-  cov.copy_indices = {0, 2};
+  cov.child_indices = {0, 2};
 
   auto sc = map.AddEntry(cov, 5, TABLE_KEYED_FULL);
   ASSERT_EQ(sc, absl::InvalidArgumentError(