From fdbad460a58100b9d835a0be3faeb1dc747cb9a7 Mon Sep 17 00:00:00 2001 From: "auto-submit[bot]" <98614782+auto-submit[bot]@users.noreply.github.com> Date: Fri, 27 Sep 2024 17:09:28 +0000 Subject: [PATCH] Reverts "[Impeller] hash even less stuff per frame. (#55092)" (flutter/engine#55491) Reverts: flutter/engine#55092 Initiated by: jonahwilliams Reason for reverting: framework golden failures. Original PR Author: jonahwilliams Reviewed By: {chinmaygarde, jtmcdole} This change reverts the following previous change: Follow up to https://github.com/flutter/engine/pull/55060 Currently we have multiple stages of hashing while font rendering, which is relatively expensive for the actualy required workload. First, we hash the contents of all text frames to compute the unique set of glyphs per frame. Then we diff this hash set against the hashmap of glyphs within the atlas. Finally we hash and lookup the final rendered bounds for each glyph. We can simplify this to 2. hash lookups for glyphs not yet in the atlas and 1. hash lookup for glyphs that are in the atlas. This is done by combing the step where we uniquely compute glyphs per frame with the diff against the current atlas. When this lookup is performed, we also store the glyph position (if found) in the text_frame itself - which allows text contents to skip the last hash, as long as the glyph has already been rendered. ### Before ![flutter_03](https://github.com/user-attachments/assets/be9c4459-f0c8-426c-b152-38861acb207f) ### After ![flutter_04](https://github.com/user-attachments/assets/1aa7cbd1-6af7-4020-8966-7e3baaef102b) Using this handy dandy test app: ```dart import 'package:flutter/material.dart'; void main() { runApp(MyApp()); } class MyApp extends StatelessWidget { Widget build(context) { return MaterialApp( home: Scaffold( appBar: AppBar( title: Text('Platform View'), ), body: SafeArea(child: Stack(children: [ SizedBox( width: 380, height: 380, child: LinearProgressIndicator(), ), Stack( children: List.generate(1000, (index) { // The problem already happens with a small amount of widgets. // Using an excessive amount of widgets is just to make the problem more evident. return Text("Lots of Texts represent a Widget with complex components."); }), ), Align( alignment: Alignment.bottomCenter, child: TextButton( child: Text("Button"), onPressed: () { print("Tap ${DateTime.now()}"); }, ), ), ], ), ), ), ); } } ``` --- .../impeller/display_list/dl_dispatcher.cc | 11 +- .../impeller/entity/contents/text_contents.cc | 71 ++++------- .../impeller/entity/contents/text_contents.h | 2 - .../backends/skia/typographer_context_skia.cc | 114 +++++++----------- .../backends/skia/typographer_context_skia.h | 7 +- .../backends/stb/typographer_context_stb.cc | 98 ++++++--------- .../backends/stb/typographer_context_stb.h | 9 +- .../impeller/typographer/font_glyph_pair.h | 16 ++- .../impeller/typographer/glyph_atlas.cc | 27 ++--- .../impeller/typographer/glyph_atlas.h | 29 ++--- .../impeller/typographer/lazy_glyph_atlas.cc | 22 ++-- .../impeller/typographer/lazy_glyph_atlas.h | 8 +- .../impeller/typographer/text_frame.cc | 57 +++------ .../flutter/impeller/typographer/text_frame.h | 45 +------ .../typographer/typographer_context.h | 3 +- .../typographer/typographer_unittests.cc | 80 ++++-------- .../Source/platform_views_controller.h | 2 - 17 files changed, 202 insertions(+), 399 deletions(-) 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"