Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1648433002:
compositor: Enable partial raster by default. (Closed)
|
Created:
4 years, 11 months ago by dshwang Modified:
4 years, 9 months ago CC:
achuith+watch_chromium.org, chromium-reviews, creis+watch_chromium.org, darin-cc_chromium.org, davemoore+watch_chromium.org, dzhioev+watch_chromium.org, jam, mkwst+moarreviews-renderer_chromium.org, mlamouri+watch-content_chromium.org, nasko+codewatch_chromium.org, oshima+watch_chromium.org, piman+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptioncompositor: Enable partial raster by default.
As enabling partial raster by default, replace
--enable-partial-raster with --disable-partial-raster.
TEST=Run ChromeOS and Android with native one-copy with partial raster.
BUG=492754
Committed: https://crrev.com/23db47f341747ea0a4e176e3047fc9f8650c07f5
Cr-Commit-Position: refs/heads/master@{#379778}
Patch Set 1 #Patch Set 2 : rebase to ToT #
Messages
Total messages: 43 (20 generated)
Description was changed from ========== compositor: Enable partial raster by default. Currently, UI compositor always uses partial raster by default. There is not any reason for renderer to enable it by default also. In addition, as enabling partial raster by default, replace --enable-partial-raster with --disable-partial-raster. TEST=Run ChromeOS and Android with native one-copy with partial raster. BUG=581786 ========== to ========== compositor: Enable partial raster by default. Currently, UI compositor always uses partial raster by default. There is not any reason for renderer to enable it by default also. In addition, as enabling partial raster by default, replace --enable-partial-raster with --disable-partial-raster. TEST=Run ChromeOS and Android with native one-copy with partial raster. BUG=581786 ==========
dongseong.hwang@intel.com changed reviewers: + piman@chromium.org, reveman@chromium.org
piman, reveman, could you review? reveman, partial one-copy is more preferable, right? I'd seen partial raster enabled by default at Oct/2015, but today I find it's disabled by default when I worked on https://codereview.chromium.org/1637423004/ I cannot find any reason to not enable it. WDYT?
Description was changed from ========== compositor: Enable partial raster by default. Currently, UI compositor always uses partial raster by default. There is not any reason for renderer to enable it by default also. In addition, as enabling partial raster by default, replace --enable-partial-raster with --disable-partial-raster. TEST=Run ChromeOS and Android with native one-copy with partial raster. BUG=581786 ========== to ========== compositor: Enable partial raster by default. Currently, UI compositor always uses partial raster by default. There is not any reason for renderer to enable it by default also. In addition, as enabling partial raster by default, replace --enable-partial-raster with --disable-partial-raster. TEST=Run ChromeOS and Android with native one-copy with partial raster. BUG=492754 ==========
dongseong.hwang@intel.com changed reviewers: + danakj@chromium.org
I found https://code.google.com/p/chromium/issues/detail?id=492754 thanks for danakj. Don't need to review. I need to digest the history. I'll ask to review again. Thank you.
Description was changed from ========== compositor: Enable partial raster by default. Currently, UI compositor always uses partial raster by default. There is not any reason for renderer to enable it by default also. In addition, as enabling partial raster by default, replace --enable-partial-raster with --disable-partial-raster. TEST=Run ChromeOS and Android with native one-copy with partial raster. BUG=492754 ========== to ========== compositor: Enable partial raster by default. As enabling partial raster by default, replace --enable-partial-raster with --disable-partial-raster. TEST=Run ChromeOS and Android with native one-copy with partial raster. BUG=492754 ==========
hi, could you review again? I fixed the blocker issue crbug.com/528922
LGTM
lgtm
The CQ bit was checked by dongseong.hwang@intel.com
thank you for review!
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1648433002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1648433002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
The CQ bit was checked by dongseong.hwang@intel.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1648433002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1648433002/20001
The CQ bit was unchecked by dongseong.hwang@intel.com
dongseong.hwang@intel.com changed reviewers: - piman@chromium.org
dongseong.hwang@intel.com changed reviewers: + brettw@chromium.org, piman@chromium.org
brettw, could you review as owner? Dana and David gave lgtm.
On 2016/02/29 09:15:56, dshwang wrote: > brettw, could you review as owner? Dana and David gave lgtm. You probly want a content/ owner instead
dongseong.hwang@intel.com changed reviewers: - brettw@chromium.org, piman@chromium.org
dongseong.hwang@intel.com changed reviewers: + avi@chromium.org, dzhioev@chromium.org, piman@chromium.org
On 2016/02/29 18:21:01, danakj wrote: > On 2016/02/29 09:15:56, dshwang wrote: > > brettw, could you review as owner? Dana and David gave lgtm. > > You probly want a content/ owner instead Ok. Avi, could you review content/ ? dzhioev, could you review chrome/ ?
lgtm
Thank you for review! > dzhioev, could you review chrome/ ? PTAL
dongseong.hwang@intel.com changed reviewers: - dzhioev@chromium.org
dongseong.hwang@intel.com changed reviewers: + dzhioev@chromium.org
On 2016/03/02 13:12:50, dshwang wrote: > Thank you for review! > > > dzhioev, could you review chrome/ ? > PTAL piman, could you review chrome/browser/chromeos/login/chrome_restart_request.cc ?
LGTM!
On 2016/03/07 21:05:00, piman wrote: > LGTM! Thank you for reviewing!
The CQ bit was checked by dongseong.hwang@intel.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1648433002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1648433002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
LGTM
The CQ bit was checked by dongseong.hwang@intel.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1648433002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1648433002/20001
Message was sent while issue was closed.
Description was changed from ========== compositor: Enable partial raster by default. As enabling partial raster by default, replace --enable-partial-raster with --disable-partial-raster. TEST=Run ChromeOS and Android with native one-copy with partial raster. BUG=492754 ========== to ========== compositor: Enable partial raster by default. As enabling partial raster by default, replace --enable-partial-raster with --disable-partial-raster. TEST=Run ChromeOS and Android with native one-copy with partial raster. BUG=492754 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== compositor: Enable partial raster by default. As enabling partial raster by default, replace --enable-partial-raster with --disable-partial-raster. TEST=Run ChromeOS and Android with native one-copy with partial raster. BUG=492754 ========== to ========== compositor: Enable partial raster by default. As enabling partial raster by default, replace --enable-partial-raster with --disable-partial-raster. TEST=Run ChromeOS and Android with native one-copy with partial raster. BUG=492754 Committed: https://crrev.com/23db47f341747ea0a4e176e3047fc9f8650c07f5 Cr-Commit-Position: refs/heads/master@{#379778} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/23db47f341747ea0a4e176e3047fc9f8650c07f5 Cr-Commit-Position: refs/heads/master@{#379778} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
