Commit 7faedca8 authored by vollick's avatar vollick Committed by Commit bot

Generalize scroll parent work in CalculateDrawProperties

Previously, CDP would fail if the scroll child wasn't the immediate
descendant of the lowest common ancestor of the scroll parent and child.

With this patch I've fixed up two things so that CDP can handle these
cases.

1. Dumber sorting.
  I've made sorting simpler by removing assumptions about the tree
  topology. Instead, I assigned "sort weights" to the layers in
  PreCalculateMetaInformation. The approach is pretty naive. As I
  walk the tree I assign a monotonically increasing weight. If I ever
  hit a scroll child before its parent, I jump up to the lowest common
  ancestor and make sure that the child of the LCA is assigned the
  smallest weight in the tree so far (the negative of the next
  monotonically increasing weight). This ensures that the scroll parent
  will be visited first. Here's a picture of a layer tree and the
  weights that would be assigned.

  lca(0)
  + filler(1)
  | + scroll child(2)
  + more-filler(-3)
    + scroll parent(4)

  It would be a huge bummer if we had to sort all the time, though, so
  I've taken care to flag the LCA if I ever have to assign a negative
  weight so that we only sort in that case.

  Here's my argument for the correctness of the algorithm. Clearly,
  we force correct ordering of a particular scroll child/parent pair,
  but the worry is that by setting a negative weight, we'll force an
  illegal ordering (i.e., we'll "undo" an ordering we've forced
  earlier in the algorithm). Here's a diagram.

  lca
  + subtree with scroll parent #1
  + subtree with scroll child #1 and scroll_parent #2
  + subtree with scroll child #2.

  Now, say we're processing scroll child #2. We want to be sure that
  we don't go reordering subtrees and mess up the relative ordering of
  scroll child #1 and scroll parent #1's subtrees.

  But we know that this is impossible. Since we've already visited the
  subtree with scroll child #1, scroll child #2 will necessarily get a
  higher sort weight, so we'll never need to force any reordering in
  this case; we won't assign any negative weights here.

  Say the ordering were changed such that when processing scroll child
  #2, we haven't seen scroll child #1's subtree. We're also fine in
  this case because we will do any ordering fixups required when we do
  eventually visit scroll child #1.

  The only case where we get into trouble is when we have a cycle. For
  example,

  lca
   + scroll parent 1
   | + scroll child 2
   + scroll parent 2
     + scroll child 1

  No ordering could satisfy a situation with a cycle, but this is
  an impossible situation as CSS stacking defines a well defined, non
  circular order.

2. Proper clip transformations.
  We sometimes need to put the scroll parent's clip into the space of
  the scroll child's immediate parent. This needs to go through the
  LCA. Previously, we'd assumed that the LCA was going to be the scroll
  child's immediate parent, but we no longer make that assumption.

BUG=403866

Review URL: https://codereview.chromium.org/548963002

Cr-Commit-Position: refs/heads/master@{#294521}
parent faab4288
...@@ -29,8 +29,6 @@ struct CC_EXPORT DrawProperties { ...@@ -29,8 +29,6 @@ struct CC_EXPORT DrawProperties {
num_unclipped_descendants(0), num_unclipped_descendants(0),
layer_or_descendant_has_copy_request(false), layer_or_descendant_has_copy_request(false),
layer_or_descendant_has_input_handler(false), layer_or_descendant_has_input_handler(false),
has_child_with_a_scroll_parent(false),
sorted_for_recursion(false),
index_of_first_descendants_addition(0), index_of_first_descendants_addition(0),
num_descendants_added(0), num_descendants_added(0),
index_of_first_render_surface_layer_list_addition(0), index_of_first_render_surface_layer_list_addition(0),
...@@ -39,7 +37,12 @@ struct CC_EXPORT DrawProperties { ...@@ -39,7 +37,12 @@ struct CC_EXPORT DrawProperties {
ideal_contents_scale(0.f), ideal_contents_scale(0.f),
maximum_animation_contents_scale(0.f), maximum_animation_contents_scale(0.f),
page_scale_factor(0.f), page_scale_factor(0.f),
device_scale_factor(0.f) {} device_scale_factor(0.f),
children_need_sorting(false),
sequence_number(0),
sort_weight(0),
sort_weight_sequence_number(0),
depth(0) {}
// Transforms objects from content space to target surface space, where // Transforms objects from content space to target surface space, where
// this layer would be drawn. // this layer would be drawn.
...@@ -106,16 +109,6 @@ struct CC_EXPORT DrawProperties { ...@@ -106,16 +109,6 @@ struct CC_EXPORT DrawProperties {
// If true, the layer or one of its descendants has a wheel or touch handler. // If true, the layer or one of its descendants has a wheel or touch handler.
bool layer_or_descendant_has_input_handler; bool layer_or_descendant_has_input_handler;
// This is true if the layer has any direct child that has a scroll parent.
// This layer will not be the scroll parent in this case. This information
// lets us avoid work in CalculateDrawPropertiesInternal -- if none of our
// children have scroll parents, we will not need to recur out of order.
bool has_child_with_a_scroll_parent;
// This is true if the order (wrt to its siblings in the tree) in which the
// layer will be visited while computing draw properties has been determined.
bool sorted_for_recursion;
// If this layer is visited out of order, its contribution to the descendant // If this layer is visited out of order, its contribution to the descendant
// and render surface layer lists will be put aside in a temporary list. // and render surface layer lists will be put aside in a temporary list.
// These values will allow for an efficient reordering of these additions. // These values will allow for an efficient reordering of these additions.
...@@ -147,6 +140,30 @@ struct CC_EXPORT DrawProperties { ...@@ -147,6 +140,30 @@ struct CC_EXPORT DrawProperties {
// The device scale factor that is applied to the layer. // The device scale factor that is applied to the layer.
float device_scale_factor; float device_scale_factor;
// This is used to detect cases when a layer needs to sort its children to
// ensure that scroll parents are visited before scroll children.
bool children_need_sorting;
// This sequence number is used to determine if we've visited this layer in
// PreCalculateMetaInformation.
int sequence_number;
// Determines the order the layer should be processed in
// CalculateDrawProperties wrt to its siblings. If you have lower
// |sort_weight| you get visited first.
int sort_weight;
// This sequence number is used to determine if a sort weight has already been
// assigned for a layer. We may assign a sort weight before we visit a layer,
// and it's important that we don't clobber it when we do eventually visit the
// layer.
int sort_weight_sequence_number;
// This is the topological depth in the tree. (E.g., root has depth zero).
// This is used to make it faster to find the lowest common ancestor of two
// layers.
int depth;
}; };
} // namespace cc } // namespace cc
......
...@@ -123,7 +123,8 @@ LayerTreeHost::LayerTreeHost(LayerTreeHostClient* client, ...@@ -123,7 +123,8 @@ LayerTreeHost::LayerTreeHost(LayerTreeHostClient* client,
total_frames_used_for_lcd_text_metrics_(0), total_frames_used_for_lcd_text_metrics_(0),
id_(s_layer_tree_host_sequence_number.GetNext() + 1), id_(s_layer_tree_host_sequence_number.GetNext() + 1),
next_commit_forces_redraw_(false), next_commit_forces_redraw_(false),
shared_bitmap_manager_(manager) { shared_bitmap_manager_(manager),
render_surface_layer_list_id_(0) {
if (settings_.accelerated_animation_enabled) if (settings_.accelerated_animation_enabled)
animation_registrar_ = AnimationRegistrar::Create(); animation_registrar_ = AnimationRegistrar::Create();
rendering_stats_instrumentation_->set_record_rendering_stats( rendering_stats_instrumentation_->set_record_rendering_stats(
...@@ -827,10 +828,7 @@ bool LayerTreeHost::UpdateLayers(Layer* root_layer, ...@@ -827,10 +828,7 @@ bool LayerTreeHost::UpdateLayers(Layer* root_layer,
TRACE_EVENT0("cc", "LayerTreeHost::UpdateLayers::CalcDrawProps"); TRACE_EVENT0("cc", "LayerTreeHost::UpdateLayers::CalcDrawProps");
bool can_render_to_separate_surface = true; bool can_render_to_separate_surface = true;
// TODO(vmpstr): Passing 0 as the current render surface layer list id means ++render_surface_layer_list_id_;
// that we won't be able to detect if a layer is part of |update_list|.
// Change this if this information is required.
int render_surface_layer_list_id = 0;
LayerTreeHostCommon::CalcDrawPropsMainInputs inputs( LayerTreeHostCommon::CalcDrawPropsMainInputs inputs(
root_layer, root_layer,
device_viewport_size(), device_viewport_size(),
...@@ -843,7 +841,7 @@ bool LayerTreeHost::UpdateLayers(Layer* root_layer, ...@@ -843,7 +841,7 @@ bool LayerTreeHost::UpdateLayers(Layer* root_layer,
can_render_to_separate_surface, can_render_to_separate_surface,
settings_.layer_transforms_should_scale_layer_contents, settings_.layer_transforms_should_scale_layer_contents,
&update_list, &update_list,
render_surface_layer_list_id); render_surface_layer_list_id_);
LayerTreeHostCommon::CalculateDrawProperties(&inputs); LayerTreeHostCommon::CalculateDrawProperties(&inputs);
if (total_frames_used_for_lcd_text_metrics_ <= if (total_frames_used_for_lcd_text_metrics_ <=
......
...@@ -456,6 +456,10 @@ class CC_EXPORT LayerTreeHost { ...@@ -456,6 +456,10 @@ class CC_EXPORT LayerTreeHost {
ScopedPtrVector<SwapPromise> swap_promise_list_; ScopedPtrVector<SwapPromise> swap_promise_list_;
std::set<SwapPromiseMonitor*> swap_promise_monitor_; std::set<SwapPromiseMonitor*> swap_promise_monitor_;
// We associate work done in CalculateDrawProperties with a sequence number
// to avoid having to do extra walks to clear values.
int render_surface_layer_list_id_;
DISALLOW_COPY_AND_ASSIGN(LayerTreeHost); DISALLOW_COPY_AND_ASSIGN(LayerTreeHost);
}; };
......
This diff is collapsed.
...@@ -223,7 +223,7 @@ LayerTreeHostCommon::CalcDrawPropsInputsForTesting<LayerType, ...@@ -223,7 +223,7 @@ LayerTreeHostCommon::CalcDrawPropsInputsForTesting<LayerType,
true, true,
false, false,
render_surface_layer_list, render_surface_layer_list,
0) { 1) {
DCHECK(root_layer); DCHECK(root_layer);
DCHECK(render_surface_layer_list); DCHECK(render_surface_layer_list);
} }
...@@ -247,7 +247,7 @@ LayerTreeHostCommon::CalcDrawPropsInputsForTesting<LayerType, ...@@ -247,7 +247,7 @@ LayerTreeHostCommon::CalcDrawPropsInputsForTesting<LayerType,
true, true,
false, false,
render_surface_layer_list, render_surface_layer_list,
0) { 1) {
DCHECK(root_layer); DCHECK(root_layer);
DCHECK(render_surface_layer_list); DCHECK(render_surface_layer_list);
} }
......
This diff is collapsed.
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