Moving RenderSurface creation outside of CalcDrawProps

cc/trees/layer_tree_host_common.cc

Index: cc/trees/layer_tree_host_common.cc
diff --git a/cc/trees/layer_tree_host_common.cc b/cc/trees/layer_tree_host_common.cc
index 840a32b2ef4a59037f22e33e251c9edee72bc649..b3c6010874eb34f927354331eb236bba8cd10850 100644
--- a/cc/trees/layer_tree_host_common.cc
+++ b/cc/trees/layer_tree_host_common.cc
@@ -14,7 +14,9 @@
 #include "cc/layers/layer_iterator.h"
 #include "cc/layers/render_surface.h"
 #include "cc/layers/render_surface_impl.h"
+#include "cc/test/fake_layer_tree_host_impl.h"

[danakj, 2014/09/03 19:30:34]

including cc/test from a non-test file, this looks like a mistake?

[awoloszyn, 2014/09/09 15:31:36]

Done.

 #include "cc/trees/layer_sorter.h"
+#include "cc/trees/layer_tree_host.h"

[danakj, 2014/09/03 19:30:34]

why?

[awoloszyn, 2014/09/09 15:31:36]

This is no longer needed, was from earlier patch. Have removed.

 #include "cc/trees/layer_tree_impl.h"
 #include "ui/gfx/rect_conversions.h"
 #include "ui/gfx/transform.h"
@@ -548,9 +550,8 @@ static inline void SavePaintPropertiesLayer(Layer* layer) {
     layer->replica_layer()->mask_layer()->SavePaintProperties();
 }
 
-template <typename LayerType>
 static bool SubtreeShouldRenderToSeparateSurface(
-    LayerType* layer,
+    Layer* layer,
     bool axis_aligned_with_respect_to_parent) {
   //
   // A layer and its descendants should render onto a new RenderSurfaceImpl if
@@ -1106,18 +1107,6 @@ static inline void CalculateAnimationContentsScale(
       std::max(ancestor_transform_scales.x(), ancestor_transform_scales.y());
 }
 
-template <typename LayerType>
-static inline typename LayerType::RenderSurfaceType* CreateOrReuseRenderSurface(
-    LayerType* layer) {
-  if (!layer->render_surface()) {
-    layer->CreateRenderSurface();
-    return layer->render_surface();
-  }
-
-  layer->render_surface()->ClearLayerLists();
-  return layer->render_surface();
-}
-
 template <typename LayerTypePtr>
 static inline void MarkLayerWithRenderSurfaceLayerListId(
     LayerTypePtr layer,
@@ -1813,13 +1802,13 @@ static void CalculateDrawPropertiesInternal(
       ? 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());
-  } else {
-    render_to_separate_surface = IsRootLayer(layer);
-  }
+  bool render_to_separate_surface =
+      IsRootLayer(layer) ||
+      (globals.can_render_to_separate_surface && layer->render_surface());
+
+  DCHECK(globals.can_render_to_separate_surface || IsRootLayer(layer) ||

[danakj, 2014/09/03 19:30:34]

this looks a bit awkward, like it's just repeating back the above. i think we
can remove this and instead..

[awoloszyn, 2014/09/09 15:31:37]

Done.

+         !render_to_separate_surface);
+
   if (render_to_separate_surface) {
     // Check back-face visibility before continuing with this surface and its

[danakj, 2014/09/03 19:30:34]

DCHECK(layer->render_surface()) here

[awoloszyn, 2014/09/09 15:31:36]

Done.

     // subtree
@@ -1829,9 +1818,11 @@ static void CalculateDrawPropertiesInternal(
       return;
     }
 
+    layer_draw_properties.render_target = layer;

[danakj, 2014/09/03 19:30:34]

remove this, did this twice also on L1825

[awoloszyn, 2014/09/09 15:31:37]

Done.

     typename LayerType::RenderSurfaceType* render_surface =
-        CreateOrReuseRenderSurface(layer);
-
+        layer->render_surface();
+    render_surface->ClearLayerLists();
+    layer_draw_properties.render_target = layer;
     if (IsRootLayer(layer)) {
       // The root layer's render surface size is predetermined and so the root
       // layer can't directly support non-identity transforms.  It should just
@@ -2006,8 +1997,6 @@ static void CalculateDrawPropertiesInternal(
         animating_opacity_to_screen;
     data_for_children.parent_matrix = combined_transform;
 
-    layer->ClearRenderSurface();
-
     // Layers without render_surfaces directly inherit the ancestor's clip
     // status.
     layer_or_ancestor_clips_descendants = ancestor_clips_subtree;
@@ -2034,7 +2023,7 @@ static void CalculateDrawPropertiesInternal(
 
   if (LayerClipsSubtree(layer)) {
     layer_or_ancestor_clips_descendants = true;
-    if (ancestor_clips_subtree && !layer->render_surface()) {
+    if (ancestor_clips_subtree && !render_to_separate_surface) {
       // A layer without render surface shares the same target as its ancestor.
       clip_rect_in_target_space =
           ancestor_clip_rect_in_target_space;
@@ -2059,8 +2048,8 @@ static void CalculateDrawPropertiesInternal(
   }
 
   typename LayerType::LayerListType& descendants =
-      (layer->render_surface() ? layer->render_surface()->layer_list()
-                               : *layer_list);
+      (render_to_separate_surface ? layer->render_surface()->layer_list()
+                                  : *layer_list);
 
   // Any layers that are appended after this point are in the layer's subtree
   // and should be included in the sorting process.
@@ -2138,7 +2127,10 @@ static void CalculateDrawPropertiesInternal(
         &descendants,
         accumulated_surface_state,
         current_render_surface_layer_list_id);
-    if (child->render_surface() &&
+    // If our child has a render_target that is not our render_target
+    // then it has it's own surface

[danakj, 2014/09/03 19:30:34]

nit: period

[awoloszyn, 2014/09/09 15:31:36]

Done.

+    if (child->render_target() != layer->render_target() &&
+        child->render_surface() &&

[danakj, 2014/09/03 19:30:34]

the render_surface() must be true in that case, so no need to check it here?

[awoloszyn, 2014/09/09 15:31:36]

I have re-worked this to be a bit more clear. A child has a render_surface if
child->render_target() == child.

         !child->render_surface()->layer_list().empty() &&
         !child->render_surface()->content_rect().IsEmpty()) {
       // This child will contribute its render surface, which means
@@ -2177,12 +2169,12 @@ static void CalculateDrawPropertiesInternal(
   // target surface space).
   gfx::Rect local_drawable_content_rect_of_subtree =
       accumulated_surface_state->back().drawable_content_rect;
-  if (layer->render_surface()) {
+  if (render_to_separate_surface) {
     DCHECK(accumulated_surface_state->back().render_target == layer);
     accumulated_surface_state->pop_back();
   }
 
-  if (layer->render_surface() && !IsRootLayer(layer) &&
+  if (render_to_separate_surface && !IsRootLayer(layer) &&
       layer->render_surface()->layer_list().empty()) {
     RemoveSurfaceForEarlyExit(layer, render_surface_layer_list);
     return;
@@ -2208,10 +2200,10 @@ static void CalculateDrawPropertiesInternal(
   // one.
   if (IsRootLayer(layer)) {
     // The root layer's surface's content_rect is always the entire viewport.
-    DCHECK(layer->render_surface());
+    DCHECK(render_to_separate_surface);
     layer->render_surface()->SetContentRect(
         ancestor_clip_rect_in_target_space);
-  } else if (layer->render_surface()) {
+  } else if (render_to_separate_surface) {
     typename LayerType::RenderSurfaceType* render_surface =
         layer->render_surface();
     gfx::Rect clipped_content_rect = local_drawable_content_rect_of_subtree;
@@ -2305,7 +2297,7 @@ static void CalculateDrawPropertiesInternal(
 
   // If neither this layer nor any of its children were added, early out.
   if (sorting_start_index == descendants.size()) {
-    DCHECK(!layer->render_surface() || IsRootLayer(layer));
+    DCHECK(!render_to_separate_surface || IsRootLayer(layer));
     return;
   }
 
@@ -2383,8 +2375,48 @@ static void ProcessCalcDrawPropsInputs(
   data_for_recursion->subtree_is_visible_from_ancestor = true;
 }
 
+void CreateOrDestroyRenderSurface(Layer* layer,
+                                  bool can_render_to_separate_surface,
+                                  gfx::Transform* transform) {
+  if (IsRootLayer(layer) ||
+      (can_render_to_separate_surface &&
+       SubtreeShouldRenderToSeparateSurface(
+           layer,
+           transform->IsIdentity() || transform->Preserves2dAxisAlignment()))) {

[danakj, 2014/09/03 19:30:34]

nit: Preserves2dAxisAlignment should already return true for Identity?

[awoloszyn, 2014/09/09 15:31:37]

It does already return true for Identity. transform->IsIdentity will be a
cheaper check than transform->Preserves... although this is true only in the
case where SkMatrix44::getType() is already cached. I will remove the
IsIdentity, since it does not obviously provide us with an improvement.

+    transform->MakeIdentity();
+    if (!layer->render_surface()) {
+      layer->CreateRenderSurface();
+    }
+    layer->SetHasRenderSurface(true);
+    return;
+  }
+  layer->SetHasRenderSurface(false);
+  if (layer->render_surface())
+    layer->ClearRenderSurface();
+}
+
+static void CreateRenderSurfaces(Layer* layer,
+                                 bool can_render_to_separate_surface,
+                                 const gfx::Transform& parent_transform) {
+  if (!layer)
+    return;
+  gfx::Transform transformForChildren = layer->transform();
+  transformForChildren *= parent_transform;

[danakj, 2014/09/03 19:30:34]

This is not really the same thing as checking the combined_transform. That one
includes things like the layer position, and the transform origin, etc. Couldn't
those things change whether a rotation ends up causing the transform to be 2d
aligned or not? What do you think about that?

[awoloszyn, 2014/09/09 15:31:36]

2d offsets are not able to change the 2d axis aligment of an element. Rotations
will always rotate the rect by Xdeg regardless of where the origin or position
is. We will be unable to use this to determine the ACTUAL position, but we can
safely determine the axis-align-ness of the element.

+  CreateOrDestroyRenderSurface(
+      layer, can_render_to_separate_surface, &transformForChildren);
+
+  for (size_t i = 0; i < layer->children().size(); ++i) {
+    CreateRenderSurfaces(layer->children()[i],
+                         can_render_to_separate_surface,
+                         transformForChildren);
+  }
+}
+
 void LayerTreeHostCommon::CalculateDrawProperties(
     CalcDrawPropsMainInputs* inputs) {
+  CreateRenderSurfaces(inputs->root_layer,
+                       inputs->can_render_to_separate_surface,
+                       gfx::Transform());
   LayerList dummy_layer_list;
   SubtreeGlobals<Layer> globals;
   DataForRecursion<Layer> data_for_recursion;