Add tt var metrics test cases & bug fixes

src/hb-ot-font.cc

@@ -161,7 +161,9 @@ hb_ot_get_glyph_v_origin (hb_font_t *font,
   if (ot_face->glyf->get_extents (font, glyph, &extents))
   {
     const OT::vmtx_accelerator_t &vmtx = *ot_face->vmtx;
-    hb_position_t tsb = vmtx.get_side_bearing (font, glyph);
+    hb_position_t tsb = vmtx.has_data ()?
+    			vmtx.get_side_bearing (font, glyph):
+    			((extents.height - font->get_glyph_v_advance (glyph)) / 2);
     *y = font->em_scale_y (extents.y_bearing + tsb);
     return true;
   }

src/hb-ot-glyf-table.hh

@@ -452,7 +452,7 @@ struct glyf
       phantoms[PHANTOM_LEFT].x = h_delta;
       phantoms[PHANTOM_RIGHT].x = h_adv + h_delta;
       phantoms[PHANTOM_TOP].y = v_orig;
-      phantoms[PHANTOM_BOTTOM].y = -v_adv + v_orig;
+      phantoms[PHANTOM_BOTTOM].y = -(int)v_adv + v_orig;
     }
 
     /* for a simple glyph, return contour end points, flags, along with coordinate points
@@ -539,39 +539,6 @@ struct glyf
 	      read_points<y_setter_t> (p, points_, checker));
     }
 
-    /* Note: Recursively calls itself. */
-    bool get_var_metrics (hb_codepoint_t glyph,
-			  const int *coords, unsigned int coord_count,
-			  contour_point_vector_t &phantoms /* OUT */,
-			  unsigned int depth=0) const
-    {
-      if (unlikely (depth++ > HB_MAX_NESTING_LEVEL)) return false;
-      contour_point_vector_t	points;
-      hb_vector_t<unsigned int>	end_points;
-      if (unlikely (!get_contour_points (glyph, points, end_points, true/*phantom_only*/))) return false;
-      hb_array_t<contour_point_t>	phantoms_array = points.sub_array (points.length-PHANTOM_COUNT, PHANTOM_COUNT);
-      init_phantom_points (glyph, phantoms_array);
-      if (unlikely (!gvar_accel.apply_deltas_to_points (glyph, coords, coord_count,
-      							points.as_array (), end_points.as_array ()))) return false;
-
-      for (unsigned int i = 0; i < PHANTOM_COUNT; i++)
-      	phantoms[i] = points[points.length - PHANTOM_COUNT + i];
-
-      CompositeGlyphHeader::Iterator composite;
-      if (!get_composite (glyph, &composite)) return true;	/* simple glyph */
-      do
-      {
-	if (composite.current->flags & CompositeGlyphHeader::USE_MY_METRICS)
-	{
-	  if (unlikely (!get_var_metrics (composite.current->glyphIndex, coords, coord_count,
-					  phantoms, depth))) return false;
-
-	  composite.current->transform_points (phantoms);
-	}
-      } while (composite.move_to_next());
-      return true;
-    }
-
     struct contour_bounds_t
     {
       contour_bounds_t () { min.x = min.y = FLT_MAX; max.x = max.y = -FLT_MAX; }
@@ -591,7 +558,7 @@ struct glyf
     };
 
     /* Note: Recursively calls itself.
-     * all_points does not include phantom points
+     * all_points includes phantom points
      */
     bool get_points_var (hb_codepoint_t glyph,
 			 const int *coords, unsigned int coord_count,
@@ -602,8 +569,8 @@ struct glyf
       contour_point_vector_t	points;
       hb_vector_t<unsigned int>	end_points;
       if (unlikely (!get_contour_points (glyph, points, end_points))) return false;
-      hb_array_t<contour_point_t>	phantoms_array = points.sub_array (points.length-PHANTOM_COUNT, PHANTOM_COUNT);
+      hb_array_t<contour_point_t>	phantoms = points.sub_array (points.length-PHANTOM_COUNT, PHANTOM_COUNT);
-      init_phantom_points (glyph, phantoms_array);
+      init_phantom_points (glyph, phantoms);
       if (unlikely (!gvar_accel.apply_deltas_to_points (glyph, coords, coord_count, points.as_array (), end_points.as_array ()))) return false;
 
       unsigned int comp_index = 0;
@@ -611,8 +578,7 @@ struct glyf
       if (!get_composite (glyph, &composite))
       {
       	/* simple glyph */
-      	if (likely (points.length >= PHANTOM_COUNT))
+	all_points.extend (points.as_array ());
-	  all_points.extend (points.sub_array (0, points.length - PHANTOM_COUNT));
       }
       else
       {
@@ -621,7 +587,13 @@ struct glyf
 	{
 	  contour_point_vector_t comp_points;
 	  if (unlikely (!get_points_var (composite.current->glyphIndex, coords, coord_count,
-	  				 comp_points))) return false;
+	  				 comp_points, depth))
+	      		|| comp_points.length < PHANTOM_COUNT) return false;
+
+	  /* Copy phantom points from component if USE_MY_METRICS flag set */
+	  if (composite.current->flags & CompositeGlyphHeader::USE_MY_METRICS)
+	    for (unsigned int i = 0; i < PHANTOM_COUNT; i++)
+	      phantoms[i] = comp_points[comp_points.length - PHANTOM_COUNT + i];
 
 	  /* Apply component transformation & translation */
 	  composite.current->transform_points (comp_points);
@@ -642,58 +614,82 @@ struct glyf
 	      comp_points.translate (delta);
 	    }
 	  }
-	  all_points.extend (comp_points.as_array ());
+
+	  all_points.extend (comp_points.sub_array (0, comp_points.length - PHANTOM_COUNT));
 
 	  comp_index++;
 	} while (composite.move_to_next());
+
+	all_points.extend (phantoms);
       }
 
+      if (depth == 1)
       {
 	/* Undocumented rasterizer behavior:
 	 * Shift points horizontally by the updated left side bearing
 	 */
 	contour_point_t delta;
-	delta.init (-points[points.length - PHANTOM_COUNT + PHANTOM_LEFT].x, 0.f);
+	delta.init (-phantoms[PHANTOM_LEFT].x, 0.f);
 	if (delta.x != 0.f) all_points.translate (delta);
       }
 
       return true;
     }
 
-    bool get_extents_var (hb_codepoint_t glyph,
+    bool get_var_extents_and_phantoms (hb_codepoint_t glyph,
-			  const int *coords, unsigned int coord_count,
+				       const int *coords, unsigned int coord_count,
-			  hb_glyph_extents_t *extents) const
+				       hb_glyph_extents_t *extents=nullptr /* OUt */,
+				       contour_point_vector_t *phantoms=nullptr /* OUT */) const
     {
       contour_point_vector_t all_points;
       if (unlikely (!get_points_var (glyph, coords, coord_count, all_points))) return false;
 
-      contour_bounds_t	bounds;
+      if (extents != nullptr)
-      for (unsigned int i = 0; i < all_points.length; i++)
+      {
-      	bounds.add (all_points[i]);
+	contour_bounds_t	bounds;
+	for (unsigned int i = 0; i + PHANTOM_COUNT < all_points.length; i++)
+	  bounds.add (all_points[i]);
 
-      if (bounds.min.x >= bounds.max.x)
+	if (bounds.min.x > bounds.max.x)
-      {
+	{
-	extents->width = 0;
+	  extents->width = 0;
-	extents->x_bearing = 0;
+	  extents->x_bearing = 0;
+	}
+	else
+	{
+	  extents->x_bearing = (int32_t)floorf (bounds.min.x);
+	  extents->width = (int32_t)ceilf (bounds.max.x) - extents->x_bearing;
+	}
+	if (bounds.min.y > bounds.max.y)
+	{
+	  extents->height = 0;
+	  extents->y_bearing = 0;
+	}
+	else
+	{
+	  extents->y_bearing = (int32_t)ceilf (bounds.max.y);
+	  extents->height = (int32_t)floorf (bounds.min.y) - extents->y_bearing;
+	}
       }
-      else
+      if (phantoms != nullptr)
       {
-	extents->x_bearing = (int32_t)floorf (bounds.min.x);
+      	if (unlikely (all_points.length < PHANTOM_COUNT)) return false;
-	extents->width = (int32_t)ceilf (bounds.max.x) - extents->x_bearing;
+      	for (unsigned int i = 0; i < PHANTOM_COUNT; i++)
-      }
+	  (*phantoms)[i] = all_points[all_points.length - PHANTOM_COUNT + i];
-      if (bounds.min.y >= bounds.max.y)
-      {
-	extents->height = 0;
-	extents->y_bearing = 0;
-      }
-      else
-      {
-	extents->y_bearing = (int32_t)ceilf (bounds.max.y);
-	extents->height = (int32_t)floorf (bounds.min.y) - extents->y_bearing;
       }
       return true;
     }
 
+    bool get_var_metrics (hb_codepoint_t glyph,
+			  const int *coords, unsigned int coord_count,
+			  contour_point_vector_t &phantoms) const
+    { return get_var_extents_and_phantoms (glyph, coords, coord_count, nullptr, &phantoms); }
+
+    bool get_extents_var (hb_codepoint_t glyph,
+			  const int *coords, unsigned int coord_count,
+			  hb_glyph_extents_t *extents) const
+    { return get_var_extents_and_phantoms (glyph, coords, coord_count, extents); }
+
     public:
     /* based on FontTools _g_l_y_f.py::trim */
     bool remove_padding (unsigned int start_offset,
@@ -876,13 +872,14 @@ struct glyf
 
     int get_side_bearing_var (hb_codepoint_t glyph, const int *coords, unsigned int coord_count, bool vertical) const
     {
+      hb_glyph_extents_t extents;
       contour_point_vector_t	phantoms;
       phantoms.resize (PHANTOM_COUNT);
 
-      if (unlikely (!get_var_metrics (glyph, coords, coord_count, phantoms)))
+      if (unlikely (!get_var_extents_and_phantoms (glyph, coords, coord_count, &extents, &phantoms)))
 	return vertical? vmtx_accel.get_side_bearing (glyph): hmtx_accel.get_side_bearing (glyph);
 
-      return (int)(vertical? -ceilf (phantoms[PHANTOM_TOP].y): floorf (phantoms[PHANTOM_LEFT].x));
+      return vertical? (int)ceilf (phantoms[PHANTOM_TOP].y) - extents.y_bearing: (int)floorf (phantoms[PHANTOM_LEFT].x);
     }
 
     bool get_extents (hb_font_t *font, hb_codepoint_t glyph, hb_glyph_extents_t *extents) const

src/hb-ot-hmtx-table.hh

@@ -221,6 +221,8 @@ struct hmtxvmtx
       var_table.destroy ();
     }
 
+    bool has_data () const { return table.get () != nullptr; }
+
     int get_side_bearing (hb_codepoint_t glyph) const
     {
       if (glyph < num_advances)
@@ -274,7 +276,7 @@ struct hmtxvmtx
       	if (font->num_coords)
       	{
 	  if (var_table.get_blob () != hb_blob_get_empty ())
-	    advance += var_table->get_advance_var (glyph, font->coords, font->num_coords); // TODO Optimize?!
+	    advance += roundf (var_table->get_advance_var (glyph, font->coords, font->num_coords)); // TODO Optimize?!
 	  else
 	    advance = get_advance_var_tt (font, glyph, T::tableTag==HB_OT_TAG_vmtx);
 	}

src/hb-ot-var-gvar-table.hh

@@ -673,8 +673,8 @@ struct gvar
 	/* apply specified / inferred deltas to points */
 	for (unsigned int i = 0; i < points.length; i++)
 	{
-	  points[i].x += deltas[i].x;
+	  points[i].x += roundf (deltas[i].x);
-	  points[i].y += deltas[i].y;
+	  points[i].y += roundf (deltas[i].y);
 	}
       } while (iterator.move_to_next ());
 

test/api/test-ot-metrics-tt-var.c

@@ -54,9 +54,9 @@ test_extents_tt_var (void)
   g_assert (result);
 
   g_assert_cmpint (extents.x_bearing, ==, 0);
-  g_assert_cmpint (extents.y_bearing, ==, 875);
+  g_assert_cmpint (extents.y_bearing, ==, 874);
   g_assert_cmpint (extents.width, ==, 551);
-  g_assert_cmpint (extents.height, ==, -875);
+  g_assert_cmpint (extents.height, ==, -874);
 
   hb_font_destroy (font);
 }
@@ -158,9 +158,78 @@ test_advance_tt_var_anchor (void)
   g_assert (result);
 
   g_assert_cmpint (extents.x_bearing, ==, 50);
-  g_assert_cmpint (extents.y_bearing, ==, 668);
+  g_assert_cmpint (extents.y_bearing, ==, 667);
   g_assert_cmpint (extents.width, ==, 593);
-  g_assert_cmpint (extents.height, ==, -680);
+  g_assert_cmpint (extents.height, ==, -679);
+
+  hb_font_destroy (font);
+}
+
+static void
+test_extents_tt_var_comp (void)
+{
+  hb_face_t *face = hb_test_open_font_file ("fonts/SourceSansVariable-Roman.modcomp.ttf");
+  g_assert (face);
+  hb_font_t *font = hb_font_create (face);
+  hb_face_destroy (face);
+  g_assert (font);
+  hb_ot_font_set_funcs (font);
+
+  hb_glyph_extents_t  extents;
+  float coords[1] = { 800.0f };
+  hb_font_set_var_coords_design (font, coords, 1);
+
+  hb_bool_t result;
+  result = hb_font_get_glyph_extents (font, 2, &extents);	/* Ccedilla, cedilla y-scaled by 0.8, with unscaled component offset */
+  g_assert (result);
+
+  g_assert_cmpint (extents.x_bearing, ==, 19);
+  g_assert_cmpint (extents.y_bearing, ==, 663);
+  g_assert_cmpint (extents.width, ==, 519);
+  g_assert_cmpint (extents.height, ==, -895);
+
+  result = hb_font_get_glyph_extents (font, 3, &extents);	/* Cacute, acute y-scaled by 0.8, with unscaled component offset (default) */
+  g_assert (result);
+
+  g_assert_cmpint (extents.x_bearing, ==, 19);
+  g_assert_cmpint (extents.y_bearing, ==, 909);
+  g_assert_cmpint (extents.width, ==, 519);
+  g_assert_cmpint (extents.height, ==, -921);
+
+  result = hb_font_get_glyph_extents (font, 4, &extents);	/* Ccaron, caron y-scaled by 0.8, with scaled component offset */
+  g_assert (result);
+
+  g_assert_cmpint (extents.x_bearing, ==, 19);
+  g_assert_cmpint (extents.y_bearing, ==, 866);
+  g_assert_cmpint (extents.width, ==, 519);
+  g_assert_cmpint (extents.height, ==, -878);
+
+  hb_font_destroy (font);
+}
+
+static void
+test_advance_tt_var_comp_v (void)
+{
+  hb_face_t *face = hb_test_open_font_file ("fonts/SourceSansVariable-Roman.modcomp.ttf");
+  g_assert (face);
+  hb_font_t *font = hb_font_create (face);
+  hb_face_destroy (face);
+  g_assert (font);
+  hb_ot_font_set_funcs (font);
+
+  float coords[1] = { 800.0f };
+  hb_font_set_var_coords_design (font, coords, 1);
+
+  hb_position_t x, y;
+  hb_font_get_glyph_advance_for_direction(font, 2, HB_DIRECTION_TTB, &x, &y);	/* No VVAR; 'C' in composite Ccedilla determines metrics */
+
+  g_assert_cmpint (x, ==, 0);
+  g_assert_cmpint (y, ==, -991);
+
+  hb_font_get_glyph_origin_for_direction(font, 2, HB_DIRECTION_TTB, &x, &y);
+
+  g_assert_cmpint (x, ==, 292);
+  g_assert_cmpint (y, ==, 1013);
 
   hb_font_destroy (font);
 }
@@ -174,6 +243,8 @@ main (int argc, char **argv)
   hb_test_add (test_advance_tt_var_nohvar);
   hb_test_add (test_advance_tt_var_hvarvvar);
   hb_test_add (test_advance_tt_var_anchor);
+  hb_test_add (test_extents_tt_var_comp);
+  hb_test_add (test_advance_tt_var_comp_v);
 
   return hb_test_run ();
 }