cc : Fix occlusion tracking bug for contributing surface

We currently assume that the occlusion calculated for contributing
surface (the second last element of the stack in occlusion tracker) is
always the occlusion by its target. This is not true.

BUG=501135
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel

Review-Url: https://codereview.chromium.org/2147283002
Cr-Commit-Position: refs/heads/master@{#405611}

cc/trees/draw_property_utils.cc

@@ -54,21 +54,6 @@ static void ValidateRenderSurfaceForLayer(LayerImpl* layer) {
   DCHECK(effect_node->background_filters.IsEmpty());
 }
 
-#endif
-
-static void ApplySublayerScale(const int effect_node_id,
-                               const EffectTree& effect_tree,
-                               gfx::Transform* transform) {
-  const EffectNode* effect_node = effect_tree.Node(effect_node_id);
-  const EffectNode* target_effect_node =
-      effect_node->has_render_surface
-          ? effect_node
-          : effect_tree.Node(effect_node->target_id);
-  transform->matrix().postScale(target_effect_node->sublayer_scale.x(),
-                                target_effect_node->sublayer_scale.y(), 1.f);
-}
-
-#if DCHECK_IS_ON()
 void VerifySublayerScalesMatch(const int effect_node_id,
                                const int target_transform_id,
                                const EffectTree& effect_tree,
@@ -108,7 +93,7 @@ bool ComputeClipRectInTargetSpace(const LayerType* layer,
                                         target_node_id, &clip_to_target)) {
       // We don't have to apply sublayer scale when target is root.
       if (target_node_id != 0) {
-        ApplySublayerScale(layer->effect_tree_index(), effect_tree,
+        PostConcatSublayerScale(layer->effect_tree_index(), effect_tree,
                                 &clip_to_target);
 #if DCHECK_IS_ON()
         VerifySublayerScalesMatch(layer->effect_tree_index(), target_node_id,
@@ -177,7 +162,7 @@ static ConditionalClip ComputeLocalRectInTargetSpace(
     return ConditionalClip{false, gfx::RectF()};
   // We don't have to apply sublayer scale when target is root.
   if (target_transform_id != 0) {
-    ApplySublayerScale(target_effect_id, effect_tree, &current_to_target);
+    PostConcatSublayerScale(target_effect_id, effect_tree, &current_to_target);
 #if DCHECK_IS_ON()
     VerifySublayerScalesMatch(target_effect_id, target_transform_id,
                               effect_tree, transform_tree);
@@ -715,6 +700,30 @@ static void ResetIfHasNanCoordinate(gfx::RectF* rect) {
     *rect = gfx::RectF();
 }
 
+void PostConcatSublayerScale(const int effect_node_id,
+                             const EffectTree& effect_tree,
+                             gfx::Transform* transform) {
+  // TODO(jaydasika): This function should not compute target effect node id. It
+  // should receive it from callers.
+  const EffectNode* effect_node = effect_tree.Node(effect_node_id);
+  const EffectNode* target_effect_node =
+      effect_node->has_render_surface
+          ? effect_node
+          : effect_tree.Node(effect_node->target_id);
+  transform->matrix().postScale(target_effect_node->sublayer_scale.x(),
+                                target_effect_node->sublayer_scale.y(), 1.f);
+}
+
+void ConcatInverseSublayerScale(const int effect_node_id,
+                                const EffectTree& effect_tree,
+                                gfx::Transform* transform) {
+  const EffectNode* effect_node = effect_tree.Node(effect_node_id);
+  if (effect_node->sublayer_scale.x() != 0.0 &&
+      effect_node->sublayer_scale.y() != 0.0)
+    transform->Scale(1.0 / effect_node->sublayer_scale.x(),
+                     1.0 / effect_node->sublayer_scale.y());
+}
+
 void ComputeClips(ClipTree* clip_tree,
                   const TransformTree& transform_tree,
                   const EffectTree& effect_tree,
@@ -761,7 +770,7 @@ void ComputeClips(ClipTree* clip_tree,
           &parent_to_current);
       // We don't have to apply sublayer scale when target is root.
       if (clip_node->target_transform_id != 0) {
-        ApplySublayerScale(clip_node->target_effect_id, effect_tree,
+        PostConcatSublayerScale(clip_node->target_effect_id, effect_tree,
                                 &parent_to_current);
 #if DCHECK_IS_ON()
         VerifySublayerScalesMatch(clip_node->target_effect_id,
@@ -835,7 +844,7 @@ void ComputeClips(ClipTree* clip_tree,
             &source_to_target);
         // We don't have to apply sublayer scale when target is root.
         if (clip_node->target_transform_id != 0) {
-          ApplySublayerScale(clip_node->target_effect_id, effect_tree,
+          PostConcatSublayerScale(clip_node->target_effect_id, effect_tree,
                                   &source_to_target);
 #if DCHECK_IS_ON()
           VerifySublayerScalesMatch(clip_node->target_effect_id,
@@ -997,8 +1006,8 @@ static void VerifyDrawTransformsMatch(LayerImpl* layer,
                                                   &draw_transform);
   // We don't have to apply sublayer scale when target is root.
   if (destination_id != 0) {
-    ApplySublayerScale(layer->effect_tree_index(), property_trees->effect_tree,
-                       &draw_transform);
+    PostConcatSublayerScale(layer->effect_tree_index(),
+                            property_trees->effect_tree, &draw_transform);
 #if DCHECK_IS_ON()
     VerifySublayerScalesMatch(layer->effect_tree_index(), destination_id,
                               property_trees->effect_tree,
@@ -1194,7 +1203,7 @@ static void SetSurfaceDrawTransform(const TransformTree& transform_tree,
                                   &render_surface_transform);
   // We don't have to apply sublayer scale when target is root.
   if (target_transform_node->id != 0) {
-    ApplySublayerScale(effect_node->target_id, effect_tree,
+    PostConcatSublayerScale(effect_node->target_id, effect_tree,
                             &render_surface_transform);
 #if DCHECK_IS_ON()
     VerifySublayerScalesMatch(effect_node->target_id, target_transform_node->id,
@@ -1252,7 +1261,7 @@ static void SetSurfaceClipRect(const ClipNode* parent_clip_node,
 
   // We don't have to apply sublayer scale when target is root.
   if (transform_tree.TargetId(transform_node->id) != 0) {
-    ApplySublayerScale(render_surface->EffectTreeIndex(), effect_tree,
+    PostConcatSublayerScale(render_surface->EffectTreeIndex(), effect_tree,
                             &clip_parent_target_to_target);
 #if DCHECK_IS_ON()
     VerifySublayerScalesMatch(render_surface->EffectTreeIndex(),

cc/trees/draw_property_utils.h

@@ -28,6 +28,13 @@ class PropertyTrees;
 
 namespace draw_property_utils {
 
+void CC_EXPORT PostConcatSublayerScale(const int effect_node_id,
+                                       const EffectTree& effect_tree,
+                                       gfx::Transform* transform);
+
+void CC_EXPORT ConcatInverseSublayerScale(const int effect_node_id,
+                                          const EffectTree& effect_tree,
+                                          gfx::Transform* transform);
 // Computes combined clips for every node in |clip_tree|. This function requires
 // that |transform_tree| has been updated via |ComputeTransforms|.
 void CC_EXPORT ComputeClips(ClipTree* clip_tree,

cc/trees/layer_tree_host_common_unittest.cc

@@ -25,6 +25,7 @@
 #include "cc/layers/layer_impl.h"
 #include "cc/layers/layer_iterator.h"
 #include "cc/layers/render_surface_impl.h"
+#include "cc/layers/texture_layer_impl.h"
 #include "cc/output/copy_output_request.h"
 #include "cc/output/copy_output_result.h"
 #include "cc/proto/begin_main_frame_and_commit_state.pb.h"
@@ -3683,6 +3684,93 @@ TEST_F(LayerTreeHostCommonTest,
   EXPECT_EQ(gfx::Rect(), grand_child->visible_layer_rect());
 }
 
+TEST_F(LayerTreeHostCommonTest, OcclusionBySiblingOfTarget) {
+  FakeImplTaskRunnerProvider task_runner_provider;
+  TestSharedBitmapManager shared_bitmap_manager;
+  TestTaskGraphRunner task_graph_runner;
+  std::unique_ptr<OutputSurface> output_surface = FakeOutputSurface::Create3d();
+  FakeLayerTreeHostImpl host_impl(&task_runner_provider, &shared_bitmap_manager,
+                                  &task_graph_runner);
+
+  std::unique_ptr<LayerImpl> root =
+      LayerImpl::Create(host_impl.active_tree(), 1);
+  std::unique_ptr<LayerImpl> child =
+      LayerImpl::Create(host_impl.active_tree(), 2);
+  std::unique_ptr<TextureLayerImpl> surface =
+      TextureLayerImpl::Create(host_impl.active_tree(), 3);
+  std::unique_ptr<TextureLayerImpl> surface_child =
+      TextureLayerImpl::Create(host_impl.active_tree(), 4);
+  std::unique_ptr<TextureLayerImpl> surface_sibling =
+      TextureLayerImpl::Create(host_impl.active_tree(), 5);
+  std::unique_ptr<TextureLayerImpl> surface_child_mask =
+      TextureLayerImpl::Create(host_impl.active_tree(), 6);
+
+  surface->SetDrawsContent(true);
+  surface_child->SetDrawsContent(true);
+  surface_sibling->SetDrawsContent(true);
+  surface_child_mask->SetDrawsContent(true);
+  surface->SetContentsOpaque(true);
+  surface_child->SetContentsOpaque(true);
+  surface_sibling->SetContentsOpaque(true);
+  surface_child_mask->SetContentsOpaque(true);
+
+  surface->test_properties()->opacity = 0.5f;
+  surface_child->test_properties()->opacity = 0.6f;
+
+  gfx::Transform identity_matrix;
+  gfx::Transform translate;
+  translate.Translate(20.f, 20.f);
+
+  SetLayerPropertiesForTesting(root.get(), identity_matrix, gfx::Point3F(),
+                               gfx::PointF(), gfx::Size(1000, 1000), true,
+                               false, true);
+  SetLayerPropertiesForTesting(child.get(), identity_matrix, gfx::Point3F(),
+                               gfx::PointF(), gfx::Size(300, 300), false, true,
+                               false);
+  SetLayerPropertiesForTesting(surface.get(), translate, gfx::Point3F(),
+                               gfx::PointF(), gfx::Size(300, 300), false, true,
+                               true);
+  SetLayerPropertiesForTesting(surface_child.get(), identity_matrix,
+                               gfx::Point3F(), gfx::PointF(),
+                               gfx::Size(300, 300), false, false, true);
+  SetLayerPropertiesForTesting(surface_sibling.get(), identity_matrix,
+                               gfx::Point3F(), gfx::PointF(),
+                               gfx::Size(200, 200), false, false, false);
+
+  LayerImpl* surface_ptr = surface.get();
+  LayerImpl* surface_child_ptr = surface_child.get();
+  LayerImpl* surface_child_mask_ptr = surface_child_mask.get();
+
+  host_impl.SetViewportSize(root->bounds());
+
+  surface_child->test_properties()->SetMaskLayer(std::move(surface_child_mask));
+  surface->test_properties()->AddChild(std::move(surface_child));
+  child->test_properties()->AddChild(std::move(surface));
+  child->test_properties()->AddChild(std::move(surface_sibling));
+  root->test_properties()->AddChild(std::move(child));
+  host_impl.active_tree()->SetRootLayerForTesting(std::move(root));
+  host_impl.SetVisible(true);
+  host_impl.InitializeRenderer(output_surface.get());
+  host_impl.active_tree()->BuildLayerListAndPropertyTreesForTesting();
+  bool update_lcd_text = false;
+  host_impl.active_tree()->UpdateDrawProperties(update_lcd_text);
+
+  EXPECT_TRANSFORMATION_MATRIX_EQ(
+      surface_ptr->render_surface()->draw_transform(), translate);
+  // surface_sibling draws into the root render surface and occludes
+  // surface_child's contents.
+  Occlusion actual_occlusion =
+      surface_child_ptr->render_surface()->occlusion_in_content_space();
+  Occlusion expected_occlusion(translate, SimpleEnclosedRegion(gfx::Rect()),
+                               SimpleEnclosedRegion(gfx::Rect(200, 200)));
+  EXPECT_TRUE(expected_occlusion.IsEqual(actual_occlusion));
+
+  // Mask layer should have the same occlusion.
+  actual_occlusion =
+      surface_child_mask_ptr->draw_properties().occlusion_in_content_space;
+  EXPECT_TRUE(expected_occlusion.IsEqual(actual_occlusion));
+}
+
 TEST_F(LayerTreeHostCommonTest,
        OcclusionForLayerWithUninvertibleDrawTransform) {
   FakeImplTaskRunnerProvider task_runner_provider;
@@ -3691,6 +3779,7 @@ TEST_F(LayerTreeHostCommonTest,
   std::unique_ptr<OutputSurface> output_surface = FakeOutputSurface::Create3d();
   FakeLayerTreeHostImpl host_impl(&task_runner_provider, &shared_bitmap_manager,
                                   &task_graph_runner);
+
   std::unique_ptr<LayerImpl> root =
       LayerImpl::Create(host_impl.active_tree(), 1);
   std::unique_ptr<LayerImpl> child =

cc/trees/layer_tree_impl.cc

@@ -958,9 +958,30 @@ bool LayerTreeImpl::UpdateDrawProperties(bool update_lcd_text) {
       if (it.represents_contributing_render_surface()) {
         // Surfaces aren't used by the tile raster code, so they can have
         // occlusion regardless of replicas.
+        const RenderSurfaceImpl* occlusion_surface =
+            occlusion_tracker.OcclusionSurfaceForContributingSurface();
+        gfx::Transform draw_transform;
+        if (occlusion_surface) {
+          // We are calculating transform between two render surfaces. So, we
+          // need to apply the sublayer scale at target and remove the sublayer
+          // scale at source.
+          property_trees()->transform_tree.ComputeTransform(
+              it->render_surface()->TransformTreeIndex(),
+              occlusion_surface->TransformTreeIndex(), &draw_transform);
+          // We don't have to apply sublayer scale when target is root.
+          if (occlusion_surface->TransformTreeIndex() != 0) {
+            draw_property_utils::PostConcatSublayerScale(
+                occlusion_surface->EffectTreeIndex(),
+                property_trees()->effect_tree, &draw_transform);
+          }
+          draw_property_utils::ConcatInverseSublayerScale(
+              it->render_surface()->EffectTreeIndex(),
+              property_trees()->effect_tree, &draw_transform);
+        }
+
         Occlusion occlusion =
             occlusion_tracker.GetCurrentOcclusionForContributingSurface(
-                it->render_surface()->draw_transform());
+                draw_transform);
         it->render_surface()->set_occlusion_in_content_space(occlusion);
         // Masks are used to draw the contributing surface, so should have
         // the same occlusion as the surface (nothing inside the surface
@@ -970,8 +991,7 @@ bool LayerTreeImpl::UpdateDrawProperties(bool update_lcd_text) {
               inside_replica
                   ? Occlusion()
                   : occlusion_tracker.GetCurrentOcclusionForContributingSurface(
-                        it->render_surface()->draw_transform() *
-                        it->DrawTransform());
+                        draw_transform * it->DrawTransform());
           mask->draw_properties().occlusion_in_content_space = mask_occlusion;
         }
         if (LayerImpl* replica_mask =

cc/trees/occlusion_tracker.cc

@@ -48,6 +48,14 @@ Occlusion OcclusionTracker::GetCurrentOcclusionForContributingSurface(
                    second_last.occlusion_from_inside_target);
 }
 
+const RenderSurfaceImpl*
+OcclusionTracker::OcclusionSurfaceForContributingSurface() const {
+  // A contributing surface doesn't get occluded by things inside its own
+  // surface, so only things outside the surface can occlude it. That occlusion
+  // is found just below the top of the stack (if it exists).
+  return (stack_.size() < 2) ? nullptr : stack_[stack_.size() - 2].target;
+}
+
 void OcclusionTracker::EnterLayer(const LayerIteratorPosition& layer_iterator) {
   LayerImpl* render_target = layer_iterator.target_render_surface_layer;
 

cc/trees/occlusion_tracker.h

@@ -40,6 +40,7 @@ class CC_EXPORT OcclusionTracker {
   Occlusion GetCurrentOcclusionForContributingSurface(
       const gfx::Transform& draw_transform) const;
 
+  const RenderSurfaceImpl* OcclusionSurfaceForContributingSurface() const;
   // Called at the beginning of each step in the LayerIterator's front-to-back
   // traversal.
   void EnterLayer(const LayerIteratorPosition& layer_iterator);