diff --git a/engine/src/flutter/impeller/aiks/aiks_unittests.cc b/engine/src/flutter/impeller/aiks/aiks_unittests.cc index 7c545009654..c6aa437ec64 100644 --- a/engine/src/flutter/impeller/aiks/aiks_unittests.cc +++ b/engine/src/flutter/impeller/aiks/aiks_unittests.cc @@ -649,5 +649,35 @@ TEST_P(AiksTest, SaveLayerDrawsBehindSubsequentEntities) { ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); } +TEST_P(AiksTest, SiblingSaveLayerBoundsAreRespected) { + Canvas canvas; + Paint paint; + Rect rect(0, 0, 1000, 1000); + + // Black, green, and red squares offset by [10, 10]. + { + canvas.SaveLayer({}, Rect::MakeXYWH(25, 25, 25, 25)); + paint.color = Color::Black(); + canvas.DrawRect(rect, paint); + canvas.Restore(); + } + + { + canvas.SaveLayer({}, Rect::MakeXYWH(35, 35, 25, 25)); + paint.color = Color::Green(); + canvas.DrawRect(rect, paint); + canvas.Restore(); + } + + { + canvas.SaveLayer({}, Rect::MakeXYWH(45, 45, 25, 25)); + paint.color = Color::Red(); + canvas.DrawRect(rect, paint); + canvas.Restore(); + } + + ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); +} + } // namespace testing } // namespace impeller diff --git a/engine/src/flutter/impeller/aiks/canvas.cc b/engine/src/flutter/impeller/aiks/canvas.cc index a6f881553ea..3022887f70a 100644 --- a/engine/src/flutter/impeller/aiks/canvas.cc +++ b/engine/src/flutter/impeller/aiks/canvas.cc @@ -138,12 +138,12 @@ void Canvas::DrawCircle(Point center, Scalar radius, Paint paint) { } void Canvas::SaveLayer(Paint paint, std::optional bounds) { - GetCurrentPass().SetDelegate( - std::make_unique(paint, bounds)); - Save(true); GetCurrentPass().SetBlendMode(paint.blend_mode); + GetCurrentPass().SetDelegate( + std::make_unique(paint, bounds)); + if (bounds.has_value()) { // Render target switches due to a save layer can be elided. In such cases // where passes are collapsed into their parent, the clipping effect to diff --git a/engine/src/flutter/impeller/entity/entity_pass.cc b/engine/src/flutter/impeller/entity/entity_pass.cc index eec666bc84d..07ffc248f11 100644 --- a/engine/src/flutter/impeller/entity/entity_pass.cc +++ b/engine/src/flutter/impeller/entity/entity_pass.cc @@ -59,7 +59,7 @@ std::optional EntityPass::GetElementsCoverage() const { coverage = entity->GetCoverage(); } else if (auto subpass = std::get_if>(&element)) { - coverage = subpass->get()->GetElementsCoverage(); + coverage = GetSubpassCoverage(*subpass->get()); } else { FML_UNREACHABLE(); } @@ -86,7 +86,7 @@ std::optional EntityPass::GetSubpassCoverage( // The delegates don't have an opinion on what the entity coverage has to be. // Just use that as-is. - auto delegate_coverage = delegate_->GetCoverageRect(); + auto delegate_coverage = subpass.delegate_->GetCoverageRect(); if (!delegate_coverage.has_value()) { return entities_coverage; } @@ -146,11 +146,11 @@ bool EntityPass::Render(ContentContext& renderer, std::get_if>(&element)) { auto subpass = subpass_ptr->get(); - if (delegate_->CanElide()) { + if (subpass->delegate_->CanElide()) { continue; } - if (delegate_->CanCollapseIntoParentPass()) { + if (subpass->delegate_->CanCollapseIntoParentPass()) { // Directly render into the parent pass and move on. if (!subpass->Render(renderer, parent_pass, position)) { return false; @@ -181,7 +181,7 @@ bool EntityPass::Render(ContentContext& renderer, } auto offscreen_texture_contents = - delegate_->CreateContentsForSubpassTarget(subpass_texture); + subpass->delegate_->CreateContentsForSubpassTarget(subpass_texture); if (!offscreen_texture_contents) { // This is an error because the subpass delegate said the pass couldn't diff --git a/engine/src/flutter/impeller/entity/entity_pass.h b/engine/src/flutter/impeller/entity/entity_pass.h index 545eaf536fa..fc665c3b3e5 100644 --- a/engine/src/flutter/impeller/entity/entity_pass.h +++ b/engine/src/flutter/impeller/entity/entity_pass.h @@ -55,6 +55,10 @@ class EntityPass { void SetBlendMode(Entity::BlendMode blend_mode); + std::optional GetSubpassCoverage(const EntityPass& subpass) const; + + std::optional GetElementsCoverage() const; + private: std::vector elements_; @@ -67,10 +71,6 @@ class EntityPass { std::shared_ptr lazy_glyph_atlas_ = std::make_shared(); - std::optional GetSubpassCoverage(const EntityPass& subpass) const; - - std::optional GetElementsCoverage() const; - FML_DISALLOW_COPY_AND_ASSIGN(EntityPass); }; diff --git a/engine/src/flutter/impeller/entity/entity_unittests.cc b/engine/src/flutter/impeller/entity/entity_unittests.cc index 50998e6dcc2..90c6c0b292e 100644 --- a/engine/src/flutter/impeller/entity/entity_unittests.cc +++ b/engine/src/flutter/impeller/entity/entity_unittests.cc @@ -11,6 +11,8 @@ #include "impeller/entity/contents/solid_color_contents.h" #include "impeller/entity/contents/solid_stroke_contents.h" #include "impeller/entity/entity.h" +#include "impeller/entity/entity_pass.h" +#include "impeller/entity/entity_pass_delegate.h" #include "impeller/entity/entity_playground.h" #include "impeller/geometry/geometry_unittests.h" #include "impeller/geometry/path_builder.h" @@ -32,6 +34,74 @@ TEST_P(EntityTest, CanCreateEntity) { ASSERT_TRUE(entity.GetTransformation().IsIdentity()); } +class TestPassDelegate final : public EntityPassDelegate { + public: + TestPassDelegate(std::optional coverage) : coverage_(coverage) {} + + // |EntityPassDelegate| + ~TestPassDelegate() override = default; + + // |EntityPassDelegate| + std::optional GetCoverageRect() override { return coverage_; } + + // |EntityPassDelgate| + bool CanElide() override { return false; } + + // |EntityPassDelgate| + bool CanCollapseIntoParentPass() override { return false; } + + // |EntityPassDelgate| + std::shared_ptr CreateContentsForSubpassTarget( + std::shared_ptr target) override { + return nullptr; + } + + private: + const std::optional coverage_; +}; + +TEST_P(EntityTest, EntityPassSubpassCoverageIsCorrect) { + EntityPass pass; + + auto subpass0 = std::make_unique(); + { + Entity entity; + entity.SetContents(SolidColorContents::Make( + PathBuilder{}.AddRect(Rect::MakeLTRB(0, 0, 100, 100)).TakePath(), + Color::Red())); + subpass0->AddEntity(entity); + subpass0->SetDelegate( + std::make_unique(Rect::MakeLTRB(50, 50, 150, 150))); + } + + auto subpass1 = std::make_unique(); + { + Entity entity; + entity.SetContents(SolidColorContents::Make( + PathBuilder{}.AddRect(Rect::MakeLTRB(500, 500, 1000, 1000)).TakePath(), + Color::Red())); + subpass1->AddEntity(entity); + subpass1->SetDelegate( + std::make_unique(Rect::MakeLTRB(800, 800, 900, 900))); + } + + auto subpass0_coverage = pass.GetSubpassCoverage(*subpass0.get()); + ASSERT_TRUE(subpass0_coverage.has_value()); + ASSERT_RECT_NEAR(subpass0_coverage.value(), Rect::MakeLTRB(50, 50, 100, 100)); + + auto subpass1_coverage = pass.GetSubpassCoverage(*subpass1.get()); + ASSERT_TRUE(subpass1_coverage.has_value()); + ASSERT_RECT_NEAR(subpass1_coverage.value(), + Rect::MakeLTRB(800, 800, 900, 900)); + + pass.AddSubpass(std::move(subpass0)); + pass.AddSubpass(std::move(subpass1)); + + auto coverage = pass.GetElementsCoverage(); + ASSERT_TRUE(coverage.has_value()); + ASSERT_RECT_NEAR(coverage.value(), Rect::MakeLTRB(50, 50, 900, 900)); +} + TEST_P(EntityTest, CanDrawRect) { Entity entity; entity.SetContents(SolidColorContents::Make(