Commit 221dcd9e authored by Xianzhu Wang's avatar Xianzhu Wang Committed by Commit Bot

[Pre-CompositeAfterPaint] Allow chunk to escape noop layer effects

In pre-CompositeAfterPaint, we may squash one layer into another, but
the squashing layer may create more effect nodes not for real effects,
causing squashed layer's effect to escape the squashing layer's effect.
It's hard for compositor-before-paint to avoid this because it doesn't
know if PaintPropertyTreeBuilder will create such effect nodes.

In https://chromium-review.googlesource.com/c/1484671, I changed
I changed 'break' to 'return' for chunk-effect-escaping-layer-effect,
which prevented the chunk effect from being applied.

For the case we can continue because the extra effects are noop.

Bug: 971558
Change-Id: Ic009092ea9b371f79e30c4cdb2801d1c2319e4cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1649411Reviewed-by: default avatarPhilip Rogers <pdr@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#667357}
parent d8fb84f4
...@@ -202,6 +202,9 @@ class ConversionContext { ...@@ -202,6 +202,9 @@ class ConversionContext {
const EffectPaintPropertyNode* effect; const EffectPaintPropertyNode* effect;
// See ConversionContext::previous_transform_. // See ConversionContext::previous_transform_.
const TransformPaintPropertyNode* previous_transform; const TransformPaintPropertyNode* previous_transform;
#if DCHECK_IS_ON()
bool has_pre_cap_effect_hierarchy_issue = false;
#endif
}; };
void PushState(StateEntry::PairedType, int saved_count); void PushState(StateEntry::PairedType, int saved_count);
void PopState(); void PopState();
...@@ -426,6 +429,16 @@ void ConversionContext::StartClip( ...@@ -426,6 +429,16 @@ void ConversionContext::StartClip(
current_transform_ = &local_transform; current_transform_ = &local_transform;
} }
bool HasRealEffects(const EffectPaintPropertyNode& current,
const EffectPaintPropertyNode& ancestor) {
for (const auto* node = &current; node != &ancestor;
node = SafeUnalias(node->Parent())) {
if (node->HasRealEffects())
return true;
}
return false;
}
void ConversionContext::SwitchToEffect( void ConversionContext::SwitchToEffect(
const EffectPaintPropertyNode& target_effect_arg) { const EffectPaintPropertyNode& target_effect_arg) {
const auto& target_effect = target_effect_arg.Unalias(); const auto& target_effect = target_effect_arg.Unalias();
...@@ -435,6 +448,11 @@ void ConversionContext::SwitchToEffect( ...@@ -435,6 +448,11 @@ void ConversionContext::SwitchToEffect(
// Step 1: Exit all effects until the lowest common ancestor is found. // Step 1: Exit all effects until the lowest common ancestor is found.
const auto& lca_effect = const auto& lca_effect =
LowestCommonAncestor(target_effect, *current_effect_).Unalias(); LowestCommonAncestor(target_effect, *current_effect_).Unalias();
#if DCHECK_IS_ON()
bool has_pre_cap_effect_hierarchy_issue = false;
#endif
while (current_effect_ != &lca_effect) { while (current_effect_ != &lca_effect) {
// This EndClips() and the later EndEffect() pop to the parent effect. // This EndClips() and the later EndEffect() pop to the parent effect.
EndClips(); EndClips();
...@@ -448,9 +466,16 @@ void ConversionContext::SwitchToEffect( ...@@ -448,9 +466,16 @@ void ConversionContext::SwitchToEffect(
<< target_effect.ToTreeString().Utf8().data() << target_effect.ToTreeString().Utf8().data()
<< "current_effect_:\n" << "current_effect_:\n"
<< current_effect_->ToTreeString().Utf8().data(); << current_effect_->ToTreeString().Utf8().data();
has_pre_cap_effect_hierarchy_issue = true;
#endif #endif
if (RuntimeEnabledFeatures::CompositeAfterPaintEnabled()) if (RuntimeEnabledFeatures::CompositeAfterPaintEnabled())
NOTREACHED(); NOTREACHED();
// In pre-CompositeAfterPaint, we may squash one layer into another, but
// the squashing layer may create more effect nodes not for real effects,
// causing squashed layer's effect to escape the squashing layer's effect.
// We can continue because the extra effects are noop.
if (!HasRealEffects(*current_effect_, lca_effect))
break;
return; return;
} }
EndEffect(); EndEffect();
...@@ -459,7 +484,7 @@ void ConversionContext::SwitchToEffect( ...@@ -459,7 +484,7 @@ void ConversionContext::SwitchToEffect(
// Step 2: Collect all effects between the target effect and the current // Step 2: Collect all effects between the target effect and the current
// effect. At this point the current effect must be an ancestor of the target. // effect. At this point the current effect must be an ancestor of the target.
Vector<const EffectPaintPropertyNode*, 1u> pending_effects; Vector<const EffectPaintPropertyNode*, 1u> pending_effects;
for (const auto* effect = &target_effect; effect != current_effect_; for (const auto* effect = &target_effect; effect != &lca_effect;
effect = SafeUnalias(effect->Parent())) { effect = SafeUnalias(effect->Parent())) {
// This should never happen unless the DCHECK in step 1 failed. // This should never happen unless the DCHECK in step 1 failed.
if (!effect) if (!effect)
...@@ -470,8 +495,17 @@ void ConversionContext::SwitchToEffect( ...@@ -470,8 +495,17 @@ void ConversionContext::SwitchToEffect(
// Step 3: Now apply the list of effects in top-down order. // Step 3: Now apply the list of effects in top-down order.
for (size_t i = pending_effects.size(); i--;) { for (size_t i = pending_effects.size(); i--;) {
const EffectPaintPropertyNode* sub_effect = pending_effects[i]; const EffectPaintPropertyNode* sub_effect = pending_effects[i];
DCHECK_EQ(current_effect_, SafeUnalias(sub_effect->Parent())); #if DCHECK_IS_ON()
if (!has_pre_cap_effect_hierarchy_issue)
DCHECK_EQ(current_effect_, SafeUnalias(sub_effect->Parent()));
#endif
StartEffect(*sub_effect); StartEffect(*sub_effect);
#if DCHECK_IS_ON()
state_stack_.back().has_pre_cap_effect_hierarchy_issue =
has_pre_cap_effect_hierarchy_issue;
// This applies only to the first new effect.
has_pre_cap_effect_hierarchy_issue = false;
#endif
} }
} }
...@@ -575,10 +609,13 @@ void ConversionContext::UpdateEffectBounds( ...@@ -575,10 +609,13 @@ void ConversionContext::UpdateEffectBounds(
} }
void ConversionContext::EndEffect() { void ConversionContext::EndEffect() {
#if DCHECK_IS_ON()
const auto& previous_state = state_stack_.back(); const auto& previous_state = state_stack_.back();
DCHECK_EQ(previous_state.type, StateEntry::kEffect); DCHECK_EQ(previous_state.type, StateEntry::kEffect);
DCHECK_EQ(SafeUnalias(current_effect_->Parent()), previous_state.effect); if (!previous_state.has_pre_cap_effect_hierarchy_issue)
DCHECK_EQ(SafeUnalias(current_effect_->Parent()), previous_state.effect);
DCHECK_EQ(current_clip_, previous_state.clip); DCHECK_EQ(current_clip_, previous_state.clip);
#endif
DCHECK(effect_bounds_stack_.size()); DCHECK(effect_bounds_stack_.size());
const auto& bounds_info = effect_bounds_stack_.back(); const auto& bounds_info = effect_bounds_stack_.back();
......
...@@ -200,6 +200,12 @@ class PLATFORM_EXPORT EffectPaintPropertyNode ...@@ -200,6 +200,12 @@ class PLATFORM_EXPORT EffectPaintPropertyNode
return state_.filters_origin; return state_.filters_origin;
} }
bool HasRealEffects() const {
return Opacity() != 1.0f || GetColorFilter() != kColorFilterNone ||
BlendMode() != SkBlendMode::kSrcOver || !Filter().IsEmpty() ||
!BackdropFilter().IsEmpty();
}
// Returns a rect covering the pixels that can be affected by pixels in // Returns a rect covering the pixels that can be affected by pixels in
// |inputRect|. The rects are in the space of localTransformSpace. // |inputRect|. The rects are in the space of localTransformSpace.
FloatRect MapRect(const FloatRect& input_rect) const; FloatRect MapRect(const FloatRect& input_rect) const;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment