From 03db0fa927e9f10d4434d7cdc37718db3a98d5c4 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 8 Sep 2024 10:52:39 -0700 Subject: [PATCH 1/4] [Impeller] hash less text stuff per frame. --- impeller/entity/contents/text_contents.cc | 7 +- impeller/typographer/BUILD.gn | 6 + .../backends/skia/typographer_context_skia.cc | 28 ++--- impeller/typographer/font_glyph_pair.h | 108 ++++++++++-------- impeller/typographer/glyph_atlas.cc | 4 +- impeller/typographer/glyph_atlas.h | 16 ++- impeller/typographer/text_frame.cc | 6 +- impeller/typographer/text_run.h | 2 +- 8 files changed, 107 insertions(+), 70 deletions(-) diff --git a/impeller/entity/contents/text_contents.cc b/impeller/entity/contents/text_contents.cc index 6dde4e65b6d7e..8254902e8df09 100644 --- a/impeller/entity/contents/text_contents.cc +++ b/impeller/entity/contents/text_contents.cc @@ -222,8 +222,11 @@ bool TextContents::Render(const ContentContext& renderer, Point subpixel = TextFrame::ComputeSubpixelPosition( glyph_position, font.GetAxisAlignment(), offset_, scale_); std::optional> maybe_atlas_glyph_bounds = - font_atlas->FindGlyphBounds( - SubpixelGlyph{glyph_position.glyph, subpixel, properties_}); + font_atlas->FindGlyphBounds(SubpixelGlyph{ + glyph_position.glyph, subpixel, + (properties_.stroke || frame_->HasColor()) + ? std::optional(properties_) + : std::nullopt}); if (!maybe_atlas_glyph_bounds.has_value()) { VALIDATION_LOG << "Could not find glyph position in the atlas."; continue; diff --git a/impeller/typographer/BUILD.gn b/impeller/typographer/BUILD.gn index 90789a87a4582..3db66ab1d27ae 100644 --- a/impeller/typographer/BUILD.gn +++ b/impeller/typographer/BUILD.gn @@ -32,6 +32,12 @@ impeller_component("typographer") { "../base", "../geometry", "../renderer", + "//flutter/third_party/abseil-cpp/absl/algorithm", + "//flutter/third_party/abseil-cpp/absl/algorithm:container", + "//flutter/third_party/abseil-cpp/absl/base:config", + "//flutter/third_party/abseil-cpp/absl/container:flat_hash_map", + "//flutter/third_party/abseil-cpp/absl/container:flat_hash_set", + "//flutter/third_party/abseil-cpp/absl/container:node_hash_map", ] deps = [ "//flutter/fml" ] diff --git a/impeller/typographer/backends/skia/typographer_context_skia.cc b/impeller/typographer/backends/skia/typographer_context_skia.cc index 653377315c5c4..88a92f411b962 100644 --- a/impeller/typographer/backends/skia/typographer_context_skia.cc +++ b/impeller/typographer/backends/skia/typographer_context_skia.cc @@ -208,7 +208,7 @@ static void DrawGlyph(SkCanvas* canvas, const ScaledFont& scaled_font, const SubpixelGlyph& glyph, const Rect& scaled_bounds, - const GlyphProperties& prop, + const std::optional& prop, bool has_color) { const auto& metrics = scaled_font.font.GetMetrics(); SkGlyphID glyph_id = glyph.glyph.index; @@ -222,18 +222,20 @@ static void DrawGlyph(SkCanvas* canvas, sk_font.setSubpixel(true); sk_font.setSize(sk_font.getSize() * scaled_font.scale); - auto glyph_color = - has_color ? glyph.properties.color.ToARGB() : SK_ColorBLACK; + // auto glyph_color = + // has_color ? glyph.properties.color.ToARGB() : SK_ColorBLACK; + + auto glyph_color = SK_ColorBLACK; SkPaint glyph_paint; glyph_paint.setColor(glyph_color); glyph_paint.setBlendMode(SkBlendMode::kSrc); - if (prop.stroke) { + if (prop.has_value() && prop->stroke) { glyph_paint.setStroke(true); - glyph_paint.setStrokeWidth(prop.stroke_width * scaled_font.scale); - glyph_paint.setStrokeCap(ToSkiaCap(glyph.properties.stroke_cap)); - glyph_paint.setStrokeJoin(ToSkiaJoin(glyph.properties.stroke_join)); - glyph_paint.setStrokeMiter(prop.stroke_miter * scaled_font.scale); + glyph_paint.setStrokeWidth(prop->stroke_width * scaled_font.scale); + glyph_paint.setStrokeCap(ToSkiaCap(glyph.properties->stroke_cap)); + glyph_paint.setStrokeJoin(ToSkiaJoin(glyph.properties->stroke_join)); + glyph_paint.setStrokeMiter(prop->stroke_miter * scaled_font.scale); } canvas->save(); canvas->translate(glyph.subpixel_offset.x, glyph.subpixel_offset.y); @@ -384,12 +386,12 @@ static Rect ComputeGlyphSize(const SkFont& font, Scalar scale) { SkRect scaled_bounds; SkPaint glyph_paint; - if (glyph.properties.stroke) { + if (glyph.properties.has_value() && glyph.properties->stroke) { glyph_paint.setStroke(true); - glyph_paint.setStrokeWidth(glyph.properties.stroke_width * scale); - glyph_paint.setStrokeCap(ToSkiaCap(glyph.properties.stroke_cap)); - glyph_paint.setStrokeJoin(ToSkiaJoin(glyph.properties.stroke_join)); - glyph_paint.setStrokeMiter(glyph.properties.stroke_miter * scale); + glyph_paint.setStrokeWidth(glyph.properties->stroke_width * scale); + glyph_paint.setStrokeCap(ToSkiaCap(glyph.properties->stroke_cap)); + glyph_paint.setStrokeJoin(ToSkiaJoin(glyph.properties->stroke_join)); + glyph_paint.setStrokeMiter(glyph.properties->stroke_miter * scale); } font.getBounds(&glyph.glyph.index, 1, &scaled_bounds, &glyph_paint); diff --git a/impeller/typographer/font_glyph_pair.h b/impeller/typographer/font_glyph_pair.h index c5d3c508f4994..93d4a5522918a 100644 --- a/impeller/typographer/font_glyph_pair.h +++ b/impeller/typographer/font_glyph_pair.h @@ -5,15 +5,16 @@ #ifndef FLUTTER_IMPELLER_TYPOGRAPHER_FONT_GLYPH_PAIR_H_ #define FLUTTER_IMPELLER_TYPOGRAPHER_FONT_GLYPH_PAIR_H_ -#include -#include - +#include "fml/hash_combine.h" #include "impeller/geometry/color.h" #include "impeller/geometry/path.h" #include "impeller/geometry/point.h" #include "impeller/typographer/font.h" #include "impeller/typographer/glyph.h" +#include "flutter/third_party/abseil-cpp/absl/container/flat_hash_map.h" +#include "flutter/third_party/abseil-cpp/absl/container/flat_hash_set.h" + namespace impeller { struct GlyphProperties { @@ -32,6 +33,19 @@ struct GlyphProperties { struct ScaledFont { Font font; Scalar scale; + + struct Hash { + constexpr std::size_t operator()(const impeller::ScaledFont& sf) const { + return fml::HashCombine(sf.font.GetHash(), sf.scale); + } + }; + + struct Equal { + constexpr bool operator()(const impeller::ScaledFont& lhs, + const impeller::ScaledFont& rhs) const { + return lhs.font.IsEqual(rhs.font) && lhs.scale == rhs.scale; + } + }; }; //------------------------------------------------------------------------------ @@ -40,18 +54,58 @@ struct ScaledFont { struct SubpixelGlyph { Glyph glyph; Point subpixel_offset; - GlyphProperties properties; + std::optional properties; SubpixelGlyph(Glyph p_glyph, Point p_subpixel_offset, - GlyphProperties p_properties) + std::optional p_properties) : glyph(p_glyph), subpixel_offset(p_subpixel_offset), properties(p_properties) {} + + struct Hash { + constexpr std::size_t operator()(const impeller::SubpixelGlyph& sg) const { + if (!sg.properties.has_value()) { + return fml::HashCombine(sg.glyph.index, sg.subpixel_offset.x, + sg.subpixel_offset.y); + } + return fml::HashCombine( + sg.glyph.index, sg.subpixel_offset.x, sg.subpixel_offset.y, + sg.properties->color.ToARGB(), sg.properties->stroke, + sg.properties->stroke_cap, sg.properties->stroke_join, + sg.properties->stroke_miter, sg.properties->stroke_width); + } + }; + + struct Equal { + constexpr bool operator()(const impeller::SubpixelGlyph& lhs, + const impeller::SubpixelGlyph& rhs) const { + if (!lhs.properties.has_value() && !rhs.properties.has_value()) { + return lhs.glyph.index == rhs.glyph.index && + lhs.glyph.type == rhs.glyph.type && + lhs.subpixel_offset == rhs.subpixel_offset; + } + return lhs.glyph.index == rhs.glyph.index && + lhs.glyph.type == rhs.glyph.type && + lhs.subpixel_offset == rhs.subpixel_offset && + lhs.properties.has_value() && rhs.properties.has_value() && + lhs.properties->color.ToARGB() == rhs.properties->color.ToARGB() && + lhs.properties->stroke == rhs.properties->stroke && + lhs.properties->stroke_cap == rhs.properties->stroke_cap && + lhs.properties->stroke_join == rhs.properties->stroke_join && + lhs.properties->stroke_miter == rhs.properties->stroke_miter && + lhs.properties->stroke_width == rhs.properties->stroke_width; + } + }; }; using FontGlyphMap = - std::unordered_map>; + absl::flat_hash_map, + ScaledFont::Hash, + ScaledFont::Equal>; //------------------------------------------------------------------------------ /// @brief A font along with a glyph in that font rendered at a particular @@ -66,46 +120,4 @@ struct FontGlyphPair { } // namespace impeller -template <> -struct std::hash { - constexpr std::size_t operator()(const impeller::ScaledFont& sf) const { - return fml::HashCombine(sf.font.GetHash(), sf.scale); - } -}; - -template <> -struct std::equal_to { - constexpr bool operator()(const impeller::ScaledFont& lhs, - const impeller::ScaledFont& rhs) const { - return lhs.font.IsEqual(rhs.font) && lhs.scale == rhs.scale; - } -}; - -template <> -struct std::hash { - constexpr std::size_t operator()(const impeller::SubpixelGlyph& sg) const { - return fml::HashCombine( - sg.glyph.index, sg.subpixel_offset.x, sg.subpixel_offset.y, - sg.properties.color.ToARGB(), sg.properties.stroke, - sg.properties.stroke_cap, sg.properties.stroke_join, - sg.properties.stroke_miter, sg.properties.stroke_width); - } -}; - -template <> -struct std::equal_to { - constexpr bool operator()(const impeller::SubpixelGlyph& lhs, - const impeller::SubpixelGlyph& rhs) const { - return lhs.glyph.index == rhs.glyph.index && - lhs.glyph.type == rhs.glyph.type && - lhs.subpixel_offset == rhs.subpixel_offset && - lhs.properties.color.ToARGB() == rhs.properties.color.ToARGB() && - lhs.properties.stroke == rhs.properties.stroke && - lhs.properties.stroke_cap == rhs.properties.stroke_cap && - lhs.properties.stroke_join == rhs.properties.stroke_join && - lhs.properties.stroke_miter == rhs.properties.stroke_miter && - lhs.properties.stroke_width == rhs.properties.stroke_width; - } -}; - #endif // FLUTTER_IMPELLER_TYPOGRAPHER_FONT_GLYPH_PAIR_H_ diff --git a/impeller/typographer/glyph_atlas.cc b/impeller/typographer/glyph_atlas.cc index 77b8dfd923933..fa0d0b1e61284 100644 --- a/impeller/typographer/glyph_atlas.cc +++ b/impeller/typographer/glyph_atlas.cc @@ -66,8 +66,8 @@ void GlyphAtlas::SetTexture(std::shared_ptr texture) { void GlyphAtlas::AddTypefaceGlyphPositionAndBounds(const FontGlyphPair& pair, Rect position, Rect bounds) { - font_atlas_map_[pair.scaled_font].positions_[pair.glyph] = - std::make_pair(position, bounds); + auto& x = font_atlas_map_[pair.scaled_font]; + x.positions_[pair.glyph] = std::make_pair(position, bounds); } std::optional> GlyphAtlas::FindFontGlyphBounds( diff --git a/impeller/typographer/glyph_atlas.h b/impeller/typographer/glyph_atlas.h index b94fe90e6a59b..facbd878f848c 100644 --- a/impeller/typographer/glyph_atlas.h +++ b/impeller/typographer/glyph_atlas.h @@ -8,13 +8,15 @@ #include #include #include -#include #include "impeller/core/texture.h" #include "impeller/geometry/rect.h" #include "impeller/typographer/font_glyph_pair.h" #include "impeller/typographer/rectangle_packer.h" +#include "flutter/third_party/abseil-cpp/absl/container/flat_hash_map.h" +#include "flutter/third_party/abseil-cpp/absl/container/node_hash_map.h" + namespace impeller { class FontGlyphAtlas; @@ -136,7 +138,11 @@ class GlyphAtlas { const Type type_; std::shared_ptr texture_; - std::unordered_map font_atlas_map_; + absl::node_hash_map + font_atlas_map_; GlyphAtlas(const GlyphAtlas&) = delete; @@ -214,7 +220,11 @@ class FontGlyphAtlas { private: friend class GlyphAtlas; - std::unordered_map> positions_; + absl::flat_hash_map, + SubpixelGlyph::Hash, + SubpixelGlyph::Equal> + positions_; FontGlyphAtlas(const FontGlyphAtlas&) = delete; diff --git a/impeller/typographer/text_frame.cc b/impeller/typographer/text_frame.cc index a0ce36d683a23..b715616fb4d87 100644 --- a/impeller/typographer/text_frame.cc +++ b/impeller/typographer/text_frame.cc @@ -89,6 +89,10 @@ void TextFrame::CollectUniqueFontGlyphPairs( Scalar scale, Point offset, const GlyphProperties& properties) const { + std::optional lookup = + (properties.stroke || HasColor()) + ? std::optional(properties) + : std::nullopt; for (const TextRun& run : GetRuns()) { const Font& font = run.GetFont(); auto rounded_scale = @@ -98,7 +102,7 @@ void TextFrame::CollectUniqueFontGlyphPairs( run.GetGlyphPositions()) { Point subpixel = ComputeSubpixelPosition( glyph_position, font.GetAxisAlignment(), offset, scale); - set.emplace(glyph_position.glyph, subpixel, properties); + set.emplace(glyph_position.glyph, subpixel, lookup); } } } diff --git a/impeller/typographer/text_run.h b/impeller/typographer/text_run.h index 6bec4d19187a2..76da59ff6fcd6 100644 --- a/impeller/typographer/text_run.h +++ b/impeller/typographer/text_run.h @@ -7,7 +7,7 @@ #include -#include "impeller/geometry/matrix.h" +#include "impeller/geometry/point.h" #include "impeller/typographer/font.h" #include "impeller/typographer/glyph.h" From 8b7eb15bef25f50e6c86a907805f506008d66ef6 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 9 Sep 2024 20:17:15 -0700 Subject: [PATCH 2/4] fix text color. --- .../typographer/backends/skia/typographer_context_skia.cc | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/impeller/typographer/backends/skia/typographer_context_skia.cc b/impeller/typographer/backends/skia/typographer_context_skia.cc index 88a92f411b962..8b3de94c08cb2 100644 --- a/impeller/typographer/backends/skia/typographer_context_skia.cc +++ b/impeller/typographer/backends/skia/typographer_context_skia.cc @@ -222,10 +222,7 @@ static void DrawGlyph(SkCanvas* canvas, sk_font.setSubpixel(true); sk_font.setSize(sk_font.getSize() * scaled_font.scale); - // auto glyph_color = - // has_color ? glyph.properties.color.ToARGB() : SK_ColorBLACK; - - auto glyph_color = SK_ColorBLACK; + auto glyph_color = prop.has_value() ? prop->color.ToARGB() : SK_ColorBLACK; SkPaint glyph_paint; glyph_paint.setColor(glyph_color); From ffcbbc737b1512ea73f5fac55120ef6fa6b45de0 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 9 Sep 2024 21:09:30 -0700 Subject: [PATCH 3/4] ++ --- .../typographer/backends/skia/typographer_context_skia.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/impeller/typographer/backends/skia/typographer_context_skia.cc b/impeller/typographer/backends/skia/typographer_context_skia.cc index 8b3de94c08cb2..a86e2ee6b8b41 100644 --- a/impeller/typographer/backends/skia/typographer_context_skia.cc +++ b/impeller/typographer/backends/skia/typographer_context_skia.cc @@ -230,8 +230,8 @@ static void DrawGlyph(SkCanvas* canvas, if (prop.has_value() && prop->stroke) { glyph_paint.setStroke(true); glyph_paint.setStrokeWidth(prop->stroke_width * scaled_font.scale); - glyph_paint.setStrokeCap(ToSkiaCap(glyph.properties->stroke_cap)); - glyph_paint.setStrokeJoin(ToSkiaJoin(glyph.properties->stroke_join)); + glyph_paint.setStrokeCap(ToSkiaCap(prop->stroke_cap)); + glyph_paint.setStrokeJoin(ToSkiaJoin(prop->stroke_join)); glyph_paint.setStrokeMiter(prop->stroke_miter * scaled_font.scale); } canvas->save(); From 06a695f515995dfccbd95c7ff7fcc5259ced2312 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 10 Sep 2024 14:10:48 -0700 Subject: [PATCH 4/4] remove absl. --- impeller/typographer/BUILD.gn | 6 ------ impeller/typographer/font_glyph_pair.h | 18 +++++++++--------- impeller/typographer/glyph_atlas.cc | 4 ++-- impeller/typographer/glyph_atlas.h | 20 +++++++++----------- 4 files changed, 20 insertions(+), 28 deletions(-) diff --git a/impeller/typographer/BUILD.gn b/impeller/typographer/BUILD.gn index 3db66ab1d27ae..90789a87a4582 100644 --- a/impeller/typographer/BUILD.gn +++ b/impeller/typographer/BUILD.gn @@ -32,12 +32,6 @@ impeller_component("typographer") { "../base", "../geometry", "../renderer", - "//flutter/third_party/abseil-cpp/absl/algorithm", - "//flutter/third_party/abseil-cpp/absl/algorithm:container", - "//flutter/third_party/abseil-cpp/absl/base:config", - "//flutter/third_party/abseil-cpp/absl/container:flat_hash_map", - "//flutter/third_party/abseil-cpp/absl/container:flat_hash_set", - "//flutter/third_party/abseil-cpp/absl/container:node_hash_map", ] deps = [ "//flutter/fml" ] diff --git a/impeller/typographer/font_glyph_pair.h b/impeller/typographer/font_glyph_pair.h index 93d4a5522918a..d4c12fa4195ee 100644 --- a/impeller/typographer/font_glyph_pair.h +++ b/impeller/typographer/font_glyph_pair.h @@ -5,6 +5,9 @@ #ifndef FLUTTER_IMPELLER_TYPOGRAPHER_FONT_GLYPH_PAIR_H_ #define FLUTTER_IMPELLER_TYPOGRAPHER_FONT_GLYPH_PAIR_H_ +#include +#include + #include "fml/hash_combine.h" #include "impeller/geometry/color.h" #include "impeller/geometry/path.h" @@ -12,9 +15,6 @@ #include "impeller/typographer/font.h" #include "impeller/typographer/glyph.h" -#include "flutter/third_party/abseil-cpp/absl/container/flat_hash_map.h" -#include "flutter/third_party/abseil-cpp/absl/container/flat_hash_set.h" - namespace impeller { struct GlyphProperties { @@ -100,12 +100,12 @@ struct SubpixelGlyph { }; using FontGlyphMap = - absl::flat_hash_map, - ScaledFont::Hash, - ScaledFont::Equal>; + std::unordered_map, + ScaledFont::Hash, + ScaledFont::Equal>; //------------------------------------------------------------------------------ /// @brief A font along with a glyph in that font rendered at a particular diff --git a/impeller/typographer/glyph_atlas.cc b/impeller/typographer/glyph_atlas.cc index fa0d0b1e61284..77b8dfd923933 100644 --- a/impeller/typographer/glyph_atlas.cc +++ b/impeller/typographer/glyph_atlas.cc @@ -66,8 +66,8 @@ void GlyphAtlas::SetTexture(std::shared_ptr texture) { void GlyphAtlas::AddTypefaceGlyphPositionAndBounds(const FontGlyphPair& pair, Rect position, Rect bounds) { - auto& x = font_atlas_map_[pair.scaled_font]; - x.positions_[pair.glyph] = std::make_pair(position, bounds); + font_atlas_map_[pair.scaled_font].positions_[pair.glyph] = + std::make_pair(position, bounds); } std::optional> GlyphAtlas::FindFontGlyphBounds( diff --git a/impeller/typographer/glyph_atlas.h b/impeller/typographer/glyph_atlas.h index facbd878f848c..587ece46f045e 100644 --- a/impeller/typographer/glyph_atlas.h +++ b/impeller/typographer/glyph_atlas.h @@ -8,15 +8,13 @@ #include #include #include +#include #include "impeller/core/texture.h" #include "impeller/geometry/rect.h" #include "impeller/typographer/font_glyph_pair.h" #include "impeller/typographer/rectangle_packer.h" -#include "flutter/third_party/abseil-cpp/absl/container/flat_hash_map.h" -#include "flutter/third_party/abseil-cpp/absl/container/node_hash_map.h" - namespace impeller { class FontGlyphAtlas; @@ -138,10 +136,10 @@ class GlyphAtlas { const Type type_; std::shared_ptr texture_; - absl::node_hash_map + std::unordered_map font_atlas_map_; GlyphAtlas(const GlyphAtlas&) = delete; @@ -220,10 +218,10 @@ class FontGlyphAtlas { private: friend class GlyphAtlas; - absl::flat_hash_map, - SubpixelGlyph::Hash, - SubpixelGlyph::Equal> + std::unordered_map, + SubpixelGlyph::Hash, + SubpixelGlyph::Equal> positions_; FontGlyphAtlas(const FontGlyphAtlas&) = delete;