974381 - [Dolphin][Homescreen][V1.4] Lag happened on homescreen under edit mode

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Homescreen, defect, P1)

Description

[William Hsu [:whsu]]

Attachment: WP_20140219_018.mp4

* 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

Comment 1

[Jason Smith [:jsmith]]

Bad perf regression, but I don't think it's a smoketest blocker.

Comment 2

[Matthew Vaughan]

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.

Comment 3

[William Hsu [:whsu]]

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

[Jason Smith [:jsmith]]

(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.

Comment 5

[William Hsu [:whsu]]

(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

[Jason Smith [:jsmith]]

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

[Cristian Rodriguez (:crdlc)]

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

Comment 8

[Rachel Pribble [:rpribble]]

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

Comment 9

[:Eli Perelman]

Attachment: Cleopatra Profile

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

[Mason Chang [Inactive] [:mchang]]

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).

Comment 11

[:Eli Perelman]

Comment on attachment 8393241 [details]
Cleopatra Profile

http://people.mozilla.org/~bgirard/cleopatra/#report=b88506e17daf601d4f34c82c1cc9eb1ccd0b19a1

Comment 12

[:Eli Perelman]

Attachment: Fuller Cleopatra Profile

Mason, please see the new attachment. This one involves going into Homescreen edit mode, and 2 page swipes.

Comment 13

[Mason Chang [Inactive] [:mchang]]

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

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

Tiling disabled makes a difference?

Comment 15

[:Eli Perelman]

Attachment: Cleopatra Profile, Tiling Disabled

The delay seems to behave the same with tiling disabled.

Comment 16

[:Eli Perelman]

Attachment: Cleopatra Profile, Tiling Disabled

Comment 17

[:Eli Perelman]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/18193

Comment 18

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 8404912 [details] [review]
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] [review]
> 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.

Comment 19

[Cristian Rodriguez (:crdlc)]

I gonna enjoy holidays til 04-21 :(

Comment 20

[:Eli Perelman]

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.

Comment 21

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(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 :)

Comment 22

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 8404912 [details] [review]
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.

Comment 23

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

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.

Comment 24

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(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 ?

Comment 25

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: 2014-04-16-16-56-22.png

Screenshot before entering edit mode

Comment 26

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: 2014-04-16-16-55-44.png

Screenshot after entering edit mode

Comment 27

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: remove.animation.and.move.opacity.patch

just something dirty like that makes the edit mode so much better for me.

Comment 28

[:Eli Perelman]

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?

Comment 29

[:Eli Perelman]

Vivien, please see comment 28.

Comment 30

[:Eli Perelman]

Vivien, how would you feel about taking your CSS changes and my small JS change and combining them for this bug?

Comment 31

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(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.

Comment 32

[:Eli Perelman]

UX: Could you please test out our patches and let us know how you would like to proceed with this issue?

Comment 33

[Gordon Brander :gordonb]

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).

Comment 34

[:Eli Perelman]

Attachment: Eli Perelman's Patch Video

This video demonstrates the changes from Eli Perelman's patch.

Comment 35

[:Eli Perelman]

Attachment: Vivien's Patch Video

This video demonstrates the changes from Vivien's patch.

Comment 36

[Gordon Brander :gordonb]

Jacqueline, since you're owning this interaction for Home 2.0, could you advise on a good interim approach for current home screen? Thanks!

Comment 37

[Jacqueline Savory [:jsavory] UX]

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.

Comment 38

[Mike Lee [:mlee]]

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.

Comment 39

[Peter La]

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?

Comment 40

[Kevin Grandon :kgrandon]

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.

Comment 41

[Peter La]

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 42

[Peter La]

See above comment.

Comment 43

[Kevin Grandon :kgrandon]

(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.

Comment 44

[:Eli Perelman]

Do we have an interim solution to this?

Comment 45

[Stephany Wilkes]

Clearing the UX general flag and nudged Peter and Patryk to respond here. Sorry for blocking!

Comment 46

[Peter La]

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?

Comment 47

[Peter La]

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.

Comment 48

[:Eli Perelman]

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?

Comment 49

[Peter La]

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?

Comment 50

[Kevin Grandon :kgrandon]

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.

Comment 51

[:Eli Perelman]

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

[Jason Smith [:jsmith]]

Carrying over blocking flag & regression keyword from dupe.

Comment 54

[:Eli Perelman]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20250

New patch for committing against v1.4.

Comment 55

[Ryan VanderMeulen [:RyanVM]]

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.