Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(92)

Issue 2866063002: Improve overdraw with multiple windows (Closed)

Created:
3 years, 7 months ago by yiyix
Modified:
3 years, 7 months ago
Reviewers:
danakj
CC:
chromium-reviews, cc-bugs_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Improve overdraw with multiple windows When 2 windows are opened simultaneously (overlapped or non-overlapped) namely window 1 and window 2, before this patch, defining the occluded rect is used to computing the largest unionized rectangle that is formed with these 2 windows and mark layers behind it occluded. However, when 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 each other in the Z order. For example, if a unionized rectangle uses area from both window 1 and window 2: if a pixel is from window 1, it occludes 1 layer of overdrawing (drawing the pixel from the wallpaper); if a pixel is from window 2, it occludes 2 layer of overdrawing (drawing the pixel from the wallpaper and the pixel from browser UI). In this patch, a weighted function is introduced to optimize the calculate the unionized rectangle where the area in window 2 is multiplied by 2, so the unionized rectangle is the rectangle with the highest weight. BUG=680636 TEST=SimpleEnclosedRegionTest.Union CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2866063002 Cr-Commit-Position: refs/heads/master@{#474451} Committed: https://chromium.googlesource.com/chromium/src/+/34c7a895b3da858147c3bc836b23584a8c01410a

Patch Set 1 #

Total comments: 27

Patch Set 2 : address comments #

Total comments: 1

Patch Set 3 : remove occlusion_track_unittests and rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+119 lines, -52 lines) Patch
M cc/base/simple_enclosed_region.cc View 1 2 2 chunks +15 lines, -2 lines 0 comments Download
M cc/base/simple_enclosed_region_unittest.cc View 1 1 chunk +104 lines, -50 lines 0 comments Download

Messages

Total messages: 62 (53 generated)
yiyix
@danakj, could you please review this patch? Thank you very much.
3 years, 7 months ago (2017-05-15 21:13:56 UTC) #36
danakj
https://codereview.chromium.org/2866063002/diff/140001/cc/base/simple_enclosed_region.cc File cc/base/simple_enclosed_region.cc (right): https://codereview.chromium.org/2866063002/diff/140001/cc/base/simple_enclosed_region.cc#newcode133 cc/base/simple_enclosed_region.cc:133: int64_t weighted_rect_area = I suggest to build 3 rects, ...
3 years, 7 months ago (2017-05-15 21:47:43 UTC) #37
yiyix
@danakj, could you please review this updated pathed? Thank you https://codereview.chromium.org/2866063002/diff/140001/cc/base/simple_enclosed_region.cc File cc/base/simple_enclosed_region.cc (right): https://codereview.chromium.org/2866063002/diff/140001/cc/base/simple_enclosed_region.cc#newcode133 ...
3 years, 7 months ago (2017-05-17 03:00:32 UTC) #43
danakj
Thanks, I'm mostly concerned about the occlusion tracker test. I've left a bunch of comments ...
3 years, 7 months ago (2017-05-17 15:15:57 UTC) #46
danakj
https://codereview.chromium.org/2866063002/diff/160001/cc/base/simple_enclosed_region.cc File cc/base/simple_enclosed_region.cc (right): https://codereview.chromium.org/2866063002/diff/160001/cc/base/simple_enclosed_region.cc#newcode141 cc/base/simple_enclosed_region.cc:141: // in the Z order. So, the area of ...
3 years, 7 months ago (2017-05-17 15:17:40 UTC) #47
yiyix
On 2017/05/17 15:15:57, danakj wrote: > Thanks, I'm mostly concerned about the occlusion tracker test. ...
3 years, 7 months ago (2017-05-24 18:28:23 UTC) #54
danakj
LGTM
3 years, 7 months ago (2017-05-24 21:55:34 UTC) #57
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2866063002/220001
3 years, 7 months ago (2017-05-24 22:24:21 UTC) #59
commit-bot: I haz the power
3 years, 7 months ago (2017-05-24 22:30:39 UTC) #62
Message was sent while issue was closed.
Committed patchset #3 (id:220001) as
https://chromium.googlesource.com/chromium/src/+/34c7a895b3da858147c3bc836b23...

Powered by Google App Engine
This is Rietveld 408576698