diff --git a/engine/src/flutter/impeller/display_list/dl_dispatcher.cc b/engine/src/flutter/impeller/display_list/dl_dispatcher.cc index 67bca9dbd03..3ecebfc3522 100644 --- a/engine/src/flutter/impeller/display_list/dl_dispatcher.cc +++ b/engine/src/flutter/impeller/display_list/dl_dispatcher.cc @@ -1458,13 +1458,10 @@ void TextFrameDispatcher::drawTextFrame( } auto scale = (matrix_ * Matrix::MakeTranslation(Point(x, y))).GetMaxBasisLengthXY(); - renderer_.GetLazyGlyphAtlas()->AddTextFrame( - text_frame, // - scale, // - Point(x, y), // - (properties.stroke || text_frame->HasColor()) // - ? std::optional(properties) // - : std::nullopt // + renderer_.GetLazyGlyphAtlas()->AddTextFrame(*text_frame, // + scale, // + Point(x, y), // + properties // ); } diff --git a/engine/src/flutter/impeller/entity/contents/text_contents.cc b/engine/src/flutter/impeller/entity/contents/text_contents.cc index 687b2abc407..3c5db239301 100644 --- a/engine/src/flutter/impeller/entity/contents/text_contents.cc +++ b/engine/src/flutter/impeller/entity/contents/text_contents.cc @@ -90,10 +90,6 @@ bool TextContents::Render(const ContentContext& renderer, VALIDATION_LOG << "Cannot render glyphs without prepared atlas."; return false; } - if (!frame_->IsFrameComplete()) { - VALIDATION_LOG << "Failed to find font glyph bounds."; - return false; - } // Information shared by all glyph draw calls. pass.SetCommandLabel("TextFrame"); @@ -173,12 +169,16 @@ bool TextContents::Render(const ContentContext& renderer, VS::PerVertexData* vtx_contents = reinterpret_cast(contents); size_t i = 0u; - size_t bounds_offset = 0u; for (const TextRun& run : frame_->GetRuns()) { const Font& font = run.GetFont(); Scalar rounded_scale = TextFrame::RoundScaledFontSize( scale_, font.GetMetrics().point_size); - FontGlyphAtlas* font_atlas = nullptr; + const FontGlyphAtlas* font_atlas = + atlas->GetFontGlyphAtlas(font, rounded_scale); + if (!font_atlas) { + VALIDATION_LOG << "Could not find font in the atlas."; + continue; + } // Adjust glyph position based on the subpixel rounding // used by the font. @@ -201,45 +201,22 @@ bool TextContents::Render(const ContentContext& renderer, Point screen_offset = (entity_transform * Point(0, 0)); for (const TextRun::GlyphPosition& glyph_position : run.GetGlyphPositions()) { - const FrameBounds& frame_bounds = - frame_->GetFrameBounds(bounds_offset); - bounds_offset++; - auto atlas_glyph_bounds = frame_bounds.atlas_bounds; - auto glyph_bounds = frame_bounds.glyph_bounds; - - // If frame_bounds.is_placeholder is true, this is the first frame - // the glyph has been rendered and so its atlas position was not - // known when the glyph was recorded. Perform a slow lookup into the - // glyph atlas hash table. - if (frame_bounds.is_placeholder) { - // Note: uses unrounded scale for more accurate subpixel position. - if (!font_atlas) { - font_atlas = atlas->GetOrCreateFontGlyphAtlas( - ScaledFont{font, rounded_scale}); - } - - if (!font_atlas) { - VALIDATION_LOG << "Could not find font in the atlas."; - continue; - } - // Note: uses unrounded scale for more accurate subpixel position. - Point subpixel = TextFrame::ComputeSubpixelPosition( - glyph_position, font.GetAxisAlignment(), offset_, scale_); - - std::optional maybe_atlas_glyph_bounds = - font_atlas->FindGlyphBounds(SubpixelGlyph{ - glyph_position.glyph, // - subpixel, // - GetGlyphProperties() // - }); - if (!maybe_atlas_glyph_bounds.has_value()) { - VALIDATION_LOG << "Could not find glyph position in the atlas."; - continue; - } - atlas_glyph_bounds = - maybe_atlas_glyph_bounds.value().atlas_bounds; + // Note: uses unrounded scale for more accurate subpixel position. + 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_.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; } - + const Rect& atlas_glyph_bounds = + maybe_atlas_glyph_bounds.value().first; + Rect glyph_bounds = maybe_atlas_glyph_bounds.value().second; Rect scaled_bounds = glyph_bounds.Scale(1.0 / rounded_scale); // For each glyph, we compute two rectangles. One for the vertex // positions and one for the texture coordinates (UVs). The atlas @@ -289,10 +266,4 @@ bool TextContents::Render(const ContentContext& renderer, return pass.Draw().ok(); } -std::optional TextContents::GetGlyphProperties() const { - return (properties_.stroke || frame_->HasColor()) - ? std::optional(properties_) - : std::nullopt; -} - } // namespace impeller diff --git a/engine/src/flutter/impeller/entity/contents/text_contents.h b/engine/src/flutter/impeller/entity/contents/text_contents.h index 091da3d7488..f379f1836e3 100644 --- a/engine/src/flutter/impeller/entity/contents/text_contents.h +++ b/engine/src/flutter/impeller/entity/contents/text_contents.h @@ -62,8 +62,6 @@ class TextContents final : public Contents { RenderPass& pass) const override; private: - std::optional GetGlyphProperties() const; - std::shared_ptr frame_; Scalar scale_ = 1.0; Scalar inherited_opacity_ = 1.0; diff --git a/engine/src/flutter/impeller/typographer/backends/skia/typographer_context_skia.cc b/engine/src/flutter/impeller/typographer/backends/skia/typographer_context_skia.cc index 0b029299188..a86e2ee6b8b 100644 --- a/engine/src/flutter/impeller/typographer/backends/skia/typographer_context_skia.cc +++ b/engine/src/flutter/impeller/typographer/backends/skia/typographer_context_skia.cc @@ -282,8 +282,7 @@ static bool BulkUpdateAtlasBitmap(const GlyphAtlas& atlas, if (!data.has_value()) { continue; } - auto [pos, bounds, placeholder] = data.value(); - FML_DCHECK(!placeholder); + auto [pos, bounds] = data.value(); Size size = pos.GetSize(); if (size.IsEmpty()) { continue; @@ -326,9 +325,7 @@ static bool UpdateAtlasBitmap(const GlyphAtlas& atlas, if (!data.has_value()) { continue; } - auto [pos, bounds, placeholder] = data.value(); - FML_DCHECK(!placeholder); - + auto [pos, bounds] = data.value(); Size size = pos.GetSize(); if (size.IsEmpty()) { continue; @@ -405,72 +402,45 @@ static Rect ComputeGlyphSize(const SkFont& font, scaled_bounds.fBottom); }; -std::pair, std::vector> -TypographerContextSkia::CollectNewGlyphs( - const std::shared_ptr& atlas, - const std::vector>& text_frames) { - std::vector new_glyphs; - std::vector glyph_sizes; - for (const auto& frame : text_frames) { - // TODO(jonahwilliams): unless we destroy the atlas (which we know about), - // we could probably guarantee that a text frame that is complete does not - // need to be processed unless the scale or properties changed. I'm leaving - // this as a future optimization. - frame->ClearFrameBounds(); +static void CollectNewGlyphs(const std::shared_ptr& atlas, + const FontGlyphMap& font_glyph_map, + std::vector& new_glyphs, + std::vector& glyph_sizes) { + for (const auto& font_value : font_glyph_map) { + const ScaledFont& scaled_font = font_value.first; + const FontGlyphAtlas* font_glyph_atlas = + atlas->GetFontGlyphAtlas(scaled_font.font, scaled_font.scale); - for (const auto& run : frame->GetRuns()) { - auto metrics = run.GetFont().GetMetrics(); + auto metrics = scaled_font.font.GetMetrics(); - auto rounded_scale = - TextFrame::RoundScaledFontSize(frame->GetScale(), metrics.point_size); - ScaledFont scaled_font{.font = run.GetFont(), .scale = rounded_scale}; + SkFont sk_font( + TypefaceSkia::Cast(*scaled_font.font.GetTypeface()).GetSkiaTypeface(), + metrics.point_size, metrics.scaleX, metrics.skewX); + sk_font.setEdging(SkFont::Edging::kAntiAlias); + sk_font.setHinting(SkFontHinting::kSlight); + sk_font.setEmbolden(metrics.embolden); + // Rather than computing the bounds at the requested point size and scaling + // up the bounds, we scale up the font size and request the bounds. This + // seems to give more accurate bounds information. + sk_font.setSize(sk_font.getSize() * scaled_font.scale); + sk_font.setSubpixel(true); - FontGlyphAtlas* font_glyph_atlas = - atlas->GetOrCreateFontGlyphAtlas(scaled_font); - FML_DCHECK(!!font_glyph_atlas); - - SkFont sk_font( - TypefaceSkia::Cast(*scaled_font.font.GetTypeface()).GetSkiaTypeface(), - metrics.point_size, metrics.scaleX, metrics.skewX); - sk_font.setEdging(SkFont::Edging::kAntiAlias); - sk_font.setHinting(SkFontHinting::kSlight); - sk_font.setEmbolden(metrics.embolden); - // Rather than computing the bounds at the requested point size and - // scaling up the bounds, we scale up the font size and request the - // bounds. This seems to give more accurate bounds information. - sk_font.setSize(sk_font.getSize() * scaled_font.scale); - sk_font.setSubpixel(true); - - for (const auto& glyph_position : run.GetGlyphPositions()) { - Point subpixel = TextFrame::ComputeSubpixelPosition( - glyph_position, scaled_font.font.GetAxisAlignment(), - frame->GetOffset(), rounded_scale); - SubpixelGlyph subpixel_glyph(glyph_position.glyph, subpixel, - frame->GetProperties()); - const auto& font_glyph_bounds = - font_glyph_atlas->FindGlyphBounds(subpixel_glyph); - - if (!font_glyph_bounds.has_value()) { - new_glyphs.push_back(FontGlyphPair{scaled_font, subpixel_glyph}); - auto glyph_bounds = - ComputeGlyphSize(sk_font, subpixel_glyph, scaled_font.scale); - glyph_sizes.push_back(glyph_bounds); - - auto frame_bounds = FrameBounds{ - Rect::MakeLTRB(0, 0, 0, 0), // - glyph_bounds, // - /*placeholder=*/true // - }; - - frame->AppendFrameBounds(frame_bounds); - font_glyph_atlas->AppendGlyph(subpixel_glyph, frame_bounds); - } else { - frame->AppendFrameBounds(font_glyph_bounds.value()); + if (font_glyph_atlas) { + for (const SubpixelGlyph& glyph : font_value.second) { + if (!font_glyph_atlas->FindGlyphBounds(glyph)) { + new_glyphs.emplace_back(scaled_font, glyph); + glyph_sizes.push_back( + ComputeGlyphSize(sk_font, glyph, scaled_font.scale)); } } + } else { + for (const SubpixelGlyph& glyph : font_value.second) { + new_glyphs.emplace_back(scaled_font, glyph); + glyph_sizes.push_back( + ComputeGlyphSize(sk_font, glyph, scaled_font.scale)); + } } } - return {std::move(new_glyphs), std::move(glyph_sizes)}; } std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( @@ -478,7 +448,7 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( GlyphAtlas::Type type, HostBuffer& host_buffer, const std::shared_ptr& atlas_context, - const std::vector>& text_frames) const { + const FontGlyphMap& font_glyph_map) const { TRACE_EVENT0("impeller", __FUNCTION__); if (!IsValid()) { return nullptr; @@ -486,7 +456,7 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( std::shared_ptr last_atlas = atlas_context->GetGlyphAtlas(); FML_DCHECK(last_atlas->GetType() == type); - if (text_frames.empty()) { + if (font_glyph_map.empty()) { return last_atlas; } @@ -495,7 +465,9 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( // with the current atlas and reuse if possible. For each new font and // glyph pair, compute the glyph size at scale. // --------------------------------------------------------------------------- - auto [new_glyphs, glyph_sizes] = CollectNewGlyphs(last_atlas, text_frames); + std::vector new_glyphs; + std::vector glyph_sizes; + CollectNewGlyphs(last_atlas, font_glyph_map, new_glyphs, glyph_sizes); if (new_glyphs.size() == 0) { return last_atlas; } @@ -564,11 +536,9 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( blit_old_atlas = false; new_atlas = std::make_shared(type); - auto [update_glyphs, update_sizes] = - CollectNewGlyphs(new_atlas, text_frames); - new_glyphs = std::move(update_glyphs); - glyph_sizes = std::move(update_sizes); - + new_glyphs.clear(); + glyph_sizes.clear(); + CollectNewGlyphs(new_atlas, font_glyph_map, new_glyphs, glyph_sizes); glyph_positions.clear(); glyph_positions.reserve(new_glyphs.size()); first_missing_index = 0; diff --git a/engine/src/flutter/impeller/typographer/backends/skia/typographer_context_skia.h b/engine/src/flutter/impeller/typographer/backends/skia/typographer_context_skia.h index 181839df9d4..c2500874061 100644 --- a/engine/src/flutter/impeller/typographer/backends/skia/typographer_context_skia.h +++ b/engine/src/flutter/impeller/typographer/backends/skia/typographer_context_skia.h @@ -27,14 +27,9 @@ class TypographerContextSkia : public TypographerContext { GlyphAtlas::Type type, HostBuffer& host_buffer, const std::shared_ptr& atlas_context, - const std::vector>& text_frames) - const override; + const FontGlyphMap& font_glyph_map) const override; private: - static std::pair, std::vector> - CollectNewGlyphs(const std::shared_ptr& atlas, - const std::vector>& text_frames); - TypographerContextSkia(const TypographerContextSkia&) = delete; TypographerContextSkia& operator=(const TypographerContextSkia&) = delete; diff --git a/engine/src/flutter/impeller/typographer/backends/stb/typographer_context_stb.cc b/engine/src/flutter/impeller/typographer/backends/stb/typographer_context_stb.cc index 674b0f44f80..2c8fa2dcc68 100644 --- a/engine/src/flutter/impeller/typographer/backends/stb/typographer_context_stb.cc +++ b/engine/src/flutter/impeller/typographer/backends/stb/typographer_context_stb.cc @@ -283,7 +283,7 @@ static bool UpdateAtlasBitmap(const GlyphAtlas& atlas, continue; } DrawGlyph(bitmap.get(), pair.scaled_font, pair.glyph.glyph, - pos->atlas_bounds, has_color); + pos.value().first, has_color); } return true; } @@ -396,62 +396,12 @@ static Rect ComputeGlyphSize(const ScaledFont& font, return Rect::MakeLTRB(0, 0, x1 - x0, y1 - y0); } -void TypographerContextSTB::CollectNewGlyphs( - const std::shared_ptr& atlas, - const std::vector>& text_frames, - std::vector& new_glyphs, - std::vector& glyph_sizes) { - for (const auto& frame : text_frames) { - frame->ClearFrameBounds(); - - for (const auto& run : frame->GetRuns()) { - auto metrics = run.GetFont().GetMetrics(); - - auto rounded_scale = - TextFrame::RoundScaledFontSize(frame->GetScale(), metrics.point_size); - ScaledFont scaled_font{.font = run.GetFont(), .scale = rounded_scale}; - - FontGlyphAtlas* font_glyph_atlas = - atlas->GetOrCreateFontGlyphAtlas(scaled_font); - - for (const auto& glyph_position : run.GetGlyphPositions()) { - Point subpixel = TextFrame::ComputeSubpixelPosition( - glyph_position, scaled_font.font.GetAxisAlignment(), - frame->GetOffset(), rounded_scale); - SubpixelGlyph subpixel_glyph(glyph_position.glyph, // - subpixel, // - frame->GetProperties() // - ); - const auto& font_glyph_bounds = - font_glyph_atlas->FindGlyphBounds(subpixel_glyph); - - if (!font_glyph_bounds.has_value()) { - new_glyphs.push_back(FontGlyphPair{scaled_font, subpixel_glyph}); - auto glyph_bounds = ComputeGlyphSize(scaled_font, subpixel_glyph); - glyph_sizes.push_back(glyph_bounds); - - auto frame_bounds = FrameBounds{ - Rect::MakeLTRB(0, 0, 0, 0), // - glyph_bounds, // - /*placeholder=*/true // - }; - - frame->AppendFrameBounds(frame_bounds); - font_glyph_atlas->AppendGlyph(subpixel_glyph, frame_bounds); - } else { - frame->AppendFrameBounds(font_glyph_bounds.value()); - } - } - } - } -} - std::shared_ptr TypographerContextSTB::CreateGlyphAtlas( Context& context, GlyphAtlas::Type type, HostBuffer& host_buffer, const std::shared_ptr& atlas_context, - const std::vector>& text_frames) const { + const FontGlyphMap& font_glyph_map) const { TRACE_EVENT0("impeller", __FUNCTION__); if (!IsValid()) { return nullptr; @@ -459,7 +409,7 @@ std::shared_ptr TypographerContextSTB::CreateGlyphAtlas( auto& atlas_context_stb = GlyphAtlasContextSTB::Cast(*atlas_context); std::shared_ptr last_atlas = atlas_context->GetGlyphAtlas(); - if (text_frames.empty()) { + if (font_glyph_map.empty()) { return last_atlas; } @@ -469,7 +419,24 @@ std::shared_ptr TypographerContextSTB::CreateGlyphAtlas( // --------------------------------------------------------------------------- std::vector new_glyphs; std::vector new_sizes; - CollectNewGlyphs(last_atlas, text_frames, new_glyphs, new_sizes); + for (const auto& font_value : font_glyph_map) { + const ScaledFont& scaled_font = font_value.first; + const FontGlyphAtlas* font_glyph_atlas = + last_atlas->GetFontGlyphAtlas(scaled_font.font, scaled_font.scale); + if (font_glyph_atlas) { + for (const SubpixelGlyph& glyph : font_value.second) { + if (!font_glyph_atlas->FindGlyphBounds(glyph)) { + new_glyphs.emplace_back(scaled_font, glyph); + new_sizes.push_back(ComputeGlyphSize(scaled_font, glyph)); + } + } + } else { + for (const SubpixelGlyph& glyph : font_value.second) { + new_glyphs.emplace_back(scaled_font, glyph); + new_sizes.push_back(ComputeGlyphSize(scaled_font, glyph)); + } + } + } if (last_atlas->GetType() == type && new_glyphs.size() == 0) { return last_atlas; @@ -527,15 +494,19 @@ std::shared_ptr TypographerContextSTB::CreateGlyphAtlas( // --------------------------------------------------------------------------- // Step 3b: Get the optimum size of the texture atlas. // --------------------------------------------------------------------------- - new_sizes.clear(); - new_glyphs.clear(); - glyph_positions.clear(); - + std::vector font_glyph_pairs; + font_glyph_pairs.reserve(std::accumulate( + font_glyph_map.begin(), font_glyph_map.end(), 0, + [](const int a, const auto& b) { return a + b.second.size(); })); + for (const auto& font_value : font_glyph_map) { + const ScaledFont& scaled_font = font_value.first; + for (const SubpixelGlyph& glyph : font_value.second) { + font_glyph_pairs.push_back({scaled_font, glyph}); + } + } auto glyph_atlas = std::make_shared(type); - CollectNewGlyphs(glyph_atlas, text_frames, new_glyphs, new_sizes); - auto atlas_size = OptimumAtlasSizeForFontGlyphPairs( - new_glyphs, // + font_glyph_pairs, // glyph_positions, // atlas_context, // type, // @@ -553,7 +524,7 @@ std::shared_ptr TypographerContextSTB::CreateGlyphAtlas( // sanity check of counts. This could also be just an assertion as only a // construction issue would cause such a failure. // --------------------------------------------------------------------------- - if (glyph_positions.size() != new_glyphs.size()) { + if (glyph_positions.size() != font_glyph_pairs.size()) { return nullptr; } @@ -562,7 +533,8 @@ std::shared_ptr TypographerContextSTB::CreateGlyphAtlas( // --------------------------------------------------------------------------- { size_t i = 0; - for (auto it = new_glyphs.begin(); it != new_glyphs.end(); ++i, ++it) { + for (auto it = font_glyph_pairs.begin(); it != font_glyph_pairs.end(); + ++i, ++it) { glyph_atlas->AddTypefaceGlyphPositionAndBounds(*it, glyph_positions[i], new_sizes[i]); } diff --git a/engine/src/flutter/impeller/typographer/backends/stb/typographer_context_stb.h b/engine/src/flutter/impeller/typographer/backends/stb/typographer_context_stb.h index c6133a6a351..87eedbe92a3 100644 --- a/engine/src/flutter/impeller/typographer/backends/stb/typographer_context_stb.h +++ b/engine/src/flutter/impeller/typographer/backends/stb/typographer_context_stb.h @@ -30,16 +30,9 @@ class TypographerContextSTB : public TypographerContext { GlyphAtlas::Type type, HostBuffer& host_buffer, const std::shared_ptr& atlas_context, - const std::vector>& text_frames) - const override; + const FontGlyphMap& font_glyph_map) const override; private: - static void CollectNewGlyphs( - const std::shared_ptr& atlas, - const std::vector>& text_frames, - std::vector& new_glyphs, - std::vector& glyph_sizes); - TypographerContextSTB(const TypographerContextSTB&) = delete; TypographerContextSTB& operator=(const TypographerContextSTB&) = delete; diff --git a/engine/src/flutter/impeller/typographer/font_glyph_pair.h b/engine/src/flutter/impeller/typographer/font_glyph_pair.h index e530f5b3edc..d4c12fa4195 100644 --- a/engine/src/flutter/impeller/typographer/font_glyph_pair.h +++ b/engine/src/flutter/impeller/typographer/font_glyph_pair.h @@ -5,6 +5,10 @@ #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" @@ -95,6 +99,14 @@ struct SubpixelGlyph { }; }; +using FontGlyphMap = + std::unordered_map, + ScaledFont::Hash, + ScaledFont::Equal>; + //------------------------------------------------------------------------------ /// @brief A font along with a glyph in that font rendered at a particular /// scale and subpixel position. @@ -102,8 +114,8 @@ struct SubpixelGlyph { struct FontGlyphPair { FontGlyphPair(const ScaledFont& sf, const SubpixelGlyph& g) : scaled_font(sf), glyph(g) {} - ScaledFont scaled_font; - SubpixelGlyph glyph; + const ScaledFont& scaled_font; + const SubpixelGlyph& glyph; }; } // namespace impeller diff --git a/engine/src/flutter/impeller/typographer/glyph_atlas.cc b/engine/src/flutter/impeller/typographer/glyph_atlas.cc index c917a70a6f6..77b8dfd9239 100644 --- a/engine/src/flutter/impeller/typographer/glyph_atlas.cc +++ b/engine/src/flutter/impeller/typographer/glyph_atlas.cc @@ -6,7 +6,6 @@ #include #include -#include "impeller/typographer/font_glyph_pair.h" namespace impeller { @@ -68,10 +67,10 @@ void GlyphAtlas::AddTypefaceGlyphPositionAndBounds(const FontGlyphPair& pair, Rect position, Rect bounds) { font_atlas_map_[pair.scaled_font].positions_[pair.glyph] = - FrameBounds{position, bounds, /*is_placeholder=*/false}; + std::make_pair(position, bounds); } -std::optional GlyphAtlas::FindFontGlyphBounds( +std::optional> GlyphAtlas::FindFontGlyphBounds( const FontGlyphPair& pair) const { const auto& found = font_atlas_map_.find(pair.scaled_font); if (found == font_atlas_map_.end()) { @@ -80,14 +79,13 @@ std::optional GlyphAtlas::FindFontGlyphBounds( return found->second.FindGlyphBounds(pair.glyph); } -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; +const FontGlyphAtlas* GlyphAtlas::GetFontGlyphAtlas(const Font& font, + Scalar scale) const { + const auto& found = font_atlas_map_.find(ScaledFont{font, scale}); + if (found == font_atlas_map_.end()) { + return nullptr; } - font_atlas_map_[scaled_font] = FontGlyphAtlas(); - return &font_atlas_map_[scaled_font]; + return &found->second; } size_t GlyphAtlas::GetGlyphCount() const { @@ -110,7 +108,7 @@ size_t GlyphAtlas::IterateGlyphs( for (const auto& glyph_value : font_value.second.positions_) { count++; if (!iterator(font_value.first, glyph_value.first, - glyph_value.second.atlas_bounds)) { + glyph_value.second.first)) { return count; } } @@ -118,7 +116,7 @@ size_t GlyphAtlas::IterateGlyphs( return count; } -std::optional FontGlyphAtlas::FindGlyphBounds( +std::optional> FontGlyphAtlas::FindGlyphBounds( const SubpixelGlyph& glyph) const { const auto& found = positions_.find(glyph); if (found == positions_.end()) { @@ -127,9 +125,4 @@ std::optional FontGlyphAtlas::FindGlyphBounds( return found->second; } -void FontGlyphAtlas::AppendGlyph(const SubpixelGlyph& glyph, - const FrameBounds& frame_bounds) { - positions_[glyph] = frame_bounds; -} - } // namespace impeller diff --git a/engine/src/flutter/impeller/typographer/glyph_atlas.h b/engine/src/flutter/impeller/typographer/glyph_atlas.h index 87c54903183..587ece46f04 100644 --- a/engine/src/flutter/impeller/typographer/glyph_atlas.h +++ b/engine/src/flutter/impeller/typographer/glyph_atlas.h @@ -19,16 +19,6 @@ namespace impeller { class FontGlyphAtlas; -struct FrameBounds { - /// The bounds of the glyph within the glyph atlas. - Rect atlas_bounds; - /// The local glyph bounds. - Rect glyph_bounds; - /// Whether [atlas_bounds] are still a placeholder and have - /// not yet been computed. - bool is_placeholder = true; -}; - //------------------------------------------------------------------------------ /// @brief A texture containing the bitmap representation of glyphs in /// different fonts along with the ability to query the location of @@ -125,7 +115,7 @@ class GlyphAtlas { /// @return The location of the font-glyph pair in the atlas. /// `std::nullopt` if the pair is not in the atlas. /// - std::optional FindFontGlyphBounds( + std::optional> FindFontGlyphBounds( const FontGlyphPair& pair) const; //---------------------------------------------------------------------------- @@ -140,7 +130,7 @@ class GlyphAtlas { /// scale are not available in the atlas. The pointer is only /// valid for the lifetime of the GlyphAtlas. /// - FontGlyphAtlas* GetOrCreateFontGlyphAtlas(const ScaledFont& scaled_font); + const FontGlyphAtlas* GetFontGlyphAtlas(const Font& font, Scalar scale) const; private: const Type type_; @@ -223,25 +213,20 @@ class FontGlyphAtlas { /// @return The location of the glyph in the atlas. /// `std::nullopt` if the glyph is not in the atlas. /// - std::optional FindGlyphBounds(const SubpixelGlyph& glyph) const; - - //---------------------------------------------------------------------------- - /// @brief Append the frame bounds of a glyph to this atlas. - /// - /// This may indicate a placeholder glyph location to be replaced - /// at a later time, as indicated by FrameBounds.placeholder. - void AppendGlyph(const SubpixelGlyph& glyph, const FrameBounds& frame_bounds); + std::optional> FindGlyphBounds( + const SubpixelGlyph& glyph) const; private: friend class GlyphAtlas; - std::unordered_map, SubpixelGlyph::Hash, SubpixelGlyph::Equal> positions_; FontGlyphAtlas(const FontGlyphAtlas&) = delete; + + FontGlyphAtlas& operator=(const FontGlyphAtlas&) = delete; }; } // namespace impeller diff --git a/engine/src/flutter/impeller/typographer/lazy_glyph_atlas.cc b/engine/src/flutter/impeller/typographer/lazy_glyph_atlas.cc index 106ca538bf2..be5688f6a7f 100644 --- a/engine/src/flutter/impeller/typographer/lazy_glyph_atlas.cc +++ b/engine/src/flutter/impeller/typographer/lazy_glyph_atlas.cc @@ -7,7 +7,6 @@ #include "fml/logging.h" #include "impeller/base/validation.h" #include "impeller/typographer/glyph_atlas.h" -#include "impeller/typographer/text_frame.h" #include "impeller/typographer/typographer_context.h" #include @@ -31,22 +30,23 @@ LazyGlyphAtlas::LazyGlyphAtlas( LazyGlyphAtlas::~LazyGlyphAtlas() = default; -void LazyGlyphAtlas::AddTextFrame(const std::shared_ptr& frame, +void LazyGlyphAtlas::AddTextFrame(const TextFrame& frame, Scalar scale, Point offset, - std::optional properties) { - frame->SetPerFrameData(scale, offset, properties); + const GlyphProperties& properties) { FML_DCHECK(alpha_atlas_ == nullptr && color_atlas_ == nullptr); - if (frame->GetAtlasType() == GlyphAtlas::Type::kAlphaBitmap) { - alpha_text_frames_.push_back(frame); + if (frame.GetAtlasType() == GlyphAtlas::Type::kAlphaBitmap) { + frame.CollectUniqueFontGlyphPairs(alpha_glyph_map_, scale, offset, + properties); } else { - color_text_frames_.push_back(frame); + frame.CollectUniqueFontGlyphPairs(color_glyph_map_, scale, offset, + properties); } } void LazyGlyphAtlas::ResetTextFrames() { - alpha_text_frames_.clear(); - color_text_frames_.clear(); + alpha_glyph_map_.clear(); + color_glyph_map_.clear(); alpha_atlas_.reset(); color_atlas_.reset(); } @@ -75,8 +75,8 @@ const std::shared_ptr& LazyGlyphAtlas::CreateOrGetGlyphAtlas( return kNullGlyphAtlas; } - auto& glyph_map = type == GlyphAtlas::Type::kAlphaBitmap ? alpha_text_frames_ - : color_text_frames_; + auto& glyph_map = type == GlyphAtlas::Type::kAlphaBitmap ? alpha_glyph_map_ + : color_glyph_map_; const std::shared_ptr& atlas_context = type == GlyphAtlas::Type::kAlphaBitmap ? alpha_context_ : color_context_; std::shared_ptr atlas = typographer_context_->CreateGlyphAtlas( diff --git a/engine/src/flutter/impeller/typographer/lazy_glyph_atlas.h b/engine/src/flutter/impeller/typographer/lazy_glyph_atlas.h index 47c6d466df1..bead4bda983 100644 --- a/engine/src/flutter/impeller/typographer/lazy_glyph_atlas.h +++ b/engine/src/flutter/impeller/typographer/lazy_glyph_atlas.h @@ -19,10 +19,10 @@ class LazyGlyphAtlas { ~LazyGlyphAtlas(); - void AddTextFrame(const std::shared_ptr& frame, + void AddTextFrame(const TextFrame& frame, Scalar scale, Point offset, - std::optional properties); + const GlyphProperties& properties); void ResetTextFrames(); @@ -34,8 +34,8 @@ class LazyGlyphAtlas { private: std::shared_ptr typographer_context_; - std::vector> alpha_text_frames_; - std::vector> color_text_frames_; + FontGlyphMap alpha_glyph_map_; + FontGlyphMap color_glyph_map_; std::shared_ptr alpha_context_; std::shared_ptr color_context_; mutable std::shared_ptr alpha_atlas_; diff --git a/engine/src/flutter/impeller/typographer/text_frame.cc b/engine/src/flutter/impeller/typographer/text_frame.cc index b72bb621c05..b715616fb4d 100644 --- a/engine/src/flutter/impeller/typographer/text_frame.cc +++ b/engine/src/flutter/impeller/typographer/text_frame.cc @@ -84,44 +84,27 @@ Point TextFrame::ComputeSubpixelPosition( } } -void TextFrame::SetPerFrameData(Scalar scale, - Point offset, - std::optional properties) { - scale_ = scale; - offset_ = offset; - properties_ = properties; -} - -Scalar TextFrame::GetScale() const { - return scale_; -} - -Point TextFrame::GetOffset() const { - return offset_; -} - -std::optional TextFrame::GetProperties() const { - return properties_; -} - -void TextFrame::AppendFrameBounds(const FrameBounds& frame_bounds) { - bound_values_.push_back(frame_bounds); -} - -void TextFrame::ClearFrameBounds() { - bound_values_.clear(); -} - -bool TextFrame::IsFrameComplete() const { - size_t run_size = 0; - for (const auto& x : runs_) { - run_size += x.GetGlyphCount(); +void TextFrame::CollectUniqueFontGlyphPairs( + FontGlyphMap& glyph_map, + 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 = + RoundScaledFontSize(scale, font.GetMetrics().point_size); + auto& set = glyph_map[ScaledFont{font, rounded_scale}]; + for (const TextRun::GlyphPosition& glyph_position : + run.GetGlyphPositions()) { + Point subpixel = ComputeSubpixelPosition( + glyph_position, font.GetAxisAlignment(), offset, scale); + set.emplace(glyph_position.glyph, subpixel, lookup); + } } - return bound_values_.size() == run_size; -} - -const FrameBounds& TextFrame::GetFrameBounds(size_t index) const { - return bound_values_.at(index); } } // namespace impeller diff --git a/engine/src/flutter/impeller/typographer/text_frame.h b/engine/src/flutter/impeller/typographer/text_frame.h index 0604d6cc1eb..b742e96e2a7 100644 --- a/engine/src/flutter/impeller/typographer/text_frame.h +++ b/engine/src/flutter/impeller/typographer/text_frame.h @@ -16,8 +16,6 @@ namespace impeller { /// This object is typically the entrypoint in the Impeller type /// rendering subsystem. /// -/// A text frame should not be reused in multiple places within a single frame, -/// as internally it is used as a cache for various glyph properties. class TextFrame { public: TextFrame(); @@ -26,6 +24,11 @@ class TextFrame { ~TextFrame(); + void CollectUniqueFontGlyphPairs(FontGlyphMap& glyph_map, + Scalar scale, + Point offset, + const GlyphProperties& properties) const; + static Point ComputeSubpixelPosition( const TextRun::GlyphPosition& glyph_position, AxisAlignment alignment, @@ -66,53 +69,17 @@ class TextFrame { bool HasColor() const; //---------------------------------------------------------------------------- - /// @brief The type of atlas this run should be place in. + /// @brief The type of atlas this run should be emplaced in. GlyphAtlas::Type GetAtlasType() const; - /// @brief Verifies that all glyphs in this text frame have computed bounds - /// information. - bool IsFrameComplete() const; - - /// @brief Retrieve the frame bounds for the glyph at [index]. - /// - /// This method is only valid if [IsFrameComplete] returns true. - const FrameBounds& GetFrameBounds(size_t index) const; - - /// @brief Store text frame scale, offset, and properties for hashing in th - /// glyph atlas. - void SetPerFrameData(Scalar scale, - Point offset, - std::optional properties); - TextFrame& operator=(TextFrame&& other) = default; TextFrame(const TextFrame& other) = default; private: - friend class TypographerContextSkia; - friend class TypographerContextSTB; - friend class LazyGlyphAtlas; - - Scalar GetScale() const; - - Point GetOffset() const; - - std::optional GetProperties() const; - - void AppendFrameBounds(const FrameBounds& frame_bounds); - - void ClearFrameBounds(); - std::vector runs_; Rect bounds_; bool has_color_; - - // Data that is cached when rendering the text frame and is only - // valid for a single frame. - std::vector bound_values_; - Scalar scale_ = 0; - Point offset_; - std::optional properties_; }; } // namespace impeller diff --git a/engine/src/flutter/impeller/typographer/typographer_context.h b/engine/src/flutter/impeller/typographer/typographer_context.h index 138a911d89c..2c45c419dc8 100644 --- a/engine/src/flutter/impeller/typographer/typographer_context.h +++ b/engine/src/flutter/impeller/typographer/typographer_context.h @@ -9,7 +9,6 @@ #include "impeller/renderer/context.h" #include "impeller/typographer/glyph_atlas.h" -#include "impeller/typographer/text_frame.h" namespace impeller { @@ -34,7 +33,7 @@ class TypographerContext { GlyphAtlas::Type type, HostBuffer& host_buffer, const std::shared_ptr& atlas_context, - const std::vector>& text_frames) const = 0; + const FontGlyphMap& font_glyph_map) const = 0; protected: //---------------------------------------------------------------------------- diff --git a/engine/src/flutter/impeller/typographer/typographer_unittests.cc b/engine/src/flutter/impeller/typographer/typographer_unittests.cc index 751977d5022..7c8e9cd0885 100644 --- a/engine/src/flutter/impeller/typographer/typographer_unittests.cc +++ b/engine/src/flutter/impeller/typographer/typographer_unittests.cc @@ -36,10 +36,11 @@ static std::shared_ptr CreateGlyphAtlas( GlyphAtlas::Type type, Scalar scale, const std::shared_ptr& atlas_context, - const std::shared_ptr& frame) { - frame->SetPerFrameData(scale, {0, 0}, std::nullopt); + const TextFrame& frame) { + FontGlyphMap font_glyph_map; + frame.CollectUniqueFontGlyphPairs(font_glyph_map, scale, {0, 0}, {}); return typographer_context->CreateGlyphAtlas(context, type, host_buffer, - atlas_context, {frame}); + atlas_context, font_glyph_map); } static std::shared_ptr CreateGlyphAtlas( @@ -50,13 +51,15 @@ static std::shared_ptr CreateGlyphAtlas( Scalar scale, const std::shared_ptr& atlas_context, const std::vector>& frames, - const std::vector>& properties) { + const std::vector& properties) { + FontGlyphMap font_glyph_map; size_t offset = 0; for (auto& frame : frames) { - frame->SetPerFrameData(scale, {0, 0}, properties[offset++]); + frame->CollectUniqueFontGlyphPairs(font_glyph_map, scale, {0, 0}, + properties[offset++]); } return typographer_context->CreateGlyphAtlas(context, type, host_buffer, - atlas_context, frames); + atlas_context, font_glyph_map); } TEST_P(TypographerTest, CanConvertTextBlob) { @@ -89,8 +92,7 @@ TEST_P(TypographerTest, CanCreateGlyphAtlas) { auto atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer, GlyphAtlas::Type::kAlphaBitmap, 1.0f, atlas_context, - MakeTextFrameFromTextBlobSkia(blob)); - + *MakeTextFrameFromTextBlobSkia(blob)); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); ASSERT_EQ(atlas->GetType(), GlyphAtlas::Type::kAlphaBitmap); @@ -135,14 +137,14 @@ TEST_P(TypographerTest, LazyAtlasTracksColor) { LazyGlyphAtlas lazy_atlas(TypographerContextSkia::Make()); - lazy_atlas.AddTextFrame(frame, 1.0f, {0, 0}, {}); + lazy_atlas.AddTextFrame(*frame, 1.0f, {0, 0}, {}); frame = MakeTextFrameFromTextBlobSkia( SkTextBlob::MakeFromString("😀 ", emoji_font)); ASSERT_TRUE(frame->GetAtlasType() == GlyphAtlas::Type::kColorBitmap); - lazy_atlas.AddTextFrame(frame, 1.0f, {0, 0}, {}); + lazy_atlas.AddTextFrame(*frame, 1.0f, {0, 0}, {}); // Creates different atlases for color and red bitmap. auto color_atlas = lazy_atlas.CreateOrGetGlyphAtlas( @@ -166,7 +168,7 @@ TEST_P(TypographerTest, GlyphAtlasWithOddUniqueGlyphSize) { auto atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer, GlyphAtlas::Type::kAlphaBitmap, 1.0f, atlas_context, - MakeTextFrameFromTextBlobSkia(blob)); + *MakeTextFrameFromTextBlobSkia(blob)); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); @@ -186,7 +188,7 @@ TEST_P(TypographerTest, GlyphAtlasIsRecycledIfUnchanged) { auto atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer, GlyphAtlas::Type::kAlphaBitmap, 1.0f, atlas_context, - MakeTextFrameFromTextBlobSkia(blob)); + *MakeTextFrameFromTextBlobSkia(blob)); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); ASSERT_EQ(atlas, atlas_context->GetGlyphAtlas()); @@ -196,7 +198,7 @@ TEST_P(TypographerTest, GlyphAtlasIsRecycledIfUnchanged) { auto next_atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer, GlyphAtlas::Type::kAlphaBitmap, 1.0f, atlas_context, - MakeTextFrameFromTextBlobSkia(blob)); + *MakeTextFrameFromTextBlobSkia(blob)); ASSERT_EQ(atlas, next_atlas); ASSERT_EQ(atlas_context->GetGlyphAtlas(), atlas); } @@ -218,15 +220,15 @@ TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) { auto blob = SkTextBlob::MakeFromString(test_string, sk_font); ASSERT_TRUE(blob); + FontGlyphMap font_glyph_map; size_t size_count = 8; - std::vector> frames; for (size_t index = 0; index < size_count; index += 1) { - frames.push_back(MakeTextFrameFromTextBlobSkia(blob)); - frames.back()->SetPerFrameData(0.6 * index, {0, 0}, {}); + MakeTextFrameFromTextBlobSkia(blob)->CollectUniqueFontGlyphPairs( + font_glyph_map, 0.6 * index, {0, 0}, {}); }; auto atlas = context->CreateGlyphAtlas(*GetContext(), GlyphAtlas::Type::kAlphaBitmap, - *host_buffer, atlas_context, frames); + *host_buffer, atlas_context, font_glyph_map); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); @@ -260,7 +262,7 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecycledIfUnchanged) { auto atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer, GlyphAtlas::Type::kAlphaBitmap, 1.0f, atlas_context, - MakeTextFrameFromTextBlobSkia(blob)); + *MakeTextFrameFromTextBlobSkia(blob)); auto old_packer = atlas_context->GetRectPacker(); ASSERT_NE(atlas, nullptr); @@ -275,7 +277,7 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecycledIfUnchanged) { auto next_atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer, GlyphAtlas::Type::kAlphaBitmap, 1.0f, atlas_context, - MakeTextFrameFromTextBlobSkia(blob2)); + *MakeTextFrameFromTextBlobSkia(blob2)); ASSERT_EQ(atlas, next_atlas); auto* second_texture = next_atlas->GetTexture().get(); @@ -306,7 +308,7 @@ TEST_P(TypographerTest, GlyphColorIsPartOfCacheKey) { SkTextBlob::MakeFromString("😂", emoji_font)); auto frame_2 = MakeTextFrameFromTextBlobSkia( SkTextBlob::MakeFromString("😂", emoji_font)); - std::vector> properties = { + auto properties = { GlyphProperties{.color = Color::Red()}, GlyphProperties{.color = Color::Blue()}, }; @@ -336,7 +338,7 @@ TEST_P(TypographerTest, GlyphColorIsIgnoredForNonEmojiFonts) { MakeTextFrameFromTextBlobSkia(SkTextBlob::MakeFromString("A", sk_font)); auto frame_2 = MakeTextFrameFromTextBlobSkia(SkTextBlob::MakeFromString("A", sk_font)); - std::vector> properties = { + auto properties = { GlyphProperties{}, GlyphProperties{}, }; @@ -430,7 +432,7 @@ TEST_P(TypographerTest, GlyphAtlasTextureWillGrowTilMaxTextureSize) { auto atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer, GlyphAtlas::Type::kAlphaBitmap, 1.0f, atlas_context, - MakeTextFrameFromTextBlobSkia(blob)); + *MakeTextFrameFromTextBlobSkia(blob)); // Continually append new glyphs until the glyph size grows to the maximum. // Note that the sizes here are more or less experimentally determined, but // the important expectation is that the atlas size will shrink again after @@ -471,7 +473,7 @@ TEST_P(TypographerTest, GlyphAtlasTextureWillGrowTilMaxTextureSize) { atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer, GlyphAtlas::Type::kAlphaBitmap, 50 + i, atlas_context, - MakeTextFrameFromTextBlobSkia(blob)); + *MakeTextFrameFromTextBlobSkia(blob)); ASSERT_TRUE(!!atlas); EXPECT_EQ(atlas->GetTexture()->GetTextureDescriptor().size, expected_sizes[i]); @@ -483,38 +485,6 @@ TEST_P(TypographerTest, GlyphAtlasTextureWillGrowTilMaxTextureSize) { ASSERT_EQ(atlas->GetGlyphCount(), 2u); } -TEST_P(TypographerTest, TextFrameInitialBoundsArePlaceholder) { - SkFont font = flutter::testing::CreateTestFontOfSize(12); - auto blob = SkTextBlob::MakeFromString( - "the quick brown fox jumped over the lazy dog.", font); - ASSERT_TRUE(blob); - auto frame = MakeTextFrameFromTextBlobSkia(blob); - - EXPECT_FALSE(frame->IsFrameComplete()); - - auto context = TypographerContextSkia::Make(); - auto atlas_context = - context->CreateGlyphAtlasContext(GlyphAtlas::Type::kAlphaBitmap); - auto host_buffer = HostBuffer::Create(GetContext()->GetResourceAllocator()); - - auto atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer, - GlyphAtlas::Type::kAlphaBitmap, 1.0f, - atlas_context, frame); - - // The glyph position in the atlas was not known when this value - // was recorded. It is marked as a placeholder. - EXPECT_TRUE(frame->IsFrameComplete()); - EXPECT_TRUE(frame->GetFrameBounds(0).is_placeholder); - - atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer, - GlyphAtlas::Type::kAlphaBitmap, 1.0f, atlas_context, - frame); - - // The second time the glyph is rendered, the bounds are correcly known. - EXPECT_TRUE(frame->IsFrameComplete()); - EXPECT_FALSE(frame->GetFrameBounds(0).is_placeholder); -} - } // namespace testing } // namespace impeller diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/platform_views_controller.h b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/platform_views_controller.h index 217a6e200e8..1079b6d767c 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/platform_views_controller.h +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/platform_views_controller.h @@ -6,8 +6,6 @@ #define FLUTTER_SHELL_PLATFORM_DARWIN_IOS_FRAMEWORK_SOURCE_PLATFORM_VIEWS_CONTROLLER_H_ #include -#include -#include #include "flutter/flow/surface.h" #include "flutter/fml/memory/weak_ptr.h"