diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index 9fe792d2097ea..99b4deb4d726a 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -1278,7 +1278,11 @@ bool RenderTextInCanvasSkia(const std::shared_ptr& context, } // Create the Impeller text frame and draw it at the designated baseline. - auto frame = MakeTextFrameFromTextBlobSkia(blob); + auto maybe_frame = MakeTextFrameFromTextBlobSkia(blob); + if (!maybe_frame.has_value()) { + return false; + } + auto frame = maybe_frame.value(); Paint text_paint; text_paint.color = Color::Yellow().WithAlpha(options.alpha); @@ -1467,7 +1471,7 @@ TEST_P(AiksTest, CanRenderTextOutsideBoundaries) { { auto blob = SkTextBlob::MakeFromString(t.text, sk_font); ASSERT_NE(blob, nullptr); - auto frame = MakeTextFrameFromTextBlobSkia(blob); + auto frame = MakeTextFrameFromTextBlobSkia(blob).value(); canvas.DrawTextFrame(frame, Point(), text_paint); } canvas.Restore(); @@ -3060,7 +3064,12 @@ TEST_P(AiksTest, TextForegroundShaderWithTransform) { auto blob = SkTextBlob::MakeFromString("Hello", sk_font); ASSERT_NE(blob, nullptr); - auto frame = MakeTextFrameFromTextBlobSkia(blob); + auto maybe_frame = MakeTextFrameFromTextBlobSkia(blob); + ASSERT_TRUE(maybe_frame.has_value()); + if (!maybe_frame.has_value()) { + return; + } + auto frame = maybe_frame.value(); canvas.DrawTextFrame(frame, Point(), text_paint); ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); diff --git a/impeller/display_list/dl_dispatcher.cc b/impeller/display_list/dl_dispatcher.cc index 09f6417f5ad31..01f9b2766c1f0 100644 --- a/impeller/display_list/dl_dispatcher.cc +++ b/impeller/display_list/dl_dispatcher.cc @@ -1111,7 +1111,11 @@ void DlDispatcher::drawDisplayList( void DlDispatcher::drawTextBlob(const sk_sp& blob, SkScalar x, SkScalar y) { - const auto text_frame = MakeTextFrameFromTextBlobSkia(blob); + const auto maybe_text_frame = MakeTextFrameFromTextBlobSkia(blob); + if (!maybe_text_frame.has_value()) { + return; + } + const auto text_frame = maybe_text_frame.value(); if (paint_.style == Paint::Style::kStroke || paint_.color_source.GetType() != ColorSource::Type::kColor) { auto path = skia_conversions::PathDataFromTextBlob(blob); diff --git a/impeller/entity/contents/text_contents.cc b/impeller/entity/contents/text_contents.cc index d8e68b2929e65..a93967c938061 100644 --- a/impeller/entity/contents/text_contents.cc +++ b/impeller/entity/contents/text_contents.cc @@ -59,16 +59,8 @@ void TextContents::SetOffset(Vector2 offset) { offset_ = offset; } -std::optional TextContents::GetTextFrameBounds() const { - return frame_.GetBounds(); -} - std::optional TextContents::GetCoverage(const Entity& entity) const { - auto bounds = frame_.GetBounds(); - if (!bounds.has_value()) { - return std::nullopt; - } - return bounds->TransformBounds(entity.GetTransformation()); + return frame_.GetBounds().TransformBounds(entity.GetTransformation()); } void TextContents::PopulateGlyphAtlas( diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc index 7523efb5a4b14..5af2bd9d23b33 100644 --- a/impeller/entity/entity_unittests.cc +++ b/impeller/entity/entity_unittests.cc @@ -2174,7 +2174,7 @@ TEST_P(EntityTest, InheritOpacityTest) { SkFont font; font.setSize(30); auto blob = SkTextBlob::MakeFromString("A", font); - auto frame = MakeTextFrameFromTextBlobSkia(blob); + auto frame = MakeTextFrameFromTextBlobSkia(blob).value(); auto lazy_glyph_atlas = std::make_shared(TypographerContextSkia::Make()); lazy_glyph_atlas->AddTextFrame(frame, 1.0f); diff --git a/impeller/typographer/backends/skia/text_frame_skia.cc b/impeller/typographer/backends/skia/text_frame_skia.cc index 008e965d14cd5..11c256a8af918 100644 --- a/impeller/typographer/backends/skia/text_frame_skia.cc +++ b/impeller/typographer/backends/skia/text_frame_skia.cc @@ -39,16 +39,17 @@ static Rect ToRect(const SkRect& rect) { static constexpr Scalar kScaleSize = 100000.0f; -TextFrame MakeTextFrameFromTextBlobSkia(const sk_sp& blob) { +std::optional MakeTextFrameFromTextBlobSkia( + const sk_sp& blob) { + // Handling nullptr text blobs feels overly defensive here, as I don't + // actually know if this happens. if (!blob) { return {}; } - TextFrame frame; - + std::vector runs; + bool has_color = false; for (SkTextBlobRunIterator run(blob.get()); !run.done(); run.next()) { - TextRun text_run(ToFont(run)); - // TODO(jonahwilliams): ask Skia for a public API to look this up. // https://github.com/flutter/flutter/issues/112005 SkStrikeSpec strikeSpec = SkStrikeSpec::MakeWithNoDevice(run.font()); @@ -57,12 +58,6 @@ TextFrame MakeTextFrameFromTextBlobSkia(const sk_sp& blob) { const auto glyph_count = run.glyphCount(); const auto* glyphs = run.glyphs(); switch (run.positioning()) { - case SkTextBlobRunIterator::kDefault_Positioning: - FML_DLOG(ERROR) << "Unimplemented."; - break; - case SkTextBlobRunIterator::kHorizontal_Positioning: - FML_DLOG(ERROR) << "Unimplemented."; - break; case SkTextBlobRunIterator::kFull_Positioning: { std::vector glyph_bounds; glyph_bounds.resize(glyph_count); @@ -75,6 +70,8 @@ TextFrame MakeTextFrameFromTextBlobSkia(const sk_sp& blob) { font.setSize(kScaleSize); font.getBounds(glyphs, glyph_count, glyph_bounds.data(), nullptr); + std::vector positions; + positions.reserve(glyph_count); for (auto i = 0u; i < glyph_count; i++) { // kFull_Positioning has two scalars per glyph. const SkPoint* glyph_points = run.points(); @@ -82,25 +79,23 @@ TextFrame MakeTextFrameFromTextBlobSkia(const sk_sp& blob) { Glyph::Type type = paths.glyph(glyphs[i])->isColor() ? Glyph::Type::kBitmap : Glyph::Type::kPath; + has_color |= type == Glyph::Type::kBitmap; - text_run.AddGlyph( + positions.emplace_back(TextRun::GlyphPosition{ Glyph{glyphs[i], type, ToRect(glyph_bounds[i]).Scale(font_size / kScaleSize)}, - Point{point->x(), point->y()}); + Point{point->x(), point->y()}}); } + TextRun text_run(ToFont(run), positions); + runs.emplace_back(text_run); break; } - case SkTextBlobRunIterator::kRSXform_Positioning: - FML_DLOG(ERROR) << "Unimplemented."; - break; default: FML_DLOG(ERROR) << "Unimplemented."; continue; } - frame.AddTextRun(std::move(text_run)); } - - return frame; + return TextFrame(runs, ToRect(blob->bounds()), has_color); } } // namespace impeller diff --git a/impeller/typographer/backends/skia/text_frame_skia.h b/impeller/typographer/backends/skia/text_frame_skia.h index 93431446afe32..53170a248d3a1 100644 --- a/impeller/typographer/backends/skia/text_frame_skia.h +++ b/impeller/typographer/backends/skia/text_frame_skia.h @@ -10,6 +10,7 @@ namespace impeller { -TextFrame MakeTextFrameFromTextBlobSkia(const sk_sp& blob); +std::optional MakeTextFrameFromTextBlobSkia( + const sk_sp& blob); } // namespace impeller diff --git a/impeller/typographer/backends/stb/text_frame_stb.cc b/impeller/typographer/backends/stb/text_frame_stb.cc index f2d9e9177509e..5e6060ba32551 100644 --- a/impeller/typographer/backends/stb/text_frame_stb.cc +++ b/impeller/typographer/backends/stb/text_frame_stb.cc @@ -52,10 +52,16 @@ TextFrame MakeTextFrameSTB(const std::shared_ptr& typeface_stb, } } - TextFrame frame; - frame.AddTextRun(std::move(run)); + std::optional result; + for (const auto& glyph_position : run.GetGlyphPositions()) { + Rect glyph_rect = + Rect(glyph_position.position + glyph_position.glyph.bounds.origin, + glyph_position.glyph.bounds.size); + result = result.has_value() ? result->Union(glyph_rect) : glyph_rect; + } - return frame; + std::vector runs = {run}; + return TextFrame(runs, result.value_or(Rect::MakeLTRB(0, 0, 0, 0)), false); } } // namespace impeller diff --git a/impeller/typographer/text_frame.cc b/impeller/typographer/text_frame.cc index 898f69bdfe5e1..9c02a7393bd62 100644 --- a/impeller/typographer/text_frame.cc +++ b/impeller/typographer/text_frame.cc @@ -8,30 +8,13 @@ namespace impeller { TextFrame::TextFrame() = default; -TextFrame::~TextFrame() = default; - -std::optional TextFrame::GetBounds() const { - std::optional result; - - for (const auto& run : runs_) { - for (const auto& glyph_position : run.GetGlyphPositions()) { - Rect glyph_rect = - Rect(glyph_position.position + glyph_position.glyph.bounds.origin, - glyph_position.glyph.bounds.size); - result = result.has_value() ? result->Union(glyph_rect) : glyph_rect; - } - } +TextFrame::TextFrame(std::vector& runs, Rect bounds, bool has_color) + : runs_(std::move(runs)), bounds_(bounds), has_color_(has_color) {} - return result; -} +TextFrame::~TextFrame() = default; -bool TextFrame::AddTextRun(TextRun&& run) { - if (!run.IsValid()) { - return false; - } - has_color_ |= run.HasColor(); - runs_.emplace_back(std::move(run)); - return true; +Rect TextFrame::GetBounds() const { + return bounds_; } size_t TextFrame::GetRunCount() const { diff --git a/impeller/typographer/text_frame.h b/impeller/typographer/text_frame.h index 54ea12df30f6b..35316351e66f0 100644 --- a/impeller/typographer/text_frame.h +++ b/impeller/typographer/text_frame.h @@ -20,6 +20,8 @@ class TextFrame { public: TextFrame(); + TextFrame(std::vector& runs, Rect bounds, bool has_color); + ~TextFrame(); void CollectUniqueFontGlyphPairs(FontGlyphPair::Set& set, Scalar scale) const; @@ -30,9 +32,9 @@ class TextFrame { /// @brief The conservative bounding box for this text frame. /// /// @return The bounds rectangle. If there are no glyphs in this text - /// frame, std::nullopt is returned. + /// frame and empty Rectangle is returned instead. /// - std::optional GetBounds() const; + Rect GetBounds() const; //---------------------------------------------------------------------------- /// @brief The number of runs in this text frame. @@ -41,15 +43,6 @@ class TextFrame { /// size_t GetRunCount() const; - //---------------------------------------------------------------------------- - /// @brief Adds a new text run to the text frame. - /// - /// @param[in] run The run - /// - /// @return If the text run could be added to this frame. - /// - bool AddTextRun(TextRun&& run); - //---------------------------------------------------------------------------- /// @brief Returns a reference to all the text runs in this frame. /// @@ -77,6 +70,7 @@ class TextFrame { private: std::vector runs_; + Rect bounds_; bool has_color_ = false; }; diff --git a/impeller/typographer/text_run.cc b/impeller/typographer/text_run.cc index ee0af62b5029f..6fc3a3ba261c4 100644 --- a/impeller/typographer/text_run.cc +++ b/impeller/typographer/text_run.cc @@ -13,11 +13,18 @@ TextRun::TextRun(const Font& font) : font_(font) { is_valid_ = true; } +TextRun::TextRun(const Font& font, std::vector& glyphs) + : font_(font), glyphs_(std::move(glyphs)) { + if (!font_.IsValid()) { + return; + } + is_valid_ = true; +} + TextRun::~TextRun() = default; bool TextRun::AddGlyph(Glyph glyph, Point position) { glyphs_.emplace_back(GlyphPosition{glyph, position}); - has_color_ |= glyph.type == Glyph::Type::kBitmap; return true; } @@ -37,8 +44,4 @@ const Font& TextRun::GetFont() const { return font_; } -bool TextRun::HasColor() const { - return has_color_; -} - } // namespace impeller diff --git a/impeller/typographer/text_run.h b/impeller/typographer/text_run.h index 2bf433afca2f8..340db94d9f40d 100644 --- a/impeller/typographer/text_run.h +++ b/impeller/typographer/text_run.h @@ -31,7 +31,9 @@ class TextRun { /// /// @param[in] font The font /// - TextRun(const Font& font); + explicit TextRun(const Font& font); + + TextRun(const Font& font, std::vector& glyphs); ~TextRun(); @@ -68,15 +70,10 @@ class TextRun { /// const Font& GetFont() const; - //---------------------------------------------------------------------------- - /// @brief Whether any glyph in this run has color. - bool HasColor() const; - private: Font font_; std::vector glyphs_; bool is_valid_ = false; - bool has_color_ = false; }; } // namespace impeller diff --git a/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc index 4a2bed4dc52cc..fd412297b4941 100644 --- a/impeller/typographer/typographer_unittests.cc +++ b/impeller/typographer/typographer_unittests.cc @@ -40,7 +40,7 @@ TEST_P(TypographerTest, CanConvertTextBlob) { auto blob = SkTextBlob::MakeFromString( "the quick brown fox jumped over the lazy dog.", font); ASSERT_TRUE(blob); - auto frame = MakeTextFrameFromTextBlobSkia(blob); + auto frame = MakeTextFrameFromTextBlobSkia(blob).value(); ASSERT_EQ(frame.GetRunCount(), 1u); for (const auto& run : frame.GetRuns()) { ASSERT_TRUE(run.IsValid()); @@ -62,7 +62,7 @@ TEST_P(TypographerTest, CanCreateGlyphAtlas) { ASSERT_TRUE(blob); auto atlas = CreateGlyphAtlas( *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, - atlas_context, MakeTextFrameFromTextBlobSkia(blob)); + atlas_context, MakeTextFrameFromTextBlobSkia(blob).value()); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); ASSERT_EQ(atlas->GetType(), GlyphAtlas::Type::kAlphaBitmap); @@ -108,7 +108,7 @@ TEST_P(TypographerTest, LazyAtlasTracksColor) { auto blob = SkTextBlob::MakeFromString("hello", sk_font); ASSERT_TRUE(blob); - auto frame = MakeTextFrameFromTextBlobSkia(blob); + auto frame = MakeTextFrameFromTextBlobSkia(blob).value(); ASSERT_FALSE(frame.GetAtlasType() == GlyphAtlas::Type::kColorBitmap); @@ -117,7 +117,8 @@ TEST_P(TypographerTest, LazyAtlasTracksColor) { lazy_atlas.AddTextFrame(frame, 1.0f); frame = MakeTextFrameFromTextBlobSkia( - SkTextBlob::MakeFromString("😀 ", emoji_font)); + SkTextBlob::MakeFromString("😀 ", emoji_font)) + .value(); ASSERT_TRUE(frame.GetAtlasType() == GlyphAtlas::Type::kColorBitmap); @@ -142,7 +143,7 @@ TEST_P(TypographerTest, GlyphAtlasWithOddUniqueGlyphSize) { ASSERT_TRUE(blob); auto atlas = CreateGlyphAtlas( *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, - atlas_context, MakeTextFrameFromTextBlobSkia(blob)); + atlas_context, MakeTextFrameFromTextBlobSkia(blob).value()); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); @@ -159,7 +160,7 @@ TEST_P(TypographerTest, GlyphAtlasIsRecycledIfUnchanged) { ASSERT_TRUE(blob); auto atlas = CreateGlyphAtlas( *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, - atlas_context, MakeTextFrameFromTextBlobSkia(blob)); + atlas_context, MakeTextFrameFromTextBlobSkia(blob).value()); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); ASSERT_EQ(atlas, atlas_context->GetGlyphAtlas()); @@ -168,7 +169,7 @@ TEST_P(TypographerTest, GlyphAtlasIsRecycledIfUnchanged) { auto next_atlas = CreateGlyphAtlas( *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, - atlas_context, MakeTextFrameFromTextBlobSkia(blob)); + atlas_context, MakeTextFrameFromTextBlobSkia(blob).value()); ASSERT_EQ(atlas, next_atlas); ASSERT_EQ(atlas_context->GetGlyphAtlas(), atlas); } @@ -191,7 +192,7 @@ TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) { FontGlyphPair::Set set; size_t size_count = 8; for (size_t index = 0; index < size_count; index += 1) { - MakeTextFrameFromTextBlobSkia(blob).CollectUniqueFontGlyphPairs( + MakeTextFrameFromTextBlobSkia(blob).value().CollectUniqueFontGlyphPairs( set, 0.6 * index); }; auto atlas = @@ -225,7 +226,7 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecycledIfUnchanged) { ASSERT_TRUE(blob); auto atlas = CreateGlyphAtlas( *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, - atlas_context, MakeTextFrameFromTextBlobSkia(blob)); + atlas_context, MakeTextFrameFromTextBlobSkia(blob).value()); auto old_packer = atlas_context->GetRectPacker(); ASSERT_NE(atlas, nullptr); @@ -239,7 +240,7 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecycledIfUnchanged) { auto blob2 = SkTextBlob::MakeFromString("spooky 2", sk_font); auto next_atlas = CreateGlyphAtlas( *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, - atlas_context, MakeTextFrameFromTextBlobSkia(blob2)); + atlas_context, MakeTextFrameFromTextBlobSkia(blob2).value()); ASSERT_EQ(atlas, next_atlas); auto* second_texture = next_atlas->GetTexture().get(); @@ -258,7 +259,7 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecreatedIfTypeChanges) { ASSERT_TRUE(blob); auto atlas = CreateGlyphAtlas( *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, - atlas_context, MakeTextFrameFromTextBlobSkia(blob)); + atlas_context, MakeTextFrameFromTextBlobSkia(blob).value()); auto old_packer = atlas_context->GetRectPacker(); ASSERT_NE(atlas, nullptr); @@ -273,7 +274,7 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecreatedIfTypeChanges) { auto blob2 = SkTextBlob::MakeFromString("spooky 1", sk_font); auto next_atlas = CreateGlyphAtlas( *GetContext(), context.get(), GlyphAtlas::Type::kColorBitmap, 1.0f, - atlas_context, MakeTextFrameFromTextBlobSkia(blob2)); + atlas_context, MakeTextFrameFromTextBlobSkia(blob2).value()); ASSERT_NE(atlas, next_atlas); auto* second_texture = next_atlas->GetTexture().get(); @@ -308,12 +309,14 @@ TEST_P(TypographerTest, MaybeHasOverlapping) { SkFont sk_font(typeface, 0.5f); auto frame = - MakeTextFrameFromTextBlobSkia(SkTextBlob::MakeFromString("1", sk_font)); + MakeTextFrameFromTextBlobSkia(SkTextBlob::MakeFromString("1", sk_font)) + .value(); // Single character has no overlapping ASSERT_FALSE(frame.MaybeHasOverlapping()); auto frame_2 = MakeTextFrameFromTextBlobSkia( - SkTextBlob::MakeFromString("123456789", sk_font)); + SkTextBlob::MakeFromString("123456789", sk_font)) + .value(); ASSERT_FALSE(frame_2.MaybeHasOverlapping()); }