1023940 - Optimizations for gaia homescreen edit mode

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Kevin Grandon :kgrandon]

This is a placeholder bug to track optimizing gfx code for the gaia homescreen edit mode.

Comment 1

[Kevin Grandon :kgrandon]

Milan - Just doing a ni? on you here so you are aware. We are still in the process of making some final dragdrop improvements from the gaia side of things, and should have something landed this week - it's taken a bit longer than I would've hoped.

We don't plan to make any major changes though if possible, so you may be able to start doing some initial investigation here. I can help upload a profile this week if it would help.

Comment 2

[Benoit Girard (:BenWa)]

Can you collect 2 profiles:
0) Make sure you built with 'export B2G_DUMP_PAINTING=1' in your userconfig.
1) First one using:
./profile.sh start -p b2g -t Compositor && ./profile.sh start -p Vertical
2) Second one (reboot in between):
set layout.display-list.dump;true via edit-prefs.sh
./profile.sh start -p b2g -t Compositor && ./profile.sh start -p Vertical

Comment 3

[Jason Smith [:jsmith]]

Kevin - Is this required to ship vertical homescreen? Or this is just a strong want, but we would ship without this if we ran out of time?

Comment 4

[Kevin Grandon :kgrandon]

I would like to block for now, we really need this to be as polished as possible. Benoit - I'll work on getting you those profiles asap.

Comment 5

[Jason Smith [:jsmith]]

Okay - Milan - Can you flag feature-b2g flag here, since we're claiming this is feature work & a blocker for ship in 2.0?

Comment 6

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

Done - I like that better than making it 2.0+.  We're going to work on the 2.0 branch based solution, but will still ask for uplift approvals for individual patches.

Comment 7

[Kevin Grandon :kgrandon]

Benwa - I'm still unable to build for flames, but would profiles on a nexus4 help? If not I'll try to see if someone can try to get these for us.

First profile:
https://people.mozilla.org/~bgirard/cleopatra/#report=7e8b98504a78a8bd48b1db8cb89d210a6a0fa7ed

With: layout.display-list.dump = true
https://people.mozilla.org/~bgirard/cleopatra/#report=cfe7d3e398f8a859ef95985cb61d2dfbbaf1f83a

These are the profiles of a few dragdrop operations on the homescreen. I tried to use similar motions, but they probably aren't exact or anything. Let me know if these were helpful, if not I'll try again/on different hardware.

Comment 8

[Benoit Girard (:BenWa)]

In the second profile you must of missed one of the step because the display list isn't in the profile. The profile also aren't synced up. We had that bug in 1.4 so I wonder if you perhaps have some resources that are out of date.

No worries, I'll have a look.

Comment 9

[Benoit Girard (:BenWa)]

Video profile of the edit screen:
http://people.mozilla.org/~bgirard/cleopatra/#report=04102901652b7363c3969fefaec593230204e870

Comment 10

[Benoit Girard (:BenWa)]

Attachment: Part 1: Apply will-change only during edit-mode

This drastically reduces the rasterization time and jank dragging the icon around.

Comment 11

[Kevin Grandon :kgrandon]

Thanks. This is going to make the scrolling suffer again though, no? I'll run this by UX real quick, it seems like that may be ok in edit mode.

Comment 12

[Benoit Girard (:BenWa)]

We shouldn't be scrolling much in edit mode. But yes it will make it suffer a bit until bug 1017251 is solved.

Actually I just realize you can be in edit mode and not be dragging any icon. So if you'd like we can activate will-change only when you're dragging an icon around. We really need will change when we're dragging icons because when we move the inactive icons around right now we have to do a lot of painting and we want that animation to be low latency, so IMO the right tool there is will-change: transform. Right now without it it's painfully slow.

Comment 13

[Kevin Grandon :kgrandon]

Attachment: Part 1 - Apply will-change when dragging an icon

After dogfooding for a while I noticed that scrolling in edit mode is rather common, you often want to rearrange icons from the top, to the bottom, so you make either minor or major scroll adjustments. The hit to scrolling is bad enough, that I don't think we should do it.

When you do begin a drag action though, there is already a delay for the context menu, so I think that might be a decent place to have will-change. There might be a bit of jankiness starting or stopping a drag though, if the user tries to scroll right after.

Comment 14

[Kevin Grandon :kgrandon]

Comment on attachment 8442437 [details] [diff] [review]
Part 1: Apply will-change only during edit-mode

Review of attachment 8442437 [details] [diff] [review]:
-----------------------------------------------------------------

Clearing for now in case we decide to go with the version that has will-change for drag actions.

Comment 15

[Benoit Girard (:BenWa)]

The hit to scrolling is not that noticeable on my end. Are you running with layers border enabled or some similar debugging feature?

Comment 16

[Kevin Grandon :kgrandon]

(In reply to Benoit Girard (:BenWa) from comment #15)
> The hit to scrolling is not that noticeable on my end. Are you running with
> layers border enabled or some similar debugging feature?

It's similar when I opened the other bug, and you suggested that we remove will-change. (I can find that again and show you to video if you want). It's mostly noticeable on APZ deceleration. 

I don't have any debugging features turned on.

Comment 17

[Benoit Girard (:BenWa)]

Yes, but I had layers border on when I saw it which multiplies the hit. It's best to not count on us speed up that code path in time.

Alright let's only apply will-change when you're actively dragging icons then.

Comment 18

[Kevin Grandon :kgrandon]

Comment on attachment 8442501 [details] [review]
Part 1 - Apply will-change when dragging an icon

A review from either of you guys would be fine for this. Thanks.

Comment 19

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8442501 [details] [review]
Part 1 - Apply will-change when dragging an icon

LGTM

Comment 20

[Kevin Grandon :kgrandon]

Will-change modification landed: https://github.com/mozilla-b2g/gaia/commit/df882b8ec7d6476ca3e4f91bd1c44a5ac9c12c77

Comment 21

[Kevin Grandon :kgrandon]

As discussed in our standup we landed will-change support as the first part of this bug. This is good enough for FL quality, and additional work will be uplifted.

Candice - can you remove the FL and FC blocking status? Thanks!

Comment 22

[Benoit Girard (:BenWa)]

I've got patches for bug 1010584 that are nearly ready. I expect that this will be the last piece to resolve this issue.

Comment 23

[Candice Serran (:cserran)]

Is this still in progress? There hasn't been any movement on this for a couple sprints now. Please update target milestone or close. Thanks

Comment 24

[Kevin Grandon :kgrandon]

Dependent bugs are closed, so I think we can call this one fixed now.