From 8e6f1d0bffcdcc4c3f49e84c2fdedafdeb35f207 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 14 Jul 2023 11:03:01 -0700 Subject: [PATCH 1/3] Revert "Revert https://github.com/flutter/engine/pull/43533 (#43654)" This reverts commit 255ef6cb38d1cfc1696e4cc8d7297cebabf80433. --- impeller/aiks/aiks_unittests.cc | 29 ++++++ impeller/aiks/canvas.cc | 5 - impeller/aiks/canvas.h | 1 - impeller/display_list/dl_dispatcher.cc | 3 +- .../contents/color_source_text_contents.cc | 7 ++ .../contents/color_source_text_contents.h | 5 + impeller/entity/contents/content_context.h | 10 ++ impeller/entity/contents/contents.h | 6 ++ impeller/entity/contents/text_contents.cc | 99 +++++++++---------- impeller/entity/contents/text_contents.h | 11 ++- impeller/entity/entity.cc | 4 + impeller/entity/entity.h | 2 + impeller/entity/entity_pass.cc | 35 +++++++ impeller/entity/entity_pass.h | 6 ++ impeller/entity/entity_unittests.cc | 2 +- .../backends/skia/text_frame_skia.cc | 7 +- .../backends/skia/text_frame_skia.h | 3 +- .../backends/skia/text_render_context_skia.cc | 59 ++++------- .../backends/skia/text_render_context_skia.h | 2 +- impeller/typographer/font.h | 12 +-- impeller/typographer/font_glyph_pair.h | 11 ++- impeller/typographer/lazy_glyph_atlas.cc | 20 +--- impeller/typographer/lazy_glyph_atlas.h | 6 +- impeller/typographer/text_frame.cc | 11 +++ impeller/typographer/text_frame.h | 2 + impeller/typographer/text_render_context.cc | 15 --- impeller/typographer/text_render_context.h | 9 +- impeller/typographer/typographer_unittests.cc | 65 ++++++------ 28 files changed, 248 insertions(+), 199 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index b8a5c7dc73357..7fe1b6bcaab8d 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -2877,6 +2877,35 @@ TEST_P(AiksTest, CanCanvasDrawPicture) { ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); } +TEST_P(AiksTest, DrawPictureWithText) { + Canvas subcanvas; + ASSERT_TRUE(RenderTextInCanvas( + GetContext(), subcanvas, + "the quick brown fox jumped over the lazy dog!.?", "Roboto-Regular.ttf")); + subcanvas.Translate({0, 10}); + subcanvas.Scale(Vector2(3, 3)); + ASSERT_TRUE(RenderTextInCanvas( + GetContext(), subcanvas, + "the quick brown fox jumped over the very big lazy dog!.?", + "Roboto-Regular.ttf")); + auto picture = subcanvas.EndRecordingAsPicture(); + + Canvas canvas; + canvas.Scale(Vector2(.2, .2)); + canvas.Save(); + canvas.Translate({200, 200}); + canvas.Scale(Vector2(3.5, 3.5)); // The text must not be blurry after this. + canvas.DrawPicture(picture); + canvas.Restore(); + + canvas.Scale(Vector2(1.5, 1.5)); + ASSERT_TRUE(RenderTextInCanvas( + GetContext(), canvas, + "the quick brown fox jumped over the smaller lazy dog!.?", + "Roboto-Regular.ttf")); + ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); +} + TEST_P(AiksTest, MatrixBackdropFilter) { Canvas canvas; canvas.SaveLayer({}, std::nullopt, diff --git a/impeller/aiks/canvas.cc b/impeller/aiks/canvas.cc index 82424b2c86d59..9e9b89e25a041 100644 --- a/impeller/aiks/canvas.cc +++ b/impeller/aiks/canvas.cc @@ -42,7 +42,6 @@ void Canvas::Initialize(std::optional cull_rect) { base_pass_ = std::make_unique(); current_pass_ = base_pass_.get(); xformation_stack_.emplace_back(CanvasStackEntry{.cull_rect = cull_rect}); - lazy_glyph_atlas_ = std::make_shared(); FML_DCHECK(GetSaveCount() == 1u); FML_DCHECK(base_pass_->GetSubpassesDepth() == 1u); } @@ -51,7 +50,6 @@ void Canvas::Reset() { base_pass_ = nullptr; current_pass_ = nullptr; xformation_stack_ = {}; - lazy_glyph_atlas_ = nullptr; } void Canvas::Save() { @@ -516,15 +514,12 @@ void Canvas::SaveLayer(const Paint& paint, void Canvas::DrawTextFrame(const TextFrame& text_frame, Point position, const Paint& paint) { - lazy_glyph_atlas_->AddTextFrame(text_frame); - Entity entity; entity.SetStencilDepth(GetStencilDepth()); entity.SetBlendMode(paint.blend_mode); auto text_contents = std::make_shared(); text_contents->SetTextFrame(text_frame); - text_contents->SetGlyphAtlas(lazy_glyph_atlas_); if (paint.color_source.GetType() != ColorSource::Type::kColor) { auto color_text_contents = std::make_shared(); diff --git a/impeller/aiks/canvas.h b/impeller/aiks/canvas.h index aa3da9f0681a2..84f1c486bc30f 100644 --- a/impeller/aiks/canvas.h +++ b/impeller/aiks/canvas.h @@ -162,7 +162,6 @@ class Canvas { std::unique_ptr base_pass_; EntityPass* current_pass_ = nullptr; std::deque xformation_stack_; - std::shared_ptr lazy_glyph_atlas_; std::optional initial_cull_rect_; void Initialize(std::optional cull_rect); diff --git a/impeller/display_list/dl_dispatcher.cc b/impeller/display_list/dl_dispatcher.cc index 51ba0ea924782..9964e9e71eaf9 100644 --- a/impeller/display_list/dl_dispatcher.cc +++ b/impeller/display_list/dl_dispatcher.cc @@ -1104,8 +1104,7 @@ void DlDispatcher::drawDisplayList( void DlDispatcher::drawTextBlob(const sk_sp blob, SkScalar x, SkScalar y) { - Scalar scale = canvas_.GetCurrentTransformation().GetMaxBasisLengthXY(); - const auto text_frame = TextFrameFromTextBlob(blob, scale); + const auto text_frame = TextFrameFromTextBlob(blob); if (paint_.style == Paint::Style::kStroke) { auto path = skia_conversions::PathDataFromTextBlob(blob); auto bounds = text_frame.GetBounds(); diff --git a/impeller/entity/contents/color_source_text_contents.cc b/impeller/entity/contents/color_source_text_contents.cc index 16407e0bb2da0..a09e44c8dcb35 100644 --- a/impeller/entity/contents/color_source_text_contents.cc +++ b/impeller/entity/contents/color_source_text_contents.cc @@ -4,6 +4,7 @@ #include "impeller/entity/contents/color_source_text_contents.h" +#include "color_source_text_contents.h" #include "impeller/entity/contents/content_context.h" #include "impeller/entity/contents/texture_contents.h" #include "impeller/renderer/render_pass.h" @@ -33,6 +34,12 @@ void ColorSourceTextContents::SetTextPosition(Point position) { position_ = position; } +void ColorSourceTextContents::PopulateGlyphAtlas( + const std::shared_ptr& lazy_glyph_atlas, + Scalar scale) const { + text_contents_->PopulateGlyphAtlas(lazy_glyph_atlas, scale); +} + bool ColorSourceTextContents::Render(const ContentContext& renderer, const Entity& entity, RenderPass& pass) const { diff --git a/impeller/entity/contents/color_source_text_contents.h b/impeller/entity/contents/color_source_text_contents.h index e93738c8f3760..d66a7ea313458 100644 --- a/impeller/entity/contents/color_source_text_contents.h +++ b/impeller/entity/contents/color_source_text_contents.h @@ -32,6 +32,11 @@ class ColorSourceTextContents final : public Contents { // |Contents| std::optional GetCoverage(const Entity& entity) const override; + // |Contents| + void PopulateGlyphAtlas( + const std::shared_ptr& lazy_glyph_atlas, + Scalar scale) const override; + // |Contents| bool Render(const ContentContext& renderer, const Entity& entity, diff --git a/impeller/entity/contents/content_context.h b/impeller/entity/contents/content_context.h index d2e1a16c8f92c..7693dfd3622f7 100644 --- a/impeller/entity/contents/content_context.h +++ b/impeller/entity/contents/content_context.h @@ -696,8 +696,18 @@ class ContentContext { const SubpassCallback& subpass_callback, bool msaa_enabled = true) const; + void SetLazyGlyphAtlas( + const std::shared_ptr& lazy_glyph_atlas) { + lazy_glyph_atlas_ = lazy_glyph_atlas; + } + + std::shared_ptr GetLazyGlyphAtlas() const { + return lazy_glyph_atlas_; + } + private: std::shared_ptr context_; + std::shared_ptr lazy_glyph_atlas_; template using Variants = std::unordered_map& lazy_glyph_atlas, + Scalar scale) const {} + virtual bool Render(const ContentContext& renderer, const Entity& entity, RenderPass& pass) const = 0; diff --git a/impeller/entity/contents/text_contents.cc b/impeller/entity/contents/text_contents.cc index a4e46641b2e14..5428cf830feba 100644 --- a/impeller/entity/contents/text_contents.cc +++ b/impeller/entity/contents/text_contents.cc @@ -29,18 +29,15 @@ void TextContents::SetTextFrame(const TextFrame& frame) { frame_ = frame; } -void TextContents::SetGlyphAtlas(std::shared_ptr atlas) { - lazy_atlas_ = std::move(atlas); -} - std::shared_ptr TextContents::ResolveAtlas( GlyphAtlas::Type type, + const std::shared_ptr& lazy_atlas, std::shared_ptr atlas_context, std::shared_ptr context) const { - FML_DCHECK(lazy_atlas_); - if (lazy_atlas_) { - return lazy_atlas_->CreateOrGetGlyphAtlas(type, std::move(atlas_context), - std::move(context)); + FML_DCHECK(lazy_atlas); + if (lazy_atlas) { + return lazy_atlas->CreateOrGetGlyphAtlas(type, std::move(atlas_context), + std::move(context)); } return nullptr; @@ -78,14 +75,43 @@ std::optional TextContents::GetCoverage(const Entity& entity) const { return bounds->TransformBounds(entity.GetTransformation()); } -static bool CommonRender(const ContentContext& renderer, - const Entity& entity, - RenderPass& pass, - const Color& color, - const TextFrame& frame, - Vector2 offset, - const std::shared_ptr& atlas, - Command& cmd) { +void TextContents::PopulateGlyphAtlas( + const std::shared_ptr& lazy_glyph_atlas, + Scalar scale) const { + lazy_glyph_atlas->AddTextFrame(frame_, scale); +} + +bool TextContents::Render(const ContentContext& renderer, + const Entity& entity, + RenderPass& pass) const { + auto color = GetColor(); + if (color.IsTransparent()) { + return true; + } + + auto type = frame_.GetAtlasType(); + auto scale = entity.DeriveTextScale(); + auto atlas = + ResolveAtlas(type, renderer.GetLazyGlyphAtlas(), + renderer.GetGlyphAtlasContext(type), renderer.GetContext()); + + if (!atlas || !atlas->IsValid()) { + VALIDATION_LOG << "Cannot render glyphs without prepared atlas."; + return false; + } + + // Information shared by all glyph draw calls. + Command cmd; + cmd.label = "TextFrame"; + auto opts = OptionsFromPassAndEntity(pass, entity); + opts.primitive_type = PrimitiveType::kTriangle; + if (type == GlyphAtlas::Type::kAlphaBitmap) { + cmd.pipeline = renderer.GetGlyphAtlasPipeline(opts); + } else { + cmd.pipeline = renderer.GetGlyphAtlasColorPipeline(opts); + } + cmd.stencil_reference = entity.GetStencilDepth(); + using VS = GlyphAtlasPipeline::VertexShader; using FS = GlyphAtlasPipeline::FragmentShader; @@ -95,7 +121,7 @@ static bool CommonRender(const ContentContext& renderer, frame_info.atlas_size = Vector2{static_cast(atlas->GetTexture()->GetSize().width), static_cast(atlas->GetTexture()->GetSize().height)}; - frame_info.offset = offset; + frame_info.offset = offset_; frame_info.is_translation_scale = entity.GetTransformation().IsTranslationScaleOnly(); frame_info.entity_transform = entity.GetTransformation(); @@ -141,7 +167,7 @@ static bool CommonRender(const ContentContext& renderer, auto& host_buffer = pass.GetTransientsBuffer(); size_t vertex_count = 0; - for (const auto& run : frame.GetRuns()) { + for (const auto& run : frame_.GetRuns()) { vertex_count += run.GetGlyphPositions().size(); } vertex_count *= 6; @@ -151,10 +177,10 @@ static bool CommonRender(const ContentContext& renderer, [&](uint8_t* contents) { VS::PerVertexData vtx; size_t vertex_offset = 0; - for (const auto& run : frame.GetRuns()) { + for (const auto& run : frame_.GetRuns()) { const Font& font = run.GetFont(); for (const auto& glyph_position : run.GetGlyphPositions()) { - FontGlyphPair font_glyph_pair{font, glyph_position.glyph}; + FontGlyphPair font_glyph_pair{font, glyph_position.glyph, scale}; auto maybe_atlas_glyph_bounds = atlas->FindFontGlyphBounds(font_glyph_pair); if (!maybe_atlas_glyph_bounds.has_value()) { @@ -191,37 +217,4 @@ static bool CommonRender(const ContentContext& renderer, return pass.AddCommand(cmd); } -bool TextContents::Render(const ContentContext& renderer, - const Entity& entity, - RenderPass& pass) const { - auto color = GetColor(); - if (color.IsTransparent()) { - return true; - } - - auto type = frame_.GetAtlasType(); - auto atlas = ResolveAtlas(type, renderer.GetGlyphAtlasContext(type), - renderer.GetContext()); - - if (!atlas || !atlas->IsValid()) { - VALIDATION_LOG << "Cannot render glyphs without prepared atlas."; - return false; - } - - // Information shared by all glyph draw calls. - Command cmd; - cmd.label = "TextFrame"; - auto opts = OptionsFromPassAndEntity(pass, entity); - opts.primitive_type = PrimitiveType::kTriangle; - if (type == GlyphAtlas::Type::kAlphaBitmap) { - cmd.pipeline = renderer.GetGlyphAtlasPipeline(opts); - } else { - cmd.pipeline = renderer.GetGlyphAtlasColorPipeline(opts); - } - cmd.stencil_reference = entity.GetStencilDepth(); - - return CommonRender(renderer, entity, pass, color, frame_, offset_, atlas, - cmd); -} - } // namespace impeller diff --git a/impeller/entity/contents/text_contents.h b/impeller/entity/contents/text_contents.h index 14bd354086e72..fe89068ae2a14 100644 --- a/impeller/entity/contents/text_contents.h +++ b/impeller/entity/contents/text_contents.h @@ -28,14 +28,14 @@ class TextContents final : public Contents { void SetTextFrame(const TextFrame& frame); - void SetGlyphAtlas(std::shared_ptr atlas); - void SetColor(Color color); Color GetColor() const; + // |Contents| bool CanInheritOpacity(const Entity& entity) const override; + // |Contents| void SetInheritedOpacity(Scalar opacity) override; void SetOffset(Vector2 offset); @@ -45,6 +45,11 @@ class TextContents final : public Contents { // |Contents| std::optional GetCoverage(const Entity& entity) const override; + // |Contents| + void PopulateGlyphAtlas( + const std::shared_ptr& lazy_glyph_atlas, + Scalar scale) const override; + // |Contents| bool Render(const ContentContext& renderer, const Entity& entity, @@ -54,11 +59,11 @@ class TextContents final : public Contents { TextFrame frame_; Color color_; Scalar inherited_opacity_ = 1.0; - mutable std::shared_ptr lazy_atlas_; Vector2 offset_; std::shared_ptr ResolveAtlas( GlyphAtlas::Type type, + const std::shared_ptr& lazy_atlas, std::shared_ptr atlas_context, std::shared_ptr context) const; diff --git a/impeller/entity/entity.cc b/impeller/entity/entity.cc index d0b2226735fc5..ad6a6842d68ba 100644 --- a/impeller/entity/entity.cc +++ b/impeller/entity/entity.cc @@ -166,4 +166,8 @@ bool Entity::Render(const ContentContext& renderer, return contents_->Render(renderer, *this, parent_pass); } +Scalar Entity::DeriveTextScale() const { + return GetTransformation().GetMaxBasisLengthXY(); +} + } // namespace impeller diff --git a/impeller/entity/entity.h b/impeller/entity/entity.h index 2063213b18072..271f9a0db8248 100644 --- a/impeller/entity/entity.h +++ b/impeller/entity/entity.h @@ -94,6 +94,8 @@ class Entity { std::optional AsBackgroundColor(ISize target_size) const; + Scalar DeriveTextScale() const; + private: Matrix transformation_; std::shared_ptr contents_; diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index 19b1a2efd1a33..af074caebd278 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -8,6 +8,7 @@ #include #include +#include "flutter/fml/closure.h" #include "flutter/fml/logging.h" #include "flutter/fml/macros.h" #include "flutter/fml/trace_event.h" @@ -252,6 +253,18 @@ bool EntityPass::Render(ContentContext& renderer, return false; } + renderer.SetLazyGlyphAtlas(std::make_shared()); + fml::ScopedCleanupClosure reset_lazy_glyph_atlas( + [&renderer]() { renderer.SetLazyGlyphAtlas(nullptr); }); + + IterateAllEntities([lazy_glyph_atlas = + renderer.GetLazyGlyphAtlas()](const Entity& entity) { + if (auto contents = entity.GetContents()) { + contents->PopulateGlyphAtlas(lazy_glyph_atlas, entity.DeriveTextScale()); + } + return true; + }); + StencilCoverageStack stencil_coverage_stack = {StencilCoverageLayer{ .coverage = Rect::MakeSize(root_render_target.GetRenderTargetSize()), .stencil_depth = 0}}; @@ -878,6 +891,28 @@ void EntityPass::IterateAllEntities( } } +void EntityPass::IterateAllEntities( + const std::function& iterator) const { + if (!iterator) { + return; + } + + for (const auto& element : elements_) { + if (auto entity = std::get_if(&element)) { + if (!iterator(*entity)) { + return; + } + continue; + } + if (auto subpass = std::get_if>(&element)) { + const EntityPass* entity_pass = subpass->get(); + entity_pass->IterateAllEntities(iterator); + continue; + } + FML_UNREACHABLE(); + } +} + bool EntityPass::IterateUntilSubpass( const std::function& iterator) { if (!iterator) { diff --git a/impeller/entity/entity_pass.h b/impeller/entity/entity_pass.h index e3dbada3d7eed..d90abfb99e568 100644 --- a/impeller/entity/entity_pass.h +++ b/impeller/entity/entity_pass.h @@ -79,6 +79,12 @@ class EntityPass { /// of child passes. The iteration order is depth-first. void IterateAllEntities(const std::function& iterator); + /// @brief Iterate all entities in this pass, recursively including entities + /// of child passes. The iteration order is depth-first and does not + /// allow modification of the entities. + void IterateAllEntities( + const std::function& iterator) const; + /// @brief Iterate entities in this pass up until the first subpass is found. /// This is useful for limiting look-ahead optimizations. /// diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc index 28bc4d8d11493..05818e84270f7 100644 --- a/impeller/entity/entity_unittests.cc +++ b/impeller/entity/entity_unittests.cc @@ -2184,7 +2184,7 @@ TEST_P(EntityTest, InheritOpacityTest) { auto blob = SkTextBlob::MakeFromString("A", font); auto frame = TextFrameFromTextBlob(blob); auto lazy_glyph_atlas = std::make_shared(); - lazy_glyph_atlas->AddTextFrame(frame); + lazy_glyph_atlas->AddTextFrame(frame, 1.0f); auto text_contents = std::make_shared(); text_contents->SetTextFrame(frame); diff --git a/impeller/typographer/backends/skia/text_frame_skia.cc b/impeller/typographer/backends/skia/text_frame_skia.cc index e451695dc8b6d..cac02a80968b6 100644 --- a/impeller/typographer/backends/skia/text_frame_skia.cc +++ b/impeller/typographer/backends/skia/text_frame_skia.cc @@ -17,7 +17,7 @@ namespace impeller { -static Font ToFont(const SkTextBlobRunIterator& run, Scalar scale) { +static Font ToFont(const SkTextBlobRunIterator& run) { auto& font = run.font(); auto typeface = std::make_shared(font.refTypefaceOrDefault()); @@ -25,7 +25,6 @@ static Font ToFont(const SkTextBlobRunIterator& run, Scalar scale) { font.getMetrics(&sk_metrics); Font::Metrics metrics; - metrics.scale = scale; metrics.point_size = font.getSize(); metrics.embolden = font.isEmbolden(); metrics.skewX = font.getSkewX(); @@ -38,7 +37,7 @@ static Rect ToRect(const SkRect& rect) { return Rect::MakeLTRB(rect.fLeft, rect.fTop, rect.fRight, rect.fBottom); } -TextFrame TextFrameFromTextBlob(const sk_sp& blob, Scalar scale) { +TextFrame TextFrameFromTextBlob(const sk_sp& blob) { if (!blob) { return {}; } @@ -46,7 +45,7 @@ TextFrame TextFrameFromTextBlob(const sk_sp& blob, Scalar scale) { TextFrame frame; for (SkTextBlobRunIterator run(blob.get()); !run.done(); run.next()) { - TextRun text_run(ToFont(run, scale)); + TextRun text_run(ToFont(run)); // TODO(jonahwilliams): ask Skia for a public API to look this up. // https://github.com/flutter/flutter/issues/112005 diff --git a/impeller/typographer/backends/skia/text_frame_skia.h b/impeller/typographer/backends/skia/text_frame_skia.h index 80d6c33921fa3..a12b0f5ec60fa 100644 --- a/impeller/typographer/backends/skia/text_frame_skia.h +++ b/impeller/typographer/backends/skia/text_frame_skia.h @@ -10,7 +10,6 @@ namespace impeller { -TextFrame TextFrameFromTextBlob(const sk_sp& blob, - Scalar scale = 1.0f); +TextFrame TextFrameFromTextBlob(const sk_sp& blob); } // namespace impeller diff --git a/impeller/typographer/backends/skia/text_render_context_skia.cc b/impeller/typographer/backends/skia/text_render_context_skia.cc index 9028a5cba0cc3..8553ce87a7ed4 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.cc +++ b/impeller/typographer/backends/skia/text_render_context_skia.cc @@ -40,23 +40,6 @@ TextRenderContextSkia::TextRenderContextSkia(std::shared_ptr context) TextRenderContextSkia::~TextRenderContextSkia() = default; -static FontGlyphPair::Set CollectUniqueFontGlyphPairs( - GlyphAtlas::Type type, - const TextRenderContext::FrameIterator& frame_iterator) { - TRACE_EVENT0("impeller", __FUNCTION__); - FontGlyphPair::Set set; - while (const TextFrame* frame = frame_iterator()) { - for (const TextRun& run : frame->GetRuns()) { - const Font& font = run.GetFont(); - for (const TextRun::GlyphPosition& glyph_position : - run.GetGlyphPositions()) { - set.insert({font, glyph_position.glyph}); - } - } - } - return set; -} - static size_t PairsFitInAtlasOfSize( const FontGlyphPair::Set& pairs, const ISize& atlas_size, @@ -73,8 +56,7 @@ static size_t PairsFitInAtlasOfSize( for (auto it = pairs.begin(); it != pairs.end(); ++i, ++it) { const auto& pair = *it; - const auto glyph_size = - ISize::Ceil((pair.glyph.bounds * pair.font.GetMetrics().scale).size); + const auto glyph_size = ISize::Ceil((pair.glyph.bounds * pair.scale).size); IPoint16 location_in_atlas; if (!rect_packer->addRect(glyph_size.width + kPadding, // glyph_size.height + kPadding, // @@ -111,8 +93,7 @@ static bool CanAppendToExistingAtlas( for (size_t i = 0; i < extra_pairs.size(); i++) { const FontGlyphPair& pair = extra_pairs[i]; - const auto glyph_size = - ISize::Ceil((pair.glyph.bounds * pair.font.GetMetrics().scale).size); + const auto glyph_size = ISize::Ceil((pair.glyph.bounds * pair.scale).size); IPoint16 location_in_atlas; if (!rect_packer->addRect(glyph_size.width + kPadding, // glyph_size.height + kPadding, // @@ -176,8 +157,8 @@ static void DrawGlyph(SkCanvas* canvas, const Rect& location, bool has_color) { const auto& metrics = font_glyph.font.GetMetrics(); - const auto position = SkPoint::Make(location.origin.x / metrics.scale, - location.origin.y / metrics.scale); + const auto position = SkPoint::Make(location.origin.x / font_glyph.scale, + location.origin.y / font_glyph.scale); SkGlyphID glyph_id = font_glyph.glyph.index; SkFont sk_font( @@ -192,7 +173,7 @@ static void DrawGlyph(SkCanvas* canvas, SkPaint glyph_paint; glyph_paint.setColor(glyph_color); canvas->resetMatrix(); - canvas->scale(metrics.scale, metrics.scale); + canvas->scale(font_glyph.scale, font_glyph.scale); canvas->drawGlyphs( 1u, // count &glyph_id, // glyphs @@ -331,25 +312,19 @@ static std::shared_ptr UploadGlyphTextureAtlas( std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( GlyphAtlas::Type type, std::shared_ptr atlas_context, - FrameIterator frame_iterator) const { + const FontGlyphPair::Set& font_glyph_pairs) const { TRACE_EVENT0("impeller", __FUNCTION__); if (!IsValid()) { return nullptr; } std::shared_ptr last_atlas = atlas_context->GetGlyphAtlas(); - // --------------------------------------------------------------------------- - // Step 1: Collect unique font-glyph pairs in the frame. - // --------------------------------------------------------------------------- - - FontGlyphPair::Set font_glyph_pairs = - CollectUniqueFontGlyphPairs(type, frame_iterator); if (font_glyph_pairs.empty()) { return last_atlas; } // --------------------------------------------------------------------------- - // Step 2: Determine if the atlas type and font glyph pairs are compatible + // Step 1: Determine if the atlas type and font glyph pairs are compatible // with the current atlas and reuse if possible. // --------------------------------------------------------------------------- FontGlyphPairRefVector new_glyphs; @@ -363,7 +338,7 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( } // --------------------------------------------------------------------------- - // Step 3: Determine if the additional missing glyphs can be appended to the + // Step 2: Determine if the additional missing glyphs can be appended to the // existing bitmap without recreating the atlas. This requires that // the type is identical. // --------------------------------------------------------------------------- @@ -376,7 +351,7 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( // added. // --------------------------------------------------------------------------- - // Step 4: Record the positions in the glyph atlas of the newly added + // Step 3a: Record the positions in the glyph atlas of the newly added // glyphs. // --------------------------------------------------------------------------- for (size_t i = 0, count = glyph_positions.size(); i < count; i++) { @@ -384,7 +359,7 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( } // --------------------------------------------------------------------------- - // Step 5: Draw new font-glyph pairs into the existing bitmap. + // Step 4a: Draw new font-glyph pairs into the existing bitmap. // --------------------------------------------------------------------------- auto bitmap = atlas_context->GetBitmap(); if (!UpdateAtlasBitmap(*last_atlas, bitmap, new_glyphs)) { @@ -392,7 +367,7 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( } // --------------------------------------------------------------------------- - // Step 6: Update the existing texture with the updated bitmap. + // Step 5a: Update the existing texture with the updated bitmap. // --------------------------------------------------------------------------- if (!UpdateGlyphTextureAtlas(bitmap, last_atlas->GetTexture())) { return nullptr; @@ -402,7 +377,7 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( // A new glyph atlas must be created. // --------------------------------------------------------------------------- - // Step 4: Get the optimum size of the texture atlas. + // Step 3b: Get the optimum size of the texture atlas. // --------------------------------------------------------------------------- auto glyph_atlas = std::make_shared(type); auto atlas_size = OptimumAtlasSizeForFontGlyphPairs( @@ -413,7 +388,7 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( return nullptr; } // --------------------------------------------------------------------------- - // Step 5: Find location of font-glyph pairs in the atlas. We have this from + // Step 4b: Find location of font-glyph pairs in the atlas. We have this from // the last step. So no need to do create another rect packer. But just do a // sanity check of counts. This could also be just an assertion as only a // construction issue would cause such a failure. @@ -423,7 +398,7 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( } // --------------------------------------------------------------------------- - // Step 6: Record the positions in the glyph atlas. + // Step 5b: Record the positions in the glyph atlas. // --------------------------------------------------------------------------- { size_t i = 0; @@ -434,7 +409,7 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( } // --------------------------------------------------------------------------- - // Step 7: Draw font-glyph pairs in the correct spot in the atlas. + // Step 6b: Draw font-glyph pairs in the correct spot in the atlas. // --------------------------------------------------------------------------- auto bitmap = CreateAtlasBitmap(*glyph_atlas, atlas_size); if (!bitmap) { @@ -443,7 +418,7 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( atlas_context->UpdateBitmap(bitmap); // --------------------------------------------------------------------------- - // Step 8: Upload the atlas as a texture. + // Step 7b: Upload the atlas as a texture. // --------------------------------------------------------------------------- PixelFormat format; switch (type) { @@ -461,7 +436,7 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( } // --------------------------------------------------------------------------- - // Step 9: Record the texture in the glyph atlas. + // Step 8b: Record the texture in the glyph atlas. // --------------------------------------------------------------------------- glyph_atlas->SetTexture(std::move(texture)); diff --git a/impeller/typographer/backends/skia/text_render_context_skia.h b/impeller/typographer/backends/skia/text_render_context_skia.h index 0c97a8716e7ad..2e2e6ad97cf8b 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.h +++ b/impeller/typographer/backends/skia/text_render_context_skia.h @@ -19,7 +19,7 @@ class TextRenderContextSkia : public TextRenderContext { std::shared_ptr CreateGlyphAtlas( GlyphAtlas::Type type, std::shared_ptr atlas_context, - FrameIterator iterator) const override; + const FontGlyphPair::Set& font_glyph_pairs) const override; private: FML_DISALLOW_COPY_AND_ASSIGN(TextRenderContextSkia); diff --git a/impeller/typographer/font.h b/impeller/typographer/font.h index de80478ccbaf6..bbfc569860b72 100644 --- a/impeller/typographer/font.h +++ b/impeller/typographer/font.h @@ -28,12 +28,6 @@ class Font : public Comparable { /// the baseline with an upper-left-origin coordinate system. /// struct Metrics { - //-------------------------------------------------------------------------- - /// The scaling factor that should be used when rendering this font to an - /// atlas. This should normally be set in accordance with the transformation - /// matrix that will be used to position glyph geometry. - /// - Scalar scale = 1.0f; //-------------------------------------------------------------------------- /// The point size of the font. /// @@ -43,8 +37,8 @@ class Font : public Comparable { Scalar scaleX = 1.0f; constexpr bool operator==(const Metrics& o) const { - return scale == o.scale && point_size == o.point_size && - embolden == o.embolden && skewX == o.skewX && scaleX == o.scaleX; + return point_size == o.point_size && embolden == o.embolden && + skewX == o.skewX && scaleX == o.scaleX; } }; @@ -80,6 +74,6 @@ class Font : public Comparable { template <> struct std::hash { constexpr std::size_t operator()(const impeller::Font::Metrics& m) const { - return fml::HashCombine(m.scale, m.point_size); + return fml::HashCombine(m.point_size, m.skewX, m.scaleX); } }; diff --git a/impeller/typographer/font_glyph_pair.h b/impeller/typographer/font_glyph_pair.h index ed455095356dd..0b89c803af88e 100644 --- a/impeller/typographer/font_glyph_pair.h +++ b/impeller/typographer/font_glyph_pair.h @@ -16,28 +16,29 @@ namespace impeller { //------------------------------------------------------------------------------ -/// @brief A font along with a glyph in that font. Used in glyph atlases as -/// keys. +/// @brief A font along with a glyph in that font rendered at a particular +/// scale. Used in glyph atlases as keys. /// struct FontGlyphPair { struct Hash; struct Equal; using Set = std::unordered_set; - using Vector = std::vector; Font font; Glyph glyph; + Scalar scale; struct Hash { std::size_t operator()(const FontGlyphPair& p) const { - return fml::HashCombine(p.font.GetHash(), p.glyph.index, p.glyph.type); + return fml::HashCombine(p.font.GetHash(), p.glyph.index, p.glyph.type, + p.scale); } }; struct Equal { bool operator()(const FontGlyphPair& lhs, const FontGlyphPair& rhs) const { return lhs.font.IsEqual(rhs.font) && lhs.glyph.index == rhs.glyph.index && - lhs.glyph.type == rhs.glyph.type; + lhs.glyph.type == rhs.glyph.type && lhs.scale == rhs.scale; } }; }; diff --git a/impeller/typographer/lazy_glyph_atlas.cc b/impeller/typographer/lazy_glyph_atlas.cc index 799b56a35877f..94c738e37b798 100644 --- a/impeller/typographer/lazy_glyph_atlas.cc +++ b/impeller/typographer/lazy_glyph_atlas.cc @@ -15,12 +15,12 @@ LazyGlyphAtlas::LazyGlyphAtlas() = default; LazyGlyphAtlas::~LazyGlyphAtlas() = default; -void LazyGlyphAtlas::AddTextFrame(const TextFrame& frame) { +void LazyGlyphAtlas::AddTextFrame(const TextFrame& frame, Scalar scale) { FML_DCHECK(atlas_map_.empty()); if (frame.GetAtlasType() == GlyphAtlas::Type::kAlphaBitmap) { - alpha_frames_.emplace_back(frame); + frame.CollectUniqueFontGlyphPairs(alpha_set_, scale); } else { - color_frames_.emplace_back(frame); + frame.CollectUniqueFontGlyphPairs(color_set_, scale); } } @@ -39,19 +39,9 @@ std::shared_ptr LazyGlyphAtlas::CreateOrGetGlyphAtlas( if (!text_context || !text_context->IsValid()) { return nullptr; } - size_t i = 0; - auto frames = - type == GlyphAtlas::Type::kAlphaBitmap ? alpha_frames_ : color_frames_; - TextRenderContext::FrameIterator iterator = [&]() -> const TextFrame* { - if (i >= frames.size()) { - return nullptr; - } - const auto& result = frames[i]; - i++; - return &result; - }; + auto& set = type == GlyphAtlas::Type::kAlphaBitmap ? alpha_set_ : color_set_; auto atlas = - text_context->CreateGlyphAtlas(type, std::move(atlas_context), iterator); + text_context->CreateGlyphAtlas(type, std::move(atlas_context), set); if (!atlas || !atlas->IsValid()) { VALIDATION_LOG << "Could not create valid atlas."; return nullptr; diff --git a/impeller/typographer/lazy_glyph_atlas.h b/impeller/typographer/lazy_glyph_atlas.h index 067601e65e64f..f79b46db9d383 100644 --- a/impeller/typographer/lazy_glyph_atlas.h +++ b/impeller/typographer/lazy_glyph_atlas.h @@ -19,7 +19,7 @@ class LazyGlyphAtlas { ~LazyGlyphAtlas(); - void AddTextFrame(const TextFrame& frame); + void AddTextFrame(const TextFrame& frame, Scalar scale); std::shared_ptr CreateOrGetGlyphAtlas( GlyphAtlas::Type type, @@ -27,8 +27,8 @@ class LazyGlyphAtlas { std::shared_ptr context) const; private: - std::vector alpha_frames_; - std::vector color_frames_; + FontGlyphPair::Set alpha_set_; + FontGlyphPair::Set color_set_; mutable std::unordered_map> atlas_map_; diff --git a/impeller/typographer/text_frame.cc b/impeller/typographer/text_frame.cc index ed3c97cc82b3e..4191446f05462 100644 --- a/impeller/typographer/text_frame.cc +++ b/impeller/typographer/text_frame.cc @@ -79,4 +79,15 @@ bool TextFrame::MaybeHasOverlapping() const { return false; } +void TextFrame::CollectUniqueFontGlyphPairs(FontGlyphPair::Set& set, + Scalar scale) const { + for (const TextRun& run : GetRuns()) { + const Font& font = run.GetFont(); + for (const TextRun::GlyphPosition& glyph_position : + run.GetGlyphPositions()) { + set.insert({font, glyph_position.glyph, scale}); + } + } +} + } // namespace impeller diff --git a/impeller/typographer/text_frame.h b/impeller/typographer/text_frame.h index a47e4c0e2252a..512f7b705e3e3 100644 --- a/impeller/typographer/text_frame.h +++ b/impeller/typographer/text_frame.h @@ -22,6 +22,8 @@ class TextFrame { ~TextFrame(); + void CollectUniqueFontGlyphPairs(FontGlyphPair::Set& set, Scalar scale) const; + //---------------------------------------------------------------------------- /// @brief The conservative bounding box for this text frame. /// diff --git a/impeller/typographer/text_render_context.cc b/impeller/typographer/text_render_context.cc index 8cc8dbf00a02e..1b7aba10a7b74 100644 --- a/impeller/typographer/text_render_context.cc +++ b/impeller/typographer/text_render_context.cc @@ -26,19 +26,4 @@ const std::shared_ptr& TextRenderContext::GetContext() const { return context_; } -std::shared_ptr TextRenderContext::CreateGlyphAtlas( - GlyphAtlas::Type type, - std::shared_ptr atlas_context, - const TextFrame& frame) const { - size_t count = 0; - FrameIterator iterator = [&]() -> const TextFrame* { - count++; - if (count == 1) { - return &frame; - } - return nullptr; - }; - return CreateGlyphAtlas(type, std::move(atlas_context), iterator); -} - } // namespace impeller diff --git a/impeller/typographer/text_render_context.h b/impeller/typographer/text_render_context.h index c63ac912a1fa8..d909388105f93 100644 --- a/impeller/typographer/text_render_context.h +++ b/impeller/typographer/text_render_context.h @@ -37,20 +37,13 @@ class TextRenderContext { /// const std::shared_ptr& GetContext() const; - using FrameIterator = std::function; - // TODO(dnfield): Callers should not need to know which type of atlas to // create. https://github.com/flutter/flutter/issues/111640 virtual std::shared_ptr CreateGlyphAtlas( GlyphAtlas::Type type, std::shared_ptr atlas_context, - FrameIterator iterator) const = 0; - - std::shared_ptr CreateGlyphAtlas( - GlyphAtlas::Type type, - std::shared_ptr atlas_context, - const TextFrame& frame) const; + const FontGlyphPair::Set& font_glyph_pairs) const = 0; protected: //---------------------------------------------------------------------------- diff --git a/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc index e807ac8a26d74..e800ea258b44e 100644 --- a/impeller/typographer/typographer_unittests.cc +++ b/impeller/typographer/typographer_unittests.cc @@ -23,6 +23,17 @@ namespace testing { using TypographerTest = PlaygroundTest; INSTANTIATE_PLAYGROUND_SUITE(TypographerTest); +static std::shared_ptr CreateGlyphAtlas( + const TextRenderContext* context, + GlyphAtlas::Type type, + Scalar scale, + const std::shared_ptr& atlas_context, + const TextFrame& frame) { + FontGlyphPair::Set set; + frame.CollectUniqueFontGlyphPairs(set, scale); + return context->CreateGlyphAtlas(type, atlas_context, set); +} + TEST_P(TypographerTest, CanConvertTextBlob) { SkFont font; auto blob = SkTextBlob::MakeFromString( @@ -49,8 +60,8 @@ TEST_P(TypographerTest, CanCreateGlyphAtlas) { auto blob = SkTextBlob::MakeFromString("hello", sk_font); ASSERT_TRUE(blob); auto atlas = - context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, - TextFrameFromTextBlob(blob)); + CreateGlyphAtlas(context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, + atlas_context, TextFrameFromTextBlob(blob)); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); ASSERT_EQ(atlas->GetType(), GlyphAtlas::Type::kAlphaBitmap); @@ -102,13 +113,13 @@ TEST_P(TypographerTest, LazyAtlasTracksColor) { LazyGlyphAtlas lazy_atlas; - lazy_atlas.AddTextFrame(frame); + lazy_atlas.AddTextFrame(frame, 1.0f); frame = TextFrameFromTextBlob(SkTextBlob::MakeFromString("😀 ", emoji_font)); ASSERT_TRUE(frame.GetAtlasType() == GlyphAtlas::Type::kColorBitmap); - lazy_atlas.AddTextFrame(frame); + lazy_atlas.AddTextFrame(frame, 1.0f); // Creates different atlases for color and alpha bitmap. auto color_context = std::make_shared(); @@ -130,8 +141,8 @@ TEST_P(TypographerTest, GlyphAtlasWithOddUniqueGlyphSize) { auto blob = SkTextBlob::MakeFromString("AGH", sk_font); ASSERT_TRUE(blob); auto atlas = - context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, - TextFrameFromTextBlob(blob)); + CreateGlyphAtlas(context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, + atlas_context, TextFrameFromTextBlob(blob)); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); @@ -147,8 +158,8 @@ TEST_P(TypographerTest, GlyphAtlasIsRecycledIfUnchanged) { auto blob = SkTextBlob::MakeFromString("spooky skellingtons", sk_font); ASSERT_TRUE(blob); auto atlas = - context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, - TextFrameFromTextBlob(blob)); + CreateGlyphAtlas(context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, + atlas_context, TextFrameFromTextBlob(blob)); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); ASSERT_EQ(atlas, atlas_context->GetGlyphAtlas()); @@ -156,8 +167,8 @@ TEST_P(TypographerTest, GlyphAtlasIsRecycledIfUnchanged) { // now attempt to re-create an atlas with the same text blob. auto next_atlas = - context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, - TextFrameFromTextBlob(blob)); + CreateGlyphAtlas(context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, + atlas_context, TextFrameFromTextBlob(blob)); ASSERT_EQ(atlas, next_atlas); ASSERT_EQ(atlas_context->GetGlyphAtlas(), atlas); } @@ -166,7 +177,6 @@ TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) { auto context = TextRenderContext::Create(GetContext()); auto atlas_context = std::make_shared(); ASSERT_TRUE(context && context->IsValid()); - SkFont sk_font; const char* test_string = "QWERTYUIOPASDFGHJKLZXCVBNMqewrtyuiopasdfghjklzxcvbnm,.<>[]{};':" @@ -174,22 +184,17 @@ TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) { "œ∑´®†¥¨ˆøπ““‘‘åß∂ƒ©˙∆˚¬…æ≈ç√∫˜µ≤≥≥≥≥÷¡™£¢∞§¶•ªº–≠⁄€‹›fifl‡°·‚—±Œ„´‰Á¨Ø∏”’/" "* Í˝ */¸˛Ç◊ı˜Â¯˘¿"; + SkFont sk_font; auto blob = SkTextBlob::MakeFromString(test_string, sk_font); ASSERT_TRUE(blob); - TextFrame frame; - size_t count = 0; - const int size_count = 8; - TextRenderContext::FrameIterator iterator = [&]() -> const TextFrame* { - if (count < size_count) { - count++; - frame = TextFrameFromTextBlob(blob, 0.6 * count); - return &frame; - } - return nullptr; + FontGlyphPair::Set set; + size_t size_count = 8; + for (size_t index = 0; index < size_count; index += 1) { + TextFrameFromTextBlob(blob).CollectUniqueFontGlyphPairs(set, 0.6 * index); }; auto atlas = context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, - atlas_context, iterator); + std::move(atlas_context), set); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); @@ -217,8 +222,8 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecycledIfUnchanged) { auto blob = SkTextBlob::MakeFromString("spooky 1", sk_font); ASSERT_TRUE(blob); auto atlas = - context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, - TextFrameFromTextBlob(blob)); + CreateGlyphAtlas(context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, + atlas_context, TextFrameFromTextBlob(blob)); auto old_packer = atlas_context->GetRectPacker(); ASSERT_NE(atlas, nullptr); @@ -231,8 +236,8 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecycledIfUnchanged) { auto blob2 = SkTextBlob::MakeFromString("spooky 2", sk_font); auto next_atlas = - context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, - TextFrameFromTextBlob(blob2)); + CreateGlyphAtlas(context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, + atlas_context, TextFrameFromTextBlob(blob2)); ASSERT_EQ(atlas, next_atlas); auto* second_texture = next_atlas->GetTexture().get(); @@ -250,8 +255,8 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecreatedIfTypeChanges) { auto blob = SkTextBlob::MakeFromString("spooky 1", sk_font); ASSERT_TRUE(blob); auto atlas = - context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, - TextFrameFromTextBlob(blob)); + CreateGlyphAtlas(context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, + atlas_context, TextFrameFromTextBlob(blob)); auto old_packer = atlas_context->GetRectPacker(); ASSERT_NE(atlas, nullptr); @@ -265,8 +270,8 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecreatedIfTypeChanges) { auto blob2 = SkTextBlob::MakeFromString("spooky 1", sk_font); auto next_atlas = - context->CreateGlyphAtlas(GlyphAtlas::Type::kColorBitmap, atlas_context, - TextFrameFromTextBlob(blob2)); + CreateGlyphAtlas(context.get(), GlyphAtlas::Type::kColorBitmap, 1.0f, + atlas_context, TextFrameFromTextBlob(blob2)); ASSERT_NE(atlas, next_atlas); auto* second_texture = next_atlas->GetTexture().get(); From c2880695328d412d4ab1884552571289db2c1487 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 14 Jul 2023 11:16:40 -0700 Subject: [PATCH 2/3] Record the CTM scale independent of how EntityPass::Render may change it --- impeller/aiks/aiks_unittests.cc | 36 +++++++++++++++++++ .../contents/color_source_text_contents.cc | 2 +- .../contents/color_source_text_contents.h | 2 +- impeller/entity/contents/contents.h | 5 +-- impeller/entity/contents/text_contents.cc | 6 ++-- impeller/entity/contents/text_contents.h | 3 +- 6 files changed, 46 insertions(+), 8 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index 7fe1b6bcaab8d..b0a279eec3d05 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -2952,5 +2952,41 @@ APPLY_COLOR_FILTER_GRADIENT_TEST(Radial); APPLY_COLOR_FILTER_GRADIENT_TEST(Conical); APPLY_COLOR_FILTER_GRADIENT_TEST(Sweep); +TEST_P(AiksTest, DrawScaledTextWithPerspectiveNoSaveLayer) { + Canvas canvas; + // clang-format off + canvas.Transform(Matrix( + 2.000000, 0.000000, 0.000000, 0.000000, + 1.445767, 2.637070, -0.507928, 0.001524, + -2.451887, -0.534662, 0.861399, -0.002584, + 1063.481934, 1025.951416, -48.300270, 1.144901 + )); + // clang-format on + + ASSERT_TRUE(RenderTextInCanvas(GetContext(), canvas, "Hello world", + "Roboto-Regular.ttf")); + + ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); +} + +TEST_P(AiksTest, DrawScaledTextWithPerspectiveSaveLayer) { + Canvas canvas; + Paint save_paint; + canvas.SaveLayer(save_paint); + // clang-format off + canvas.Transform(Matrix( + 2.000000, 0.000000, 0.000000, 0.000000, + 1.445767, 2.637070, -0.507928, 0.001524, + -2.451887, -0.534662, 0.861399, -0.002584, + 1063.481934, 1025.951416, -48.300270, 1.144901 + )); + // clang-format on + + ASSERT_TRUE(RenderTextInCanvas(GetContext(), canvas, "Hello world", + "Roboto-Regular.ttf")); + + ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); +} + } // namespace testing } // namespace impeller diff --git a/impeller/entity/contents/color_source_text_contents.cc b/impeller/entity/contents/color_source_text_contents.cc index a09e44c8dcb35..8032bbb629d55 100644 --- a/impeller/entity/contents/color_source_text_contents.cc +++ b/impeller/entity/contents/color_source_text_contents.cc @@ -36,7 +36,7 @@ void ColorSourceTextContents::SetTextPosition(Point position) { void ColorSourceTextContents::PopulateGlyphAtlas( const std::shared_ptr& lazy_glyph_atlas, - Scalar scale) const { + Scalar scale) { text_contents_->PopulateGlyphAtlas(lazy_glyph_atlas, scale); } diff --git a/impeller/entity/contents/color_source_text_contents.h b/impeller/entity/contents/color_source_text_contents.h index d66a7ea313458..18aa1a450ad4a 100644 --- a/impeller/entity/contents/color_source_text_contents.h +++ b/impeller/entity/contents/color_source_text_contents.h @@ -35,7 +35,7 @@ class ColorSourceTextContents final : public Contents { // |Contents| void PopulateGlyphAtlas( const std::shared_ptr& lazy_glyph_atlas, - Scalar scale) const override; + Scalar scale) override; // |Contents| bool Render(const ContentContext& renderer, diff --git a/impeller/entity/contents/contents.h b/impeller/entity/contents/contents.h index a80b82560d8e1..245b8f2d6d514 100644 --- a/impeller/entity/contents/contents.h +++ b/impeller/entity/contents/contents.h @@ -54,10 +54,11 @@ class Contents { virtual ~Contents(); - /// @brief Add any text data to the specified lazy atlas. + /// @brief Add any text data to the specified lazy atlas. The scale parameter + /// must be used again later when drawing the text. virtual void PopulateGlyphAtlas( const std::shared_ptr& lazy_glyph_atlas, - Scalar scale) const {} + Scalar scale) {} virtual bool Render(const ContentContext& renderer, const Entity& entity, diff --git a/impeller/entity/contents/text_contents.cc b/impeller/entity/contents/text_contents.cc index 5428cf830feba..339bb27549166 100644 --- a/impeller/entity/contents/text_contents.cc +++ b/impeller/entity/contents/text_contents.cc @@ -77,8 +77,9 @@ std::optional TextContents::GetCoverage(const Entity& entity) const { void TextContents::PopulateGlyphAtlas( const std::shared_ptr& lazy_glyph_atlas, - Scalar scale) const { + Scalar scale) { lazy_glyph_atlas->AddTextFrame(frame_, scale); + scale_ = scale; } bool TextContents::Render(const ContentContext& renderer, @@ -90,7 +91,6 @@ bool TextContents::Render(const ContentContext& renderer, } auto type = frame_.GetAtlasType(); - auto scale = entity.DeriveTextScale(); auto atlas = ResolveAtlas(type, renderer.GetLazyGlyphAtlas(), renderer.GetGlyphAtlasContext(type), renderer.GetContext()); @@ -180,7 +180,7 @@ bool TextContents::Render(const ContentContext& renderer, for (const auto& run : frame_.GetRuns()) { const Font& font = run.GetFont(); for (const auto& glyph_position : run.GetGlyphPositions()) { - FontGlyphPair font_glyph_pair{font, glyph_position.glyph, scale}; + FontGlyphPair font_glyph_pair{font, glyph_position.glyph, scale_}; auto maybe_atlas_glyph_bounds = atlas->FindFontGlyphBounds(font_glyph_pair); if (!maybe_atlas_glyph_bounds.has_value()) { diff --git a/impeller/entity/contents/text_contents.h b/impeller/entity/contents/text_contents.h index fe89068ae2a14..7c805841e91e1 100644 --- a/impeller/entity/contents/text_contents.h +++ b/impeller/entity/contents/text_contents.h @@ -48,7 +48,7 @@ class TextContents final : public Contents { // |Contents| void PopulateGlyphAtlas( const std::shared_ptr& lazy_glyph_atlas, - Scalar scale) const override; + Scalar scale) override; // |Contents| bool Render(const ContentContext& renderer, @@ -57,6 +57,7 @@ class TextContents final : public Contents { private: TextFrame frame_; + Scalar scale_; Color color_; Scalar inherited_opacity_ = 1.0; Vector2 offset_; From 9fdf7753f4dd1ce18fc37c5f992b48e1171ec9a9 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 14 Jul 2023 13:36:00 -0700 Subject: [PATCH 3/3] Update impeller/entity/contents/text_contents.h Co-authored-by: Brandon DeRosier --- impeller/entity/contents/text_contents.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/entity/contents/text_contents.h b/impeller/entity/contents/text_contents.h index 7c805841e91e1..c75daa0a50660 100644 --- a/impeller/entity/contents/text_contents.h +++ b/impeller/entity/contents/text_contents.h @@ -57,7 +57,7 @@ class TextContents final : public Contents { private: TextFrame frame_; - Scalar scale_; + Scalar scale_ = 1.0; Color color_; Scalar inherited_opacity_ = 1.0; Vector2 offset_;