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

Issue 738185 link

Starred by 2 users

Issue metadata

Status: Archived
Owner:
Closed: Jul 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug


Participants' hotlists:
LoginRefresh


Sign in to add a comment

Blurred wallpaper causes significant (3x) animation slowdown

Project Member Reported by jdufault@chromium.org, Jun 29 2017

Issue description

In views-based lock screen, enabling blur on the wallpaper significantly slows down any animation, ie, switching a user goes from ~59fps to ~23fps on kevin. This makes the animation feel very jarring and is a significant regression from the existing webui implementation.

The wallpaper is being blurred, which is static content. The animation does not use blur (but it does use transparency).
 
Labels: M-62

Comment 2 by r...@chromium.org, Jun 30 2017

Cc: abodenha@chromium.org
Is this a known issue? If so, is there an estimate to fix this?
This is very, very important for our views based lock screen that we intend to launch literally right after Eve launch.

This and  bug 732539  sound like they should both be merged into bug 646062 but I'll let wutau@ decide.

Comment 4 by wutao@chromium.org, Jun 30 2017

jdufault@, if you have a chance, could you measure the fps on eve.
From the screen rotation animation, I noticed that samus is much faster than kevin. (90% vs 60% animation smoothness).
I wonder here is the same, on eve the performance may close to 40-50fps.


Comment 5 by wutao@chromium.org, Jun 30 2017

There are two issues for the blur: a) how to make the blur faster and b) how to cache the blur results.
1)  bug 732539 , is the first time to launch blur, so mainly related to a).
2) this  bug 738185 , will be related to a) and b), but could be improved if we can cache the blurred background.

reveman@ and I are experimental a) right now. The final performance would highly depend on how Chrome to do occlusion culling.
If occlusion is not efficient, the prototype could make it even slow.

For b), it is even more complicated and we do not have a good mechanism to invalidate the cache. And if there is something animating under the blurring layer, we cannot cache at all.
For the lock screen, is it true we will only show the blurred wallpaper? If yes, I wonder if reveman@ would agree that we can do special handling to cache the blurred wallpaper.


Comment 6 by wutao@chromium.org, Jul 12 2017

Cc: senorblanco@chromium.org
+senorblanco@

We had a chance to test the fps on EVE, the fps is about 50fps with blur turned on.

For kevin, here are some benchmarks:
1. ~20fps, set background filter, (radius 20.0f).
2. ~9fps, set background filter, (radius 2.0f**there is a ctx.ctm make 2x scale in kevin**, which make it final sigma 4.0 at skia blur, no scaling at skia blur side).

3. ~23fps, set background filter, (radius 20.0f) commented out x, y Gaussian convolution, keep down/up scaling.
4. ~29fps, set background filter, (radius 20.0f) only down scaling.
5. ~40fps, set background filter, (radius 20.0f) but return at the beginning of GaussianBlur().
6. ~55fps, not set background filter.

Scaling seems do speed up the blur but also takes some time.

Comment 7 by wutao@chromium.org, Jul 12 2017

For link, the fps is ~39fps with blur (20.0f).
I think we have a good long term plan for making this fast across all devices. I'd rather not have us side tracked for small short term gains on specific devices that doesn't align with our long term plan.

It sounds like the login/lock-screen blur can be implemented without using background blur. Ie. blur the background layer instead of using a background blur filter on the UI on top of it. That will always be faster and makes sense to do when possible even in the future when we have fast background blur support. Has this been considered?

Comment 9 by wutao@chromium.org, Jul 12 2017

reveman@, as in comment 5, if we always minimize all apps and only show wallpaper, we can pre-blur wallpaper and cached the blurred wallpaper. We can invalidate the cache when there is display configuration changes.

or do you mean that we real time calculate the blur of background: like 1) copy output request of background layer, 2) generate blur for the copy, 3) insert blurred layer to root_window_layer, and 4) show lock screen?
We should add API so we can add blur effect to the layer we need. Without any copy request and such. This is so much simpler then background blur that if it's useful we should have API for it.

Comment 11 by wutao@chromium.org, Jul 12 2017

Cc: robertphillips@chromium.org
Sounds perfect for wallpaper since we have SkBitmap for wallpaper. We can use SkPaint and SkCanvas on it directly.

For other any arbitrary layers, do we still need to convert the layer to SkBitmap by copyOutputOf?, or we have other way to call Skia Blur function on layers?


I think we should just expose cc:Layer::SetFilters to UI for this: https://cs.chromium.org/chromium/src/cc/layers/layer.h?l=147

when combined with forced render surface and caching it should provide good performance on all devices in situations where we can use this simpler filter mechanism.

Comment 13 by wutao@chromium.org, Jul 14 2017

Cc: danakj@chromium.org
It will works for a single layer with fullscreen size behind the lock screen. wallpaper in this case could work.

But this method may not work for the new launcher blur, which still requires background_filter. We may not find a single layer beneath the new launcher widget.
Yes, this mechanism will only work for the login/lock screen UI where we control the contents that needs to be blurred. Real background blur will still need to be improved as planned but if there's interest in getting something working for the login/lock screen before that then this would be good solution as it would be worthwhile to have even in the future when we have fast background blur.

I think the launcher and window switcher should use a simple opacity filter until we've implemented efficient blur for ChromeOS.
This bug is reporting 3x slow down on a machine with a decent GPU. It's going to be much worse on some older Chromebooks. I'd like to suggest the following plan of action:

1. Land render surface (RS) caching and forced RS usage support for UI.
2. Expose cc::Layer::SetFilters and blur effect to UI through ui::Layer
3. Use 1) and 2) to implement a very efficient blur for login/lock screen.
4. Implement fast background blur using down sampling when compositing (significantly more work than above items).

Chrome OS UI like launcher and window switcher will use opacity in place of background blur until 4) has been completed. Login/lock screen and similar situations where what needs to be blurred is contained in a layer can use 3) for ideal performance. That should be fast enough for those specific cases even without 4).
Yes, there is definitely interest in making this happen for login/lock, as we're currently moving blur out of webui due to load delay issues.
Labels: -M-62 M-61
Updating bug to m61 as login/lock is targeting m61 as well.

Comment 18 by wutao@chromium.org, Jul 14 2017

sg.

For step 1, land RS caching. The cl is in final stage to land, reveman@ PTAL on this cl: https://codereview.chromium.org/2873593002

I can work on step 2 now.

Comment 19 by wutao@chromium.org, Jul 17 2017

Tried a quick prototype on step 2 on kevin,

The fps is ~35fps compared to the background filter with ~20fps.

The render surface cache does not improve fps since we still need to calculate the filter_image for every frame. We need to come up a method to cache the blurred filter_image as well.



I wasn't expecting much difference without caching so 35 instead of 20 fps is nice. The benefit is that caching becomes easier with this approach. How did you try to set up the caching? Is caching working if you move the blurred layer into a parent layer and enable render surface caching on the parent layer?

Comment 21 by wutao@chromium.org, Jul 17 2017

#20,
For the blur, I get the wallpaper_layer and add a Blur filter by Layer::SetLayerFilters().

For the caching, I use our new SetCacheRenderSurface() code, which will cache and reuse the texture of the wallpaper_layer.

Do you mean if I also cache the parent layer, the blurred filter_image can be cached without further code change? Sounds cool. Will try it soon.
The wallpaper layer needs to have a parent that is not the root layer but I assume it will work if you make sure that's the case. Also, when using this approach there's no need to do SetCacheRenderSurface on the wallpaper_layer itself.

Comment 23 by wutao@chromium.org, Jul 17 2017

Good news, with parent layer cache, it can be ~53fps.

A problem is that the wallpaper layer has a damage_rect (-60, -60, 60, 60). This will make the parent layer cannot be cached (currently I just ignore this damage). I need to find out where this come from.

Also I will test if the partial_swap will generate artifacts boxes for us.

Comment 24 by r...@chromium.org, Jul 17 2017

Can we also see what FPS we get on much slower devices? We wouldn't want to add blur and have our lowest end devices become close to unusable.

Comment 25 by wutao@chromium.org, Jul 17 2017

What are those slow devices? I can borrow some to test.

Comment 26 by r...@chromium.org, Jul 17 2017

Cc: zalcorn@chromium.org
Zach, which would be a good super-slow device to test this (and other login features) on?

Comment 27 by wutao@chromium.org, Jul 17 2017

Cc: omrilio@chromium.org

Comment 28 by wzang@chromium.org, Jul 17 2017

I got a slow device from Zach and I'll give it to wutao@ for testing.

Comment 29 by wutao@chromium.org, Jul 18 2017

Found out where the damage_rect (-60, -60, 60, 60) comes from. It is calculated from layer's background filter. I open another issue to track this:  bug 745101 
What background filter? This approach is about not using a background filter but only a normal blur filter on the wallpaper layer so that's surprising.

Comment 32 by zalcorn@google.com, Jul 18 2017

For future reference, Daisy / Snow is a good reference slow device because it's one of the oldest and most popular devices we support.

Comment 33 Deleted

why is foreground blur + cache slower? I'd expect a cache hit to give us the same performance as no blur..

Comment 35 by wutao@chromium.org, Jul 18 2017

Delete #33 for wrong info. It was the background blur.

Comment 36 by wutao@chromium.org, Jul 18 2017

Tested on daisy/snow:

With foreground blur + cache: max 41fps.
Without blur: max 50fps.

Looking what will cause the difference.
Should I use Frame Rate counter for this fps, or using tracing?

Comment 37 by r...@chromium.org, Jul 18 2017

With foreground blur, Daisy is faster than Kevin?

Comment 38 by wutao@chromium.org, Jul 18 2017

#37, no when we compare foreground filter. The max fps with blur for kevin is 53fps, daisy is 41fps.

All the value mentioned above is using ash-debug-shortcuts Frame Rate Counter. The value is the max frame rate when manually changing contents (click the toggle pin button) in the show-login-dev-overlay lock screen.


However, I am wondering is this max value meaningful? It might be some peak value in the frame rate.

reveman@ could you suggest some stable measurement. We need:
1. constantly changed contents,
2. stable frame rate calculator.


Comment 39 by wutao@chromium.org, Jul 18 2017

reveman@ #35, one observation is that when I turn on cache render surface with/without blur, the fps is similar.
Does this can be explained that cache render surface has some performance cost for such as simple layer as wallpaper?
I suspect that drawing the wallpaper instead of a cached render surface is more efficient because the wallpaper is tiled. Would at least make sense on devices with partial update but Kevin is not such a device today so that's a bit surprising but it's possible that the tiled source is still more efficient for the GPU. If this is the case then above numbers lgtm.

Can you create an ash perf test similar to the one you created for background blur? I think that's the best way to measure the performance. The FPS counter is not ideal as that UI is causing some overhead that might affect the result in unexpected ways.
Project Member

Comment 41 by bugdroid1@chromium.org, Jul 21 2017

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

commit 11c9d1dcb9e338be177fd46e10b15ea958bf000a
Author: wutao <wutao@chromium.org>
Date: Fri Jul 21 08:12:45 2017

CrOS: Use foreground blur for lock screen.

Using foreground blur make it possible to cache the render surface and
provide good performance.

Bug:  738185 
Test: tested locally.
Change-Id: I838c072ecd4deeb9e181e08644d215877616f0ef
Reviewed-on: https://chromium-review.googlesource.com/578713
Commit-Queue: Tao Wu <wutao@chromium.org>
Reviewed-by: Jacob Dufault <jdufault@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488618}
[modify] https://crrev.com/11c9d1dcb9e338be177fd46e10b15ea958bf000a/ash/login/ui/lock_screen.cc
[modify] https://crrev.com/11c9d1dcb9e338be177fd46e10b15ea958bf000a/ash/login/ui/lock_screen.h
[modify] https://crrev.com/11c9d1dcb9e338be177fd46e10b15ea958bf000a/ui/compositor/layer.cc
[modify] https://crrev.com/11c9d1dcb9e338be177fd46e10b15ea958bf000a/ui/compositor/layer.h

Comment 42 by wutao@chromium.org, Jul 26 2017

First step in #15 (Land render surface (RS) caching and forced RS usage support for UI.) is under review.

It is tracked in this bug:  708513 

Close this one.

Comment 43 by wutao@chromium.org, Jul 26 2017

Status: Fixed (was: Assigned)

Comment 44 by wutao@chromium.org, Jul 26 2017

Labels: Merge-Request-61
Project Member

Comment 45 by sheriffbot@chromium.org, Jul 27 2017

Labels: -Merge-Request-61 Hotlist-Merge-Approved Merge-Approved-61
Your change meets the bar and is auto-approved for M61. Please go ahead and merge the CL to branch 3163 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid @(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 46 by bugdroid1@chromium.org, Jul 28 2017

Labels: -merge-approved-61 merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2b4404bba0ddeed39a249ac3e77d5df99ce52846

commit 2b4404bba0ddeed39a249ac3e77d5df99ce52846
Author: Qiang Xu <warx@chromium.org>
Date: Fri Jul 28 22:42:27 2017

[merge to m61] CrOS: Use foreground blur for lock screen.

merge to m61 on behalf of wutao@

Using foreground blur make it possible to cache the render surface and
provide good performance.

(cherry picked from commit 11c9d1dcb9e338be177fd46e10b15ea958bf000a)

TBR: jdufault@chromium.org, danakj@chromium.org
Bug:  738185 
Test: tested locally.
Change-Id: I838c072ecd4deeb9e181e08644d215877616f0ef
Reviewed-on: https://chromium-review.googlesource.com/578713
Commit-Queue: Tao Wu <wutao@chromium.org>
Reviewed-by: Jacob Dufault <jdufault@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#488618}
Reviewed-on: https://chromium-review.googlesource.com/592424
Reviewed-by: Qiang(Joe) Xu <warx@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#120}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/2b4404bba0ddeed39a249ac3e77d5df99ce52846/ash/login/ui/lock_screen.cc
[modify] https://crrev.com/2b4404bba0ddeed39a249ac3e77d5df99ce52846/ash/login/ui/lock_screen.h
[modify] https://crrev.com/2b4404bba0ddeed39a249ac3e77d5df99ce52846/ui/compositor/layer.cc
[modify] https://crrev.com/2b4404bba0ddeed39a249ac3e77d5df99ce52846/ui/compositor/layer.h

Comment 47 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment