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

Issue 603683006: cc: Remove low quality mode and cleanup tile versions. (Closed)

Created:
6 years, 2 months ago by vmpstr
Modified:
6 years, 2 months ago
Reviewers:
danakj, reveman, enne (OOO)
CC:
chromium-reviews, cc-bugs_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

cc: Remove low quality mode and cleanup tile versions. This patch removes low quality mode for low resolution tiles. It also cleansup some logic with tile versions, since there's only one now. As a follow-up we can see if we should move the tile version stuff onto tile directly, although I kind of prefer having a separate class just for managing what mode we're drawing. BUG=417876 R=enne, reveman, danakj Committed: https://crrev.com/488516f064c0f28e21d7748d583e4d61affafe89 Cr-Commit-Position: refs/heads/master@{#297181}

Patch Set 1 #

Patch Set 2 : update #

Total comments: 7

Patch Set 3 : #

Patch Set 4 : remove paint_simplifier include #

Total comments: 10

Patch Set 5 : update #

Unified diffs Side-by-side diffs Delta from patch set Stats (+101 lines, -316 lines) Patch
M cc/BUILD.gn View 1 chunk +0 lines, -2 lines 0 comments Download
M cc/cc.gyp View 1 chunk +0 lines, -2 lines 0 comments Download
M cc/debug/frame_viewer_instrumentation.h View 2 chunks +3 lines, -7 lines 0 comments Download
M cc/layers/picture_layer_impl.cc View 1 2 3 4 5 chunks +15 lines, -18 lines 0 comments Download
M cc/layers/picture_layer_impl_unittest.cc View 1 2 3 4 2 chunks +4 lines, -8 lines 0 comments Download
M cc/resources/managed_tile_state.h View 1 2 3 4 5 chunks +6 lines, -9 lines 0 comments Download
M cc/resources/managed_tile_state.cc View 1 2 3 4 3 chunks +15 lines, -25 lines 0 comments Download
M cc/resources/picture_layer_tiling.h View 1 chunk +1 line, -2 lines 0 comments Download
M cc/resources/picture_layer_tiling_perftest.cc View 1 2 3 4 2 chunks +4 lines, -6 lines 0 comments Download
M cc/resources/picture_layer_tiling_unittest.cc View 1 2 3 4 2 chunks +4 lines, -8 lines 0 comments Download
D cc/resources/raster_mode.h View 1 chunk +0 lines, -30 lines 0 comments Download
D cc/resources/raster_mode.cc View 1 chunk +0 lines, -25 lines 0 comments Download
M cc/resources/tile.h View 1 2 3 4 4 chunks +6 lines, -35 lines 0 comments Download
M cc/resources/tile.cc View 1 2 3 4 1 chunk +4 lines, -26 lines 0 comments Download
M cc/resources/tile_manager.h View 1 2 3 4 4 chunks +2 lines, -9 lines 0 comments Download
M cc/resources/tile_manager.cc View 1 2 3 4 20 chunks +31 lines, -96 lines 0 comments Download
M cc/test/fake_picture_layer_impl.cc View 1 2 3 4 1 chunk +1 line, -2 lines 0 comments Download
M cc/trees/layer_tree_host_unittest.cc View 1 2 3 4 1 chunk +5 lines, -6 lines 0 comments Download

Messages

Total messages: 18 (2 generated)
vmpstr
PTAL
6 years, 2 months ago (2014-09-25 23:14:57 UTC) #1
reveman
Ok, this is cool as it might allow us to make the Tile class itself ...
6 years, 2 months ago (2014-09-26 19:02:12 UTC) #2
vmpstr
On 2014/09/26 19:02:12, reveman wrote: > Ok, this is cool as it might allow us ...
6 years, 2 months ago (2014-09-26 19:14:56 UTC) #3
reveman
On 2014/09/26 19:14:56, vmpstr wrote: > On 2014/09/26 19:02:12, reveman wrote: > > Ok, this ...
6 years, 2 months ago (2014-09-26 21:28:13 UTC) #4
vmpstr
PTAL
6 years, 2 months ago (2014-09-26 22:00:27 UTC) #5
danakj
Ooh all those confusing for loops. LGTM % reveman https://codereview.chromium.org/603683006/diff/20001/cc/resources/managed_tile_state.cc File cc/resources/managed_tile_state.cc (right): https://codereview.chromium.org/603683006/diff/20001/cc/resources/managed_tile_state.cc#newcode81 cc/resources/managed_tile_state.cc:81: ...
6 years, 2 months ago (2014-09-26 22:38:21 UTC) #6
vmpstr
https://codereview.chromium.org/603683006/diff/20001/cc/resources/managed_tile_state.cc File cc/resources/managed_tile_state.cc (right): https://codereview.chromium.org/603683006/diff/20001/cc/resources/managed_tile_state.cc#newcode81 cc/resources/managed_tile_state.cc:81: has_resource |= (draw_info_.resource_.get() != 0); On 2014/09/26 22:38:21, danakj ...
6 years, 2 months ago (2014-09-26 22:51:39 UTC) #7
danakj
https://codereview.chromium.org/603683006/diff/20001/cc/resources/tile_manager.cc File cc/resources/tile_manager.cc (left): https://codereview.chromium.org/603683006/diff/20001/cc/resources/tile_manager.cc#oldcode130 cc/resources/tile_manager.cc:130: draw_filter = skia::AdoptRef(new skia::PaintSimplifier); On 2014/09/26 22:51:39, vmpstr wrote: ...
6 years, 2 months ago (2014-09-26 23:01:44 UTC) #8
vmpstr
On 2014/09/26 23:01:44, danakj wrote: > https://codereview.chromium.org/603683006/diff/20001/cc/resources/tile_manager.cc > File cc/resources/tile_manager.cc (left): > > https://codereview.chromium.org/603683006/diff/20001/cc/resources/tile_manager.cc#oldcode130 > ...
6 years, 2 months ago (2014-09-26 23:04:43 UTC) #9
vmpstr
On 2014/09/26 23:04:43, vmpstr wrote: > On 2014/09/26 23:01:44, danakj wrote: > > > https://codereview.chromium.org/603683006/diff/20001/cc/resources/tile_manager.cc ...
6 years, 2 months ago (2014-09-26 23:12:39 UTC) #10
reveman
https://codereview.chromium.org/603683006/diff/60001/cc/resources/managed_tile_state.h File cc/resources/managed_tile_state.h (right): https://codereview.chromium.org/603683006/diff/60001/cc/resources/managed_tile_state.h#newcode40 cc/resources/managed_tile_state.h:40: class CC_EXPORT TileDrawInfo { nit: is the "Tile" prefix ...
6 years, 2 months ago (2014-09-27 02:57:32 UTC) #11
vmpstr
PTAL https://codereview.chromium.org/603683006/diff/60001/cc/resources/managed_tile_state.h File cc/resources/managed_tile_state.h (right): https://codereview.chromium.org/603683006/diff/60001/cc/resources/managed_tile_state.h#newcode40 cc/resources/managed_tile_state.h:40: class CC_EXPORT TileDrawInfo { On 2014/09/27 02:57:31, reveman ...
6 years, 2 months ago (2014-09-29 14:09:50 UTC) #12
reveman
lgtm
6 years, 2 months ago (2014-09-29 14:54:33 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/603683006/80001
6 years, 2 months ago (2014-09-29 14:56:06 UTC) #15
commit-bot: I haz the power
Committed patchset #5 (id:80001) as 43a497b729f531c7bc185f184cc6f54ca4c900a7
6 years, 2 months ago (2014-09-29 15:45:34 UTC) #16
commit-bot: I haz the power
6 years, 2 months ago (2014-09-29 15:46:09 UTC) #17
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/488516f064c0f28e21d7748d583e4d61affafe89
Cr-Commit-Position: refs/heads/master@{#297181}

Powered by Google App Engine
This is Rietveld 408576698