1054904 - Do not wait for foreground app to paint before starting unlock

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System::Lockscreen, defect, P2)

Description

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Copied from bug 1038500 comment 34:

> Tested on a Flame running nightly build with build identifier 2140816160201
> I’ve tested slide unlock and pass code lock. For both I see a noticeable lag
> between the end of user operation and transition start. 
> 
> This is not the smoothness customers expect from a premium brand. 
> 
> Please improve

Comment 1

[Tim Guan-tin Chien [:timdream] (please needinfo)]

AFAIK we need to wait for the foreground app to paint before starting the transition, otherwise we will see a flash of white during transition. Can't remember the bug number on that.

Depend on the time frame of this bug we can decide what's the best approach. Can we always cover the foreground app with screenshot? Can we setVisible(true) before the unlock handle reaches the end?

If there isn't any priority, I will again wait for the proper platform fix first in bug 1034001. Kevin, what's the priority?

Greg also told me we have previously found there were many sync operation happen right before the unlock, as we dispatch willunlock/unlock event to other modules. That's something worthy to look at too.

Comment 2

[Kai-Chih Hu [:khu]]

Since Candice is handling all the engineering topics with this specific partner, it's worthy to double confirm with Candice here. Candice, what do you think? Thanks.

Comment 3

[Olof Wickström]

Someone in bug 1038500 commented that he had compared with an iPhone. The iPhone is generally a good reference when working with smoothness.

Comment 4

[Greg Weng [:snowmantw][:gweng][:λ]]

While I've studied what the unlocking does on iPhone, I've found it uses a UI trick to do that:

1. When you slide, the screen get darker and darker
2. When you slide to the end, the screen become fully dark and the animation starts to play

I think this eliminates the 'lag' time from sliding at end to the start of animation. Because user can feel the process is continuous from the UX side. So I do some experience to see what if we set opacity on our phone to get close to the similar effect:

https://www.youtube.com/watch?v=qDWHI5Kr7DE&feature=youtu.be

Please note I *did not* complete the same effect as iPhone's. What I want to do is to show if the  overlay become transparent before the animation start, we could possibly eliminate the lag from UI aspect, too. The video also show the waiting time of app-rendering is tiny: you can see the foreground app shows almost at the moment I release the finger, so according to it I don't think the waiting time is the root cause of unlocking lag.

This brings another problem is the 'lag' you and others feel, should be estimate only from the time the user's finger reached the end of slider. Because if you estimate from the beginning of sliding, you would naturally feel a 'lag' since the flow is 'sliding-stop-animation', while the interrupted 'stop' stage would bring a lag feeling to user, and this makes the result loses its accuracy. The correct way to estimate it, or to 'feel' it, is drag the finger to the end of the slider, and to estimate from the time you release the finger to the animation start to perform.

Comment 5

[Greg Weng [:snowmantw][:gweng][:λ]]

I suggest we may implement the unlocking opacity change to see if this could eliminate the issue, which may involve UX improvement as I mentioned, or at least narrow down the possible cause greatly (and to eliminate the difference between different device with different animation). So I would NI UX team to see if to get close with iPhone's trick is a do-able feature.

Comment 6

[Greg Weng [:snowmantw][:gweng][:λ]]

You can see the UI change (become darker) while iPhone is unlocking:

https://www.youtube.com/watch?v=gpQbLG1S_XI

And the I think the black, rather than immediately fade out is a trick, too. Because to perform a fade-to black before the background app shows you can say it's in fact a 'lag' inside it as well. The flows are also different:

iPhone: (sliding-darker) - (become complete black) - (from black to show the app)
Flame : (sliding, do no change) - (fade out to show the app)

Comment 7

[Stephany Wilkes]

Flagging Peter and Hung on this. 

Peter and Hung, please use caution here. Remember that we should not imitate the iPhone here due to the patents on lockscreen and unlock that we have been through many times. Whatever we change on unlock will need a check with the legal team.

Comment 10

[Peter La]

Hi Greg,

I’ve looked at your video but I find it difficult to tell if you’ve done anything different with the transition.  It looks more or less the same to me.  I would recommend against doing the fade to black for potential legal reasons as Stephany cited.  Right now the delay is pretty small between release and the beginning of the transition, but it could use a slight improvement if possible.  NI’ing Chris Lord/Kevin Grandon to see if they have any ideas that may help here.
 
I’m also concerned with the transition itself not looking smooth.  A few factors contribute to this feeling:

1) The zoom transition of the lockscreen (where it looks like a screenshot of the lockscreen is scaled up) has the following problems:
    a) It seems to end abruptly - it doesn’t fade smoothly to 0% opacity at the end, but appears to jump from something like 25% to 0% instantaneously.
    b) It includes the wallpaper, so you’ll see the wallpaper grow, and then disappear and reveal the normal sized wallpaper again on the homescreen.  I think it would look smoother if only the UI elements that reside on top of the wallpaper are captured in an image and scaled (not sure if this will be performant).

2) The status bar icons change between lock screen and homescreen (the time appears when on homescreen, pushing the indicators to the left).  This change occurs abruptly, with no fade transition.  You can check this yourself by covering up the status bar with one hand while unlocking - the transition feels smoother without this little hiccup.

3) The application icons on homescreen also appear instantaneously rather than fade + zoom in (scale up).

I will attach a couple of video samples showing something that looks better, but I am unsure how feasible it is.

Comment 11

[Peter La]

Attachment: Transition1.mov.zip

Here's a sample transition that illustrates:

1) scaling up only the foreground UI elements in the lockscreen (independant of the wallpaper)
2) having the homescreen icons/search bar/status bar scale up and fade in

Comment 12

[Peter La]

Attachment: Transition1-Slowmo.mov.zip

The same animation slowed down to 1/3 speed.

Comment 13

[Peter La]

Hi Chris/Kevin,

Please see comment 10.

Comment 14

[Kevin Grandon :kgrandon]

I'm a bit confused here. It seems like we should make what we have perform well first, before changing the implementation. There is probably a lot we can investigate here, I don't see a profile here for example, so we should take one and have some graphics experts dig into it. I'm not sure if this has been done in another bug though.

Comment 15

[Candice Serran (:cserran)]

[Blocking Requested - why for this release]: This is a Tako requirement. Thought it's a perf improvement, it needs to be done for 2.1 hence does not have feature flag: 2.1. Thanks!

Comment 16

[Tim Guan-tin Chien [:timdream] (please needinfo)]

:benWa has filed a bunch of performance bugs starting from bug 1063708. Should we dup the bug there if this bug is only about performance issue?

Comment 17

[Tim Guan-tin Chien [:timdream] (please needinfo)]

I am converting this bug to what's actionable -- update the visuals so that we don't have to wait for the foreground app to paint before unlock. I will put my proposal in the next comment.

Comment 18

[Tim Guan-tin Chien [:timdream] (please needinfo)]

So, please read bug 1063708 comment 12 (and other comments). We have identified the majority of the time spent from the user lift the finger to the transition start is on waiting for the foreground app to paint, rather than major performance issues on the system app itself. For complex apps the callback takes more than 1 sec to return, and we had to give up at 500ms (and start the transition).

The proper fix of this bug is again unfortunately bug 1034001. Since we are asked to find a solution for release branch, we can't depend on that. So again, just like what comment 5 proposed, we should find a visual comprise for this bug that is doable for v2.1.

There were a lot of brainstorming going on today try to leverage the pieces we have in the code. Let's beginning by taking about what will happen if we remove that async callback:

a1) If the foreground app is home screen, as soon as the user lift it's finger, in <100ms, the lock screen will begin to fade out, but the user will see blank background w/o icons. During the transition, text and icons will pop up one by one as they being painted by Gecko.
a2) If the foreground app is not home screen, the user will see a while, blank screen. The app will be painted all at once when Gecko finally done painting (maybe during the transition, maybe after.)

(a1) IMHO is not a bad experience -- if UX agree this is better than what we have now (a ~500ms wait on lock screen) we can do it w/o any non trivial code change, so I am asking Peter/Hung to consider (a1).

(a2) is bad. It was discussed that we can hide the while screen with screenshot (and maybe hide a1 too) but we realized screenshot will not respond to user action until the paint callback is returned -- so that's worse than what we have right now. Therefore, for this situation we propose having the user seeing the icon overlay -- something the user will see when she/he uses edge swipe -- and remove the semi-transparent icon overlay until the app is painted (b2).

So, is a1+b2 doable in the point of view of visual designers? Please respond on the feasibility.

Peter, I don't know why you needinfo'd clord and kgrandon but not people of my team, or me -- lock screen has always being done here -- we lost two weeks for this communicate gap.

Anyway, I looked at your comment 10, I don't think that can achieve what we want, since this bug is talking about the delay between the user lift the finger from unlock handle to the time we start transition (and start showing the foreground app). Your animation does not attempt to show anything else other than the foreground app in between.

Comment 19

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Alive and Greg, please add if I missed anything in the above comment.

Comment 20

[Peter La]

Hi Tim,

I'll need more detail to properly assess option a1 or b2.  I think a video that illustrates each in action would be ideal.

Things I need to know/see are:

a1) Do the icons _fade_ in or just appear instantaneously?  Do they really appear one at a time, one row at a time, or?  How long does it take for all of the icons to populate the screen?  What about the status bar/rocketbar - when do they appear?

b2) What icon would you show in the proposed semi-transparent overlay?  This solution doesn't seem like it would be a great experience when I try to picture it.  Seems like it would be jarring.

Overall, I can't really make a call until I see what these look like in action.  Unless they are a lot better than the minor lag that we experience currently, I'm not sure that they are actually a better experience.

In response to:

> Peter, I don't know why you needinfo'd clord and kgrandon but not people of my team, or me -- lock screen has always being done here -- we lost two weeks for this communicate gap.

I am simply asking them for ideas.  I know Chris has been looking at different transitions in the OS and I thought he might be able to provide some insights.  I'm not sure why this would cause you to have a 2 week delay?

Peter.

Comment 21

[Greg Weng [:snowmantw][:gweng][:λ]]

Things we discussed are listed. It looks now need more communication with UX, so clear the NI.

Comment 22

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Peter La from comment #20)
> Hi Tim,
> 
> I'll need more detail to properly assess option a1 or b2.  I think a video
> that illustrates each in action would be ideal.
> 
> Things I need to know/see are:
> 
> a1) Do the icons _fade_ in or just appear instantaneously?  Do they really
> appear one at a time, one row at a time, or?  How long does it take for all
> of the icons to populate the screen?  What about the status bar/rocketbar -
> when do they appear?

* We cannot control the ordering. We cannot know the timing. We cannot control gecko to draw A or B at first.
* statusbar is always there.
* rocketbar is now part of homescreen so it will be hidden at first too.

> 
> b2) What icon would you show in the proposed semi-transparent overlay?  This
> solution doesn't seem like it would be a great experience when I try to
> picture it.  Seems like it would be jarring.

The icon stuff is already used in swipe navigation - please try to swipe an app from left/right when you are already having an app opened and you will see it - we call it identification overlay.

> 
> Overall, I can't really make a call until I see what these look like in
> action.  Unless they are a lot better than the minor lag that we experience
> currently, I'm not sure that they are actually a better experience.
> 
> In response to:
> 
> > Peter, I don't know why you needinfo'd clord and kgrandon but not people of my team, or me -- lock screen has always being done here -- we lost two weeks for this communicate gap.
> 
> I am simply asking them for ideas.  I know Chris has been looking at
> different transitions in the OS and I thought he might be able to provide
> some insights.  I'm not sure why this would cause you to have a 2 week delay?
> 
> Peter.

Comment 23

[Greg Weng [:snowmantw][:gweng][:λ]]

Several videos to show different cases to help UX to know what we try to change:

1. LockScreen without wait Homescreen, or other complex app like marketplace to be fully repaint:

http://youtu.be/uggxgDCK--8

This is the video shows if LockScreen don't wait Homescreen, or other complex app to repaint itself, what would happen on Flame. As you can see, user would see some icons and components are empty, until the repainting done.

However, this definitely eliminate the 'lagging' problem. And, this is *expected* because:

2. Without LockScreen, to turn off the screen on current build, it would show the same issue:

http://youtu.be/9U6d0eLl8LU

As you can see, without LockScreen, user are expected to see the un-fully painting Homescreen on the current build. So, if we only hide this with LockScreen, there is an inconsistent spec.

And we found there is almost no different on low-memory device:

3. Without waiting, on 319MB Flame:

http://youtu.be/qpEUyey-tDk

Comment 24

[Milan Sreckovic [:milan] (needinfo for best results)]

+1 for not waiting.

Comment 25

[Peter La]

Alive, thanks for answering my questions.

Greg, thanks for the videos.  #1 (without waiting) looks like an improvement over what we have right now - it's noticeably more responsive without changing the overall feel too much.  Based on the description I was afraid that you would see the stages of drawing of the homescreen too clearly, but it happens really fast, so you don't notice it too much.

I'd like to take a closer look on my own device if you have a branch you can share with me, to make sure what I'm seeing isn't being smoothed over by video framerate/fuzziness.

Thanks,
Peter.

Comment 26

[Greg Weng [:snowmantw][:gweng][:λ]]

This is the branch remove the waiting time:

https://github.com/snowmantw/gaia/tree/waitnorepaint

Please let me know if you have any problem.

Comment 27

[Peter La]

Hi Greg,

Thanks for the branch.

I've loaded and reviewed the 'waitnorepaint' branch against the current behaviour on two Flame devices for side-by-side comparison.

Compared to watching the 'waitnorepaint' solution on video, viewing it in real life feels more jarring and less polished than the current behaviour, and I don't think that the gains in response time is significant enough to warrant the extra 'jank' introduced in the proposed solution.

So unless there is another solution that is better than the 'waitnorepaint' solution, I recommend sticking with the current behaviour.

Peter.

Comment 28

[Peter La]

See comment 27.

Comment 29

[Bruce Huang [:bhuang] (inactive)]

I think we should still remove the wait behavior since the lock screen's purpose is just to unlock.  We can explore in follow up bugs different solutions for how the system should repaint, either by introducing an overlay screen/animation or improving the performance of the repaint.

Comment 30

[Benoit Girard (:BenWa)]

(In reply to Peter La from comment #27)
> Compared to watching the 'waitnorepaint' solution on video, viewing it in
> real life feels more jarring and less polished than the current behaviour,
> and I don't think that the gains in response time is significant enough to
> warrant the extra 'jank' introduced in the proposed solution.

I think it's also worth considering a good model for the System vs. App responsiveness and the user perception.

The current behavior makes the System appear unresponsive if the app is resource hungry. To the user if the unlock is delayed then the System is unresponsive. This behavior also means that the System' unlock time is -unbounded- and get get arbitrary expensive depending on the app.

If we go with the behavior suggested in this bug we can aim to keep the System's unlock in -constant time-. This way we can signal to the user that the B2G System is always responsive and that it's clearly their app that's just slow.

IMO the System app should -never- block on the app. The System should be fast at unlock, switching apps, closing them, accessing the notification menu. If we do it right the user will notice that the System is snappy and will correctly blame delays on poorly designed app (which we can aim to avoid in the Gaia/certified app).

However I should note that I think it's acceptable to give a small timeout (like the ~300ms we're planning to do for e10s tab switch) to prevent a jarring switch for fast and snappy apps.

Comment 31

[Chris Lord [:cwiiis]]

I do wonder why we have to repaint app underneath the lockscreen at all - is this because it gets optimised out of the display list? Could we stop that from happening?

Comment 32

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Chris Lord [:cwiiis] from comment #31)
> I do wonder why we have to repaint app underneath the lockscreen at all - is
> this because it gets optimised out of the display list? Could we stop that
> from happening?

Yes, that's bug 1034001 -- first we need a way from Gaia System to ask Gecko to keep the rendering.

Comment 33

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Do you know what's the action item on this bug? We really can't start working on other bugs blocking bug 1063708 before this one.

Comment 34

[Bruce Huang [:bhuang] (inactive)]

The concern in comment 27 is valid, but it should not be solved by delaying the unlock.  Let's go ahead with the change here, there are already multiple follow up bugs addressing the render behavior that happens behind the lock screen, so we should tackle those to fix the overall experience.

Comment 35

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Greg Weng [:snowmantw][:gweng][:λ] from comment #26)
> This is the branch remove the waiting time:
> 
> https://github.com/snowmantw/gaia/tree/waitnorepaint
> 
> Please let me know if you have any problem.

Greg, would you mind submit a pull request from this branch? That should fix this bug.

Comment 36

[Greg Weng [:snowmantw][:gweng][:λ]]

Attachment: Patch

Waiting CI.

Comment 37

[Greg Weng [:snowmantw][:gweng][:λ]]

Attachment: Patch v2.1

Waiting CI.

Comment 38

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8499721 [details] [review]
Patch

What happen to the tests? We are not testing |responseUnlock()| anywhere?

Comment 39

[Greg Weng [:snowmantw][:gweng][:λ]]

We test the related event in 

https://github.com/snowmantw/gaia/blob/issue1054904/apps/system/test/unit/lockscreen_window_manager_test.js#L203

But the test is irrelevant to responseUnlock, since it test the end of the handling chain, that the resonseUnlock is just the one of middle steps.  And since we remove the waiting conditions in the responseUnlock, the method is exactly the same with closeApp, semantically (I would not suggest to remove it directly because we still need the name to keep our design make sense: request-response should be paired, and it's better to be cleared from naming).

Comment 40

[Greg Weng [:snowmantw][:gweng][:λ]]

Gaia-Try failed at a irrelevant and looks like a intermittent Settings Gij test. I now submit the approval.

Comment 41

[Greg Weng [:snowmantw][:gweng][:λ]]

Comment on attachment 8499723 [details] [review]
Patch v2.1

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Past design require us to wait the repainting and this cause some 'performance' issue, although this is what we expected. Details are described at the above comments.
[User impact] if declined: The issue continues.
[Testing completed]: Gaia-Try seems have lots of intermittent errors (some of them I've encountered on other totally irrelevant patches). But this change has been tested on local and manually on device. I can hold the patch until making Gaia-Try all green, but I want to submit the approval request first.
[Risk to taking this patch] (and alternatives if risky): We may need other solutions to solve the repainting delay for UX.
[String changes made]: No

Comment 42

[Greg Weng [:snowmantw][:gweng][:λ]]

Now Gaia-Try seems suffered from the same failures the master patch encountered and thus the failures are not caused by my patch, so I would land the master version first.

Comment 43

[Greg Weng [:snowmantw][:gweng][:λ]]

master: https://github.com/mozilla-b2g/gaia/commit/07ad200c49ff85ca73b3f35211b34df89da00753

Comment 44

[howie [:howie]]

Comment on attachment 8499723 [details] [review]
Patch v2.1

remove approval flag, keep on master for now