Skip to content

Commit

Permalink
[Impeller] disable entity culling by default. (#48717)
Browse files Browse the repository at this point in the history
From local testing across both flutter galleries, wonderous, some other test apps, the only time this code successfully culls is during the stretch overscroll (and we cull 1 or so entries). The cost of this culling is approximately 20% of entity rendering time, and about 5% of overall raster time.

![image](https://github.com/flutter/engine/assets/8975114/ef85629c-48a8-457f-8385-0c8136404571)
  • Loading branch information
jonahwilliams committed Dec 6, 2023
1 parent 41e2935 commit 2dbc5d2
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 9 deletions.
7 changes: 3 additions & 4 deletions impeller/entity/contents/clip_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,8 @@ Contents::ClipCoverage ClipContents::GetClipCoverage(
FML_UNREACHABLE();
}

bool ClipContents::ShouldRender(
const Entity& entity,
const std::optional<Rect>& clip_coverage) const {
bool ClipContents::ShouldRender(const Entity& entity,
const std::optional<Rect> clip_coverage) const {
return true;
}

Expand Down Expand Up @@ -161,7 +160,7 @@ Contents::ClipCoverage ClipRestoreContents::GetClipCoverage(

bool ClipRestoreContents::ShouldRender(
const Entity& entity,
const std::optional<Rect>& clip_coverage) const {
const std::optional<Rect> clip_coverage) const {
return true;
}

Expand Down
4 changes: 2 additions & 2 deletions impeller/entity/contents/clip_contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class ClipContents final : public Contents {

// |Contents|
bool ShouldRender(const Entity& entity,
const std::optional<Rect>& clip_coverage) const override;
const std::optional<Rect> clip_coverage) const override;

// |Contents|
bool Render(const ContentContext& renderer,
Expand Down Expand Up @@ -78,7 +78,7 @@ class ClipRestoreContents final : public Contents {

// |Contents|
bool ShouldRender(const Entity& entity,
const std::optional<Rect>& clip_coverage) const override;
const std::optional<Rect> clip_coverage) const override;

// |Contents|
bool Render(const ContentContext& renderer,
Expand Down
3 changes: 1 addition & 2 deletions impeller/entity/contents/contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,10 @@ bool Contents::ApplyColorFilter(
}

bool Contents::ShouldRender(const Entity& entity,
const std::optional<Rect>& clip_coverage) const {
const std::optional<Rect> clip_coverage) const {
if (!clip_coverage.has_value()) {
return false;
}

auto coverage = GetCoverage(entity);
if (!coverage.has_value()) {
return false;
Expand Down
2 changes: 1 addition & 1 deletion impeller/entity/contents/contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ class Contents {
const std::string& label = "Snapshot") const;

virtual bool ShouldRender(const Entity& entity,
const std::optional<Rect>& clip_coverage) const;
const std::optional<Rect> clip_coverage) const;

//----------------------------------------------------------------------------
/// @brief Return the color source's intrinsic size, if available.
Expand Down
4 changes: 4 additions & 0 deletions impeller/entity/entity.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,11 @@ Contents::ClipCoverage Entity::GetClipCoverage(
}

bool Entity::ShouldRender(const std::optional<Rect>& clip_coverage) const {
#ifdef IMPELLER_CONTENT_CULLING
return contents_->ShouldRender(*this, clip_coverage);
#else
return true;
#endif // IMPELLER_CONTENT_CULLING
}

void Entity::SetContents(std::shared_ptr<Contents> contents) {
Expand Down
14 changes: 14 additions & 0 deletions impeller/entity/entity_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1605,6 +1605,20 @@ TEST_P(EntityTest, SolidFillShouldRenderIsCorrect) {
}
}

TEST_P(EntityTest, DoesNotCullEntitiesByDefault) {
auto fill = std::make_shared<SolidColorContents>();
fill->SetColor(Color::CornflowerBlue());
fill->SetGeometry(
Geometry::MakeRect(Rect::MakeLTRB(-1000, -1000, -900, -900)));

Entity entity;
entity.SetContents(fill);

// Even though the entity is offscreen, this should still render because we do
// not compute the coverage intersection by default.
EXPECT_TRUE(entity.ShouldRender(Rect::MakeLTRB(0, 0, 100, 100)));
}

TEST_P(EntityTest, ClipContentsShouldRenderIsCorrect) {
// For clip ops, `ShouldRender` should always return true.

Expand Down

0 comments on commit 2dbc5d2

Please sign in to comment.