[subset] Update hb_subset_input_override_name_table API

Override the name string for the NameRecord identified by name_id,
platform_id, encoding_id and language_ids specified by the user.
If a record with specified name_id does not exist, this API will create
a new NameRecord with provided info and insert it to the name table.

src/hb-ot-name-table.hh

@@ -144,7 +144,7 @@ struct NameRecord
 
   NameRecord* copy (hb_serialize_context_t *c, const void *base
 #ifdef HB_EXPERIMENTAL_API
-                    , const hb_hashmap_t<unsigned, hb_bytes_t> *name_table_overrides
+                    , const hb_hashmap_t<hb_ot_name_record_ids_t, hb_bytes_t> *name_table_overrides
 #endif
 		    ) const
   {
@@ -153,33 +153,49 @@ struct NameRecord
     auto *out = c->embed (this);
     if (unlikely (!out)) return_trace (nullptr);
 #ifdef HB_EXPERIMENTAL_API
-    if (name_table_overrides->has (nameID))
+    hb_ot_name_record_ids_t record_ids (platformID, encodingID, languageID, nameID);
-    {
+    hb_bytes_t* name_bytes;
-      hb_bytes_t name_bytes = name_table_overrides->get (nameID);
+
-      unsigned text_size = hb_ot_name_convert_utf<hb_utf8_t, hb_utf16_be_t> (name_bytes, nullptr, nullptr);
+    if (name_table_overrides->has (record_ids, &name_bytes)) {
+      hb_bytes_t encoded_bytes = *name_bytes;
+      char *name_str_utf16_be = nullptr;
 
-      text_size++; // needs to consider NULL terminator for use in hb_ot_name_convert_utf()
+      if (platformID != 1)
-      unsigned byte_len = text_size * hb_utf16_be_t::codepoint_t::static_size;
-      char *name_str_utf16_be = (char *) hb_calloc (byte_len, 1);
-      if (!name_str_utf16_be)
       {
-        c->revert (snap);
+        unsigned text_size = hb_ot_name_convert_utf<hb_utf8_t, hb_utf16_be_t> (*name_bytes, nullptr, nullptr);
-        return_trace (nullptr);
+  
+        text_size++; // needs to consider NULL terminator for use in hb_ot_name_convert_utf()
+        unsigned byte_len = text_size * hb_utf16_be_t::codepoint_t::static_size;
+        name_str_utf16_be = (char *) hb_calloc (byte_len, 1);
+        if (!name_str_utf16_be)
+        {
+          c->revert (snap);
+          return_trace (nullptr);
+        }
+        hb_ot_name_convert_utf<hb_utf8_t, hb_utf16_be_t> (*name_bytes, &text_size,
+                                                          (hb_utf16_be_t::codepoint_t *) name_str_utf16_be);
+  
+        unsigned encoded_byte_len = text_size * hb_utf16_be_t::codepoint_t::static_size;
+        if (!encoded_byte_len || !c->check_assign (out->length, encoded_byte_len, HB_SERIALIZE_ERROR_INT_OVERFLOW)) {
+          c->revert (snap);
+          hb_free (name_str_utf16_be);
+          return_trace (nullptr);
+        }
+  
+        encoded_bytes = hb_bytes_t (name_str_utf16_be, encoded_byte_len);
       }
-      hb_ot_name_convert_utf<hb_utf8_t, hb_utf16_be_t> (name_bytes, &text_size,
+      else
-                                                        (hb_utf16_be_t::codepoint_t *) name_str_utf16_be);
+      {
-
+        // mac platform, copy the UTF-8 string(all ascii characters) as is
-      unsigned encoded_byte_len = text_size * hb_utf16_be_t::codepoint_t::static_size;
+        if (!c->check_assign (out->length, encoded_bytes.length, HB_SERIALIZE_ERROR_INT_OVERFLOW)) {
-      if (!encoded_byte_len || !c->check_assign (out->length, encoded_byte_len, HB_SERIALIZE_ERROR_INT_OVERFLOW)) {
+          c->revert (snap);
-        c->revert (snap);
+          return_trace (nullptr);
-        hb_free (name_str_utf16_be);
+        }
-        return_trace (nullptr);
       }
 
-      hb_bytes_t utf16_be_bytes (name_str_utf16_be, encoded_byte_len);
       out->offset = 0;
       c->push ();
-      utf16_be_bytes.copy (c);
+      encoded_bytes.copy (c);
       c->add_link (out->offset, c->pop_pack (), hb_serialize_context_t::Tail, 0);
       hb_free (name_str_utf16_be);
     }
@@ -303,7 +319,8 @@ struct name
 		  Iterator it,
 		  const void *src_string_pool
 #ifdef HB_EXPERIMENTAL_API
-		  , const hb_hashmap_t<unsigned, hb_bytes_t> *name_table_overrides
+                  , const hb_vector_t<hb_ot_name_record_ids_t>& insert_name_records
+		  , const hb_hashmap_t<hb_ot_name_record_ids_t, hb_bytes_t> *name_table_overrides
 #endif
 		  )
   {
@@ -311,13 +328,19 @@ struct name
 
     if (unlikely (!c->extend_min ((*this))))  return_trace (false);
 
+    unsigned total_count = it.len ()
+#ifdef HB_EXPERIMENTAL_API
+        + insert_name_records.length
+#endif
+        ;
     this->format = 0;
-    this->count = it.len ();
+    if (!c->check_assign (this->count, total_count, HB_SERIALIZE_ERROR_INT_OVERFLOW))
+      return false;
 
-    NameRecord *name_records = (NameRecord *) hb_calloc (it.len (), NameRecord::static_size);
+    NameRecord *name_records = (NameRecord *) hb_calloc (total_count, NameRecord::static_size);
     if (unlikely (!name_records)) return_trace (false);
 
-    hb_array_t<NameRecord> records (name_records, it.len ());
+    hb_array_t<NameRecord> records (name_records, total_count);
 
     for (const NameRecord& record : it)
     {
@@ -325,6 +348,22 @@ struct name
       name_records++;
     }
 
+#ifdef HB_EXPERIMENTAL_API
+    for (unsigned i = 0; i < insert_name_records.length; i++)
+    {
+      const hb_ot_name_record_ids_t& ids = insert_name_records[i];
+      NameRecord record;
+      record.platformID = ids.platform_id;
+      record.encodingID = ids.encoding_id;
+      record.languageID = ids.language_id;
+      record.nameID = ids.name_id;
+      record.length = 0; // handled in NameRecord copy()
+      record.offset = 0;
+      memcpy (name_records, &record, NameRecord::static_size);
+      name_records++;
+    }
+#endif
+
     records.qsort ();
 
     c->copy_all (records,
@@ -350,6 +389,11 @@ struct name
     name *name_prime = c->serializer->start_embed<name> ();
     if (unlikely (!name_prime)) return_trace (false);
 
+#ifdef HB_EXPERIMENTAL_API
+    const hb_hashmap_t<hb_ot_name_record_ids_t, hb_bytes_t> *name_table_overrides =
+        c->plan->name_table_overrides;
+#endif
+    
     auto it =
     + nameRecordZ.as_array (count)
     | hb_filter (c->plan->name_ids, &NameRecord::nameID)
@@ -359,15 +403,48 @@ struct name
           (c->plan->flags & HB_SUBSET_FLAGS_NAME_LEGACY)
           || namerecord.isUnicode ();
     })
+#ifdef HB_EXPERIMENTAL_API
+    | hb_filter ([&] (const NameRecord& namerecord) {
+      if (name_table_overrides->is_empty ())
+        return true;
+      hb_ot_name_record_ids_t rec_ids (namerecord.platformID,
+                                       namerecord.encodingID,
+                                       namerecord.languageID,
+                                       namerecord.nameID);
+
+      hb_bytes_t *p;
+      if (name_table_overrides->has (rec_ids, &p) &&
+          (*p).length == 0)
+        return false;
+      return true;
+    })
+#endif
     ;
 
-    name_prime->serialize (c->serializer,
-			   it, std::addressof (this + stringOffset)
 #ifdef HB_EXPERIMENTAL_API
-			   , c->plan->name_table_overrides
+    hb_vector_t<hb_ot_name_record_ids_t> insert_name_records;
+    if (!name_table_overrides->is_empty ())
+    {
+      if (unlikely (!insert_name_records.alloc (name_table_overrides->get_population ())))
+        return_trace (false);
+      for (const auto& record_ids : name_table_overrides->keys ())
+      {
+        if (name_table_overrides->get (record_ids).length == 0)
+          continue;
+        if (has_name_record_with_ids (record_ids))
+          continue;
+        insert_name_records.push (record_ids);
+      }
+    }
 #endif
-			   );
+
-    return_trace (name_prime->count);
+    return (name_prime->serialize (c->serializer, it,
+                                   std::addressof (this + stringOffset)
+#ifdef HB_EXPERIMENTAL_API
+                                   , insert_name_records
+                                   , name_table_overrides
+#endif
+                                   ));
   }
 
   bool sanitize_records (hb_sanitize_context_t *c) const
@@ -477,6 +554,23 @@ struct name
     hb_vector_t<hb_ot_name_entry_t> names;
   };
 
+  private:
+  // sometimes NameRecords are not sorted in the font file, so use linear search
+  // here
+  bool has_name_record_with_ids (const hb_ot_name_record_ids_t& record_ids) const
+  {
+    for (const auto& record : nameRecordZ.as_array (count))
+    {
+      if (record.platformID == record_ids.platform_id &&
+          record.encodingID == record_ids.encoding_id &&
+          record.languageID == record_ids.language_id &&
+          record.nameID == record_ids.name_id)
+        return true;
+    }
+    return false;
+  }
+
+  public:
   /* We only implement format 0 for now. */
   HBUINT16	format;		/* Format selector (=0/1). */
   HBUINT16	count;		/* Number of name records. */

src/hb-subset-input.cc

@@ -26,7 +26,7 @@
 
 #include "hb-subset.hh"
 #include "hb-set.hh"
-
+#include "hb-utf.hh"
 /**
  * hb_subset_input_create_or_fail:
  *
@@ -50,7 +50,7 @@ hb_subset_input_create_or_fail (void)
 
   input->axes_location = hb_hashmap_create<hb_tag_t, float> ();
 #ifdef HB_EXPERIMENTAL_API
-  input->name_table_overrides = hb_hashmap_create<unsigned, hb_bytes_t> ();
+  input->name_table_overrides = hb_hashmap_create<hb_ot_name_record_ids_t, hb_bytes_t> ();
 #endif
 
   if (!input->axes_location ||
@@ -258,8 +258,8 @@ hb_subset_input_destroy (hb_subset_input_t *input)
 #ifdef HB_EXPERIMENTAL_API
   if (input->name_table_overrides)
   {
-    for (auto _ : input->name_table_overrides->values ())
+    for (auto _ : *input->name_table_overrides)
-      _.fini ();
+      _.second.fini ();
   }
   hb_hashmap_destroy (input->name_table_overrides);
 #endif
@@ -514,38 +514,67 @@ hb_subset_preprocess (hb_face_t *source)
  * hb_subset_input_override_name_table:
  * @input: a #hb_subset_input_t object.
  * @name_id: name_id of a nameRecord
+ * @platform_id: platform ID of a nameRecord
+ * @encoding_id: encoding ID of a nameRecord
+ * @language_id: language ID of a nameRecord
  * @name_str: pointer to name string new value or null to indicate should remove
  * @str_len: the size of @name_str, or -1 if it is `NULL`-terminated
  *
- * Override the name string of a nameRecord with specified name_id
+ * Override the name string of the NameRecord identified by name_id,
+ * platform_id, encoding_id and language_id. If a record with that name_id
+ * doesn't exist, create it and insert to the name table.
+ *
+ * Note: for mac platform, we only support name_str with all ascii characters,
+ * name_str with non-ascii characters will be ignored.
+ *
  * Since: EXPERIMENTAL
  **/
-HB_EXTERN void
+HB_EXTERN hb_bool_t
 hb_subset_input_override_name_table (hb_subset_input_t  *input,
                                      hb_ot_name_id_t     name_id,
+                                     unsigned            platform_id,
+                                     unsigned            encoding_id,
+                                     unsigned            language_id,
                                      const char         *name_str,
                                      int                 str_len /* -1 means nul-terminated */)
 {
   if (!name_str)
   {
-    hb_set_del (hb_subset_input_set(input, HB_SUBSET_SETS_NAME_ID), name_id);
+    str_len = 0;
-    return;
   }
-
+  else if (str_len == -1)
-  if (str_len == -1)
-    str_len = strlen (name_str);
-
-  if (!str_len)
   {
-    hb_set_del (hb_subset_input_set(input, HB_SUBSET_SETS_NAME_ID), name_id);
+      str_len = strlen (name_str);
-    return;
   }
 
-  char *override_name = (char *) hb_malloc (str_len);
+  hb_bytes_t name_bytes (nullptr, 0);
-  if (unlikely (!override_name)) return;
+  if (str_len)
-
+  {
-  hb_memcpy (override_name, name_str, str_len);
+    if (platform_id == 1)
-  input->name_table_overrides->set (name_id, hb_bytes_t (override_name, str_len));
+    {
+      const uint8_t *src = reinterpret_cast<const uint8_t*> (name_str);
+      const uint8_t *src_end = src + str_len;
+
+      hb_codepoint_t unicode;
+      const hb_codepoint_t replacement = HB_BUFFER_REPLACEMENT_CODEPOINT_DEFAULT;
+      while (src < src_end)
+      {
+        src = hb_utf8_t::next (src, src_end, &unicode, replacement);
+        if (unicode >= 0x0080u)
+        {
+          printf ("Non-ascii character detected, ignored...This API supports acsii characters only for mac platform\n");
+          return false;
+        }
+      }
+    }
+    char *override_name = (char *) hb_malloc (str_len);
+    if (unlikely (!override_name)) return false;
+
+    hb_memcpy (override_name, name_str, str_len);
+    name_bytes = hb_bytes_t (override_name, str_len);
+  }
+  input->name_table_overrides->set (hb_ot_name_record_ids_t (platform_id, encoding_id, language_id, name_id), name_bytes);
+  return true;
 }
 
 #endif

src/hb-subset-input.hh

@@ -36,6 +36,48 @@
 
 #include "hb-font.hh"
 
+struct hb_ot_name_record_ids_t
+{
+  hb_ot_name_record_ids_t () = default;
+  hb_ot_name_record_ids_t (unsigned platform_id_,
+                           unsigned encoding_id_,
+                           unsigned language_id_,
+                           unsigned name_id_)
+      :platform_id (platform_id_),
+      encoding_id (encoding_id_),
+      language_id (language_id_),
+      name_id (name_id_) {}
+
+  bool operator != (const hb_ot_name_record_ids_t o) const
+  { return !(*this == o); }
+
+  inline bool operator == (const hb_ot_name_record_ids_t& o) const
+  {
+    return platform_id == o.platform_id &&
+           encoding_id == o.encoding_id &&
+           language_id == o.language_id &&
+           name_id == o.name_id;
+  }
+
+  inline uint32_t hash () const
+  {
+    uint32_t current = 0;
+    current = current * 31 + hb_hash (platform_id);
+    current = current * 31 + hb_hash (encoding_id);
+    current = current * 31 + hb_hash (language_id);
+    current = current * 31 + hb_hash (name_id);
+    return current;
+  }
+
+  unsigned platform_id;
+  unsigned encoding_id;
+  unsigned language_id;
+  unsigned name_id;
+};
+
+typedef struct hb_ot_name_record_ids_t hb_ot_name_record_ids_t;
+
+
 HB_MARK_AS_FLAG_T (hb_subset_flags_t);
 
 struct hb_subset_input_t
@@ -66,7 +108,7 @@ struct hb_subset_input_t
 
   hb_hashmap_t<hb_tag_t, float> *axes_location;
 #ifdef HB_EXPERIMENTAL_API
-  hb_hashmap_t<unsigned, hb_bytes_t> *name_table_overrides;
+  hb_hashmap_t<hb_ot_name_record_ids_t, hb_bytes_t> *name_table_overrides;
 #endif
 
   inline unsigned num_sets () const

src/hb-subset-plan.cc

@@ -888,25 +888,19 @@ hb_subset_plan_create_or_fail (hb_face_t	 *face,
   plan->check_success (plan->hmtx_map = hb_hashmap_create<unsigned, hb_pair_t<unsigned, int>> ());
 
 #ifdef HB_EXPERIMENTAL_API
-  plan->check_success (plan->name_table_overrides = hb_hashmap_create<unsigned, hb_bytes_t> ());
+  plan->check_success (plan->name_table_overrides = hb_hashmap_create<hb_ot_name_record_ids_t, hb_bytes_t> ());
   if (plan->name_table_overrides && input->name_table_overrides)
   {
     for (auto _ : *input->name_table_overrides)
     {
-      unsigned name_id = _.first;
       hb_bytes_t name_bytes = _.second;
       unsigned len = name_bytes.length;
-
       char *name_str = (char *) hb_malloc (len);
       if (unlikely (!plan->check_success (name_str)))
-      {
-        for (auto bytes : plan->name_table_overrides->values ())
-          bytes.fini ();
         break;
-      }
 
       hb_memcpy (name_str, name_bytes.arrayZ, len);
-      plan->name_table_overrides->set (name_id, hb_bytes_t (name_str, len));
+      plan->name_table_overrides->set (_.first, hb_bytes_t (name_str, len));
     }
   }
 #endif

src/hb-subset-plan.hh

@@ -90,8 +90,8 @@ struct hb_subset_plan_t
 #ifdef HB_EXPERIMENTAL_API
     if (name_table_overrides)
     {
-      for (auto _ : name_table_overrides->values ())
+      for (auto _ : *name_table_overrides)
-        _.fini ();
+        _.second.fini ();
     }
     hb_hashmap_destroy (name_table_overrides);
 #endif
@@ -206,9 +206,9 @@ struct hb_subset_plan_t
   hb_hashmap_t<unsigned, hb_pair_t<unsigned, int>> *vmtx_map;
 
 #ifdef HB_EXPERIMENTAL_API
-  // name table overrides map: name_id->name string new value or None
+  // name table overrides map: hb_ot_name_record_ids_t-> name string new value or
-  // to indicate should remove
+  // None to indicate should remove
-  hb_hashmap_t<unsigned, hb_bytes_t> *name_table_overrides;
+  hb_hashmap_t<hb_ot_name_record_ids_t, hb_bytes_t> *name_table_overrides;
 #endif
 
   const hb_subset_accelerator_t* accelerator;

src/hb-subset.h

@@ -185,9 +185,12 @@ hb_subset_input_pin_axis_location (hb_subset_input_t  *input,
 HB_EXTERN hb_face_t *
 hb_subset_preprocess (hb_face_t *source);
 
-HB_EXTERN void
+HB_EXTERN hb_bool_t
 hb_subset_input_override_name_table (hb_subset_input_t  *input,
 				     hb_ot_name_id_t     name_id,
+				     unsigned            platform_id,
+				     unsigned            encoding_id,
+				     unsigned            language_id,
 				     const char         *name_str,
 				     int                 str_len);
 

test/api/test-subset-nameids.c

@@ -75,18 +75,24 @@ test_subset_name_overrides (void)
   hb_face_t *face_expected = hb_test_open_font_file ("fonts/nameID.override.expected.ttf");
 
   char str1[] = "Roboto Test";
+  char str1_3[] = "Roboto Test unicode platform";
   char str2[] = "Bold";
   char str6[] = "Roboto-Bold";
+  char str12[] = "Non ascii test Ü";
+  char str16[] = "Roboto-test-inserting";
  
   hb_set_t *name_ids = hb_set_create();
   hb_face_t *face_subset;
   hb_set_add_range (name_ids, 0, 15);
 
   hb_subset_input_t *subset_input = hb_subset_test_create_input_from_nameids (name_ids);
-  hb_subset_input_override_name_table (subset_input, 1, str1, -1);
+  hb_subset_input_override_name_table (subset_input, 1, 1, 0, 0, str1, -1);
-  hb_subset_input_override_name_table (subset_input, 2, str2, 4);
+  hb_subset_input_override_name_table (subset_input, 1, 3, 1, 0x409, str1_3, -1);
-  hb_subset_input_override_name_table (subset_input, 6, str6, -1);
+  hb_subset_input_override_name_table (subset_input, 2, 1, 0, 0, str2, 4);
-  hb_subset_input_override_name_table (subset_input, 14, NULL, -1);
+  hb_subset_input_override_name_table (subset_input, 6, 1, 0, 0, str6, -1);
+  hb_subset_input_override_name_table (subset_input, 12, 1, 0, 0, str12, -1);
+  hb_subset_input_override_name_table (subset_input, 14, 1, 0, 0, NULL, -1);
+  hb_subset_input_override_name_table (subset_input, 16, 1, 0, 0, str16, -1);
 
   face_subset = hb_subset_test_create_subset (face_origin, subset_input);
   hb_set_destroy (name_ids);