Commit fa03b6b1 authored by Chris Harrelson's avatar Chris Harrelson Committed by Commit Bot

[BGPT] Omit synthetic mask layers that don't mask any content.

A synthetic mask layer is not needed if there is no content under
it which draws. This patch implements that optimization, and also
adds a new optimization to CompositedLayerMapping to set DrawsContent
to false for layers which have empty size.

The patch also removes one unused method from cc::Layer.

Bug: 947715

Change-Id: I7625fc251a3dc04ce5319edc56a9dc5fccf27564
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1566029
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: default avatarXianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#650514}
parent 5c81652a
......@@ -231,7 +231,6 @@ class CC_EXPORT Layer : public base::RefCounted<Layer> {
// for each matching pixel.
void SetMaskLayer(PictureLayer* mask_layer);
PictureLayer* mask_layer() { return inputs_.mask_layer.get(); }
const PictureLayer* mask_layer() const { return inputs_.mask_layer.get(); }
// Marks the |dirty_rect| as being changed, which will cause a commit and
// the compositor to submit a new frame with a damage rect that includes the
......
......@@ -2790,6 +2790,9 @@ bool CompositedLayerMapping::HasVisibleNonCompositingDescendant(
}
bool CompositedLayerMapping::ContainsPaintedContent() const {
if (CompositedBounds().IsEmpty())
return false;
if (GetLayoutObject().IsImage() && IsDirectlyCompositedImage())
return false;
......
......@@ -2720,6 +2720,28 @@ TEST_F(CompositedLayerMappingTest, ContentsNotOpaqueWithForegroundLayer) {
EXPECT_FALSE(mapping->MainGraphicsLayer()->ContentsOpaque());
}
TEST_F(CompositedLayerMappingTest, EmptyBoundsDoesntDrawContent) {
if (RuntimeEnabledFeatures::CompositeAfterPaintEnabled())
return;
SetHtmlInnerHTML(R"HTML(
<style>
div {
width: 100px;
height: 0px;
position: relative;
isolation: isolate;
}
</style>
<div id='target' style='will-change: transform; background: blue'>
</div>
)HTML");
PaintLayer* target_layer =
ToLayoutBoxModelObject(GetLayoutObjectByElementId("target"))->Layer();
CompositedLayerMapping* mapping = target_layer->GetCompositedLayerMapping();
EXPECT_FALSE(mapping->MainGraphicsLayer()->DrawsContent());
}
TEST_F(CompositedLayerMappingTest, TouchActionRectsWithoutContent) {
if (RuntimeEnabledFeatures::CompositeAfterPaintEnabled())
return;
......
......@@ -286,6 +286,9 @@ PaintArtifactCompositor::CompositedLayerForPendingLayer(
auto cc_layer = content_layer_client->UpdateCcPictureLayer(
paint_artifact, paint_chunks, cc_combined_bounds,
pending_layer.property_tree_state);
if (cc_combined_bounds.IsEmpty())
cc_layer->SetIsDrawable(false);
new_content_layer_clients.push_back(std::move(content_layer_client));
if (extra_data_for_testing_enabled_)
extra_data_for_testing_->content_layers.push_back(cc_layer);
......@@ -894,7 +897,7 @@ void PaintArtifactCompositor::Update(
int clip_id = property_tree_manager_.EnsureCompositorClipNode(clip);
int effect_id =
property_tree_manager_.SwitchToEffectNodeWithSynthesizedClip(
property_state.Effect(), clip);
property_state.Effect(), clip, layer->DrawsContent());
blink_effects.resize(effect_id + 1);
blink_effects[effect_id] = &property_state.Effect();
// The compositor scroll node is not directly stored in the property tree
......
......@@ -2619,6 +2619,49 @@ TEST_P(PaintArtifactCompositorTest, SynthesizedClipSimple) {
*GetPropertyTrees().effect_tree.Node(mask_effect_0_id);
ASSERT_EQ(mask_isolation_0_id, mask_effect_0.parent_id);
EXPECT_EQ(SkBlendMode::kDstIn, mask_effect_0.blend_mode);
// The masks DrawsContent because it has content that it masks which also
// DrawsContent.
EXPECT_TRUE(clip_mask0->DrawsContent());
}
TEST_P(PaintArtifactCompositorTest, SynthesizedClipIsNotDrawable) {
// This tests the simplist case that a single layer needs to be clipped
// by a single composited rounded clip.
FloatSize corner(5, 5);
FloatRoundedRect rrect(FloatRect(50, 50, 300, 200), corner, corner, corner,
corner);
auto c1 = CreateClip(c0(), t0(), rrect);
TestPaintArtifact artifact;
artifact.Chunk(t0(), *c1, e0())
.RectDrawing(FloatRect(0, 0, 0, 0), Color::kBlack);
Update(artifact.Build());
// Expectation in effect stack diagram:
// l1
// [ mask_isolation_0 ]
// [ e0 ]
// One content layer, no clip mask (because layer doesn't draw content).
ASSERT_EQ(1u, RootLayer()->children().size());
ASSERT_EQ(1u, ContentLayerCount());
ASSERT_EQ(0u, SynthesizedClipLayerCount());
const cc::Layer* content0 = RootLayer()->children()[0].get();
constexpr int c0_id = 1;
constexpr int e0_id = 1;
EXPECT_EQ(ContentLayerAt(0), content0);
int c1_id = content0->clip_tree_index();
const cc::ClipNode& cc_c1 = *GetPropertyTrees().clip_tree.Node(c1_id);
EXPECT_EQ(gfx::RectF(50, 50, 300, 200), cc_c1.clip);
ASSERT_EQ(c0_id, cc_c1.parent_id);
int mask_isolation_0_id = content0->effect_tree_index();
const cc::EffectNode& mask_isolation_0 =
*GetPropertyTrees().effect_tree.Node(mask_isolation_0_id);
ASSERT_EQ(e0_id, mask_isolation_0.parent_id);
EXPECT_EQ(SkBlendMode::kSrcOver, mask_isolation_0.blend_mode);
}
TEST_P(PaintArtifactCompositorTest,
......
......@@ -504,12 +504,15 @@ int PropertyTreeManager::EnsureCompositorScrollNode(
}
void PropertyTreeManager::EmitClipMaskLayer() {
cc::EffectNode& mask_isolation = *GetEffectTree().Node(current_.effect_id);
if (pending_synthetic_mask_layers_.Contains(mask_isolation.id))
return;
int clip_id = EnsureCompositorClipNode(*current_.clip);
CompositorElementId mask_isolation_id, mask_effect_id;
cc::Layer* mask_layer = client_.CreateOrReuseSynthesizedClipLayer(
*current_.clip, mask_isolation_id, mask_effect_id);
cc::EffectNode& mask_isolation = *GetEffectTree().Node(current_.effect_id);
// Assignment of mask_isolation.stable_id was delayed until now.
// See PropertyTreeManager::SynthesizeCcEffectsForClipsIfNeeded().
DCHECK_EQ(static_cast<uint64_t>(cc::EffectNode::INVALID_STABLE_ID),
......@@ -557,6 +560,9 @@ void PropertyTreeManager::CloseCcEffect() {
if (IsCurrentCcEffectSyntheticForNonTrivialClip())
EmitClipMaskLayer();
if (IsCurrentCcEffectSynthetic())
pending_synthetic_mask_layers_.erase(current_.effect_id);
current_ = previous_state;
effect_stack_.pop_back();
......@@ -568,7 +574,8 @@ void PropertyTreeManager::CloseCcEffect() {
int PropertyTreeManager::SwitchToEffectNodeWithSynthesizedClip(
const EffectPaintPropertyNode& next_effect,
const ClipPaintPropertyNode& next_clip) {
const ClipPaintPropertyNode& next_clip,
bool layer_draws_content) {
// This function is expected to be invoked right before emitting each layer.
// It keeps track of the nesting of clip and effects, output a composited
// effect node whenever an effect is entered, or a non-trivial clip is
......@@ -623,6 +630,10 @@ int PropertyTreeManager::SwitchToEffectNodeWithSynthesizedClip(
BuildEffectNodesRecursively(next_effect);
SynthesizeCcEffectsForClipsIfNeeded(next_clip, SkBlendMode::kSrcOver);
if (layer_draws_content)
pending_synthetic_mask_layers_.clear();
return current_.effect_id;
}
......@@ -745,6 +756,8 @@ SkBlendMode PropertyTreeManager::SynthesizeCcEffectsForClipsIfNeeded(
synthetic_effect.transform_id = EnsureCompositorTransformNode(transform);
synthetic_effect.double_sided = !transform.IsBackfaceHidden();
synthetic_effect.has_render_surface = true;
pending_synthetic_mask_layers_.insert(synthetic_effect.id);
// Clip and kDstIn do not commute. This shall never be reached because
// kDstIn is only used internally to implement CSS clip-path and mask,
// and there is never a difference between the output clip of the effect
......
......@@ -114,7 +114,8 @@ class PropertyTreeManager {
// effect, i.e. applying the clip as a mask.
int SwitchToEffectNodeWithSynthesizedClip(
const EffectPaintPropertyNode& next_effect,
const ClipPaintPropertyNode& next_clip);
const ClipPaintPropertyNode& next_clip,
bool layer_draws_content);
// Expected to be invoked after emitting the last layer. This will exit all
// effects on the effect stack, generating clip mask layers for all the
// unclosed synthesized clips.
......@@ -262,6 +263,10 @@ class PropertyTreeManager {
// recent push.
Vector<EffectState> effect_stack_;
// A set of synthetic clips masks which will be applied if a layer under them
// is encountered which draws content (and thus necessitates the mask).
HashSet<int> pending_synthetic_mask_layers_;
#if DCHECK_IS_ON()
HashSet<const EffectPaintPropertyNode*> effect_nodes_converted_;
bool initialized_ = false;
......
......@@ -29,6 +29,7 @@ Layer tree when the fixed elements are out-of-view (should have just a root laye
"name": "LayoutBlockFlow (positioned) DIV id='fixed2'",
"position": [100, 100],
"contentsOpaque": true,
"drawsContent": false,
"backgroundColor": "#C0C0C0"
}
]
......@@ -102,6 +103,7 @@ Layer tree when the fixed elements are out-of-view again (should have just a roo
"name": "LayoutBlockFlow (positioned) DIV id='fixed2'",
"position": [100, 100],
"contentsOpaque": true,
"drawsContent": false,
"backgroundColor": "#C0C0C0"
}
]
......
......@@ -22,6 +22,7 @@ Item 1
"name": "LayoutBlockFlow DIV id='composited-container'",
"position": [8, 8],
"contentsOpaque": true,
"drawsContent": false,
"backfaceVisibility": "hidden"
}
]
......
......@@ -26,6 +26,7 @@
"name": "LayoutBlockFlow (positioned) DIV class='stacking-context'",
"position": [8, 8],
"contentsOpaque": true,
"drawsContent": false,
"backgroundColor": "#FF0000"
},
{
......@@ -51,7 +52,8 @@
},
{
"name": "LayoutBlockFlow (positioned) DIV class='stacking-context' (foreground) Layer",
"position": [8, 8]
"position": [8, 8],
"drawsContent": false
},
{
"name": "LayoutBlockFlow HTML (foreground) Layer",
......
......@@ -26,6 +26,7 @@
"name": "LayoutBlockFlow (positioned) DIV class='stacking-context'",
"position": [8, 8],
"contentsOpaque": true,
"drawsContent": false,
"backgroundColor": "#FF0000"
},
{
......@@ -45,11 +46,13 @@
},
{
"name": "LayoutBlockFlow (positioned) DIV class='stacking-context' (foreground) Layer",
"position": [8, 8]
"position": [8, 8],
"drawsContent": false
},
{
"name": "LayoutBlockFlow (positioned) DIV class='accelerated stacking-context'",
"contentsOpaque": true,
"drawsContent": false,
"backgroundColor": "#FF0000",
"transform": 2
},
......@@ -70,6 +73,7 @@
},
{
"name": "LayoutBlockFlow (positioned) DIV class='accelerated stacking-context' (foreground) Layer",
"drawsContent": false,
"transform": 2
},
{
......
......@@ -19,7 +19,8 @@
},
{
"name": "LayoutInline (relative positioned) SPAN",
"position": [108, 108]
"position": [108, 108],
"drawsContent": false
},
{
"name": "LayoutNGBlockFlow (relative positioned) (floating) DIV id='float'",
......
......@@ -19,7 +19,8 @@
},
{
"name": "LayoutInline (relative positioned) SPAN",
"position": [108, 108]
"position": [108, 108],
"drawsContent": false
},
{
"name": "LayoutNGBlockFlow (relative positioned) (floating) DIV id='float'",
......
......@@ -20,6 +20,7 @@
{
"name": "LayoutTableRow TR",
"position": [8, 18],
"drawsContent": false,
"backgroundColor": "#0000FF"
},
{
......
......@@ -19,7 +19,8 @@
},
{
"name": "LayoutInline (relative positioned) SPAN",
"position": [108, 108]
"position": [108, 108],
"drawsContent": false
},
{
"name": "LayoutBlockFlow (relative positioned) (floating) DIV id='float'",
......
......@@ -19,7 +19,8 @@
},
{
"name": "LayoutInline (relative positioned) SPAN",
"position": [108, 108]
"position": [108, 108],
"drawsContent": false
},
{
"name": "LayoutBlockFlow (relative positioned) (floating) DIV id='float'",
......
......@@ -26,7 +26,8 @@
},
{
"name": "LayoutInline (relative positioned) SPAN",
"position": [108, 108]
"position": [108, 108],
"drawsContent": false
}
]
}
......
......@@ -20,6 +20,7 @@
{
"name": "LayoutTableRow TR",
"position": [8, 18],
"drawsContent": false,
"backgroundColor": "#0000FF"
},
{
......
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