Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(2)

Issue 2790583004: Add second copy request after screen rotation to flatten the layers in animation. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 months, 3 weeks ago by wutao
Modified:
5 months ago
Reviewers:
oshima
CC:
chromium-reviews, kalyank, sadrul, asvitkine+watch_chromium.org, bruthig, danakj
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Add a copy request after screen rotation to flatten the layers in animation. Currently we use all the layers after rotation to do animation during screen rotation. The solution is using compositor copy request to flatten the layer hierarchy and make animations more efficient. BUG=678763 TEST=Unittest and Manual Review-Url: https://codereview.chromium.org/2790583004 Cr-Commit-Position: refs/heads/master@{#465864} Committed: https://chromium.googlesource.com/chromium/src/+/6d394112c70e0edb2e8f1464bcb9f29dfa54bbb7

Patch Set 1 #

Total comments: 6

Patch Set 2 : Add a copy request after screen rotation to flatten the layers in animation. #

Total comments: 16

Patch Set 3 : Refactor to get old layer transformer. #

Patch Set 4 : Move some functions to ash/utility/transformer_util. #

Total comments: 12

Patch Set 5 : Fix for comments in patch 4. #

Patch Set 6 : Rebased on all the changes previously. #

Total comments: 6

Patch Set 7 : Fix nits in patch 6. #

Patch Set 8 : Rebased and fixed a bug. #

Total comments: 6

Patch Set 9 : Add new tests and add doc to handle new rotation request in |Rotate()|. #

Total comments: 10

Patch Set 10 : Fix nits and add docs. #

Total comments: 2

Patch Set 11 : Add one more state for rotating and fix corner case. #

Total comments: 2

Patch Set 12 : Do not observe screen rotation animator in display_configuration_controller. #

Total comments: 2

Patch Set 13 : Separate the two test sets for slow/smooth animation. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+622 lines, -314 lines) Patch
M ash/BUILD.gn View 1 2 3 4 5 6 7 8 9 1 chunk +2 lines, -0 lines 0 comments Download
M ash/display/display_configuration_controller.h View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +1 line, -8 lines 0 comments Download
M ash/display/display_configuration_controller.cc View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +0 lines, -11 lines 0 comments Download
M ash/display/display_configuration_controller_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +9 lines, -2 lines 0 comments Download
M ash/display/root_window_transformers.cc View 1 2 3 4 5 2 chunks +10 lines, -66 lines 0 comments Download
M ash/rotator/screen_rotation_animator.h View 1 2 3 4 5 6 7 8 9 10 11 12 5 chunks +53 lines, -10 lines 0 comments Download
M ash/rotator/screen_rotation_animator.cc View 1 2 3 4 5 6 7 8 9 10 11 9 chunks +218 lines, -171 lines 0 comments Download
M ash/rotator/screen_rotation_animator_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 14 chunks +231 lines, -37 lines 0 comments Download
M ash/rotator/test/screen_rotation_animator_test_api.cc View 1 7 8 9 10 11 12 1 chunk +4 lines, -0 lines 0 comments Download
A ash/utility/transformer_util.h View 1 2 3 4 1 chunk +26 lines, -0 lines 0 comments Download
A ash/utility/transformer_util.cc View 1 2 3 4 1 chunk +56 lines, -0 lines 0 comments Download
M testing/buildbot/filters/ash_mus_unittests.filter View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +12 lines, -9 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 62 (31 generated)
wutao
Hi Oshima, When this cl landed, it will bring us to 15 fps for the ...
5 months, 3 weeks ago (2017-03-31 01:56:30 UTC) #2
danakj
Thanks it looks like you're using copy requests correctly, but you could make things less ...
5 months, 3 weeks ago (2017-03-31 19:32:30 UTC) #4
wutao
Hi Oshima, Could you please have a look. This will be the last major CL ...
5 months, 2 weeks ago (2017-04-07 06:59:11 UTC) #6
oshima
can you also test Internal Secondary + External Primary start rotation unplug external while capturing ...
5 months, 2 weeks ago (2017-04-07 20:48:57 UTC) #7
wutao
Hi Oshima, I added a new test as you suggested. For the refactoring of CreateRotationTransform ...
5 months, 2 weeks ago (2017-04-10 08:01:07 UTC) #8
wutao
Hi Oshima, Friendly ping. Tao
5 months, 1 week ago (2017-04-13 16:46:24 UTC) #9
wutao
Hi Oshima, I moved some helper files to ash/utility/transformer_util. I only changed the following two ...
5 months, 1 week ago (2017-04-13 21:05:25 UTC) #10
oshima
https://codereview.chromium.org/2790583004/diff/60001/ash/display/root_window_transformers.h File ash/display/root_window_transformers.h (right): https://codereview.chromium.org/2790583004/diff/60001/ash/display/root_window_transformers.h#newcode22 ash/display/root_window_transformers.h:22: class Transform; do you need these changes? https://codereview.chromium.org/2790583004/diff/60001/ash/public/cpp/shell_window_ids.h File ...
5 months, 1 week ago (2017-04-13 23:51:47 UTC) #11
wutao
Hi Oshima, I cleaned it up. Thank you. Tao https://codereview.chromium.org/2790583004/diff/60001/ash/display/root_window_transformers.h File ash/display/root_window_transformers.h (right): https://codereview.chromium.org/2790583004/diff/60001/ash/display/root_window_transformers.h#newcode22 ash/display/root_window_transformers.h:22: ...
5 months, 1 week ago (2017-04-14 00:17:54 UTC) #12
wutao
Hi Oshima, I rebased on all the changes landed and not landed (in review). Please ...
5 months, 1 week ago (2017-04-14 02:27:40 UTC) #13
oshima
still half way. i'll review the rest tomorrow morning https://codereview.chromium.org/2790583004/diff/100001/ash/devtools/ash_devtools_unittest.cc File ash/devtools/ash_devtools_unittest.cc (right): https://codereview.chromium.org/2790583004/diff/100001/ash/devtools/ash_devtools_unittest.cc#newcode181 ash/devtools/ash_devtools_unittest.cc:181: ...
5 months, 1 week ago (2017-04-14 04:54:26 UTC) #14
wutao
On 2017/04/14 04:54:26, oshima wrote: > still half way. i'll review the rest tomorrow morning ...
5 months, 1 week ago (2017-04-14 05:53:09 UTC) #15
oshima
On 2017/04/14 05:53:09, wutao wrote: > On 2017/04/14 04:54:26, oshima wrote: > > still half ...
5 months, 1 week ago (2017-04-14 07:37:13 UTC) #16
wutao
Hi Oshima, Upload some changes for patch 6. Please review. Thank you, Tao https://codereview.chromium.org/2790583004/diff/100001/ash/devtools/ash_devtools_unittest.cc File ...
5 months, 1 week ago (2017-04-14 07:48:28 UTC) #17
wutao
Hi Oshima, I merged and uploaded the changes. Fixed a bug in previous patch that ...
5 months, 1 week ago (2017-04-15 00:07:41 UTC) #18
oshima
https://codereview.chromium.org/2790583004/diff/140001/ash/rotator/screen_rotation_animator.cc File ash/rotator/screen_rotation_animator.cc (right): https://codereview.chromium.org/2790583004/diff/140001/ash/rotator/screen_rotation_animator.cc#newcode327 ash/rotator/screen_rotation_animator.cc:327: screen_rotation_container_layer_->SetOpacity(0.0f); Do you need this? Putting the fully opaque ...
5 months, 1 week ago (2017-04-17 15:25:23 UTC) #23
oshima
fyi: looks like mash test are failing.
5 months, 1 week ago (2017-04-17 15:26:06 UTC) #24
wutao
Hi Oshima, I fixed several things in this patch: 1. Add two tests to remove ...
5 months, 1 week ago (2017-04-17 23:30:31 UTC) #26
oshima
close. once you fixes the double increase for pending request, i'll approve. https://codereview.chromium.org/2790583004/diff/160001/ash/rotator/screen_rotation_animator.cc File ash/rotator/screen_rotation_animator.cc ...
5 months, 1 week ago (2017-04-18 18:32:25 UTC) #31
wutao
Hi Oshima, Please take another look. Thank you, Tao https://codereview.chromium.org/2790583004/diff/160001/ash/rotator/screen_rotation_animator.cc File ash/rotator/screen_rotation_animator.cc (right): https://codereview.chromium.org/2790583004/diff/160001/ash/rotator/screen_rotation_animator.cc#newcode299 ash/rotator/screen_rotation_animator.cc:299: ...
5 months, 1 week ago (2017-04-18 19:56:58 UTC) #32
oshima
https://codereview.chromium.org/2790583004/diff/180001/ash/rotator/screen_rotation_animator.cc File ash/rotator/screen_rotation_animator.cc (right): https://codereview.chromium.org/2790583004/diff/180001/ash/rotator/screen_rotation_animator.cc#newcode466 ash/rotator/screen_rotation_animator.cc:466: StartRotationAnimation(std::move(last_pending_request_)); This will not stop the currently running animation, ...
5 months, 1 week ago (2017-04-19 00:59:02 UTC) #37
wutao
Hi Oshima, ptal. Thanks, Tao https://codereview.chromium.org/2790583004/diff/180001/ash/rotator/screen_rotation_animator.cc File ash/rotator/screen_rotation_animator.cc (right): https://codereview.chromium.org/2790583004/diff/180001/ash/rotator/screen_rotation_animator.cc#newcode466 ash/rotator/screen_rotation_animator.cc:466: StartRotationAnimation(std::move(last_pending_request_)); On 2017/04/19 00:59:01, ...
5 months, 1 week ago (2017-04-19 06:32:52 UTC) #38
oshima
let's discuss offline today https://codereview.chromium.org/2790583004/diff/200001/ash/rotator/screen_rotation_animator.cc File ash/rotator/screen_rotation_animator.cc (left): https://codereview.chromium.org/2790583004/diff/200001/ash/rotator/screen_rotation_animator.cc#oldcode408 ash/rotator/screen_rotation_animator.cc:408: screen_rotation_state_ = IDLE; looks like ...
5 months ago (2017-04-19 15:07:41 UTC) #43
wutao
Hi Oshima, ptal. Thanks, Tao https://codereview.chromium.org/2790583004/diff/200001/ash/rotator/screen_rotation_animator.cc File ash/rotator/screen_rotation_animator.cc (left): https://codereview.chromium.org/2790583004/diff/200001/ash/rotator/screen_rotation_animator.cc#oldcode408 ash/rotator/screen_rotation_animator.cc:408: screen_rotation_state_ = IDLE; On ...
5 months ago (2017-04-19 17:48:32 UTC) #44
wutao
Hi Oshima, ptal. Thanks, Tao
5 months ago (2017-04-19 17:48:34 UTC) #45
wutao
On 2017/04/19 17:48:34, wutao wrote: > Hi Oshima, > > ptal. > > Thanks, > ...
5 months ago (2017-04-19 19:01:52 UTC) #50
oshima
https://codereview.chromium.org/2790583004/diff/220001/ash/rotator/screen_rotation_animator.h File ash/rotator/screen_rotation_animator.h (right): https://codereview.chromium.org/2790583004/diff/220001/ash/rotator/screen_rotation_animator.h#newcode153 ash/rotator/screen_rotation_animator.h:153: } last one. instead, just append switch in the ...
5 months ago (2017-04-19 22:56:52 UTC) #51
wutao
Hi Oshima, Please take another look. Thank you, Tao https://codereview.chromium.org/2790583004/diff/220001/ash/rotator/screen_rotation_animator.h File ash/rotator/screen_rotation_animator.h (right): https://codereview.chromium.org/2790583004/diff/220001/ash/rotator/screen_rotation_animator.h#newcode153 ash/rotator/screen_rotation_animator.h:153: ...
5 months ago (2017-04-20 00:32:52 UTC) #52
oshima
lgtm thanks for your patience!
5 months ago (2017-04-20 01:45:54 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/2790583004/240001
5 months ago (2017-04-20 02:12:06 UTC) #59
commit-bot: I haz the power
5 months ago (2017-04-20 02:17:50 UTC) #62
Message was sent while issue was closed.
Committed patchset #13 (id:240001) as
https://chromium.googlesource.com/chromium/src/+/6d394112c70e0edb2e8f1464bcb9...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b