There is a noticeable delay (~200ms?) between the time that one pans right on the "time screen" to the first page of the app list. This makes one feel that they are /fighting/ the UI to get the app list. Panning left to e.me also has this problem but is dwarfed by bug 842213.
Created attachment 715509 [details] Patch v1 This patch is based on the idea of a dedicated method for changing opacity carefully written to avoid as much as possible allocations and condition checks. Moreover we set the opacity with only one decimal. Thanks to those changes we achieve 8-10 fps more than before in pages affected by opacity transformations.
With this patch, we don't see much change in the initial lag (around 150-200 ms). During the lag, we see a paint occurring with no updates to the compositor.
Hi Michael, to test this patch you should go to landing page and then move your finger around in a small circle as fast as you can. The frame rate increases about 8-10 fps with this patch in compare to master. Maybe the lag is due to set the opacity... OK, this is one thing, now, please could you give your STR to calculate that lag? Could you confirm us that my patch improves the frame rate? thanks a lot
Comment on attachment 715509 [details] Patch v1 I made some comments but I would also like to wait for Michael to explain how they measure this lag /fps so we check if the patch is useful or not.
The lag no idea :) but fps increases a lot!! If the patch doesn't resolve this perhaps we could open other about fps because I wouldn't like to loose it. Thanks for your help
(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #4) > Comment on attachment 715509 [details] > Patch v1 > > I made some comments but I would also like to wait for Michael to explain > how they measure this lag /fps so we check if the patch is useful or not. This patch is definitely useful (thanks!) but doesn't help what the issue reported by this bug, which is the *initial* lag we one swipes/pans right from the "clock screen" to the first page of apps. Panning makes this lag more obvious. We can measure this by adding tracepoints in the system at the time that input is received and when a display update occurs. During this time we see a ~100ms paint happening with no display updates. We need this to occur in 100/60 ms to prevent stutter
Do you mean that we are translated pages with display none during 100ms, isn't it? thanks
Not sure quite, let me try again: 1) User initiates pan and input is received by Gecko 2) Gecko renders a bunch of stuff (for ~100ms) 3) First display update occurs If we can make #2 much smaller than we'll get a better UX when the user swipes/pans into the app list.
Duplicate of this bug: 843263
(In reply to Michael Wu [:mwu] from comment #1) > I had a patch but it's gone now. However, the hack was pretty simple - in > apps/homescreen/js/grid.js, I changed all instances of > togglePagesVisibility(start, end); to togglePagesVisibility(start - 1, end + > 1); so we always show the next and previous pages. The problem is the next > and previous pages default to being positioned on the current page, so they > all pile on top of each other until you move them out of the way. This (shouldn't) pin the pages visible, because once they're repositioned by gaia they'll be offscreen and gecko will nuke them again.
I am going to provide a new patch in a couple of hours
Created attachment 716459 [details] Patch v2 This patch is based on the idea about having visibles the current, previous and next pages. Maximum are three pages with display block, the rest are not available (display none). Then, we don't force re-painting and translations during the first touchmove event. At that point, we have the pages ready to pan. Please, Michael, could you test it?
Created attachment 716790 [details] [diff] [review] Stupid gecko tricks (WIP; foiled by spaghetti) This is the spec for a change that will make a big different in the lag on switching screens. It uses some specialized gecko knowledge to force all the homescreen pages to stay rendered at all times, and then uses opacity to "hide" them when they're "offscreen". The patch here is a kind of "declarative spec" for the change. In a nutshell, we want to - wrap page <div>s in an outer div to enable the opacity hack. Transform and transition this new outer div. - stop using |visibility: visible/none| to hide offscreen pages - instead of translating offscreen pages off the screen, put them at translateX(0) and give them a negligible opacity The change seems relatively simple to implement but I'm flailing around trying to make the .js changes :(. We edit page.container/movableContainer styles from a bunch of different places, and I don't know which places to change, and how. So I didn't include the WIP .js changes. Can anyone provide guidance on how to apply this change? Is there a bug on file for cleaning up the code here? :)
Could somebody check my new patch?
working on chris comments
After working on different approaches I have these results: 1) Patch based on Chris Jones comments Branch: https://github.com/crdlc/gaia/tree/lag-cjones-comments Commit : https://github.com/crdlc/gaia/commit/82423d7059afb579dc62724d6f6cbe1cbb97325d Maybe (or I am sure :)) I forgot some comments because the result are not so good * Lag disappear. First goal achieved * Re-paint issue is still present * After navigating for all pages and ev.me the fps are worst 2) Patch based on my idea (only current, previous and next pages are displayed, the rest are out of memory) Branch: https://github.com/crdlc/gaia/tree/bug-842218-v2 Commit: https://github.com/crdlc/gaia/commit/c01cda3711314b8ec6af9befc68fa8ca8369b1ba Results: * Seems that totally the lag goes away * Re-painted issue disappeared * After navigating for all pages and ev.me the fps are the same Could you test both approaches and give me your feedback? Thanks a lot Chris
(In reply to crdlc from comment #16) > After working on different approaches I have these results: > > 1) Patch based on Chris Jones comments > > Branch: https://github.com/crdlc/gaia/tree/lag-cjones-comments > Commit : > https://github.com/crdlc/gaia/commit/82423d7059afb579dc62724d6f6cbe1cbb97325d > > Maybe (or I am sure :)) I forgot some comments because the result are not so > good This doesn't implement the "don't move pages offscreen" part of comment 13 (when that happens, gecko will throw away the rendered content), so I'm not surprised the perf is bad :).
Hi, I updated both branches and I have more or less the same results with your tricks. Has it been implemented correctly? https://github.com/crdlc/gaia/tree/lag-cjones-comments https://github.com/crdlc/gaia/commit/776ba9ccb805317fdf0faa0e27343dd48adbb947 My other patch in my honest opinion works fine, did you test? https://github.com/crdlc/gaia/tree/bug-842218-v2 https://github.com/crdlc/gaia/commit/71e7f9cdebd3b24631b745f0cd6e50697a99ec87 Thanks
Hi crdlc, the second patch works fine for me.
ok thanks for your feedback Jan! Michael, could you test as well the lag? thanks a lot
Comment on attachment 717864 [details] Patch v2 That sounds much smarter to update page visibility after the transition than before the pan..
Attachment #717864 - Flags: review?(21) → review+
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Comment on attachment 717864 [details] Patch v2 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: more lag, less FPS and sometimes pages are not re-painted Testing completed: yes Risk to taking this patch (and alternatives if risky): low risk String or UUID changes made by this patch:
Attachment #717864 - Flags: approval-gaia-v1?
status-b2g18: --- → affected
status-b2g18-v1.0.0: --- → wontfix
status-b2g18-v1.0.1: --- → wontfix
tracking-b2g18: --- → +
Comment on attachment 717864 [details] Patch v2 Based on Jan's testing we can take this fix for uplift to v1-train.
Attachment #717864 - Flags: approval-gaia-v1? → approval-gaia-v1+
I'm happy to go with whichever implementatioon Vivien prefers.
(In reply to crdlc from comment #21) > Michael, could you test as well the lag? Yep, definitely helps. Thanks!
Attachment #717864 - Flags: feedback?(mvines) → feedback+
Commit 07b2f27bb7a94035bcc33e1c2a99ce7b61c4e6aa does not apply to v1-train. This means that there are merge conflicts which need to be resolved. If there are dependencies that are not approved for branch landing, or have yet to land on master, please let me know If a manual merge is required, a good place to start might be: cd gaia git checkout v1-train git cherry-pick -x -m1 07b2f27bb7a94035bcc33e1c2a99ce7b61c4e6aa <RESOLVE MERGE CONFLICTS>
v1-train has this patch https://github.com/mozilla-b2g/gaia/commit/faa2cd3042ab0a6d2c6d73b64d7991fee811735f
Requesting uplift to 1.0.1. This patch was a part of the MWC build.
blocking-b2g: --- → tef?
To ensure that this patch works well on its own (seeing it just landed in v1-train) can we get some QA confirmation that this fix does what it states in out builds - side by side comparison?
QA Contact: jsmith
Going to find a new assignee to the verification.
QA Contact: jsmith
This issue was compared on both Unagi build Id:20130227070202 V1 and on Unagi, build id:20130225070200 V1.0.1. Couldn't notify any big difference. For making it much clear,below is the link, which shows comparision on both V1 and V1.0.1 http://www.youtube.com/watch?v=6I4x5Vfdi88&feature=youtu.be
QA Contact: pgorantla
Please create three pages with icons and go to second page and then move your finger around in a small circle as fast as you can. You can see as next and previous ones are not repainted correctly
Adding QA wanted again for comment 35
pallavi - can you address comment 35?
Keywords: qawanted → verifyme
Yes, I can see the noticeable difference between V1 and V1.0.1 On Unagi,build id:20130225070200 V1.0.1 the pages are not repainting correctly On Unagi, build id:20130228070204 V1 Issue was fixed
Status: RESOLVED → VERIFIED
It blocks bug 827130, that is tef+ and cannot be merged in v1.0.1, so this bug should be tef+
blocking-b2g: tef? → shira+
status-b2g18-v1.0.1: wontfix → affected
(jhford -- uplift to v1.0.1 please?)
status-b2g18-v1.0.1: affected → fixed
You need to log in before you can comment on or make changes to this bug.