Skip to content

Commit cb45cf7

Browse files
author
Jonah Williams
authored
[Impeller] Dont blow away coverage hint on advanced blends. (flutter#46219)
Advnaced blends are given a fairly accurate coverage hint based on the entity size. We were blowing this value away when applying the stencil coverage, resulting in oversized offscreen textures. This should improve the android advanced blend benchmark
1 parent 6bf8067 commit cb45cf7

File tree

4 files changed

+104
-5
lines changed

4 files changed

+104
-5
lines changed

impeller/entity/contents/content_context.cc

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,16 +162,19 @@ static std::unique_ptr<PipelineT> CreateDefaultPipeline(
162162

163163
ContentContext::ContentContext(
164164
std::shared_ptr<Context> context,
165-
std::shared_ptr<TypographerContext> typographer_context)
165+
std::shared_ptr<TypographerContext> typographer_context,
166+
std::shared_ptr<RenderTargetAllocator> render_target_allocator)
166167
: context_(std::move(context)),
167168
lazy_glyph_atlas_(
168169
std::make_shared<LazyGlyphAtlas>(std::move(typographer_context))),
169170
tessellator_(std::make_shared<Tessellator>()),
170171
#if IMPELLER_ENABLE_3D
171172
scene_context_(std::make_shared<scene::SceneContext>(context_)),
172173
#endif // IMPELLER_ENABLE_3D
173-
render_target_cache_(std::make_shared<RenderTargetCache>(
174-
context_->GetResourceAllocator())) {
174+
render_target_cache_(render_target_allocator == nullptr
175+
? std::make_shared<RenderTargetCache>(
176+
context_->GetResourceAllocator())
177+
: std::move(render_target_allocator)) {
175178
if (!context_ || !context_->IsValid()) {
176179
return;
177180
}

impeller/entity/contents/content_context.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,8 @@ class ContentContext {
343343
public:
344344
explicit ContentContext(
345345
std::shared_ptr<Context> context,
346-
std::shared_ptr<TypographerContext> typographer_context);
346+
std::shared_ptr<TypographerContext> typographer_context,
347+
std::shared_ptr<RenderTargetAllocator> render_target_allocator = nullptr);
347348

348349
~ContentContext();
349350

impeller/entity/entity_pass.cc

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -783,7 +783,16 @@ bool EntityPass::OnRender(
783783
// rendered output will actually be used, and so we set this to the current
784784
// stencil coverage (which is the max clip bounds). The contents may
785785
// optionally use this hint to avoid unnecessary rendering work.
786-
element_entity.GetContents()->SetCoverageHint(current_stencil_coverage);
786+
if (element_entity.GetContents()->GetCoverageHint().has_value()) {
787+
// If the element already has a coverage hint (because its an advanced
788+
// blend), then we need to intersect the stencil coverage hint with the
789+
// existing coverage hint.
790+
element_entity.GetContents()->SetCoverageHint(
791+
current_stencil_coverage->Intersection(
792+
element_entity.GetContents()->GetCoverageHint().value()));
793+
} else {
794+
element_entity.GetContents()->SetCoverageHint(current_stencil_coverage);
795+
}
787796

788797
switch (stencil_coverage.type) {
789798
case Contents::StencilCoverage::Type::kNoChange:

impeller/entity/entity_unittests.cc

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,14 @@
77
#include <memory>
88
#include <optional>
99
#include <unordered_map>
10+
#include <utility>
1011
#include <vector>
1112

1213
#include "flutter/testing/testing.h"
1314
#include "fml/logging.h"
1415
#include "fml/time/time_point.h"
1516
#include "gtest/gtest.h"
17+
#include "impeller/core/texture_descriptor.h"
1618
#include "impeller/entity/contents/atlas_contents.h"
1719
#include "impeller/entity/contents/clip_contents.h"
1820
#include "impeller/entity/contents/conical_gradient_contents.h"
@@ -42,6 +44,7 @@
4244
#include "impeller/geometry/geometry_asserts.h"
4345
#include "impeller/geometry/path_builder.h"
4446
#include "impeller/geometry/sigma.h"
47+
#include "impeller/geometry/vector.h"
4548
#include "impeller/playground/playground.h"
4649
#include "impeller/playground/widgets.h"
4750
#include "impeller/renderer/command.h"
@@ -2441,6 +2444,89 @@ TEST_P(EntityTest, TextContentsCeilsGlyphScaleToDecimal) {
24412444
ASSERT_EQ(TextFrame::RoundScaledFontSize(0.0f, 12), 0.0f);
24422445
}
24432446

2447+
class TestRenderTargetAllocator : public RenderTargetAllocator {
2448+
public:
2449+
explicit TestRenderTargetAllocator(std::shared_ptr<Allocator> allocator)
2450+
: RenderTargetAllocator(std::move(allocator)) {}
2451+
2452+
~TestRenderTargetAllocator() = default;
2453+
2454+
std::shared_ptr<Texture> CreateTexture(
2455+
const TextureDescriptor& desc) override {
2456+
allocated_.push_back(desc);
2457+
return RenderTargetAllocator::CreateTexture(desc);
2458+
}
2459+
2460+
void Start() override { RenderTargetAllocator::Start(); }
2461+
2462+
void End() override { RenderTargetAllocator::End(); }
2463+
2464+
std::vector<TextureDescriptor> GetDescriptors() const { return allocated_; }
2465+
2466+
private:
2467+
std::vector<TextureDescriptor> allocated_;
2468+
};
2469+
2470+
TEST_P(EntityTest, AdvancedBlendCoverageHintIsNotResetByEntityPass) {
2471+
if (GetContext()->GetCapabilities()->SupportsFramebufferFetch()) {
2472+
GTEST_SKIP() << "Backends that support framebuffer fetch dont use coverage "
2473+
"for advanced blends.";
2474+
}
2475+
2476+
auto contents = std::make_shared<SolidColorContents>();
2477+
contents->SetGeometry(Geometry::MakeRect({100, 100, 100, 100}));
2478+
contents->SetColor(Color::Red());
2479+
2480+
Entity entity;
2481+
entity.SetTransformation(Matrix::MakeScale(Vector3(2, 2, 1)));
2482+
entity.SetBlendMode(BlendMode::kColorBurn);
2483+
entity.SetContents(contents);
2484+
2485+
auto coverage = entity.GetCoverage();
2486+
EXPECT_TRUE(coverage.has_value());
2487+
2488+
auto pass = std::make_unique<EntityPass>();
2489+
auto test_allocator = std::make_shared<TestRenderTargetAllocator>(
2490+
GetContext()->GetResourceAllocator());
2491+
auto stencil_config = RenderTarget::AttachmentConfig{
2492+
.storage_mode = StorageMode::kDevicePrivate,
2493+
.load_action = LoadAction::kClear,
2494+
.store_action = StoreAction::kDontCare,
2495+
.clear_color = Color::BlackTransparent()};
2496+
auto rt = RenderTarget::CreateOffscreen(
2497+
*GetContext(), *test_allocator, ISize::MakeWH(1000, 1000), "Offscreen",
2498+
RenderTarget::kDefaultColorAttachmentConfig, stencil_config);
2499+
auto content_context = ContentContext(
2500+
GetContext(), TypographerContextSkia::Make(), test_allocator);
2501+
pass->AddEntity(entity);
2502+
2503+
EXPECT_TRUE(pass->Render(content_context, rt));
2504+
2505+
if (test_allocator->GetDescriptors().size() == 6u) {
2506+
EXPECT_EQ(test_allocator->GetDescriptors()[0].size, ISize(1000, 1000));
2507+
EXPECT_EQ(test_allocator->GetDescriptors()[1].size, ISize(1000, 1000));
2508+
2509+
EXPECT_EQ(test_allocator->GetDescriptors()[2].size, ISize(200, 200));
2510+
EXPECT_EQ(test_allocator->GetDescriptors()[3].size, ISize(200, 200));
2511+
EXPECT_EQ(test_allocator->GetDescriptors()[4].size, ISize(200, 200));
2512+
EXPECT_EQ(test_allocator->GetDescriptors()[5].size, ISize(200, 200));
2513+
} else if (test_allocator->GetDescriptors().size() == 9u) {
2514+
// Onscreen render target.
2515+
EXPECT_EQ(test_allocator->GetDescriptors()[0].size, ISize(1000, 1000));
2516+
EXPECT_EQ(test_allocator->GetDescriptors()[1].size, ISize(1000, 1000));
2517+
EXPECT_EQ(test_allocator->GetDescriptors()[2].size, ISize(1000, 1000));
2518+
EXPECT_EQ(test_allocator->GetDescriptors()[3].size, ISize(1000, 1000));
2519+
EXPECT_EQ(test_allocator->GetDescriptors()[4].size, ISize(1000, 1000));
2520+
2521+
EXPECT_EQ(test_allocator->GetDescriptors()[5].size, ISize(200, 200));
2522+
EXPECT_EQ(test_allocator->GetDescriptors()[5].size, ISize(200, 200));
2523+
EXPECT_EQ(test_allocator->GetDescriptors()[6].size, ISize(200, 200));
2524+
EXPECT_EQ(test_allocator->GetDescriptors()[7].size, ISize(200, 200));
2525+
} else {
2526+
EXPECT_TRUE(false);
2527+
}
2528+
}
2529+
24442530
} // namespace testing
24452531
} // namespace impeller
24462532

0 commit comments

Comments
 (0)