1280656 - Use Android ViewConfiguration to get max fling velocity.

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Toolbar, defect, P3)

Description

[Randall Barker [:rbarker]]

The native Android fling animation should use ViewConfiguration to get the max fling speed.

Comment 1

[Randall Barker [:rbarker]]

Attachment: 0001-Bug-1280656-Use-Android-ViewConfiguration-to-get-max-fling-velocity.-r-16061717-f328aee.patch

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8763259 [details] [diff] [review]
0001-Bug-1280656-Use-Android-ViewConfiguration-to-get-max-fling-velocity.-r-16061717-f328aee.patch

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

::: gfx/layers/apz/src/AndroidAPZ.cpp
@@ +106,5 @@
>    if (length > 0.0f) {
>      mFlingDirection = velocity / length;
> +
> +    if (length > sMaxFlingSpeed) {
> +       velocity = mFlingDirection * sMaxFlingSpeed;

You should skip the clamping if sMaxFlingSpeed is 0, like if it couldn't be read properly.

Comment 3

[Randall Barker [:rbarker]]

Attachment: 0001-Bug-1280656-Use-Android-ViewConfiguration-to-get-max-fling-velocity.-r-16061717-33eacc9.patch

Comment 4

[Botond Ballo [:botond]]

(Sorry for the slow response time on reviews. I'm at a C++ standards meeting this week, and while I've been trying to squeeze some time to do some reviews, amidst the evening sessions and impromptu discussions I haven't managed to yet.)

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8763261 [details] [diff] [review]
0001-Bug-1280656-Use-Android-ViewConfiguration-to-get-max-fling-velocity.-r-16061717-33eacc9.patch

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

LGTM, seems pretty straightforward.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b9f69e5a6565
Use Android ViewConfiguration to get max fling velocity. r=kats

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b9f69e5a6565

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Flag for uplift?

Comment 9

[Randall Barker [:rbarker]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #8)
> Flag for uplift?

I'm not sure. On my (mid to low range) devices it doesn't seem to make much of a difference but I believe on devices with more powerful CPUs, where it is more able to keep up with paint requests, this patch appears to reduce checker boarding a little.

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Bug 1279433 at least seems to be better, probably because of this. I vote we uplift it. Can you request approval?

Comment 11

[Randall Barker [:rbarker]]

Comment on attachment 8763261 [details] [diff] [review]
0001-Bug-1280656-Use-Android-ViewConfiguration-to-get-max-fling-velocity.-r-16061717-33eacc9.patch

Approval Request Comment
[Feature/regressing bug #]: Fling velocity exceeds the maximum speed which allows fennec to checker board more easily.
[User impact if declined]: Appears to increase checker boarding
[Describe test coverage new/current, TreeHerder]: just manual testing
[Risks and why]: Minimal. Code prevents the starting velocity of a fling from exceeding a maximum speed.
[String/UUID change made/needed]: none.

Comment 12

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8763261 [details] [diff] [review]
0001-Bug-1280656-Use-Android-ViewConfiguration-to-get-max-fling-velocity.-r-16061717-33eacc9.patch

OK, improve the situation, taking it.

Should be in 48 beta 6

Comment 13

[Sylvestre Ledru [:Sylvestre]]

QE, could you verify this one (bug 1279433 has some str)

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/6c31d36da0d4

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/1d7d231b0a58

Comment 16

[u421692]

I was able to reproduce this issue consistently on Firefox 48 Beta 4 on Samsung Galaxy Tab S2 (Android 5.0.2). 
I am not seeing this issue on Firefox 48 Beta 6, latest Nightly and Aurora, on the same device.
Marking this as verified fixed; we might want to check if this issue still persist on another device, to open up another bug.

Comment 17

[Stefan Deiac]

Based on comment 16 I will remove the qe-verify flag, thanks.