cc: early return if unoccluded_region_in_target_surface is empty

cc: early return if unoccluded_region_in_target_surface is empty

R=danakj@chromium.org
BUG=none

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=282980

[hyunki, 2014-07-14 13:09:12]

Please take a look. Thanks. :)

[danakj, 2014-07-14 15:14:54]

https://codereview.chromium.org/381613004/diff/20001/cc/trees/occlusion_track...
File cc/trees/occlusion_tracker.cc (right):

https://codereview.chromium.org/381613004/diff/20001/cc/trees/occlusion_track...
cc/trees/occlusion_tracker.cc:570: if
(unoccluded_region_in_target_surface.IsEmpty())
ProjectEnclosingClippedRect and Intersect both have early outs that would be hit
in this case. Is adding another early out here a win? Did you measure this in
some way?

[hyunki, 2014-07-14 15:29:34]

https://codereview.chromium.org/381613004/diff/20001/cc/trees/occlusion_track...
File cc/trees/occlusion_tracker.cc (right):

https://codereview.chromium.org/381613004/diff/20001/cc/trees/occlusion_track...
cc/trees/occlusion_tracker.cc:570: if
(unoccluded_region_in_target_surface.IsEmpty())
On 2014/07/14 15:14:53, danakj wrote:
> ProjectEnclosingClippedRect and Intersect both have early outs that would be
hit
> in this case. Is adding another early out here a win? Did you measure this in
> some way?

I think Intersect have early out.
However, when I checked ProjectEnclosingClippedRect, it checks transform only 
and doesn't check source rect (unoccluded_rect_in_target_surface), thus I think
it doesn't out early.

[danakj, 2014-07-14 16:00:09]

https://codereview.chromium.org/381613004/diff/20001/cc/trees/occlusion_track...
File cc/trees/occlusion_tracker.cc (right):

https://codereview.chromium.org/381613004/diff/20001/cc/trees/occlusion_track...
cc/trees/occlusion_tracker.cc:570: if
(unoccluded_region_in_target_surface.IsEmpty())
On 2014/07/14 15:29:34, hyunki wrote:
> On 2014/07/14 15:14:53, danakj wrote:
> > ProjectEnclosingClippedRect and Intersect both have early outs that would be
> hit
> > in this case. Is adding another early out here a win? Did you measure this
in
> > some way?
> 
> I think Intersect have early out.
> However, when I checked ProjectEnclosingClippedRect, it checks transform only 
> and doesn't check source rect (unoccluded_rect_in_target_surface), thus I
think
> it doesn't out early.   

Oh, right sorry. Ok sounds good, does this have any impact on the
occlusion_tracker_perftests?

https://codereview.chromium.org/381613004/diff/20001/cc/trees/occlusion_track...
cc/trees/occlusion_tracker.cc:614: 
How about here?

[hyunki, 2014-07-14 16:25:09]

https://codereview.chromium.org/381613004/diff/20001/cc/trees/occlusion_track...
File cc/trees/occlusion_tracker.cc (right):

https://codereview.chromium.org/381613004/diff/20001/cc/trees/occlusion_track...
cc/trees/occlusion_tracker.cc:570: if
(unoccluded_region_in_target_surface.IsEmpty())

> Oh, right sorry. Ok sounds good, does this have any impact on the
> occlusion_tracker_perftests?

When I added logs, I've checked it falls early outs for many cases. (perftest
and unittest both)
perftest doesn't give big improvements on my machine as belows.

--> Before
[1/2] OcclusionTrackerPerfTest.UnoccludedContentRect_FullyOccluded (2005 ms)
[2/2] OcclusionTrackerPerfTest.UnoccludedContentRect_10OpaqueLayers (2005 ms)
--> After
[1/2] OcclusionTrackerPerfTest.UnoccludedContentRect_FullyOccluded (1999 ms)
[2/2] OcclusionTrackerPerfTest.UnoccludedContentRect_10OpaqueLayers (2000 ms)

https://codereview.chromium.org/381613004/diff/20001/cc/trees/occlusion_track...
cc/trees/occlusion_tracker.cc:614: 
On 2014/07/14 16:00:08, danakj wrote:
> How about here?

Done.

[danakj, 2014-07-14 16:26:02]

LGTM

https://codereview.chromium.org/381613004/diff/20001/cc/trees/occlusion_track...
File cc/trees/occlusion_tracker.cc (right):

https://codereview.chromium.org/381613004/diff/20001/cc/trees/occlusion_track...
cc/trees/occlusion_tracker.cc:570: if
(unoccluded_region_in_target_surface.IsEmpty())
On 2014/07/14 16:25:09, hyunki wrote:
> 
> > Oh, right sorry. Ok sounds good, does this have any impact on the
> > occlusion_tracker_perftests?
> 
> When I added logs, I've checked it falls early outs for many cases. (perftest
> and unittest both)
> perftest doesn't give big improvements on my machine as belows.
> 
> --> Before
> [1/2] OcclusionTrackerPerfTest.UnoccludedContentRect_FullyOccluded (2005 ms)
> [2/2] OcclusionTrackerPerfTest.UnoccludedContentRect_10OpaqueLayers (2005 ms)
> --> After
> [1/2] OcclusionTrackerPerfTest.UnoccludedContentRect_FullyOccluded (1999 ms)
> [2/2] OcclusionTrackerPerfTest.UnoccludedContentRect_10OpaqueLayers (2000 ms)

It's not testing this case, so I would expect no change or a regression. No
change is good.

[danakj, 2014-07-14 16:26:08]

The CQ bit was checked by danakj@chromium.org

[hyunki, 2014-07-14 16:27:15]

Thanks for the review. :-)

[commit-bot: I haz the power, 2014-07-14 16:27:36]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hyunki.baik@samsung.com/381613004/40001

[commit-bot: I haz the power, 2014-07-14 18:34:55]

Change committed as 282980