Closed Bug 796242 Opened 12 years ago Closed 12 years ago

Homescreen doesn't start dragging until movement exceeds tap threshold

Categories

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

Product:
Firefox OS Graveyard
Component:
Gaia::Homescreen
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(blocking-basecamp:-, b2g18 fixed)

Status:
RESOLVED FIXED
Project Flags:
blocking-basecamp -
Tracking Flags:
Tracking Status
b2g18 --- fixed

People

(Reporter: ghtobz, Assigned: mwu)

References

Details

(Keywords: perf, Whiteboard: interaction, UX-P1)

Attachments

(1 file)

Improve panning on the homescreen
1.07 KB, patch
gal
: review+
cjones
: feedback+
timdream
: feedback+
cjones
: approval-gaia-v1+
Details | Diff | Splinter Review 
[GitHub issue by timdream on 2012-07-02T11:51:30Z, https://github.com/mozilla-b2g/gaia/issues/2111]
This affects both touch events and mouse events. Somewhere (driver, gonk, gecko) must have implemented a "threshold" to not fire move events if the finger is not moved far away enough.

For any animation we made for something to follow the finger, this results the animation appears "jumping" when starts. Try to test it out on lock screen for instance.

Does anyone ever seem a bugzilla bug about this?
[GitHub comment by etiennesegonzac on 2012-07-02T13:47:48Z]
Nope, but I will offer several beers for this one :)
[GitHub comment by kanru on 2012-07-04T08:37:45Z]
This only affects SGS2 right? You can run |adb shell getevent| and observe the events come from kernel. I think it is a hw or kernel issue.
[GitHub comment by timdream on 2012-07-04T08:41:19Z]
@kanru it affects Otoro also.
[GitHub comment by timdream on 2012-07-04T09:06:21Z]
@kanru OK, for SGS2 `getevent` doesn't report anything during the blank.

You stated there is no filtering in place in kernel, so I guess it's hw issue? RESOLVE WONTFIX?
[GitHub comment by kanru on 2012-07-04T09:15:39Z]
I might be wrong about the kernel code. Let me check Otoro first :)
[GitHub comment by timdream on 2012-08-14T09:32:43Z]
@kanru Do we have an update on this? Ready to claim your beer from @etiennesegonzac ?
[GitHub comment by kanru on 2012-08-20T16:28:08Z]
no luck..
Priority: P2 → --
Whiteboard: [label:homescreen][label:contacts][label:needsGeckoSupport][label:system][label:system/lockscreen] → interaction, UX-P2
Chris, and thoughts here? Improving this would be a nice perceived performance win...
Whiteboard: interaction, UX-P2 → interaction, UX-P1
Flags: needinfo?(jones.chris.g)
I agree that this would be a nice win but I don't have any new data to contribute.
Flags: needinfo?(jones.chris.g)
Moving this out of Gaia component.
Component: Gaia → General
Who would be the right person to look into this further?

Vivien, any ideas?
On the otoro, getevents shows that we're getting events before the homescreen starts to move. So, at least a noticeable part of the latency on the homescreen comes from somewhere else. I think it's coming from gaia.

https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/js/grid.js#L49

Note how we never start our panning animation code until we exceed the tap threshold. I've fixed this in the past as part of some homescreen panning improvements but it got reverted.
(In reply to Michael Wu [:mwu] from comment #12)
> On the otoro, getevents shows that we're getting events before the
> homescreen starts to move. So, at least a noticeable part of the latency on
> the homescreen comes from somewhere else. I think it's coming from gaia.
> 
> https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/js/grid.
> js#L49
> 
> Note how we never start our panning animation code until we exceed the tap
> threshold. I've fixed this in the past as part of some homescreen panning
> improvements but it got reverted.

So if I understand correctly, there are two issues at play:

1.) Unnecessary gap between receiving movement events and then drawing movement onscreen 
2.) Distance-based threshold in panning code.

A time-based safety threshold has been proposed in other related bugs (eg: #766066), in order to solve for false button and keyboard registrations when user is scrolling. The idea being:

* Register input
* Wait N milliseconds
* If input state is still active and XY coordinates have changed, you're scrolling! No need to show the hit state. :)
* If input state is still active and XY coordinates have _not_ changed, show button hit state.

That was our evaluation of the iOS and Android approaches. Might be part of the solution here as well? Remove distance-based threshold (as you did earlier) and replace with time-based?
Keywords: perf
(In reply to Josh Carpenter [:jcarpenter] from comment #13)
> (In reply to Michael Wu [:mwu] from comment #12)
> > On the otoro, getevents shows that we're getting events before the
> > homescreen starts to move. So, at least a noticeable part of the latency on
> > the homescreen comes from somewhere else. I think it's coming from gaia.
> > 
> > https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/js/grid.
> > js#L49
> > 
> > Note how we never start our panning animation code until we exceed the tap
> > threshold. I've fixed this in the past as part of some homescreen panning
> > improvements but it got reverted.
> 
> So if I understand correctly, there are two issues at play:
> 
> 1.) Unnecessary gap between receiving movement events and then drawing
> movement onscreen 

I think of this as all the unknown factors adding lag.

> 2.) Distance-based threshold in panning code.
> 

Yeah.

I'm going to take a quick look at the generic panning code we have for webapps - there might be a similar threshold panning issue there too.

> A time-based safety threshold has been proposed in other related bugs (eg:
> #766066), in order to solve for false button and keyboard registrations when
> user is scrolling. The idea being:
> 
> * Register input
> * Wait N milliseconds
> * If input state is still active and XY coordinates have changed, you're
> scrolling! No need to show the hit state. :)
> * If input state is still active and XY coordinates have _not_ changed, show
> button hit state.
> 
> That was our evaluation of the iOS and Android approaches. Might be part of
> the solution here as well? Remove distance-based threshold (as you did
> earlier) and replace with time-based?

What I did to reduce the lag here was merely to start animating immediately. The old code waiting until after the threshold was crossed to start animating. It still uses a distance based threshold, so the behavior is the same when it comes to registering taps, but the animation starts as soon as our X coordinate changes. It seems to have a positive effect - etienne tried it and saw an improvement.

I think distance based thresholds vs. time based is a somewhat orthogonal issue to the one here.
This is the gaia patch that etienne tried. It starts animating the homescreen faster.
Is there any movement on this bug?
Comment on attachment 689725 [details] [diff] [review]
Improve panning on the homescreen

Josh, do you mind seeing if this patch improves things for you? I think this patch gives us about as much as we can get for v1. I checked the panning code for other apps but they don't seem to have any glaring issues like this one in the homescreen.
Attachment #689725 - Flags: review?(21)
Attachment #689725 - Flags: feedback?(jcarpenter)
Hi Michael, unfortunately I lack the skills to apply and test a patch like this. At this point I'll say that home screen performance has improved to point where I'd be fine without this additional boost, but I would still like to solve the problem of "false hits" when scrolling. For the later, do you perceive any improvement when scrolling through Settings, for example?
Comment on attachment 689725 [details] [diff] [review]
Improve panning on the homescreen

Please treat this as a high priority soft-blocker.
Attachment #689725 - Flags: feedback?(timdream+bugs)
Comment on attachment 689725 [details] [diff] [review]
Improve panning on the homescreen

This is a valid Gaia homescreen fix which doesn't belong to this bug. We should land it, but in another bug :)
Attachment #689725 - Flags: feedback?(timdream+bugs) → feedback+
Comment on attachment 689725 [details] [diff] [review]
Improve panning on the homescreen

Nice fix. Land it!
Attachment #689725 - Flags: review?(21) → review+
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #20)
> Comment on attachment 689725 [details] [diff] [review]
> Improve panning on the homescreen
> 
> This is a valid Gaia homescreen fix which doesn't belong to this bug. We
> should land it, but in another bug :)

Fair enough.
It ain't gecko, it's the homescreen.
Summary: *start and *move event can never happen near enough no matter how slow the finger moves → Homescreen doesn't start dragging until movement exceeds tap threshold
Component: General → Gaia::Homescreen
Comment on attachment 689725 [details] [diff] [review]
Improve panning on the homescreen

I can confirm that this patch is a noticeable responsiveness win.
Attachment #689725 - Flags: feedback?(jcarpenter) → feedback+
damnable bugzilla ...
Assignee: nobody → mwu
Comment on attachment 689725 [details] [diff] [review]
Improve panning on the homescreen

Very safe patch with improvement to UX-P1 and addresses partner concerns.

Let's get this landed.
Attachment #689725 - Flags: approval-gaia-master+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: