From 299f983dc47e19bbbdb08e47ca0558dedaae637f Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Sun, 20 Aug 2023 13:36:37 -0700 Subject: [PATCH 1/7] Supply the text backend to the AiksContext at runtime; allow for overriding the text backend in the Aiks playground/goldens --- impeller/aiks/aiks_context.cc | 7 ++++-- impeller/aiks/aiks_context.h | 13 +++++++++- impeller/aiks/aiks_playground.cc | 20 ++++++++++++--- impeller/aiks/aiks_playground.h | 8 ++++-- impeller/aiks/aiks_unittests.cc | 14 +++++------ impeller/display_list/dl_playground.cc | 3 ++- impeller/entity/contents/content_context.cc | 8 ++++-- impeller/entity/contents/content_context.h | 5 +++- impeller/entity/contents/text_contents.cc | 8 +++--- impeller/entity/contents/text_contents.h | 3 +-- impeller/entity/entity_playground.cc | 12 ++++++--- impeller/entity/entity_unittests.cc | 3 ++- .../golden_tests/golden_playground_test.h | 8 ++++-- .../golden_playground_test_mac.cc | 19 ++++++++++---- .../golden_playground_test_stub.cc | 7 ++++-- impeller/golden_tests/golden_tests.cc | 2 +- impeller/golden_tests/metal_screenshoter.h | 10 +++----- impeller/golden_tests/metal_screenshoter.mm | 4 +-- .../renderer/compute_subgroup_unittests.cc | 6 ++--- .../backends/skia/text_render_context_skia.cc | 12 ++++----- .../backends/skia/text_render_context_skia.h | 3 +++ impeller/typographer/lazy_glyph_atlas.cc | 22 ++++++++++------ impeller/typographer/lazy_glyph_atlas.h | 9 ++++--- impeller/typographer/text_render_context.h | 3 --- impeller/typographer/typographer_unittests.cc | 25 +++++++++---------- 25 files changed, 148 insertions(+), 86 deletions(-) diff --git a/impeller/aiks/aiks_context.cc b/impeller/aiks/aiks_context.cc index e724ac597af9a..b1fdab42b8e33 100644 --- a/impeller/aiks/aiks_context.cc +++ b/impeller/aiks/aiks_context.cc @@ -5,16 +5,19 @@ #include "impeller/aiks/aiks_context.h" #include "impeller/aiks/picture.h" +#include "impeller/typographer/text_render_context.h" namespace impeller { -AiksContext::AiksContext(std::shared_ptr context) +AiksContext::AiksContext(std::shared_ptr context, + std::unique_ptr text_render_context) : context_(std::move(context)) { if (!context_ || !context_->IsValid()) { return; } - content_context_ = std::make_unique(context_); + content_context_ = std::make_unique( + context_, std::move(text_render_context)); if (!content_context_->IsValid()) { return; } diff --git a/impeller/aiks/aiks_context.h b/impeller/aiks/aiks_context.h index fe62ebca47b45..1c008b4bbc2ed 100644 --- a/impeller/aiks/aiks_context.h +++ b/impeller/aiks/aiks_context.h @@ -10,6 +10,7 @@ #include "impeller/entity/contents/content_context.h" #include "impeller/renderer/context.h" #include "impeller/renderer/render_target.h" +#include "impeller/typographer/text_render_context.h" namespace impeller { @@ -18,7 +19,17 @@ class RenderPass; class AiksContext { public: - AiksContext(std::shared_ptr context); + /// Construct a new AiksContext. + /// + /// @param context The Impeller context that Aiks should use for + /// allocating resources and executing device + /// commands. Required. + /// @param text_render_context The text backend to use for rendering text. If + /// `nullptr` is supplied, then attempting to draw + /// text with Aiks will result in validation + /// errors. + AiksContext(std::shared_ptr context, + std::unique_ptr text_render_context); ~AiksContext(); diff --git a/impeller/aiks/aiks_playground.cc b/impeller/aiks/aiks_playground.cc index 5fc3124f283f8..9fda4406bfd37 100644 --- a/impeller/aiks/aiks_playground.cc +++ b/impeller/aiks/aiks_playground.cc @@ -6,6 +6,7 @@ #include "impeller/aiks/aiks_context.h" +#include "impeller/typographer/backends/skia/text_render_context_skia.h" #include "third_party/imgui/imgui.h" namespace impeller { @@ -14,19 +15,30 @@ AiksPlayground::AiksPlayground() = default; AiksPlayground::~AiksPlayground() = default; -bool AiksPlayground::OpenPlaygroundHere(const Picture& picture) { +bool AiksPlayground::OpenPlaygroundHere( + const Picture& picture, + std::unique_ptr text_render_context_override) { + auto text_context = text_render_context_override + ? std::move(text_render_context_override) + : TextRenderContextSkia::Make(GetContext()); return OpenPlaygroundHere( [&picture](AiksContext& renderer, RenderTarget& render_target) -> bool { return renderer.Render(picture, render_target); - }); + }, + std::move(text_context)); } -bool AiksPlayground::OpenPlaygroundHere(AiksPlaygroundCallback callback) { +bool AiksPlayground::OpenPlaygroundHere( + AiksPlaygroundCallback callback, + std::unique_ptr text_render_context_override) { if (!switches_.enable_playground) { return true; } - AiksContext renderer(GetContext()); + auto text_context = text_render_context_override + ? std::move(text_render_context_override) + : TextRenderContextSkia::Make(GetContext()); + AiksContext renderer(GetContext(), std::move(text_context)); if (!renderer.IsValid()) { return false; diff --git a/impeller/aiks/aiks_playground.h b/impeller/aiks/aiks_playground.h index 02647faa7e3b1..0aa269c73a990 100644 --- a/impeller/aiks/aiks_playground.h +++ b/impeller/aiks/aiks_playground.h @@ -20,9 +20,13 @@ class AiksPlayground : public PlaygroundTest { ~AiksPlayground(); - bool OpenPlaygroundHere(const Picture& picture); + bool OpenPlaygroundHere(const Picture& picture, + std::unique_ptr + text_render_context_override = nullptr); - bool OpenPlaygroundHere(AiksPlaygroundCallback callback); + bool OpenPlaygroundHere(AiksPlaygroundCallback callback, + std::unique_ptr + text_render_context_override = nullptr); private: FML_DISALLOW_COPY_AND_ASSIGN(AiksPlayground); diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index d1dcadac7f105..557e122b62f24 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -1002,7 +1002,7 @@ TEST_P(AiksTest, CanPictureConvertToImage) { recorder_canvas.DrawRect({200.0, 200.0, 600, 600}, paint); Canvas canvas; - AiksContext renderer(GetContext()); + AiksContext renderer(GetContext(), nullptr); paint.color = Color::BlackTransparent(); canvas.DrawPaint(paint); Picture picture = recorder_canvas.EndRecordingAsPicture(); @@ -2139,7 +2139,7 @@ TEST_P(AiksTest, DrawPaintAbsorbsClears) { std::shared_ptr spy = ContextSpy::Make(); std::shared_ptr real_context = GetContext(); std::shared_ptr mock_context = spy->MakeContext(real_context); - AiksContext renderer(mock_context); + AiksContext renderer(mock_context, nullptr); std::shared_ptr image = picture.ToImage(renderer, {300, 300}); ASSERT_EQ(spy->render_passes_.size(), 1llu); @@ -2159,7 +2159,7 @@ TEST_P(AiksTest, DrawRectAbsorbsClears) { Picture picture = canvas.EndRecordingAsPicture(); std::shared_ptr real_context = GetContext(); std::shared_ptr mock_context = spy->MakeContext(real_context); - AiksContext renderer(mock_context); + AiksContext renderer(mock_context, nullptr); std::shared_ptr image = picture.ToImage(renderer, {300, 300}); ASSERT_EQ(spy->render_passes_.size(), 1llu); @@ -2179,7 +2179,7 @@ TEST_P(AiksTest, DrawRectAbsorbsClearsNegativeRRect) { Picture picture = canvas.EndRecordingAsPicture(); std::shared_ptr real_context = GetContext(); std::shared_ptr mock_context = spy->MakeContext(real_context); - AiksContext renderer(mock_context); + AiksContext renderer(mock_context, nullptr); std::shared_ptr image = picture.ToImage(renderer, {300, 300}); ASSERT_EQ(spy->render_passes_.size(), 1llu); @@ -2199,7 +2199,7 @@ TEST_P(AiksTest, DrawRectAbsorbsClearsNegativeRotation) { Picture picture = canvas.EndRecordingAsPicture(); std::shared_ptr real_context = GetContext(); std::shared_ptr mock_context = spy->MakeContext(real_context); - AiksContext renderer(mock_context); + AiksContext renderer(mock_context, nullptr); std::shared_ptr image = picture.ToImage(renderer, {300, 300}); ASSERT_EQ(spy->render_passes_.size(), 1llu); @@ -2219,7 +2219,7 @@ TEST_P(AiksTest, DrawRectAbsorbsClearsNegative) { Picture picture = canvas.EndRecordingAsPicture(); std::shared_ptr real_context = GetContext(); std::shared_ptr mock_context = spy->MakeContext(real_context); - AiksContext renderer(mock_context); + AiksContext renderer(mock_context, nullptr); std::shared_ptr image = picture.ToImage(renderer, {301, 301}); ASSERT_EQ(spy->render_passes_.size(), 1llu); @@ -2243,7 +2243,7 @@ TEST_P(AiksTest, ClipRectElidesNoOpClips) { std::shared_ptr spy = ContextSpy::Make(); std::shared_ptr real_context = GetContext(); std::shared_ptr mock_context = spy->MakeContext(real_context); - AiksContext renderer(mock_context); + AiksContext renderer(mock_context, nullptr); std::shared_ptr image = picture.ToImage(renderer, {300, 300}); ASSERT_EQ(spy->render_passes_.size(), 1llu); diff --git a/impeller/display_list/dl_playground.cc b/impeller/display_list/dl_playground.cc index 539b32f6a7a6b..9c5d1b4903170 100644 --- a/impeller/display_list/dl_playground.cc +++ b/impeller/display_list/dl_playground.cc @@ -7,6 +7,7 @@ #include "flutter/testing/testing.h" #include "impeller/aiks/aiks_context.h" #include "impeller/display_list/dl_dispatcher.h" +#include "impeller/typographer/backends/skia/text_render_context_skia.h" #include "third_party/imgui/imgui.h" #include "third_party/skia/include/core/SkData.h" @@ -29,7 +30,7 @@ bool DlPlayground::OpenPlaygroundHere(DisplayListPlaygroundCallback callback) { return true; } - AiksContext context(GetContext()); + AiksContext context(GetContext(), TextRenderContextSkia::Make(GetContext())); if (!context.IsValid()) { return false; } diff --git a/impeller/entity/contents/content_context.cc b/impeller/entity/contents/content_context.cc index 94fd796a7eed2..c18c3265d65da 100644 --- a/impeller/entity/contents/content_context.cc +++ b/impeller/entity/contents/content_context.cc @@ -16,6 +16,7 @@ #include "impeller/renderer/render_pass.h" #include "impeller/renderer/render_target.h" #include "impeller/tessellator/tessellator.h" +#include "impeller/typographer/text_render_context.h" namespace impeller { @@ -159,9 +160,12 @@ static std::unique_ptr CreateDefaultPipeline( return std::make_unique(context, desc); } -ContentContext::ContentContext(std::shared_ptr context) +ContentContext::ContentContext( + std::shared_ptr context, + std::unique_ptr text_render_context) : context_(std::move(context)), - lazy_glyph_atlas_(std::make_shared()), + lazy_glyph_atlas_( + std::make_shared(std::move(text_render_context))), tessellator_(std::make_shared()), scene_context_(std::make_shared(context_)), render_target_cache_(std::make_shared( diff --git a/impeller/entity/contents/content_context.h b/impeller/entity/contents/content_context.h index 86da8f98b2287..457f784c59abf 100644 --- a/impeller/entity/contents/content_context.h +++ b/impeller/entity/contents/content_context.h @@ -19,6 +19,7 @@ #include "impeller/renderer/pipeline.h" #include "impeller/renderer/render_target.h" #include "impeller/scene/scene_context.h" +#include "impeller/typographer/text_render_context.h" #ifdef IMPELLER_DEBUG #include "impeller/entity/checkerboard.frag.h" @@ -339,7 +340,9 @@ class RenderTargetCache; class ContentContext { public: - explicit ContentContext(std::shared_ptr context); + explicit ContentContext( + std::shared_ptr context, + std::unique_ptr text_render_context); ~ContentContext(); diff --git a/impeller/entity/contents/text_contents.cc b/impeller/entity/contents/text_contents.cc index 26a5db145c258..3b1a2586174c1 100644 --- a/impeller/entity/contents/text_contents.cc +++ b/impeller/entity/contents/text_contents.cc @@ -31,11 +31,10 @@ void TextContents::SetTextFrame(const TextFrame& frame) { std::shared_ptr TextContents::ResolveAtlas( GlyphAtlas::Type type, - const std::shared_ptr& lazy_atlas, - std::shared_ptr context) const { + const std::shared_ptr& lazy_atlas) const { FML_DCHECK(lazy_atlas); if (lazy_atlas) { - return lazy_atlas->CreateOrGetGlyphAtlas(type, std::move(context)); + return lazy_atlas->CreateOrGetGlyphAtlas(type); } return nullptr; @@ -89,8 +88,7 @@ bool TextContents::Render(const ContentContext& renderer, } auto type = frame_.GetAtlasType(); - auto atlas = - ResolveAtlas(type, renderer.GetLazyGlyphAtlas(), renderer.GetContext()); + auto atlas = ResolveAtlas(type, renderer.GetLazyGlyphAtlas()); if (!atlas || !atlas->IsValid()) { VALIDATION_LOG << "Cannot render glyphs without prepared atlas."; diff --git a/impeller/entity/contents/text_contents.h b/impeller/entity/contents/text_contents.h index 4499586735a7d..15f17ff809168 100644 --- a/impeller/entity/contents/text_contents.h +++ b/impeller/entity/contents/text_contents.h @@ -64,8 +64,7 @@ class TextContents final : public Contents { std::shared_ptr ResolveAtlas( GlyphAtlas::Type type, - const std::shared_ptr& lazy_atlas, - std::shared_ptr context) const; + const std::shared_ptr& lazy_atlas) const; FML_DISALLOW_COPY_AND_ASSIGN(TextContents); }; diff --git a/impeller/entity/entity_playground.cc b/impeller/entity/entity_playground.cc index abdb428e200da..f6b645d5c1878 100644 --- a/impeller/entity/entity_playground.cc +++ b/impeller/entity/entity_playground.cc @@ -1,11 +1,12 @@ // Copyright 2013 The Flutter Authors. All rights reserved. +// Copyright 2013 The Flutter Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. #include "impeller/entity/entity_playground.h" #include "impeller/entity/contents/content_context.h" - +#include "impeller/typographer/backends/skia/text_render_context_skia.h" #include "third_party/imgui/imgui.h" namespace impeller { @@ -19,7 +20,8 @@ bool EntityPlayground::OpenPlaygroundHere(EntityPass& entity_pass) { return true; } - ContentContext content_context(GetContext()); + ContentContext content_context(GetContext(), + TextRenderContextSkia::Make(GetContext())); if (!content_context.IsValid()) { return false; } @@ -35,7 +37,8 @@ bool EntityPlayground::OpenPlaygroundHere(Entity entity) { return true; } - ContentContext content_context(GetContext()); + ContentContext content_context(GetContext(), + TextRenderContextSkia::Make(GetContext())); if (!content_context.IsValid()) { return false; } @@ -50,7 +53,8 @@ bool EntityPlayground::OpenPlaygroundHere(EntityPlaygroundCallback callback) { return true; } - ContentContext content_context(GetContext()); + ContentContext content_context(GetContext(), + TextRenderContextSkia::Make(GetContext())); if (!content_context.IsValid()) { return false; } diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc index 35194d5541d0a..5c4840f48276b 100644 --- a/impeller/entity/entity_unittests.cc +++ b/impeller/entity/entity_unittests.cc @@ -2175,7 +2175,8 @@ TEST_P(EntityTest, InheritOpacityTest) { font.setSize(30); auto blob = SkTextBlob::MakeFromString("A", font); auto frame = TextFrameFromTextBlob(blob); - auto lazy_glyph_atlas = std::make_shared(); + auto lazy_glyph_atlas = std::make_shared( + TextRenderContextSkia::Make(GetContext())); lazy_glyph_atlas->AddTextFrame(frame, 1.0f); auto text_contents = std::make_shared(); diff --git a/impeller/golden_tests/golden_playground_test.h b/impeller/golden_tests/golden_playground_test.h index a165fdd103189..3a2c8123452a3 100644 --- a/impeller/golden_tests/golden_playground_test.h +++ b/impeller/golden_tests/golden_playground_test.h @@ -32,9 +32,13 @@ class GoldenPlaygroundTest PlaygroundBackend GetBackend() const; - bool OpenPlaygroundHere(const Picture& picture); + bool OpenPlaygroundHere(const Picture& picture, + std::unique_ptr + text_render_context_override = nullptr); - bool OpenPlaygroundHere(const AiksPlaygroundCallback& callback); + bool OpenPlaygroundHere(const AiksPlaygroundCallback& callback, + std::unique_ptr + text_render_context_override = nullptr); std::shared_ptr CreateTextureForFixture( const char* fixture_name, diff --git a/impeller/golden_tests/golden_playground_test_mac.cc b/impeller/golden_tests/golden_playground_test_mac.cc index 55209e7836746..92f23a022ee66 100644 --- a/impeller/golden_tests/golden_playground_test_mac.cc +++ b/impeller/golden_tests/golden_playground_test_mac.cc @@ -10,6 +10,7 @@ #include "flutter/impeller/aiks/picture.h" #include "flutter/impeller/golden_tests/golden_digest.h" #include "flutter/impeller/golden_tests/metal_screenshoter.h" +#include "impeller/typographer/backends/skia/text_render_context_skia.h" namespace impeller { @@ -123,14 +124,22 @@ PlaygroundBackend GoldenPlaygroundTest::GetBackend() const { return GetParam(); } -bool GoldenPlaygroundTest::OpenPlaygroundHere(const Picture& picture) { - auto screenshot = - pimpl_->screenshoter->MakeScreenshot(picture, pimpl_->window_size); +bool GoldenPlaygroundTest::OpenPlaygroundHere( + const Picture& picture, + std::unique_ptr text_render_context_override) { + auto text_context = text_render_context_override + ? std::move(text_render_context_override) + : TextRenderContextSkia::Make(GetContext()); + AiksContext renderer(GetContext(), std::move(text_context)); + + auto screenshot = pimpl_->screenshoter->MakeScreenshot(renderer, picture, + pimpl_->window_size); return SaveScreenshot(std::move(screenshot)); } bool GoldenPlaygroundTest::OpenPlaygroundHere( - const AiksPlaygroundCallback& callback) { + const AiksPlaygroundCallback& callback, + std::unique_ptr text_render_context_override) { return false; } @@ -161,7 +170,7 @@ std::shared_ptr GoldenPlaygroundTest::OpenAssetAsRuntimeStage( } std::shared_ptr GoldenPlaygroundTest::GetContext() const { - return pimpl_->screenshoter->GetContext().GetContext(); + return pimpl_->screenshoter->GetPlayground().GetContext(); } Point GoldenPlaygroundTest::GetContentScale() const { diff --git a/impeller/golden_tests/golden_playground_test_stub.cc b/impeller/golden_tests/golden_playground_test_stub.cc index 054dc5553e624..d1a398be08fec 100644 --- a/impeller/golden_tests/golden_playground_test_stub.cc +++ b/impeller/golden_tests/golden_playground_test_stub.cc @@ -18,12 +18,15 @@ PlaygroundBackend GoldenPlaygroundTest::GetBackend() const { return GetParam(); } -bool GoldenPlaygroundTest::OpenPlaygroundHere(const Picture& picture) { +bool GoldenPlaygroundTest::OpenPlaygroundHere( + const Picture& picture, + std::unique_ptr text_render_context_override) { return false; } bool GoldenPlaygroundTest::OpenPlaygroundHere( - const AiksPlaygroundCallback& callback) { + const AiksPlaygroundCallback& callback, + std::unique_ptr text_render_context_override) { return false; } diff --git a/impeller/golden_tests/golden_tests.cc b/impeller/golden_tests/golden_tests.cc index c99d9abc05852..08bbcf04fb9c6 100644 --- a/impeller/golden_tests/golden_tests.cc +++ b/impeller/golden_tests/golden_tests.cc @@ -56,7 +56,7 @@ class GoldenTests : public ::testing::Test { void SetUp() override { testing::GoldenDigest::Instance()->AddDimension( "gpu_string", - Screenshoter().GetContext().GetContext()->DescribeGpuModel()); + Screenshoter().GetAiksContext().GetContext()->DescribeGpuModel()); } private: diff --git a/impeller/golden_tests/metal_screenshoter.h b/impeller/golden_tests/metal_screenshoter.h index 217c5bc774b3d..961d3186f5c81 100644 --- a/impeller/golden_tests/metal_screenshoter.h +++ b/impeller/golden_tests/metal_screenshoter.h @@ -12,24 +12,20 @@ namespace impeller { namespace testing { -/// Converts `Picture`'s to `MetalScreenshot`'s with the playground backend. +/// Converts `Picture`s to `MetalScreenshot`s with the playground backend. class MetalScreenshoter { public: MetalScreenshoter(); - std::unique_ptr MakeScreenshot(const Picture& picture, + std::unique_ptr MakeScreenshot(AiksContext& aiks_context, + const Picture& picture, const ISize& size = {300, 300}); - AiksContext& GetContext() { return *aiks_context_; } - - const AiksContext& GetContext() const { return *aiks_context_; } - const PlaygroundImpl& GetPlayground() const { return *playground_; } private: std::unique_ptr playground_; - std::unique_ptr aiks_context_; }; } // namespace testing diff --git a/impeller/golden_tests/metal_screenshoter.mm b/impeller/golden_tests/metal_screenshoter.mm index acf41f07a200c..1d01b65386a8a 100644 --- a/impeller/golden_tests/metal_screenshoter.mm +++ b/impeller/golden_tests/metal_screenshoter.mm @@ -17,15 +17,15 @@ FML_CHECK(::glfwInit() == GLFW_TRUE); playground_ = PlaygroundImpl::Create(PlaygroundBackend::kMetal, PlaygroundSwitches{}); - aiks_context_.reset(new AiksContext(playground_->GetContext())); } std::unique_ptr MetalScreenshoter::MakeScreenshot( + AiksContext& aiks_context, const Picture& picture, const ISize& size) { Vector2 content_scale = playground_->GetContentScale(); std::shared_ptr image = picture.ToImage( - *aiks_context_, + aiks_context, ISize(size.width * content_scale.x, size.height * content_scale.y)); std::shared_ptr texture = image->GetTexture(); id metal_texture = diff --git a/impeller/renderer/compute_subgroup_unittests.cc b/impeller/renderer/compute_subgroup_unittests.cc index 8c9cc4fba14ec..bec4030735911 100644 --- a/impeller/renderer/compute_subgroup_unittests.cc +++ b/impeller/renderer/compute_subgroup_unittests.cc @@ -125,7 +125,7 @@ TEST_P(ComputeSubgroupTest, PathPlayground) { } ImGui::End(); - ContentContext renderer(context); + ContentContext renderer(context, nullptr); if (!renderer.IsValid()) { return false; } @@ -329,7 +329,7 @@ TEST_P(ComputeSubgroupTest, LargePath) { ->count; }); - ContentContext renderer(context); + ContentContext renderer(context, nullptr); if (!renderer.IsValid()) { return false; } @@ -413,7 +413,7 @@ TEST_P(ComputeSubgroupTest, QuadAndCubicInOnePath) { ASSERT_EQ(status, ComputeTessellator::Status::kOk); auto callback = [&](RenderPass& pass) -> bool { - ContentContext renderer(context); + ContentContext renderer(context, nullptr); if (!renderer.IsValid()) { return false; } diff --git a/impeller/typographer/backends/skia/text_render_context_skia.cc b/impeller/typographer/backends/skia/text_render_context_skia.cc index 1dca003c66a23..a26b698f88a08 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.cc +++ b/impeller/typographer/backends/skia/text_render_context_skia.cc @@ -12,6 +12,7 @@ #include "impeller/core/allocator.h" #include "impeller/typographer/backends/skia/typeface_skia.h" #include "impeller/typographer/rectangle_packer.h" +#include "impeller/typographer/text_render_context.h" #include "third_party/skia/include/core/SkBitmap.h" #include "third_party/skia/include/core/SkCanvas.h" #include "third_party/skia/include/core/SkFont.h" @@ -24,17 +25,16 @@ namespace impeller { using FontGlyphPairRefVector = std::vector>; -std::unique_ptr TextRenderContext::Create( - std::shared_ptr context) { - // There is only one backend today. - return std::make_unique(std::move(context)); -} - // TODO(bdero): We might be able to remove this per-glyph padding if we fix // the underlying causes of the overlap. // https://github.com/flutter/flutter/issues/114563 constexpr auto kPadding = 2; +std::unique_ptr TextRenderContextSkia::Make( + std::shared_ptr context) { + return std::make_unique(std::move(context)); +} + TextRenderContextSkia::TextRenderContextSkia(std::shared_ptr context) : TextRenderContext(std::move(context)) {} diff --git a/impeller/typographer/backends/skia/text_render_context_skia.h b/impeller/typographer/backends/skia/text_render_context_skia.h index 2e2e6ad97cf8b..1e5bf4f402ded 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.h +++ b/impeller/typographer/backends/skia/text_render_context_skia.h @@ -11,6 +11,9 @@ namespace impeller { class TextRenderContextSkia : public TextRenderContext { public: + static std::unique_ptr Make( + std::shared_ptr context); + TextRenderContextSkia(std::shared_ptr context); ~TextRenderContextSkia() override; diff --git a/impeller/typographer/lazy_glyph_atlas.cc b/impeller/typographer/lazy_glyph_atlas.cc index f98f8af6d62b3..0bb4628a2784a 100644 --- a/impeller/typographer/lazy_glyph_atlas.cc +++ b/impeller/typographer/lazy_glyph_atlas.cc @@ -11,8 +11,10 @@ namespace impeller { -LazyGlyphAtlas::LazyGlyphAtlas() - : alpha_context_(std::make_shared()), +LazyGlyphAtlas::LazyGlyphAtlas( + std::unique_ptr text_render_context) + : text_render_context_(std::move(text_render_context)), + alpha_context_(std::make_shared()), color_context_(std::make_shared()) {} LazyGlyphAtlas::~LazyGlyphAtlas() = default; @@ -33,8 +35,7 @@ void LazyGlyphAtlas::ResetTextFrames() { } std::shared_ptr LazyGlyphAtlas::CreateOrGetGlyphAtlas( - GlyphAtlas::Type type, - std::shared_ptr context) const { + GlyphAtlas::Type type) const { { auto atlas_it = atlas_map_.find(type); if (atlas_it != atlas_map_.end()) { @@ -42,14 +43,21 @@ std::shared_ptr LazyGlyphAtlas::CreateOrGetGlyphAtlas( } } - auto text_context = TextRenderContext::Create(std::move(context)); - if (!text_context || !text_context->IsValid()) { + if (!text_render_context_) { + VALIDATION_LOG << "Unable to render text because a TextRenderContext has " + "not been set."; return nullptr; } + if (!text_render_context_->IsValid()) { + VALIDATION_LOG + << "Unable to render text because the TextRenderContext is invalid."; + return nullptr; + } + auto& set = type == GlyphAtlas::Type::kAlphaBitmap ? alpha_set_ : color_set_; auto atlas_context = type == GlyphAtlas::Type::kAlphaBitmap ? alpha_context_ : color_context_; - auto atlas = text_context->CreateGlyphAtlas(type, atlas_context, set); + auto atlas = text_render_context_->CreateGlyphAtlas(type, 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 7289538eb4be9..6686cca9f67a7 100644 --- a/impeller/typographer/lazy_glyph_atlas.h +++ b/impeller/typographer/lazy_glyph_atlas.h @@ -10,12 +10,14 @@ #include "impeller/renderer/context.h" #include "impeller/typographer/glyph_atlas.h" #include "impeller/typographer/text_frame.h" +#include "impeller/typographer/text_render_context.h" namespace impeller { class LazyGlyphAtlas { public: - LazyGlyphAtlas(); + explicit LazyGlyphAtlas( + std::unique_ptr text_render_context); ~LazyGlyphAtlas(); @@ -24,10 +26,11 @@ class LazyGlyphAtlas { void ResetTextFrames(); std::shared_ptr CreateOrGetGlyphAtlas( - GlyphAtlas::Type type, - std::shared_ptr context) const; + GlyphAtlas::Type type) const; private: + std::unique_ptr text_render_context_; + FontGlyphPair::Set alpha_set_; FontGlyphPair::Set color_set_; std::shared_ptr alpha_context_; diff --git a/impeller/typographer/text_render_context.h b/impeller/typographer/text_render_context.h index d909388105f93..a77f18c6fbd86 100644 --- a/impeller/typographer/text_render_context.h +++ b/impeller/typographer/text_render_context.h @@ -23,9 +23,6 @@ namespace impeller { /// class TextRenderContext { public: - static std::unique_ptr Create( - std::shared_ptr context); - virtual ~TextRenderContext(); virtual bool IsValid() const; diff --git a/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc index 02e8e790f8367..2d4a1d552f40c 100644 --- a/impeller/typographer/typographer_unittests.cc +++ b/impeller/typographer/typographer_unittests.cc @@ -8,7 +8,6 @@ #include "impeller/typographer/backends/skia/text_render_context_skia.h" #include "impeller/typographer/lazy_glyph_atlas.h" #include "impeller/typographer/rectangle_packer.h" -#include "third_party/skia/include/core/SkBitmap.h" #include "third_party/skia/include/core/SkData.h" #include "third_party/skia/include/core/SkFontMgr.h" #include "third_party/skia/include/core/SkRect.h" @@ -48,12 +47,12 @@ TEST_P(TypographerTest, CanConvertTextBlob) { } TEST_P(TypographerTest, CanCreateRenderContext) { - auto context = TextRenderContext::Create(GetContext()); + auto context = TextRenderContextSkia::Make(GetContext()); ASSERT_TRUE(context && context->IsValid()); } TEST_P(TypographerTest, CanCreateGlyphAtlas) { - auto context = TextRenderContext::Create(GetContext()); + auto context = TextRenderContextSkia::Make(GetContext()); auto atlas_context = std::make_shared(); ASSERT_TRUE(context && context->IsValid()); SkFont sk_font; @@ -111,7 +110,7 @@ TEST_P(TypographerTest, LazyAtlasTracksColor) { ASSERT_FALSE(frame.GetAtlasType() == GlyphAtlas::Type::kColorBitmap); - LazyGlyphAtlas lazy_atlas; + LazyGlyphAtlas lazy_atlas(TextRenderContextSkia::Make(GetContext())); lazy_atlas.AddTextFrame(frame, 1.0f); @@ -122,17 +121,17 @@ TEST_P(TypographerTest, LazyAtlasTracksColor) { lazy_atlas.AddTextFrame(frame, 1.0f); // Creates different atlases for color and alpha bitmap. - auto color_atlas = lazy_atlas.CreateOrGetGlyphAtlas( - GlyphAtlas::Type::kColorBitmap, GetContext()); + auto color_atlas = + lazy_atlas.CreateOrGetGlyphAtlas(GlyphAtlas::Type::kColorBitmap); - auto bitmap_atlas = lazy_atlas.CreateOrGetGlyphAtlas( - GlyphAtlas::Type::kAlphaBitmap, GetContext()); + auto bitmap_atlas = + lazy_atlas.CreateOrGetGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap); ASSERT_FALSE(color_atlas == bitmap_atlas); } TEST_P(TypographerTest, GlyphAtlasWithOddUniqueGlyphSize) { - auto context = TextRenderContext::Create(GetContext()); + auto context = TextRenderContextSkia::Make(GetContext()); auto atlas_context = std::make_shared(); ASSERT_TRUE(context && context->IsValid()); SkFont sk_font; @@ -149,7 +148,7 @@ TEST_P(TypographerTest, GlyphAtlasWithOddUniqueGlyphSize) { } TEST_P(TypographerTest, GlyphAtlasIsRecycledIfUnchanged) { - auto context = TextRenderContext::Create(GetContext()); + auto context = TextRenderContextSkia::Make(GetContext()); auto atlas_context = std::make_shared(); ASSERT_TRUE(context && context->IsValid()); SkFont sk_font; @@ -172,7 +171,7 @@ TEST_P(TypographerTest, GlyphAtlasIsRecycledIfUnchanged) { } TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) { - auto context = TextRenderContext::Create(GetContext()); + auto context = TextRenderContextSkia::Make(GetContext()); auto atlas_context = std::make_shared(); ASSERT_TRUE(context && context->IsValid()); @@ -213,7 +212,7 @@ TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) { } TEST_P(TypographerTest, GlyphAtlasTextureIsRecycledIfUnchanged) { - auto context = TextRenderContext::Create(GetContext()); + auto context = TextRenderContextSkia::Make(GetContext()); auto atlas_context = std::make_shared(); ASSERT_TRUE(context && context->IsValid()); SkFont sk_font; @@ -246,7 +245,7 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecycledIfUnchanged) { } TEST_P(TypographerTest, GlyphAtlasTextureIsRecreatedIfTypeChanges) { - auto context = TextRenderContext::Create(GetContext()); + auto context = TextRenderContextSkia::Make(GetContext()); auto atlas_context = std::make_shared(); ASSERT_TRUE(context && context->IsValid()); SkFont sk_font; From 5c5ea90a965fcca2b36b423c1eda25d933a4dfdf Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Sun, 20 Aug 2023 14:03:35 -0700 Subject: [PATCH 2/7] Don't unnecessarily hold a ref to an Impeller context in the TextRenderContext --- impeller/aiks/aiks_playground.cc | 4 +- impeller/display_list/dl_playground.cc | 2 +- impeller/entity/contents/text_contents.cc | 8 +- impeller/entity/contents/text_contents.h | 1 + impeller/entity/entity_playground.cc | 9 +- impeller/entity/entity_unittests.cc | 4 +- .../golden_playground_test_mac.cc | 2 +- .../backends/skia/text_render_context_skia.cc | 15 ++-- .../backends/skia/text_render_context_skia.h | 6 +- impeller/typographer/lazy_glyph_atlas.cc | 4 +- impeller/typographer/lazy_glyph_atlas.h | 1 + impeller/typographer/text_render_context.cc | 10 +-- impeller/typographer/text_render_context.h | 14 +--- impeller/typographer/typographer_unittests.cc | 83 ++++++++++--------- 14 files changed, 73 insertions(+), 90 deletions(-) diff --git a/impeller/aiks/aiks_playground.cc b/impeller/aiks/aiks_playground.cc index 9fda4406bfd37..b0e506641a5a1 100644 --- a/impeller/aiks/aiks_playground.cc +++ b/impeller/aiks/aiks_playground.cc @@ -20,7 +20,7 @@ bool AiksPlayground::OpenPlaygroundHere( std::unique_ptr text_render_context_override) { auto text_context = text_render_context_override ? std::move(text_render_context_override) - : TextRenderContextSkia::Make(GetContext()); + : TextRenderContextSkia::Make(); return OpenPlaygroundHere( [&picture](AiksContext& renderer, RenderTarget& render_target) -> bool { return renderer.Render(picture, render_target); @@ -37,7 +37,7 @@ bool AiksPlayground::OpenPlaygroundHere( auto text_context = text_render_context_override ? std::move(text_render_context_override) - : TextRenderContextSkia::Make(GetContext()); + : TextRenderContextSkia::Make(); AiksContext renderer(GetContext(), std::move(text_context)); if (!renderer.IsValid()) { diff --git a/impeller/display_list/dl_playground.cc b/impeller/display_list/dl_playground.cc index 9c5d1b4903170..b6d1f5468fe1e 100644 --- a/impeller/display_list/dl_playground.cc +++ b/impeller/display_list/dl_playground.cc @@ -30,7 +30,7 @@ bool DlPlayground::OpenPlaygroundHere(DisplayListPlaygroundCallback callback) { return true; } - AiksContext context(GetContext(), TextRenderContextSkia::Make(GetContext())); + AiksContext context(GetContext(), TextRenderContextSkia::Make()); if (!context.IsValid()) { return false; } diff --git a/impeller/entity/contents/text_contents.cc b/impeller/entity/contents/text_contents.cc index 3b1a2586174c1..9267771b9c59c 100644 --- a/impeller/entity/contents/text_contents.cc +++ b/impeller/entity/contents/text_contents.cc @@ -12,10 +12,8 @@ #include "impeller/core/sampler_descriptor.h" #include "impeller/entity/contents/content_context.h" #include "impeller/entity/entity.h" -#include "impeller/geometry/path_builder.h" #include "impeller/renderer/render_pass.h" #include "impeller/renderer/sampler_library.h" -#include "impeller/tessellator/tessellator.h" #include "impeller/typographer/glyph_atlas.h" #include "impeller/typographer/lazy_glyph_atlas.h" @@ -30,11 +28,12 @@ void TextContents::SetTextFrame(const TextFrame& frame) { } std::shared_ptr TextContents::ResolveAtlas( + Context& context, GlyphAtlas::Type type, const std::shared_ptr& lazy_atlas) const { FML_DCHECK(lazy_atlas); if (lazy_atlas) { - return lazy_atlas->CreateOrGetGlyphAtlas(type); + return lazy_atlas->CreateOrGetGlyphAtlas(context, type); } return nullptr; @@ -88,7 +87,8 @@ bool TextContents::Render(const ContentContext& renderer, } auto type = frame_.GetAtlasType(); - auto atlas = ResolveAtlas(type, renderer.GetLazyGlyphAtlas()); + auto atlas = + ResolveAtlas(*renderer.GetContext(), type, renderer.GetLazyGlyphAtlas()); if (!atlas || !atlas->IsValid()) { VALIDATION_LOG << "Cannot render glyphs without prepared atlas."; diff --git a/impeller/entity/contents/text_contents.h b/impeller/entity/contents/text_contents.h index 15f17ff809168..70f83d10e45ae 100644 --- a/impeller/entity/contents/text_contents.h +++ b/impeller/entity/contents/text_contents.h @@ -63,6 +63,7 @@ class TextContents final : public Contents { Vector2 offset_; std::shared_ptr ResolveAtlas( + Context& context, GlyphAtlas::Type type, const std::shared_ptr& lazy_atlas) const; diff --git a/impeller/entity/entity_playground.cc b/impeller/entity/entity_playground.cc index f6b645d5c1878..5bffd89cf02e3 100644 --- a/impeller/entity/entity_playground.cc +++ b/impeller/entity/entity_playground.cc @@ -20,8 +20,7 @@ bool EntityPlayground::OpenPlaygroundHere(EntityPass& entity_pass) { return true; } - ContentContext content_context(GetContext(), - TextRenderContextSkia::Make(GetContext())); + ContentContext content_context(GetContext(), TextRenderContextSkia::Make()); if (!content_context.IsValid()) { return false; } @@ -37,8 +36,7 @@ bool EntityPlayground::OpenPlaygroundHere(Entity entity) { return true; } - ContentContext content_context(GetContext(), - TextRenderContextSkia::Make(GetContext())); + ContentContext content_context(GetContext(), TextRenderContextSkia::Make()); if (!content_context.IsValid()) { return false; } @@ -53,8 +51,7 @@ bool EntityPlayground::OpenPlaygroundHere(EntityPlaygroundCallback callback) { return true; } - ContentContext content_context(GetContext(), - TextRenderContextSkia::Make(GetContext())); + ContentContext content_context(GetContext(), TextRenderContextSkia::Make()); if (!content_context.IsValid()) { return false; } diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc index 5c4840f48276b..90edc60222883 100644 --- a/impeller/entity/entity_unittests.cc +++ b/impeller/entity/entity_unittests.cc @@ -2175,8 +2175,8 @@ TEST_P(EntityTest, InheritOpacityTest) { font.setSize(30); auto blob = SkTextBlob::MakeFromString("A", font); auto frame = TextFrameFromTextBlob(blob); - auto lazy_glyph_atlas = std::make_shared( - TextRenderContextSkia::Make(GetContext())); + auto lazy_glyph_atlas = + std::make_shared(TextRenderContextSkia::Make()); lazy_glyph_atlas->AddTextFrame(frame, 1.0f); auto text_contents = std::make_shared(); diff --git a/impeller/golden_tests/golden_playground_test_mac.cc b/impeller/golden_tests/golden_playground_test_mac.cc index 92f23a022ee66..1e9f9ff26abce 100644 --- a/impeller/golden_tests/golden_playground_test_mac.cc +++ b/impeller/golden_tests/golden_playground_test_mac.cc @@ -129,7 +129,7 @@ bool GoldenPlaygroundTest::OpenPlaygroundHere( std::unique_ptr text_render_context_override) { auto text_context = text_render_context_override ? std::move(text_render_context_override) - : TextRenderContextSkia::Make(GetContext()); + : TextRenderContextSkia::Make(); AiksContext renderer(GetContext(), std::move(text_context)); auto screenshot = pimpl_->screenshoter->MakeScreenshot(renderer, picture, diff --git a/impeller/typographer/backends/skia/text_render_context_skia.cc b/impeller/typographer/backends/skia/text_render_context_skia.cc index a26b698f88a08..d34995ce2bbcd 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.cc +++ b/impeller/typographer/backends/skia/text_render_context_skia.cc @@ -16,8 +16,6 @@ #include "third_party/skia/include/core/SkBitmap.h" #include "third_party/skia/include/core/SkCanvas.h" #include "third_party/skia/include/core/SkFont.h" -#include "third_party/skia/include/core/SkFontMetrics.h" -#include "third_party/skia/include/core/SkRSXform.h" #include "third_party/skia/include/core/SkSurface.h" namespace impeller { @@ -30,13 +28,11 @@ using FontGlyphPairRefVector = // https://github.com/flutter/flutter/issues/114563 constexpr auto kPadding = 2; -std::unique_ptr TextRenderContextSkia::Make( - std::shared_ptr context) { - return std::make_unique(std::move(context)); +std::unique_ptr TextRenderContextSkia::Make() { + return std::make_unique(); } -TextRenderContextSkia::TextRenderContextSkia(std::shared_ptr context) - : TextRenderContext(std::move(context)) {} +TextRenderContextSkia::TextRenderContextSkia() = default; TextRenderContextSkia::~TextRenderContextSkia() = default; @@ -310,6 +306,7 @@ static std::shared_ptr UploadGlyphTextureAtlas( } std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( + Context& context, GlyphAtlas::Type type, std::shared_ptr atlas_context, const FontGlyphPair::Set& font_glyph_pairs) const { @@ -429,8 +426,8 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( format = PixelFormat::kR8G8B8A8UNormInt; break; } - auto texture = UploadGlyphTextureAtlas(GetContext()->GetResourceAllocator(), - bitmap, atlas_size, format); + auto texture = UploadGlyphTextureAtlas(context.GetResourceAllocator(), bitmap, + atlas_size, format); if (!texture) { return nullptr; } diff --git a/impeller/typographer/backends/skia/text_render_context_skia.h b/impeller/typographer/backends/skia/text_render_context_skia.h index 1e5bf4f402ded..3113ea5225772 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.h +++ b/impeller/typographer/backends/skia/text_render_context_skia.h @@ -11,15 +11,15 @@ namespace impeller { class TextRenderContextSkia : public TextRenderContext { public: - static std::unique_ptr Make( - std::shared_ptr context); + static std::unique_ptr Make(); - TextRenderContextSkia(std::shared_ptr context); + TextRenderContextSkia(); ~TextRenderContextSkia() override; // |TextRenderContext| std::shared_ptr CreateGlyphAtlas( + Context& context, GlyphAtlas::Type type, std::shared_ptr atlas_context, const FontGlyphPair::Set& font_glyph_pairs) const override; diff --git a/impeller/typographer/lazy_glyph_atlas.cc b/impeller/typographer/lazy_glyph_atlas.cc index 0bb4628a2784a..2b0ac7ab713c3 100644 --- a/impeller/typographer/lazy_glyph_atlas.cc +++ b/impeller/typographer/lazy_glyph_atlas.cc @@ -35,6 +35,7 @@ void LazyGlyphAtlas::ResetTextFrames() { } std::shared_ptr LazyGlyphAtlas::CreateOrGetGlyphAtlas( + Context& context, GlyphAtlas::Type type) const { { auto atlas_it = atlas_map_.find(type); @@ -57,7 +58,8 @@ std::shared_ptr LazyGlyphAtlas::CreateOrGetGlyphAtlas( auto& set = type == GlyphAtlas::Type::kAlphaBitmap ? alpha_set_ : color_set_; auto atlas_context = type == GlyphAtlas::Type::kAlphaBitmap ? alpha_context_ : color_context_; - auto atlas = text_render_context_->CreateGlyphAtlas(type, atlas_context, set); + auto atlas = + text_render_context_->CreateGlyphAtlas(context, type, 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 6686cca9f67a7..5c2051d11b5bf 100644 --- a/impeller/typographer/lazy_glyph_atlas.h +++ b/impeller/typographer/lazy_glyph_atlas.h @@ -26,6 +26,7 @@ class LazyGlyphAtlas { void ResetTextFrames(); std::shared_ptr CreateOrGetGlyphAtlas( + Context& context, GlyphAtlas::Type type) const; private: diff --git a/impeller/typographer/text_render_context.cc b/impeller/typographer/text_render_context.cc index 1b7aba10a7b74..8476b62c7b6aa 100644 --- a/impeller/typographer/text_render_context.cc +++ b/impeller/typographer/text_render_context.cc @@ -8,11 +8,7 @@ namespace impeller { -TextRenderContext::TextRenderContext(std::shared_ptr context) - : context_(std::move(context)) { - if (!context_ || !context_->IsValid()) { - return; - } +TextRenderContext::TextRenderContext() { is_valid_ = true; } @@ -22,8 +18,4 @@ bool TextRenderContext::IsValid() const { return is_valid_; } -const std::shared_ptr& TextRenderContext::GetContext() const { - return context_; -} - } // namespace impeller diff --git a/impeller/typographer/text_render_context.h b/impeller/typographer/text_render_context.h index a77f18c6fbd86..754e9c26bb5c0 100644 --- a/impeller/typographer/text_render_context.h +++ b/impeller/typographer/text_render_context.h @@ -10,7 +10,6 @@ #include "flutter/fml/macros.h" #include "impeller/renderer/context.h" #include "impeller/typographer/glyph_atlas.h" -#include "impeller/typographer/text_frame.h" namespace impeller { @@ -27,17 +26,11 @@ class TextRenderContext { virtual bool IsValid() const; - //---------------------------------------------------------------------------- - /// @brief Get the underlying graphics context. - /// - /// @return The context. - /// - const std::shared_ptr& GetContext() const; - // 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( + Context& context, GlyphAtlas::Type type, std::shared_ptr atlas_context, const FontGlyphPair::Set& font_glyph_pairs) const = 0; @@ -47,12 +40,9 @@ class TextRenderContext { /// @brief Create a new context to render text that talks to an /// underlying graphics context. /// - /// @param[in] context The graphics context - /// - TextRenderContext(std::shared_ptr context); + TextRenderContext(); private: - std::shared_ptr context_; bool is_valid_ = false; FML_DISALLOW_COPY_AND_ASSIGN(TextRenderContext); diff --git a/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc index 2d4a1d552f40c..09c78daea06b3 100644 --- a/impeller/typographer/typographer_unittests.cc +++ b/impeller/typographer/typographer_unittests.cc @@ -23,14 +23,16 @@ using TypographerTest = PlaygroundTest; INSTANTIATE_PLAYGROUND_SUITE(TypographerTest); static std::shared_ptr CreateGlyphAtlas( - const TextRenderContext* context, + Context& context, + const TextRenderContext* text_render_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); + return text_render_context->CreateGlyphAtlas(context, type, atlas_context, + set); } TEST_P(TypographerTest, CanConvertTextBlob) { @@ -47,20 +49,20 @@ TEST_P(TypographerTest, CanConvertTextBlob) { } TEST_P(TypographerTest, CanCreateRenderContext) { - auto context = TextRenderContextSkia::Make(GetContext()); + auto context = TextRenderContextSkia::Make(); ASSERT_TRUE(context && context->IsValid()); } TEST_P(TypographerTest, CanCreateGlyphAtlas) { - auto context = TextRenderContextSkia::Make(GetContext()); + auto context = TextRenderContextSkia::Make(); auto atlas_context = std::make_shared(); ASSERT_TRUE(context && context->IsValid()); SkFont sk_font; auto blob = SkTextBlob::MakeFromString("hello", sk_font); ASSERT_TRUE(blob); - auto atlas = - CreateGlyphAtlas(context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, - atlas_context, TextFrameFromTextBlob(blob)); + auto atlas = CreateGlyphAtlas(*GetContext(), 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); @@ -110,7 +112,7 @@ TEST_P(TypographerTest, LazyAtlasTracksColor) { ASSERT_FALSE(frame.GetAtlasType() == GlyphAtlas::Type::kColorBitmap); - LazyGlyphAtlas lazy_atlas(TextRenderContextSkia::Make(GetContext())); + LazyGlyphAtlas lazy_atlas(TextRenderContextSkia::Make()); lazy_atlas.AddTextFrame(frame, 1.0f); @@ -121,25 +123,25 @@ TEST_P(TypographerTest, LazyAtlasTracksColor) { lazy_atlas.AddTextFrame(frame, 1.0f); // Creates different atlases for color and alpha bitmap. - auto color_atlas = - lazy_atlas.CreateOrGetGlyphAtlas(GlyphAtlas::Type::kColorBitmap); + auto color_atlas = lazy_atlas.CreateOrGetGlyphAtlas( + *GetContext(), GlyphAtlas::Type::kColorBitmap); - auto bitmap_atlas = - lazy_atlas.CreateOrGetGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap); + auto bitmap_atlas = lazy_atlas.CreateOrGetGlyphAtlas( + *GetContext(), GlyphAtlas::Type::kAlphaBitmap); ASSERT_FALSE(color_atlas == bitmap_atlas); } TEST_P(TypographerTest, GlyphAtlasWithOddUniqueGlyphSize) { - auto context = TextRenderContextSkia::Make(GetContext()); + auto context = TextRenderContextSkia::Make(); auto atlas_context = std::make_shared(); ASSERT_TRUE(context && context->IsValid()); SkFont sk_font; auto blob = SkTextBlob::MakeFromString("AGH", sk_font); ASSERT_TRUE(blob); - auto atlas = - CreateGlyphAtlas(context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, - atlas_context, TextFrameFromTextBlob(blob)); + auto atlas = CreateGlyphAtlas(*GetContext(), context.get(), + GlyphAtlas::Type::kAlphaBitmap, 1.0f, + atlas_context, TextFrameFromTextBlob(blob)); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); @@ -148,30 +150,30 @@ TEST_P(TypographerTest, GlyphAtlasWithOddUniqueGlyphSize) { } TEST_P(TypographerTest, GlyphAtlasIsRecycledIfUnchanged) { - auto context = TextRenderContextSkia::Make(GetContext()); + auto context = TextRenderContextSkia::Make(); auto atlas_context = std::make_shared(); ASSERT_TRUE(context && context->IsValid()); SkFont sk_font; auto blob = SkTextBlob::MakeFromString("spooky skellingtons", sk_font); ASSERT_TRUE(blob); - auto atlas = - CreateGlyphAtlas(context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, - atlas_context, TextFrameFromTextBlob(blob)); + auto atlas = CreateGlyphAtlas(*GetContext(), 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()); // now attempt to re-create an atlas with the same text blob. - auto next_atlas = - CreateGlyphAtlas(context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, - atlas_context, TextFrameFromTextBlob(blob)); + auto next_atlas = CreateGlyphAtlas( + *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, + atlas_context, TextFrameFromTextBlob(blob)); ASSERT_EQ(atlas, next_atlas); ASSERT_EQ(atlas_context->GetGlyphAtlas(), atlas); } TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) { - auto context = TextRenderContextSkia::Make(GetContext()); + auto context = TextRenderContextSkia::Make(); auto atlas_context = std::make_shared(); ASSERT_TRUE(context && context->IsValid()); @@ -190,8 +192,9 @@ TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) { for (size_t index = 0; index < size_count; index += 1) { TextFrameFromTextBlob(blob).CollectUniqueFontGlyphPairs(set, 0.6 * index); }; - auto atlas = context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, - std::move(atlas_context), set); + auto atlas = + context->CreateGlyphAtlas(*GetContext(), GlyphAtlas::Type::kAlphaBitmap, + std::move(atlas_context), set); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); @@ -212,15 +215,15 @@ TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) { } TEST_P(TypographerTest, GlyphAtlasTextureIsRecycledIfUnchanged) { - auto context = TextRenderContextSkia::Make(GetContext()); + auto context = TextRenderContextSkia::Make(); auto atlas_context = std::make_shared(); ASSERT_TRUE(context && context->IsValid()); SkFont sk_font; auto blob = SkTextBlob::MakeFromString("spooky 1", sk_font); ASSERT_TRUE(blob); - auto atlas = - CreateGlyphAtlas(context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, - atlas_context, TextFrameFromTextBlob(blob)); + auto atlas = CreateGlyphAtlas(*GetContext(), context.get(), + GlyphAtlas::Type::kAlphaBitmap, 1.0f, + atlas_context, TextFrameFromTextBlob(blob)); auto old_packer = atlas_context->GetRectPacker(); ASSERT_NE(atlas, nullptr); @@ -232,9 +235,9 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecycledIfUnchanged) { // Now create a new glyph atlas with a nearly identical blob. auto blob2 = SkTextBlob::MakeFromString("spooky 2", sk_font); - auto next_atlas = - CreateGlyphAtlas(context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, - atlas_context, TextFrameFromTextBlob(blob2)); + auto next_atlas = CreateGlyphAtlas( + *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, + atlas_context, TextFrameFromTextBlob(blob2)); ASSERT_EQ(atlas, next_atlas); auto* second_texture = next_atlas->GetTexture().get(); @@ -245,15 +248,15 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecycledIfUnchanged) { } TEST_P(TypographerTest, GlyphAtlasTextureIsRecreatedIfTypeChanges) { - auto context = TextRenderContextSkia::Make(GetContext()); + auto context = TextRenderContextSkia::Make(); auto atlas_context = std::make_shared(); ASSERT_TRUE(context && context->IsValid()); SkFont sk_font; auto blob = SkTextBlob::MakeFromString("spooky 1", sk_font); ASSERT_TRUE(blob); - auto atlas = - CreateGlyphAtlas(context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, - atlas_context, TextFrameFromTextBlob(blob)); + auto atlas = CreateGlyphAtlas(*GetContext(), context.get(), + GlyphAtlas::Type::kAlphaBitmap, 1.0f, + atlas_context, TextFrameFromTextBlob(blob)); auto old_packer = atlas_context->GetRectPacker(); ASSERT_NE(atlas, nullptr); @@ -266,9 +269,9 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecreatedIfTypeChanges) { // but change the type. auto blob2 = SkTextBlob::MakeFromString("spooky 1", sk_font); - auto next_atlas = - CreateGlyphAtlas(context.get(), GlyphAtlas::Type::kColorBitmap, 1.0f, - atlas_context, TextFrameFromTextBlob(blob2)); + auto next_atlas = CreateGlyphAtlas( + *GetContext(), context.get(), GlyphAtlas::Type::kColorBitmap, 1.0f, + atlas_context, TextFrameFromTextBlob(blob2)); ASSERT_NE(atlas, next_atlas); auto* second_texture = next_atlas->GetTexture().get(); From a43b69ec7bc3d0da0489d2bcc9f794591344d9e1 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Sun, 20 Aug 2023 14:40:25 -0700 Subject: [PATCH 3/7] Install the skia text backend when building shell surfaces --- shell/common/rasterizer.h | 4 +++- shell/gpu/gpu_surface_gl_impeller.cc | 10 ++++++---- shell/gpu/gpu_surface_metal_impeller.mm | 8 +++++--- shell/gpu/gpu_surface_vulkan_impeller.cc | 8 +++++--- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/shell/common/rasterizer.h b/shell/common/rasterizer.h index bb05e4475f998..ff2a4d4cf99c0 100644 --- a/shell/common/rasterizer.h +++ b/shell/common/rasterizer.h @@ -23,6 +23,7 @@ #include "flutter/fml/synchronization/waitable_event.h" #include "flutter/fml/time/time_delta.h" #include "flutter/fml/time/time_point.h" +#include "impeller/typographer/backends/skia/text_render_context_skia.h" #if IMPELLER_SUPPORTS_RENDERING // GN is having trouble understanding how this works in the Fuchsia builds. #include "flutter/impeller/aiks/aiks_context.h" // nogncheck @@ -544,7 +545,8 @@ class Rasterizer final : public SnapshotDelegate, return surface_->GetAiksContext(); } if (auto context = impeller_context_.lock()) { - return std::make_shared(context); + return std::make_shared( + context, impeller::TextRenderContextSkia::Make()); } #endif return nullptr; diff --git a/shell/gpu/gpu_surface_gl_impeller.cc b/shell/gpu/gpu_surface_gl_impeller.cc index 84372944eb6d4..6cb4faa8dbed1 100644 --- a/shell/gpu/gpu_surface_gl_impeller.cc +++ b/shell/gpu/gpu_surface_gl_impeller.cc @@ -5,9 +5,10 @@ #include "flutter/shell/gpu/gpu_surface_gl_impeller.h" #include "flutter/fml/make_copyable.h" -#include "flutter/impeller/display_list/dl_dispatcher.h" -#include "flutter/impeller/renderer/backend/gles/surface_gles.h" -#include "flutter/impeller/renderer/renderer.h" +#include "impeller/display_list/dl_dispatcher.h" +#include "impeller/renderer/backend/gles/surface_gles.h" +#include "impeller/renderer/renderer.h" +#include "impeller/typographer/backends/skia/text_render_context_skia.h" namespace flutter { @@ -28,7 +29,8 @@ GPUSurfaceGLImpeller::GPUSurfaceGLImpeller( return; } - auto aiks_context = std::make_shared(context); + auto aiks_context = std::make_shared( + context, impeller::TextRenderContextSkia::Make()); if (!aiks_context->IsValid()) { return; diff --git a/shell/gpu/gpu_surface_metal_impeller.mm b/shell/gpu/gpu_surface_metal_impeller.mm index 2db00a25dc4fd..c95e53599b57e 100644 --- a/shell/gpu/gpu_surface_metal_impeller.mm +++ b/shell/gpu/gpu_surface_metal_impeller.mm @@ -11,8 +11,9 @@ #include "flutter/fml/make_copyable.h" #include "flutter/fml/mapping.h" #include "flutter/fml/trace_event.h" -#include "flutter/impeller/display_list/dl_dispatcher.h" -#include "flutter/impeller/renderer/backend/metal/surface_mtl.h" +#include "impeller/display_list/dl_dispatcher.h" +#include "impeller/renderer/backend/metal/surface_mtl.h" +#include "impeller/typographer/backends/skia/text_render_context_skia.h" static_assert(!__has_feature(objc_arc), "ARC must be disabled."); @@ -35,7 +36,8 @@ render_target_type_(delegate->GetRenderTargetType()), impeller_renderer_(CreateImpellerRenderer(context)), aiks_context_( - std::make_shared(impeller_renderer_ ? context : nullptr)), + std::make_shared(impeller_renderer_ ? context : nullptr, + impeller::TextRenderContextSkia::Make())), render_to_surface_(render_to_surface) { // If this preference is explicitly set, we allow for disabling partial repaint. NSNumber* disablePartialRepaint = diff --git a/shell/gpu/gpu_surface_vulkan_impeller.cc b/shell/gpu/gpu_surface_vulkan_impeller.cc index 42c5f1d53c61e..4ffc92afdb10d 100644 --- a/shell/gpu/gpu_surface_vulkan_impeller.cc +++ b/shell/gpu/gpu_surface_vulkan_impeller.cc @@ -5,10 +5,11 @@ #include "flutter/shell/gpu/gpu_surface_vulkan_impeller.h" #include "flutter/fml/make_copyable.h" -#include "flutter/impeller/display_list/dl_dispatcher.h" -#include "flutter/impeller/renderer/renderer.h" +#include "impeller/display_list/dl_dispatcher.h" #include "impeller/renderer/backend/vulkan/surface_context_vk.h" +#include "impeller/renderer/renderer.h" #include "impeller/renderer/surface.h" +#include "impeller/typographer/backends/skia/text_render_context_skia.h" namespace flutter { @@ -23,7 +24,8 @@ GPUSurfaceVulkanImpeller::GPUSurfaceVulkanImpeller( return; } - auto aiks_context = std::make_shared(context); + auto aiks_context = std::make_shared( + context, impeller::TextRenderContextSkia::Make()); if (!aiks_context->IsValid()) { return; } From 86f9559d6f60e9bdbaf8293362d69e1cdb9b25e2 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Sun, 20 Aug 2023 14:53:03 -0700 Subject: [PATCH 4/7] GoldenTest tests --- impeller/golden_tests/golden_tests.cc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/impeller/golden_tests/golden_tests.cc b/impeller/golden_tests/golden_tests.cc index 08bbcf04fb9c6..d57bbf751d5d5 100644 --- a/impeller/golden_tests/golden_tests.cc +++ b/impeller/golden_tests/golden_tests.cc @@ -7,6 +7,7 @@ #include #include "flutter/fml/platform/darwin/scoped_nsautorelease_pool.h" +#include "impeller/aiks/aiks_context.h" #include "impeller/aiks/canvas.h" #include "impeller/entity/contents/conical_gradient_contents.h" #include "impeller/geometry/path_builder.h" @@ -56,7 +57,7 @@ class GoldenTests : public ::testing::Test { void SetUp() override { testing::GoldenDigest::Instance()->AddDimension( "gpu_string", - Screenshoter().GetAiksContext().GetContext()->DescribeGpuModel()); + Screenshoter().GetPlayground().GetContext()->DescribeGpuModel()); } private: @@ -79,7 +80,10 @@ TEST_F(GoldenTests, ConicalGradient) { paint.style = Paint::Style::kFill; canvas.DrawRect(Rect(10, 10, 250, 250), paint); Picture picture = canvas.EndRecordingAsPicture(); - auto screenshot = Screenshoter().MakeScreenshot(picture); + + auto aiks_context = + AiksContext(Screenshoter().GetPlayground().GetContext(), nullptr); + auto screenshot = Screenshoter().MakeScreenshot(aiks_context, picture); ASSERT_TRUE(SaveScreenshot(std::move(screenshot))); } } // namespace testing From e2b4863731260036d9ddb64b7c302cb6b3837f88 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Sun, 20 Aug 2023 15:25:41 -0700 Subject: [PATCH 5/7] Fix rasterizer includes --- shell/common/rasterizer.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/shell/common/rasterizer.h b/shell/common/rasterizer.h index ff2a4d4cf99c0..d413edf6ac2cb 100644 --- a/shell/common/rasterizer.h +++ b/shell/common/rasterizer.h @@ -23,12 +23,12 @@ #include "flutter/fml/synchronization/waitable_event.h" #include "flutter/fml/time/time_delta.h" #include "flutter/fml/time/time_point.h" -#include "impeller/typographer/backends/skia/text_render_context_skia.h" #if IMPELLER_SUPPORTS_RENDERING // GN is having trouble understanding how this works in the Fuchsia builds. -#include "flutter/impeller/aiks/aiks_context.h" // nogncheck -#include "flutter/impeller/renderer/context.h" // nogncheck -#endif // IMPELLER_SUPPORTS_RENDERING +#include "impeller/aiks/aiks_context.h" // nogncheck +#include "impeller/renderer/context.h" // nogncheck +#include "impeller/typographer/backends/skia/text_render_context_skia.h" // nogncheck +#endif // IMPELLER_SUPPORTS_RENDERING #include "flutter/lib/ui/snapshot_delegate.h" #include "flutter/shell/common/pipeline.h" #include "flutter/shell/common/snapshot_controller.h" From 68b57bbf2e90eb44c95db2394e99b03feba18c6b Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Sun, 20 Aug 2023 15:37:27 -0700 Subject: [PATCH 6/7] Oopsies --- impeller/entity/entity_playground.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/impeller/entity/entity_playground.cc b/impeller/entity/entity_playground.cc index 5bffd89cf02e3..d93f77dd3cb51 100644 --- a/impeller/entity/entity_playground.cc +++ b/impeller/entity/entity_playground.cc @@ -1,5 +1,4 @@ // Copyright 2013 The Flutter Authors. All rights reserved. -// Copyright 2013 The Flutter Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. From c2685a25c963c7e9757b4e190eebfffd26e2d639 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Mon, 21 Aug 2023 12:30:24 -0700 Subject: [PATCH 7/7] Store text context in AiksPlayground --- impeller/aiks/aiks_context.cc | 2 +- impeller/aiks/aiks_context.h | 2 +- impeller/aiks/aiks_playground.cc | 31 +++++++++---------- impeller/aiks/aiks_playground.h | 14 +++++---- impeller/entity/contents/content_context.cc | 2 +- impeller/entity/contents/content_context.h | 2 +- impeller/entity/entity_playground.cc | 14 ++++++--- impeller/entity/entity_playground.h | 10 ++++-- .../golden_tests/golden_playground_test.h | 16 ++++++---- .../golden_playground_test_mac.cc | 24 ++++++++------ .../golden_playground_test_stub.cc | 16 ++++++---- .../backends/skia/text_render_context_skia.cc | 4 +-- .../backends/skia/text_render_context_skia.h | 2 +- impeller/typographer/lazy_glyph_atlas.cc | 2 +- impeller/typographer/lazy_glyph_atlas.h | 4 +-- 15 files changed, 84 insertions(+), 61 deletions(-) diff --git a/impeller/aiks/aiks_context.cc b/impeller/aiks/aiks_context.cc index b1fdab42b8e33..fc0a3efc45557 100644 --- a/impeller/aiks/aiks_context.cc +++ b/impeller/aiks/aiks_context.cc @@ -10,7 +10,7 @@ namespace impeller { AiksContext::AiksContext(std::shared_ptr context, - std::unique_ptr text_render_context) + std::shared_ptr text_render_context) : context_(std::move(context)) { if (!context_ || !context_->IsValid()) { return; diff --git a/impeller/aiks/aiks_context.h b/impeller/aiks/aiks_context.h index 1c008b4bbc2ed..9564a8a19cbab 100644 --- a/impeller/aiks/aiks_context.h +++ b/impeller/aiks/aiks_context.h @@ -29,7 +29,7 @@ class AiksContext { /// text with Aiks will result in validation /// errors. AiksContext(std::shared_ptr context, - std::unique_ptr text_render_context); + std::shared_ptr text_render_context); ~AiksContext(); diff --git a/impeller/aiks/aiks_playground.cc b/impeller/aiks/aiks_playground.cc index b0e506641a5a1..cb39ce7647182 100644 --- a/impeller/aiks/aiks_playground.cc +++ b/impeller/aiks/aiks_playground.cc @@ -4,41 +4,38 @@ #include "impeller/aiks/aiks_playground.h" -#include "impeller/aiks/aiks_context.h" +#include +#include "impeller/aiks/aiks_context.h" #include "impeller/typographer/backends/skia/text_render_context_skia.h" +#include "impeller/typographer/text_render_context.h" #include "third_party/imgui/imgui.h" namespace impeller { -AiksPlayground::AiksPlayground() = default; +AiksPlayground::AiksPlayground() + : text_render_context_(TextRenderContextSkia::Make()) {} AiksPlayground::~AiksPlayground() = default; -bool AiksPlayground::OpenPlaygroundHere( - const Picture& picture, - std::unique_ptr text_render_context_override) { - auto text_context = text_render_context_override - ? std::move(text_render_context_override) - : TextRenderContextSkia::Make(); +void AiksPlayground::SetTextRenderContext( + std::shared_ptr text_render_context) { + text_render_context_ = std::move(text_render_context); +} + +bool AiksPlayground::OpenPlaygroundHere(const Picture& picture) { return OpenPlaygroundHere( [&picture](AiksContext& renderer, RenderTarget& render_target) -> bool { return renderer.Render(picture, render_target); - }, - std::move(text_context)); + }); } -bool AiksPlayground::OpenPlaygroundHere( - AiksPlaygroundCallback callback, - std::unique_ptr text_render_context_override) { +bool AiksPlayground::OpenPlaygroundHere(AiksPlaygroundCallback callback) { if (!switches_.enable_playground) { return true; } - auto text_context = text_render_context_override - ? std::move(text_render_context_override) - : TextRenderContextSkia::Make(); - AiksContext renderer(GetContext(), std::move(text_context)); + AiksContext renderer(GetContext(), text_render_context_); if (!renderer.IsValid()) { return false; diff --git a/impeller/aiks/aiks_playground.h b/impeller/aiks/aiks_playground.h index 0aa269c73a990..f391ea4d87776 100644 --- a/impeller/aiks/aiks_playground.h +++ b/impeller/aiks/aiks_playground.h @@ -8,6 +8,7 @@ #include "impeller/aiks/aiks_context.h" #include "impeller/aiks/picture.h" #include "impeller/playground/playground_test.h" +#include "impeller/typographer/text_render_context.h" namespace impeller { @@ -20,15 +21,16 @@ class AiksPlayground : public PlaygroundTest { ~AiksPlayground(); - bool OpenPlaygroundHere(const Picture& picture, - std::unique_ptr - text_render_context_override = nullptr); + void SetTextRenderContext( + std::shared_ptr text_render_context); - bool OpenPlaygroundHere(AiksPlaygroundCallback callback, - std::unique_ptr - text_render_context_override = nullptr); + bool OpenPlaygroundHere(const Picture& picture); + + bool OpenPlaygroundHere(AiksPlaygroundCallback callback); private: + std::shared_ptr text_render_context_; + FML_DISALLOW_COPY_AND_ASSIGN(AiksPlayground); }; diff --git a/impeller/entity/contents/content_context.cc b/impeller/entity/contents/content_context.cc index c18c3265d65da..6f76a60ab8eb2 100644 --- a/impeller/entity/contents/content_context.cc +++ b/impeller/entity/contents/content_context.cc @@ -162,7 +162,7 @@ static std::unique_ptr CreateDefaultPipeline( ContentContext::ContentContext( std::shared_ptr context, - std::unique_ptr text_render_context) + std::shared_ptr text_render_context) : context_(std::move(context)), lazy_glyph_atlas_( std::make_shared(std::move(text_render_context))), diff --git a/impeller/entity/contents/content_context.h b/impeller/entity/contents/content_context.h index 457f784c59abf..3c67f85c66fc4 100644 --- a/impeller/entity/contents/content_context.h +++ b/impeller/entity/contents/content_context.h @@ -342,7 +342,7 @@ class ContentContext { public: explicit ContentContext( std::shared_ptr context, - std::unique_ptr text_render_context); + std::shared_ptr text_render_context); ~ContentContext(); diff --git a/impeller/entity/entity_playground.cc b/impeller/entity/entity_playground.cc index d93f77dd3cb51..103dfab02f4f6 100644 --- a/impeller/entity/entity_playground.cc +++ b/impeller/entity/entity_playground.cc @@ -10,16 +10,22 @@ namespace impeller { -EntityPlayground::EntityPlayground() = default; +EntityPlayground::EntityPlayground() + : text_render_context_(TextRenderContextSkia::Make()) {} EntityPlayground::~EntityPlayground() = default; +void EntityPlayground::SetTextRenderContext( + std::shared_ptr text_render_context) { + text_render_context_ = std::move(text_render_context); +} + bool EntityPlayground::OpenPlaygroundHere(EntityPass& entity_pass) { if (!switches_.enable_playground) { return true; } - ContentContext content_context(GetContext(), TextRenderContextSkia::Make()); + ContentContext content_context(GetContext(), text_render_context_); if (!content_context.IsValid()) { return false; } @@ -35,7 +41,7 @@ bool EntityPlayground::OpenPlaygroundHere(Entity entity) { return true; } - ContentContext content_context(GetContext(), TextRenderContextSkia::Make()); + ContentContext content_context(GetContext(), text_render_context_); if (!content_context.IsValid()) { return false; } @@ -50,7 +56,7 @@ bool EntityPlayground::OpenPlaygroundHere(EntityPlaygroundCallback callback) { return true; } - ContentContext content_context(GetContext(), TextRenderContextSkia::Make()); + ContentContext content_context(GetContext(), text_render_context_); if (!content_context.IsValid()) { return false; } diff --git a/impeller/entity/entity_playground.h b/impeller/entity/entity_playground.h index a2f05876497c3..d9d83259b710f 100644 --- a/impeller/entity/entity_playground.h +++ b/impeller/entity/entity_playground.h @@ -4,12 +4,13 @@ #pragma once +#include "impeller/playground/playground_test.h" + #include "flutter/fml/macros.h" #include "impeller/entity/contents/content_context.h" #include "impeller/entity/entity.h" #include "impeller/entity/entity_pass.h" - -#include "impeller/playground/playground_test.h" +#include "impeller/typographer/text_render_context.h" namespace impeller { @@ -22,6 +23,9 @@ class EntityPlayground : public PlaygroundTest { ~EntityPlayground(); + void SetTextRenderContext( + std::shared_ptr text_render_context); + bool OpenPlaygroundHere(Entity entity); bool OpenPlaygroundHere(EntityPass& entity_pass); @@ -29,6 +33,8 @@ class EntityPlayground : public PlaygroundTest { bool OpenPlaygroundHere(EntityPlaygroundCallback callback); private: + std::shared_ptr text_render_context_; + FML_DISALLOW_COPY_AND_ASSIGN(EntityPlayground); }; diff --git a/impeller/golden_tests/golden_playground_test.h b/impeller/golden_tests/golden_playground_test.h index 3a2c8123452a3..34f879c0d225f 100644 --- a/impeller/golden_tests/golden_playground_test.h +++ b/impeller/golden_tests/golden_playground_test.h @@ -11,6 +11,7 @@ #include "flutter/impeller/playground/playground.h" #include "flutter/impeller/renderer/render_target.h" #include "flutter/testing/testing.h" +#include "impeller/typographer/text_render_context.h" #if FML_OS_MACOSX #include "flutter/fml/platform/darwin/scoped_nsautorelease_pool.h" @@ -26,19 +27,20 @@ class GoldenPlaygroundTest GoldenPlaygroundTest(); + ~GoldenPlaygroundTest() override; + void SetUp(); void TearDown(); PlaygroundBackend GetBackend() const; - bool OpenPlaygroundHere(const Picture& picture, - std::unique_ptr - text_render_context_override = nullptr); + void SetTextRenderContext( + std::shared_ptr text_render_context); + + bool OpenPlaygroundHere(const Picture& picture); - bool OpenPlaygroundHere(const AiksPlaygroundCallback& callback, - std::unique_ptr - text_render_context_override = nullptr); + bool OpenPlaygroundHere(const AiksPlaygroundCallback& callback); std::shared_ptr CreateTextureForFixture( const char* fixture_name, @@ -62,6 +64,8 @@ class GoldenPlaygroundTest fml::ScopedNSAutoreleasePool autorelease_pool_; #endif + std::shared_ptr text_render_context_; + struct GoldenPlaygroundTestImpl; // This is only a shared_ptr so it can work with a forward declared type. std::shared_ptr pimpl_; diff --git a/impeller/golden_tests/golden_playground_test_mac.cc b/impeller/golden_tests/golden_playground_test_mac.cc index 1e9f9ff26abce..0b57755dba6e8 100644 --- a/impeller/golden_tests/golden_playground_test_mac.cc +++ b/impeller/golden_tests/golden_playground_test_mac.cc @@ -4,6 +4,7 @@ #include #include +#include #include "flutter/impeller/golden_tests/golden_playground_test.h" @@ -11,6 +12,7 @@ #include "flutter/impeller/golden_tests/golden_digest.h" #include "flutter/impeller/golden_tests/metal_screenshoter.h" #include "impeller/typographer/backends/skia/text_render_context_skia.h" +#include "impeller/typographer/text_render_context.h" namespace impeller { @@ -86,7 +88,15 @@ struct GoldenPlaygroundTest::GoldenPlaygroundTestImpl { }; GoldenPlaygroundTest::GoldenPlaygroundTest() - : pimpl_(new GoldenPlaygroundTest::GoldenPlaygroundTestImpl()) {} + : text_render_context_(TextRenderContextSkia::Make()), + pimpl_(new GoldenPlaygroundTest::GoldenPlaygroundTestImpl()) {} + +GoldenPlaygroundTest::~GoldenPlaygroundTest() = default; + +void GoldenPlaygroundTest::SetTextRenderContext( + std::shared_ptr text_render_context) { + text_render_context_ = std::move(text_render_context); +}; void GoldenPlaygroundTest::TearDown() { ASSERT_FALSE(dlopen("/usr/local/lib/libMoltenVK.dylib", RTLD_NOLOAD)); @@ -124,13 +134,8 @@ PlaygroundBackend GoldenPlaygroundTest::GetBackend() const { return GetParam(); } -bool GoldenPlaygroundTest::OpenPlaygroundHere( - const Picture& picture, - std::unique_ptr text_render_context_override) { - auto text_context = text_render_context_override - ? std::move(text_render_context_override) - : TextRenderContextSkia::Make(); - AiksContext renderer(GetContext(), std::move(text_context)); +bool GoldenPlaygroundTest::OpenPlaygroundHere(const Picture& picture) { + AiksContext renderer(GetContext(), text_render_context_); auto screenshot = pimpl_->screenshoter->MakeScreenshot(renderer, picture, pimpl_->window_size); @@ -138,8 +143,7 @@ bool GoldenPlaygroundTest::OpenPlaygroundHere( } bool GoldenPlaygroundTest::OpenPlaygroundHere( - const AiksPlaygroundCallback& callback, - std::unique_ptr text_render_context_override) { + const AiksPlaygroundCallback& callback) { return false; } diff --git a/impeller/golden_tests/golden_playground_test_stub.cc b/impeller/golden_tests/golden_playground_test_stub.cc index d1a398be08fec..44da5a642e0b2 100644 --- a/impeller/golden_tests/golden_playground_test_stub.cc +++ b/impeller/golden_tests/golden_playground_test_stub.cc @@ -6,7 +6,14 @@ namespace impeller { -GoldenPlaygroundTest::GoldenPlaygroundTest() {} +GoldenPlaygroundTest::GoldenPlaygroundTest() = default; + +GoldenPlaygroundTest::~GoldenPlaygroundTest() = default; + +void GoldenPlaygroundTest::SetTextRenderContext( + std::shared_ptr text_render_context) { + text_render_context_ = std::move(text_render_context); +}; void GoldenPlaygroundTest::TearDown() {} @@ -18,15 +25,12 @@ PlaygroundBackend GoldenPlaygroundTest::GetBackend() const { return GetParam(); } -bool GoldenPlaygroundTest::OpenPlaygroundHere( - const Picture& picture, - std::unique_ptr text_render_context_override) { +bool GoldenPlaygroundTest::OpenPlaygroundHere(const Picture& picture) { return false; } bool GoldenPlaygroundTest::OpenPlaygroundHere( - const AiksPlaygroundCallback& callback, - std::unique_ptr text_render_context_override) { + const AiksPlaygroundCallback& callback) { return false; } diff --git a/impeller/typographer/backends/skia/text_render_context_skia.cc b/impeller/typographer/backends/skia/text_render_context_skia.cc index d34995ce2bbcd..89554e555f8ea 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.cc +++ b/impeller/typographer/backends/skia/text_render_context_skia.cc @@ -28,8 +28,8 @@ using FontGlyphPairRefVector = // https://github.com/flutter/flutter/issues/114563 constexpr auto kPadding = 2; -std::unique_ptr TextRenderContextSkia::Make() { - return std::make_unique(); +std::shared_ptr TextRenderContextSkia::Make() { + return std::make_shared(); } TextRenderContextSkia::TextRenderContextSkia() = default; diff --git a/impeller/typographer/backends/skia/text_render_context_skia.h b/impeller/typographer/backends/skia/text_render_context_skia.h index 3113ea5225772..32d5a083c2dd9 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.h +++ b/impeller/typographer/backends/skia/text_render_context_skia.h @@ -11,7 +11,7 @@ namespace impeller { class TextRenderContextSkia : public TextRenderContext { public: - static std::unique_ptr Make(); + static std::shared_ptr Make(); TextRenderContextSkia(); diff --git a/impeller/typographer/lazy_glyph_atlas.cc b/impeller/typographer/lazy_glyph_atlas.cc index 2b0ac7ab713c3..85f7c5dce0da2 100644 --- a/impeller/typographer/lazy_glyph_atlas.cc +++ b/impeller/typographer/lazy_glyph_atlas.cc @@ -12,7 +12,7 @@ namespace impeller { LazyGlyphAtlas::LazyGlyphAtlas( - std::unique_ptr text_render_context) + std::shared_ptr text_render_context) : text_render_context_(std::move(text_render_context)), alpha_context_(std::make_shared()), color_context_(std::make_shared()) {} diff --git a/impeller/typographer/lazy_glyph_atlas.h b/impeller/typographer/lazy_glyph_atlas.h index 5c2051d11b5bf..f29dbfea6e506 100644 --- a/impeller/typographer/lazy_glyph_atlas.h +++ b/impeller/typographer/lazy_glyph_atlas.h @@ -17,7 +17,7 @@ namespace impeller { class LazyGlyphAtlas { public: explicit LazyGlyphAtlas( - std::unique_ptr text_render_context); + std::shared_ptr text_render_context); ~LazyGlyphAtlas(); @@ -30,7 +30,7 @@ class LazyGlyphAtlas { GlyphAtlas::Type type) const; private: - std::unique_ptr text_render_context_; + std::shared_ptr text_render_context_; FontGlyphPair::Set alpha_set_; FontGlyphPair::Set color_set_;