Merge pull request #4052 from googlefonts/instancer_recalc_bounds

[instancer] recalc bounds by default when --instance option is enabled

src/OT/glyf/Glyph.hh

@@ -80,13 +80,20 @@ struct Glyph
   }
 
   void update_mtx (const hb_subset_plan_t *plan,
-                   int xMin, int yMax,
+                   int xMin, int xMax,
+                   int yMin, int yMax,
                    const contour_point_vector_t &all_points) const
   {
     hb_codepoint_t new_gid = 0;
     if (!plan->new_gid_for_old_gid (gid, &new_gid))
       return;
 
+    if (type != EMPTY)
+    {
+      plan->bounds_width_map.set (new_gid, xMax - xMin);
+      plan->bounds_height_map.set (new_gid, yMax - yMin);
+    }
+
     unsigned len = all_points.length;
     float leftSideX = all_points[len - 4].x;
     float rightSideX = all_points[len - 3].x;
@@ -109,7 +116,7 @@ struct Glyph
                              hb_bytes_t &dest_bytes /* OUT */) const
   {
     GlyphHeader *glyph_header = nullptr;
-    if (type != EMPTY && all_points.length > 4)
+    if (!plan->pinned_at_default && type != EMPTY && all_points.length > 4)
     {
       glyph_header = (GlyphHeader *) hb_calloc (1, GlyphHeader::static_size);
       if (unlikely (!glyph_header)) return false;
@@ -133,18 +140,33 @@ struct Glyph
       yMax = hb_max (yMax, y);
     }
 
-    update_mtx (plan, roundf (xMin), roundf (yMax), all_points);
+    update_mtx (plan, roundf (xMin), roundf (xMax), roundf (yMin), roundf (yMax), all_points);
  
-    /*for empty glyphs: all_points only include phantom points.
+    int rounded_xMin = roundf (xMin);
+    int rounded_xMax = roundf (xMax);
+    int rounded_yMin = roundf (yMin);
+    int rounded_yMax = roundf (yMax);
+
+    if (type != EMPTY)
+    {
+      plan->head_maxp_info.xMin = hb_min (plan->head_maxp_info.xMin, rounded_xMin);
+      plan->head_maxp_info.yMin = hb_min (plan->head_maxp_info.yMin, rounded_yMin);
+      plan->head_maxp_info.xMax = hb_max (plan->head_maxp_info.xMax, rounded_xMax);
+      plan->head_maxp_info.yMax = hb_max (plan->head_maxp_info.yMax, rounded_yMax);
+    }
+
+    /* when pinned at default, no need to compile glyph header
+     * and for empty glyphs: all_points only include phantom points.
      * just update metrics and then return */
     if (!glyph_header)
       return true;
 
     glyph_header->numberOfContours = header->numberOfContours;
-    glyph_header->xMin = roundf (xMin);
-    glyph_header->yMin = roundf (yMin);
-    glyph_header->xMax = roundf (xMax);
-    glyph_header->yMax = roundf (yMax);
+
+    glyph_header->xMin = rounded_xMin;
+    glyph_header->yMin = rounded_yMin;
+    glyph_header->xMax = rounded_xMax;
+    glyph_header->yMax = rounded_yMax;
 
     dest_bytes = hb_bytes_t ((const char *)glyph_header, GlyphHeader::static_size);
     return true;
@@ -157,15 +179,22 @@ struct Glyph
                                   hb_bytes_t &dest_end /* OUT */)
   {
     contour_point_vector_t all_points, deltas;
-    if (!get_points (font, glyf, all_points, &deltas, false, false))
+    unsigned composite_contours = 0;
+    if (!get_points (font, glyf, all_points, &deltas, &plan->head_maxp_info, &composite_contours, false, false))
       return false;
 
     // .notdef, set type to empty so we only update metrics and don't compile bytes for
     // it
     if (gid == 0 &&
         !(plan->flags & HB_SUBSET_FLAGS_NOTDEF_OUTLINE))
+    {
       type = EMPTY;
+      dest_start = hb_bytes_t ();
+      dest_end = hb_bytes_t ();
+    }
 
+    //dont compile bytes when pinned at default, just recalculate bounds
+    if (!plan->pinned_at_default) {
       switch (type) {
       case COMPOSITE:
         if (!CompositeGlyph (*header, bytes).compile_bytes_with_deltas (dest_start,
@@ -186,6 +215,7 @@ struct Glyph
         dest_end = hb_bytes_t ();
         break;
       }
+    }
 
     if (!compile_header_bytes (plan, all_points, dest_start))
     {
@@ -203,6 +233,8 @@ struct Glyph
   bool get_points (hb_font_t *font, const accelerator_t &glyf_accelerator,
 		   contour_point_vector_t &all_points /* OUT */,
 		   contour_point_vector_t *deltas = nullptr, /* OUT */
+		   head_maxp_info_t * head_maxp_info = nullptr, /* OUT */
+		   unsigned *composite_contours = nullptr, /* OUT */
 		   bool shift_points_hori = true,
 		   bool use_my_metrics = true,
 		   bool phantom_only = false,
@@ -216,6 +248,11 @@ struct Glyph
     if (unlikely (*edge_count > HB_GLYF_MAX_EDGE_COUNT)) return false;
     (*edge_count)++;
     
+    if (head_maxp_info)
+    {
+      head_maxp_info->maxComponentDepth = hb_max (head_maxp_info->maxComponentDepth, depth);
+    }
+
     if (!coords)
       coords = hb_array (font->coords, font->num_coords);
 
@@ -226,6 +263,10 @@ struct Glyph
 
     switch (type) {
     case SIMPLE:
+      if (depth == 0 && head_maxp_info)
+        head_maxp_info->maxContours = hb_max (head_maxp_info->maxContours, (unsigned) header->numberOfContours);
+      if (depth > 0 && composite_contours)
+        *composite_contours += (unsigned) header->numberOfContours;
       if (unlikely (!SimpleGlyph (*header, bytes).get_contour_points (points, phantom_only)))
 	return false;
       break;
@@ -301,6 +342,8 @@ struct Glyph
 
     switch (type) {
     case SIMPLE:
+      if (depth == 0 && head_maxp_info)
+        head_maxp_info->maxPoints = hb_max (head_maxp_info->maxPoints, points.length - 4);
       if (!inplace)
 	all_points.extend (points.as_array ());
       break;
@@ -311,12 +354,13 @@ struct Glyph
       for (auto &item : get_composite_iterator ())
       {
         comp_points.reset ();
-
 	if (unlikely (!glyf_accelerator.glyph_for_gid (item.get_gid ())
 				       .get_points (font,
 						    glyf_accelerator,
 						    comp_points,
 						    deltas,
+						    head_maxp_info,
+						    composite_contours,
 						    shift_points_hori,
 						    use_my_metrics,
 						    phantom_only,
@@ -358,6 +402,13 @@ struct Glyph
 	comp_index++;
       }
 
+      if (head_maxp_info && depth == 0)
+      {
+        if (composite_contours)
+          head_maxp_info->maxCompositeContours = hb_max (head_maxp_info->maxCompositeContours, *composite_contours);
+        head_maxp_info->maxCompositePoints = hb_max (head_maxp_info->maxCompositePoints, all_points.length);
+        head_maxp_info->maxComponentElements = hb_max (head_maxp_info->maxComponentElements, comp_index);
+      }
       all_points.extend (phantoms);
     } break;
 #ifndef HB_NO_VAR_COMPOSITES
@@ -383,6 +434,8 @@ struct Glyph
 						    glyf_accelerator,
 						    comp_points,
 						    deltas,
+						    head_maxp_info,
+						    nullptr,
 						    shift_points_hori,
 						    use_my_metrics,
 						    phantom_only,

src/OT/glyf/glyf-helpers.hh

@@ -44,6 +44,13 @@ _add_head_and_set_loca_version (hb_subset_plan_t *plan, bool use_short_loca)
 
   head *head_prime = (head *) hb_blob_get_data_writable (head_prime_blob, nullptr);
   head_prime->indexToLocFormat = use_short_loca ? 0 : 1;
+  if (plan->normalized_coords)
+  {
+    head_prime->xMin = plan->head_maxp_info.xMin;
+    head_prime->xMax = plan->head_maxp_info.xMax;
+    head_prime->yMin = plan->head_maxp_info.yMin;
+    head_prime->yMax = plan->head_maxp_info.yMax;
+  }
   bool success = plan->add_table (HB_OT_TAG_head, head_prime_blob);
 
   hb_blob_destroy (head_prime_blob);

src/OT/glyf/glyf.hh

@@ -80,7 +80,7 @@ struct glyf
     _populate_subset_glyphs (c->plan, glyphs);
 
     hb_font_t *font = nullptr;
-    if (!c->plan->pinned_at_default)
+    if (c->plan->normalized_coords)
     {
       font = _create_font_for_instancing (c->plan);
       if (unlikely (!font)) return false;
@@ -108,6 +108,7 @@ struct glyf
 
     if (font)
     {
+      if (!c->plan->pinned_at_default)
         _free_compiled_subset_glyphs (&glyphs);
       hb_font_destroy (font);
     }
@@ -194,7 +195,7 @@ struct glyf_accelerator_t
     contour_point_vector_t all_points;
 
     bool phantom_only = !consumer.is_consuming_contour_points ();
-    if (unlikely (!glyph_for_gid (gid).get_points (font, *this, all_points, nullptr, true, true, phantom_only)))
+    if (unlikely (!glyph_for_gid (gid).get_points (font, *this, all_points, nullptr, nullptr, nullptr, true, true, phantom_only)))
       return false;
 
     if (consumer.is_consuming_contour_points ())
@@ -406,7 +407,7 @@ glyf::_populate_subset_glyphs (const hb_subset_plan_t   *plan,
 
     if (unlikely (new_gid == 0 &&
                   !(plan->flags & HB_SUBSET_FLAGS_NOTDEF_OUTLINE)) &&
-                  plan->pinned_at_default)
+                  !plan->normalized_coords)
       subset_glyph.source_glyph = glyf_impl::Glyph ();
     else
     {

src/hb-ot-head-table.hh

@@ -148,10 +148,12 @@ struct head
 					   January 1, 1904. 64-bit integer */
   LONGDATETIME	modified;		/* Number of seconds since 12:00 midnight,
 					   January 1, 1904. 64-bit integer */
+  public:
   HBINT16	xMin;			/* For all glyph bounding boxes. */
   HBINT16	yMin;			/* For all glyph bounding boxes. */
   HBINT16	xMax;			/* For all glyph bounding boxes. */
   HBINT16	yMax;			/* For all glyph bounding boxes. */
+  protected:
   HBUINT16	macStyle;		/* Bit 0: Bold (if set to 1);
 					 * Bit 1: Italic (if set to 1)
 					 * Bit 2: Underline (if set to 1)

src/hb-ot-hmtx-table.hh

@@ -78,7 +78,9 @@ struct hmtxvmtx
   { return T::is_horizontal ? &plan->hmtx_map : &plan->vmtx_map; }
 
   bool subset_update_header (hb_subset_context_t *c,
-			     unsigned int num_hmetrics) const
+			     unsigned int num_hmetrics,
+			     const hb_hashmap_t<hb_codepoint_t, hb_pair_t<unsigned, int>> *mtx_map,
+			     const hb_map_t *bounds_map) const
   {
     hb_blob_t *src_blob = hb_sanitize_context_t ().reference_table<H> (c->plan->source, H::tableTag);
     hb_blob_t *dest_blob = hb_blob_copy_writable_or_fail (src_blob);
@@ -108,6 +110,36 @@ struct hmtxvmtx
 	HB_ADD_MVAR_VAR (HB_OT_METRICS_TAG_VERTICAL_CARET_RUN,      caretSlopeRun);
 	HB_ADD_MVAR_VAR (HB_OT_METRICS_TAG_VERTICAL_CARET_OFFSET,   caretOffset);
       }
+
+      int min_lsb = 0x7FFF;
+      int min_training_sb = 0x7FFF;
+      int max_extent = -0x7FFF;
+      unsigned max_adv = 0;
+      for (const auto _ : *mtx_map)
+      {
+        hb_codepoint_t gid = _.first;
+        unsigned adv = _.second.first;
+        int lsb = _.second.second;
+        max_adv = hb_max (max_adv, adv);
+
+        if (bounds_map->has (gid))
+        {
+          unsigned bound_width = bounds_map->get (gid);
+          int rsb = adv - lsb - bound_width;
+          int extent = lsb + bound_width;
+          min_lsb = hb_min (min_lsb, lsb);
+          min_training_sb = hb_min (min_training_sb, rsb);
+          max_extent = hb_max (max_extent, extent);
+        }
+      }
+
+      table->advanceMax = max_adv;
+      if (!bounds_map->is_empty ())
+      {
+        table->minLeadingBearing = min_lsb;
+        table->minTrailingBearing = min_training_sb;
+        table->maxExtent = max_extent;
+      }
     }
 #endif
 
@@ -189,7 +221,8 @@ struct hmtxvmtx
       return_trace (false);
 
     // Amend header num hmetrics
-    if (unlikely (!subset_update_header (c, num_long_metrics)))
+    if (unlikely (!subset_update_header (c, num_long_metrics, mtx_map,
+                                         T::is_horizontal ? &c->plan->bounds_width_map : &c->plan->bounds_height_map)))
       return_trace (false);
 
     return_trace (true);
@@ -362,15 +395,13 @@ struct hmtxvmtx
                                          unsigned new_gid,
                                          const accelerator_t &_mtx) const
   {
-    if (mtx_map->is_empty () ||
-        (new_gid == 0 && !mtx_map->has (new_gid)))
+    if (mtx_map->is_empty ())
     {
       hb_codepoint_t old_gid = 0;
       return plan->old_gid_for_new_gid (new_gid, &old_gid) ?
              _mtx.get_advance_without_var_unscaled (old_gid) : 0;
     }
-    else
-    { return mtx_map->get (new_gid).first; }
+    return mtx_map->get (new_gid).first;
   }
 
   protected:

src/hb-ot-maxp-table.hh

@@ -109,11 +109,24 @@ struct maxp
 
       if (c->plan->flags & HB_SUBSET_FLAGS_NO_HINTING)
 	drop_hint_fields (dest_v1);
+
+      if (c->plan->normalized_coords)
+        instancing_update_fields (c->plan->head_maxp_info, dest_v1);
     }
 
     return_trace (true);
   }
 
+  void instancing_update_fields (head_maxp_info_t& maxp_info, maxpV1Tail* dest_v1) const
+  {
+    dest_v1->maxPoints = maxp_info.maxPoints;
+    dest_v1->maxContours = maxp_info.maxContours;
+    dest_v1->maxCompositePoints = maxp_info.maxCompositePoints;
+    dest_v1->maxCompositeContours = maxp_info.maxCompositeContours;
+    dest_v1->maxComponentElements = maxp_info.maxComponentElements;
+    dest_v1->maxComponentDepth = maxp_info.maxComponentDepth;
+  }
+
   static void drop_hint_fields (maxpV1Tail* dest_v1)
   {
     dest_v1->maxZones = 1;

src/hb-subset-plan.hh

@@ -41,6 +41,30 @@ namespace OT {
 struct Feature;
 }
 
+struct head_maxp_info_t
+{
+  head_maxp_info_t ()
+      :xMin (0x7FFF), xMax (-0x7FFF), yMin (0x7FFF), yMax (-0x7FFF),
+      maxPoints (0), maxContours (0),
+      maxCompositePoints (0),
+      maxCompositeContours (0),
+      maxComponentElements (0),
+      maxComponentDepth (0) {}
+
+  int xMin;
+  int xMax;
+  int yMin;
+  int yMax;
+  unsigned maxPoints;
+  unsigned maxContours;
+  unsigned maxCompositePoints;
+  unsigned maxCompositeContours;
+  unsigned maxComponentElements;
+  unsigned maxComponentDepth;
+};
+
+typedef struct head_maxp_info_t head_maxp_info_t;
+
 struct hb_subset_plan_t
 {
   HB_INTERNAL hb_subset_plan_t (hb_face_t *,
@@ -163,6 +187,13 @@ struct hb_subset_plan_t
   mutable hb_hashmap_t<hb_codepoint_t, hb_pair_t<unsigned, int>> hmtx_map;
   //vmtx metrics map: new gid->(advance, lsb)
   mutable hb_hashmap_t<hb_codepoint_t, hb_pair_t<unsigned, int>> vmtx_map;
+  //boundsWidth map: new gid->boundsWidth, boundWidth=xMax - xMin
+  mutable hb_map_t bounds_width_map;
+  //boundsHeight map: new gid->boundsHeight, boundsHeight=yMax - yMin
+  mutable hb_map_t bounds_height_map;
+
+  //recalculated head/maxp table info after instancing
+  mutable head_maxp_info_t head_maxp_info;
 
 #ifdef HB_EXPERIMENTAL_API
   // name table overrides map: hb_ot_name_record_ids_t-> name string new value or

src/hb-subset.cc

@@ -413,7 +413,8 @@ _dependencies_satisfied (hb_subset_plan_t *plan, hb_tag_t tag,
   {
   case HB_OT_TAG_hmtx:
   case HB_OT_TAG_vmtx:
-    return plan->pinned_at_default || !pending_subset_tags.has (HB_OT_TAG_glyf);
+  case HB_OT_TAG_maxp:
+    return !plan->normalized_coords || !pending_subset_tags.has (HB_OT_TAG_glyf);
   default:
     return true;
   }

test/subset/generate-expected-outputs.py

@@ -33,7 +33,6 @@ def generate_expected_output(input_file, unicodes, profile_flags, instance_flags
 		instance_path = os.path.join(tempfile.mkdtemp (), font_name)
 		args = ["fonttools", "varLib.instancer",
 			"--no-overlap-flag",
-			"--no-recalc-bounds",
 			"--no-recalc-timestamp",
 			"--output=%s" % instance_path,
 			input_file]
@@ -43,6 +42,8 @@ def generate_expected_output(input_file, unicodes, profile_flags, instance_flags
 
 	fonttools_path = os.path.join(tempfile.mkdtemp (), font_name)
 	args = ["fonttools", "subset", input_path]
+	if instance_flags:
+		args.extend(["--recalc-bounds"])
 	args.extend(["--drop-tables+=DSIG",
 		     "--drop-tables-=sbix",
 		     "--no-harfbuzz-repacker", # disable harfbuzz repacker so we aren't comparing to ourself.