Skip to content

Commit f2ea655

Browse files
Jonah Williamszanderso
authored andcommitted
[Impeller] disable entity culling by default. (flutter#48717)
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)
1 parent bc2e856 commit f2ea655

6 files changed

Lines changed: 24 additions & 7 deletions

File tree

impeller/entity/contents/clip_contents.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ Contents::StencilCoverage ClipContents::GetStencilCoverage(
6666

6767
bool ClipContents::ShouldRender(
6868
const Entity& entity,
69-
const std::optional<Rect>& stencil_coverage) const {
69+
const std::optional<Rect> stencil_coverage) const {
7070
return true;
7171
}
7272

@@ -163,7 +163,7 @@ Contents::StencilCoverage ClipRestoreContents::GetStencilCoverage(
163163

164164
bool ClipRestoreContents::ShouldRender(
165165
const Entity& entity,
166-
const std::optional<Rect>& stencil_coverage) const {
166+
const std::optional<Rect> stencil_coverage) const {
167167
return true;
168168
}
169169

impeller/entity/contents/clip_contents.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ class ClipContents final : public Contents {
3535

3636
// |Contents|
3737
bool ShouldRender(const Entity& entity,
38-
const std::optional<Rect>& stencil_coverage) const override;
38+
const std::optional<Rect> stencil_coverage) const override;
3939

4040
// |Contents|
4141
bool Render(const ContentContext& renderer,
@@ -76,7 +76,7 @@ class ClipRestoreContents final : public Contents {
7676

7777
// |Contents|
7878
bool ShouldRender(const Entity& entity,
79-
const std::optional<Rect>& stencil_coverage) const override;
79+
const std::optional<Rect> stencil_coverage) const override;
8080

8181
// |Contents|
8282
bool Render(const ContentContext& renderer,

impeller/entity/contents/contents.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,11 +133,10 @@ bool Contents::ApplyColorFilter(
133133
}
134134

135135
bool Contents::ShouldRender(const Entity& entity,
136-
const std::optional<Rect>& stencil_coverage) const {
136+
const std::optional<Rect> stencil_coverage) const {
137137
if (!stencil_coverage.has_value()) {
138138
return false;
139139
}
140-
141140
auto coverage = GetCoverage(entity);
142141
if (!coverage.has_value()) {
143142
return false;

impeller/entity/contents/contents.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ class Contents {
113113
const std::string& label = "Snapshot") const;
114114

115115
virtual bool ShouldRender(const Entity& entity,
116-
const std::optional<Rect>& stencil_coverage) const;
116+
const std::optional<Rect> stencil_coverage) const;
117117

118118
//----------------------------------------------------------------------------
119119
/// @brief Return the color source's intrinsic size, if available.

impeller/entity/entity.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,11 @@ Contents::StencilCoverage Entity::GetStencilCoverage(
7171
}
7272

7373
bool Entity::ShouldRender(const std::optional<Rect>& stencil_coverage) const {
74+
#ifdef IMPELLER_CONTENT_CULLING
7475
return contents_->ShouldRender(*this, stencil_coverage);
76+
#else
77+
return true;
78+
#endif // IMPELLER_CONTENT_CULLING
7579
}
7680

7781
void Entity::SetContents(std::shared_ptr<Contents> contents) {

impeller/entity/entity_unittests.cc

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1607,6 +1607,20 @@ TEST_P(EntityTest, SolidFillShouldRenderIsCorrect) {
16071607
}
16081608
}
16091609

1610+
TEST_P(EntityTest, DoesNotCullEntitiesByDefault) {
1611+
auto fill = std::make_shared<SolidColorContents>();
1612+
fill->SetColor(Color::CornflowerBlue());
1613+
fill->SetGeometry(
1614+
Geometry::MakeRect(Rect::MakeLTRB(-1000, -1000, -900, -900)));
1615+
1616+
Entity entity;
1617+
entity.SetContents(fill);
1618+
1619+
// Even though the entity is offscreen, this should still render because we do
1620+
// not compute the coverage intersection by default.
1621+
EXPECT_TRUE(entity.ShouldRender(Rect::MakeLTRB(0, 0, 100, 100)));
1622+
}
1623+
16101624
TEST_P(EntityTest, ClipContentsShouldRenderIsCorrect) {
16111625
// For clip ops, `ShouldRender` should always return true.
16121626

0 commit comments

Comments
 (0)