From d5a8db658bd60894e35e1f535193b279bc8a34c0 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Tue, 19 Jul 2022 09:27:45 -0700 Subject: [PATCH] [Impeller] Cover ShouldRender in tests. (flutter/engine#34749) --- .../impeller/entity/contents/clip_contents.cc | 4 +-- .../impeller/entity/contents/clip_contents.h | 4 +-- .../impeller/entity/contents/contents.cc | 5 ++- .../impeller/entity/contents/contents.h | 3 +- engine/src/flutter/impeller/entity/entity.cc | 4 +-- engine/src/flutter/impeller/entity/entity.h | 2 +- .../flutter/impeller/entity/entity_pass.cc | 2 +- .../impeller/entity/entity_unittests.cc | 36 +++++++++++++++++++ 8 files changed, 48 insertions(+), 12 deletions(-) diff --git a/engine/src/flutter/impeller/entity/contents/clip_contents.cc b/engine/src/flutter/impeller/entity/contents/clip_contents.cc index b592052b2ed..c44d7b77430 100644 --- a/engine/src/flutter/impeller/entity/contents/clip_contents.cc +++ b/engine/src/flutter/impeller/entity/contents/clip_contents.cc @@ -37,7 +37,7 @@ std::optional ClipContents::GetCoverage(const Entity& entity) const { }; bool ClipContents::ShouldRender(const Entity& entity, - const RenderPass& pass) const { + const ISize& target_size) const { return true; } @@ -120,7 +120,7 @@ std::optional ClipRestoreContents::GetCoverage( }; bool ClipRestoreContents::ShouldRender(const Entity& entity, - const RenderPass& pass) const { + const ISize& target_size) const { return true; } diff --git a/engine/src/flutter/impeller/entity/contents/clip_contents.h b/engine/src/flutter/impeller/entity/contents/clip_contents.h index 9276cbcc0e3..35c2a9a27c5 100644 --- a/engine/src/flutter/impeller/entity/contents/clip_contents.h +++ b/engine/src/flutter/impeller/entity/contents/clip_contents.h @@ -29,7 +29,7 @@ class ClipContents final : public Contents { // |Contents| bool ShouldRender(const Entity& entity, - const RenderPass& pass) const override; + const ISize& target_size) const override; // |Contents| bool Render(const ContentContext& renderer, @@ -54,7 +54,7 @@ class ClipRestoreContents final : public Contents { // |Contents| bool ShouldRender(const Entity& entity, - const RenderPass& pass) const override; + const ISize& target_size) const override; // |Contents| bool Render(const ContentContext& renderer, diff --git a/engine/src/flutter/impeller/entity/contents/contents.cc b/engine/src/flutter/impeller/entity/contents/contents.cc index 3a3e885cc4d..c0f4831f1e4 100644 --- a/engine/src/flutter/impeller/entity/contents/contents.cc +++ b/engine/src/flutter/impeller/entity/contents/contents.cc @@ -58,11 +58,10 @@ std::optional Contents::RenderToSnapshot( } bool Contents::ShouldRender(const Entity& entity, - const RenderPass& pass) const { + const ISize& target_size) const { auto coverage = GetCoverage(entity); return coverage.has_value() && - Rect::MakeSize(Size(pass.GetRenderTargetSize())) - .IntersectsWithRect(coverage.value()); + Rect::MakeSize(target_size).IntersectsWithRect(coverage.value()); } } // namespace impeller diff --git a/engine/src/flutter/impeller/entity/contents/contents.h b/engine/src/flutter/impeller/entity/contents/contents.h index ef2946a6a5d..f0c166858e5 100644 --- a/engine/src/flutter/impeller/entity/contents/contents.h +++ b/engine/src/flutter/impeller/entity/contents/contents.h @@ -47,7 +47,8 @@ class Contents { const ContentContext& renderer, const Entity& entity) const; - virtual bool ShouldRender(const Entity& entity, const RenderPass& pass) const; + virtual bool ShouldRender(const Entity& entity, + const ISize& target_size) const; protected: diff --git a/engine/src/flutter/impeller/entity/entity.cc b/engine/src/flutter/impeller/entity/entity.cc index 830d93449d8..e5695ef8017 100644 --- a/engine/src/flutter/impeller/entity/entity.cc +++ b/engine/src/flutter/impeller/entity/entity.cc @@ -41,8 +41,8 @@ std::optional Entity::GetCoverage() const { return contents_->GetCoverage(*this); } -bool Entity::ShouldRender(const RenderPass& pass) const { - return contents_->ShouldRender(*this, pass); +bool Entity::ShouldRender(const ISize& target_size) const { + return contents_->ShouldRender(*this, target_size); } void Entity::SetContents(std::shared_ptr contents) { diff --git a/engine/src/flutter/impeller/entity/entity.h b/engine/src/flutter/impeller/entity/entity.h index 7f84b993e7d..7c83f0f2da7 100644 --- a/engine/src/flutter/impeller/entity/entity.h +++ b/engine/src/flutter/impeller/entity/entity.h @@ -81,7 +81,7 @@ class Entity { std::optional GetCoverage() const; - bool ShouldRender(const RenderPass& pass) const; + bool ShouldRender(const ISize& target_size) const; void SetContents(std::shared_ptr contents); diff --git a/engine/src/flutter/impeller/entity/entity_pass.cc b/engine/src/flutter/impeller/entity/entity_pass.cc index d08cf2b8651..5cae1fc91b2 100644 --- a/engine/src/flutter/impeller/entity/entity_pass.cc +++ b/engine/src/flutter/impeller/entity/entity_pass.cc @@ -363,7 +363,7 @@ bool EntityPass::OnRender(ContentContext& renderer, stencil_depth_floor); auto pass = pass_context.GetRenderPass(pass_depth); - if (!element_entity.ShouldRender(*pass)) { + if (!element_entity.ShouldRender(pass->GetRenderTargetSize())) { return true; // Nothing to render. } diff --git a/engine/src/flutter/impeller/entity/entity_unittests.cc b/engine/src/flutter/impeller/entity/entity_unittests.cc index 00fcb190dc7..87c4199c7ed 100644 --- a/engine/src/flutter/impeller/entity/entity_unittests.cc +++ b/engine/src/flutter/impeller/entity/entity_unittests.cc @@ -6,6 +6,7 @@ #include #include "flutter/testing/testing.h" +#include "impeller/entity/contents/clip_contents.h" #include "impeller/entity/contents/filters/blend_filter_contents.h" #include "impeller/entity/contents/filters/filter_contents.h" #include "impeller/entity/contents/filters/inputs/filter_input.h" @@ -1114,5 +1115,40 @@ TEST_P(EntityTest, SolidFillCoverageIsCorrect) { } } +TEST_P(EntityTest, SolidFillShouldRenderIsCorrect) { + // No path. + { + auto fill = std::make_shared(); + fill->SetColor(Color::CornflowerBlue()); + ASSERT_FALSE(fill->ShouldRender(Entity{}, {100, 100})); + } + + // With path. + { + auto fill = std::make_shared(); + fill->SetColor(Color::CornflowerBlue()); + fill->SetPath( + PathBuilder{}.AddRect(Rect::MakeLTRB(0, 0, 100, 100)).TakePath()); + ASSERT_TRUE(fill->ShouldRender(Entity{}, {100, 100})); + } +} + +TEST_P(EntityTest, ClipContentsShouldRenderIsCorrect) { + // Clip. + { + auto clip = std::make_shared(); + ASSERT_TRUE(clip->ShouldRender(Entity{}, {100, 100})); + clip->SetPath( + PathBuilder{}.AddRect(Rect::MakeLTRB(0, 0, 100, 100)).TakePath()); + ASSERT_TRUE(clip->ShouldRender(Entity{}, {100, 100})); + } + + // Clip restore. + { + auto restore = std::make_shared(); + ASSERT_TRUE(restore->ShouldRender(Entity{}, {100, 100})); + } +} + } // namespace testing } // namespace impeller