Skip to content

Commit 4d50315

Browse files
bderodnfield
bdero
authored and
dnfield
committed
Fix subpass ordering (flutter#143)
1 parent 3b1dfc4 commit 4d50315

File tree

3 files changed

+188
-126
lines changed

3 files changed

+188
-126
lines changed

‎impeller/aiks/aiks_unittests.cc‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -620,7 +620,31 @@ TEST_P(AiksTest, DrawRectStrokesRenderCorrectly) {
620620

621621
canvas.Translate({100, 100});
622622
canvas.DrawPath(PathBuilder{}.AddRect(Rect::MakeSize({100, 100})).TakePath(),
623-
paint);
623+
{paint});
624+
625+
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
626+
}
627+
628+
TEST_P(AiksTest, SaveLayerDrawsBehindSubsequentEntities) {
629+
// Compare with https://fiddle.skia.org/c/9e03de8567ffb49e7e83f53b64bcf636
630+
Canvas canvas;
631+
Paint paint;
632+
633+
paint.color = Color::Black();
634+
Rect rect(25, 25, 25, 25);
635+
canvas.DrawRect(rect, paint);
636+
637+
canvas.Translate({10, 10});
638+
canvas.SaveLayer({});
639+
640+
paint.color = Color::Green();
641+
canvas.DrawRect(rect, paint);
642+
643+
canvas.Restore();
644+
645+
canvas.Translate({10, 10});
646+
paint.color = Color::Red();
647+
canvas.DrawRect(rect, paint);
624648

625649
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
626650
}

‎impeller/entity/entity_pass.cc‎

Lines changed: 158 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
// found in the LICENSE file.
44

55
#include "impeller/entity/entity_pass.h"
6+
#include <variant>
67

8+
#include "flutter/fml/logging.h"
79
#include "flutter/fml/trace_event.h"
810
#include "impeller/base/validation.h"
911
#include "impeller/entity/contents/content_context.h"
@@ -26,22 +28,20 @@ void EntityPass::SetDelegate(std::unique_ptr<EntityPassDelegate> delegate) {
2628
}
2729

2830
void EntityPass::AddEntity(Entity entity) {
29-
entities_.emplace_back(std::move(entity));
31+
elements_.emplace_back(std::move(entity));
3032
}
3133

32-
const std::vector<Entity>& EntityPass::GetEntities() const {
33-
return entities_;
34-
}
35-
36-
void EntityPass::SetEntities(Entities entities) {
37-
entities_ = std::move(entities);
34+
void EntityPass::SetElements(std::vector<Element> elements) {
35+
elements_ = std::move(elements);
3836
}
3937

4038
size_t EntityPass::GetSubpassesDepth() const {
4139
size_t max_subpass_depth = 0u;
42-
for (const auto& subpass : subpasses_) {
43-
max_subpass_depth =
44-
std::max(max_subpass_depth, subpass->GetSubpassesDepth());
40+
for (const auto& element : elements_) {
41+
if (auto subpass = std::get_if<std::unique_ptr<EntityPass>>(&element)) {
42+
max_subpass_depth =
43+
std::max(max_subpass_depth, subpass->get()->GetSubpassesDepth());
44+
}
4545
}
4646
return max_subpass_depth + 1u;
4747
}
@@ -50,10 +50,20 @@ const std::shared_ptr<LazyGlyphAtlas>& EntityPass::GetLazyGlyphAtlas() const {
5050
return lazy_glyph_atlas_;
5151
}
5252

53-
std::optional<Rect> EntityPass::GetEntitiesCoverage() const {
53+
std::optional<Rect> EntityPass::GetElementsCoverage() const {
5454
std::optional<Rect> result;
55-
for (const auto& entity : entities_) {
56-
auto coverage = entity.GetCoverage();
55+
for (const auto& element : elements_) {
56+
std::optional<Rect> coverage;
57+
58+
if (auto entity = std::get_if<Entity>(&element)) {
59+
coverage = entity->GetCoverage();
60+
} else if (auto subpass =
61+
std::get_if<std::unique_ptr<EntityPass>>(&element)) {
62+
coverage = subpass->get()->GetElementsCoverage();
63+
} else {
64+
FML_UNREACHABLE();
65+
}
66+
5767
if (!result.has_value() && coverage.has_value()) {
5868
result = coverage;
5969
continue;
@@ -68,7 +78,7 @@ std::optional<Rect> EntityPass::GetEntitiesCoverage() const {
6878

6979
std::optional<Rect> EntityPass::GetSubpassCoverage(
7080
const EntityPass& subpass) const {
71-
auto entities_coverage = subpass.GetEntitiesCoverage();
81+
auto entities_coverage = subpass.GetElementsCoverage();
7282
// The entities don't cover anything. There is nothing to do.
7383
if (!entities_coverage.has_value()) {
7484
return std::nullopt;
@@ -94,132 +104,149 @@ EntityPass* EntityPass::GetSuperpass() const {
94104
return superpass_;
95105
}
96106

97-
const EntityPass::Subpasses& EntityPass::GetSubpasses() const {
98-
return subpasses_;
99-
}
100-
101107
EntityPass* EntityPass::AddSubpass(std::unique_ptr<EntityPass> pass) {
102108
if (!pass) {
103109
return nullptr;
104110
}
105111
FML_DCHECK(pass->superpass_ == nullptr);
106112
pass->superpass_ = this;
107-
return subpasses_.emplace_back(std::move(pass)).get();
113+
auto subpass_pointer = pass.get();
114+
elements_.emplace_back(std::move(pass));
115+
return subpass_pointer;
108116
}
109117

110118
bool EntityPass::Render(ContentContext& renderer,
111119
RenderPass& parent_pass,
112120
Point position) const {
113121
TRACE_EVENT0("impeller", "EntityPass::Render");
114122

115-
for (Entity entity : entities_) {
116-
if (!position.IsZero()) {
117-
// If the pass image is going to be rendered with a non-zero position,
118-
// apply the negative translation to entity copies before rendering them
119-
// so that they'll end up rendering to the correct on-screen position.
120-
entity.SetTransformation(Matrix::MakeTranslation(Vector3(-position)) *
121-
entity.GetTransformation());
122-
}
123-
if (!entity.Render(renderer, parent_pass)) {
124-
return false;
125-
}
126-
}
127-
128-
for (const auto& subpass : subpasses_) {
129-
if (delegate_->CanElide()) {
130-
continue;
131-
}
132-
133-
if (delegate_->CanCollapseIntoParentPass()) {
134-
// Directly render into the parent pass and move on.
135-
if (!subpass->Render(renderer, parent_pass, position)) {
123+
for (const auto& element : elements_) {
124+
// =========================================================================
125+
// Entity rendering ========================================================
126+
// =========================================================================
127+
if (const auto& entity = std::get_if<Entity>(&element)) {
128+
Entity e = *entity;
129+
if (!position.IsZero()) {
130+
// If the pass image is going to be rendered with a non-zero position,
131+
// apply the negative translation to entity copies before rendering them
132+
// so that they'll end up rendering to the correct on-screen position.
133+
e.SetTransformation(Matrix::MakeTranslation(Vector3(-position)) *
134+
e.GetTransformation());
135+
}
136+
if (!e.Render(renderer, parent_pass)) {
136137
return false;
137138
}
138139
continue;
139140
}
140141

141-
const auto subpass_coverage = GetSubpassCoverage(*subpass);
142+
// =========================================================================
143+
// Subpass rendering =======================================================
144+
// =========================================================================
145+
if (const auto& subpass_ptr =
146+
std::get_if<std::unique_ptr<EntityPass>>(&element)) {
147+
auto subpass = subpass_ptr->get();
142148

143-
if (!subpass_coverage.has_value()) {
144-
continue;
145-
}
149+
if (delegate_->CanElide()) {
150+
continue;
151+
}
146152

147-
if (subpass_coverage->size.IsEmpty()) {
148-
// It is not an error to have an empty subpass. But subpasses that can't
149-
// create their intermediates must trip errors.
150-
continue;
151-
}
153+
if (delegate_->CanCollapseIntoParentPass()) {
154+
// Directly render into the parent pass and move on.
155+
if (!subpass->Render(renderer, parent_pass, position)) {
156+
return false;
157+
}
158+
continue;
159+
}
152160

153-
auto context = renderer.GetContext();
161+
const auto subpass_coverage = GetSubpassCoverage(*subpass);
154162

155-
auto subpass_target = RenderTarget::CreateOffscreen(
156-
*context, ISize::Ceil(subpass_coverage->size));
163+
if (!subpass_coverage.has_value()) {
164+
continue;
165+
}
157166

158-
auto subpass_texture = subpass_target.GetRenderTargetTexture();
167+
if (subpass_coverage->size.IsEmpty()) {
168+
// It is not an error to have an empty subpass. But subpasses that can't
169+
// create their intermediates must trip errors.
170+
continue;
171+
}
159172

160-
if (!subpass_texture) {
161-
return false;
162-
}
173+
auto context = renderer.GetContext();
163174

164-
auto offscreen_texture_contents =
165-
delegate_->CreateContentsForSubpassTarget(subpass_texture);
166-
167-
if (!offscreen_texture_contents) {
168-
// This is an error because the subpass delegate said the pass couldn't be
169-
// collapsed into its parent. Yet, when asked how it want's to postprocess
170-
// the offscreen texture, it couldn't give us an answer.
171-
//
172-
// Theoretically, we could collapse the pass now. But that would be
173-
// wasteful as we already have the offscreen texture and we don't want to
174-
// discard it without ever using it. Just make the delegate do the right
175-
// thing.
176-
return false;
177-
}
175+
auto subpass_target = RenderTarget::CreateOffscreen(
176+
*context, ISize::Ceil(subpass_coverage->size));
178177

179-
auto sub_command_buffer = context->CreateRenderCommandBuffer();
178+
auto subpass_texture = subpass_target.GetRenderTargetTexture();
180179

181-
sub_command_buffer->SetLabel("Offscreen Command Buffer");
180+
if (!subpass_texture) {
181+
return false;
182+
}
182183

183-
if (!sub_command_buffer) {
184-
return false;
185-
}
184+
auto offscreen_texture_contents =
185+
delegate_->CreateContentsForSubpassTarget(subpass_texture);
186+
187+
if (!offscreen_texture_contents) {
188+
// This is an error because the subpass delegate said the pass couldn't
189+
// be collapsed into its parent. Yet, when asked how it want's to
190+
// postprocess the offscreen texture, it couldn't give us an answer.
191+
//
192+
// Theoretically, we could collapse the pass now. But that would be
193+
// wasteful as we already have the offscreen texture and we don't want
194+
// to discard it without ever using it. Just make the delegate do the
195+
// right thing.
196+
return false;
197+
}
186198

187-
auto sub_renderpass = sub_command_buffer->CreateRenderPass(subpass_target);
199+
auto sub_command_buffer = context->CreateRenderCommandBuffer();
188200

189-
if (!sub_renderpass) {
190-
return false;
191-
}
201+
sub_command_buffer->SetLabel("Offscreen Command Buffer");
202+
203+
if (!sub_command_buffer) {
204+
return false;
205+
}
192206

193-
sub_renderpass->SetLabel("OffscreenPass");
207+
auto sub_renderpass =
208+
sub_command_buffer->CreateRenderPass(subpass_target);
194209

195-
if (!subpass->Render(renderer, *sub_renderpass, subpass_coverage->origin)) {
196-
return false;
197-
}
210+
if (!sub_renderpass) {
211+
return false;
212+
}
198213

199-
if (!sub_renderpass->EncodeCommands(*context->GetTransientsAllocator())) {
200-
return false;
201-
}
214+
sub_renderpass->SetLabel("OffscreenPass");
202215

203-
if (!sub_command_buffer->SubmitCommands()) {
204-
return false;
205-
}
216+
if (!subpass->Render(renderer, *sub_renderpass,
217+
subpass_coverage->origin)) {
218+
return false;
219+
}
220+
221+
if (!sub_renderpass->EncodeCommands(*context->GetTransientsAllocator())) {
222+
return false;
223+
}
224+
225+
if (!sub_command_buffer->SubmitCommands()) {
226+
return false;
227+
}
228+
229+
Entity entity;
230+
entity.SetPath(PathBuilder{}
231+
.AddRect(Rect::MakeSize(subpass_coverage->size))
232+
.TakePath());
233+
entity.SetContents(std::move(offscreen_texture_contents));
234+
entity.SetStencilDepth(stencil_depth_);
235+
entity.SetBlendMode(subpass->blend_mode_);
236+
// Once we have filters being applied for SaveLayer, some special sauce
237+
// may be needed here (or in PaintPassDelegate) to ensure the filter
238+
// parameters are transformed by the `xformation_` matrix, while
239+
// continuing to apply only the subpass offset to the offscreen texture.
240+
entity.SetTransformation(Matrix::MakeTranslation(
241+
Vector3(subpass_coverage->origin - position)));
242+
if (!entity.Render(renderer, parent_pass)) {
243+
return false;
244+
}
206245

207-
Entity entity;
208-
entity.SetPath(PathBuilder{}
209-
.AddRect(Rect::MakeSize(subpass_coverage->size))
210-
.TakePath());
211-
entity.SetContents(std::move(offscreen_texture_contents));
212-
entity.SetStencilDepth(stencil_depth_);
213-
entity.SetBlendMode(subpass->blend_mode_);
214-
// Once we have filters being applied for SaveLayer, some special sauce
215-
// may be needed here (or in PaintPassDelegate) to ensure the filter
216-
// parameters are transformed by the `xformation_` matrix, while continuing
217-
// to apply only the subpass offset to the offscreen texture.
218-
entity.SetTransformation(
219-
Matrix::MakeTranslation(Vector3(subpass_coverage->origin - position)));
220-
if (!entity.Render(renderer, parent_pass)) {
221-
return false;
246+
continue;
222247
}
248+
249+
FML_UNREACHABLE();
223250
}
224251

225252
return true;
@@ -230,23 +257,39 @@ void EntityPass::IterateAllEntities(std::function<bool(Entity&)> iterator) {
230257
return;
231258
}
232259

233-
for (auto& entity : entities_) {
234-
if (!iterator(entity)) {
235-
return;
260+
for (auto& element : elements_) {
261+
if (auto entity = std::get_if<Entity>(&element)) {
262+
if (!iterator(*entity)) {
263+
return;
264+
}
265+
continue;
236266
}
237-
}
238-
239-
for (auto& subpass : subpasses_) {
240-
subpass->IterateAllEntities(iterator);
267+
if (auto subpass = std::get_if<std::unique_ptr<EntityPass>>(&element)) {
268+
subpass->get()->IterateAllEntities(iterator);
269+
continue;
270+
}
271+
FML_UNREACHABLE();
241272
}
242273
}
243274

244275
std::unique_ptr<EntityPass> EntityPass::Clone() const {
245-
auto pass = std::make_unique<EntityPass>();
246-
pass->SetEntities(entities_);
247-
for (const auto& subpass : subpasses_) {
248-
pass->AddSubpass(subpass->Clone());
276+
std::vector<Element> new_elements;
277+
new_elements.reserve(elements_.size());
278+
279+
for (const auto& element : elements_) {
280+
if (auto entity = std::get_if<Entity>(&element)) {
281+
new_elements.push_back(*entity);
282+
continue;
283+
}
284+
if (auto subpass = std::get_if<std::unique_ptr<EntityPass>>(&element)) {
285+
new_elements.push_back(subpass->get()->Clone());
286+
continue;
287+
}
288+
FML_UNREACHABLE();
249289
}
290+
291+
auto pass = std::make_unique<EntityPass>();
292+
pass->SetElements(std::move(new_elements));
250293
return pass;
251294
}
252295

0 commit comments

Comments
 (0)