diff --git a/engine/src/flutter/impeller/display_list/dl_dispatcher.cc b/engine/src/flutter/impeller/display_list/dl_dispatcher.cc index 1bdad62aff..b1ba5b7e35 100644 --- a/engine/src/flutter/impeller/display_list/dl_dispatcher.cc +++ b/engine/src/flutter/impeller/display_list/dl_dispatcher.cc @@ -1109,8 +1109,9 @@ void FirstPassDispatcher::drawTextFrame( // we do not double-apply the alpha. properties.color = paint_.color.WithAlpha(1.0); } - auto scale = - (matrix_ * Matrix::MakeTranslation(Point(x, y))).GetMaxBasisLengthXY(); + auto scale = TextFrame::RoundScaledFontSize( + (matrix_ * Matrix::MakeTranslation(Point(x, y))).GetMaxBasisLengthXY()); + renderer_.GetLazyGlyphAtlas()->AddTextFrame( text_frame, // scale, // diff --git a/engine/src/flutter/impeller/entity/contents/text_contents.cc b/engine/src/flutter/impeller/entity/contents/text_contents.cc index dfa4c42198..4d4a130480 100644 --- a/engine/src/flutter/impeller/entity/contents/text_contents.cc +++ b/engine/src/flutter/impeller/entity/contents/text_contents.cc @@ -175,8 +175,7 @@ bool TextContents::Render(const ContentContext& renderer, 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); + Scalar rounded_scale = TextFrame::RoundScaledFontSize(scale_); FontGlyphAtlas* font_atlas = nullptr; // Adjust glyph position based on the subpixel rounding @@ -220,9 +219,9 @@ bool TextContents::Render(const ContentContext& renderer, 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_); + glyph_position, font.GetAxisAlignment(), offset_, + rounded_scale); std::optional maybe_atlas_glyph_bounds = font_atlas->FindGlyphBounds(SubpixelGlyph{ diff --git a/engine/src/flutter/impeller/entity/entity_unittests.cc b/engine/src/flutter/impeller/entity/entity_unittests.cc index 8fee465fab..756203ab2f 100644 --- a/engine/src/flutter/impeller/entity/entity_unittests.cc +++ b/engine/src/flutter/impeller/entity/entity_unittests.cc @@ -2120,11 +2120,11 @@ TEST_P(EntityTest, ColorFilterContentsWithLargeGeometry) { } TEST_P(EntityTest, TextContentsCeilsGlyphScaleToDecimal) { - ASSERT_EQ(TextFrame::RoundScaledFontSize(0.4321111f, 12), 0.43f); - ASSERT_EQ(TextFrame::RoundScaledFontSize(0.5321111f, 12), 0.53f); - ASSERT_EQ(TextFrame::RoundScaledFontSize(2.1f, 12), 2.1f); - ASSERT_EQ(TextFrame::RoundScaledFontSize(0.0f, 12), 0.0f); - ASSERT_EQ(TextFrame::RoundScaledFontSize(100000000.0f, 12), 48.0f); + ASSERT_EQ(TextFrame::RoundScaledFontSize(0.4321111f), 0.43f); + ASSERT_EQ(TextFrame::RoundScaledFontSize(0.5321111f), 0.53f); + ASSERT_EQ(TextFrame::RoundScaledFontSize(2.1f), 2.1f); + ASSERT_EQ(TextFrame::RoundScaledFontSize(0.0f), 0.0f); + ASSERT_EQ(TextFrame::RoundScaledFontSize(100000000.0f), 48.0f); } TEST_P(EntityTest, SpecializationConstantsAreAppliedToVariants) { 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 80b499352b..13dc254e67 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 @@ -413,18 +413,20 @@ TypographerContextSkia::CollectNewGlyphs( const std::vector>& text_frames) { std::vector new_glyphs; std::vector glyph_sizes; + size_t generation_id = atlas->GetAtlasGeneration(); 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. + if (frame->IsFrameComplete() && + frame->GetAtlasGeneration() == generation_id && + !frame->GetFrameBounds(0).is_placeholder) { + continue; + } frame->ClearFrameBounds(); + frame->SetAtlasGeneration(generation_id); for (const auto& run : frame->GetRuns()) { auto metrics = run.GetFont().GetMetrics(); - auto rounded_scale = - TextFrame::RoundScaledFontSize(frame->GetScale(), metrics.point_size); + auto rounded_scale = TextFrame::RoundScaledFontSize(frame->GetScale()); ScaledFont scaled_font{.font = run.GetFont(), .scale = rounded_scale}; FontGlyphAtlas* font_glyph_atlas = @@ -566,7 +568,8 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( if (atlas_context->GetAtlasSize().height >= max_texture_height || context.GetBackendType() == Context::BackendType::kOpenGLES) { blit_old_atlas = false; - new_atlas = std::make_shared(type); + new_atlas = std::make_shared( + type, /*initial_generation=*/last_atlas->GetAtlasGeneration() + 1); auto [update_glyphs, update_sizes] = CollectNewGlyphs(new_atlas, text_frames); diff --git a/engine/src/flutter/impeller/typographer/font_glyph_pair.h b/engine/src/flutter/impeller/typographer/font_glyph_pair.h index e530f5b3ed..e3df2dae50 100644 --- a/engine/src/flutter/impeller/typographer/font_glyph_pair.h +++ b/engine/src/flutter/impeller/typographer/font_glyph_pair.h @@ -20,6 +20,17 @@ struct GlyphProperties { Join stroke_join = Join::kMiter; Scalar stroke_miter = 4.0; bool stroke = false; + + struct Equal { + constexpr bool operator()(const impeller::GlyphProperties& lhs, + const impeller::GlyphProperties& rhs) const { + return lhs.color.ToARGB() == rhs.color.ToARGB() && + lhs.stroke == rhs.stroke && lhs.stroke_cap == rhs.stroke_cap && + lhs.stroke_join == rhs.stroke_join && + lhs.stroke_miter == rhs.stroke_miter && + lhs.stroke_width == rhs.stroke_width; + } + }; }; //------------------------------------------------------------------------------ @@ -85,12 +96,8 @@ struct SubpixelGlyph { lhs.glyph.type == rhs.glyph.type && lhs.subpixel_offset == rhs.subpixel_offset && lhs.properties.has_value() && rhs.properties.has_value() && - lhs.properties->color.ToARGB() == rhs.properties->color.ToARGB() && - lhs.properties->stroke == rhs.properties->stroke && - lhs.properties->stroke_cap == rhs.properties->stroke_cap && - lhs.properties->stroke_join == rhs.properties->stroke_join && - lhs.properties->stroke_miter == rhs.properties->stroke_miter && - lhs.properties->stroke_width == rhs.properties->stroke_width; + GlyphProperties::Equal{}(lhs.properties.value(), + rhs.properties.value()); } }; }; diff --git a/engine/src/flutter/impeller/typographer/glyph_atlas.cc b/engine/src/flutter/impeller/typographer/glyph_atlas.cc index 093593804e..98038668fb 100644 --- a/engine/src/flutter/impeller/typographer/glyph_atlas.cc +++ b/engine/src/flutter/impeller/typographer/glyph_atlas.cc @@ -12,7 +12,8 @@ namespace impeller { GlyphAtlasContext::GlyphAtlasContext(GlyphAtlas::Type type) - : atlas_(std::make_shared(type)), atlas_size_(ISize(0, 0)) {} + : atlas_(std::make_shared(type, /*initial_generation=*/0)), + atlas_size_(ISize(0, 0)) {} GlyphAtlasContext::~GlyphAtlasContext() {} @@ -45,7 +46,8 @@ void GlyphAtlasContext::UpdateRectPacker( rect_packer_ = std::move(rect_packer); } -GlyphAtlas::GlyphAtlas(Type type) : type_(type) {} +GlyphAtlas::GlyphAtlas(Type type, size_t initial_generation) + : type_(type), generation_(initial_generation) {} GlyphAtlas::~GlyphAtlas() = default; @@ -65,6 +67,14 @@ void GlyphAtlas::SetTexture(std::shared_ptr texture) { texture_ = std::move(texture); } +size_t GlyphAtlas::GetAtlasGeneration() const { + return generation_; +} + +void GlyphAtlas::SetAtlasGeneration(size_t generation) { + generation_ = generation; +} + void GlyphAtlas::AddTypefaceGlyphPositionAndBounds(const FontGlyphPair& pair, Rect position, Rect bounds) { diff --git a/engine/src/flutter/impeller/typographer/glyph_atlas.h b/engine/src/flutter/impeller/typographer/glyph_atlas.h index 87c5490318..bf11ff056a 100644 --- a/engine/src/flutter/impeller/typographer/glyph_atlas.h +++ b/engine/src/flutter/impeller/typographer/glyph_atlas.h @@ -58,8 +58,9 @@ class GlyphAtlas { /// @brief Create an empty glyph atlas. /// /// @param[in] type How the glyphs are represented in the texture. + /// @param[in] initial_generation the atlas generation. /// - explicit GlyphAtlas(Type type); + GlyphAtlas(Type type, size_t initial_generation); ~GlyphAtlas(); @@ -142,9 +143,22 @@ class GlyphAtlas { /// FontGlyphAtlas* GetOrCreateFontGlyphAtlas(const ScaledFont& scaled_font); + //---------------------------------------------------------------------------- + /// @brief Retrieve the generation id for this glyph atlas. + /// + /// The generation id is used to match with a TextFrame to + /// determine if the frame is guaranteed to already be populated + /// in the atlas. + size_t GetAtlasGeneration() const; + + //---------------------------------------------------------------------------- + /// @brief Update the atlas generation. + void SetAtlasGeneration(size_t value); + private: const Type type_; std::shared_ptr texture_; + size_t generation_ = 0; std::unordered_map& a, + const std::optional& b) { + if (!a.has_value() && !b.has_value()) { + return true; + } + if (a.has_value() && b.has_value()) { + return GlyphProperties::Equal{}(a.value(), b.value()); + } + return false; +} +} // namespace + TextFrame::TextFrame() = default; TextFrame::TextFrame(std::vector& runs, Rect bounds, bool has_color) @@ -37,7 +51,7 @@ bool TextFrame::HasColor() const { } // static -Scalar TextFrame::RoundScaledFontSize(Scalar scale, Scalar point_size) { +Scalar TextFrame::RoundScaledFontSize(Scalar scale) { // An arbitrarily chosen maximum text scale to ensure that regardless of the // CTM, a glyph will fit in the atlas. If we clamp significantly, this may // reduce fidelity but is preferable to the alternative of failing to render. @@ -87,6 +101,12 @@ Point TextFrame::ComputeSubpixelPosition( void TextFrame::SetPerFrameData(Scalar scale, Point offset, std::optional properties) { + if (!ScalarNearlyEqual(scale_, scale) || + !ScalarNearlyEqual(offset_.x, offset.x) || + !ScalarNearlyEqual(offset_.y, offset.y) || + !TextPropertiesEquals(properties_, properties)) { + bound_values_.clear(); + } scale_ = scale; offset_ = offset; properties_ = properties; @@ -121,7 +141,15 @@ bool TextFrame::IsFrameComplete() const { } const FrameBounds& TextFrame::GetFrameBounds(size_t index) const { - return bound_values_.at(index); + return bound_values_[index]; +} + +size_t TextFrame::GetAtlasGeneration() const { + return generation_; +} + +void TextFrame::SetAtlasGeneration(size_t value) { + generation_ = value; } } // namespace impeller diff --git a/engine/src/flutter/impeller/typographer/text_frame.h b/engine/src/flutter/impeller/typographer/text_frame.h index 0604d6cc1e..067c8ebd05 100644 --- a/engine/src/flutter/impeller/typographer/text_frame.h +++ b/engine/src/flutter/impeller/typographer/text_frame.h @@ -32,7 +32,7 @@ class TextFrame { Point offset, Scalar scale); - static Scalar RoundScaledFontSize(Scalar scale, Scalar point_size); + static Scalar RoundScaledFontSize(Scalar scale); //---------------------------------------------------------------------------- /// @brief The conservative bounding box for this text frame. @@ -84,13 +84,18 @@ class TextFrame { Point offset, std::optional properties); + // A generation id for the glyph atlas this text run was associated + // with. As long as the frame generation matches the atlas generation, + // the contents are guaranteed to be populated and do not need to be + // processed. + size_t GetAtlasGeneration() const; + TextFrame& operator=(TextFrame&& other) = default; TextFrame(const TextFrame& other) = default; private: friend class TypographerContextSkia; - friend class TypographerContextSTB; friend class LazyGlyphAtlas; Scalar GetScale() const; @@ -103,14 +108,17 @@ class TextFrame { void ClearFrameBounds(); + void SetAtlasGeneration(size_t value); + 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. + // valid for the current atlas generation. std::vector bound_values_; Scalar scale_ = 0; + size_t generation_ = 0; Point offset_; std::optional properties_; }; diff --git a/engine/src/flutter/impeller/typographer/typographer_unittests.cc b/engine/src/flutter/impeller/typographer/typographer_unittests.cc index 6367314d9b..4b8cc1cbc9 100644 --- a/engine/src/flutter/impeller/typographer/typographer_unittests.cc +++ b/engine/src/flutter/impeller/typographer/typographer_unittests.cc @@ -508,7 +508,7 @@ TEST_P(TypographerTest, TextFrameInitialBoundsArePlaceholder) { GetContext()->GetIdleWaiter()); auto atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer, - GlyphAtlas::Type::kAlphaBitmap, 1.0f, + GlyphAtlas::Type::kAlphaBitmap, /*scale=*/1.0f, atlas_context, frame); // The glyph position in the atlas was not known when this value @@ -517,14 +517,101 @@ TEST_P(TypographerTest, TextFrameInitialBoundsArePlaceholder) { EXPECT_TRUE(frame->GetFrameBounds(0).is_placeholder); atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer, - GlyphAtlas::Type::kAlphaBitmap, 1.0f, atlas_context, - frame); + GlyphAtlas::Type::kAlphaBitmap, /*scale=*/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); } +TEST_P(TypographerTest, TextFrameInvalidationWithScale) { + 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(), + GetContext()->GetIdleWaiter()); + + auto atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer, + GlyphAtlas::Type::kAlphaBitmap, /*scale=*/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); + + // Change the scale and the glyph data will still be a placeholder, as the + // old data is no longer valid. + atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer, + GlyphAtlas::Type::kAlphaBitmap, /*scale=*/2.0f, + atlas_context, frame); + + // The second time the glyph is rendered, the bounds are correcly known. + EXPECT_TRUE(frame->IsFrameComplete()); + EXPECT_TRUE(frame->GetFrameBounds(0).is_placeholder); +} + +TEST_P(TypographerTest, TextFrameAtlasGenerationTracksState) { + 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(), + GetContext()->GetIdleWaiter()); + + auto atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer, + GlyphAtlas::Type::kAlphaBitmap, /*scale=*/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); + if (GetBackend() == PlaygroundBackend::kOpenGLES) { + // OpenGLES must always increase the atlas backend if the texture grows. + EXPECT_EQ(frame->GetAtlasGeneration(), 1u); + } else { + EXPECT_EQ(frame->GetAtlasGeneration(), 0u); + } + + atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer, + GlyphAtlas::Type::kAlphaBitmap, /*scale=*/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); + if (GetBackend() == PlaygroundBackend::kOpenGLES) { + EXPECT_EQ(frame->GetAtlasGeneration(), 1u); + } else { + EXPECT_EQ(frame->GetAtlasGeneration(), 0u); + } + + // Force increase the generation. + atlas_context->GetGlyphAtlas()->SetAtlasGeneration(2u); + atlas = CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer, + GlyphAtlas::Type::kAlphaBitmap, /*scale=*/1.0f, + atlas_context, frame); + + EXPECT_EQ(frame->GetAtlasGeneration(), 2u); +} + } // namespace testing } // namespace impeller