New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 678763 link

Starred by 4 users

Issue metadata

Status: Verified
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 707800


Participants' hotlists:
Fixing-touch


Sign in to add a comment

rotation animation is janky.

Project Member Reported by abodenha@chromium.org, Jan 5 2017

Issue description

When in touchview, flipping between portrait and landscape should play a 15 frame animation.  Tests to track this were recently added (b/31608632) and show:
minnie: 4
cyan: 6
samus: 13
elm: 4

It's pretty bad that we can't even play the full 15 frames on samus and more typical devices are REALLY bad.



 

Comment 1 by willg...@gmail.com, Feb 1 2017

Android apps are particularly horrible in comparison to the browser and Chrome apps. Will this issue address that specifically?
Owner: wutao@chromium.org
willg76x@ this is about one specific animation that is bad when switching orientation. Nothing Android related here.

There is a LOT of unrelated work on making Android apps better in general.  If you see issues please file bugs and include as much detail as you can.

Comment 3 by willg...@gmail.com, Feb 1 2017

sorry, should have said the rotation behavior was bad in arc apps.

I'll open another issue if there isn't one already.
willg76x@ ah, got it.  This is just for the system level UI animation.

Thanks for filing bugs. If you can grab a video it's really helpful.

Comment 5 by wutao@chromium.org, Feb 2 2017

I am using elm and the same FPS measurement in b/31608632 for the rotation animation improvement.

Current animation is done with the following steps:
c-1. Make a copy of screen layers before rotation (old_layer_tree = ::wm::RecreateLayers(root_window))
c-2. Update the display manager with the new rotation
c-3. For each child layer of root_window->layer(), perform a rotation animation from kRotationDegrees (20 degrees) to rotated position
c-4. For the old_layer_tree->root() layer, perform a rotation animation from initial_rotation_degrees to (initial_rotation_degrees - kRotationDegrees)


Discussed with bruthig@ and come up one solution to test:
p-1. Take a snapshot of the screen before rotation
p-2. Update the display manager with the new rotation
p-3. Take a snapshot of the screen after rotation
p-4. Use the 2 snapshots for the actual animation, i.e. use p-1 snapshot animation to replace c-4 and p-3 snapshot animation to replace c-3.


Comment 6 Deleted

Comment 7 Deleted

Comment 8 by wutao@chromium.org, Feb 2 2017

I tried several combinations of animations of c-3 and c-4.

Here is the FPS measurements (3 times and take average) for elm:
Original Test 1. Original code (no change): (6 + 5 + 4) / 3 => avg 5 frames
Original Test 2. Remove steps c-1, c-3, and c-4, just keep c-2: (11 + 10 + 12) / 3 => avg 11 frames
Original Test 3. Keep c-2 and c-3: (7 + 7 + 6) / 3 => avg 7 frames
Original Test 4. Keep c-1, c-2, and c-4: (9 + 9 + 10) / 3 => avg 9 frames

Comment 9 by wutao@chromium.org, Feb 2 2017

I implemented p-1 and use it to replace the animation of c-4. The followings are some tested results on elm:

Improvement Test 1, (No animation) By putting only step p-2 in the RequestCopyOfOutput callback of original screen layers and remove all the other animations: (12 + 13 + 13) / 3 => avg 13 frames

Improvement Test 2, (p-1 animation) By putting steps p-2 and p-1 screenshot animation in the RequestCopyOfOutput callback of original screen layers: (15 + 14 + 15) / 3 => avg 15 frames

Improvement Test 3, (full/similar to current animation) By putting steps p-2, p-1 screenshot animation, and  c-3 animations of all rotated layers in the RequestCopyOfOutput callback of original screen layers: (8 + 8  + 8) / 3 => avg 8 frames


Two questions need to figure out:
1. Why the frame rate in Improvement Test 1 and 2 (Update the display manager with the new rotation in the RequestCopyOfOutput callback) are both better than before (Original Test 2 and Original Test 4?
2. Why the FPS in Improvement Test 2 is better than Improvement Test 1 (15 vs 13), even do more animation?
Cc: afakhry@chromium.org danakj@chromium.org wutao@chromium.org bruthig@chromium.org
TODO: Implement p-3 in comment 5. Here is the workflow if make sense. Need to figure out how f-2 works.

f-1. In RotateScreen(), RequestCopyOfOutput on root_window->layer() with callback_original_screenshot
f-2. In callback_original_screenshot, call display_manager()->SetDisplayRotation(), and should be called when the new layout of the rotated view/layers is finished (callback_screen_rotated)
f-3. In callback_screen_rotated, RequestCopyOfOutput on rotated root_window->layer() with callback_rotated_screenshot
f-4. In callback_rotated_screenshot, perform animations of p-4

As danakj@ suggested, we might combine f-2 and f-3, i.e. insert RequestCopyOfOutput after the SetDisplayRotation(), to reduce one callback. But this may require to change some current code behavior.

Cc: mccanny@chromium.org

Comment 12 by wutao@chromium.org, Feb 28 2017

Have a working solution at local branch, which will improve the frame rate to about 15 on kevin and elm.

Will upload CLs incrementally.
The first one is to unify the code path calling ScreenRotationAnimator::Rotate through display_configuration_controller SetDisplayRotation. Further screen rotation animation improvement will base on this change.

https://codereview.chromium.org/2720913003/

Comment 13 Deleted

Uploaded another cl to handle users rotating screen too fast: https://codereview.chromium.org/2728803002

Maintain a state |display_animator_map_| in
display_configuration_controller.
If there is a new rotation request, it should populate the map with
display_id and create a new screen_rotation_animator.
If there is a new rotation request during rotation animation, it should
stop the animation immediately and add the new rotation request to the
|last_pending_request_|.
If there is no new request, in the animation observer, it should delete
the screen_rotation_animator in the map.


Project Member

Comment 15 by bugdroid1@chromium.org, Mar 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4b326a561bf0feaae081fe6f2037cde4d25a8fd9

commit 4b326a561bf0feaae081fe6f2037cde4d25a8fd9
Author: wutao <wutao@chromium.org>
Date: Thu Mar 02 17:10:35 2017

Calls display_configuration_controller to rotate screen in accelerator_controller_delegate_aura

This unifies the code path calling ScreenRotationAnimator::Rotate through display_configuration_controller SetDisplayRotation. Further screen rotation animation improvement will base on this change.

BUG= 678763 
TEST=manual, unit_tests
R=bruthig@chromium.org, oshima@chromium.org

Review-Url: https://codereview.chromium.org/2720913003
Cr-Commit-Position: refs/heads/master@{#454288}

[modify] https://crrev.com/4b326a561bf0feaae081fe6f2037cde4d25a8fd9/ash/accelerators/accelerator_controller_delegate_aura.cc
[modify] https://crrev.com/4b326a561bf0feaae081fe6f2037cde4d25a8fd9/ash/accelerators/accelerator_controller_unittest.cc
[modify] https://crrev.com/4b326a561bf0feaae081fe6f2037cde4d25a8fd9/ash/display/display_configuration_controller.cc
[modify] https://crrev.com/4b326a561bf0feaae081fe6f2037cde4d25a8fd9/ash/display/display_configuration_controller.h
[modify] https://crrev.com/4b326a561bf0feaae081fe6f2037cde4d25a8fd9/ash/display/screen_orientation_controller_chromeos.cc
[modify] https://crrev.com/4b326a561bf0feaae081fe6f2037cde4d25a8fd9/chrome/browser/extensions/display_info_provider_chromeos.cc
[modify] https://crrev.com/4b326a561bf0feaae081fe6f2037cde4d25a8fd9/chrome/browser/ui/webui/options/chromeos/display_options_handler.cc
[modify] https://crrev.com/4b326a561bf0feaae081fe6f2037cde4d25a8fd9/tools/metrics/actions/actions.xml

Project Member

Comment 16 by bugdroid1@chromium.org, Mar 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6ddafa38e3e8bfc0b856149a6a9366c7b0da80f4

commit 6ddafa38e3e8bfc0b856149a6a9366c7b0da80f4
Author: wutao <wutao@chromium.org>
Date: Sat Mar 18 03:10:08 2017

Handles users rotating screen too early

If the user rotates screen too early while the previous animation is not
finished yet, it will create another |old_layer_tree| with additional
layers of previous |old_layer_tree|. The solution is that if there is a
new rotation request during rotation animation, it should stop the
animation immediately and start the new rotation request.

BUG= 678763 
TEST=manual, unit_tests
R=oshima@chromium.org, bruthig@chromium.org

Review-Url: https://codereview.chromium.org/2728803002
Cr-Commit-Position: refs/heads/master@{#457946}

[modify] https://crrev.com/6ddafa38e3e8bfc0b856149a6a9366c7b0da80f4/ash/BUILD.gn
[modify] https://crrev.com/6ddafa38e3e8bfc0b856149a6a9366c7b0da80f4/ash/display/display_configuration_controller.cc
[modify] https://crrev.com/6ddafa38e3e8bfc0b856149a6a9366c7b0da80f4/ash/display/display_configuration_controller.h
[add] https://crrev.com/6ddafa38e3e8bfc0b856149a6a9366c7b0da80f4/ash/display/display_configuration_controller_unittest.cc
[modify] https://crrev.com/6ddafa38e3e8bfc0b856149a6a9366c7b0da80f4/ash/rotator/screen_rotation_animator.cc
[modify] https://crrev.com/6ddafa38e3e8bfc0b856149a6a9366c7b0da80f4/ash/rotator/screen_rotation_animator.h
[add] https://crrev.com/6ddafa38e3e8bfc0b856149a6a9366c7b0da80f4/ash/rotator/screen_rotation_animator_observer.h
[add] https://crrev.com/6ddafa38e3e8bfc0b856149a6a9366c7b0da80f4/ash/rotator/screen_rotation_animator_unittest.cc
[add] https://crrev.com/6ddafa38e3e8bfc0b856149a6a9366c7b0da80f4/ash/rotator/test/screen_rotation_animator_test_api.cc
[add] https://crrev.com/6ddafa38e3e8bfc0b856149a6a9366c7b0da80f4/ash/rotator/test/screen_rotation_animator_test_api.h
[modify] https://crrev.com/6ddafa38e3e8bfc0b856149a6a9366c7b0da80f4/ash/test/BUILD.gn
[modify] https://crrev.com/6ddafa38e3e8bfc0b856149a6a9366c7b0da80f4/ash/test/ash_test_helper.cc
[add] https://crrev.com/6ddafa38e3e8bfc0b856149a6a9366c7b0da80f4/ash/test/display_configuration_controller_test_api.cc
[add] https://crrev.com/6ddafa38e3e8bfc0b856149a6a9366c7b0da80f4/ash/test/display_configuration_controller_test_api.h
[modify] https://crrev.com/6ddafa38e3e8bfc0b856149a6a9366c7b0da80f4/ash/test/shell_test_api.cc
[modify] https://crrev.com/6ddafa38e3e8bfc0b856149a6a9366c7b0da80f4/ash/test/shell_test_api.h
[modify] https://crrev.com/6ddafa38e3e8bfc0b856149a6a9366c7b0da80f4/ui/display/manager/display_manager.cc
[modify] https://crrev.com/6ddafa38e3e8bfc0b856149a6a9366c7b0da80f4/ui/display/manager/display_manager.h

wutao@ what do you think about instead of stopping the animation and starting a new rotation request, to reverse the current animation from the frame it was on?

Comment 18 by wutao@chromium.org, Mar 20 2017

omrilio@, the original thoughts were 1) stop the animation to its target and continue new rotation request. 2) abort the animation to its original position, and start a rotation with combination of old rotation angle + new rotation angle. If user sends another request, then we need to start a rotation with combination of old rotation angle, new rotation angle 1, new rotation angle 2... etc. 3) oshima@ suggested in the code review to consider, start animation from current animating position, which should generate more smooth animation.
The also need to queue up all the requests and handle them a single request.

We can investigate how 2) and 3) can be done. One more thing we need to handle is that: Current code does not store the old rotation (current rotation) before rotating. The current rotation is obtained from the display manager. To implement 2) and 3), the current rotation will not necessary equal to old rotation any more.
Sounds good, thanks wutao@. 
Also, 3) sounds like a good idea! 

Comment 20 by wutao@chromium.org, Mar 24 2017

Uploaded a cl to add a histogram of screen rotation animation smoothness:
https://codereview.chromium.org/2771713004
Project Member

Comment 21 by bugdroid1@chromium.org, Mar 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3f08f81806674e0e9882a09c6f97852b184628b5

commit 3f08f81806674e0e9882a09c6f97852b184628b5
Author: wutao <wutao@chromium.org>
Date: Sat Mar 25 03:06:33 2017

Adds UMA for screen rotation animation smoothness.

Adding histgram of screen rotation animation smoothness.

BUG= 678763 
TEST=manual, created histogram locally.

Review-Url: https://codereview.chromium.org/2771713004
Cr-Commit-Position: refs/heads/master@{#459635}

[modify] https://crrev.com/3f08f81806674e0e9882a09c6f97852b184628b5/ash/rotator/screen_rotation_animator.cc
[modify] https://crrev.com/3f08f81806674e0e9882a09c6f97852b184628b5/ash/rotator/screen_rotation_animator.h
[modify] https://crrev.com/3f08f81806674e0e9882a09c6f97852b184628b5/tools/metrics/histograms/histograms.xml

Comment 22 by wutao@chromium.org, Mar 29 2017

Uploaded a cl to add one NOT_DRAWN layer in between the root_window's layer and its current children. Currently we start animation for each child of root_window. By adding a NOT_DRAWN layer, we only need to initiate one animation sequence for these children.

https://codereview.chromium.org/2786563003
What impact does this have on the events you see happening in chrome?

Comment 24 by wutao@chromium.org, Mar 29 2017

Cc: osh...@chromium.org
#23, danakj@, it is a prep for later cls to take second copy output request after rotation and revert the rotation on the first copy output result layer.
Moreover, just initiating one animator makes the code simpler and possible* more efficient (but not measurable with it alone by Chameloen board). Currently we have about 7 layers so need to initiate 7 animation sequences.


Comment 25 by wutao@chromium.org, Mar 29 2017

Uploaded a cl to use copy request to flattern the layers to do screen rotation animation. Currently we clone all the layers to do animation during screen
rotation, which is slow on some of the devices. The solution is using
compositor copy request to flatten the layer hierarchy and make
animations more efficient.

https://codereview.chromium.org/2780823002/

Comment 26 by wutao@chromium.org, Mar 29 2017

Uploaded a cl to get webcontents in ash. When screen rotates, we can observe the
compositor frame swap on the webcontents so that to send the second copy
output request after screen rotation.

https://codereview.chromium.org/2782263002
Blocking: 707800
Added an issue to clean up the flag to turn on/off smooth animation after this bug is fixed:
https://bugs.chromium.org/p/chromium/issues/detail?id=707800
Project Member

Comment 29 by bugdroid1@chromium.org, Apr 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2c0ca180a3b0db00edd152ed5b2a703942d84163

commit 2c0ca180a3b0db00edd152ed5b2a703942d84163
Author: wutao <wutao@chromium.org>
Date: Thu Apr 06 22:40:54 2017

Uses copy request to flatten the layers to do screen rotation animation.

Currently we clone all the layers to do animation during screen
rotation, which is slow on some of the devices. The solution is using
compositor copy request to flatten the layer hierarchy and make
animations more efficient.

BUG= 678763 
R=oshima@chromium.org, bruthig@chromium.org
TEST=Unittest with ScreenRotationAnimatorTest and Manual

Review-Url: https://codereview.chromium.org/2780823002
Cr-Commit-Position: refs/heads/master@{#462658}

[modify] https://crrev.com/2c0ca180a3b0db00edd152ed5b2a703942d84163/ash/DEPS
[modify] https://crrev.com/2c0ca180a3b0db00edd152ed5b2a703942d84163/ash/common/ash_switches.cc
[modify] https://crrev.com/2c0ca180a3b0db00edd152ed5b2a703942d84163/ash/common/ash_switches.h
[modify] https://crrev.com/2c0ca180a3b0db00edd152ed5b2a703942d84163/ash/rotator/screen_rotation_animator.cc
[modify] https://crrev.com/2c0ca180a3b0db00edd152ed5b2a703942d84163/ash/rotator/screen_rotation_animator.h
[modify] https://crrev.com/2c0ca180a3b0db00edd152ed5b2a703942d84163/ash/rotator/screen_rotation_animator_unittest.cc
[modify] https://crrev.com/2c0ca180a3b0db00edd152ed5b2a703942d84163/chrome/browser/about_flags.cc
[modify] https://crrev.com/2c0ca180a3b0db00edd152ed5b2a703942d84163/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/2c0ca180a3b0db00edd152ed5b2a703942d84163/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/2c0ca180a3b0db00edd152ed5b2a703942d84163/tools/metrics/histograms/histograms.xml

Uploaded a cl to send the second copy request after screen rotation. This will work with the compositor resize lock (thanks to danakj@ who fixed it) to get the right content layout after rotation.
https://codereview.chromium.org/2790583004/


Comment 31 by wutao@chromium.org, Apr 12 2017

Uploaded a cl to handle new screen rotation request while waiting for the copy request callback.

If there is a new screen rotation request while it is waiting for the
layer copy result, we should process the new rotation request and
ignore the previous layer copy result.

https://codereview.chromium.org/2816683002
Project Member

Comment 32 by bugdroid1@chromium.org, Apr 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/88733dff81ecec04786aaee3cf48f4bcd776cd80

commit 88733dff81ecec04786aaee3cf48f4bcd776cd80
Author: wutao <wutao@chromium.org>
Date: Thu Apr 13 19:39:06 2017

Handle new screen rotation request while waiting for the copy request callback.

If there is a new screen rotation request while it is waiting for the
layer copy result, we should process the new rotation request and
ignore the previous layer copy result.

BUG= 678763 
TEST=Manual

Review-Url: https://codereview.chromium.org/2816683002
Cr-Commit-Position: refs/heads/master@{#464508}

[modify] https://crrev.com/88733dff81ecec04786aaee3cf48f4bcd776cd80/ash/rotator/screen_rotation_animator.cc
[modify] https://crrev.com/88733dff81ecec04786aaee3cf48f4bcd776cd80/ash/rotator/screen_rotation_animator.h

Comment 33 by wutao@chromium.org, Apr 13 2017

Landed two cls to handle new screen rotation request while waiting for the copy request callback.
1. https://codereview.chromium.org/2809553002 
2. https://codereview.chromium.org/2816683002
Project Member

Comment 34 by bugdroid1@chromium.org, Apr 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/335760818a29274b57f59d23f555bf6a3b60fb3e

commit 335760818a29274b57f59d23f555bf6a3b60fb3e
Author: wutao <wutao@chromium.org>
Date: Fri Apr 14 18:51:10 2017

Add a NOT_DRAWN window in between the root_window and its children.

For screen rotation animation, currently we start animation for each child
layer of root_window's layer. By adding a NOT_DRAWN window, we only need to
initiate one animation sequence for these children.

BUG= 678763 
R=oshima@chromium.org
TEST=Manual

Review-Url: https://codereview.chromium.org/2786563003
Cr-Commit-Position: refs/heads/master@{#464762}

[modify] https://crrev.com/335760818a29274b57f59d23f555bf6a3b60fb3e/ash/devtools/ash_devtools_unittest.cc
[modify] https://crrev.com/335760818a29274b57f59d23f555bf6a3b60fb3e/ash/public/cpp/shell_window_ids.h
[modify] https://crrev.com/335760818a29274b57f59d23f555bf6a3b60fb3e/ash/root_window_controller.cc
[modify] https://crrev.com/335760818a29274b57f59d23f555bf6a3b60fb3e/ash/rotator/screen_rotation_animator.cc
[modify] https://crrev.com/335760818a29274b57f59d23f555bf6a3b60fb3e/ash/wm/root_window_layout_manager.cc

Project Member

Comment 35 by bugdroid1@chromium.org, Apr 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6d394112c70e0edb2e8f1464bcb9f29dfa54bbb7

commit 6d394112c70e0edb2e8f1464bcb9f29dfa54bbb7
Author: wutao <wutao@chromium.org>
Date: Thu Apr 20 02:17:35 2017

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}

[modify] https://crrev.com/6d394112c70e0edb2e8f1464bcb9f29dfa54bbb7/ash/BUILD.gn
[modify] https://crrev.com/6d394112c70e0edb2e8f1464bcb9f29dfa54bbb7/ash/display/display_configuration_controller.cc
[modify] https://crrev.com/6d394112c70e0edb2e8f1464bcb9f29dfa54bbb7/ash/display/display_configuration_controller.h
[modify] https://crrev.com/6d394112c70e0edb2e8f1464bcb9f29dfa54bbb7/ash/display/display_configuration_controller_unittest.cc
[modify] https://crrev.com/6d394112c70e0edb2e8f1464bcb9f29dfa54bbb7/ash/display/root_window_transformers.cc
[modify] https://crrev.com/6d394112c70e0edb2e8f1464bcb9f29dfa54bbb7/ash/rotator/screen_rotation_animator.cc
[modify] https://crrev.com/6d394112c70e0edb2e8f1464bcb9f29dfa54bbb7/ash/rotator/screen_rotation_animator.h
[modify] https://crrev.com/6d394112c70e0edb2e8f1464bcb9f29dfa54bbb7/ash/rotator/screen_rotation_animator_unittest.cc
[modify] https://crrev.com/6d394112c70e0edb2e8f1464bcb9f29dfa54bbb7/ash/rotator/test/screen_rotation_animator_test_api.cc
[add] https://crrev.com/6d394112c70e0edb2e8f1464bcb9f29dfa54bbb7/ash/utility/transformer_util.cc
[add] https://crrev.com/6d394112c70e0edb2e8f1464bcb9f29dfa54bbb7/ash/utility/transformer_util.h
[modify] https://crrev.com/6d394112c70e0edb2e8f1464bcb9f29dfa54bbb7/testing/buildbot/filters/ash_mus_unittests.filter

Comment 36 by wutao@chromium.org, Apr 26 2017

Uploaded a cl to flip the flag to enable the improved screen rotation animation by default: https://codereview.chromium.org/2837773003/
Project Member

Comment 37 by bugdroid1@chromium.org, Apr 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b161bf2135de81cd904dbdce7e1398c7c2f063c7

commit b161bf2135de81cd904dbdce7e1398c7c2f063c7
Author: wutao <wutao@chromium.org>
Date: Wed Apr 26 16:55:03 2017

Flip the flag to enable smooth screen rotation by default.

Flip the flag to enable the smooth screen rotation by default. Fix some
tests which evaluate the rotation results immediately. In those cases,
skipping the screen rotation animation.

BUG= 678763 
TEST=Manual and AboutFlagsHistogramTest.CheckHistograms

Review-Url: https://codereview.chromium.org/2837773003
Cr-Commit-Position: refs/heads/master@{#467348}

[modify] https://crrev.com/b161bf2135de81cd904dbdce7e1398c7c2f063c7/ash/ash_switches.cc
[modify] https://crrev.com/b161bf2135de81cd904dbdce7e1398c7c2f063c7/ash/ash_switches.h
[modify] https://crrev.com/b161bf2135de81cd904dbdce7e1398c7c2f063c7/ash/rotator/screen_rotation_animator.cc
[modify] https://crrev.com/b161bf2135de81cd904dbdce7e1398c7c2f063c7/ash/rotator/screen_rotation_animator.h
[modify] https://crrev.com/b161bf2135de81cd904dbdce7e1398c7c2f063c7/ash/rotator/screen_rotation_animator_unittest.cc
[modify] https://crrev.com/b161bf2135de81cd904dbdce7e1398c7c2f063c7/ash/test/ash_test_base.cc
[modify] https://crrev.com/b161bf2135de81cd904dbdce7e1398c7c2f063c7/ash/test/ash_test_helper.cc
[modify] https://crrev.com/b161bf2135de81cd904dbdce7e1398c7c2f063c7/ash/test/ash_test_helper.h
[modify] https://crrev.com/b161bf2135de81cd904dbdce7e1398c7c2f063c7/chrome/browser/about_flags.cc
[modify] https://crrev.com/b161bf2135de81cd904dbdce7e1398c7c2f063c7/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/b161bf2135de81cd904dbdce7e1398c7c2f063c7/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/b161bf2135de81cd904dbdce7e1398c7c2f063c7/tools/metrics/histograms/histograms.xml

Comment 38 by wutao@chromium.org, Apr 28 2017

I am adding an animator lock for screen rotation. We can add multiple locks if we want to wait for multiple event to finish. In this cl, only wait for wallpaper resize and calculate color.

When screen rotates, wallpaper needs to be resized accordingly, which
takes time. Adding a screen rotation animator lock, allows us to second
copy output request after the wallpaper is resized.

CL: https://codereview.chromium.org/2848883004/


Comment 39 by wutao@chromium.org, May 10 2017

Status: Started (was: Assigned)
Uploaded a cl to allow compositor lock to extend timeout.
This will be used in wallpaper resize and ARC apps.
The screen rotation animation will send the copy request after the compositor locks were released or timed out.

https://codereview.chromium.org/2870023002/

Comment 40 by wutao@chromium.org, May 10 2017

Uploaded a cl to wait wallpaper resize in screen rotation animation.
https://codereview.chromium.org/2872123002/
Project Member

Comment 41 by bugdroid1@chromium.org, May 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c5cabd63ff5c51e67270774173cc0f66059cf341

commit c5cabd63ff5c51e67270774173cc0f66059cf341
Author: wutao <wutao@chromium.org>
Date: Wed May 10 18:54:10 2017

Allow compositor locks to extend timeout.

Currently compositor lock will set a timeout with the timeout of the
first lock request. Incoming future locks will timeout with the same
time. Adding a flag to allow compositor locks to extend timeout based on
the most future timeout of all the locks when the flag is true.

BUG= 678763 
TEST=CompositorTestWithMockedTime.* && tested in screen rotation animation with wallpaper resize

Review-Url: https://codereview.chromium.org/2870023002
Cr-Commit-Position: refs/heads/master@{#470658}

[modify] https://crrev.com/c5cabd63ff5c51e67270774173cc0f66059cf341/ui/compositor/compositor.cc
[modify] https://crrev.com/c5cabd63ff5c51e67270774173cc0f66059cf341/ui/compositor/compositor.h
[modify] https://crrev.com/c5cabd63ff5c51e67270774173cc0f66059cf341/ui/compositor/compositor_unittest.cc

Project Member

Comment 42 by bugdroid1@chromium.org, May 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eb552059c7c48312cd176433b370481e211e8987

commit eb552059c7c48312cd176433b370481e211e8987
Author: wutao <wutao@chromium.org>
Date: Wed May 10 22:35:29 2017

Wallpaper resize extents compositor lock timeout.

Sometime wallpaper has not been resized when screen rotation animating.
Users can see a wallpaper resize after rotates the screen. The solution
is that to acquire compositor lock when resizing wallpaper so that to
prevent Screen Rotation Animator animating.

BUG= 678763 
Test=tested locally that wallpaper was resized before screen rotation
animation.

Review-Url: https://codereview.chromium.org/2872123002
Cr-Commit-Position: refs/heads/master@{#470735}

[modify] https://crrev.com/eb552059c7c48312cd176433b370481e211e8987/ash/rotator/screen_rotation_animator.cc
[modify] https://crrev.com/eb552059c7c48312cd176433b370481e211e8987/ash/wallpaper/wallpaper_controller.cc
[modify] https://crrev.com/eb552059c7c48312cd176433b370481e211e8987/ash/wallpaper/wallpaper_controller.h

Status: Fixed (was: Started)
The improvements are in Dev channel and there is significant improvement across different board from UMA.

Rotation Animation Smoothness 50% percentile (average):
KEVIN: from 23% to 60%
ELM: from 36% to 70%
MINNIE: from 43% to 73%
CYAN: from 53% to 83%

I will close this issue for now.
There could be further improvement but will be tracked in independent issue. 
Cc: tedlai@chromium.org
Ted is working on a benchmark to measure ARC++ rotation performance. CC'ed him here and will file new bugs if needed.
Status: Verified (was: Fixed)

Sign in to add a comment