Account for glyph extents in the atlas. (flutter#42)

Author: chinmaygarde

impeller/entity/contents.cc

@@ -635,11 +635,7 @@ bool TextContents::Render(const ContentContext& renderer,
   // Iterate through all the runs in the blob.
   for (const auto& run : frame_.GetRuns()) {
     auto font = run.GetFont();
-    auto glyph_size = font.GetGlyphSize();
-    if (!glyph_size.has_value()) {
-      VALIDATION_LOG << "Glyph has no size.";
-      return false;
-    }
+    auto glyph_size = ISize::Ceil(font.GetMetrics().GetBoundingBox().size);
     // Draw each glyph individually. This should probably be batched.
     for (const auto& glyph_position : run.GetGlyphPositions()) {
       FontGlyphPair font_glyph_pair{font, glyph_position.glyph};
@@ -651,9 +647,9 @@ bool TextContents::Render(const ContentContext& renderer,
 
       VS::GlyphInfo glyph_info;
       glyph_info.position = glyph_position.position.Translate(
-          {0.0, font.GetMetrics().ascent, 0.0});
-      glyph_info.glyph_size = {static_cast<Scalar>(glyph_size->width),
-                               static_cast<Scalar>(glyph_size->height)};
+          {font.GetMetrics().min_extent.x, font.GetMetrics().ascent, 0.0});
+      glyph_info.glyph_size = {static_cast<Scalar>(glyph_size.width),
+                               static_cast<Scalar>(glyph_size.height)};
       glyph_info.atlas_position = atlas_glyph_pos->origin;
       glyph_info.atlas_glyph_size = {atlas_glyph_pos->size.width,
                                      atlas_glyph_pos->size.height};

impeller/typographer/backends/skia/text_frame_skia.cc

@@ -22,6 +22,8 @@ static Font ToFont(const SkFont& font) {
   metrics.point_size = font.getSize();
   metrics.ascent = sk_metrics.fAscent;
   metrics.descent = sk_metrics.fDescent;
+  metrics.min_extent = {sk_metrics.fXMin, sk_metrics.fTop};
+  metrics.max_extent = {sk_metrics.fXMax, sk_metrics.fBottom};
 
   return Font{std::move(typeface), std::move(metrics)};
 }

impeller/typographer/backends/skia/text_render_context_skia.cc

@@ -62,21 +62,19 @@ static bool PairsFitInAtlasOfSize(const FontGlyphPair::Vector& pairs,
   glyph_positions.reserve(pairs.size());
 
   for (const auto& pair : pairs) {
-    auto glyph_size = pair.font.GetGlyphSize();
-    if (!glyph_size.has_value()) {
-      continue;
-    }
+    const auto glyph_size =
+        ISize::Ceil(pair.font.GetMetrics().GetBoundingBox().size);
     SkIPoint16 location_in_atlas;
-    if (!rect_packer->addRect(glyph_size->width,   //
-                              glyph_size->height,  //
-                              &location_in_atlas   //
+    if (!rect_packer->addRect(glyph_size.width,   //
+                              glyph_size.height,  //
+                              &location_in_atlas  //
                               )) {
       return false;
     }
     glyph_positions.emplace_back(Rect::MakeXYWH(location_in_atlas.x(),  //
                                                 location_in_atlas.y(),  //
-                                                glyph_size->width,      //
-                                                glyph_size->height      //
+                                                glyph_size.width,       //
+                                                glyph_size.height       //
                                                 ));
   }
 
@@ -122,22 +120,46 @@ static std::optional<SkBitmap> CreateAtlasBitmap(const GlyphAtlas& atlas,
                                const Rect& location) -> bool {
     const auto position = SkPoint::Make(location.origin.x, location.origin.y);
     SkGlyphID glyph_id = font_glyph.glyph.index;
-    SkFont font(
+
+    SkFont sk_font(
         TypefaceSkia::Cast(*font_glyph.font.GetTypeface()).GetSkiaTypeface(),
         font_glyph.font.GetMetrics().point_size);
 
-    SkFontMetrics metrics;
-    font.getMetrics(&metrics);
+    const auto& metrics = font_glyph.font.GetMetrics();
+
+    auto glyph_color = SK_ColorWHITE;
+
+#if 0
+    {
+      glyph_color = SkColorSetARGB(255,                //
+                                   std::rand() % 255,  //
+                                   std::rand() % 255,  //
+                                   std::rand() % 255   //
+      );
+      SkPaint debug_paint;
+      debug_paint.setARGB(255 / 4,            //
+                          std::rand() % 255,  //
+                          std::rand() % 255,  //
+                          std::rand() % 255   //
+      );
+      canvas->drawRect(SkRect::MakeXYWH(location.origin.x,    //
+                                        location.origin.y,    //
+                                        location.size.width,  //
+                                        location.size.height  //
+                                        ),
+                       debug_paint);
+    }
+#endif
 
     SkPaint glyph_paint;
-    glyph_paint.setColor(SK_ColorWHITE);
+    glyph_paint.setColor(glyph_color);
     canvas->drawGlyphs(1u,         // count
                        &glyph_id,  // glyphs
                        &position,  // positions
-                       SkPoint::Make(0.0,
-                                     -metrics.fAscent),  // origin
-                       font,                             // font
-                       glyph_paint                       // paint
+                       SkPoint::Make(-metrics.min_extent.x,
+                                     -metrics.ascent),  // origin
+                       sk_font,                         // font
+                       glyph_paint                      // paint
     );
     return true;
   });

impeller/typographer/font.cc

@@ -34,13 +34,6 @@ bool Font::IsEqual(const Font& other) const {
          is_valid_ == other.is_valid_ && metrics_ == other.metrics_;
 }
 
-std::optional<ISize> Font::GetGlyphSize() const {
-  if (!IsValid()) {
-    return std::nullopt;
-  }
-  return ISize::Ceil(typeface_->GetBoundingBox().size * metrics_.point_size);
-}
-
 const Font::Metrics& Font::GetMetrics() const {
   return metrics_;
 }

impeller/typographer/font.h

@@ -44,10 +44,34 @@ class Font : public Comparable<Font> {
     /// yields larger numbers.
     ///
     Scalar descent = 0.0f;
+    //--------------------------------------------------------------------------
+    /// The minimum glyph extents relative to the origin. Typically negative in
+    /// an upper-left-origin coordinate system.
+    ///
+    Point min_extent;
+    //--------------------------------------------------------------------------
+    /// The maximum glyph extents relative to the origin. Typically positive in
+    /// an upper-left-origin coordinate system.
+    ///
+    Point max_extent;
+
+    //--------------------------------------------------------------------------
+    /// @brief      The union of the bounding boxes of all the glyphs.
+    ///
+    /// @return     The bounding box.
+    ///
+    constexpr Rect GetBoundingBox() const {
+      return Rect::MakeLTRB(min_extent.x,  //
+                            min_extent.y,  //
+                            max_extent.x,  //
+                            max_extent.y   //
+      );
+    }
 
     constexpr bool operator==(const Metrics& o) const {
       return point_size == o.point_size && ascent == o.ascent &&
-             descent == o.descent;
+             descent == o.descent && min_extent == o.min_extent &&
+             max_extent == o.max_extent;
     }
   };
 
@@ -64,14 +88,6 @@ class Font : public Comparable<Font> {
   ///
   const std::shared_ptr<Typeface>& GetTypeface() const;
 
-  //----------------------------------------------------------------------------
-  /// @brief      A conservatively large scaled bounding box of all glyphs in
-  ///             this font.
-  ///
-  /// @return     The scaled glyph size.
-  ///
-  std::optional<ISize> GetGlyphSize() const;
-
   const Metrics& GetMetrics() const;
 
   // |Comparable<Font>|