Skip to content

Commit

Permalink
Fix backdrop mask in layer tree mode
Browse files Browse the repository at this point in the history
We missed setting the following fields for backdrop masks:
- PictureLayer::is_backdrop_filter_mask which should be set on the
  mask layer applying to any backdrop filter;
- EffectNode::backdrop_mask_element_id which should be set on the
  effect node with backdrop filter that is masked.

TODO(crbug.com/923429):
We still miss PictureLayer::is_backdrop_filter_mask in CAP.

Bug: 1030432
Change-Id: If9d949a5f04beeefa308b3e1167da391c16746e8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1956544
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#722780}
  • Loading branch information
wangxianzhu authored and Commit Bot committed Dec 8, 2019
1 parent c2cd1e5 commit 8abcbad
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 25 deletions.
7 changes: 4 additions & 3 deletions cc/layers/layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ void Layer::SetBounds(const gfx::Size& size) {
// Rounded corner clipping, bounds clipping and mask clipping can result in
// new areas of subtrees being exposed on a bounds change. Ensure the damaged
// areas are updated.
if (masks_to_bounds() || IsMaskedByChild() || HasRoundedCorner()) {
if (masks_to_bounds() || mask_layer() || HasRoundedCorner()) {
SetSubtreePropertyChanged();
SetPropertyTreesNeedRebuild();
}
Expand Down Expand Up @@ -569,8 +569,7 @@ gfx::RectF Layer::EffectiveClipRect() {

// Layer needs to clip to its bounds as well apply a clip rect. Intersect the
// two to get the effective clip.
if (masks_to_bounds() || IsMaskedByChild() ||
filters().HasFilterThatMovesPixels())
if (masks_to_bounds() || mask_layer() || filters().HasFilterThatMovesPixels())
return gfx::IntersectRects(layer_bounds, clip_rect_f);

// Clip rect is the only clip effecting the layer.
Expand All @@ -596,6 +595,8 @@ void Layer::SetMaskLayer(scoped_refptr<PictureLayer> mask_layer) {
mask_layer->inputs_.position = gfx::PointF();
mask_layer->SetIsDrawable(true);
mask_layer->SetBlendMode(SkBlendMode::kDstIn);
// This flag will be updated in PropertyTreeBuilder.
mask_layer->SetIsBackdropFilterMask(false);
AddChild(mask_layer);
}
inputs_.mask_layer = mask_layer.get();
Expand Down
5 changes: 3 additions & 2 deletions cc/layers/layer.h
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,8 @@ class CC_EXPORT Layer : public base::RefCounted<Layer> {
// for each matching pixel.
// This is for layer tree mode only.
void SetMaskLayer(scoped_refptr<PictureLayer> mask_layer);
bool IsMaskedByChild() const { return !!inputs_.mask_layer; }
const PictureLayer* mask_layer() const { return inputs_.mask_layer; }
PictureLayer* mask_layer() { return inputs_.mask_layer; }

// 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
Expand Down Expand Up @@ -781,7 +782,7 @@ class CC_EXPORT Layer : public base::RefCounted<Layer> {

// If not null, points to one of child layers which is set as mask layer
// by SetMaskLayer().
Layer* mask_layer;
PictureLayer* mask_layer;

int layer_id;

Expand Down
21 changes: 11 additions & 10 deletions cc/layers/layer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ TEST_F(LayerTest, LayerPropertyChangedForSubtree) {
scoped_refptr<Layer> grand_child = Layer::Create();
FakeContentLayerClient client;
scoped_refptr<PictureLayer> mask_layer1 = PictureLayer::Create(&client);
mask_layer1->SetElementId(LayerIdToElementIdForTesting(mask_layer1->id()));

layer_tree_host_->SetRootLayer(root);
root->AddChild(top);
Expand Down Expand Up @@ -404,7 +405,7 @@ TEST_F(LayerTest, LayerPropertyChangedForSubtree) {
child2->PushPropertiesTo(child2_impl.get());
grand_child->PushPropertiesTo(grand_child_impl.get()));

EXPECT_CALL(*layer_tree_host_, SetNeedsCommit()).Times(1);
EXPECT_CALL(*layer_tree_host_, SetNeedsCommit()).Times(2);
EXECUTE_AND_VERIFY_SUBTREE_CHANGED(
top->SetBackdropFilters(arbitrary_filters));
EXECUTE_AND_VERIFY_SUBTREE_CHANGES_RESET(
Expand Down Expand Up @@ -491,7 +492,7 @@ TEST_F(LayerTest, SetMaskLayer) {
ASSERT_EQ(1u, parent->children().size());
EXPECT_EQ(parent.get(), mask->parent());
EXPECT_EQ(mask.get(), parent->children()[0]);
EXPECT_TRUE(parent->IsMaskedByChild());
EXPECT_TRUE(parent->mask_layer());

// Should ignore mask layer's position.
EXPECT_TRUE(mask->position().IsOrigin());
Expand All @@ -502,20 +503,20 @@ TEST_F(LayerTest, SetMaskLayer) {
ASSERT_EQ(1u, parent->children().size());
EXPECT_EQ(parent.get(), mask->parent());
EXPECT_EQ(mask.get(), parent->children()[0]);
EXPECT_TRUE(parent->IsMaskedByChild());
EXPECT_TRUE(parent->mask_layer());

scoped_refptr<PictureLayer> mask2 = PictureLayer::Create(&client);
parent->SetMaskLayer(mask2);
EXPECT_FALSE(mask->parent());
ASSERT_EQ(1u, parent->children().size());
EXPECT_EQ(parent.get(), mask2->parent());
EXPECT_EQ(mask2.get(), parent->children()[0]);
EXPECT_TRUE(parent->IsMaskedByChild());
EXPECT_TRUE(parent->mask_layer());

parent->SetMaskLayer(nullptr);
EXPECT_EQ(0u, parent->children().size());
EXPECT_FALSE(mask2->parent());
EXPECT_FALSE(parent->IsMaskedByChild());
EXPECT_FALSE(parent->mask_layer());
}

TEST_F(LayerTest, RemoveMaskLayerFromParent) {
Expand All @@ -527,27 +528,27 @@ TEST_F(LayerTest, RemoveMaskLayerFromParent) {
mask->RemoveFromParent();
EXPECT_EQ(0u, parent->children().size());
EXPECT_FALSE(mask->parent());
EXPECT_FALSE(parent->IsMaskedByChild());
EXPECT_FALSE(parent->mask_layer());

scoped_refptr<PictureLayer> mask2 = PictureLayer::Create(&client);
parent->SetMaskLayer(mask2);
EXPECT_TRUE(parent->IsMaskedByChild());
EXPECT_TRUE(parent->mask_layer());
}

TEST_F(LayerTest, AddChildAfterSetMaskLayer) {
scoped_refptr<Layer> parent = Layer::Create();
FakeContentLayerClient client;
scoped_refptr<PictureLayer> mask = PictureLayer::Create(&client);
parent->SetMaskLayer(mask);
EXPECT_TRUE(parent->IsMaskedByChild());
EXPECT_TRUE(parent->mask_layer());

parent->AddChild(Layer::Create());
EXPECT_EQ(mask.get(), parent->children().back().get());
EXPECT_TRUE(parent->IsMaskedByChild());
EXPECT_TRUE(parent->mask_layer());

parent->InsertChild(Layer::Create(), parent->children().size());
EXPECT_EQ(mask.get(), parent->children().back().get());
EXPECT_TRUE(parent->IsMaskedByChild());
EXPECT_TRUE(parent->mask_layer());
}

TEST_F(LayerTest, AddSameChildTwice) {
Expand Down
3 changes: 1 addition & 2 deletions cc/trees/effect_node.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,7 @@ struct CC_EXPORT EffectNode {
gfx::PointF filters_origin;

// The element id corresponding to the mask to apply to the filtered backdrop
// image. Note that this is separate from mask_layer_id, which is a layer id,
// and is used for masking the "normal" (non-backdrop-filter) content.
// image.
ElementId backdrop_mask_element_id;

// Bounds of rounded corner rrect in the space of the transform node
Expand Down
72 changes: 66 additions & 6 deletions cc/trees/layer_tree_host_pixeltest_masks.cc
Original file line number Diff line number Diff line change
Expand Up @@ -526,10 +526,10 @@ class CircleContentLayerClient : public ContentLayerClient {
gfx::Size bounds_;
};

class LayerTreeHostMasksForBackdropFiltersPixelTest
class LayerTreeHostMasksForBackdropFiltersPixelTestWithLayerList
: public ParameterizedPixelResourceTest {
protected:
LayerTreeHostMasksForBackdropFiltersPixelTest()
LayerTreeHostMasksForBackdropFiltersPixelTestWithLayerList()
: bounds_(100, 100),
picture_client_(bounds_, SK_ColorGREEN, true),
mask_client_(bounds_) {
Expand Down Expand Up @@ -574,11 +574,12 @@ class LayerTreeHostMasksForBackdropFiltersPixelTest
CircleContentLayerClient mask_client_;
};

INSTANTIATE_TEST_SUITE_P(PixelResourceTest,
LayerTreeHostMasksForBackdropFiltersPixelTest,
::testing::ValuesIn(kTestCases));
INSTANTIATE_TEST_SUITE_P(
PixelResourceTest,
LayerTreeHostMasksForBackdropFiltersPixelTestWithLayerList,
::testing::ValuesIn(kTestCases));

TEST_P(LayerTreeHostMasksForBackdropFiltersPixelTest, Test) {
TEST_P(LayerTreeHostMasksForBackdropFiltersPixelTestWithLayerList, Test) {
base::FilePath image_name =
(raster_type() == GPU)
? base::FilePath(FILE_PATH_LITERAL("mask_of_backdrop_filter_gpu.png"))
Expand All @@ -601,6 +602,65 @@ TEST_P(LayerTreeHostMasksForBackdropFiltersPixelTest, Test) {
RunPixelResourceTestWithLayerList(image_name);
}

using LayerTreeHostMasksForBackdropFiltersPixelTestWithLayerTree =
ParameterizedPixelResourceTest;

INSTANTIATE_TEST_SUITE_P(
PixelResourceTest,
LayerTreeHostMasksForBackdropFiltersPixelTestWithLayerTree,
::testing::ValuesIn(kTestCases));

TEST_P(LayerTreeHostMasksForBackdropFiltersPixelTestWithLayerTree, Test) {
scoped_refptr<SolidColorLayer> background =
CreateSolidColorLayer(gfx::Rect(100, 100), SK_ColorWHITE);

gfx::Size picture_bounds(100, 100);
CheckerContentLayerClient picture_client(picture_bounds, SK_ColorGREEN, true);
scoped_refptr<PictureLayer> picture = PictureLayer::Create(&picture_client);
picture->SetBounds(picture_bounds);
picture->SetIsDrawable(true);

scoped_refptr<SolidColorLayer> blur =
CreateSolidColorLayer(gfx::Rect(100, 100), SK_ColorTRANSPARENT);
background->AddChild(picture);
background->AddChild(blur);

FilterOperations filters;
filters.Append(FilterOperation::CreateGrayscaleFilter(1.0));
blur->SetBackdropFilters(filters);
blur->ClearBackdropFilterBounds();

gfx::Size mask_bounds(100, 100);
CircleContentLayerClient mask_client(mask_bounds);
scoped_refptr<PictureLayer> mask = PictureLayer::Create(&mask_client);
mask->SetBounds(mask_bounds);
mask->SetIsDrawable(true);
mask->SetElementId(LayerIdToElementIdForTesting(mask->id()));

blur->SetMaskLayer(mask);

base::FilePath image_name =
(raster_type() == GPU)
? base::FilePath(FILE_PATH_LITERAL("mask_of_backdrop_filter_gpu.png"))
: base::FilePath(FILE_PATH_LITERAL("mask_of_backdrop_filter.png"));

if (renderer_type() == RENDERER_SKIA_VK && raster_type() == GPU) {
// Vulkan with GPU raster has 4 pixels errors (the circle mask shape is
// slight different).
float percentage_pixels_large_error = 0.04f; // 4px / (100*100)
float percentage_pixels_small_error = 0.0f;
float average_error_allowed_in_bad_pixels = 182.f;
int large_error_allowed = 182;
int small_error_allowed = 0;
pixel_comparator_ = std::make_unique<FuzzyPixelComparator>(
true /* discard_alpha */, percentage_pixels_large_error,
percentage_pixels_small_error, average_error_allowed_in_bad_pixels,
large_error_allowed, small_error_allowed);
}

RunPixelResourceTest(background, image_name);
}

TEST_P(LayerTreeHostMasksPixelTest, MaskOfLayerWithBlend) {
scoped_refptr<SolidColorLayer> background = CreateSolidColorLayer(
gfx::Rect(128, 128), SK_ColorWHITE);
Expand Down
9 changes: 7 additions & 2 deletions cc/trees/property_tree_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ bool HasAnyAnimationTargetingProperty(const MutatorHost& host,
// -------------------------------------------------------------------

bool LayerClipsSubtreeToItsBounds(Layer* layer) {
return layer->masks_to_bounds() || layer->IsMaskedByChild();
return layer->masks_to_bounds() || layer->mask_layer();
}

bool LayerClipsSubtree(Layer* layer) {
Expand Down Expand Up @@ -312,7 +312,7 @@ RenderSurfaceReason ComputeRenderSurfaceReason(const MutatorHost& mutator_host,
if (is_root)
return RenderSurfaceReason::kRoot;

if (layer->IsMaskedByChild()) {
if (layer->mask_layer()) {
return RenderSurfaceReason::kMask;
}

Expand Down Expand Up @@ -462,6 +462,11 @@ bool PropertyTreeBuilderContext::AddEffectNodeIfNeeded(
node->backdrop_filters = layer->backdrop_filters();
node->backdrop_filter_bounds = layer->backdrop_filter_bounds();
node->backdrop_filter_quality = layer->backdrop_filter_quality();
if (!node->backdrop_filters.IsEmpty() && layer->mask_layer()) {
DCHECK(layer->mask_layer()->element_id());
node->backdrop_mask_element_id = layer->mask_layer()->element_id();
layer->mask_layer()->SetIsBackdropFilterMask(true);
}
node->filters_origin = layer->filters_origin();
node->trilinear_filtering = layer->trilinear_filtering();
node->has_potential_opacity_animation = has_potential_opacity_animation;
Expand Down

0 comments on commit 8abcbad

Please sign in to comment.