VR: Improve scrolling

chrome/browser/android/vr_shell/vr_controller.cc

Index: chrome/browser/android/vr_shell/vr_controller.cc
diff --git a/chrome/browser/android/vr_shell/vr_controller.cc b/chrome/browser/android/vr_shell/vr_controller.cc
index 0cf8b166031931f0a267459c63539e48908735c7..41b4b992748a7a0dbae84f20e26c85785bd27796 100644
--- a/chrome/browser/android/vr_shell/vr_controller.cc
+++ b/chrome/browser/android/vr_shell/vr_controller.cc
@@ -25,7 +25,7 @@ constexpr float kDisplacementScaleFactor = 300.0f;
 constexpr float kSlopVertical = 0.165f;
 
 // Horizontal distance from the border to the center of slop.
-constexpr float kSlopHorizontal = 0.125f;
+constexpr float kSlopHorizontal = 0.15f;
 
 // Minimum distance needed in at least one direction to call two vectors
 // not equal. Also, minimum time distance needed to call two timestamps
@@ -36,6 +36,8 @@ constexpr float kCutoffHz = 10.0f;
 constexpr float kRC = static_cast<float>(1.0 / (2.0 * M_PI * kCutoffHz));
 constexpr float kNanoSecondsPerSecond = 1.0e9f;
 
+constexpr int kMaxNumOfExtrapolations = 2;
+
 class Vector {
  public:
   static inline void ClampTouchpadPosition(gvr::Vec2f* position) {
@@ -146,31 +148,43 @@ void VrController::UpdateState() {
 
 void VrController::UpdateTouchInfo() {
   CHECK(touch_info_ != nullptr) << "touch_info_ not initialized properly.";
-  gvr::Vec2f position;
-  position.x = TouchPosX();
-  position.y = TouchPosY();
+  touch_point_->position.x = TouchPosX();
+  touch_point_->position.y = TouchPosY();
   touch_info_->touch_up = TouchUpHappened();
   touch_info_->touch_down = TouchDownHappened();
   touch_info_->is_touching = IsTouching();
-  touch_info_->touch_point.position = position;
+  touch_info_->touch_point.position = touch_point_->position;
   Vector::ClampTouchpadPosition(&touch_info_->touch_point.position);
   touch_info_->touch_point.timestamp =
       gvr::GvrApi::GetTimePointNow().monotonic_system_time_nanos;
-  if (controller_state_->GetLastTouchTimestamp() == last_touch_timestamp_) {
+  touch_point_->timestamp = touch_info_->touch_point.timestamp;
+
+  if (IsTouching() && state_ == SCROLLING &&
+      (controller_state_->GetLastTouchTimestamp() == last_touch_timestamp_ ||
+       (touch_point_->position.x == cur_touch_point_->position.x &&
+        touch_point_->position.y == cur_touch_point_->position.y &&

[bshe, 2017/01/09 19:38:54]

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?

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

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.

+        extrapolated_touch_ < kMaxNumOfExtrapolations))) {
+    extrapolated_touch_++;
     // Fill the touch_info
     float duration =
         (gvr::GvrApi::GetTimePointNow().monotonic_system_time_nanos -
          last_timestamp_nanos_) /
         kNanoSecondsPerSecond;
 
-    position.x += overall_velocity_.x * duration;
-    position.y += overall_velocity_.y * duration;
-    touch_info_->touch_point.position.x = position.x;
-    touch_info_->touch_point.position.y = position.y;
+    touch_info_->touch_point.position.x =
+        touch_point_->position.x + overall_velocity_.x * duration;
+    touch_info_->touch_point.position.y =
+        touch_point_->position.y + overall_velocity_.y * duration;
+  } else {
+    if (extrapolated_touch_ == kMaxNumOfExtrapolations) {
+      Vector::SetZero(&overall_velocity_);
+    }
+    extrapolated_touch_ = 0;
   }
   last_touch_timestamp_ = controller_state_->GetLastTouchTimestamp();
   last_timestamp_nanos_ =
       gvr::GvrApi::GetTimePointNow().monotonic_system_time_nanos;
+  touch_position_changed_ = UpdateCurrentTouchpoint();
 }
 
 void VrController::Initialize(gvr_context* gvr_context) {
@@ -188,6 +202,7 @@ void VrController::Initialize(gvr_context* gvr_context) {
 std::vector<std::unique_ptr<WebGestureEvent>> VrController::DetectGestures() {
   std::vector<std::unique_ptr<WebGestureEvent>> gesture_list;
   std::unique_ptr<WebGestureEvent> gesture(new WebGestureEvent());
+  touch_point_.reset(new TouchPoint);
 
   if (controller_state_->GetConnectionState() != gvr::kControllerConnected) {
     gesture_list.push_back(std::move(gesture));
@@ -206,17 +221,18 @@ std::vector<std::unique_ptr<WebGestureEvent>> VrController::DetectGestures() {
   gesture_list.push_back(std::move(gesture));
 
   if (gesture_list.back()->type == WebInputEvent::GestureScrollEnd) {
-    if (!ButtonDownHappened(gvr::kControllerButtonClick)) {
+    if (!ButtonDownHappened(gvr::kControllerButtonClick) &&
+        (last_velocity_.x != 0.0 || last_velocity_.y != 0.0)) {
       std::unique_ptr<WebGestureEvent> fling(new WebGestureEvent());
       fling->timeStampSeconds = gesture_list.back()->timeStampSeconds;
       fling->sourceDevice = blink::WebGestureDeviceTouchpad;
       fling->type = WebInputEvent::GestureFlingStart;
       if (IsHorizontalGesture()) {
         fling->data.flingStart.velocityX =
-            overall_velocity_.x * kDisplacementScaleFactor;
+            last_velocity_.x * kDisplacementScaleFactor;
       } else {
         fling->data.flingStart.velocityY =
-            overall_velocity_.y * kDisplacementScaleFactor;
+            last_velocity_.y * kDisplacementScaleFactor;
       }
       gesture_list.push_back(std::move(fling));
     }
@@ -272,7 +288,7 @@ void VrController::HandleDetectingState(WebGestureEvent* gesture) {
 
   // Touch position is changed, the touch point moves outside of slop,
   // and the Controller's button is not down.
-  if (UpdateCurrentTouchpoint() && touch_info_->is_touching &&
+  if (touch_position_changed_ && touch_info_->is_touching &&
       !InSlop(touch_info_->touch_point.position) &&
       !ButtonDownHappened(gvr::kControllerButtonClick)) {
     state_ = SCROLLING;
@@ -287,13 +303,12 @@ void VrController::HandleDetectingState(WebGestureEvent* gesture) {
 
 void VrController::HandleScrollingState(WebGestureEvent* gesture) {
   // Update current touch point.

[bshe, 2017/01/09 19:38:54]

nit: remove the above comment

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

Done.

-  bool touch_position_changed = UpdateCurrentTouchpoint();
   if (touch_info_->touch_up || !(touch_info_->is_touching) ||
       ButtonDownHappened(gvr::kControllerButtonClick)) {
     // Gesture ends.
     gesture->type = WebInputEvent::GestureScrollEnd;
     UpdateGesture(gesture);
-  } else if (touch_position_changed) {
+  } else if (touch_position_changed_) {
     // User continues scrolling and there is a change in touch position.
     gesture->type = WebInputEvent::GestureScrollUpdate;
     UpdateGesture(gesture);
@@ -304,11 +319,12 @@ void VrController::HandleScrollingState(WebGestureEvent* gesture) {
       gesture->data.scrollUpdate.deltaY =
           displacement_.y * kDisplacementScaleFactor;
     }
+    last_velocity_ = overall_velocity_;
   }
 }
 
 bool VrController::IsHorizontalGesture() {
-  return std::abs(overall_velocity_.x) > std::abs(overall_velocity_.y);
+  return std::abs(last_velocity_.x) > std::abs(last_velocity_.y);
 }
 
 bool VrController::InSlop(const gvr::Vec2f touch_position) {
@@ -328,13 +344,14 @@ void VrController::Reset() {
   init_touch_point_.reset(new TouchPoint);
   touch_info_.reset(new TouchInfo);
   Vector::SetZero(&overall_velocity_);
+  Vector::SetZero(&last_velocity_);
 }
 
 void VrController::UpdateGesture(WebGestureEvent* gesture) {
   if (!gesture)
     LOG(ERROR) << "The gesture pointer is not initiated properly.";
-  displacement_ =
-      Vector::Subtract(cur_touch_point_->position, prev_touch_point_->position);
+  displacement_ = Vector::Subtract(touch_info_->touch_point.position,
+                                   prev_touch_point_->position);

[bshe, 2017/01/09 19:38:54]

IIUC, should prev_touch_point save previous extropolated value to match the old
behavior? Or it doesnt' matter?

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

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.

 }
 
 bool VrController::UpdateCurrentTouchpoint() {
@@ -343,7 +360,7 @@ bool VrController::UpdateCurrentTouchpoint() {
     if (!Vector::Equal(cur_touch_point_->position,
                        touch_info_->touch_point.position)) {
       prev_touch_point_.swap(cur_touch_point_);
-      *cur_touch_point_ = touch_info_->touch_point;
+      cur_touch_point_ = std::move(touch_point_);
       UpdateOverallVelocity();
       return true;
     }
@@ -353,15 +370,15 @@ bool VrController::UpdateCurrentTouchpoint() {
 
 void VrController::UpdateOverallVelocity() {
   float duration =
-      (cur_touch_point_->timestamp - prev_touch_point_->timestamp) /
+      (touch_info_->touch_point.timestamp - prev_touch_point_->timestamp) /
       kNanoSecondsPerSecond;
 
   // If the timestamp does not change, do not update velocity.
   if (duration < kDelta)
     return;
 
-  gvr::Vec2f displacement =
-      Vector::Subtract(cur_touch_point_->position, prev_touch_point_->position);
+  gvr::Vec2f displacement = Vector::Subtract(touch_info_->touch_point.position,
+                                             prev_touch_point_->position);
 
   gvr::Vec2f velocity = Vector::ScalarMult(displacement, 1 / duration);