Improve Aura overdraw by changing hole calculation

ui/gfx/compositor/layer.cc

Index: ui/gfx/compositor/layer.cc
diff --git a/ui/gfx/compositor/layer.cc b/ui/gfx/compositor/layer.cc
index bf63c3529dcb28000488afa62801794e7b2d2c4d..fa8eabf9eb445a47d9c4e0a6c3af942b5bcfaa55 100644
--- a/ui/gfx/compositor/layer.cc
+++ b/ui/gfx/compositor/layer.cc
@@ -314,53 +314,76 @@ void Layer::UpdateLayerCanvas() {
   SetCanvas(*canvas->GetSkCanvas(), draw_rect.origin());
 }
 
-void Layer::RecomputeHole() {
-  if (type_ == LAYER_HAS_NO_TEXTURE)
-    return;
-
-  // Reset to default.
+void Layer::ClearHoleRects() {
   hole_rect_ = gfx::Rect();
 
-  // Find the largest hole
-  for (size_t i = 0; i < children_.size(); ++i) {
-    // Ignore non-opaque and hidden children.
-    if (!children_[i]->IsCompletelyOpaque() || !children_[i]->visible_)
-      continue;
-
-    // Ignore children that aren't rotated by multiples of 90 degrees.
-    float degrees;
-    if (!InterpolatedTransform::FactorTRS(children_[i]->transform(),
-                                          NULL, &degrees, NULL) ||
-        !IsApproximateMultilpleOf(degrees, 90.0f))
-      continue;
-
-    // The reason why we don't just take the bounds and apply the transform is
-    // that the bounds encodes a position, too, so the effective transformation
-    // matrix is actually different that the one reported. As well, the bounds
-    // will not necessarily be at the origin.
-    gfx::Rect candidate_hole(children_[i]->bounds_.size());
-    ui::Transform transform = children_[i]->transform();
-    transform.ConcatTranslate(static_cast<float>(children_[i]->bounds_.x()),
-                              static_cast<float>(children_[i]->bounds_.y()));
-    transform.TransformRect(&candidate_hole);
-
-    // This layer might not contain the child (e.g., a portion of the child may
-    // be offscreen). Only the portion of the child that overlaps this layer is
-    // of any importance, so take the intersection.
-    candidate_hole = gfx::Rect(bounds().size()).Intersect(candidate_hole);
-
-    // Ensure we have the largest hole.
-    if (candidate_hole.size().GetArea() > hole_rect_.size().GetArea())
-      hole_rect_ = candidate_hole;
-  }
+  for (size_t i = 0; i < children_.size(); i++)
+    children_[i]->ClearHoleRects();
+}
 
-  // Free up texture memory if the hole fills bounds of layer.
-  if (!ShouldDraw() && !layer_updated_externally_)
-    texture_ = NULL;
+void Layer::GeneratePreorderTraversal(
+    std::vector<ui::Layer*>* layer_traversal) {
+  if (!visible_)
+    return;
+
+  if (fills_bounds_opaquely_ && type_ == LAYER_HAS_TEXTURE)
+    layer_traversal->push_back(this);
+
+  for (size_t i = 0; i < children_.size(); i++)
+    children_[i]->GeneratePreorderTraversal(layer_traversal);
 }
 
-bool Layer::IsCompletelyOpaque() const {
-  return fills_bounds_opaquely() && GetCombinedOpacity() == 1.0f;
+void Layer::RecomputeHole() {

[sky, 2011/10/24 15:44:52]

I don't like all the tree walking we do on any interesting change. Imagine if
during one animation cycle we changed the bounds of a bunch of layers.
Could we instead set a boolean indicating we need to update the regions to paint
and then in DrawTree update them?

+  Layer* root_layer = this;
+  while (root_layer->parent_) {
+    root_layer = root_layer->parent_;
+  }
+
+  std::vector<ui::Layer*> layer_traversal;
+  root_layer->ClearHoleRects();
+  root_layer->GeneratePreorderTraversal(&layer_traversal);
+
+  for (size_t i = 0; i < layer_traversal.size(); i++) {
+    Layer* layer = layer_traversal[i];
+    gfx::Rect bounds = gfx::Rect(layer->bounds().size());
+    ui::Transform layer_transform;
+    layer->GetTransformRelativeTo(root_layer, &layer_transform);
+
+    // Iterate through layers which are after layer_traversal[i] in draw order
+    // and find the largest candidate hole.
+    for (size_t j = i + 1; j < layer_traversal.size(); j++) {
+      gfx::Rect candidate_hole = gfx::Rect(layer_traversal[j]->bounds().size());
+
+      // Compute transform to go from bounds of layer |j| to local bounds of layer |i|.

[sky, 2011/10/24 15:44:52]

This line is > 80 chars

+      ui::Transform candidate_hole_transform;
+      layer_traversal[j]->GetTransformRelativeTo(root_layer,

[sky, 2011/10/24 15:44:52]

This is a lot of transform building. Rather than having
GeneratePreorderTraversal generate Layers, maybe it should generate:
struct {
  Transform transform_relative_to_root;
  Layer* layer;
}

+          &candidate_hole_transform);
+      if (!layer_transform.Invert())
+        continue;
+      candidate_hole_transform.ConcatTransform(layer_transform);
+
+      // cannot punch a hole if the relative transform between the two layers
+      // is not multiple of 90.
+      float degrees;
+      if (!InterpolatedTransform::FactorTRS(candidate_hole_transform, NULL,
+          &degrees, NULL) || !IsApproximateMultilpleOf(degrees, 90.0f))
+        continue;
+
+      // can only punch a hole if the two layers have the same opacity.
+      if (layer->GetCombinedOpacity() !=
+          layer_traversal[j]->GetCombinedOpacity())

[Ian Vollick, 2011/10/22 01:06:03]

Shouldn't we punch a hole only if layer_traversal[j]->GetCombinedOpacity() is
1.0? Otherwise, we'd still see a bit of layer i under layer j, wouldn't we?

+        continue;
+
+      candidate_hole_transform.TransformRect(&candidate_hole);
+      candidate_hole = candidate_hole.Intersect(bounds);
+
+      if (candidate_hole.size().GetArea() > layer->hole_rect().size().GetArea())
+        layer->set_hole_rect(candidate_hole);
+    }
+    // Free up texture memory if the hole fills bounds of layer.
+    if (!layer->ShouldDraw() && !layer_updated_externally())
+      layer->DropTexture();
+  }
 }
 
 // static
@@ -399,6 +422,11 @@ void Layer::PunchHole(const gfx::Rect& rect,
                              rect.bottom() - trimmed_rect.bottom()));
 }
 
+void Layer::DropTexture() {
+  if (!layer_updated_externally_)
+    texture_ = NULL;
+}
+
 void Layer::DropTextures() {
   if (!layer_updated_externally_)
     texture_ = NULL;
@@ -476,25 +504,9 @@ void Layer::SetTransformImmediately(const ui::Transform& transform) {
 }
 
 void Layer::SetOpacityImmediately(float opacity) {
-  bool was_opaque = GetCombinedOpacity() == 1.0f;
   opacity_ = opacity;
-  bool is_opaque = GetCombinedOpacity() == 1.0f;
-
-  // If our opacity has changed we need to recompute our hole, our parent's hole
-  // and the holes of all our descendants.
-  if (was_opaque != is_opaque) {
-    if (parent_)
-      parent_->RecomputeHole();
-    std::queue<Layer*> to_process;
-    to_process.push(this);
-    while (!to_process.empty()) {
-      Layer* current = to_process.front();
-      to_process.pop();
-      current->RecomputeHole();
-      for (size_t i = 0; i < current->children_.size(); ++i)
-        to_process.push(current->children_.at(i));
-    }
-  }
+  if (parent_)
+    parent_->RecomputeHole();
 }
 
 void Layer::SetBoundsFromAnimator(const gfx::Rect& bounds) {