cc::Fix computation of surface backfacing-ness when using property trees

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/trace_event/trace_event.h"
 #include "cc/base/math_util.h"
@@ skipping 1742 matching lines @@
       data_from_ancestor.full_hierarchy_matrix;
 
   bool render_to_separate_surface =
       IsRootLayer(layer) ||
       (globals.can_render_to_separate_surface && layer->render_surface());
 
   if (render_to_separate_surface) {
     DCHECK(layer->render_surface());
     // Check back-face visibility before continuing with this surface and its
     // subtree
+    RenderSurfaceImpl* render_surface = layer->render_surface();
     if (!layer->double_sided() &&
         IsSurfaceBackFaceVisible(layer, combined_transform)) {
+      gfx::Transform draw_transform = combined_transform;

[ajuma, 2015/11/10 00:17:15]

Please add a comment that we need this logic since the surface-skipping code we
use later depends on the surface's draw transform.

+      draw_transform.Scale(1.0 / combined_transform_scales.x(),
+                           1.0 / combined_transform_scales.y());
+      render_surface->SetDrawTransform(draw_transform);

[jaydasika, 2015/11/09 22:56:45]

CDP and PT render surface draw transform don't match without this change.

[ajuma, 2015/11/09 23:31:21]

Is this because the "skipped" surface is still winding up in the RSLL, and
having its draw transform compared? See the suggestion below for possibly fixing
this.

       return;
     }
 
-    RenderSurfaceImpl* render_surface = layer->render_surface();
-
     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
       // forward top-level transforms to the rest of the tree.
       data_for_children.parent_matrix = combined_transform;
     } else {
       // The owning layer's draw transform has a scale from content to layer
       // space which we do not want; so here we use the combined_transform
       // instead of the draw_transform. However, we do need to add a different
       // scale factor that accounts for the surface's pixel dimensions.
@@ skipping 691 matching lines @@
     layer->draw_properties().render_target = nullptr;
     return;
   }
 
   bool render_to_separate_surface =
       IsRootLayer(layer) ||
       (can_render_to_separate_surface && layer->render_surface());
 
   if (render_to_separate_surface) {
     DCHECK(layer->render_surface());
-    if (!layer->double_sided() &&
-        IsSurfaceBackFaceVisible(layer, layer->draw_transform())) {
-      layer->ClearRenderSurfaceLayerList();
-      layer->draw_properties().render_target = nullptr;
-      return;
-    }
 
     if (use_property_trees) {
       RenderSurfaceDrawProperties draw_properties;
       ComputeSurfaceDrawPropertiesUsingPropertyTrees(
           layer->render_surface(), property_trees, &draw_properties);
       // TODO(ajuma): Once property tree verification is removed, make the above
       // call directly set the surface's properties, so that the copying below
       // is no longer needed.
       layer->render_surface()->SetIsClipped(draw_properties.is_clipped);
       layer->render_surface()->SetDrawOpacity(draw_properties.draw_opacity);
       layer->render_surface()->SetDrawTransform(draw_properties.draw_transform);
       layer->render_surface()->SetScreenSpaceTransform(
           draw_properties.screen_space_transform);
       layer->render_surface()->SetReplicaDrawTransform(
           draw_properties.replica_draw_transform);
       layer->render_surface()->SetReplicaScreenSpaceTransform(
           draw_properties.replica_screen_space_transform);
       layer->render_surface()->SetClipRect(draw_properties.clip_rect);
     }
 
+    if (!layer->double_sided() &&
+        IsSurfaceBackFaceVisible(layer,
+                                 layer->render_surface()->draw_transform())) {
+      layer->ClearRenderSurfaceLayerList();
+      layer->draw_properties().render_target = nullptr;

[ajuma, 2015/11/09 23:31:21]

Would calling RemoveSurfaceForEarlyExit here fix the CDP/PT draw transform
mismatch that you mention above (without the need for the code added earlier
that computes that draw transform of a skipped surface)?

[jaydasika, 2015/11/10 00:06:40]

No that won't still fix the mismatch. We either need to add the code in CDP(at
previous comment) or call draw_transform of layer for !use_property_tree case
here.

[ajuma, 2015/11/10 00:17:15]

Oh, I think I understand: the old code relied on the fact that when we
early-return in the code at the previous comment, the owning layer's transform
doesn't get reparented by the surface.

+      return;
+    }
+
     if (IsRootLayer(layer)) {
       // The root surface does not contribute to any other surface, it has no
       // target.
       layer->render_surface()->set_contributes_to_drawn_surface(false);
     } else {
       // 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);
     }
@@ skipping 325 matching lines @@
 
 PropertyTrees* GetPropertyTrees(Layer* layer) {
   return layer->layer_tree_host()->property_trees();
 }
 
 PropertyTrees* GetPropertyTrees(LayerImpl* layer) {
   return layer->layer_tree_impl()->property_trees();
 }
 
 }  // namespace cc