Skip to content

Commit

Permalink
cc: Move background filters to the effect tree
Browse files Browse the repository at this point in the history
This removes background filters from LayerImpl, and adds them to the
effect tree and to LayerImplTestProperties.

BUG=622410
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review-Url: https://codereview.chromium.org/2099743002
Cr-Commit-Position: refs/heads/master@{#401965}
  • Loading branch information
alijuma authored and Commit bot committed Jun 24, 2016
1 parent 31f1735 commit 50bce7e
Show file tree
Hide file tree
Showing 21 changed files with 98 additions and 78 deletions.
15 changes: 13 additions & 2 deletions cc/layers/layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ Layer::Layer()
should_check_backface_visibility_(false),
force_render_surface_for_testing_(false),
subtree_property_changed_(false),
layer_property_changed_(false),
has_will_change_transform_hint_(false),
background_color_(0),
safe_opaque_background_color_(0),
Expand Down Expand Up @@ -462,6 +463,7 @@ void Layer::SetBackgroundFilters(const FilterOperations& filters) {
if (background_filters_ == filters)
return;
background_filters_ = filters;
SetLayerPropertyChanged();
SetNeedsCommit();
}

Expand Down Expand Up @@ -1167,11 +1169,10 @@ void Layer::PushPropertiesTo(LayerImpl* layer) {
layer->SetDrawsContent(DrawsContent());
// subtree_property_changed_ is propagated to all descendants while building
// property trees. So, it is enough to check it only for the current layer.
if (subtree_property_changed_)
if (subtree_property_changed_ || layer_property_changed_)
layer->NoteLayerPropertyChanged();
if (!FilterIsAnimating())
layer->SetFilters(filters_);
layer->SetBackgroundFilters(background_filters());
layer->SetMasksToBounds(masks_to_bounds_);
layer->set_main_thread_scrolling_reasons(main_thread_scrolling_reasons_);
layer->SetNonFastScrollableRegion(non_fast_scrollable_region_);
Expand Down Expand Up @@ -1216,6 +1217,7 @@ void Layer::PushPropertiesTo(LayerImpl* layer) {

// Reset any state that should be cleared for the next update.
subtree_property_changed_ = false;
layer_property_changed_ = false;
update_rect_ = gfx::Rect();

layer_tree_host()->RemoveLayerShouldPushProperties(this);
Expand Down Expand Up @@ -1378,6 +1380,7 @@ void Layer::LayerSpecificPropertiesToProto(proto::LayerProperties* proto) {
base->set_draws_content(draws_content_);
base->set_hide_layer_and_subtree(hide_layer_and_subtree_);
base->set_subtree_property_changed(subtree_property_changed_);
base->set_layer_property_changed(layer_property_changed_);

// TODO(nyquist): Add support for serializing FilterOperations for
// |filters_| and |background_filters_|. See crbug.com/541321.
Expand Down Expand Up @@ -1462,6 +1465,7 @@ void Layer::FromLayerSpecificPropertiesProto(
draws_content_ = base.draws_content();
hide_layer_and_subtree_ = base.hide_layer_and_subtree();
subtree_property_changed_ = base.subtree_property_changed();
layer_property_changed_ = base.layer_property_changed();
masks_to_bounds_ = base.masks_to_bounds();
main_thread_scrolling_reasons_ = base.main_thread_scrolling_reasons();
non_fast_scrollable_region_ =
Expand Down Expand Up @@ -1600,6 +1604,13 @@ void Layer::SetSubtreePropertyChanged() {
SetNeedsPushProperties();
}

void Layer::SetLayerPropertyChanged() {
if (layer_property_changed_)
return;
layer_property_changed_ = true;
SetNeedsPushProperties();
}

gfx::ScrollOffset Layer::ScrollOffsetForAnimation() const {
return CurrentScrollOffset();
}
Expand Down
4 changes: 4 additions & 0 deletions cc/layers/layer.h
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,9 @@ class CC_EXPORT Layer : public base::RefCounted<Layer> {
void SetSubtreePropertyChanged();
bool subtree_property_changed() const { return subtree_property_changed_; }

void SetLayerPropertyChanged();
bool layer_property_changed() const { return layer_property_changed_; }

void DidBeginTracing();

int num_copy_requests_in_target_subtree();
Expand Down Expand Up @@ -646,6 +649,7 @@ class CC_EXPORT Layer : public base::RefCounted<Layer> {
bool should_check_backface_visibility_ : 1;
bool force_render_surface_for_testing_ : 1;
bool subtree_property_changed_ : 1;
bool layer_property_changed_ : 1;
bool has_will_change_transform_hint_ : 1;
Region non_fast_scrollable_region_;
Region touch_event_handler_region_;
Expand Down
10 changes: 0 additions & 10 deletions cc/layers/layer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,6 @@ void LayerImpl::PushPropertiesTo(LayerImpl* layer) {
layer->clip_tree_index_ = clip_tree_index_;
layer->scroll_tree_index_ = scroll_tree_index_;
layer->filters_ = filters_;
layer->background_filters_ = background_filters_;
layer->sorting_context_id_ = sorting_context_id_;
layer->has_will_change_transform_hint_ = has_will_change_transform_hint_;

Expand Down Expand Up @@ -806,15 +805,6 @@ bool LayerImpl::HasPotentiallyRunningFilterAnimation() const {
return layer_tree_impl_->HasPotentiallyRunningFilterAnimation(this);
}

void LayerImpl::SetBackgroundFilters(
const FilterOperations& filters) {
if (background_filters_ == filters)
return;

background_filters_ = filters;
NoteLayerPropertyChanged();
}

void LayerImpl::SetMasksToBounds(bool masks_to_bounds) {
if (masks_to_bounds_ == masks_to_bounds)
return;
Expand Down
6 changes: 0 additions & 6 deletions cc/layers/layer_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -196,11 +196,6 @@ class CC_EXPORT LayerImpl {
bool FilterIsAnimating() const;
bool HasPotentiallyRunningFilterAnimation() const;

void SetBackgroundFilters(const FilterOperations& filters);
const FilterOperations& background_filters() const {
return background_filters_;
}

void SetMasksToBounds(bool masks_to_bounds);
bool masks_to_bounds() const { return masks_to_bounds_; }

Expand Down Expand Up @@ -564,7 +559,6 @@ class CC_EXPORT LayerImpl {
int scroll_tree_index_;

FilterOperations filters_;
FilterOperations background_filters_;

protected:
friend class TreeSynchronizer;
Expand Down
2 changes: 2 additions & 0 deletions cc/layers/layer_impl_test_properties.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "base/memory/ptr_util.h"
#include "cc/layers/layer_collections.h"
#include "cc/layers/layer_position_constraint.h"
#include "cc/output/filter_operations.h"
#include "ui/gfx/geometry/point3_f.h"

namespace cc {
Expand All @@ -37,6 +38,7 @@ struct CC_EXPORT LayerImplTestProperties {
int num_descendants_that_draw_content;
size_t num_unclipped_descendants;
float opacity;
FilterOperations background_filters;
LayerPositionConstraint position_constraint;
gfx::Point3F transform_origin;
LayerImpl* scroll_parent;
Expand Down
6 changes: 0 additions & 6 deletions cc/layers/layer_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,6 @@ TEST(LayerImplTest, VerifyLayerChangesAreTrackedProperly) {
EXECUTE_AND_VERIFY_ONLY_LAYER_CHANGED(root->SetDrawsContent(true));
EXECUTE_AND_VERIFY_ONLY_LAYER_CHANGED(
root->SetBackgroundColor(arbitrary_color));
EXECUTE_AND_VERIFY_ONLY_LAYER_CHANGED(
root->SetBackgroundFilters(arbitrary_filters));

// Special case: check that SetBounds changes behavior depending on
// masksToBounds.
Expand Down Expand Up @@ -313,8 +311,6 @@ TEST(LayerImplTest, VerifyNeedsUpdateDrawProperties) {
layer->NoteLayerPropertyChanged());
VERIFY_NEEDS_UPDATE_DRAW_PROPERTIES(
layer->SetBackgroundColor(arbitrary_color));
VERIFY_NEEDS_UPDATE_DRAW_PROPERTIES(
layer->SetBackgroundFilters(arbitrary_filters));
VERIFY_NEEDS_UPDATE_DRAW_PROPERTIES(
layer->OnOpacityAnimated(arbitrary_number));
VERIFY_NEEDS_UPDATE_DRAW_PROPERTIES(layer->SetBlendMode(arbitrary_blend_mode);
Expand All @@ -334,8 +330,6 @@ TEST(LayerImplTest, VerifyNeedsUpdateDrawProperties) {
VERIFY_NO_NEEDS_UPDATE_DRAW_PROPERTIES(layer->SetDrawsContent(true));
VERIFY_NO_NEEDS_UPDATE_DRAW_PROPERTIES(
layer->SetBackgroundColor(arbitrary_color));
VERIFY_NO_NEEDS_UPDATE_DRAW_PROPERTIES(
layer->SetBackgroundFilters(arbitrary_filters));
VERIFY_NO_NEEDS_UPDATE_DRAW_PROPERTIES(
layer->SetBlendMode(arbitrary_blend_mode));
VERIFY_NO_NEEDS_UPDATE_DRAW_PROPERTIES(
Expand Down
23 changes: 23 additions & 0 deletions cc/layers/layer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,23 @@ using ::testing::_;
grand_child->layer_tree_host()->LayerNeedsPushPropertiesForTesting( \
grand_child.get()));

#define EXECUTE_AND_VERIFY_ONLY_LAYER_CHANGED(code_to_test) \
code_to_test; \
root->layer_tree_host()->BuildPropertyTreesForTesting(); \
EXPECT_TRUE(root->layer_property_changed()); \
EXPECT_FALSE(root->subtree_property_changed()); \
EXPECT_TRUE(root->layer_tree_host()->LayerNeedsPushPropertiesForTesting( \
root.get())); \
EXPECT_FALSE(child->layer_property_changed()); \
EXPECT_FALSE(child->subtree_property_changed()); \
EXPECT_FALSE(child->layer_tree_host()->LayerNeedsPushPropertiesForTesting( \
child.get())); \
EXPECT_FALSE(grand_child->layer_property_changed()); \
EXPECT_FALSE(grand_child->subtree_property_changed()); \
EXPECT_FALSE( \
grand_child->layer_tree_host()->LayerNeedsPushPropertiesForTesting( \
grand_child.get()));

namespace cc {

// This class is a friend of Layer, and is used as a wrapper for all the tests
Expand Down Expand Up @@ -1110,6 +1127,12 @@ TEST_F(LayerTest, LayerPropertyChangedForSubtree) {
child2->PushPropertiesTo(child2_impl.get());
grand_child->PushPropertiesTo(grand_child_impl.get()));

EXPECT_CALL(*layer_tree_host_, SetNeedsCommit()).Times(1);
EXECUTE_AND_VERIFY_ONLY_LAYER_CHANGED(
root->SetBackgroundFilters(arbitrary_filters));
EXECUTE_AND_VERIFY_SUBTREE_CHANGES_RESET(
root->PushPropertiesTo(root_impl.get()));

gfx::PointF arbitrary_point_f = gfx::PointF(0.125f, 0.25f);
EXPECT_CALL(*layer_tree_host_, SetNeedsCommit()).Times(1);
root->SetPosition(arbitrary_point_f);
Expand Down
8 changes: 6 additions & 2 deletions cc/layers/render_surface_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "cc/debug/debug_colors.h"
#include "cc/layers/layer_impl.h"
#include "cc/layers/render_pass_sink.h"
#include "cc/output/filter_operations.h"
#include "cc/quads/debug_border_draw_quad.h"
#include "cc/quads/render_pass.h"
#include "cc/quads/render_pass_draw_quad.h"
Expand Down Expand Up @@ -155,6 +156,10 @@ bool RenderSurfaceImpl::HasReplicaMask() const {
return OwningEffectNode()->data.replica_mask_layer_id != -1;
}

const FilterOperations& RenderSurfaceImpl::BackgroundFilters() const {
return OwningEffectNode()->data.background_filters;
}

bool RenderSurfaceImpl::HasCopyRequest() const {
return OwningEffectNode()->data.has_copy_request;
}
Expand Down Expand Up @@ -391,8 +396,7 @@ void RenderSurfaceImpl::AppendQuads(RenderPass* render_pass,
quad->SetNew(shared_quad_state, content_rect(), visible_layer_rect,
render_pass_id, mask_resource_id, mask_uv_scale,
mask_texture_size, owning_layer_->filters(),
owning_layer_to_target_scale,
owning_layer_->background_filters());
owning_layer_to_target_scale, BackgroundFilters());
}

} // namespace cc
3 changes: 3 additions & 0 deletions cc/layers/render_surface_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
namespace cc {

class DamageTracker;
class FilterOperations;
class Occlusion;
class RenderPassId;
class RenderPassSink;
Expand Down Expand Up @@ -150,6 +151,8 @@ class CC_EXPORT RenderSurfaceImpl {
LayerImpl* ReplicaMaskLayer();
bool HasReplicaMask() const;

const FilterOperations& BackgroundFilters() const;

bool HasCopyRequest() const;

void ResetPropertyChangedFlag() { surface_property_changed_ = false; }
Expand Down
3 changes: 2 additions & 1 deletion cc/proto/layer.proto
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ message LayerProperties {
optional SolidColorScrollbarLayerProperties solid_scrollbar = 7;
}

// NEXT ID: 54
// NEXT ID: 55
message BaseLayerProperties {
optional Point3F transform_origin = 1;
optional uint32 background_color = 2;
Expand All @@ -82,6 +82,7 @@ message BaseLayerProperties {
optional bool draws_content = 9;
optional bool hide_layer_and_subtree = 10;
optional bool subtree_property_changed = 47;
optional bool layer_property_changed = 54;
// TODO(nyquist): Add support for FilterOperation. See crbug.com/541321.
// repeated FilterOperation filters = 12;
// repeated FilterOperation background_filters = 13;
Expand Down
1 change: 0 additions & 1 deletion cc/proto/property_tree.proto
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ message EffectNodeData {
optional float screen_space_opacity = 2;
optional bool has_render_surface = 3;
optional bool has_copy_request = 4;
optional bool has_background_filters = 5;
optional bool hidden_by_backface_visibility = 14;
optional bool double_sided = 13;
optional bool is_drawn = 6;
Expand Down
18 changes: 9 additions & 9 deletions cc/trees/damage_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ gfx::Rect DamageTracker::TrackDamageFromActiveLayers(
continue;

if (layer->render_surface() && layer->render_surface() != target_surface)
ExtendDamageForRenderSurface(layer, &damage_rect);
ExtendDamageForRenderSurface(layer->render_surface(), &damage_rect);
else
ExtendDamageForLayer(layer, &damage_rect);
}
Expand Down Expand Up @@ -343,7 +343,7 @@ void DamageTracker::ExtendDamageForLayer(LayerImpl* layer,
}

void DamageTracker::ExtendDamageForRenderSurface(
LayerImpl* layer,
RenderSurfaceImpl* render_surface,
gfx::Rect* target_damage_rect) {
// There are two ways a "descendant surface" can damage regions of the "target
// surface":
Expand All @@ -360,10 +360,9 @@ void DamageTracker::ExtendDamageForRenderSurface(
// as well, and that damage should propagate to the target surface.
//

RenderSurfaceImpl* render_surface = layer->render_surface();

bool surface_is_new = false;
SurfaceRectMapData& data = RectDataForSurface(layer->id(), &surface_is_new);
SurfaceRectMapData& data =
RectDataForSurface(render_surface->OwningLayerId(), &surface_is_new);
gfx::Rect old_surface_rect = data.rect_;

// The drawableContextRect() already includes the replica if it exists.
Expand Down Expand Up @@ -428,10 +427,11 @@ void DamageTracker::ExtendDamageForRenderSurface(
// those pixels from the surface with only the contents of layers below this
// one in them. This means we need to redraw any pixels in the surface being
// used for the blur in this layer this frame.
if (layer->background_filters().HasFilterThatMovesPixels()) {
ExpandDamageRectInsideRectWithFilters(target_damage_rect,
surface_rect_in_target_space,
layer->background_filters());
const FilterOperations& background_filters =
render_surface->BackgroundFilters();
if (background_filters.HasFilterThatMovesPixels()) {
ExpandDamageRectInsideRectWithFilters(
target_damage_rect, surface_rect_in_target_space, background_filters);
}
}

Expand Down
2 changes: 1 addition & 1 deletion cc/trees/damage_tracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class CC_EXPORT DamageTracker {

// These helper functions are used only in TrackDamageFromActiveLayers().
void ExtendDamageForLayer(LayerImpl* layer, gfx::Rect* target_damage_rect);
void ExtendDamageForRenderSurface(LayerImpl* layer,
void ExtendDamageForRenderSurface(RenderSurfaceImpl* render_surface,
gfx::Rect* target_damage_rect);

struct LayerRectMapData {
Expand Down
3 changes: 2 additions & 1 deletion cc/trees/damage_tracker_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,8 @@ TEST_F(DamageTrackerTest, VerifyDamageForBackgroundBlurredChild) {

// Setting the filter will damage the whole surface.
ClearDamageForAllSurfaces(root);
child1->SetBackgroundFilters(filters);
child1->test_properties()->background_filters = filters;
child1->NoteLayerPropertyChanged();
root->layer_tree_impl()->property_trees()->needs_rebuild = true;
EmulateDrawingOneFrame(root);

Expand Down
2 changes: 1 addition & 1 deletion cc/trees/draw_property_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ static void ValidateRenderSurfaceForLayer(LayerImpl* layer) {
return;

DCHECK(layer->filters().IsEmpty()) << "layer: " << layer->id();
DCHECK(layer->background_filters().IsEmpty()) << "layer: " << layer->id();
DCHECK(!IsRootLayer(layer)) << "layer: " << layer->id();
EffectNode* effect_node =
layer->layer_tree_impl()->property_trees()->effect_tree.Node(
Expand All @@ -49,6 +48,7 @@ static void ValidateRenderSurfaceForLayer(LayerImpl* layer) {
return;
DCHECK_EQ(effect_node->data.mask_layer_id, -1) << "layer: " << layer->id();
DCHECK_EQ(effect_node->data.replica_layer_id, -1) << "layer: " << layer->id();
DCHECK(effect_node->data.background_filters.IsEmpty());
}

#endif
Expand Down
2 changes: 1 addition & 1 deletion cc/trees/layer_tree_host_common_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1337,7 +1337,7 @@ TEST_F(LayerTreeHostCommonTest,
child->SetDrawsContent(true);
FilterOperations filters;
filters.Append(FilterOperation::CreateBlurFilter(1.5f));
render_surface1->SetBackgroundFilters(filters);
render_surface1->test_properties()->background_filters = filters;

{
LayerImplList render_surface_layer_list;
Expand Down
Loading

0 comments on commit 50bce7e

Please sign in to comment.