From faa5bea1b96ddb3ca9eb69a7dc88edabb01884b4 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Tue, 6 Sep 2022 23:57:31 -0700 Subject: [PATCH] [Impeller] Use ascent/descent rather than deprecated top/bottom for glyph bounds (flutter/engine#35960) --- .../impeller/entity/contents/text_contents.cc | 4 ++- .../backends/skia/text_frame_skia.cc | 4 +-- .../backends/skia/text_render_context_skia.cc | 25 +++++++++++-------- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/engine/src/flutter/impeller/entity/contents/text_contents.cc b/engine/src/flutter/impeller/entity/contents/text_contents.cc index c082aabb4a1..3768dc05efe 100644 --- a/engine/src/flutter/impeller/entity/contents/text_contents.cc +++ b/engine/src/flutter/impeller/entity/contents/text_contents.cc @@ -138,7 +138,9 @@ bool TextContents::Render(const ContentContext& renderer, Point{font.GetMetrics().min_extent.x, font.GetMetrics().ascent}; vtx.glyph_size = Point{static_cast(glyph_size.width), static_cast(glyph_size.height)}; - vtx.atlas_position = atlas_glyph_pos->origin; + vtx.atlas_position = + atlas_glyph_pos->origin + Point{1 / atlas_glyph_pos->size.width, + 1 / atlas_glyph_pos->size.height}; vtx.atlas_glyph_size = Point{atlas_glyph_pos->size.width, atlas_glyph_pos->size.height}; vertex_builder.AppendVertex(std::move(vtx)); diff --git a/engine/src/flutter/impeller/typographer/backends/skia/text_frame_skia.cc b/engine/src/flutter/impeller/typographer/backends/skia/text_frame_skia.cc index fa3e7406844..287876de3a7 100644 --- a/engine/src/flutter/impeller/typographer/backends/skia/text_frame_skia.cc +++ b/engine/src/flutter/impeller/typographer/backends/skia/text_frame_skia.cc @@ -23,8 +23,8 @@ static Font ToFont(const SkFont& font, Scalar scale) { 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}; + metrics.min_extent = {sk_metrics.fXMin, sk_metrics.fAscent}; + metrics.max_extent = {sk_metrics.fXMax, sk_metrics.fDescent}; return Font{std::move(typeface), std::move(metrics)}; } diff --git a/engine/src/flutter/impeller/typographer/backends/skia/text_render_context_skia.cc b/engine/src/flutter/impeller/typographer/backends/skia/text_render_context_skia.cc index cb6fbec001c..aab49ed2fec 100644 --- a/engine/src/flutter/impeller/typographer/backends/skia/text_render_context_skia.cc +++ b/engine/src/flutter/impeller/typographer/backends/skia/text_render_context_skia.cc @@ -122,13 +122,14 @@ static std::shared_ptr CreateAtlasBitmap(const GlyphAtlas& atlas, atlas.IterateGlyphs([canvas](const FontGlyphPair& font_glyph, const Rect& location) -> bool { - const auto position = SkPoint::Make(location.origin.x, location.origin.y); + const auto position = + SkPoint::Make(location.origin.x / font_glyph.font.GetMetrics().scale, + location.origin.y / font_glyph.font.GetMetrics().scale); SkGlyphID glyph_id = font_glyph.glyph.index; SkFont sk_font( TypefaceSkia::Cast(*font_glyph.font.GetTypeface()).GetSkiaTypeface(), - font_glyph.font.GetMetrics().point_size * - font_glyph.font.GetMetrics().scale); + font_glyph.font.GetMetrics().point_size); const auto& metrics = font_glyph.font.GetMetrics(); @@ -136,14 +137,16 @@ static std::shared_ptr CreateAtlasBitmap(const GlyphAtlas& atlas, SkPaint glyph_paint; glyph_paint.setColor(glyph_color); - canvas->drawGlyphs( - 1u, // count - &glyph_id, // glyphs - &position, // positions - SkPoint::Make(-metrics.min_extent.x * metrics.scale, - -metrics.ascent * metrics.scale), // origin - sk_font, // font - glyph_paint // paint + canvas->resetMatrix(); + canvas->scale(font_glyph.font.GetMetrics().scale, + font_glyph.font.GetMetrics().scale); + canvas->drawGlyphs(1u, // count + &glyph_id, // glyphs + &position, // positions + SkPoint::Make(-metrics.min_extent.x, + -metrics.ascent), // origin + sk_font, // font + glyph_paint // paint ); return true; });