Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
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
33 changes: 33 additions & 0 deletions impeller/entity/entity_pass.cc
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,43 @@ std::optional<Rect> EntityPass::GetElementsCoverage(

std::optional<Rect> unfiltered_coverage =
GetSubpassCoverage(subpass, std::nullopt);

// If the current pass elements have any coverage so far and there's a
// backdrop filter, then incorporate the backdrop filter in the
// pre-filtered coverage of the subpass.
if (result.has_value() && subpass.backdrop_filter_proc_) {
std::shared_ptr<FilterContents> backdrop_filter =
subpass.backdrop_filter_proc_(FilterInput::Make(result.value()),
subpass.xformation_,
Entity::RenderingMode::kSubpass);
if (backdrop_filter) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is subpass.backdrop_filter_proc_ and no backdrop filter response an error state? Should we add a validation log for that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was thinking maybe we should gracefully support nullptr results for filters that do nothing, but on second thought I think that's confusing and filters should internally elide instead (as they already do)... Added a log.

auto backdrop_coverage = backdrop_filter->GetCoverage({});
backdrop_coverage->origin += result->origin;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this line be under the if on the line below?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah this could be placed just above unfiltered_coverage = backdrop_coverage

if (backdrop_coverage.has_value()) {
if (unfiltered_coverage.has_value()) {
unfiltered_coverage = coverage->Union(*backdrop_coverage);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like coverage could be undefined here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, yeah, if coverage is nullopt we should just unfiltered_coverage = backdrop_coverage

} else {
unfiltered_coverage = backdrop_coverage;
}
}
} else {
VALIDATION_LOG << "The EntityPass backdrop filter proc didn't return "
"a valid filter.";
}
}

if (!unfiltered_coverage.has_value()) {
continue;
}

// Additionally, subpass textures may be passed through filters, which may
// modify the coverage.
//
// Note that we currently only assume that ImageFilters (such as blurs and
// matrix transforms) may modify coverage, although it's technically
// possible ColorFilters to affect coverage as well. For example: A
// ColorMatrixFilter could output a completely transparent result, and
// we could potentially detect this case as zero coverage in the future.
std::shared_ptr<FilterContents> image_filter =
subpass.delegate_->WithImageFilter(*unfiltered_coverage,
subpass.xformation_);
Expand Down