Generalize scroll parent work in CalculateDrawProperties

cc/trees/layer_tree_host_common.cc

 // Copyright 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/trees/layer_tree_host_common.h"
 
 #include <algorithm>
 
 #include "base/debug/trace_event.h"
 #include "cc/base/math_util.h"
@@ skipping 184 matching lines @@
   // CalculateDrawPropertiesInternal. If we, say, create a render surface, these
   // clip rects will want to be in its target space, not ours.
   if (clip_parent == layer->clip_parent()) {
     *clip_rect_in_parent_target_space = TranslateRectToTargetSpace<LayerType>(
         *clip_parent,
         *layer->parent(),
         *clip_rect_in_parent_target_space,
         TranslateRectDirectionToDescendant);
   } else {
     // If we're being clipped by our scroll parent, we must translate through
-    // our common ancestor. This happens to be our parent, so it is sufficent to
-    // translate from our clip parent's space to the space of its ancestor (our
-    // parent).
+    // our common ancestor.
+    const LayerType* lca =
+        HighestScrollParentAncestorNotOnChildAncestorChain(
+            layer->draw_properties().sequence_number, layer, clip_parent)
+            ->parent();
+
     *clip_rect_in_parent_target_space =
-        TranslateRectToTargetSpace<LayerType>(*layer->parent(),
+        TranslateRectToTargetSpace<LayerType>(*lca,
                                               *clip_parent,
                                               *clip_rect_in_parent_target_space,
                                               TranslateRectDirectionToAncestor);
+
+    // The lowest common ancestor is usually, but not always, our parent. We
+    // must check if we need to transform from the lca's space to our parent's.
+    if (lca != layer->parent()) {
+      *clip_rect_in_parent_target_space = TranslateRectToTargetSpace<LayerType>(
+          *lca,
+          *layer->parent(),
+          *clip_rect_in_parent_target_space,
+          TranslateRectDirectionToDescendant);
+    }
   }
 }
 
 // We collect an accumulated drawable content rect per render surface.
 // Typically, a layer will contribute to only one surface, the surface
 // associated with its render target. Clip children, however, may affect
 // several surfaces since there may be several surfaces between the clip child
 // and its parent.
 //
 // NB: we accumulate the layer's *clipped* drawable content rect.
@@ skipping 925 matching lines @@
        it != layer_list->end();
        ++it) {
     MarkLayerWithRenderSurfaceLayerListId(*it,
                                           current_render_surface_layer_list_id);
     MarkMasksWithRenderSurfaceLayerListId(*it,
                                           current_render_surface_layer_list_id);
   }
 }
 
 template <typename LayerType>
+static inline bool HaveVisited(LayerType* layer, size_t sequence_number) {
+  return layer->draw_properties().sequence_number == sequence_number;
+}
+
+template <typename LayerType>
+static inline bool HaveAssignedSortWeight(LayerType* layer,
+                                          size_t sequence_number) {
+  return layer->draw_properties().sort_weight_sequence_number ==
+         sequence_number;
+}
+
+template <typename LayerType>
 static inline void RemoveSurfaceForEarlyExit(
     LayerType* layer_to_remove,
     typename LayerType::RenderSurfaceListType* render_surface_layer_list) {
   DCHECK(layer_to_remove->render_surface());
   // Technically, we know that the layer we want to remove should be
   // at the back of the render_surface_layer_list. However, we have had
   // bugs before that added unnecessary layers here
   // (https://bugs.webkit.org/show_bug.cgi?id=74147), but that causes
   // things to crash. So here we proactively remove any additional
   // layers from the end of the list.
   while (render_surface_layer_list->back() != layer_to_remove) {
     MarkLayerListWithRenderSurfaceLayerListId(
         &render_surface_layer_list->back()->render_surface()->layer_list(), 0);
     MarkLayerWithRenderSurfaceLayerListId(render_surface_layer_list->back(), 0);
 
     render_surface_layer_list->back()->ClearRenderSurfaceLayerList();
     render_surface_layer_list->pop_back();
   }
   DCHECK_EQ(render_surface_layer_list->back(), layer_to_remove);
   MarkLayerListWithRenderSurfaceLayerListId(
       &layer_to_remove->render_surface()->layer_list(), 0);
   MarkLayerWithRenderSurfaceLayerListId(layer_to_remove, 0);
   render_surface_layer_list->pop_back();
   layer_to_remove->ClearRenderSurfaceLayerList();
 }
 
+template <typename LayerType>
+static inline LayerType* HighestScrollParentAncestorNotOnChildAncestorChain(
+    size_t sequence_number,
+    LayerType* scroll_child,
+    LayerType* scroll_parent) {
+  // Say we have the following tree.
+  //
+  // a
+  // +-b
+  // | +-c
+  // |   +-scroll_child
+  // +-d
+  //   +-e
+  //     +-scroll_parent
+  //
+  // This function aims to find find the highest ancestor of |scroll_parent|
+  // that's not on |scroll_child|'s ancestor chain. d, in this case.
+  //
+  // Fortunately, we have some hints that make finding the lowest common
+  // ancestor a bit easier than usual. We know if we've visited a layer, and for
+  // those layers that we've visited, we know their depth in the tree.
+  if (!HaveVisited(scroll_parent, sequence_number)) {
+    DCHECK(scroll_parent->parent());
+    // This case is easy. Since we walk the tree in DFS order, if we haven't
+    // visited the scroll parent, we just have to walk up to the first visited
+    // layer, which will be the LCA.
+    while (!HaveVisited(scroll_parent->parent(), sequence_number))
+      scroll_parent = scroll_parent->parent();
+    // This would mean that |scroll_parent| is a descendant of |scroll_child|,
+    // which is an error.
+    DCHECK_NE(scroll_child, scroll_parent->parent());
+  } else {
+    // In this case, we've visited both the scroll_parent and scroll_child, so
+    // it's not as easy as walking to the first visited ancestor of the scroll
+    // parent. But since we've visited the layers, we know their depths, and we
+    // can efficiently find the LCA by walking up nodes at the same height.
+
+    // First get the layers to the same depth.

[danakj, 2014/09/11 19:22:05]

nit: I think this comment was clearer before. Now it sounds like you're mutating
layers.

[Ian Vollick, 2014/09/11 19:58:08]

Yeah, I've switched it up for something that's hopefully clearer.

+    while (scroll_child->draw_properties().depth !=
+           scroll_parent->draw_properties().depth) {
+      int child_is_deeper = scroll_child->draw_properties().depth >
+                            scroll_parent->draw_properties().depth;
+      if (child_is_deeper)
+        scroll_child = scroll_child->parent();
+      else
+        scroll_parent = scroll_parent->parent();
+    }
+
+    // This would only happen if |scroll_child| and |scroll_parent| were on the
+    // same ancestor chain, which is an error. We set up scroll parent scroll
+    // child relationships in blink precisely because the scroll child is not a

[danakj, 2014/09/11 19:22:05]

oh thanks for this comment, that's helpful. but can you do this without
mentioning blink? like "scroll parents and children are only meant to be used
when..." or something

[Ian Vollick, 2014/09/11 19:58:08]

Done.

+    // descendant of the scroll parent and will therefore not scroll with it
+    // naturally and needs special treatment by the compositor.
+    DCHECK_NE(scroll_child, scroll_parent);
+
+    // Now that scroll_child and scroll_parent are at the same depth, we can
+    // walk up in tandem to find the LCA.
+    while (scroll_child->parent() != scroll_parent->parent()) {
+      scroll_child = scroll_child->parent();
+      scroll_parent = scroll_parent->parent();
+    }
+  }
+
+  return scroll_parent;
+}
+
 struct PreCalculateMetaInformationRecursiveData {
   bool layer_or_descendant_has_copy_request;
   bool layer_or_descendant_has_input_handler;
   int num_unclipped_descendants;
 
   PreCalculateMetaInformationRecursiveData()
       : layer_or_descendant_has_copy_request(false),
         layer_or_descendant_has_input_handler(false),
         num_unclipped_descendants(0) {}
 
   void Merge(const PreCalculateMetaInformationRecursiveData& data) {
     layer_or_descendant_has_copy_request |=
         data.layer_or_descendant_has_copy_request;
     layer_or_descendant_has_input_handler |=
         data.layer_or_descendant_has_input_handler;
     num_unclipped_descendants +=
         data.num_unclipped_descendants;
   }
 };
 
+static int next_sort_weight(int* last_sort_weight) {
+  return ++(*last_sort_weight);
+}
+
+template <typename LayerType>
+static inline void AssignSortWeights(LayerType* layer,
+                                     size_t sequence_number,
+                                     int* last_sort_weight) {
+  // Give |layer| the highest sort weight in the tree so far.
+  if (!HaveAssignedSortWeight(layer, sequence_number)) {
+    layer->draw_properties().sort_weight = next_sort_weight(last_sort_weight);
+    layer->draw_properties().sort_weight_sequence_number = sequence_number;
+  }
+
+  if (LayerType* scroll_parent = layer->scroll_parent()) {
+    // It's important to note where we need to take care of ordering so that
+    // scroll parents are visited before scroll children; it's at their lowest
+    // common ancestor.
+    //
+    // LCA
+    // +-scroll_parent_ancestor
+    // | +- ...
+    // |   + scroll_parent
+    // |
+    // +-scroll_child_ancestor
+    //   +- ..
+    //     + scroll_child
+    //
+    // So given the above tree, we need to guarentee that scroll_parent_ancestor
+    // is processed before scroll_child. We ensure that by assigning it the
+    // lowest sort weight in the tree. We also want to flag LCA if we need to do
+    // any funny business with sort weights so that it knows to do special
+    // sorting of its descendants.
+    LayerType* scroll_parent_ancestor =
+        HighestScrollParentAncestorNotOnChildAncestorChain(
+            sequence_number, layer, scroll_parent);
+
+    // We know we need to reorder if we haven't yet assigned
+    // |scroll_parent_ancestor| a sort weight. Sort weights are monotonically
+    // increasing, so unless we intervene, it will be given a higher sort weight
+    // than |layer|.
+    bool need_to_reorder =
+        !HaveAssignedSortWeight(scroll_parent_ancestor, sequence_number);
+
+    LayerType* lca = scroll_parent_ancestor->parent();
+
+    // We also might need to reorder if scroll_parent_ancestor has been visited
+    // and has an unacceptable sort weight.
+    if (!need_to_reorder) {
+      LayerType* scroll_child_ancestor = layer;
+      while (scroll_child_ancestor->parent() &&
+             scroll_child_ancestor->parent() != lca) {
+        scroll_child_ancestor = scroll_child_ancestor->parent();
+      }
+      // We must have assigned a sort weight to this layer since its on our
+      // ancestor chain.
+      DCHECK(HaveAssignedSortWeight(scroll_child_ancestor, sequence_number));
+      if (scroll_child_ancestor->draw_properties().sort_weight <
+          scroll_parent_ancestor->draw_properties().sort_weight)
+        need_to_reorder = true;
+    }
+
+    if (need_to_reorder) {
+      // Give |scroll_parent_ancestor| the lowest weight in the tree so far.
+      scroll_parent_ancestor->draw_properties().sort_weight =
+          -next_sort_weight(last_sort_weight);
+      scroll_parent_ancestor->draw_properties().sort_weight_sequence_number =
+          sequence_number;
+
+      // Mark the LCA as needing to sort.
+      lca->draw_properties().children_need_sorting = true;
+    }
+  }
+}
+
 // Recursively walks the layer tree to compute any information that is needed
 // before doing the main recursion.
 template <typename LayerType>
 static void PreCalculateMetaInformation(
     LayerType* layer,
-    PreCalculateMetaInformationRecursiveData* recursive_data) {
-
-  layer->draw_properties().sorted_for_recursion = false;
-  layer->draw_properties().has_child_with_a_scroll_parent = false;
+    PreCalculateMetaInformationRecursiveData* recursive_data,
+    int depth,
+    size_t sequence_number,
+    int* last_sort_weight) {
+  layer->draw_properties().children_need_sorting = false;
+  layer->draw_properties().depth = depth;
+  layer->draw_properties().sequence_number = sequence_number;
 
   if (!HasInvertibleOrAnimatedTransform(layer)) {
     // Layers with singular transforms should not be drawn, the whole subtree
     // can be skipped.
     return;
   }
 
   if (layer->clip_parent())
     recursive_data->num_unclipped_descendants++;
 
+  AssignSortWeights(layer, sequence_number, last_sort_weight);
+
   for (size_t i = 0; i < layer->children().size(); ++i) {
     LayerType* child_layer =
         LayerTreeHostCommon::get_layer_as_raw_ptr(layer->children(), i);
 
     PreCalculateMetaInformationRecursiveData data_for_child;
-    PreCalculateMetaInformation(child_layer, &data_for_child);
-
-    if (child_layer->scroll_parent())
-      layer->draw_properties().has_child_with_a_scroll_parent = true;
+    PreCalculateMetaInformation(child_layer,
+                                &data_for_child,
+                                depth + 1,
+                                sequence_number,
+                                last_sort_weight);
     recursive_data->Merge(data_for_child);
   }
 
   if (layer->clip_children()) {
     int num_clip_children = layer->clip_children()->size();
     DCHECK_GE(recursive_data->num_unclipped_descendants, num_clip_children);
     recursive_data->num_unclipped_descendants -= num_clip_children;
   }
 
   if (layer->HasCopyRequest())
@@ skipping 66 matching lines @@
   bool ancestor_is_animating_scale;
   bool ancestor_clips_subtree;
   typename LayerType::RenderSurfaceType*
       nearest_occlusion_immune_ancestor_surface;
   bool in_subtree_of_page_scale_application_layer;
   bool subtree_can_use_lcd_text;
   bool subtree_is_visible_from_ancestor;
 };
 
 template <typename LayerType>
-static LayerType* GetChildContainingLayer(const LayerType& parent,
-                                          LayerType* layer) {
-  for (LayerType* ancestor = layer; ancestor; ancestor = ancestor->parent()) {
-    if (ancestor->parent() == &parent)
-      return ancestor;
+struct AscendingScrollWeightComparator {
+  bool operator()(LayerType* lhs, LayerType* rhs) {
+    return lhs->draw_properties().sort_weight <
+           rhs->draw_properties().sort_weight;
   }
-  NOTREACHED();
-  return 0;
+};
+
+template <typename LayerType>
+static void SortChildrenForRecursion(std::vector<LayerType*>* out,
+                                     const LayerType& parent) {
+  out->resize(parent.children().size());
+  for (size_t i = 0; i < parent.children().size(); ++i)
+    (*out)[i] = LayerTreeHostCommon::get_layer_as_raw_ptr(parent.children(), i);
+
+  // Ensures the children are sorted in ascending sort weight.
+  std::sort(
+      out->begin(), out->end(), AscendingScrollWeightComparator<LayerType>());
 }
 
 template <typename LayerType>
-static void AddScrollParentChain(std::vector<LayerType*>* out,
-                                 const LayerType& parent,
-                                 LayerType* layer) {
-  // At a high level, this function walks up the chain of scroll parents
-  // recursively, and once we reach the end of the chain, we add the child
-  // of |parent| containing each scroll ancestor as we unwind. The result is
-  // an ordering of parent's children that ensures that scroll parents are
-  // visited before their descendants.
-  // Take for example this layer tree:
-  //
-  // + stacking_context
-  //   + scroll_child (1)
-  //   + scroll_parent_graphics_layer (*)
-  //   | + scroll_parent_scrolling_layer
-  //   |   + scroll_parent_scrolling_content_layer (2)
-  //   + scroll_grandparent_graphics_layer (**)
-  //     + scroll_grandparent_scrolling_layer
-  //       + scroll_grandparent_scrolling_content_layer (3)
-  //
-  // The scroll child is (1), its scroll parent is (2) and its scroll
-  // grandparent is (3). Note, this doesn't mean that (2)'s scroll parent is
-  // (3), it means that (*)'s scroll parent is (3). We don't want our list to
-  // look like [ (3), (2), (1) ], even though that does have the ancestor chain
-  // in the right order. Instead, we want [ (**), (*), (1) ]. That is, only want
-  // (1)'s siblings in the list, but we want them to appear in such an order
-  // that the scroll ancestors get visited in the correct order.
-  //
-  // So our first task at this step of the recursion is to determine the layer
-  // that we will potentionally add to the list. That is, the child of parent
-  // containing |layer|.
-  LayerType* child = GetChildContainingLayer(parent, layer);
-  if (child->draw_properties().sorted_for_recursion)
-    return;
-
-  if (LayerType* scroll_parent = child->scroll_parent())
-    AddScrollParentChain(out, parent, scroll_parent);
-
-  out->push_back(child);
-  child->draw_properties().sorted_for_recursion = true;
-}
-
-template <typename LayerType>
-static bool SortChildrenForRecursion(std::vector<LayerType*>* out,
-                                     const LayerType& parent) {
-  out->reserve(parent.children().size());
-  bool order_changed = false;
-  for (size_t i = 0; i < parent.children().size(); ++i) {
-    LayerType* current =
-        LayerTreeHostCommon::get_layer_as_raw_ptr(parent.children(), i);
-
-    if (current->draw_properties().sorted_for_recursion) {
-      order_changed = true;
-      continue;
-    }
-
-    AddScrollParentChain(out, parent, current);
-  }
-
-  DCHECK_EQ(parent.children().size(), out->size());
-  return order_changed;
-}
-
-template <typename LayerType>
 static void GetNewDescendantsStartIndexAndCount(LayerType* layer,
                                                 size_t* start_index,
                                                 size_t* count) {
   *start_index = layer->draw_properties().index_of_first_descendants_addition;
   *count = layer->draw_properties().num_descendants_added;
 }
 
 template <typename LayerType>
 static void GetNewRenderSurfacesStartIndexAndCount(LayerType* layer,
                                                    size_t* start_index,
@@ skipping 279 matching lines @@
     // 3.0, then 1 physical pixel is only 1/3 of a CSS pixel.
     gfx::Vector2dF parent_scales = MathUtil::ComputeTransform2dScaleComponents(
         data_from_ancestor.parent_matrix, 1.f);
     effective_scroll_delta -=
         gfx::ScaleVector2d(current_translation - previous_translation,
                            1.f / parent_scales.x(),
                            1.f / parent_scales.y());
   }
 
   // Apply adjustment from position constraints.
-  ApplyPositionAdjustment(layer, data_from_ancestor.fixed_container,
-      data_from_ancestor.scroll_compensation_matrix, &combined_transform);
+  ApplyPositionAdjustment(layer,
+                          data_from_ancestor.fixed_container,
+                          data_from_ancestor.scroll_compensation_matrix,
+                          &combined_transform);
 
   bool combined_is_animating_scale = false;
   float combined_maximum_animation_contents_scale = 0.f;
   if (globals.can_adjust_raster_scales) {
     CalculateAnimationContentsScale(
         layer,
         data_from_ancestor.ancestor_is_animating_scale,
         data_from_ancestor.maximum_animation_contents_scale,
         data_from_ancestor.parent_matrix,
         combined_transform,
@@ skipping 80 matching lines @@
     layer_draw_properties.screen_space_transform.FlattenTo2d();
   layer_draw_properties.screen_space_transform.PreconcatTransform
       (layer_draw_properties.target_space_transform);
 
   // Adjusting text AA method during animation may cause repaints, which in-turn
   // causes jank.
   bool adjust_text_aa =
       !animating_opacity_to_screen && !animating_transform_to_screen;
   // To avoid color fringing, LCD text should only be used on opaque layers with
   // just integral translation.
-  bool layer_can_use_lcd_text =
-      data_from_ancestor.subtree_can_use_lcd_text &&
-      accumulated_draw_opacity == 1.f &&
-      layer_draw_properties.target_space_transform.
-          IsIdentityOrIntegerTranslation();
+  bool layer_can_use_lcd_text = data_from_ancestor.subtree_can_use_lcd_text &&
+                                accumulated_draw_opacity == 1.f &&
+                                layer_draw_properties.target_space_transform
+                                    .IsIdentityOrIntegerTranslation();
 
   gfx::Rect content_rect(layer->content_bounds());
 
   // full_hierarchy_matrix is the matrix that transforms objects between screen
   // space (except projection matrix) and the most recent RenderSurfaceImpl's
   // space.  next_hierarchy_matrix will only change if this layer uses a new
   // RenderSurfaceImpl, otherwise remains the same.
   data_for_children.full_hierarchy_matrix =
       data_from_ancestor.full_hierarchy_matrix;
 
   // If the subtree will scale layer contents by the transform hierarchy, then
   // we should scale things into the render surface by the transform hierarchy
   // to take advantage of that.
   gfx::Vector2dF render_surface_sublayer_scale =
       globals.can_adjust_raster_scales
       ? combined_transform_scales
       : gfx::Vector2dF(layer_scale_factors, layer_scale_factors);
 
   bool render_to_separate_surface;
   if (globals.can_render_to_separate_surface) {
     render_to_separate_surface = SubtreeShouldRenderToSeparateSurface(
-          layer, combined_transform.Preserves2dAxisAlignment());
+        layer, combined_transform.Preserves2dAxisAlignment());

[danakj, 2014/09/11 19:22:05]

these unrelated format changes may make future merging more difficuly

[Ian Vollick, 2014/09/11 19:58:08]

I put 'em back. (This was the work of git cl format)

   } else {
     render_to_separate_surface = IsRootLayer(layer);
   }
   if (render_to_separate_surface) {
     // Check back-face visibility before continuing with this surface and its
     // subtree
     if (!layer->double_sided() && TransformToParentIsKnown(layer) &&
         IsSurfaceBackFaceVisible(layer, combined_transform)) {
       layer->ClearRenderSurfaceLayerList();
       return;
@@ skipping 29 matching lines @@
                 render_surface_sublayer_scale.y() / layer->contents_scale_y());
 
       // Inside the surface's subtree, we scale everything to the owning layer's
       // scale.  The sublayer matrix transforms layer rects into target surface
       // content space.  Conceptually, all layers in the subtree inherit the
       // scale at the point of the render surface in the transform hierarchy,
       // but we apply it explicitly to the owning layer and the remainder of the
       // subtree independently.
       DCHECK(data_for_children.parent_matrix.IsIdentity());
       data_for_children.parent_matrix.Scale(render_surface_sublayer_scale.x(),
-                            render_surface_sublayer_scale.y());
+                                            render_surface_sublayer_scale.y());
 
       // Even if the |layer_is_drawn|, it only contributes to a drawn surface
       // when the |layer_is_visible|.
       layer->render_surface()->set_contributes_to_drawn_surface(
           layer_is_visible);
     }
 
     // The opacity value is moved from the layer to its surface, so that the
     // entire subtree properly inherits opacity.
     render_surface->SetDrawOpacity(accumulated_draw_opacity);
@@ skipping 210 matching lines @@
     if (layer->should_flatten_transform())
       data_for_children.parent_matrix.FlattenTo2d();
 
     data_for_children.scroll_compensation_matrix =
         ComputeScrollCompensationMatrixForChildren(
             layer,
             data_from_ancestor.parent_matrix,
             data_from_ancestor.scroll_compensation_matrix,
             effective_scroll_delta);
     data_for_children.fixed_container =
-        layer->IsContainerForFixedPositionLayers() ?
-            layer : data_from_ancestor.fixed_container;
+        layer->IsContainerForFixedPositionLayers()
+            ? layer
+            : data_from_ancestor.fixed_container;
 
     data_for_children.clip_rect_in_target_space = clip_rect_in_target_space;
     data_for_children.clip_rect_of_target_surface_in_target_space =
         clip_rect_of_target_surface_in_target_space;
     data_for_children.ancestor_clips_subtree =
         layer_or_ancestor_clips_descendants;
     data_for_children.nearest_occlusion_immune_ancestor_surface =
         nearest_occlusion_immune_ancestor_surface;
     data_for_children.subtree_is_visible_from_ancestor = layer_is_drawn;
   }
 
   std::vector<LayerType*> sorted_children;
-  bool child_order_changed = false;
-  if (layer_draw_properties.has_child_with_a_scroll_parent)
-    child_order_changed = SortChildrenForRecursion(&sorted_children, *layer);
+  if (layer_draw_properties.children_need_sorting)
+    SortChildrenForRecursion(&sorted_children, *layer);
 
   for (size_t i = 0; i < layer->children().size(); ++i) {
     // If one of layer's children has a scroll parent, then we may have to
     // visit the children out of order. The new order is stored in
     // sorted_children. Otherwise, we'll grab the child directly from the
     // layer's list of children.
     LayerType* child =
-        layer_draw_properties.has_child_with_a_scroll_parent
+        layer_draw_properties.children_need_sorting
             ? sorted_children[i]
             : LayerTreeHostCommon::get_layer_as_raw_ptr(layer->children(), i);
 
     child->draw_properties().index_of_first_descendants_addition =
         descendants.size();
     child->draw_properties().index_of_first_render_surface_layer_list_addition =
         render_surface_layer_list->size();
 
     CalculateDrawPropertiesInternal<LayerType>(
         child,
@@ skipping 17 matching lines @@
     child->draw_properties().num_descendants_added =
         descendants.size() -
         child->draw_properties().index_of_first_descendants_addition;
     child->draw_properties().num_render_surfaces_added =
         render_surface_layer_list->size() -
         child->draw_properties()
             .index_of_first_render_surface_layer_list_addition;
   }
 
   // Add the unsorted layer list contributions, if necessary.
-  if (child_order_changed) {
+  if (layer_draw_properties.children_need_sorting) {
     SortLayerListContributions(
         *layer,
         GetLayerListForSorting(render_surface_layer_list),
         render_surface_layer_list_child_sorting_start_index,
         &GetNewRenderSurfacesStartIndexAndCount<LayerType>);
 
     SortLayerListContributions(
         *layer,
         &descendants,
         layer_list_child_sorting_start_index,
@@ skipping 210 matching lines @@
   data_for_recursion->subtree_is_visible_from_ancestor = true;
 }
 
 void LayerTreeHostCommon::CalculateDrawProperties(
     CalcDrawPropsMainInputs* inputs) {
   LayerList dummy_layer_list;
   SubtreeGlobals<Layer> globals;
   DataForRecursion<Layer> data_for_recursion;
   ProcessCalcDrawPropsInputs(*inputs, &globals, &data_for_recursion);
 
+  // This is used as a global throughout the precalculate recursion.
+  int last_sort_weight = 0;
   PreCalculateMetaInformationRecursiveData recursive_data;
-  PreCalculateMetaInformation(inputs->root_layer, &recursive_data);
+  PreCalculateMetaInformation(inputs->root_layer,
+                              &recursive_data,
+                              0,
+                              inputs->current_render_surface_layer_list_id + 1,

[danakj, 2014/09/11 19:22:05]

why +1?

[Ian Vollick, 2014/09/11 19:58:08]

0 doesn't work as a sequence number. Although no real caller of this fn passes 0
as a RSLL-id, the tests did. I'd added one here to avoid test churn, but it
turned out to be trivial to update the tests, so I've just done that.

+                              &last_sort_weight);
+
   std::vector<AccumulatedSurfaceState<Layer> > accumulated_surface_state;
   CalculateDrawPropertiesInternal<Layer>(
       inputs->root_layer,
       globals,
       data_for_recursion,
       inputs->render_surface_layer_list,
       &dummy_layer_list,
       &accumulated_surface_state,
       inputs->current_render_surface_layer_list_id);
 
   // The dummy layer list should not have been used.
   DCHECK_EQ(0u, dummy_layer_list.size());
   // A root layer render_surface should always exist after
   // CalculateDrawProperties.
   DCHECK(inputs->root_layer->render_surface());
 }
 
 void LayerTreeHostCommon::CalculateDrawProperties(
     CalcDrawPropsImplInputs* inputs) {
   LayerImplList dummy_layer_list;
   SubtreeGlobals<LayerImpl> globals;
   DataForRecursion<LayerImpl> data_for_recursion;
   ProcessCalcDrawPropsInputs(*inputs, &globals, &data_for_recursion);
 
   LayerSorter layer_sorter;
   globals.layer_sorter = &layer_sorter;
 
+  // This is used as a global throughout the precalculate recursion.
+  int last_sort_weight = 0;
   PreCalculateMetaInformationRecursiveData recursive_data;
-  PreCalculateMetaInformation(inputs->root_layer, &recursive_data);
+  PreCalculateMetaInformation(inputs->root_layer,
+                              &recursive_data,
+                              0,
+                              inputs->current_render_surface_layer_list_id + 1,

[danakj, 2014/09/11 19:22:05]

why +1?

[Ian Vollick, 2014/09/11 19:58:08]

Zero doesn't work here. In practice, no real caller of CalcDrawProps passes
zero, but the tests do. If you'd prefer that I modify the tests and add an
assertion, I'd be happy to do that.

+                              &last_sort_weight);
+
   std::vector<AccumulatedSurfaceState<LayerImpl> >
       accumulated_surface_state;
   CalculateDrawPropertiesInternal<LayerImpl>(
       inputs->root_layer,
       globals,
       data_for_recursion,
       inputs->render_surface_layer_list,
       &dummy_layer_list,
       &accumulated_surface_state,
       inputs->current_render_surface_layer_list_id);
 
   // The dummy layer list should not have been used.
   DCHECK_EQ(0u, dummy_layer_list.size());
   // A root layer render_surface should always exist after
   // CalculateDrawProperties.
   DCHECK(inputs->root_layer->render_surface());
 }
 
 }  // namespace cc