VR: Improve scrolling

VR: Improve scrolling

Scrolling is improved by
1- update touchinfo when it has been the same for the last 2 cycles.
2- using the last available scroll velocity for fling.
3- update kSlopHorizontal with new results from user study

BUG=679408
Review-Url: https://codereview.chromium.org/2591253002
Cr-Commit-Position: refs/heads/master@{#446708}
Committed: https://chromium.googlesource.com/chromium/src/+/2214f886f76efe98d1c80eba3c6f326b18967ba3

[asimjour1, 2017-01-05 20:33:37]

Patchset #2 (id:20001) has been deleted

[asimjour1, 2017-01-05 20:40:56]

Description was changed from

==========
VR: Improve scrolling

Scrolling is improved by 1- update touchinfo when it has been the
same for the last 2 cycles. 2- using the last available
scroll velocity for fling.

BUG=
==========

to

==========
VR: Improve scrolling

Scrolling is improved by 1- update touchinfo when it has been the
same for the last 2 cycles. 2- using the last available
scroll velocity for fling.

BUG=
==========

[asimjour1, 2017-01-05 20:40:56]

asimjour@chromium.org changed reviewers:
+ bshe@chromium.org

[asimjour1, 2017-01-09 15:14:32]

On 2017/01/05 20:40:56, asimjour1 wrote:
> mailto:asimjour@chromium.org changed reviewers:
> + mailto:bshe@chromium.org

PTAL

[asimjour1, 2017-01-09 18:21:04]

Description was changed from

==========
VR: Improve scrolling

Scrolling is improved by 1- update touchinfo when it has been the
same for the last 2 cycles. 2- using the last available
scroll velocity for fling.

BUG=
==========

to

==========
VR: Improve scrolling

Scrolling is improved by
1- update touchinfo when it has been the same for the last 2 cycles.
2- using the last available scroll velocity for fling.
3- update kSlopHorizontal with new results from user study

BUG=679408
==========

[asimjour1, 2017-01-09 18:21:24]

On 2017/01/09 15:14:32, asimjour1 wrote:
> On 2017/01/05 20:40:56, asimjour1 wrote:
> > mailto:asimjour@chromium.org changed reviewers:
> > + mailto:bshe@chromium.org
> 
> PTAL

PTAL

[bshe, 2017-01-09 19:38:55]

https://codereview.chromium.org/2591253002/diff/40001/chrome/browser/android/...
File chrome/browser/android/vr_shell/vr_controller.cc (right):

https://codereview.chromium.org/2591253002/diff/40001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_controller.cc:165: touch_point_->position.y
== cur_touch_point_->position.y &&
Vector::Equal(touch_point_->position, cur_touch_point_->position)?
Also, should this compared with pre_touch_point_ since cur_touch_point_ hasn't
updated for this frame IIUC?

https://codereview.chromium.org/2591253002/diff/40001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_controller.cc:305: // Update current touch
point.
nit: remove the above comment

https://codereview.chromium.org/2591253002/diff/40001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_controller.cc:354:
prev_touch_point_->position);
IIUC, should prev_touch_point save previous extropolated value to match the old
behavior? Or it doesnt' matter?

https://codereview.chromium.org/2591253002/diff/40001/chrome/browser/android/...
File chrome/browser/android/vr_shell/vr_controller.h (right):

https://codereview.chromium.org/2591253002/diff/40001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_controller.h:138: std::unique_ptr<TouchPoint>
touch_point_;
Is it possible to use cur_touch_point_ directly to eliminate the need for
touch_point_?

[asimjour1, 2017-01-11 21:59:45]

https://codereview.chromium.org/2591253002/diff/40001/chrome/browser/android/...
File chrome/browser/android/vr_shell/vr_controller.cc (right):

https://codereview.chromium.org/2591253002/diff/40001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_controller.cc:165: touch_point_->position.y
== cur_touch_point_->position.y &&
On 2017/01/09 19:38:54, bshe wrote:
> Vector::Equal(touch_point_->position, cur_touch_point_->position)?
> Also, should this compared with pre_touch_point_ since cur_touch_point_ hasn't
> updated for this frame IIUC?

Used Vector::Equal

touch_point_ is the point that VrController just received from the controller.
The extrapolation and fixing the touch_point_ happens before cur_touch_point_
and pre_touch_point_ get new values for this cycle. As a result, the comparing
the cur_touch_point_ and touch_point_ is correct.

https://codereview.chromium.org/2591253002/diff/40001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_controller.cc:305: // Update current touch
point.
On 2017/01/09 19:38:54, bshe wrote:
> nit: remove the above comment

Done.

https://codereview.chromium.org/2591253002/diff/40001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_controller.cc:354:
prev_touch_point_->position);
On 2017/01/09 19:38:54, bshe wrote:
> IIUC, should prev_touch_point save previous extrapolated value to match the
old
> behavior? Or it doesnt' matter?

It shouldn't be the previous extrapolated point. if we keep the extrapolated
points instead of the actual reported points then it won't be possible to detect
a steady finger. It means there will be a small fling at the end of the moves.
we can measure the displacement from the extrapolated points if we add two more
variables to also keep track of the actual points.

https://codereview.chromium.org/2591253002/diff/40001/chrome/browser/android/...
File chrome/browser/android/vr_shell/vr_controller.h (right):

https://codereview.chromium.org/2591253002/diff/40001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_controller.h:138: std::unique_ptr<TouchPoint>
touch_point_;
On 2017/01/09 19:38:54, bshe wrote:
> Is it possible to use cur_touch_point_ directly to eliminate the need for
> touch_point_?

It is possible. But more changes would be needed. 
prev_touch_point_ is updated only when the a new touch_point is reported or
extrapolation happens. Also, replacing the prev_touch_point_ with
cur_touch_point_ should be moved to the end of each controller update.

I'll make a bug and address this in another CL.

[asimjour1, 2017-01-25 18:36:04]

PTAL

[bshe, 2017-01-26 17:58:08]

On 2017/01/25 18:36:04, asimjour1 wrote:
> PTAL

lgtm sorry for the delay

[asimjour1, 2017-01-27 17:31:14]

The CQ bit was checked by asimjour@chromium.org

[commit-bot: I haz the power, 2017-01-27 17:31:31]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-27 18:09:00]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1485538274107180,
"parent_rev": "961dc47e8f5b088debbab559fe81b85f32e9f9e4", "commit_rev":
"2214f886f76efe98d1c80eba3c6f326b18967ba3"}

[commit-bot: I haz the power, 2017-01-27 18:10:09]

Description was changed from

==========
VR: Improve scrolling

Scrolling is improved by
1- update touchinfo when it has been the same for the last 2 cycles.
2- using the last available scroll velocity for fling.
3- update kSlopHorizontal with new results from user study

BUG=679408
==========

to

==========
VR: Improve scrolling

Scrolling is improved by
1- update touchinfo when it has been the same for the last 2 cycles.
2- using the last available scroll velocity for fling.
3- update kSlopHorizontal with new results from user study

BUG=679408

Review-Url: https://codereview.chromium.org/2591253002
Cr-Commit-Position: refs/heads/master@{#446708}
Committed:
https://chromium.googlesource.com/chromium/src/+/2214f886f76efe98d1c80eba3c6f...
==========

[commit-bot: I haz the power, 2017-01-27 18:10:10]

Committed patchset #5 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/2214f886f76efe98d1c80eba3c6f...