CFF1 no-desubr fixes

make sure charstring/subrs not ending with endchar/return handled correctly
if no local subrs, skip serializing Subrs op in Private
misc fixes

src/hb-ot-cff-common.hh

@@ -198,7 +198,7 @@ struct CFFIndex
   { return (const char *)this + min_size + offset_array_size (); }
 
   inline unsigned int data_size (void) const
-  { return HBINT8::static_size; };
+  { return HBINT8::static_size; }
 
   ByteStr operator [] (unsigned int index) const
   {
@@ -331,6 +331,17 @@ struct Dict : UnsizedByteStr
   }
 
   /* in parallel to above */
+  template <typename DICTVAL, typename OP_SERIALIZER, typename PARAM>
+  inline static unsigned int calculate_serialized_size (const DICTVAL &dictval,
+                                                        OP_SERIALIZER& opszr,
+                                                        PARAM& param)
+  {
+    unsigned int size = 0;
+    for (unsigned int i = 0; i < dictval.get_count (); i++)
+      size += opszr.calculate_serialized_size (dictval[i], param);
+    return size;
+  }
+
   template <typename DICTVAL, typename OP_SERIALIZER>
   inline static unsigned int calculate_serialized_size (const DICTVAL &dictval,
                                                         OP_SERIALIZER& opszr)

src/hb-subset-cff-common.hh

@@ -255,7 +255,7 @@ struct CFFPrivateDict_OpSerializer : OpSerializer
       return true;
     if (opstr.op == OpCode_Subrs)
     {
-      if (desubroutinize)
+      if (desubroutinize || (subrsOffset == 0))
         return_trace (true);
       else
         return_trace (FontDict::serialize_offset4_op (c, opstr.op, subrsOffset));
@@ -264,13 +264,14 @@ struct CFFPrivateDict_OpSerializer : OpSerializer
       return_trace (copy_opstr (c, opstr));
   }
 
-  inline unsigned int calculate_serialized_size (const OpStr &opstr) const
+  inline unsigned int calculate_serialized_size (const OpStr &opstr,
+                                                 bool has_localsubr=true) const
   {
     if (drop_hints && DictOpSet::is_hint_op (opstr.op))
       return 0;
     if (opstr.op == OpCode_Subrs)
     {
-      if (desubroutinize)
+      if (desubroutinize || !has_localsubr)
         return 0;
       else
         return OpCode_Size (OpCode_longintdict) + 4 + OpCode_Size (opstr.op);
@@ -487,34 +488,32 @@ struct SubrSubsetParam
     drop_hints = drop_hints_;
   }
 
-  template <typename ENV>
+  inline ParsedCStr *get_parsed_str_for_context (CallContext &context)
-  inline void set_current_str (ENV &env)
   {
-    const CallContext &context = env.context;
-  
     switch (context.type)
     {
       case CSType_CharString:
-        current_parsed_str = parsed_charstring;
+        return parsed_charstring;
-        break;
       
       case CSType_LocalSubr:
         if (likely (context.subr_num < parsed_local_subrs->len))
-          current_parsed_str = &(*parsed_local_subrs)[context.subr_num];
+          return &(*parsed_local_subrs)[context.subr_num];
-        else
-          env.set_error ();
-        break;
       
       case CSType_GlobalSubr:
         if (likely (context.subr_num < parsed_global_subrs->len))
-          current_parsed_str = &(*parsed_global_subrs)[context.subr_num];
+          return &(*parsed_global_subrs)[context.subr_num];
-        else
-          env.set_error ();
-        break;
-
-      default:
-        assert (0);
     }
+    return nullptr;
+  }
+
+  template <typename ENV>
+  inline void set_current_str (ENV &env)
+  {
+    ParsedCStr  *parsed_str = get_parsed_str_for_context (env.context);
+    if (likely (parsed_str != nullptr))
+      current_parsed_str = parsed_str;
+    else
+      env.set_error ();
   }
 
   ParsedCStr    *current_parsed_str;
@@ -559,7 +558,9 @@ struct SubrRemap : Remap
 
   inline int biased_num (unsigned int old_num) const
   {
-    return (int)(*this)[old_num] - bias;
+    hb_codepoint_t new_num = (*this)[old_num];
+    assert (new_num != CFF_UNDEF_CODE);
+    return (int)new_num - bias;
   }
 
   protected:
@@ -670,8 +671,8 @@ struct SubrSubsetter
       if (unlikely (!interp.interpret (param)))
         return false;
 
-      /* copy CFF1 width or CFF2 vsindex to the parsed charstring for encoding */
+      /* finalize parsed string esp. copy CFF1 width or CFF2 vsindex to the parsed charstring for encoding */
-      SUBSETTER::set_parsed_prefix (interp.env, parsed_charstrings[i]);
+      SUBSETTER::finalize_parsed_str (interp.env, param, parsed_charstrings[i]);
     }
     
     if (drop_hints)
@@ -692,7 +693,7 @@ struct SubrSubsetter
           parsed_charstrings[i].set_hint_removed ();
       }
 
-      /* after dropping hints recreate closures from subrs actually used */
+      /* after dropping hints recreate closures of actually used subrs */
       closures.reset ();
       for (unsigned int i = 0; i < glyphs.len; i++)
       {
@@ -724,7 +725,7 @@ struct SubrSubsetter
     return true;
   }
 
-  inline bool encode_subrs (const ParsedCStrs &subrs, const SubrRemap& remap, StrBuffArray &buffArray) const
+  inline bool encode_subrs (const ParsedCStrs &subrs, const SubrRemap& remap, unsigned int fd, StrBuffArray &buffArray) const
   {
     unsigned int  count = remap.get_count ();
   
@@ -735,7 +736,7 @@ struct SubrSubsetter
       hb_codepoint_t new_num = remap[old_num];
       if (new_num != CFF_UNDEF_CODE)
       {
-        if (unlikely (!encode_str (subrs[old_num], 0, buffArray[new_num])))
+        if (unlikely (!encode_str (subrs[old_num], fd, buffArray[new_num])))
           return false;
       }
     }
@@ -744,12 +745,12 @@ struct SubrSubsetter
 
   inline bool encode_globalsubrs (StrBuffArray &buffArray)
   {
-    return encode_subrs (parsed_global_subrs, remaps.global_remap, buffArray);
+    return encode_subrs (parsed_global_subrs, remaps.global_remap, 0, buffArray);
   }
 
   inline bool encode_localsubrs (unsigned int fd, StrBuffArray &buffArray) const
   {
-    return encode_subrs (parsed_local_subrs[fd], remaps.local_remaps[fd], buffArray);
+    return encode_subrs (parsed_local_subrs[fd], remaps.local_remaps[fd], fd, buffArray);
   }
 
   protected:

src/hb-subset-cff1.cc

@@ -390,9 +390,6 @@ struct CFF1CSOpSet_SubrSubset : CFF1CSOpSet<CFF1CSOpSet_SubrSubset, SubrSubsetPa
     param.current_parsed_str->add_call_op (op, substr, env.context.subr_num);
     hb_set_add (closure, env.context.subr_num);
     param.set_current_str (env);
-    if ( unlikely (!param.current_parsed_str->is_parsed ()
-                && (param.current_parsed_str->values.len > 0)))
-      env.set_error ();
   }
 
   private:
@@ -401,10 +398,24 @@ struct CFF1CSOpSet_SubrSubset : CFF1CSOpSet<CFF1CSOpSet_SubrSubset, SubrSubsetPa
 
 struct CFF1SubrSubsetter : SubrSubsetter<CFF1SubrSubsetter, CFF1Subrs, const OT::cff1::accelerator_subset_t, CFF1CSInterpEnv, CFF1CSOpSet_SubrSubset>
 {
-  static inline void set_parsed_prefix (const CFF1CSInterpEnv &env, ParsedCStr &charstring)
+  static inline void finalize_parsed_str (CFF1CSInterpEnv &env, SubrSubsetParam& param, ParsedCStr &charstring)
   {
+    /* insert width at the beginning of the charstring as necessary */
     if (env.has_width)
       charstring.set_prefix (env.width);
+
+    /* subroutines/charstring left on the call stack are legally left unmarked
+     * unmarked when a subroutine terminates with endchar. mark them.
+     */
+    param.current_parsed_str->set_parsed ();
+    for (unsigned int i = 0; i < env.callStack.get_count (); i++)
+    {
+      ParsedCStr  *parsed_str = param.get_parsed_str_for_context (env.callStack[i]);
+      if (likely (parsed_str != nullptr))
+        parsed_str->set_parsed ();
+      else
+        env.set_error ();
+    }
   }
 };
 
@@ -774,15 +785,16 @@ struct cff_subset_plan {
     {
       if (fdmap.includes (i))
       {
+        bool  has_localsubrs = offsets.localSubrsInfos[i].size > 0;
         CFFPrivateDict_OpSerializer privSzr (desubroutinize, plan->drop_hints);
-        unsigned int  priv_size = PrivateDict::calculate_serialized_size (acc.privateDicts[i], privSzr);
+        unsigned int  priv_size = PrivateDict::calculate_serialized_size (acc.privateDicts[i], privSzr, has_localsubrs);
         TableInfo  privInfo = { final_size, priv_size, 0 };
         FontDictValuesMod fontdict_mod;
         fontdict_mod.init ( &acc.fontDicts[i], sidmap[acc.fontDicts[i].fontName], privInfo );
         fontdicts_mod.push (fontdict_mod);
         final_size += privInfo.size;
 
-        if (!plan->desubroutinize && (offsets.localSubrsInfos[i].size > 0))
+        if (!plan->desubroutinize && has_localsubrs)
         {
           offsets.localSubrsInfos[i].offset = final_size;
           final_size += offsets.localSubrsInfos[i].size;
@@ -1014,7 +1026,8 @@ static inline bool _write_cff1 (const cff_subset_plan &plan,
       bool result;
       CFFPrivateDict_OpSerializer privSzr (plan.desubroutinize, plan.drop_hints);
       /* N.B. local subrs immediately follows its corresponding private dict. i.e., subr offset == private dict size */
-      result = pd->serialize (&c, acc.privateDicts[i], privSzr, priv_size);
+      unsigned int  subroffset = (plan.offsets.localSubrsInfos[i].size > 0)? priv_size: 0;
+      result = pd->serialize (&c, acc.privateDicts[i], privSzr, subroffset);
       if (unlikely (!result))
       {
         DEBUG_MSG (SUBSET, nullptr, "failed to serialize CFF Private Dict[%d]", i);