Move early outs to the top of View::Paint and OnPaintLayer

ui/views/view.cc

 // Copyright (c) 2012 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.
 
 #define _USE_MATH_DEFINES // For VC++ to get M_PI. This has to be first.
 
 #include "ui/views/view.h"
 
 #include <algorithm>
 #include <cmath>
@@ skipping 746 matching lines @@
   if (layer()) {
     layer()->SchedulePaint(rect);
   } else if (parent_) {
     // Translate the requested paint rect to the parent's coordinate system
     // then pass this notification up to the parent.
     parent_->SchedulePaintInRect(ConvertRectToParent(rect));
   }
 }
 
 void View::Paint(gfx::Canvas* canvas, const CullSet& cull_set) {
+  if (!visible_)
+    return;
   // The cull_set may allow us to skip painting without canvas construction or
   // even canvas rect intersection.
-  if (cull_set.ShouldPaint(this)) {
-    TRACE_EVENT1("views", "View::Paint", "class", GetClassName());
+  if (!cull_set.ShouldPaint(this))
+    return;
 
-    gfx::ScopedCanvas scoped_canvas(canvas);
+  TRACE_EVENT1("views", "View::Paint", "class", GetClassName());
 
-    // Paint this View and its children, setting the clip rect to the bounds
-    // of this View and translating the origin to the local bounds' top left
-    // point.
-    //
-    // Note that the X (or left) position we pass to ClipRectInt takes into
-    // consideration whether or not the view uses a right-to-left layout so that
-    // we paint our view in its mirrored position if need be.
-    gfx::Rect clip_rect = bounds();
-    clip_rect.Inset(clip_insets_);
-    if (parent_)
-      clip_rect.set_x(parent_->GetMirroredXForRect(clip_rect));
-    canvas->ClipRect(clip_rect);
-    if (canvas->IsClipEmpty())
+  gfx::ScopedCanvas scoped_canvas(canvas);
+
+  // Paint this View and its children, setting the clip rect to the bounds
+  // of this View and translating the origin to the local bounds' top left
+  // point.
+  //
+  // Note that the X (or left) position we pass to ClipRectInt takes into
+  // consideration whether or not the view uses a right-to-left layout so that
+  // we paint our view in its mirrored position if need be.
+  gfx::Rect clip_rect = bounds();
+  clip_rect.Inset(clip_insets_);
+  if (parent_)
+    clip_rect.set_x(parent_->GetMirroredXForRect(clip_rect));
+  canvas->ClipRect(clip_rect);
+  if (canvas->IsClipEmpty())
+    return;
+
+  // Non-empty clip, translate the graphics such that 0,0 corresponds to where
+  // this view is located (related to its parent).
+  canvas->Translate(GetMirroredPosition().OffsetFromOrigin());
+  canvas->Transform(GetTransform());
+
+  // If we are a paint root, we need to construct our own CullSet object for
+  // propagation to our children.
+  if (IsPaintRoot()) {
+    if (!bounds_tree_)
+      bounds_tree_.reset(new BoundsTree(2, 5));
+
+    // Recompute our bounds tree as needed.
+    UpdateRootBounds(bounds_tree_.get(), gfx::Vector2d());
+
+    // Grab the clip rect from the supplied canvas to use as the query rect.
+    gfx::Rect canvas_bounds;
+    if (!canvas->GetClipBounds(&canvas_bounds)) {
+      NOTREACHED() << "Failed to get clip bounds from the canvas!";
       return;
+    }
 
-    // Non-empty clip, translate the graphics such that 0,0 corresponds to where
-    // this view is located (related to its parent).
-    canvas->Translate(GetMirroredPosition().OffsetFromOrigin());
-    canvas->Transform(GetTransform());
-
-    // If we are a paint root, we need to construct our own CullSet object for
-    // propagation to our children.
-    if (IsPaintRoot()) {
-      if (!bounds_tree_)
-        bounds_tree_.reset(new BoundsTree(2, 5));
-
-      // Recompute our bounds tree as needed.
-      UpdateRootBounds(bounds_tree_.get(), gfx::Vector2d());
-
-      // Grab the clip rect from the supplied canvas to use as the query rect.
-      gfx::Rect canvas_bounds;
-      if (!canvas->GetClipBounds(&canvas_bounds)) {
-        NOTREACHED() << "Failed to get clip bounds from the canvas!";
-        return;
-      }
-
-      // Now query our bounds_tree_ for a set of damaged views that intersect
-      // our canvas bounds.
-      scoped_ptr<base::hash_set<intptr_t> > damaged_views(
-          new base::hash_set<intptr_t>());
-      bounds_tree_->AppendIntersectingRecords(
-          canvas_bounds, damaged_views.get());
-      // Construct a CullSet to wrap the damaged views set, it will delete it
-      // for us on scope exit.
-      CullSet paint_root_cull_set(damaged_views.Pass());
-      // Paint all descendents using our new cull set.
-      PaintCommon(canvas, paint_root_cull_set);
-    } else {
-      // Not a paint root, so we can proceed as normal.
-      PaintCommon(canvas, cull_set);
-    }
+    // Now query our bounds_tree_ for a set of damaged views that intersect
+    // our canvas bounds.
+    scoped_ptr<base::hash_set<intptr_t>> damaged_views(
+        new base::hash_set<intptr_t>());
+    bounds_tree_->AppendIntersectingRecords(canvas_bounds, damaged_views.get());
+    // Construct a CullSet to wrap the damaged views set, it will delete it
+    // for us on scope exit.
+    CullSet paint_root_cull_set(damaged_views.Pass());
+    // Paint all descendents using our new cull set.
+    PaintCommon(canvas, paint_root_cull_set);
+  } else {
+    // Not a paint root, so we can proceed as normal.
+    PaintCommon(canvas, cull_set);
   }
 }
 
 void View::set_background(Background* b) {
   background_.reset(b);
 }
 
 void View::SetBorder(scoped_ptr<Border> b) { border_ = b.Pass(); }
 
 ui::ThemeProvider* View::GetThemeProvider() const {
@@ skipping 620 matching lines @@
   } else {
     for (int i = 0, count = child_count(); i < count; ++i) {
       View* child = child_at(i);
       child->UpdateChildLayerBounds(
           offset + gfx::Vector2d(child->GetMirroredX(), child->y()));
     }
   }
 }
 
 void View::OnPaintLayer(gfx::Canvas* canvas) {
+  if (!visible_)

[sky, 2015/03/24 22:11:21]

I'm tempted to say this should be a DCHECK, but it looks like view doesn't
attempt to enforce the layers visibility matches the visibility of this. If we
do end up here, should we at least do 1470? Otherwise won't the layer end up
with garbage?

[danakj, 2015/03/24 22:15:20]

It would get garbage if it claimed to be opaque "set_fills_bounds_opaquely", and
would get transparent otherwise. Ya doing the clear seems fine then, it matches
the same conditions.

+    return;
   if (!layer() || !layer()->fills_bounds_opaquely())
     canvas->DrawColor(SK_ColorBLACK, SkXfermode::kClear_Mode);
   PaintCommon(canvas, CullSet());
 }
 
 void View::OnDelegatedFrameDamage(
     const gfx::Rect& damage_rect_in_dip) {
 }
 
 void View::OnDeviceScaleFactorChanged(float device_scale_factor) {
@@ skipping 266 matching lines @@
   } else if (parent_ && type == SCHEDULE_PAINT_SIZE_SAME) {
     // The compositor doesn't Draw() until something on screen changes, so
     // if our position changes but nothing is being animated on screen, then
     // tell the compositor to redraw the scene. We know layer() exists due to
     // the above if clause.
     layer()->ScheduleDraw();
   }
 }
 
 void View::PaintCommon(gfx::Canvas* canvas, const CullSet& cull_set) {
-  if (!visible_)
-    return;
-
   {
     // If the View we are about to paint requested the canvas to be flipped, we
     // should change the transform appropriately.
     // The canvas mirroring is undone once the View is done painting so that we
     // don't pass the canvas with the mirrored transform to Views that didn't
     // request the canvas to be flipped.
     gfx::ScopedCanvas scoped(canvas);
     if (FlipCanvasOnPaintForRTLUI()) {
       canvas->Translate(gfx::Vector2d(width(), 0));
       canvas->Scale(-1, 1);
@@ skipping 709 matching lines @@
   // Message the RootView to do the drag and drop. That way if we're removed
   // the RootView can detect it and avoid calling us back.
   gfx::Point widget_location(event.location());
   ConvertPointToWidget(this, &widget_location);
   widget->RunShellDrag(this, data, widget_location, drag_operations, source);
   // WARNING: we may have been deleted.
   return true;
 }
 
 }  // namespace views