Eliminate pinch drift by removing touch radius from span calculation

Eliminate pinch drift by removing touch radius from span calculation

Previously, we included touch radius in our pinch span computation.
This caused some drift between the users fingers and the web contents
during pinch. Removing the touch radius fixes this problem.

BUG=590026
Committed: https://crrev.com/b89f4c6eefa4f3a8d346c3f747c993845bb34cc7
Cr-Commit-Position: refs/heads/master@{#386377}

[tdresser, 2016-03-09 20:16:56]

The CQ bit was checked by tdresser@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-09 20:17:20]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1778063002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1778063002/1

[tdresser, 2016-03-09 20:17:26]

tdresser@chromium.org changed reviewers:
+ aelias@chromium.org

[commit-bot: I haz the power, 2016-03-09 21:43:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-09 21:43:37]

Dry run: This issue passed the CQ dry run.

[tdresser, 2016-03-21 15:05:53]

I spent a bunch of time playing with this on an N7, as you requested offline.

As far as I can tell, there's only one issue.

Because we're now ignoring the touch radius, the span is now significantly
smaller. This causes us to hit the minimum span threshold much earlier.

We're currently pulling the minimum span size from the Android platform, but
that value is far too big now (in my opinion).

Any thoughts on the correct step forward here?

We could perform some post processing on the value Android passes to us
(subtracting 2 * average touch radius), but that would likely be pretty ugly.

[aelias_OOO_until_Jul13, 2016-03-22 20:15:40]

Are you sure we aren't pulling the value from our copy in
ui/android/java/res/values/dimens.xml, instead of the Android system itself?

I'd suggest changing the value in our copy, if so.  (The Android-side change can
then also easily mirror this approach by changing both the value and the
algorithm in sync.)

[tdresser, 2016-03-23 17:20:14]

At least on my N5,
https://code.google.com/p/chromium/codesearch#chromium/src/ui/android/java/sr...
is successful, and we pull it from the native configuration.

We could potentially compute the minimum span based on the minimum touch radius
and the existing minimum span, but that's pretty hacky.

We could also just stop reading the native value.

[aelias_OOO_until_Jul13, 2016-03-23 22:15:03]

Yeah, we can stop reading the native value, since it's coupled to the algorithm.
 Even if Android changes the value likewise, it would only affect the newest
version, so we need to ignore it anyway.

[tdresser, 2016-03-24 14:26:55]

I've shrunk the min scaling span by about 2 * the minimum touch major.

For the Nexus 5:
27mm - 2 * 48dp = 
2.7cm - 2 * (48dp * 3px/dp * 1in/445px * 2.54cm/in) = 
1.056cm

I bumped this up to 1.2cm as it's safer to have this number a bit bigger.

This feels fine on the Nexus 7 (2012).

[aelias_OOO_until_Jul13, 2016-03-25 23:17:38]

Adding adamp@ to check if Android-side is fine with doing the same change.

Adam, I understand Kingston already talked to you about this; for context, we
cloned the Android gesture recognizer in C++ so that ChromeOS and Windows can
have the exact same gesture feel.  We aim to make sure that always have the
exact same behavior as the latest version of Android, though, so I want to avoid
landing any change that doesn't have a mirroring Android-side one.

[tdresser, 2016-04-04 12:05:28]

On 2016/03/25 23:17:38, aelias wrote:
> Adding adamp@ to check if Android-side is fine with doing the same change.
> 
> Adam, I understand Kingston already talked to you about this; for context, we
> cloned the Android gesture recognizer in C++ so that ChromeOS and Windows can
> have the exact same gesture feel.  We aim to make sure that always have the
> exact same behavior as the latest version of Android, though, so I want to
avoid
> landing any change that doesn't have a mirroring Android-side one.

adamp: Ping!

[adamp, 2016-04-04 23:08:45]

On 2016/04/04 12:05:28, tdresser wrote:
> On 2016/03/25 23:17:38, aelias wrote:
> > Adding adamp@ to check if Android-side is fine with doing the same change.
> > 
> > Adam, I understand Kingston already talked to you about this; for context,
we
> > cloned the Android gesture recognizer in C++ so that ChromeOS and Windows
can
> > have the exact same gesture feel.  We aim to make sure that always have the
> > exact same behavior as the latest version of Android, though, so I want to
> avoid
> > landing any change that doesn't have a mirroring Android-side one.
> 
> adamp: Ping!

Mirroring change has been merged into Android.

[tdresser, 2016-04-05 12:30:04]

On 2016/04/04 23:08:45, adamp wrote:
> On 2016/04/04 12:05:28, tdresser wrote:
> > On 2016/03/25 23:17:38, aelias wrote:
> > > Adding adamp@ to check if Android-side is fine with doing the same change.
> > > 
> > > Adam, I understand Kingston already talked to you about this; for context,
> we
> > > cloned the Android gesture recognizer in C++ so that ChromeOS and Windows
> can
> > > have the exact same gesture feel.  We aim to make sure that always have
the
> > > exact same behavior as the latest version of Android, though, so I want to
> > avoid
> > > landing any change that doesn't have a mirroring Android-side one.
> > 
> > adamp: Ping!
> 
> Mirroring change has been merged into Android.

aelias@, PTAL.

[aelias_OOO_until_Jul13, 2016-04-05 18:07:39]

lgtm

[tdresser, 2016-04-05 18:10:25]

+sadrul@ for
ui/mojo/init/ui_init.cc
ui/android/java/res/values/dimens.xml

[tdresser, 2016-04-06 14:36:46]

tdresser@chromium.org changed reviewers:
+ sadrul@chromium.org

[tdresser, 2016-04-06 14:36:47]

+sadrul for real this time, for
ui/mojo/init/ui_init.cc
ui/android/java/res/values/dimens.xml

[sadrul, 2016-04-08 14:20:25]

stamp lgtm (sorry about the delay)

[tdresser, 2016-04-11 12:12:39]

The CQ bit was checked by tdresser@chromium.org

[commit-bot: I haz the power, 2016-04-11 12:12:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1778063002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1778063002/20001

[commit-bot: I haz the power, 2016-04-11 13:23:31]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-04-11 13:24:27]

Description was changed from

==========
Eliminate pinch drift by removing touch radius from span calculation

Previously, we included touch radius in our pinch span computation.
This caused some drift between the users fingers and the web contents
during pinch. Removing the touch radius fixes this problem.

BUG=590026
==========

to

==========
Eliminate pinch drift by removing touch radius from span calculation

Previously, we included touch radius in our pinch span computation.
This caused some drift between the users fingers and the web contents
during pinch. Removing the touch radius fixes this problem.

BUG=590026

Committed: https://crrev.com/b89f4c6eefa4f3a8d346c3f747c993845bb34cc7
Cr-Commit-Position: refs/heads/master@{#386377}
==========

[commit-bot: I haz the power, 2016-04-11 13:24:28]

Patchset 2 (id:??) landed as
https://crrev.com/b89f4c6eefa4f3a8d346c3f747c993845bb34cc7
Cr-Commit-Position: refs/heads/master@{#386377}