[Impeller] cache even more text frame data to skip lookups. (#56798)
Every text frame must inform the glyph atlas about the glyphs + scale it contains. When this happens, the glyph atlas will populate the glyph and then tell teh text frame about the location in the atlas, so that the text contents shader can sample it correctly.
Once this has been done once for a given text frame + scale + offset, we can actually just keep reusing the same data provided 1) the atlas itself hasn't changed and 2) the scale/offset/properties are the same.
This constitutes a nice CPU usage reduction in places where there is a lot of text that isn't being invalidated, like scrolling on the flutter gallery.
diff --git a/impeller/display_list/dl_dispatcher.cc b/impeller/display_list/dl_dispatcher.cc
index 1bdad62..b1ba5b7 100644
--- a/impeller/display_list/dl_dispatcher.cc
+++ b/impeller/display_list/dl_dispatcher.cc
@@ -1109,8 +1109,9 @@
// 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/impeller/entity/contents/text_contents.cc b/impeller/entity/contents/text_contents.cc
index dfa4c42..4d4a130 100644
--- a/impeller/entity/contents/text_contents.cc
+++ b/impeller/entity/contents/text_contents.cc
@@ -175,8 +175,7 @@
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 @@
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<FrameBounds> maybe_atlas_glyph_bounds =
font_atlas->FindGlyphBounds(SubpixelGlyph{
diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc
index 8fee465..756203a 100644
--- a/impeller/entity/entity_unittests.cc
+++ b/impeller/entity/entity_unittests.cc
@@ -2120,11 +2120,11 @@
}
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/impeller/typographer/backends/skia/typographer_context_skia.cc b/impeller/typographer/backends/skia/typographer_context_skia.cc
index 80b4993..13dc254 100644
--- a/impeller/typographer/backends/skia/typographer_context_skia.cc
+++ b/impeller/typographer/backends/skia/typographer_context_skia.cc
@@ -413,18 +413,20 @@
const std::vector<std::shared_ptr<TextFrame>>& text_frames) {
std::vector<FontGlyphPair> new_glyphs;
std::vector<Rect> 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 @@
if (atlas_context->GetAtlasSize().height >= max_texture_height ||
context.GetBackendType() == Context::BackendType::kOpenGLES) {
blit_old_atlas = false;
- new_atlas = std::make_shared<GlyphAtlas>(type);
+ new_atlas = std::make_shared<GlyphAtlas>(
+ type, /*initial_generation=*/last_atlas->GetAtlasGeneration() + 1);
auto [update_glyphs, update_sizes] =
CollectNewGlyphs(new_atlas, text_frames);
diff --git a/impeller/typographer/font_glyph_pair.h b/impeller/typographer/font_glyph_pair.h
index e530f5b..e3df2da 100644
--- a/impeller/typographer/font_glyph_pair.h
+++ b/impeller/typographer/font_glyph_pair.h
@@ -20,6 +20,17 @@
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 @@
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/impeller/typographer/glyph_atlas.cc b/impeller/typographer/glyph_atlas.cc
index 0935938..9803866 100644
--- a/impeller/typographer/glyph_atlas.cc
+++ b/impeller/typographer/glyph_atlas.cc
@@ -12,7 +12,8 @@
namespace impeller {
GlyphAtlasContext::GlyphAtlasContext(GlyphAtlas::Type type)
- : atlas_(std::make_shared<GlyphAtlas>(type)), atlas_size_(ISize(0, 0)) {}
+ : atlas_(std::make_shared<GlyphAtlas>(type, /*initial_generation=*/0)),
+ atlas_size_(ISize(0, 0)) {}
GlyphAtlasContext::~GlyphAtlasContext() {}
@@ -45,7 +46,8 @@
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 @@
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/impeller/typographer/glyph_atlas.h b/impeller/typographer/glyph_atlas.h
index 87c5490..bf11ff0 100644
--- a/impeller/typographer/glyph_atlas.h
+++ b/impeller/typographer/glyph_atlas.h
@@ -58,8 +58,9 @@
/// @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 @@
///
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> texture_;
+ size_t generation_ = 0;
std::unordered_map<ScaledFont,
FontGlyphAtlas,
diff --git a/impeller/typographer/text_frame.cc b/impeller/typographer/text_frame.cc
index b72bb62..3535fb2 100644
--- a/impeller/typographer/text_frame.cc
+++ b/impeller/typographer/text_frame.cc
@@ -3,11 +3,25 @@
// found in the LICENSE file.
#include "impeller/typographer/text_frame.h"
+#include "impeller/geometry/scalar.h"
#include "impeller/typographer/font.h"
#include "impeller/typographer/font_glyph_pair.h"
namespace impeller {
+namespace {
+static bool TextPropertiesEquals(const std::optional<GlyphProperties>& a,
+ const std::optional<GlyphProperties>& 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<TextRun>& runs, Rect bounds, bool has_color)
@@ -37,7 +51,7 @@
}
// 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 @@
void TextFrame::SetPerFrameData(Scalar scale,
Point offset,
std::optional<GlyphProperties> 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 @@
}
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/impeller/typographer/text_frame.h b/impeller/typographer/text_frame.h
index 0604d6c..067c8eb 100644
--- a/impeller/typographer/text_frame.h
+++ b/impeller/typographer/text_frame.h
@@ -32,7 +32,7 @@
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 @@
Point offset,
std::optional<GlyphProperties> 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 @@
void ClearFrameBounds();
+ void SetAtlasGeneration(size_t value);
+
std::vector<TextRun> 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<FrameBounds> bound_values_;
Scalar scale_ = 0;
+ size_t generation_ = 0;
Point offset_;
std::optional<GlyphProperties> properties_;
};
diff --git a/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc
index 6367314..4b8cc1c 100644
--- a/impeller/typographer/typographer_unittests.cc
+++ b/impeller/typographer/typographer_unittests.cc
@@ -508,7 +508,7 @@
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 @@
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
