From 324a9f9339b166a4c01563f928694415c5071a17 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 27 Nov 2024 14:58:48 -0800 Subject: [PATCH 01/10] Moved font glyph to flat_hash_map --- impeller/typographer/BUILD.gn | 1 + impeller/typographer/glyph_atlas.cc | 13 ++++++------- impeller/typographer/glyph_atlas.h | 21 +++++++++++---------- 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/impeller/typographer/BUILD.gn b/impeller/typographer/BUILD.gn index acdbeb80a8e4d..eb4a18cc26cd8 100644 --- a/impeller/typographer/BUILD.gn +++ b/impeller/typographer/BUILD.gn @@ -32,6 +32,7 @@ impeller_component("typographer") { "../base", "../geometry", "../renderer", + "//flutter/third_party/abseil-cpp/absl/container:flat_hash_map", ] deps = [ "//flutter/fml" ] diff --git a/impeller/typographer/glyph_atlas.cc b/impeller/typographer/glyph_atlas.cc index 98038668fb501..9d75d6271065c 100644 --- a/impeller/typographer/glyph_atlas.cc +++ b/impeller/typographer/glyph_atlas.cc @@ -78,7 +78,9 @@ void GlyphAtlas::SetAtlasGeneration(size_t generation) { void GlyphAtlas::AddTypefaceGlyphPositionAndBounds(const FontGlyphPair& pair, Rect position, Rect bounds) { - font_atlas_map_[pair.scaled_font].positions_[pair.glyph] = + FontAtlasMap::iterator it = font_atlas_map_.find(pair.scaled_font); + FML_DCHECK(it != font_atlas_map_.end()); + it->second.positions_[pair.glyph] = FrameBounds{position, bounds, /*is_placeholder=*/false}; } @@ -93,12 +95,9 @@ std::optional GlyphAtlas::FindFontGlyphBounds( FontGlyphAtlas* GlyphAtlas::GetOrCreateFontGlyphAtlas( const ScaledFont& scaled_font) { - const auto& found = font_atlas_map_.find(scaled_font); - if (found != font_atlas_map_.end()) { - return &found->second; - } - font_atlas_map_[scaled_font] = FontGlyphAtlas(); - return &font_atlas_map_[scaled_font]; + auto [iter, inserted] = + font_atlas_map_.try_emplace(scaled_font, FontGlyphAtlas()); + return &iter->second; } size_t GlyphAtlas::GetGlyphCount() const { diff --git a/impeller/typographer/glyph_atlas.h b/impeller/typographer/glyph_atlas.h index bf11ff056ae24..2ca398682a0d9 100644 --- a/impeller/typographer/glyph_atlas.h +++ b/impeller/typographer/glyph_atlas.h @@ -8,8 +8,8 @@ #include #include #include -#include +#include "flutter/third_party/abseil-cpp/absl/container/flat_hash_map.h" #include "impeller/core/texture.h" #include "impeller/geometry/rect.h" #include "impeller/typographer/font_glyph_pair.h" @@ -160,11 +160,11 @@ class GlyphAtlas { std::shared_ptr texture_; size_t generation_ = 0; - std::unordered_map - font_atlas_map_; + using FontAtlasMap = absl::flat_hash_map; + FontAtlasMap font_atlas_map_; GlyphAtlas(const GlyphAtlas&) = delete; @@ -228,6 +228,7 @@ class GlyphAtlasContext { class FontGlyphAtlas { public: FontGlyphAtlas() = default; + FontGlyphAtlas(FontGlyphAtlas&&) = default; //---------------------------------------------------------------------------- /// @brief Find the location of a glyph in the atlas. @@ -249,10 +250,10 @@ class FontGlyphAtlas { private: friend class GlyphAtlas; - std::unordered_map + absl::flat_hash_map positions_; FontGlyphAtlas(const FontGlyphAtlas&) = delete; From 250cadc0cf19e442c7f7fca5158c94fe7deb2622 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 27 Nov 2024 16:34:08 -0800 Subject: [PATCH 02/10] moved to absl hash --- impeller/typographer/BUILD.gn | 1 + impeller/typographer/font_glyph_pair.h | 32 ++++++++++++-------------- impeller/typographer/glyph_atlas.h | 5 ++-- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/impeller/typographer/BUILD.gn b/impeller/typographer/BUILD.gn index eb4a18cc26cd8..3689891ffc0d9 100644 --- a/impeller/typographer/BUILD.gn +++ b/impeller/typographer/BUILD.gn @@ -33,6 +33,7 @@ impeller_component("typographer") { "../geometry", "../renderer", "//flutter/third_party/abseil-cpp/absl/container:flat_hash_map", + "//flutter/third_party/abseil-cpp/absl/hash:hash", ] deps = [ "//flutter/fml" ] diff --git a/impeller/typographer/font_glyph_pair.h b/impeller/typographer/font_glyph_pair.h index e3df2dae50c11..fbefb59b968da 100644 --- a/impeller/typographer/font_glyph_pair.h +++ b/impeller/typographer/font_glyph_pair.h @@ -41,11 +41,10 @@ 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); - } - }; + template + friend H AbslHashValue(H h, const ScaledFont& sf) { + return H::combine(std::move(h), sf.font.GetHash(), sf.scale); + } struct Equal { constexpr bool operator()(const impeller::ScaledFont& lhs, @@ -70,19 +69,18 @@ struct SubpixelGlyph { 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); + template + friend H AbslHashValue(H h, const SubpixelGlyph& sg) { + if (!sg.properties.has_value()) { + return H::combine(std::move(h), sg.glyph.index, sg.subpixel_offset.x, + sg.subpixel_offset.y); } - }; + return H::combine(std::move(h), 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, diff --git a/impeller/typographer/glyph_atlas.h b/impeller/typographer/glyph_atlas.h index 2ca398682a0d9..ec2e842502a66 100644 --- a/impeller/typographer/glyph_atlas.h +++ b/impeller/typographer/glyph_atlas.h @@ -10,6 +10,7 @@ #include #include "flutter/third_party/abseil-cpp/absl/container/flat_hash_map.h" +#include "flutter/third_party/abseil-cpp/absl/hash/hash.h" #include "impeller/core/texture.h" #include "impeller/geometry/rect.h" #include "impeller/typographer/font_glyph_pair.h" @@ -162,7 +163,7 @@ class GlyphAtlas { using FontAtlasMap = absl::flat_hash_map, ScaledFont::Equal>; FontAtlasMap font_atlas_map_; @@ -252,7 +253,7 @@ class FontGlyphAtlas { absl::flat_hash_map, SubpixelGlyph::Equal> positions_; From 483effec63b742e51fd0f8a4feb6a480c0d0873e Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 2 Dec 2024 11:50:26 -0800 Subject: [PATCH 03/10] updated abseil --- DEPS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DEPS b/DEPS index e9f7b62c8d6e7..d8600a100b57c 100644 --- a/DEPS +++ b/DEPS @@ -593,7 +593,7 @@ deps = { Var('chromium_git') + '/external/github.com/GPUOpen-LibrariesAndSDKs/VulkanMemoryAllocator' + '@' + '7de5cc00de50e71a3aab22dea52fbb7ff4efceb6', 'src/flutter/third_party/abseil-cpp': - Var('flutter_git') + '/third_party/abseil-cpp.git' + '@' + 'ff6504dc527b25fef0f3c531e7dba0ed6b69c162', + Var('flutter_git') + '/third_party/abseil-cpp.git' + '@' + '599954e7ca3c86dcfd1ed6c809b323e59d86ceaf', # Dart packages 'src/flutter/third_party/pkg/archive': From 5d0bbca935f206221d7e6ec7e6842a565322c7ef Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 2 Dec 2024 13:15:27 -0800 Subject: [PATCH 04/10] Revert "updated abseil" This reverts commit 483effec63b742e51fd0f8a4feb6a480c0d0873e. --- DEPS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DEPS b/DEPS index d8600a100b57c..e9f7b62c8d6e7 100644 --- a/DEPS +++ b/DEPS @@ -593,7 +593,7 @@ deps = { Var('chromium_git') + '/external/github.com/GPUOpen-LibrariesAndSDKs/VulkanMemoryAllocator' + '@' + '7de5cc00de50e71a3aab22dea52fbb7ff4efceb6', 'src/flutter/third_party/abseil-cpp': - Var('flutter_git') + '/third_party/abseil-cpp.git' + '@' + '599954e7ca3c86dcfd1ed6c809b323e59d86ceaf', + Var('flutter_git') + '/third_party/abseil-cpp.git' + '@' + 'ff6504dc527b25fef0f3c531e7dba0ed6b69c162', # Dart packages 'src/flutter/third_party/pkg/archive': From 20687885992bdc98a69d37c157b5d5926682f5e9 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 2 Dec 2024 13:18:05 -0800 Subject: [PATCH 05/10] removed hash dependency --- impeller/typographer/BUILD.gn | 1 - impeller/typographer/glyph_atlas.h | 1 - 2 files changed, 2 deletions(-) diff --git a/impeller/typographer/BUILD.gn b/impeller/typographer/BUILD.gn index 3689891ffc0d9..eb4a18cc26cd8 100644 --- a/impeller/typographer/BUILD.gn +++ b/impeller/typographer/BUILD.gn @@ -33,7 +33,6 @@ impeller_component("typographer") { "../geometry", "../renderer", "//flutter/third_party/abseil-cpp/absl/container:flat_hash_map", - "//flutter/third_party/abseil-cpp/absl/hash:hash", ] deps = [ "//flutter/fml" ] diff --git a/impeller/typographer/glyph_atlas.h b/impeller/typographer/glyph_atlas.h index ec2e842502a66..f9b7baef8d7ac 100644 --- a/impeller/typographer/glyph_atlas.h +++ b/impeller/typographer/glyph_atlas.h @@ -10,7 +10,6 @@ #include #include "flutter/third_party/abseil-cpp/absl/container/flat_hash_map.h" -#include "flutter/third_party/abseil-cpp/absl/hash/hash.h" #include "impeller/core/texture.h" #include "impeller/geometry/rect.h" #include "impeller/typographer/font_glyph_pair.h" From f6a9a0388822257b19ef49ef3d6abc7e549b4e47 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 2 Dec 2024 13:41:04 -0800 Subject: [PATCH 06/10] try my abseil branch --- DEPS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DEPS b/DEPS index e9f7b62c8d6e7..c1f8d5979f1f3 100644 --- a/DEPS +++ b/DEPS @@ -593,7 +593,7 @@ deps = { Var('chromium_git') + '/external/github.com/GPUOpen-LibrariesAndSDKs/VulkanMemoryAllocator' + '@' + '7de5cc00de50e71a3aab22dea52fbb7ff4efceb6', 'src/flutter/third_party/abseil-cpp': - Var('flutter_git') + '/third_party/abseil-cpp.git' + '@' + 'ff6504dc527b25fef0f3c531e7dba0ed6b69c162', + 'https://github.com/gaaclarke/flutter-abseil-cpp.git' + '@' + '59efcc9ce43c60f9897f3073463eb58ed299c7d4', # Dart packages 'src/flutter/third_party/pkg/archive': From c420b45232a2043c15b4296a4d5dcf3da02babe4 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 2 Dec 2024 14:04:01 -0800 Subject: [PATCH 07/10] Revert "try my abseil branch" This reverts commit f6a9a0388822257b19ef49ef3d6abc7e549b4e47. --- DEPS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DEPS b/DEPS index c1f8d5979f1f3..e9f7b62c8d6e7 100644 --- a/DEPS +++ b/DEPS @@ -593,7 +593,7 @@ deps = { Var('chromium_git') + '/external/github.com/GPUOpen-LibrariesAndSDKs/VulkanMemoryAllocator' + '@' + '7de5cc00de50e71a3aab22dea52fbb7ff4efceb6', 'src/flutter/third_party/abseil-cpp': - 'https://github.com/gaaclarke/flutter-abseil-cpp.git' + '@' + '59efcc9ce43c60f9897f3073463eb58ed299c7d4', + Var('flutter_git') + '/third_party/abseil-cpp.git' + '@' + 'ff6504dc527b25fef0f3c531e7dba0ed6b69c162', # Dart packages 'src/flutter/third_party/pkg/archive': From bb3fc1a9b3d4046d556f67355cc3fa70aa9415b3 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 2 Dec 2024 14:12:03 -0800 Subject: [PATCH 08/10] ifdef'd out absl --- impeller/typographer/BUILD.gn | 7 ++++++- impeller/typographer/glyph_atlas.h | 23 +++++++++++++++++------ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/impeller/typographer/BUILD.gn b/impeller/typographer/BUILD.gn index eb4a18cc26cd8..d8aa733f63641 100644 --- a/impeller/typographer/BUILD.gn +++ b/impeller/typographer/BUILD.gn @@ -32,9 +32,14 @@ impeller_component("typographer") { "../base", "../geometry", "../renderer", - "//flutter/third_party/abseil-cpp/absl/container:flat_hash_map", ] + if (!is_fuchsia) { + public_deps += [ + "//flutter/third_party/abseil-cpp/absl/container:flat_hash_map", + ] + } + deps = [ "//flutter/fml" ] } diff --git a/impeller/typographer/glyph_atlas.h b/impeller/typographer/glyph_atlas.h index f9b7baef8d7ac..5a524aeae8116 100644 --- a/impeller/typographer/glyph_atlas.h +++ b/impeller/typographer/glyph_atlas.h @@ -9,7 +9,9 @@ #include #include +#if !defined(OS_FUCHSIA) #include "flutter/third_party/abseil-cpp/absl/container/flat_hash_map.h" +#endif #include "impeller/core/texture.h" #include "impeller/geometry/rect.h" #include "impeller/typographer/font_glyph_pair.h" @@ -250,12 +252,21 @@ class FontGlyphAtlas { private: friend class GlyphAtlas; - absl::flat_hash_map, - SubpixelGlyph::Equal> - positions_; - +#if defined(OS_FUCHSIA) + // TODO(gaaclarke): Migrate to use absl. I couldn't get it working since absl + // has special logic in its GN files for Fuchsia that I couldn't sort out. + using PositionsMap = absl::flat_hash_map, + SubpixelGlyph::Equal>; +#else + using PositionsMap = absl::flat_hash_map, + SubpixelGlyph::Equal>; +#endif + + PositionsMap positions_; FontGlyphAtlas(const FontGlyphAtlas&) = delete; }; From f17c2be9821b811fadae9cba80b8bbaa7b35b55d Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 2 Dec 2024 16:24:25 -0800 Subject: [PATCH 09/10] ifdef'd absl for fuchsia --- impeller/typographer/BUILD.gn | 5 ++- impeller/typographer/glyph_atlas.h | 51 +++++++++++++++++++++++++----- 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/impeller/typographer/BUILD.gn b/impeller/typographer/BUILD.gn index d8aa733f63641..3470c3d0bead5 100644 --- a/impeller/typographer/BUILD.gn +++ b/impeller/typographer/BUILD.gn @@ -35,9 +35,8 @@ impeller_component("typographer") { ] if (!is_fuchsia) { - public_deps += [ - "//flutter/third_party/abseil-cpp/absl/container:flat_hash_map", - ] + public_deps += + [ "//flutter/third_party/abseil-cpp/absl/container:flat_hash_map" ] } deps = [ "//flutter/fml" ] diff --git a/impeller/typographer/glyph_atlas.h b/impeller/typographer/glyph_atlas.h index 5a524aeae8116..6818b5d9a8197 100644 --- a/impeller/typographer/glyph_atlas.h +++ b/impeller/typographer/glyph_atlas.h @@ -9,9 +9,14 @@ #include #include -#if !defined(OS_FUCHSIA) +#if defined(OS_FUCHSIA) +// TODO(gaaclarke): Migrate to use absl. I couldn't get it working since absl +// has special logic in its GN files for Fuchsia that I couldn't sort out. +#define IMPELLER_TYPOGRAPHER_USE_STD_HASH +#else #include "flutter/third_party/abseil-cpp/absl/container/flat_hash_map.h" #endif + #include "impeller/core/texture.h" #include "impeller/geometry/rect.h" #include "impeller/typographer/font_glyph_pair.h" @@ -21,6 +26,30 @@ namespace impeller { class FontGlyphAtlas; +/// Helper for AbslHashAdapter. Tallies a hash value with fml::HashCombine. +template +struct AbslHashAdapterCombiner { + std::size_t value = 0; + + template + static AbslHashAdapterCombiner combine(AbslHashAdapterCombiner combiner, + const Args&... args) { + combiner.value = fml::HashCombine(combiner.value, args...); + return combiner; + } +}; + +/// Adapts AbslHashValue functions to be used with std::unordered_map and the +/// fml hash functions. +template +struct AbslHashAdapter { + constexpr std::size_t operator()(const T& element) const { + AbslHashAdapterCombiner combiner; + combiner = AbslHashValue(std::move(combiner), element); + return combiner.value; + } +}; + struct FrameBounds { /// The bounds of the glyph within the glyph atlas. Rect atlas_bounds; @@ -162,10 +191,18 @@ class GlyphAtlas { std::shared_ptr texture_; size_t generation_ = 0; +#if defined(IMPELLER_TYPOGRAPHER_USE_STD_HASH) + using FontAtlasMap = std::unordered_map, + ScaledFont::Equal>; +#else using FontAtlasMap = absl::flat_hash_map, ScaledFont::Equal>; +#endif + FontAtlasMap font_atlas_map_; GlyphAtlas(const GlyphAtlas&) = delete; @@ -252,13 +289,11 @@ class FontGlyphAtlas { private: friend class GlyphAtlas; -#if defined(OS_FUCHSIA) - // TODO(gaaclarke): Migrate to use absl. I couldn't get it working since absl - // has special logic in its GN files for Fuchsia that I couldn't sort out. - using PositionsMap = absl::flat_hash_map, - SubpixelGlyph::Equal>; +#if defined(IMPELLER_TYPOGRAPHER_USE_STD_HASH) + using PositionsMap = std::unordered_map, + SubpixelGlyph::Equal>; #else using PositionsMap = absl::flat_hash_map Date: Mon, 2 Dec 2024 16:49:00 -0800 Subject: [PATCH 10/10] added build_config include --- impeller/typographer/glyph_atlas.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/impeller/typographer/glyph_atlas.h b/impeller/typographer/glyph_atlas.h index 6818b5d9a8197..de158bae31c73 100644 --- a/impeller/typographer/glyph_atlas.h +++ b/impeller/typographer/glyph_atlas.h @@ -9,6 +9,8 @@ #include #include +#include "flutter/fml/build_config.h" + #if defined(OS_FUCHSIA) // TODO(gaaclarke): Migrate to use absl. I couldn't get it working since absl // has special logic in its GN files for Fuchsia that I couldn't sort out.