Closed
Bug 974381
Opened 11 years ago
Closed 10 years ago
[Dolphin][Homescreen][V1.4] Lag happened on homescreen under edit mode
Categories
(Firefox OS Graveyard :: Gaia::Homescreen, defect, P1)
Tracking
(blocking-b2g:1.4+, b2g-v1.4 fixed, b2g-v2.0 wontfix, b2g-v2.1 wontfix)
People
(Reporter: whsu, Assigned: Eli)
References
Details
(Keywords: perf, regression, Whiteboard: [caf priority: p2][CR 673039][c=effect p=3 s= u=1.4][sprd313225][partner-blocker])
Attachments
(9 files, 3 obsolete files)
(deleted),
video/mp4
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
text/plain
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
text/x-github-pull-request
|
vingtetun
:
review+
|
Details |
* Description:
Trying to move icon or switch page of homescreen under edit mode, you will see the serious lag
Attach the demo video. (WP_20140219_018.mp4)
* Reproduction steps:
1. Press the home button
2. Long press (> 3 sec) a icon of homescreen
3. Try to drag and drop a icon
4. Try to switch page
* Expected result:
- No lag happened and all icon can be dragged and dropped smoothly
* Actual result:
- Lag happened
* Reproduction build:( Mozilla Central - V1.4 )
- Gaia 258ae6472bd0e87ae074a6b9b464273dc9cfc8d6
- Gecko https://hg.mozilla.org/mozilla-central/rev/267e8340c47a
- BuildID 20140218160203
- Version 30.0a1
Reporter | ||
Updated•11 years ago
|
Keywords: regression,
smoketest
Comment 1•11 years ago
|
||
Bad perf regression, but I don't think it's a smoketest blocker.
blocking-b2g: --- → 1.4?
Updated•11 years ago
|
Whiteboard: [c=effect p= s= u=1.4]
Updated•11 years ago
|
Priority: -- → P1
Comment 2•11 years ago
|
||
Another tester and I were unable to reproduce this issue on the 02/19 1.3 or 1.4 builds. We also attempted to reproduce the issue using the build the reporter used. Unfortunately it looks like the reporter used an engineering nightly build, based off the attached video. It appears we do not have the ability to get what the reporter used, and instead used the non-eng nightly that had a matching BuildID.
Reporter | ||
Comment 3•11 years ago
|
||
Here are the build I used.
- /pvt/mozilla.org/b2gotoro/nightly/mozilla-central-hamachi-eng/2014/02/2014-02-18-16-02-03/
I used eng build that just because it had test tools on it and easily identify bugs. The commit number should be same as user build.
By the way, maybe you can install many apps, then try to reproduce it.
Thanks.
Comment 4•11 years ago
|
||
(In reply to William Hsu [:whsu] from comment #3)
> Here are the build I used.
> -
> /pvt/mozilla.org/b2gotoro/nightly/mozilla-central-hamachi-eng/2014/02/2014-
> 02-18-16-02-03/
>
> I used eng build that just because it had test tools on it and easily
> identify bugs. The commit number should be same as user build.
For any daily testing, we really need to use the prod builds by default. Not everything is the same with eng builds.
>
> By the way, maybe you can install many apps, then try to reproduce it.
> Thanks.
We need better STR here - this isn't actionable as it stands.
blocking-b2g: 1.4? → ---
Keywords: regressionwindow-wanted → steps-wanted
Reporter | ||
Comment 5•11 years ago
|
||
(In reply to Jason Smith [:jsmith] from comment #4)
> (In reply to William Hsu [:whsu] from comment #3)
> > Here are the build I used.
> > -
> > /pvt/mozilla.org/b2gotoro/nightly/mozilla-central-hamachi-eng/2014/02/2014-
> > 02-18-16-02-03/
> >
> > I used eng build that just because it had test tools on it and easily
> > identify bugs. The commit number should be same as user build.
>
> For any daily testing, we really need to use the prod builds by default. Not
> everything is the same with eng builds.
After I tested, it also happened on latest User Build.
>
> >
> > By the way, maybe you can install many apps, then try to reproduce it.
> > Thanks.
>
> We need better STR here - this isn't actionable as it stands.
I didn't have special steps here.
But, I forgot tell you. You need to install many app (I installed more than 16 apps)
After that, you will see the animation become unnormal and delay.
If you feel the lag is acceptable, please close this case.
Thanks! :)
* Reproduction steps:
1. Press the home button
2. Long press (> 3 sec) a icon of homescreen
3. Try to drag and drop a icon
4. Try to switch page
* Build Information:
- Gaia 6e71ab4da1b08586ea0c758edb7aa199ee34cd2f
- Gecko https://hg.mozilla.org/mozilla-central/rev/bb030d47c946
- BuildID 20140219160202
- Version 30.0a1
Comment 6•11 years ago
|
||
Okay - let's get comparison results then on 1.3 vs. 1.4 based on the above comment's STR.
QA Wanted - can we do a comparative analysis of 1.3 vs. 1.4 here?
Comment 7•11 years ago
|
||
Hi all, there is not any change on gaia, as far as I know, in this part of the code so I think that it depends on build. Should I move the component to another one?
https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/js/grid.js#L549
The duration of the panning animation is 300ms but the "transitionend" event is received after two seconds approx or more.
Thanks a lot
Updated•11 years ago
|
QA Contact: rpribble
Comment 8•11 years ago
|
||
Lag is present in comparable amount on latest versions of both 1.3 and 1.4. Test was run on both builds with exactly three pages full of icons. Occurs on both as shown in the original video attachment listed with bug.
Environmental Variables:
Device: buri v1.3 moz
BuildID: 20140225004004
Gaia: 6e883bde818d4d53aef7b5b075b4b34267918360
Gecko: e597280f9300
Version: 28.0
Firmware Version: v1.2-device.cfg
Environmental Variables:
Device: buri v1.4 moz
BuildID: 20140225040205
Gaia: e0f39c7179c8b297326c0e2313950610be1f5c52
Gecko: e3daaa4c73dd
Version: 30.0a1
Firmware Version: v1.2-device.cfg
Keywords: qawanted
Updated•11 years ago
|
Priority: P1 → P2
Whiteboard: [c=effect p= s= u=1.4] → [c=effect p= s= u=]
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → eperelman
Status: NEW → ASSIGNED
Assignee | ||
Updated•11 years ago
|
Whiteboard: [c=effect p= s= u=] → [c=effect p=3 s=2014.03.14 u=]
Assignee | ||
Updated•11 years ago
|
Whiteboard: [c=effect p=3 s=2014.03.14 u=] → [c=effect p=3 s=2014.03.28 u=]
Assignee | ||
Updated•11 years ago
|
Whiteboard: [c=effect p=3 s=2014.03.28 u=] → [c=effect p=3 s= u=]
Assignee | ||
Comment 9•11 years ago
|
||
Attached a profile containing 3 homescreen swipes while in edit mode. There is a longer-than-expected delay between when the swipe animation is complete and the |transitionend| event is fired.
Comment 10•11 years ago
|
||
Can you get a profile of both the homescreen app and b2g? Run ./profile.sh ps and it should bring up a list of running apps. Then:
./profile.sh start -p b2g -t Compositor && ./profile.sh start -p Homescreen (or homescreen, it's case sensitive).
Assignee | ||
Comment 11•11 years ago
|
||
Comment on attachment 8393241 [details]
Cleopatra Profile
http://people.mozilla.org/~bgirard/cleopatra/#report=b88506e17daf601d4f34c82c1cc9eb1ccd0b19a1
Assignee | ||
Comment 12•11 years ago
|
||
Mason, please see the new attachment. This one involves going into Homescreen edit mode, and 2 page swipes.
Attachment #8393241 -
Attachment is obsolete: true
Comment 13•11 years ago
|
||
Wow that's pretty bad, you have a 714 ms gralloc and a 876 ms rasterize time. Probably some ugly condition in Gecko causing the large gralloc.
Comment 14•11 years ago
|
||
Tiling disabled makes a difference?
Assignee | ||
Comment 15•11 years ago
|
||
The delay seems to behave the same with tiling disabled.
Assignee | ||
Updated•11 years ago
|
Assignee: eperelman → nobody
Status: ASSIGNED → NEW
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → eperelman
Assignee | ||
Comment 16•11 years ago
|
||
Attachment #8394849 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 17•11 years ago
|
||
Attachment #8404912 -
Flags: review?(21)
Comment 18•11 years ago
|
||
Comment on attachment 8404912 [details]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/18193
(In reply to Eli Perelman from comment #17)
> Created attachment 8404912 [details]
> Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/18193
Can you explain what you're trying to do with this patch and how it solve the issue ? (sorry if I missed that and it was into some comments, I just didn't see it).
Also Cristian review will be enough to me.
Attachment #8404912 -
Flags: review?(21) → review?(crdlc)
Comment 19•11 years ago
|
||
I gonna enjoy holidays til 04-21 :(
Assignee | ||
Comment 20•11 years ago
|
||
Sure Vivien. I have made a few comments on the PR, but I'll explain the intentions more here:
When transitioning between pages in edit mode, there is a severe lag in switching the icons from "non-shaking" mode to "shaking" mode, as well as getting the icon to snap back into its grid after being dropped. After profiling it, the root cause appears to be something within Gecko, see comment 13.
Although the problem may be within Gecko, that doesn't mean we can't try to work around the issue from the Gaia side, and that's what my PR addresses. By leaving the icons shaking, it has a perceived performance improvement over the previous interaction, even though the processing that takes place behind the scenes takes the same amount of time. We mask this from the user by making the UI responsive, decoupled from its processing if you will.
Unfortunately this didn't improve the interaction of the dropping icon, and invoking the `GridManager.onDragStop` method before handling the other logic serially has a great improvement on the responsiveness of the interaction.
Please let me know if you have any other questions or need me to clarify.
Assignee | ||
Updated•11 years ago
|
Whiteboard: [c=effect p=3 s= u=] → [c=effect p=3 s=2014.04.11 u=]
Assignee | ||
Updated•11 years ago
|
Whiteboard: [c=effect p=3 s=2014.04.11 u=] → [c=effect p=3 s= u=]
Assignee | ||
Updated•11 years ago
|
Attachment #8404912 -
Flags: review?(crdlc) → review?(21)
Comment 21•11 years ago
|
||
(In reply to Eli Perelman from comment #20)
> Sure Vivien. I have made a few comments on the PR, but I'll explain the
> intentions more here:
>
> When transitioning between pages in edit mode, there is a severe lag in
> switching the icons from "non-shaking" mode to "shaking" mode, as well as
> getting the icon to snap back into its grid after being dropped. After
> profiling it, the root cause appears to be something within Gecko, see
> comment 13.
I'm not against a workaround but I would like to fix the underlying Gecko issue here. Once the issue is identified, depending on how long it will took to fix it that's would be ok or not to land a workaround.
Hope it makes sense ?
Milan, do you have any free resources to investigate the underlying bug here ? Since there is a lot of animations and gralloc I feel like this belongs to your team :)
Flags: needinfo?(milan)
Comment 22•11 years ago
|
||
Comment on attachment 8404912 [details]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/18193
Let's see if the platfomr folks can do anything before landing that.
Attachment #8404912 -
Flags: review?(21)
Comment 23•11 years ago
|
||
The timing is against us here. BenWa is the person that could look at this, but he's sick and not back until 4/21, which is rather late for 1.4. If you have a workaround, I would land that at least on 1.4.
Flags: needinfo?(milan) → needinfo?(21)
Comment 24•11 years ago
|
||
(In reply to Milan Sreckovic [:milan] from comment #23)
> The timing is against us here. BenWa is the person that could look at this,
> but he's sick and not back until 4/21, which is rather late for 1.4. If you
> have a workaround, I would land that at least on 1.4.
Ok a quick look at the what's going on on the homescreen with the layers borders turned on explains the gralloc/rasterize expensiveness AFAICT.
First screenshot is with the layer borders effect turned on and before long pressing to enter edit mode.
Second screenshot is with the layer borders effect turned on after a long press to enter edit mode.
It seems like we are creating one dedicated layer for each icon on the homescreen, and animate all of those independently. It seems more a UX issue than a platform issue here, as I doubt this effect can be made fast on low end devices.
I think we should just remove this effect as the little arrow icon and opacity are already indicators that the user is in edit mode.
Eli, what do you think of you removing this effect and opening a followup for a better UX proposal ?
Flags: needinfo?(21)
Comment 25•11 years ago
|
||
Screenshot before entering edit mode
Comment 26•11 years ago
|
||
Screenshot after entering edit mode
Comment 27•11 years ago
|
||
just something dirty like that makes the edit mode so much better for me.
Assignee | ||
Comment 28•11 years ago
|
||
IMHO, I do believe that removing the effect makes the UI simpler and takes away some of the gimmicky-ness of the edit mode. It does make things faster from the Gaia side especially, so I support it. Again though, this is just opinion, UX should probably weigh in. Thoughts?
Assignee | ||
Comment 30•11 years ago
|
||
Vivien, how would you feel about taking your CSS changes and my small JS change and combining them for this bug?
Comment 31•11 years ago
|
||
(In reply to Eli Perelman from comment #30)
> Vivien, how would you feel about taking your CSS changes and my small JS
> change and combining them for this bug?
Sounds good to me. Please open a followup asking UX for a better approach though.
Flags: needinfo?(21)
Assignee | ||
Comment 32•11 years ago
|
||
UX: Could you please test out our patches and let us know how you would like to proceed with this issue?
Flags: needinfo?(gbrander)
Assignee | ||
Updated•11 years ago
|
Comment 33•11 years ago
|
||
If we remove the pulse, we have to cue the user in that this is "edit mode" in some other way.
The 2.0 homescreen features a re-think of edit mode, partially because of these issues. Given that's in the pipeline, I'm wondering if there is a way we can tune the performance of the mode in the current homescreen without drastically changing the UX.
I will try to test these patches asap (wish I could have gotten to it earlier, but crazy week).
Flags: needinfo?(gbrander)
Assignee | ||
Comment 34•11 years ago
|
||
This video demonstrates the changes from Eli Perelman's patch.
Attachment #8417641 -
Flags: ui-review?(gbrander)
Assignee | ||
Comment 35•11 years ago
|
||
This video demonstrates the changes from Vivien's patch.
Attachment #8417642 -
Flags: ui-review?(gbrander)
Comment 36•11 years ago
|
||
Jacqueline, since you're owning this interaction for Home 2.0, could you advise on a good interim approach for current home screen? Thanks!
Flags: needinfo?(jsavory)
Comment 37•11 years ago
|
||
Peter is currently working on this issue for the 2.0 homescreen and the solution should be able to work on the current homescreen as well. Ni? on Peter to share solution when available.
Flags: needinfo?(jsavory) → needinfo?(pla)
Assignee | ||
Updated•11 years ago
|
Assignee | ||
Updated•11 years ago
|
Comment 38•11 years ago
|
||
UX, can you please choose one of the two proposed solutions since Homescreen 2.0 will supersede either. Videos have been attached to make it easier for you to see the end-user changes without having to apply patches. This issue has been sitting in our queue for a month and we'd really like to resolve this as soon as possible so we can move on to other pressing issues.
Flags: needinfo?(firefoxos-ux-bugzilla)
Comment 39•11 years ago
|
||
Hi,
Sorry for the delay here. I'm working on a few proposals that I'll review with UX, product, and Kevin Grandon tomorrow, after which I will get back to you on a decision. I believe either path is viable, but Vivien's option would need to be tweaked visually to make it communicate 'edit mode' to the user.
Because Monday is a holiday in Canada, most likely a final solution will be presented back here on Tuesday.
Kevin, will you be implementing the final solution?
Flags: needinfo?(pla) → needinfo?(kgrandon)
Comment 40•11 years ago
|
||
The systemsFE team will likely be doing the implementation for the vertical homescreen slated to replace the old homescreen in 2.0. The same performance issues/concerns are there. Our current solution is to not have an animation, but if UX wants one - we will need to investigate a solution. At this point I might suggest some sort of slower/random wobble between all of the individual icons. We would probably only need a few active layers at a time for this.
I've also filed bug 1010584 to try to track down why having so many layers is slow. A fix there could possibly help both homescreens.
Flags: needinfo?(kgrandon)
Comment 41•11 years ago
|
||
Thanks Kevin.
How would performance be if we animated opacity on the entire icon set? Something like a sine function that cycled between 50% and 80%.
Comment 43•11 years ago
|
||
(In reply to Peter La from comment #41)
> Thanks Kevin.
>
> How would performance be if we animated opacity on the entire icon set?
> Something like a sine function that cycled between 50% and 80%.
Hard to say without protoyping this - I think we will have someone look at this in the next 1-2 weeks. I don't really see this performing much better than the current solution though. We're going to have a hard time with anything that impacts every icon at the same time I think.
Flags: needinfo?(kgrandon)
Comment 45•11 years ago
|
||
Clearing the UX general flag and nudged Peter and Patryk to respond here. Sorry for blocking!
Flags: needinfo?(firefoxos-ux-bugzilla)
Comment 46•11 years ago
|
||
Hi,
So here is a summary of the current state of things:
1) You guys have proposed 2 solutions.
2) Neither of these solutions is my ideal solution for 2.0.
3) My ideal solution for 2.0 is to take Eli's solution (https://bug974381.bugzilla.mozilla.org/attachment.cgi?id=8417641) and substitute the size 'pulsing' animation (animating scale), replacing it with an opacity pulse animation (https://www.dropbox.com/s/ju8j0flrp6ywd80/EditModeOpacityAnimation_v1.mov). Please download and play my video with 'Loop' on.
4) We don't know if this opacity pulse will be performant enough > need volunteers to figure that out.
5) If an interim solution is needed before trying the ideal solution (opacity pulse), then go with Eli's size pulse.
6) Both opacity pulse and size pulse would not animate during icon drag.
To recap: ideally we want opacity pulse with animation paused during icon drag, but if we can't get that, then the fallback is Eli's solution (size pulse with animation pause during icon drag).
Does that make sense?
Flags: needinfo?(pla)
Comment 47•11 years ago
|
||
Oh - I forgot to mention that Kevin has suggested we try an alternate to pulsing ALL icons' opacity at the same time, and instead alternate. This solution is also a possibility, but I haven't had time to mock it up yet.
Assignee | ||
Comment 48•11 years ago
|
||
I was under the presumption that this patch is intended for only the current homescreen design, to pre-2.0. Since homescreen will have a new design and new optimizations, this patch would probably be changed along with the 2.0 homescreen.
I would like to see this make it into v1.3, v1.3t, and v1.4 if possible.
In light of that, I believe what you are saying is that you approve of my patch and would like to see that merged into master with the intent that its relevance will be diminished with the upcoming homescreen redesign. Can you confirm?
Flags: needinfo?(pla)
Comment 49•11 years ago
|
||
Yes, that makes total sense. Please go ahead with it, thanks!
Kevin, should I go ahead and file a bug for the opacity animation version for 2.0 or is there already a bug for edit mode?
Flags: needinfo?(pla) → needinfo?(kgrandon)
Comment 50•11 years ago
|
||
Let's track any icon editing work for vertical homescreen in bug 1009530. This isn't 1.4+ so I feel like we should either nom it or close it as wontfix as we are moving to a new homescreen in 2.0.
Flags: needinfo?(kgrandon)
Assignee | ||
Updated•11 years ago
|
Attachment #8417641 -
Flags: ui-review?(gbrander)
Assignee | ||
Updated•11 years ago
|
Attachment #8417642 -
Flags: ui-review?(gbrander)
Assignee | ||
Comment 51•10 years ago
|
||
Status update: I have a patch ready for this, but it's failing a couple of unit tests from bitrot and the removal of the data attribute. Still working to get this resolved.
Comment 53•10 years ago
|
||
Carrying over blocking flag & regression keyword from dupe.
blocking-b2g: --- → 1.4+
Keywords: regression
Updated•10 years ago
|
Whiteboard: [c=effect p=3 s= u=] → [c=effect p=3 s= u=][sprd313225][partner-blocker]
Assignee | ||
Updated•10 years ago
|
Priority: P2 → P1
Whiteboard: [c=effect p=3 s= u=][sprd313225][partner-blocker] → [c=effect p=3 s= u=1.4][sprd313225][partner-blocker]
Updated•10 years ago
|
Whiteboard: [c=effect p=3 s= u=1.4][sprd313225][partner-blocker] → [CR 673039][c=effect p=3 s= u=1.4][sprd313225][partner-blocker]
Updated•10 years ago
|
Whiteboard: [CR 673039][c=effect p=3 s= u=1.4][sprd313225][partner-blocker] → [caf priority: p2][CR 673039][c=effect p=3 s= u=1.4][sprd313225][partner-blocker]
Updated•10 years ago
|
Summary: [Homescreen][V1.4] Lag happened on homescreen under edit mode → [Dolphin][Homescreen][V1.4] Lag happened on homescreen under edit mode
Assignee | ||
Updated•10 years ago
|
Attachment #8404912 -
Attachment is obsolete: true
Assignee | ||
Comment 54•10 years ago
|
||
New patch for committing against v1.4.
Attachment #8437695 -
Flags: review?(21)
Attachment #8437695 -
Flags: review?(21) → review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 55•10 years ago
|
||
v1.4: https://github.com/mozilla-b2g/gaia/commit/80bf1039c6ce8bcde57ce06ecb09e40c18c538c6
Setting 2.0+ to wontfix based on the comments here that the vertical homescreen work makes this obsolete. Please reopen and post patches if I've misunderstood.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-b2g-v1.4:
--- → fixed
status-b2g-v2.0:
--- → wontfix
status-b2g-v2.1:
--- → wontfix
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.0 S4 (20june)
You need to log in
before you can comment on or make changes to this bug.
Description
•