Closed
Bug 1006274
Opened 8 years ago
Closed 8 years ago
[tarako][perf] When hang up the phone, it takes time 3.5s, which costs 2s more than android
Categories
(Firefox OS Graveyard :: Gaia::Dialer, defect, P1)
Tracking
(blocking-b2g:1.3T+, b2g-v1.3T verified, b2g-v1.4 affected)
People
(Reporter: wei.gao, Assigned: etienne)
Details
(Keywords: perf, Whiteboard: [c=progress p= s= u=tarako] [sprd307322][partner-blocker][tarako_only][dolphin land])
Attachments
(3 files, 2 obsolete files)
【Repro Steps*】 1. Dial a phone number, such as 10086 2. Click to hang up the call 3. After that, it takes time 3.5s to show dialpad, which costs 2s more than android Spreadtrum bug 307322 http://bugzilla.spreadtrum.com/bugzilla/show_bug.cgi?id=307322
Reporter | ||
Comment 1•8 years ago
|
||
When hang up the phone, the interface transform effects takes 2s which is too long for hand up. After removing this effects, the time to hang up the phone is similar to android. I propose to remove the effects of hang up, taking into account the performance of 6821. The patch file Remove_hangup_effects.patch is to remove the effects of hang up. Please help to review, thanks.
Reporter | ||
Comment 2•8 years ago
|
||
Comment on attachment 8417793 [details] [diff] [review] Remove hangup effects NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] [Bug caused by] (feature/regressing bug #): [User impact] if declined: [Testing completed]: [Risk to taking this patch] (and alternatives if risky): [String changes made]:
Attachment #8417793 -
Flags: review?(tzhuang)
Attachment #8417793 -
Flags: review?(ehung)
Updated•8 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: [tarako]When hang up the phone, it takes time 3.5s, which costs 2s more than android → [tarako][perf] When hang up the phone, it takes time 3.5s, which costs 2s more than android
Comment 3•8 years ago
|
||
1.3t+ please.
Flags: needinfo?(ttsai)
Flags: needinfo?(styang)
Flags: needinfo?(kkuo)
Whiteboard: [sprd307322][partner-blocker][tarako_only]
Updated•8 years ago
|
Flags: needinfo?(fabrice)
Updated•8 years ago
|
Attachment #8417793 -
Flags: review?(etienne)
Comment 4•8 years ago
|
||
Hi James, you can pick the reviewer from the suggested list when you're finding the proper owner to review it. thanks.
Flags: needinfo?(ttsai)
Flags: needinfo?(styang)
Flags: needinfo?(kkuo)
Flags: needinfo?(fabrice)
Comment 5•8 years ago
|
||
Comment on attachment 8417793 [details] [diff] [review] Remove hangup effects Review of attachment 8417793 [details] [diff] [review]: ----------------------------------------------------------------- I can't review it but only give comments since Etienne is the module owner. I'll suggest you directly remove the two transition lines instead of mark them off. Also, this changes UX/UI behavior. We should ask for UX input.
Attachment #8417793 -
Flags: review?(tzhuang)
Comment 6•8 years ago
|
||
Comment on attachment 8417793 [details] [diff] [review] Remove hangup effects Sorry, I can't review dialer patch.
Attachment #8417793 -
Flags: review?(ehung)
Comment 7•8 years ago
|
||
Hi Mike, We need your opinion. (Or reassign to others to have a look at it) I record a video here with transition effect removed in case you'd like to see it right away. Thanks
Flags: needinfo?(mtsai)
Comment 8•8 years ago
|
||
triage: i don't think this is acceptable to remove the "call ended" screen the "call ended" screen was requested to be added as a feature by one of our carrier partner for their IOT and it is a bit confusing that the call ends immediately like the video i think the problem is the black screen after ending the call, not the "call ended" indication screen ni? Wei.gao, is it really your intention to remove the "call ended" indication screen?
Flags: needinfo?(wei.gao)
Comment 9•8 years ago
|
||
We'll remove all gaia animation when enter and exit apps for tarako only.
Reporter | ||
Comment 10•8 years ago
|
||
(In reply to Joe Cheng [:jcheng] from comment #8) > triage: i don't think this is acceptable to remove the "call ended" screen > the "call ended" screen was requested to be added as a feature by one of our > carrier partner for their IOT and it is a bit confusing that the call ends > immediately like the video > > i think the problem is the black screen after ending the call, not the "call > ended" indication screen > > ni? Wei.gao, is it really your intention to remove the "call ended" > indication screen? hi Joe The black screen is always exist no matter remove the transition effect or not and it is not the main reason that takes so much time when hang up. We hope the transition effect can bring better user experience, but if the performance can't display smoothly as we hope, I think removing the transition effect on 6821 is more sensible. Thank you.
Flags: needinfo?(wei.gao)
Assignee | ||
Comment 11•8 years ago
|
||
Comment on attachment 8417793 [details] [diff] [review] Remove hangup effects Review of attachment 8417793 [details] [diff] [review]: ----------------------------------------------------------------- Like the comment just above the line modified mentions |This transition is needed|. On the JS side we're waiting for the transitionend event. Also like Joe said this is preventing us from displaying the the "Call ended" message which was a mandatory partner request. I think we need a stronger rationale for this change, and if we move forward we need to make this a clean option with settings like for the app opening transitions.
Attachment #8417793 -
Flags: review?(etienne) → review-
Reporter | ||
Comment 12•8 years ago
|
||
(In reply to Etienne Segonzac (:etienne) from comment #11) > Comment on attachment 8417793 [details] [diff] [review] > Remove hangup effects > > Review of attachment 8417793 [details] [diff] [review]: > ----------------------------------------------------------------- After discussion within our group, our strategy is to remove those effects that are independent from logic functions, and reduce time of those effects which are related to internal logic. Such as changing 2s to 500ms, so as to make the performance better.
Comment 13•8 years ago
|
||
Thanks for the video, I counted it, I think there is a 2 seconds delay. If you want UX opinion, we would like it to hangout as soon as possible. But if the performance is not good, I think with that price band device, users may have the tolerance and patience to wait for 2 seconds.
Flags: needinfo?(mtsai)
Assignee | ||
Comment 14•8 years ago
|
||
(In reply to GoodMike from comment #13) > Thanks for the video, I counted it, I think there is a 2 seconds delay. If > you want UX opinion, we would like it to hangout as soon as possible. But if > the performance is not good, I think with that price band device, users may > have the tolerance and patience to wait for 2 seconds. Just to clarify. The 2 second delay is a *feature*. The phone isn't too busy to do it quicker. It was added as part of a partner requested feature to display a "Call ended" message for 2 seconds before closing the callscreen.
Comment 15•8 years ago
|
||
Hi Etienne : ) Thanks for the comment. So the "CAll ended" message is 2 seconds black screen ?
Comment 16•8 years ago
|
||
If the black screen is the "feature" o inform user that the phone is hanged up. I think it doesn't make sense. Once user hangs up a call, it makes sense that it just go back to the dialer screen as the last UI in the video. Because users know it's hanged up when seeing that page.
Comment 17•8 years ago
|
||
(In reply to wei.gao from comment #12) > (In reply to Etienne Segonzac (:etienne) from comment #11) > > Comment on attachment 8417793 [details] [diff] [review] > > Remove hangup effects > > > > Review of attachment 8417793 [details] [diff] [review]: > > ----------------------------------------------------------------- > > After discussion within our group, our strategy is to remove those effects > that are independent from logic functions, and reduce time of those effects > which are related to internal logic. Such as changing 2s to 500ms, so as to > make the performance better. Fabrice, I need this patch for tarako only workaround.
Flags: needinfo?(fabrice)
Updated•8 years ago
|
Flags: needinfo?(styang)
Flags: needinfo?(kkuo)
Reporter | ||
Comment 18•8 years ago
|
||
I uploaded a video with two different effects in it. In this video, the left mobilephone have removed hang up effect and the right one continue to keep it. We can see the effect that after hang up, the right phone continue too much time and when the transition effect is transforming, the bottom of screen is entire black. We know the hang up effect is a "feature", and we are willing to keep the feather, but out purpose is to reduce the time of common use operation and improve the performance of the phone. I am sure our customer also would like to see that. Do you think it right ? Thanks.
Comment 19•8 years ago
|
||
can you just shortern the animation for "call ended" screen? instead of taking the "call ended" screen out completely?
Flags: needinfo?(wei.gao)
Reporter | ||
Comment 20•8 years ago
|
||
(In reply to Joe Cheng [:jcheng] from comment #19) > can you just shortern the animation for "call ended" screen? instead of > taking the "call ended" screen out completely? hi Joe After we tried to reduce animation duration of the call ended effect and to verified it, we found that even though we decreased the transition time to 100ms, the performance of the hang up effect was not smoothly as we hope. I'm afraid customer can't accept this result either. Judging from our test data, only by reducing the transition duration to the less than 10ms, the effect of the hang up operation can be smoothly and perform well. What do you think about it ? Thanks.
Flags: needinfo?(wei.gao)
Reporter | ||
Comment 21•8 years ago
|
||
(In reply to Joe Cheng [:jcheng] from comment #19) > can you just shortern the animation for "call ended" screen? instead of > taking the "call ended" screen out completely? But the question is if we reduce the transition duration to less than 10ms, I think there is no difference between it and get rid of the effect directly. In my own opinion, if this is the case, I think the transition effect is more to be removed than to be reduced its duration to less than 10ms. How do you think about it ? Thanks.
Comment 22•8 years ago
|
||
I'm fine with removing the transition totally. Please provide at updated patch that addresses Etienne's comment about being notified instead of relying on the transitionend event.
Flags: needinfo?(fabrice)
Comment 23•8 years ago
|
||
Since both with transition and without transition lead to a blank screen but without is faster we (FxOS Perf Team) agree with removing the transition as it adds no value to the user experience and actually degrades it by making it slower. (In reply to Etienne Segonzac (:etienne) from comment #11) > > Like the comment just above the line modified mentions |This transition is > needed|. > On the JS side we're waiting for the transitionend event. > > Also like Joe said this is preventing us from displaying the the "Call > ended" message which was a mandatory partner request. Can we use a different (custom) event to indicate being ready to show the "Call ended" message?
Assignee: nobody → wei.gao
Status: NEW → ASSIGNED
blocking-b2g: 1.3T? → 1.3T+
Keywords: perf
Priority: -- → P1
Whiteboard: [sprd307322][partner-blocker][tarako_only] → [c=progress p= s= u=tarako] [sprd307322][partner-blocker][tarako_only]
Updated•8 years ago
|
Flags: needinfo?(kkuo)
Updated•8 years ago
|
Flags: needinfo?(styang)
Reporter | ||
Comment 24•8 years ago
|
||
hi Fabrice I upload a file Remove_hangup_effects_2.patch that the code is changed. The original code's strategy is that there is a judgement to ensure what to execute during the transition. If the effect is not transitioning, it will add a eventlistener to listen the 'transitionend' event, and some code will be executed until the transition is over. But if the effect is transitioning, or a new call is incoming, those code will be executed directly, because it will loose the 'transitionend' event during the transition. After the transition effect is removed, i modify the code to make sure those following code will be executed whatever, so as to ensure the current logic the same as before. What's your opinion on this ? Thank you.
Flags: needinfo?(fabrice)
Comment 25•8 years ago
|
||
Comment on attachment 8419254 [details] [diff] [review] Remove_hangup_effects_2.patch I think we should ask Etienne if this patch is good or not.
Attachment #8419254 -
Flags: review?(etienne)
Assignee | ||
Comment 26•8 years ago
|
||
Please take a look at this patch removing the 1.5sec transition delay. It's much simpler and has the advantage of not breaking the test suite which will make landing easier.
Assignee: wei.gao → etienne
Attachment #8417793 -
Attachment is obsolete: true
Attachment #8419254 -
Attachment is obsolete: true
Attachment #8419254 -
Flags: review?(etienne)
Attachment #8419508 -
Flags: feedback?(wei.gao)
Comment 27•8 years ago
|
||
Comment on attachment 8419508 [details] [diff] [review] Patch proposal - remove the transition delay Review of attachment 8419508 [details] [diff] [review]: ----------------------------------------------------------------- works fine here. ship it!
Attachment #8419508 -
Flags: feedback?(wei.gao) → review+
Updated•8 years ago
|
Flags: needinfo?(fabrice)
Reporter | ||
Comment 28•8 years ago
|
||
(In reply to Etienne Segonzac (:etienne) from comment #26) > Created attachment 8419508 [details] [diff] [review] > Patch proposal - remove the transition delay > > Please take a look at this patch removing the 1.5sec transition delay. > > It's much simpler and has the advantage of not breaking the test suite which > will make landing easier. hi It is really more reliable to removing the 1.5sec transition delay only, thanks. I want to know can it be meraged into 1.3t now ? Thank you.
Assignee | ||
Comment 29•8 years ago
|
||
Landed on 1.3t https://github.com/mozilla-b2g/gaia/commit/d07f2dff31e71aed9711d988677fbc5b2c6e83e4
Reporter | ||
Comment 30•8 years ago
|
||
(In reply to Etienne Segonzac (:etienne) from comment #29) > Landed on 1.3t > https://github.com/mozilla-b2g/gaia/commit/ > d07f2dff31e71aed9711d988677fbc5b2c6e83e4 OK. Thanks for your kindly help.
Updated•8 years ago
|
status-b2g-v1.4:
--- → affected
Updated•8 years ago
|
Target Milestone: --- → 2.0 S1 (9may)
Comment 31•8 years ago
|
||
Verified fixed on the Tarako v1.3T MOZ ril. Environmental Variables: Device: Tarako 1.3T BuildID: 20140602014001 Gaia: 335486c42498fa7a93c21e4d6121199728602ab8 Gecko: 55e4d83019e5 Version: 28.1 Firmware Version: SP6821a-Gonk-4.0-4-29 Keypad displays at 1.74s.
Comment 32•8 years ago
|
||
This has 1.4 affected status, do we want this for 1.4 spreadtrum devices as well?
Flags: needinfo?(mlee)
Comment 33•8 years ago
|
||
(In reply to Jan Jongboom [:janjongboom] (Telenor) from comment #32) > This has 1.4 affected status, do we want this for 1.4 spreadtrum devices as > well? I'm assuming you're asking about Dolphin. If so I believe that's Spreadtrum's decision to make. Preeti please confirm.
Flags: needinfo?(mlee) → needinfo?(praghunath)
Comment 34•8 years ago
|
||
Lets add this on 1.4 dolphin branch
Flags: needinfo?(praghunath)
Whiteboard: [c=progress p= s= u=tarako] [sprd307322][partner-blocker][tarako_only] → [c=progress p= s= u=tarako] [sprd307322][partner-blocker][tarako_only][dolphin land]
Comment 35•8 years ago
|
||
Hi Pei-pei, Please help to check if it performs the same on Dolphin and Buri before we can make better decision on the 1.4 uplift of this bug. Thanks!
Flags: needinfo?(pcheng)
Comment 36•8 years ago
|
||
Dolphin perform a little better than Tarako. The average time is about 2.5s. Buri has same performance as dolphin. I tried 5 times, avg time is 2.5s.
Flags: needinfo?(pcheng)
Comment 37•8 years ago
|
||
Let's not uplift this patch for now since it performs the same as Buri.
Flags: needinfo?(etienne)
Reporter | ||
Comment 39•8 years ago
|
||
(In reply to James Zhang (Spreadtrum) from comment #38) > Wei, land this WIP patch in sprd_patch. OK, I will do that right now.
Flags: needinfo?(wei.gao)
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(etienne)
You need to log in
before you can comment on or make changes to this bug.
Description
•