Skip to content

Commit

Permalink
[CP] Revert "[Impeller] stencil buffer record/replay instead of MSAA …
Browse files Browse the repository at this point in the history
…storage. (#51416)

Reverts #47397 to fix flutter/flutter#144211 in 3.19.

This reverts commit 04329c5.
  • Loading branch information
bdero committed Mar 19, 2024
1 parent 2e4ba9c commit ba0444a
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 71 deletions.
68 changes: 23 additions & 45 deletions impeller/entity/entity_pass.cc
Original file line number Diff line number Diff line change
Expand Up @@ -238,15 +238,18 @@ void EntityPass::AddSubpassInline(std::unique_ptr<EntityPass> pass) {
pass->advanced_blend_reads_from_pass_texture_;
}

static const constexpr RenderTarget::AttachmentConfig kDefaultStencilConfig =
RenderTarget::AttachmentConfig{
.storage_mode = StorageMode::kDeviceTransient,
.load_action = LoadAction::kDontCare,
.store_action = StoreAction::kDontCare,
};
static RenderTarget::AttachmentConfig GetDefaultStencilConfig(bool readable) {
return RenderTarget::AttachmentConfig{
.storage_mode = readable ? StorageMode::kDevicePrivate
: StorageMode::kDeviceTransient,
.load_action = LoadAction::kDontCare,
.store_action = StoreAction::kDontCare,
};
}

static EntityPassTarget CreateRenderTarget(ContentContext& renderer,
ISize size,
bool readable,
const Color& clear_color) {
auto context = renderer.GetContext();

Expand All @@ -267,8 +270,8 @@ static EntityPassTarget CreateRenderTarget(ContentContext& renderer,
.resolve_storage_mode = StorageMode::kDevicePrivate,
.load_action = LoadAction::kDontCare,
.store_action = StoreAction::kMultisampleResolve,
.clear_color = clear_color}, // color_attachment_config
kDefaultStencilConfig // stencil_attachment_config
.clear_color = clear_color}, // color_attachment_config
GetDefaultStencilConfig(readable) // stencil_attachment_config
);
} else {
target = RenderTarget::CreateOffscreen(
Expand All @@ -281,8 +284,8 @@ static EntityPassTarget CreateRenderTarget(ContentContext& renderer,
.load_action = LoadAction::kDontCare,
.store_action = StoreAction::kDontCare,
.clear_color = clear_color,
}, // color_attachment_config
kDefaultStencilConfig // stencil_attachment_config
}, // color_attachment_config
GetDefaultStencilConfig(readable) // stencil_attachment_config
);
}

Expand Down Expand Up @@ -340,7 +343,7 @@ bool EntityPass::Render(ContentContext& renderer,
// there's no need to set up a stencil attachment on the root render target.
if (reads_from_onscreen_backdrop) {
auto offscreen_target = CreateRenderTarget(
renderer, root_render_target.GetRenderTargetSize(),
renderer, root_render_target.GetRenderTargetSize(), true,
GetClearColorOrDefault(render_target.GetRenderTargetSize()));

if (!OnRender(renderer, // renderer
Expand Down Expand Up @@ -442,7 +445,8 @@ bool EntityPass::Render(ContentContext& renderer,
*renderer.GetContext(), *renderer.GetRenderTargetCache(),
color0.texture->GetSize(),
renderer.GetContext()->GetCapabilities()->SupportsOffscreenMSAA(),
"ImpellerOnscreen", kDefaultStencilConfig);
"ImpellerOnscreen",
GetDefaultStencilConfig(reads_from_onscreen_backdrop));
}

// Set up the clear color of the root pass.
Expand Down Expand Up @@ -479,10 +483,10 @@ EntityPass::EntityResult EntityPass::GetEntityForElement(
//--------------------------------------------------------------------------
/// Setup entity element.
///

if (const auto& entity = std::get_if<Entity>(&element)) {
Entity element_entity = entity->Clone();
element_entity.SetCapture(capture.CreateChild("Entity"));

if (!global_pass_position.IsZero()) {
// If the pass image is going to be rendered with a non-zero position,
// apply the negative translation to entity copies before rendering them
Expand All @@ -497,9 +501,11 @@ EntityPass::EntityResult EntityPass::GetEntityForElement(
//--------------------------------------------------------------------------
/// Setup subpass element.
///
if (const auto& subpass_ptr =
std::get_if<std::unique_ptr<EntityPass>>(&element)) {

else if (const auto& subpass_ptr =
std::get_if<std::unique_ptr<EntityPass>>(&element)) {
auto subpass = subpass_ptr->get();

if (subpass->delegate_->CanElide()) {
return EntityPass::EntityResult::Skip();
}
Expand Down Expand Up @@ -602,6 +608,7 @@ EntityPass::EntityResult EntityPass::GetEntityForElement(
auto subpass_target = CreateRenderTarget(
renderer, // renderer
subpass_size, // size
subpass->GetTotalPassReads(renderer) > 0, // readable
subpass->GetClearColorOrDefault(subpass_size)); // clear_color

if (!subpass_target.IsValid()) {
Expand Down Expand Up @@ -675,6 +682,7 @@ EntityPass::EntityResult EntityPass::GetEntityForElement(

return EntityPass::EntityResult::Success(std::move(element_entity));
}

FML_UNREACHABLE();
}

Expand All @@ -699,15 +707,6 @@ bool EntityPass::RenderElement(Entity& element_entity,
// blit the non-MSAA resolve texture of the previous pass to MSAA textures
// (let alone a transient one).
if (result.backdrop_texture) {
// Restore any clips that were recorded before the backdrop filter was
// applied.
auto& replay_entities = clip_replay_->GetReplayEntities();
for (const auto& entity : replay_entities) {
if (!entity.Render(renderer, *result.pass)) {
VALIDATION_LOG << "Failed to render entity for clip restore.";
}
}

auto size_rect = Rect::MakeSize(result.pass->GetRenderTargetSize());
auto msaa_backdrop_contents = TextureContents::MakeRect(size_rect);
msaa_backdrop_contents->SetStencilEnabled(false);
Expand Down Expand Up @@ -815,7 +814,6 @@ bool EntityPass::RenderElement(Entity& element_entity,
#endif

element_entity.SetClipDepth(element_entity.GetClipDepth() - clip_depth_floor);
clip_replay_->RecordEntity(element_entity, clip_coverage.type);
if (!element_entity.Render(renderer, *result.pass)) {
VALIDATION_LOG << "Failed to render entity.";
return false;
Expand Down Expand Up @@ -1167,24 +1165,4 @@ void EntityPass::SetEnableOffscreenCheckerboard(bool enabled) {
enable_offscreen_debug_checkerboard_ = enabled;
}

EntityPassClipRecorder::EntityPassClipRecorder() {}

void EntityPassClipRecorder::RecordEntity(const Entity& entity,
Contents::ClipCoverage::Type type) {
switch (type) {
case Contents::ClipCoverage::Type::kNoChange:
return;
case Contents::ClipCoverage::Type::kAppend:
rendered_clip_entities_.push_back(entity.Clone());
break;
case Contents::ClipCoverage::Type::kRestore:
rendered_clip_entities_.pop_back();
break;
}
}

const std::vector<Entity>& EntityPassClipRecorder::GetReplayEntities() const {
return rendered_clip_entities_;
}

} // namespace impeller
23 changes: 0 additions & 23 deletions impeller/entity/entity_pass.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
namespace impeller {

class ContentContext;
class EntityPassClipRecorder;

class EntityPass {
public:
Expand Down Expand Up @@ -295,8 +294,6 @@ class EntityPass {
bool flood_clip_ = false;
bool enable_offscreen_debug_checkerboard_ = false;
std::optional<Rect> bounds_limit_;
std::unique_ptr<EntityPassClipRecorder> clip_replay_ =
std::make_unique<EntityPassClipRecorder>();

/// These values are incremented whenever something is added to the pass that
/// requires reading from the backdrop texture. Currently, this can happen in
Expand All @@ -321,26 +318,6 @@ class EntityPass {
EntityPass& operator=(const EntityPass&) = delete;
};

/// @brief A class that tracks all clips that have been recorded in the current
/// entity pass stencil.
///
/// These clips are replayed when restoring the backdrop so that the
/// stencil buffer is left in an identical state.
class EntityPassClipRecorder {
public:
EntityPassClipRecorder();

~EntityPassClipRecorder() = default;

/// @brief Record the entity based on the provided coverage [type].
void RecordEntity(const Entity& entity, Contents::ClipCoverage::Type type);

const std::vector<Entity>& GetReplayEntities() const;

private:
std::vector<Entity> rendered_clip_entities_;
};

} // namespace impeller

#endif // FLUTTER_IMPELLER_ENTITY_ENTITY_PASS_H_
13 changes: 11 additions & 2 deletions impeller/entity/inline_pass_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ InlinePassContext::InlinePassContext(
: context_(std::move(context)),
pass_target_(pass_target),
entity_count_(entity_count),
total_pass_reads_(pass_texture_reads),
is_collapsed_(collapsed_parent_pass.has_value()) {
if (collapsed_parent_pass.has_value()) {
pass_ = collapsed_parent_pass.value().pass;
Expand Down Expand Up @@ -141,9 +142,17 @@ InlinePassContext::RenderPassResult InlinePassContext::GetRenderPass(
return {};
}

stencil->load_action = LoadAction::kClear;
stencil->store_action = StoreAction::kDontCare;
// Only clear the stencil if this is the very first pass of the
// layer.
stencil->load_action =
pass_count_ > 0 ? LoadAction::kLoad : LoadAction::kClear;
// If we're on the last pass of the layer, there's no need to store the
// stencil because nothing needs to read it.
stencil->store_action = pass_count_ == total_pass_reads_
? StoreAction::kDontCare
: StoreAction::kStore;
pass_target_.target_.SetStencilAttachment(stencil.value());

pass_target_.target_.SetColorAttachment(color0, 0);

pass_ = command_buffer_->CreateRenderPass(pass_target_.GetRenderTarget());
Expand Down
2 changes: 1 addition & 1 deletion impeller/entity/inline_pass_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class InlinePassContext {
std::shared_ptr<RenderPass> pass_;
uint32_t pass_count_ = 0;
uint32_t entity_count_ = 0;

uint32_t total_pass_reads_ = 0;
// Whether this context is collapsed into a parent entity pass.
bool is_collapsed_ = false;

Expand Down
43 changes: 43 additions & 0 deletions testing/dart/painting_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import 'dart:ui';

import 'package:litetest/litetest.dart';

typedef CanvasCallback = void Function(Canvas canvas);

void main() {
test('Vertices checks', () {
try {
Expand Down Expand Up @@ -60,4 +62,45 @@ void main() {
indices: Uint16List.fromList(const <int>[0, 2, 1, 2, 0, 1, 2, 0]),
).dispose();
});

test('BackdropFilter with multiple clips', () async {
// Regression test for https://github.com/flutter/flutter/issues/144211
Picture makePicture(CanvasCallback callback) {
final PictureRecorder recorder = PictureRecorder();
final Canvas canvas = Canvas(recorder);
callback(canvas);
return recorder.endRecording();
}
final SceneBuilder sceneBuilder = SceneBuilder();

final Picture redClippedPicture = makePicture((Canvas canvas) {
canvas.clipRect(const Rect.fromLTRB(10, 10, 200, 200));
canvas.clipRect(const Rect.fromLTRB(11, 10, 300, 200));
canvas.drawPaint(Paint()..color = const Color(0xFFFF0000));
});
sceneBuilder.addPicture(Offset.zero, redClippedPicture);

final Float64List matrix = Float64List(16);
sceneBuilder.pushBackdropFilter(ImageFilter.matrix(matrix));

final Picture whitePicture = makePicture((Canvas canvas) {
canvas.drawPaint(Paint()..color = const Color(0xFFFFFFFF));
});
sceneBuilder.addPicture(Offset.zero, whitePicture);

final Scene scene = sceneBuilder.build();
final Image image = scene.toImageSync(20, 20);

final ByteData data = (await image.toByteData())!;
expect(data.buffer.asUint32List().length, 20 * 20);
// If clipping went wrong as in the linked issue, there will be red pixels.
for (final int color in data.buffer.asUint32List()) {
expect(color, 0xFFFFFFFF);
}

scene.dispose();
image.dispose();
whitePicture.dispose();
redClippedPicture.dispose();
});
}

0 comments on commit ba0444a

Please sign in to comment.