Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CP] Revert "[Impeller] stencil buffer record/replay instead of MSAA storage. #51416

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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();
});
}