Issue 1872003003: Revert of cc : Remove subtree skipping in LayerTreeHostCommon

Issue 1872003003: Revert of cc : Remove subtree skipping in LayerTreeHostCommon (Closed)

Created:
4 years, 8 months ago by jaydasika

Modified:
4 years, 8 months ago

Reviewers:
Ian Vollick, ajuma, weiliangc

CC:
chromium-reviews, cc-bugs_chromium.org

Base URL:
https://chromium.googlesource.com/chromium/src.git@master

Target Ref:
refs/pending/heads/master

Project:
chromium

Visibility:
Public.

More Reviews

Description

Revert of cc : Remove subtree skipping in LayerTreeHostCommon (patchset #5 id:80001 of https://codereview.chromium.org/1865693003/ ) Reason for revert: crbug.com/601812 Original issue's description: > cc : Remove subtree skipping in LayerTreeHostCommon > > This CL makes sure we visit all layers while calculating > render target and RSLL. This will help us in deleting code > in layer skipping that is there to handle subtree skipping. > > BUG=601088 > CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel > > Committed: https://crrev.com/5e83ea4d3b872053c0304bc81b71262041be8c3b > Cr-Commit-Position: refs/heads/master@{#385836} TBR=weiliangc@chromium.org,ajuma@chromium.org,vollick@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=601088, 601812 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Committed: https://crrev.com/fc1427651679c7035211920eeb01944593a6b636 Cr-Commit-Position: refs/heads/master@{#386159}

Patch Set 1 #

Patch Set 2 : Rebase #

Created: 4 years, 8 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+31 lines, -27 lines)			Patch
	M	cc/trees/layer_tree_host_common.cc	View	1	6 chunks	+31 lines, -27 lines	0 comments	Download

Messages

Total messages: 13 (7 generated)

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages

jaydasika

Created Revert of cc : Remove subtree skipping in LayerTreeHostCommon

4 years, 8 months ago (2016-04-08 18:44:48 UTC) #1

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1872003003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1872003003/1

4 years, 8 months ago (2016-04-08 18:45:14 UTC) #2

commit-bot: I haz the power

Failed to apply patch for cc/trees/layer_tree_host_common.cc: While running git apply --index -3 -p1; error: patch ...

4 years, 8 months ago (2016-04-08 18:46:51 UTC) #4

Failed to apply patch for cc/trees/layer_tree_host_common.cc:
While running git apply --index -3 -p1;
  error: patch failed: cc/trees/layer_tree_host_common.cc:541
  error: repository lacks the necessary blob to fall back on 3-way merge.
  error: cc/trees/layer_tree_host_common.cc: patch does not apply

Patch:       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
a57803084aadbbe60924ec5f70961998abbb4c37..fd636302fc6ae8ebfd21c4c7b5d988f0157f9324
100644
--- a/cc/trees/layer_tree_host_common.cc
+++ b/cc/trees/layer_tree_host_common.cc
@@ -541,28 +541,29 @@
       draw_property_utils::LayerShouldBeSkipped(
           layer, layer_is_drawn, property_trees->transform_tree)) {
     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()) << IsRootLayer(layer)
+                                    << can_render_to_separate_surface
+                                    << layer->has_render_surface();
+    layer->draw_properties().render_target = layer;
+
+    if (layer->mask_layer())
+      layer->mask_layer()->draw_properties().render_target = layer;
+
+    if (layer->replica_layer() && layer->replica_layer()->mask_layer())
+      layer->replica_layer()->mask_layer()->draw_properties().render_target =
+          layer;
+
   } else {
-    bool render_to_separate_surface =
-        IsRootLayer(layer) ||
-        (can_render_to_separate_surface && layer->render_surface());
-
-    if (render_to_separate_surface) {
-      DCHECK(layer->render_surface()) << IsRootLayer(layer)
-                                      << can_render_to_separate_surface
-                                      << layer->has_render_surface();
-      layer->draw_properties().render_target = layer;
-
-      if (layer->mask_layer())
-        layer->mask_layer()->draw_properties().render_target = layer;
-
-      if (layer->replica_layer() && layer->replica_layer()->mask_layer())
-        layer->replica_layer()->mask_layer()->draw_properties().render_target =
-            layer;
-
-    } else {
-      DCHECK(!IsRootLayer(layer));
-      layer->draw_properties().render_target =
layer->parent()->render_target();
-    }
+    DCHECK(!IsRootLayer(layer));
+    layer->draw_properties().render_target = layer->parent()->render_target();
   }
   for (size_t i = 0; i < layer->children().size(); ++i) {
     CalculateRenderTarget(
@@ -608,13 +609,6 @@
     if (layer->render_surface())
       layer->ClearRenderSurfaceLayerList();
     layer->draw_properties().render_target = nullptr;
-    for (auto* child_layer : layer->children()) {
-      CalculateRenderSurfaceLayerList(
-          child_layer, property_trees, render_surface_layer_list, descendants,
-          nearest_occlusion_immune_ancestor, layer_is_drawn,
-          can_render_to_separate_surface, current_render_surface_layer_list_id,
-          max_texture_size);
-    }
     return;
   }
 
@@ -661,10 +655,11 @@
     descendants = &(layer->render_surface()->layer_list());
   }
 
+  size_t descendants_size = descendants->size();
+
   bool layer_should_be_skipped = !draw_property_utils::LayerNeedsUpdate(
       layer, layer_is_drawn, property_trees->transform_tree);
   if (!layer_should_be_skipped) {
-    DCHECK(layer->draw_properties().render_target);
     MarkLayerWithRenderSurfaceLayerListId(layer,
                                          
current_render_surface_layer_list_id);
     descendants->push_back(layer);
@@ -743,7 +738,7 @@
       layer->render_surface()->SetContentRect(surface_content_rect);
     }
     const LayerImpl* parent_target = layer->parent()->render_target();
-    if (parent_target && !IsRootLayer(parent_target)) {
+    if (!IsRootLayer(parent_target)) {
       gfx::Rect surface_content_rect =
           parent_target->render_surface()->accumulated_content_rect();
       if (render_to_separate_surface) {
@@ -771,6 +766,13 @@
   if (render_to_separate_surface && !IsRootLayer(layer) &&
       layer->render_surface()->DrawableContentRect().IsEmpty()) {
     RemoveSurfaceForEarlyExit(layer, render_surface_layer_list);
+    return;
+  }
+
+  // If neither this layer nor any of its children were added, early out.
+  if (descendants_size == descendants->size()) {
+    DCHECK(!render_to_separate_surface || IsRootLayer(layer));
+    return;
   }
 }
 
@@ -871,6 +873,8 @@
   DCHECK(inputs->can_render_to_separate_surface ==
          inputs->property_trees->non_root_surfaces_enabled);
   const bool subtree_visible_from_ancestor = true;
+  for (auto* layer : *inputs->root_layer->layer_tree_impl())
+    layer->draw_properties().render_target = nullptr;
   CalculateRenderTarget(inputs->root_layer, inputs->property_trees,
                         subtree_visible_from_ancestor,
                         inputs->can_render_to_separate_surface);

jaydasika

Description was changed from ========== Revert of cc : Remove subtree skipping in LayerTreeHostCommon (patchset ...

4 years, 8 months ago (2016-04-08 19:00:18 UTC) #6

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1872003003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1872003003/40001

4 years, 8 months ago (2016-04-08 19:02:11 UTC) #8

jaydasika

Description was changed from ========== Revert of cc : Remove subtree skipping in LayerTreeHostCommon (patchset ...

4 years, 8 months ago (2016-04-08 19:02:16 UTC) #9

commit-bot: I haz the power

Description was changed from ========== Revert of cc : Remove subtree skipping in LayerTreeHostCommon (patchset ...

4 years, 8 months ago (2016-04-08 19:02:49 UTC) #10

Message was sent while issue was closed.

Description was changed from

==========
Revert of cc : Remove subtree skipping in LayerTreeHostCommon (patchset #5
id:80001 of https://codereview.chromium.org/1865693003/ )

Reason for revert:
crbug.com/601812

Original issue's description:
> cc : Remove subtree skipping in LayerTreeHostCommon
> 
> This CL makes sure we visit all layers while calculating
> render target and RSLL. This will help us in deleting code
> in layer skipping that is there to handle subtree skipping.
> 
> BUG=601088
> CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
> 
> Committed: https://crrev.com/5e83ea4d3b872053c0304bc81b71262041be8c3b
> Cr-Commit-Position: refs/heads/master@{#385836}

TBR=weiliangc@chromium.org,ajuma@chromium.org,vollick@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=601088, 601812
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Revert of cc : Remove subtree skipping in LayerTreeHostCommon (patchset #5
id:80001 of https://codereview.chromium.org/1865693003/ )

Reason for revert:
crbug.com/601812

Original issue's description:
> cc : Remove subtree skipping in LayerTreeHostCommon
> 
> This CL makes sure we visit all layers while calculating
> render target and RSLL. This will help us in deleting code
> in layer skipping that is there to handle subtree skipping.
> 
> BUG=601088
> CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
> 
> Committed: https://crrev.com/5e83ea4d3b872053c0304bc81b71262041be8c3b
> Cr-Commit-Position: refs/heads/master@{#385836}

TBR=weiliangc@chromium.org,ajuma@chromium.org,vollick@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=601088, 601812
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

commit-bot: I haz the power

Description was changed from ========== Revert of cc : Remove subtree skipping in LayerTreeHostCommon (patchset ...

4 years, 8 months ago (2016-04-08 19:04:03 UTC) #12

Message was sent while issue was closed.

Description was changed from

==========
Revert of cc : Remove subtree skipping in LayerTreeHostCommon (patchset #5
id:80001 of https://codereview.chromium.org/1865693003/ )

Reason for revert:
crbug.com/601812

Original issue's description:
> cc : Remove subtree skipping in LayerTreeHostCommon
> 
> This CL makes sure we visit all layers while calculating
> render target and RSLL. This will help us in deleting code
> in layer skipping that is there to handle subtree skipping.
> 
> BUG=601088
> CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
> 
> Committed: https://crrev.com/5e83ea4d3b872053c0304bc81b71262041be8c3b
> Cr-Commit-Position: refs/heads/master@{#385836}

TBR=weiliangc@chromium.org,ajuma@chromium.org,vollick@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=601088, 601812
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Revert of cc : Remove subtree skipping in LayerTreeHostCommon (patchset #5
id:80001 of https://codereview.chromium.org/1865693003/ )

Reason for revert:
crbug.com/601812

Original issue's description:
> cc : Remove subtree skipping in LayerTreeHostCommon
>
> This CL makes sure we visit all layers while calculating
> render target and RSLL. This will help us in deleting code
> in layer skipping that is there to handle subtree skipping.
>
> BUG=601088
> CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
>
> Committed: https://crrev.com/5e83ea4d3b872053c0304bc81b71262041be8c3b
> Cr-Commit-Position: refs/heads/master@{#385836}

TBR=weiliangc@chromium.org,ajuma@chromium.org,vollick@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=601088, 601812
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/fc1427651679c7035211920eeb01944593a6b636
Cr-Commit-Position: refs/heads/master@{#386159}
==========

commit-bot: I haz the power

4 years, 8 months ago (2016-04-08 19:04:04 UTC) #13

Message was sent while issue was closed.

Patchset 2 (id:??) landed as
https://crrev.com/fc1427651679c7035211920eeb01944593a6b636
Cr-Commit-Position: refs/heads/master@{#386159}

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages