Closed
Bug 1062331
Opened 10 years ago
Closed 9 years ago
Turn on by default touch event smoothing with gfx.frameuniformity.hw-vsync and gfx.touch.resample
Categories
(Core Graveyard :: Widget: Gonk, defect)
Tracking
(feature-b2g:2.2+, firefox36 wontfix, firefox37 wontfix, firefox38 fixed, b2g-v2.2 fixed, b2g-master fixed)
People
(Reporter: milan, Assigned: mchang)
References
Details
Attachments
(8 files)
4.42 MB,
video/quicktime
|
Details | |
7.35 MB,
video/quicktime
|
Details | |
70 bytes,
text/plain
|
Details | |
94 bytes,
text/plain
|
Details | |
6.05 MB,
text/plain
|
Details | |
67 bytes,
text/plain
|
Details | |
855 bytes,
patch
|
kats
:
review+
|
Details | Diff | Splinter Review |
867 bytes,
patch
|
bajaj
:
approval-mozilla-b2g37+
|
Details | Diff | Splinter Review |
We need to do UX investigation then decide if we like the touch event smoothing and turn it on by default.
Reporter | ||
Comment 1•10 years ago
|
||
Start having it assigned to Gordon to get the decision.
Assignee: nobody → gbrander
feature-b2g: --- → 2.1
Comment 2•10 years ago
|
||
Toggled the following on in aurora: user_pref("gfx.frameuniformity.hw-vsync", true); user_pref("gfx.touch.resample", true); I see no difference in hitching at low or high scrolling speeds. Am I doing something wrong?
Flags: needinfo?(mchang)
Reporter | ||
Comment 3•10 years ago
|
||
With a restart, and on Flame, I assume.
Assignee | ||
Comment 4•10 years ago
|
||
Gordon and I chatted about this and Gordon wanted to test after bug 1063158 landed. I did some user testing. While scrolling with one hand and naturally, it was difficult to tell if any difference at all. Slow scroll side by side, it was easy to tell the difference, but they remarked that they "never scrolled like that". Another person is user testing a taco, which is a higher end device. I noticed that it was very janky on a taco. I enabled the pref on the taco and scrolling got MUCH better. I wonder if it will only be very user noticeable on higher end devices. But since bug 1063158 just landed, Gordon, can you please try again on the contacts or settings app? Thanks!
Flags: needinfo?(mchang)
Comment 5•10 years ago
|
||
(In reply to Mason Chang [:mchang] from comment #4) > Gordon and I chatted about this and Gordon wanted to test after bug 1063158 > landed. I did some user testing. While scrolling with one hand and > naturally, it was difficult to tell if any difference at all. Slow scroll > side by side, it was easy to tell the difference, but they remarked that > they "never scrolled like that". Another person is user testing a taco, > which is a higher end device. I noticed that it was very janky on a taco. I > enabled the pref on the taco and scrolling got MUCH better. I wonder if it > will only be very user noticeable on higher end devices. But since bug > 1063158 just landed, Gordon, can you please try again on the contacts or > settings app? Thanks! Bug 1063158 did not land - it was closed as WONTFIX. If the reason for waiting for bug 1063158 to be fixed was to fix the jankiness of the scrollbars, then I expect that to be fixed in bug 1061327 instead.
Comment 6•10 years ago
|
||
Just for informational purposes, I have started dogfooding this on my tako device. Scrolling is indeed buttery (silky) smooth. However, there are occasional periods where the jank level goes way up for a few seconds before switching back to smooth. I haven't been able to reproduce this consistently, but I will post here if I figure out how.
Comment 7•10 years ago
|
||
What you're describing sounds like what I would expect to happen when the main thread is busy, because the events will get blocked. Bug 930939 is the thing that should address that. However, what you're seeing in that scenario should be no worse than it is with the default behaviour today.
Assignee | ||
Comment 8•10 years ago
|
||
HD video with scrolling on two flames side by side. Left one has the patch.
Assignee | ||
Comment 9•10 years ago
|
||
HD video of scrolling settings
Assignee | ||
Comment 10•10 years ago
|
||
Due to the 10mb limit, had to upload this to dropbox.
Comment 11•10 years ago
|
||
Looks good per video. On device I could tell no difference, but that's not the same thing as a regression. Buttery-smooth is always a good thing, assuming there aren't regressions in responsiveness. It sounds like we've addressed the touch responsiveness issues and skate "slowness" issues by using a new algorithm, so I don't think I have more to contribute from UX side. At this point it seems to be all about the numbers. Assuming GFX is on board, we should probably take the normal approach of "land it -> file bugs against regressions", rather than bottlenecking on UX. If we find regressions, we can open a bug.
Assignee: gbrander → mchang
Comment 12•10 years ago
|
||
Just to be clear, I have been dogfooding this for a few days, and the jank I talked about in comment 6 still does show up from time to time. And it is definitely more jank than we experience with this feature turned off. I will try to get a profile next time it happens if I am in front of a computer, but I highly recommend someone else besides me dogfood before landing. I'm using the Tako.
Reporter | ||
Comment 13•10 years ago
|
||
Let's not change the pref until we sort out what Michael mentions in comment 6 and comment 12. Mason, you've done some testing on tako, does that mean you have access to one and a local build you can make?
Assignee | ||
Comment 14•10 years ago
|
||
I don't have a tako. The only reason I was able to test on a tako was because Michael was in the office with a tako and I noticed his scrolling was janky so I wanted to test this on his device. Michael also said that the tako is still pretty buggy. For example, sometimes the display won't come on for a couple of hours. If the vsync events aren't reliably coming in, we'll be janky. I'm going to dogfood on a flame for a while and see if I can reproduce. Gordon, can you also see if you can reproduce what Michael says in your testing? Thanks!
Assignee | ||
Comment 15•10 years ago
|
||
Going to test on : Gaia: e98b6c9a2ee9b2af4707f6501b7f31f37557e719 Gecko: 205307:56cba2986c61
Assignee | ||
Comment 16•10 years ago
|
||
I noticed a pretty big difference on a flame while scrolling my email inbox. Gordon, can you see any visual difference with your email?
Flags: needinfo?(gbrander)
Assignee | ||
Comment 17•10 years ago
|
||
After lots of scrolling and playing with it for a couple of hours, I got some about-memory reports of master versus with this patch on. I didn't see any memory leaks and I haven't seen any staggering jank that I haven't seen on master. Will continue evaluating over the week.
Assignee | ||
Comment 18•10 years ago
|
||
I hit my first large jank. It occurs after the screen has been off for a while, usually about 2-3 hours. When I turn on the device and unlock, the scrolling immediately after is very janky. I also see lots of checkerboarding, low res tiling, and overall system unresponsiveness. After a couple of seconds, everything smoothes out again. After talking with Michael Henretty, he said he noticed the same thing on a taco and that scrolling also usually smoothes out. This makes sense from the algorithm's point of view. We smooth out the touch input based on a consistent vsync event. Sometimes we will extrapolate, sometimes we will interpolate. If the main thread is busy and we get a touch event, we will resample based on the last two touch events. Since the last two touch events are added to the resampler on the android input thread, the touch coordinate difference between the last touch event APZ has processed and the last touch event the android input thread has seen will be large, creating very visible jank. Without touch resampling, we will dispatch all touch events in order, creating less visible jank. The upside to touch resampling is that we do less work in that we process fewer touch events. The downside is that in these bad cases, the jank will be more jarring, but at least we won't keep piling on touch events. I will do more investigation to see if I can consistently recreate this, but the "more jank when the main thread is busy" makes sense. The real way to fix this is get touch events off main thread, which is bug 930939. Personally, I think it's ok if we make the worst case worse in exchange for making the average case better.
See Also: → 1064927
Reporter | ||
Comment 19•10 years ago
|
||
(In reply to Mason Chang [:mchang] from comment #18) > .... Since the last > two touch events are added to the resampler on the android input thread, the > touch coordinate difference between the last touch event APZ has processed > and the last touch event the android input thread has seen will be large, > creating very visible jank. Without touch resampling, we will dispatch all > touch events in order, creating less visible jank. Sounds like an opportunity to enhance our algorithm to avoid this problem? I'm not sure if we have, or could have access to all this data in one place, but can we detect this scenario in code and act accordingly, perhaps not doing the event smoothing in some cases?
Assignee | ||
Comment 20•10 years ago
|
||
I am able to somewhat reproduce this. I thought of using my favorite destroy the phone test case, loading nytimes.com. I loaded nytimes.com, let it load, wait for the video to play. This usually takes a minute or so. Everything while scrolling nytimes.com will be janky, checkerboarding / low res tiling, then go to the homescreen. There will be lots of initial jank, especially after the phone has been on for a while. Here is a profile - https://people.mozilla.org/~bgirard/cleopatra/#report=27689cb1ba1ad5937e3b74e4a44cee97c2bdfbc6 We see lots of 30-50ms garbage collection near the end, and we also see that we don't composite and extra long composites. I'm also worried that we have 78 ms display list rendering times. I'm not convinced this is a resampling issue versus, it's just more apparently that the system is hurting under load.
Assignee | ||
Comment 21•10 years ago
|
||
We're using tons of memory.
Assignee | ||
Comment 22•10 years ago
|
||
Side by side scrolling with touch resampling versus master on the right. We see with touch resampling, we scroll and jank harder with intermittent smoothness. Master has constant jank, with sometimes bigger janks in between.
Assignee | ||
Updated•10 years ago
|
Comment 23•10 years ago
|
||
Per last week's meeting, clearing needsinfo https://etherpad.mozilla.org/ux-gfx. Mason wanted more eyes on this. Will give it a spin.
Flags: needinfo?(gbrander)
Assignee | ||
Comment 24•9 years ago
|
||
After our discussion, UX says they can see a major difference and approve.
Flags: needinfo?(gbrander)
Comment 25•9 years ago
|
||
Yup. No downside, all upside as far as I can tell.
Flags: needinfo?(gbrander)
Comment 26•9 years ago
|
||
as comment 24 and comment 25, case resolved.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 27•9 years ago
|
||
This actually isn't enabled by default yet, we will soon once we finish enabling parts of silk. Reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 28•9 years ago
|
||
In this case, should we still mark this one as feature-b2g:2.1? Are we targeting to change anything in 2.1?
Assignee | ||
Comment 30•9 years ago
|
||
Turn touch resampling on b2g only. Will land next week.
Attachment #8556516 -
Flags: review?(bugmail.mozilla)
Comment 31•9 years ago
|
||
Comment on attachment 8556516 [details] [diff] [review] Enable touch resampling on b2g Review of attachment 8556516 [details] [diff] [review]: ----------------------------------------------------------------- Is the wait just to let the dust settle a bit?
Attachment #8556516 -
Flags: review?(bugmail.mozilla) → review+
Assignee | ||
Comment 32•9 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #31) > Comment on attachment 8556516 [details] [diff] [review] > Enable touch resampling on b2g > > Review of attachment 8556516 [details] [diff] [review]: > ----------------------------------------------------------------- > > Is the wait just to let the dust settle a bit? Yeah, I want to give each pref a week to test before turning on the next one.
Comment 33•9 years ago
|
||
Sounds good. FYI I plan on landing 930939 as soon as I get the last r+
Assignee | ||
Comment 34•9 years ago
|
||
Manually tested on today's master, git rev 8e960009be915a67284ba3edd2b8ca67ff601f7c. Everything looking good to enable next week. Successful try - https://treeherder.mozilla.org/#/jobs?repo=try&revision=fbfa70ab3624
Assignee | ||
Comment 35•9 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/b79e7b655931
Comment 36•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b79e7b655931
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
status-firefox38:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Assignee | ||
Comment 37•9 years ago
|
||
Uplift to 2.2 next week if everything is stable.
Flags: needinfo?(mchang)
Comment 39•9 years ago
|
||
tested with bug 1118530, and other than the jankiness mentioned in comment 20, I don't see any other issues. But in some instances the browser OOMs, so filed Bug 1129126 for it. But since this also happens in 2.2, as mchang says, I don't think it's specifically related to this.
Flags: needinfo?(npark)
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(mchang)
Assignee | ||
Comment 40•9 years ago
|
||
Rebased for mozilla-b2g37_v2_2 [Approval Request Comment] Bug caused by (feature/regressing bug #): Project Silk User impact if declined: Janky scrolling while tracking a user's finger Testing completed: Manual, has been enabled on master for 1 week, tested by No-Jun in comment 39. Risk to taking this patch (and alternatives if risky): Medium - This changes how we send touch events to send only 1 per vsync interval. String or UUID changes made by this patch: None Note: requires bug 1129210, which has b2g-37 approval to land first.
Flags: needinfo?(mchang)
Attachment #8561167 -
Flags: approval-mozilla-b2g37?
Updated•9 years ago
|
Attachment #8561167 -
Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
Comment 41•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/3cef83920885
status-b2g-v2.2:
--- → fixed
status-b2g-master:
--- → fixed
status-firefox36:
--- → wontfix
status-firefox37:
--- → wontfix
Updated•6 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•