1077169 - (App-grouping) Collapsing a large group can result in an animation with a poor frame-rate

Status: RESOLVED WONTFIX

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

Description

[Maria Sandberg [:mushi]]

Steps:

1. Have a big group on the homescreen. (Dev build is perfect for this).
2. Scroll to a position where the top of the group is close to the top of the screen. 
3. Tap the collapse button. 

Observed:
Group collapses. Immediately afterwards the screen janks downwards. The scroll bar is shown briefly.

Expected:
Group collapses smoothly, keeping the top in the vertical position it was before collapsing.

Comment 1

[Chris Lord [:cwiiis]]

This jankiness is entirely fixed by the WIP patch in platform bug 927349. We can work around it, but it adds significant enough complexity that I don't think it's worth doing, rather than just waiting for that bug to land.

Do please add the relevant information in that bug if you have time-frames that you'd like honoured, however!

The scroll bar showing momentarily is a side-effect of the overflow area of the container changing - we can't really work around this, it will show at some point - we could delay when it shows by hacking around this, but if we don't want it to show, we need to change platform behaviour (the scrollbar of a page always shows when it changes size, and collapsing a group causes there to be less vertical scroll area).

Comment 2

[Maria Sandberg [:mushi]]

Thanks Chris - seems fine to wait for bug 927349 to land, I'll comment in that bug to try and get a time estimate.

Comment 3

[Chris Lord [:cwiiis]]

Actually, the jankiness isn't *entirely* fixed by bug 927349, there's still some Gaia work we can do to make collapsing/expanding really big groups work more smoothly. Currently each icon in the group will animate, but we should take care to just hide instantly the off-screen icons (past the ones that are visible in the collapsed group).

Comment 4

[Chris Lord [:cwiiis]]

Re-titling to reflect what I'm actually tracking in this bug.

Comment 5

[Chris Lord [:cwiiis]]

This doesn't block the app-grouping feature, but is a nice-to-have.

Comment 6

[Ting-Yu Chou [:ting] (away)]

In an original profile [1], there's a 590ms referesh (#2) due to removing place holders [2]. Since place holder does not need to have real HTML element when the row has at least one icon, so I simply skip place holder rendering in this case and get another profile [3]. But still the long refresh (485ms) exists, it spends ~200ms painting text shadow (blur). Should be the icon names, wonder it repaints full homescreen or only the collapsed group.

[1] http://people.mozilla.org/~bgirard/cleopatra/#report=dd50a91f9ab5d815ddb1eea4a642d4f631879464
[2] http://lxr.mozilla.org/gaia/source/shared/elements/gaia_grid/js/grid_view.js#449
[3] http://people.mozilla.org/~bgirard/cleopatra/#report=8c69f7ce1d5f8fc3c32b1904ec974e18094aa3c9

Comment 7

[Ting-Yu Chou [:ting] (away)]

Note also _renderOnToggle() spends >100ms on events gaiagrid-resize (get clientHeight), gaiagrid-saveitems, and gaiagrid-attention (get offsetTop).

Comment 8

[Ting-Yu Chou [:ting] (away)]

(In reply to Ting-Yu Chou [:ting] from comment #7)
> Note also _renderOnToggle() spends >100ms on events gaiagrid-resize (get
> clientHeight), gaiagrid-saveitems, and gaiagrid-attention (get offsetTop).

Get clientHeight and offsetTop triggers reflow and paint the icons' title (also its shadow).

Comment 9

[Ting-Yu Chou [:ting] (away)]

Another thing I noticed is the same icon's title could be painted twice while collapsing/expanding a group even it does not belong to the group:

D/x       (30194): gfxFontGroup::InitTextRun 0xb1d3b550 Social
D/x       (30194): FrameLayerBuilder::PaintItems 6 nsDisplayTransform ctx=0xb0d85ca0 rc=0xbec84e6c 0 -40 1.000000 1.000000
D/x       (30194): FrameLayerBuilder::PaintItems 1 Text ctx=0xb0d85ca0 rc=0xbec83ee4 0 -150 1.000000 1.000000
D/x       (30194): > nsTextFrame::PaintText 0xb1d3b550
D/x       (30194): AlphaBoxBlur::Blur iis=230x163, size=200x134, r=42x42 t1=10482.543539909 t2=10482.544433763
D/x       (30194): AlphaBoxBlur::Blur iis=155x87, size=142x76, r=13x13 t1=10482.544695638 t2=10482.544988086
D/x       (30194): < nsTextFrame::PaintText
D/x       (30194): FrameLayerBuilder::PaintItems 6 nsDisplayTransform ctx=0xb0d85d60 rc=0xbec84e6c 0 -40 1.000000 1.000000
D/x       (30194): FrameLayerBuilder::PaintItems 1 Text ctx=0xb0d85d60 rc=0xbec83ee4 0 -150 1.000000 1.000000
D/x       (30194): > nsTextFrame::PaintText 0xb1d3b550
D/x       (30194): AlphaBoxBlur::Blur iis=103x163, size=73x134, r=42x42 t1=10482.549657773 t2=10482.550073555
D/x       (30194): AlphaBoxBlur::Blur iis=25x33, size=12x22, r=13x13 t1=10482.550196524 t2=10482.550209909
D/x       (30194): < nsTextFrame::PaintText

Comment 10

[Ting-Yu Chou [:ting] (away)]

GridView.render() iterates throguh each icon to setCordinates() and set style transform to translate its position when collapse/expand a group, but actually only y of them is changed, wonder is it possible to shift all of them at once, for instance make all shifted icons a layer and translate it.

Comment 12

[Bobby Chien]

[Tracking Requested - why for this release]:

Comment 13

[Bobby Chien]

App-Group is deprecated since gecko 44/v2.5. Should we close this case?

Comment 14

[Chris Lord [:cwiiis]]

Yes.