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

Issue 1126813002: cc: Pass priority rect information from iterators to tilings. (Closed)

Created:
5 years, 7 months ago by USE eero AT chromium.org
Modified:
5 years, 7 months ago
Reviewers:
hendrikw, vmpstr, enne (OOO)
CC:
chromium-reviews, cc-bugs_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

cc: Pass priority rect information from iterators to tilings. As each tiling iterator in raster and eviction queues iterates one priority rect, those iterators know the priority rect for tiles. Thus picture layer tilings compute priority rects only because iterators do not pass that information to tilings. Therefore, unnecessary priority rect computation can be avoided by passing priority rect information from iterators to tilings. Committed: https://crrev.com/1b9c87ec0f730691b2bcfd1023eb8970535495d3 Cr-Commit-Position: refs/heads/master@{#330061}

Patch Set 1 #

Total comments: 18

Patch Set 2 : Clean up, naming, etc. #

Patch Set 3 : Rebase #

Total comments: 6

Patch Set 4 : Rebase #

Patch Set 5 : Rebase fix #

Unified diffs Side-by-side diffs Delta from patch set Stats (+167 lines, -92 lines) Patch
M cc/resources/picture_layer_tiling.h View 1 2 3 2 chunks +20 lines, -2 lines 0 comments Download
M cc/resources/picture_layer_tiling.cc View 1 2 3 4 3 chunks +55 lines, -22 lines 0 comments Download
M cc/resources/tiling_set_eviction_queue.h View 1 2 2 chunks +5 lines, -4 lines 0 comments Download
M cc/resources/tiling_set_eviction_queue.cc View 1 2 7 chunks +13 lines, -15 lines 0 comments Download
M cc/resources/tiling_set_raster_queue_all.h View 1 2 3 4 chunks +11 lines, -19 lines 0 comments Download
M cc/resources/tiling_set_raster_queue_all.cc View 1 2 20 chunks +59 lines, -28 lines 0 comments Download
M cc/resources/tiling_set_raster_queue_required.cc View 1 2 2 chunks +4 lines, -2 lines 0 comments Download

Messages

Total messages: 17 (3 generated)
USE eero AT chromium.org
PTAL. This removes most unneeded gfx::Rect::Intersects calls from tile priority update paths.
5 years, 7 months ago (2015-05-05 16:18:26 UTC) #2
vmpstr
This is pretty cool! https://codereview.chromium.org/1126813002/diff/1/cc/resources/picture_layer_tiling.cc File cc/resources/picture_layer_tiling.cc (right): https://codereview.chromium.org/1126813002/diff/1/cc/resources/picture_layer_tiling.cc#newcode867 cc/resources/picture_layer_tiling.cc:867: PictureLayerTiling::PriorityRect PictureLayerTiling::ComputePriorityRectForTile( Is this function ...
5 years, 7 months ago (2015-05-05 18:04:02 UTC) #3
USE eero AT chromium.org
https://codereview.chromium.org/1126813002/diff/1/cc/resources/picture_layer_tiling.cc File cc/resources/picture_layer_tiling.cc (right): https://codereview.chromium.org/1126813002/diff/1/cc/resources/picture_layer_tiling.cc#newcode867 cc/resources/picture_layer_tiling.cc:867: PictureLayerTiling::PriorityRect PictureLayerTiling::ComputePriorityRectForTile( On 2015/05/05 18:04:01, vmpstr wrote: > Is ...
5 years, 7 months ago (2015-05-06 12:05:21 UTC) #4
vmpstr
+hendrikw. This looks like fine to me, but it might conflict with some stuff you're ...
5 years, 7 months ago (2015-05-11 16:19:29 UTC) #6
hendrikw
On 2015/05/11 16:19:29, vmpstr wrote: > +hendrikw. This looks like fine to me, but it ...
5 years, 7 months ago (2015-05-11 21:22:08 UTC) #7
USE eero AT chromium.org
On 2015/05/11 21:22:08, hendrikw wrote: > e_hakkinen, would you mind rebasing on top of that? ...
5 years, 7 months ago (2015-05-12 11:29:54 UTC) #8
hendrikw
Can you give me an estimate of how many intersects you will save per tile? ...
5 years, 7 months ago (2015-05-12 22:23:49 UTC) #9
USE eero AT chromium.org
On 2015/05/12 22:23:49, hendrikw wrote: > Can you give me an estimate of how many ...
5 years, 7 months ago (2015-05-13 07:48:01 UTC) #10
USE eero AT chromium.org
https://codereview.chromium.org/1126813002/diff/40001/cc/resources/picture_layer_tiling.cc File cc/resources/picture_layer_tiling.cc (right): https://codereview.chromium.org/1126813002/diff/40001/cc/resources/picture_layer_tiling.cc#newcode862 cc/resources/picture_layer_tiling.cc:862: return TilePriority(resolution_, priority_bin, 0); On 2015/05/12 22:23:49, hendrikw wrote: ...
5 years, 7 months ago (2015-05-13 07:48:14 UTC) #11
hendrikw
On 2015/05/13 07:48:14, e_hakkinen wrote: > https://codereview.chromium.org/1126813002/diff/40001/cc/resources/picture_layer_tiling.cc > File cc/resources/picture_layer_tiling.cc (right): > > https://codereview.chromium.org/1126813002/diff/40001/cc/resources/picture_layer_tiling.cc#newcode862 > ...
5 years, 7 months ago (2015-05-13 17:48:22 UTC) #12
vmpstr
lgtm
5 years, 7 months ago (2015-05-13 17:59:30 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1126813002/80001
5 years, 7 months ago (2015-05-15 07:12:19 UTC) #15
commit-bot: I haz the power
Committed patchset #5 (id:80001)
5 years, 7 months ago (2015-05-15 08:07:32 UTC) #16
commit-bot: I haz the power
5 years, 7 months ago (2015-05-15 08:08:32 UTC) #17
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/1b9c87ec0f730691b2bcfd1023eb8970535495d3
Cr-Commit-Position: refs/heads/master@{#330061}

Powered by Google App Engine
This is Rietveld 408576698