Improve overdraw with multiple windows

cc/base/simple_enclosed_region.cc

 // Copyright 2014 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/base/simple_enclosed_region.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include "base/logging.h"
 #include "cc/base/region.h"
+#include "ui/gfx/geometry/rect.h"
 
 namespace cc {
 
 static bool RectIsLargerArea(const gfx::Rect& a, const gfx::Rect b) {
   int64_t a_area = static_cast<int64_t>(a.width()) * a.height();
   int64_t b_area = static_cast<int64_t>(b.width()) * b.height();
   return a_area > b_area;
 }
 
 SimpleEnclosedRegion::SimpleEnclosedRegion(const Region& region) {
@@ skipping 98 matching lines @@
     if (top < new_top && bottom >= new_top)
       new_top = top;
     if (bottom > new_bottom && top <= new_bottom)
       new_bottom = bottom;
   }
 
   rect_.SetRect(left, top, right - left, bottom - top);
 
   gfx::Rect adjusted_new_rect(
       new_left, new_top, new_right - new_left, new_bottom - new_top);
-  if (RectIsLargerArea(adjusted_new_rect, rect_))
+  gfx::Rect overlap = gfx::IntersectRects(rect_, adjusted_new_rect);
+
+  int64_t weighted_rect_area =

[danakj, 2017/05/15 21:47:42]

I suggest to build 3 rects, old_rect_area, overlap_area, new_rect_area, then
add/multiply etc along with compare in the if () statement. This separates the
data from the logic/algorithm and would make this more clear to read I think.

[yiyix, 2017/05/17 03:00:31]

I agree. If the area calculation is done separately, the combining function is
easier to be understand. 

+      static_cast<int64_t>(rect_.width() * rect_.height()) +

[danakj, 2017/05/15 21:47:42]

cast just the width, that will promote before the multiply instead of after. if
you promote after it will overflow at the size of width()'s type then upcast to
more bits afterward.

[yiyix, 2017/05/17 03:00:31]

Done.

+      static_cast<int64_t>(overlap.width() * overlap.height());

[danakj, 2017/05/15 21:47:42]

same

[yiyix, 2017/05/17 03:00:31]

Done.

+  int64_t weighted_adjusted_new_rect_area =
+      2 * static_cast<int64_t>(adjusted_new_rect.width() *

[danakj, 2017/05/15 21:47:42]

same

[yiyix, 2017/05/17 03:00:31]

Done.

+                               adjusted_new_rect.height());
+  if (weighted_adjusted_new_rect_area > weighted_rect_area)

[danakj, 2017/05/15 21:47:42]

Can you put a comment here explaining roughly what you say in the CL description
to explain the weights?
This is based on the assumption that as we compute occlusion, each step is more
likely to be occluded by things added to this region more recently due to the
way we build scenes with overlapping elements adjacent to eachother in the Z
order. Saying something to this effect would be very useful (both here and in
the CL description). Maybe with a concrete example of 2 layers, one containing
the other, similar to the browser UI?

[yiyix, 2017/05/17 03:00:31]

I will add this to both. Your wording is better! Thanks you. 

     rect_ = adjusted_new_rect;
 }
 
 gfx::Rect SimpleEnclosedRegion::GetRect(size_t i) const {
   DCHECK_LT(i, GetRegionComplexity());
   return rect_;
 }
 
 }  // namespace cc