added CFF1 desubr api test cases & bug fixes

src/hb-cff1-interp-cs.hh

@@ -111,16 +111,9 @@ struct CFF1CSOpSet : CSOpSet<Number, OPSET, CFF1CSInterpEnv, PARAM, PATH>
           break;
       }
       env.set_width (has_width);
-      if (has_width)
-      {
-        OPSET::process_width (env, param);
-      }
     }
   }
 
-  static inline void process_width (CFF1CSInterpEnv &env, PARAM& param)
-  {}
-
   static inline void flush_args (CFF1CSInterpEnv &env, PARAM& param)
   {
     SUPER::flush_args (env, param);

src/hb-ot-cff-common.hh

@@ -93,7 +93,12 @@ struct CFFIndex
   { return calculate_offset_array_size (offSize, count); }
 
   inline static unsigned int calculate_serialized_size (unsigned int offSize, unsigned int count, unsigned int dataSize)
-  { return min_size + calculate_offset_array_size (offSize, count) + dataSize; }
+  {
+    if (count == 0)
+      return COUNT::static_size;
+    else
+      return min_size + calculate_offset_array_size (offSize, count) + dataSize;
+  }
 
   inline bool serialize (hb_serialize_context_t *c, const CFFIndex &src)
   {
@@ -110,30 +115,39 @@ struct CFFIndex
                          const ByteStrArray &byteArray)
   {
     TRACE_SERIALIZE (this);
-    /* serialize CFFIndex header */
-    if (unlikely (!c->extend_min (*this))) return_trace (false);
-    this->count.set (byteArray.len);
-    this->offSize.set (offSize_);
-    if (!unlikely (c->allocate_size<HBUINT8> (offSize_ * (byteArray.len + 1))))
-      return_trace (false);
-  
-    /* serialize indices */
-    unsigned int  offset = 1;
-    unsigned int  i = 0;
-    for (; i < byteArray.len; i++)
+    if (byteArray.len == 0)
     {
-      set_offset_at (i, offset);
-      offset += byteArray[i].get_size ();
+      COUNT *dest = c->allocate_min<COUNT> ();
+      if (unlikely (dest == nullptr)) return_trace (false);
+      dest->set (0);
     }
-    set_offset_at (i, offset);
-
-    /* serialize data */
-    for (unsigned int i = 0; i < byteArray.len; i++)
+    else
     {
-      ByteStr  *dest = c->start_embed<ByteStr> ();
-      if (unlikely (dest == nullptr ||
-                    !dest->serialize (c, byteArray[i])))
+      /* serialize CFFIndex header */
+      if (unlikely (!c->extend_min (*this))) return_trace (false);
+      this->count.set (byteArray.len);
+      this->offSize.set (offSize_);
+      if (!unlikely (c->allocate_size<HBUINT8> (offSize_ * (byteArray.len + 1))))
         return_trace (false);
+    
+      /* serialize indices */
+      unsigned int  offset = 1;
+      unsigned int  i = 0;
+      for (; i < byteArray.len; i++)
+      {
+        set_offset_at (i, offset);
+        offset += byteArray[i].get_size ();
+      }
+      set_offset_at (i, offset);
+
+      /* serialize data */
+      for (unsigned int i = 0; i < byteArray.len; i++)
+      {
+        ByteStr  *dest = c->start_embed<ByteStr> ();
+        if (unlikely (dest == nullptr ||
+                      !dest->serialize (c, byteArray[i])))
+          return_trace (false);
+      }
     }
     return_trace (true);
   }

src/hb-subset-cff-common.hh

@@ -378,8 +378,10 @@ struct ParsedCSOp : OpStr
   inline void init (unsigned int subr_num_ = 0)
   {
     OpStr::init ();
-    flags = kDropFlag_None;
     subr_num = subr_num_;
+    drop_flag = false;
+    keep_flag = false;
+    skip_flag = false;
   }
 
   inline void fini (void)
@@ -387,20 +389,19 @@ struct ParsedCSOp : OpStr
     OpStr::fini ();
   }
 
-  inline bool for_keep (void) const { return (flags & kDropFlag_Keep) != 0; }
-  inline bool for_drop (void) const { return (flags & kDropFlag_Drop) != 0; }
-  inline void set_drop (void) { if (!for_keep ()) flags |= kDropFlag_Drop; }
-  inline void set_keep (void) { flags |= kDropFlag_Keep; }
+  inline bool for_drop (void) const { return drop_flag; }
+  inline void set_drop (void) { if (!for_keep ()) drop_flag = true; }
+  inline bool for_keep (void) const { return keep_flag; }
+  inline void set_keep (void) { keep_flag = true; }
+  inline bool for_skip (void) const { return skip_flag; }
+  inline void set_skip (void) { skip_flag = true; }
 
-  enum DropFlag
-  {
-    kDropFlag_None  = 0,
-    kDropFlag_Drop  = 1,
-    kDropFlag_Keep  = 2
-  };
-
-  unsigned int  flags;
   unsigned int  subr_num;
+
+  protected:
+  bool          drop_flag : 1;
+  bool          keep_flag : 1;
+  bool          skip_flag : 1;
 };
 
 struct ParsedCStr : ParsedValues<ParsedCSOp>
@@ -425,7 +426,7 @@ struct ParsedCStr : ParsedValues<ParsedCSOp>
     {
       unsigned int parsed_len = get_count ();
       if (likely (parsed_len > 0))
-        values[parsed_len-1].set_drop ();
+        values[parsed_len-1].set_skip ();
       
       ParsedCSOp val;
       val.init (subr_num);
@@ -629,8 +630,8 @@ struct SubrSubsetter
    * 4. re-encode all charstrings and subroutines with new subroutine numbers
    *
    * Phases #1 and #2 are done at the same time in collect_subrs ().
-   * Phase #3 requires walking charstrings/subroutines forward then backward (hence parsing), because
-   * we can't tell if a number belongs to a hint op until we see the first moveto.
+   * Phase #3 walks charstrings/subroutines forward then backward (hence parsing required),
+   * because we can't tell if a number belongs to a hint op until we see the first moveto.
    *
    * Assumption: a callsubr/callgsubr operator must immediately follow a (biased) subroutine number
    * within the same charstring/subroutine, e.g., not split across a charstring and a subroutine.
@@ -686,7 +687,8 @@ struct SubrSubsetter
                     drop_hints);
 
         bool seen_moveto = false;
-        if (drop_hints_in_str (parsed_charstrings[i], param, seen_moveto))
+        bool ends_in_hint = false;
+        if (drop_hints_in_str (parsed_charstrings[i], param, seen_moveto, ends_in_hint))
           parsed_charstrings[i].set_hint_removed ();
       }
 
@@ -755,39 +757,39 @@ struct SubrSubsetter
                                  ParsedCStrs &subrs, unsigned int subr_num,
                                  const SubrSubsetParam &param, bool &seen_moveto)
   {
-    if (drop_hints_in_str (subrs[subr_num], param, seen_moveto))
-    {
-      /* if the first op in the subr is a hint op, then all args/ops (especially including other subr calls)
-       * preceding this subr no and call op are hints */
-      /* TODO CFF2 vsindex */
-      for (unsigned int i = 0; i + 1 < pos; i++)
-        str.values[i].set_drop ();
-      return true;
-    }
-    else
-      return false;
+    bool  ends_in_hint = false;
+    bool has_hint = drop_hints_in_str (subrs[subr_num], param, seen_moveto, ends_in_hint);
+
+    /* if this subr ends with a stem hint (i.e., not a number a potential argument for moveto),
+     * then this entire subroutine must be a hint. drop its call. */
+    if (ends_in_hint)
+      str.values[pos].set_drop ();
+    
+    return has_hint;
   }
 
-  /* returns true if it sees a hint op before moveto */
-  inline bool drop_hints_in_str (ParsedCStr &str, const SubrSubsetParam &param, bool &seen_moveto)
+  /* returns true if it sees a hint op before the first moveto */
+  inline bool drop_hints_in_str (ParsedCStr &str, const SubrSubsetParam &param,
+                                 bool &seen_moveto, bool &ends_in_hint)
   {
     bool  seen_hint = false;
-    unsigned int next_check_pos = 0;
 
     for (unsigned int pos = 0; pos < str.values.len; pos++)
     {
+      bool  has_hint = false;
       switch (str.values[pos].op)
       {
         case OpCode_callsubr:
-          seen_hint |= drop_hints_in_subr (str, pos,
-                                           *param.parsed_local_subrs, str.values[pos].subr_num,
-                                           param, seen_moveto);
+          has_hint = drop_hints_in_subr (str, pos,
+                                        *param.parsed_local_subrs, str.values[pos].subr_num,
+                                        param, seen_moveto);
+                                        
           break;
 
         case OpCode_callgsubr:
-          seen_hint |= drop_hints_in_subr (str, pos,
-                                           *param.parsed_global_subrs, str.values[pos].subr_num,
-                                           param, seen_moveto);
+          has_hint = drop_hints_in_subr (str, pos,
+                                        *param.parsed_global_subrs, str.values[pos].subr_num,
+                                        param, seen_moveto);
           break;
 
         case OpCode_rmoveto:
@@ -809,19 +811,27 @@ struct SubrSubsetter
         case OpCode_vstemhm:
         case OpCode_hstem:
         case OpCode_vstem:
-          seen_hint = true;
-          for (unsigned int i = next_check_pos; i <= pos; i++)
-          {
-            /* TODO: CFF2 vsindex */
-            str.values[i].set_drop ();
-          }
-          next_check_pos = pos + 1;
+          has_hint = true;
+          str.values[pos].set_drop ();
+          if ((pos + 1 >= str.values.len) /* CFF2 */
+             || (str.values[pos + 1].op == OpCode_return))
+            ends_in_hint = true;
           break;
 
         default:
           /* NONE */
           break;
       }
+      if (has_hint)
+      {
+        for (int i = pos - 1; i >= 0; i--)
+        {
+          if (str.values[i].for_drop ())
+            break;
+          str.values[i].set_drop ();
+        }
+        seen_hint |= has_hint;
+      }
     }
 
     return seen_hint;
@@ -878,7 +888,7 @@ struct SubrSubsetter
     for (unsigned int i = 0; i < str.get_count(); i++)
     {
       const ParsedCSOp  &opstr = str.values[i];
-      if (!opstr.for_drop ())
+      if (!opstr.for_drop () && !opstr.for_skip ())
       {
         switch (opstr.op)
         {

src/hb-subset-cff1.cc

@@ -380,11 +380,6 @@ struct CFF1CSOpSet_SubrSubset : CFF1CSOpSet<CFF1CSOpSet_SubrSubset, SubrSubsetPa
     }
   }
 
-  static inline void process_width (CFF1CSInterpEnv &env, SubrSubsetParam& param)
-  {
-  
-  }
-
   protected:
   static inline void process_call_subr (OpCode op, CSType type,
                                         CFF1CSInterpEnv &env, SubrSubsetParam& param,
@@ -405,7 +400,7 @@ struct CFF1SubrSubsetter : SubrSubsetter<CFF1SubrSubsetter, CFF1Subrs, const OT:
 {
   static inline void set_parsed_prefix (const CFF1CSInterpEnv &env, ParsedCStr &charstring)
   {
-    if (env.processed_width)
+    if (env.has_width)
       charstring.set_prefix (env.width);
   }
 };
@@ -676,7 +671,7 @@ struct cff_subset_plan {
         return false;
       
       /* no global/local subroutines */
-      offsets.globalSubrsInfo.size = HBUINT16::static_size; /* count 0 only */
+      offsets.globalSubrsInfo.size = CFF1Subrs::calculate_serialized_size (1, 0, 0);
     }
     else
     {
@@ -704,14 +699,19 @@ struct cff_subset_plan {
       for (unsigned int fd = 0; fd < orig_fdcount; fd++)
       {
         subset_localsubrs[fd].init ();
+        offsets.localSubrsInfos[fd].init ();
         if (fdmap.includes (fd))
         {
           if (!subr_subsetter.encode_localsubrs (fd, subset_localsubrs[fd]))
             return false;
 
           unsigned int dataSize = subset_localsubrs[fd].total_size ();
-          offsets.localSubrsInfos[fd].offSize = calcOffSize (dataSize);
-          offsets.localSubrsInfos[fd].size = CFF1Subrs::calculate_serialized_size (offsets.localSubrsInfos[fd].offSize, subset_localsubrs[fd].len, dataSize);
+          if (dataSize > 0)
+          {
+            offsets.localSubrsInfos[fd].offset = final_size;
+            offsets.localSubrsInfos[fd].offSize = calcOffSize (dataSize);
+            offsets.localSubrsInfos[fd].size = CFF1Subrs::calculate_serialized_size (offsets.localSubrsInfos[fd].offSize, subset_localsubrs[fd].len, dataSize);
+          }
         }
       }
     }
@@ -779,8 +779,11 @@ struct cff_subset_plan {
         fontdicts_mod.push (fontdict_mod);
         final_size += privInfo.size;
 
-        if (!plan->desubroutinize)
+        if (!plan->desubroutinize && (offsets.localSubrsInfos[i].size > 0))
+        {
+          offsets.localSubrsInfos[i].offset = final_size;
           final_size += offsets.localSubrsInfos[i].size;
+        }
       }
     }
 
@@ -900,21 +903,12 @@ static inline bool _write_cff1 (const cff_subset_plan &plan,
     assert (plan.offsets.globalSubrsInfo.offset != 0);
     assert (plan.offsets.globalSubrsInfo.offset == c.head - c.start);
     
-    if (plan.desubroutinize)
+    CFF1Subrs *dest = c.start_embed <CFF1Subrs> ();
+    if (unlikely (dest == nullptr)) return false;
+    if (unlikely (!dest->serialize (&c, plan.offsets.globalSubrsInfo.offSize, plan.subset_globalsubrs)))
     {
-      CFF1Subrs *dest = c.allocate_size <CFF1Subrs> (HBUINT16::static_size);
-      if (unlikely (dest == nullptr)) return false;
-      dest->count.set (0);
-    }
-    else
-    {
-      CFF1Subrs *dest = c.start_embed <CFF1Subrs> ();
-      if (unlikely (dest == nullptr)) return false;
-      if (unlikely (!dest->serialize (&c, plan.offsets.globalSubrsInfo.offSize, plan.subset_globalsubrs)))
-      {
-        DEBUG_MSG (SUBSET, nullptr, "failed to serialize global subroutines");
-        return false;
-      }
+      DEBUG_MSG (SUBSET, nullptr, "failed to serialize global subroutines");
+      return false;
     }
   }
 
@@ -1010,7 +1004,7 @@ static inline bool _write_cff1 (const cff_subset_plan &plan,
   assert (plan.offsets.privateDictInfo.offset == c.head - c.start);
   for (unsigned int i = 0; i < acc.privateDicts.len; i++)
   {
-    if (!plan.fdmap.excludes (i))
+    if (plan.fdmap.includes (i))
     {
       PrivateDict  *pd = c.start_embed<PrivateDict> ();
       if (unlikely (pd == nullptr)) return false;
@@ -1024,14 +1018,7 @@ static inline bool _write_cff1 (const cff_subset_plan &plan,
         DEBUG_MSG (SUBSET, nullptr, "failed to serialize CFF Private Dict[%d]", i);
         return false;
       }
-    }
-  }
-
-  if (!plan.desubroutinize)
-  {
-    for (unsigned int i = 0; i < acc.privateDicts.len; i++)
-    {
-      if (!plan.fdmap.excludes (i))
+      if (plan.offsets.localSubrsInfos[i].size > 0)
       {
         CFF1Subrs *dest = c.start_embed <CFF1Subrs> ();
         if (unlikely (dest == nullptr)) return false;

test/api/test-subset-cff1.c

@@ -91,6 +91,53 @@ test_subset_cff1_strip_hints (void)
   hb_face_destroy (face_ac);
 }
 
+static void
+test_subset_cff1_desubr (void)
+{
+  hb_face_t *face_abc = hb_test_open_font_file ("fonts/SourceSansPro-Regular.abc.otf");
+  hb_face_t *face_ac = hb_test_open_font_file ("fonts/SourceSansPro-Regular.ac.nosubrs.otf");
+
+  hb_set_t *codepoints = hb_set_create ();
+  hb_subset_input_t *input;
+  hb_face_t *face_abc_subset;
+  hb_set_add (codepoints, 'a');
+  hb_set_add (codepoints, 'c');
+  input = hb_subset_test_create_input (codepoints);
+  hb_subset_input_set_desubroutinize (input, true);
+  face_abc_subset = hb_subset_test_create_subset (face_abc, input);
+  hb_set_destroy (codepoints);
+
+  hb_subset_test_check (face_ac, face_abc_subset, HB_TAG ('C','F','F',' '));
+
+  hb_face_destroy (face_abc_subset);
+  hb_face_destroy (face_abc);
+  hb_face_destroy (face_ac);
+}
+
+static void
+test_subset_cff1_desubr_strip_hints (void)
+{
+  hb_face_t *face_abc = hb_test_open_font_file ("fonts/SourceSansPro-Regular.abc.otf");
+  hb_face_t *face_ac = hb_test_open_font_file ("fonts/SourceSansPro-Regular.ac.nosubrs.nohints.otf");
+
+  hb_set_t *codepoints = hb_set_create ();
+  hb_subset_input_t *input;
+  hb_face_t *face_abc_subset;
+  hb_set_add (codepoints, 'a');
+  hb_set_add (codepoints, 'c');
+  input = hb_subset_test_create_input (codepoints);
+  hb_subset_input_set_drop_hints (input, true);
+  hb_subset_input_set_desubroutinize (input, true);
+  face_abc_subset = hb_subset_test_create_subset (face_abc, input);
+  hb_set_destroy (codepoints);
+
+  hb_subset_test_check (face_ac, face_abc_subset, HB_TAG ('C', 'F', 'F', ' '));
+
+  hb_face_destroy (face_abc_subset);
+  hb_face_destroy (face_abc);
+  hb_face_destroy (face_ac);
+}
+
 static void
 test_subset_cff1_j (void)
 {
@@ -111,6 +158,76 @@ test_subset_cff1_j (void)
   hb_face_destroy (face_41_4c2e);
 }
 
+static void
+test_subset_cff1_j_strip_hints (void)
+{
+  hb_face_t *face_41_3041_4c2e = hb_test_open_font_file ("fonts/SourceHanSans-Regular.41,3041,4C2E.otf");
+  hb_face_t *face_41_4c2e = hb_test_open_font_file ("fonts/SourceHanSans-Regular.41,4C2E.nohints.otf");
+
+  hb_set_t *codepoints = hb_set_create ();
+  hb_face_t *face_41_3041_4c2e_subset;
+  hb_subset_input_t *input;
+  hb_set_add (codepoints, 0x41);
+  hb_set_add (codepoints, 0x4C2E);
+  input = hb_subset_test_create_input (codepoints);
+  hb_subset_input_set_drop_hints (input, true);
+  face_41_3041_4c2e_subset = hb_subset_test_create_subset (face_41_3041_4c2e, input);
+  hb_set_destroy (codepoints);
+
+  hb_subset_test_check (face_41_4c2e, face_41_3041_4c2e_subset, HB_TAG ('C','F','F',' '));
+
+  hb_face_destroy (face_41_3041_4c2e_subset);
+  hb_face_destroy (face_41_3041_4c2e);
+  hb_face_destroy (face_41_4c2e);
+}
+
+static void
+test_subset_cff1_j_desubr (void)
+{
+  hb_face_t *face_41_3041_4c2e = hb_test_open_font_file ("fonts/SourceHanSans-Regular.41,3041,4C2E.otf");
+  hb_face_t *face_41_4c2e = hb_test_open_font_file ("fonts/SourceHanSans-Regular.41,4C2E.nosubrs.otf");
+
+  hb_set_t *codepoints = hb_set_create ();
+  hb_face_t *face_41_3041_4c2e_subset;
+  hb_subset_input_t *input;
+  hb_set_add (codepoints, 0x41);
+  hb_set_add (codepoints, 0x4C2E);
+  input = hb_subset_test_create_input (codepoints);
+  hb_subset_input_set_desubroutinize (input, true);
+  face_41_3041_4c2e_subset = hb_subset_test_create_subset (face_41_3041_4c2e, input);
+  hb_set_destroy (codepoints);
+
+  hb_subset_test_check (face_41_4c2e, face_41_3041_4c2e_subset, HB_TAG ('C','F','F',' '));
+
+  hb_face_destroy (face_41_3041_4c2e_subset);
+  hb_face_destroy (face_41_3041_4c2e);
+  hb_face_destroy (face_41_4c2e);
+}
+
+static void
+test_subset_cff1_j_desubr_strip_hints (void)
+{
+  hb_face_t *face_41_3041_4c2e = hb_test_open_font_file ("fonts/SourceHanSans-Regular.41,3041,4C2E.otf");
+  hb_face_t *face_41_4c2e = hb_test_open_font_file ("fonts/SourceHanSans-Regular.41,4C2E.nosubrs.nohints.otf");
+
+  hb_set_t *codepoints = hb_set_create ();
+  hb_face_t *face_41_3041_4c2e_subset;
+  hb_subset_input_t *input;
+  hb_set_add (codepoints, 0x41);
+  hb_set_add (codepoints, 0x4C2E);
+  input = hb_subset_test_create_input (codepoints);
+  hb_subset_input_set_drop_hints (input, true);
+  hb_subset_input_set_desubroutinize (input, true);
+  face_41_3041_4c2e_subset = hb_subset_test_create_subset (face_41_3041_4c2e, input);
+  hb_set_destroy (codepoints);
+
+  hb_subset_test_check (face_41_4c2e, face_41_3041_4c2e_subset, HB_TAG ('C','F','F',' '));
+
+  hb_face_destroy (face_41_3041_4c2e_subset);
+  hb_face_destroy (face_41_3041_4c2e);
+  hb_face_destroy (face_41_4c2e);
+}
+
 static void
 test_subset_cff1_expert (void)
 {
@@ -139,7 +256,12 @@ main (int argc, char **argv)
   hb_test_add (test_subset_cff1_noop);
   hb_test_add (test_subset_cff1);
   hb_test_add (test_subset_cff1_strip_hints);
+  hb_test_add (test_subset_cff1_desubr);
+  hb_test_add (test_subset_cff1_desubr_strip_hints);
   hb_test_add (test_subset_cff1_j);
+  hb_test_add (test_subset_cff1_j_strip_hints);
+  hb_test_add (test_subset_cff1_j_desubr);
+  hb_test_add (test_subset_cff1_j_desubr_strip_hints);
   hb_test_add (test_subset_cff1_expert);
 
   return hb_test_run ();