Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 0c53706

Browse files
authored
[Impeller] Fix nullopt access and simplify coverage computation in GetSubpassCoverage. (#47347)
Follow up for comments in #46130. This case shouldn't actually be possible today, but we should be able to make this reasonably testable without goldens... added an issue to follow-up here: flutter/flutter#137356 This branch noise will also melt away with: flutter/flutter#137306
1 parent 787f9ef commit 0c53706

File tree

1 file changed

+21
-44
lines changed

1 file changed

+21
-44
lines changed

impeller/entity/entity_pass.cc

Lines changed: 21 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5-
// FLUTTER_NOLINT: https://github.com/flutter/flutter/issues/132129
6-
75
#include "impeller/entity/entity_pass.h"
86

97
#include <memory>
@@ -25,6 +23,7 @@
2523
#include "impeller/entity/entity.h"
2624
#include "impeller/entity/inline_pass_context.h"
2725
#include "impeller/geometry/color.h"
26+
#include "impeller/geometry/rect.h"
2827
#include "impeller/renderer/command_buffer.h"
2928

3029
#ifdef IMPELLER_DEBUG
@@ -97,19 +96,20 @@ size_t EntityPass::GetSubpassesDepth() const {
9796

9897
std::optional<Rect> EntityPass::GetElementsCoverage(
9998
std::optional<Rect> coverage_limit) const {
100-
std::optional<Rect> result;
99+
std::optional<Rect> accumulated_coverage;
101100
for (const auto& element : elements_) {
102-
std::optional<Rect> coverage;
101+
std::optional<Rect> element_coverage;
103102

104103
if (auto entity = std::get_if<Entity>(&element)) {
105-
coverage = entity->GetCoverage();
104+
element_coverage = entity->GetCoverage();
106105

107106
// When the coverage limit is std::nullopt, that means there is no limit,
108107
// as opposed to empty coverage.
109-
if (coverage.has_value() && coverage_limit.has_value()) {
108+
if (element_coverage.has_value() && coverage_limit.has_value()) {
110109
const auto* filter = entity->GetContents()->AsFilter();
111110
if (!filter || filter->IsTranslationOnly()) {
112-
coverage = coverage->Intersection(coverage_limit.value());
111+
element_coverage =
112+
element_coverage->Intersection(coverage_limit.value());
113113
}
114114
}
115115
} else if (auto subpass_ptr =
@@ -122,21 +122,15 @@ std::optional<Rect> EntityPass::GetElementsCoverage(
122122
// If the current pass elements have any coverage so far and there's a
123123
// backdrop filter, then incorporate the backdrop filter in the
124124
// pre-filtered coverage of the subpass.
125-
if (result.has_value() && subpass.backdrop_filter_proc_) {
125+
if (accumulated_coverage.has_value() && subpass.backdrop_filter_proc_) {
126126
std::shared_ptr<FilterContents> backdrop_filter =
127-
subpass.backdrop_filter_proc_(FilterInput::Make(result.value()),
128-
subpass.xformation_,
129-
Entity::RenderingMode::kSubpass);
127+
subpass.backdrop_filter_proc_(
128+
FilterInput::Make(accumulated_coverage.value()),
129+
subpass.xformation_, Entity::RenderingMode::kSubpass);
130130
if (backdrop_filter) {
131131
auto backdrop_coverage = backdrop_filter->GetCoverage({});
132-
backdrop_coverage->origin += result->origin;
133-
if (backdrop_coverage.has_value()) {
134-
if (unfiltered_coverage.has_value()) {
135-
unfiltered_coverage = coverage->Union(*backdrop_coverage);
136-
} else {
137-
unfiltered_coverage = backdrop_coverage;
138-
}
139-
}
132+
unfiltered_coverage =
133+
Rect::Union(unfiltered_coverage, backdrop_coverage);
140134
} else {
141135
VALIDATION_LOG << "The EntityPass backdrop filter proc didn't return "
142136
"a valid filter.";
@@ -161,29 +155,19 @@ std::optional<Rect> EntityPass::GetElementsCoverage(
161155
if (image_filter) {
162156
Entity subpass_entity;
163157
subpass_entity.SetTransformation(subpass.xformation_);
164-
coverage = image_filter->GetCoverage(subpass_entity);
158+
element_coverage = image_filter->GetCoverage(subpass_entity);
165159
} else {
166-
coverage = unfiltered_coverage;
160+
element_coverage = unfiltered_coverage;
167161
}
168162

169-
if (coverage.has_value() && coverage_limit.has_value() &&
170-
(!image_filter || image_filter->IsTranslationOnly())) {
171-
coverage = coverage->Intersection(coverage_limit.value());
172-
}
163+
element_coverage = Rect::Intersection(element_coverage, coverage_limit);
173164
} else {
174165
FML_UNREACHABLE();
175166
}
176167

177-
if (!result.has_value() && coverage.has_value()) {
178-
result = coverage;
179-
continue;
180-
}
181-
if (!coverage.has_value()) {
182-
continue;
183-
}
184-
result = result->Union(coverage.value());
168+
accumulated_coverage = Rect::Union(accumulated_coverage, element_coverage);
185169
}
186-
return result;
170+
return accumulated_coverage;
187171
}
188172

189173
std::optional<Rect> EntityPass::GetSubpassCoverage(
@@ -762,16 +746,9 @@ bool EntityPass::RenderElement(Entity& element_entity,
762746
// rendered output will actually be used, and so we set this to the current
763747
// clip coverage (which is the max clip bounds). The contents may
764748
// optionally use this hint to avoid unnecessary rendering work.
765-
if (element_entity.GetContents()->GetCoverageHint().has_value()) {
766-
// If the element already has a coverage hint (because its an advanced
767-
// blend), then we need to intersect the clip coverage hint with the
768-
// existing coverage hint.
769-
element_entity.GetContents()->SetCoverageHint(
770-
current_clip_coverage->Intersection(
771-
element_entity.GetContents()->GetCoverageHint().value()));
772-
} else {
773-
element_entity.GetContents()->SetCoverageHint(current_clip_coverage);
774-
}
749+
auto element_coverage_hint = element_entity.GetContents()->GetCoverageHint();
750+
element_entity.GetContents()->SetCoverageHint(
751+
Rect::Intersection(element_coverage_hint, current_clip_coverage));
775752

776753
switch (clip_coverage.type) {
777754
case Contents::ClipCoverage::Type::kNoChange:

0 commit comments

Comments
 (0)