Controller support for VrShell

chrome/browser/android/vr_shell/vr_controller_manager.cc

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/logging.h"
+#include "chrome/browser/android/vr_shell/vr_controller_manager.h"
+#include "content/public/browser/browser_thread.h"
+
+namespace vr_shell {
+
+VrControllerManager::VrControllerManager(gvr_context_* gvr_context) {}

[bshe, 2016/09/21 15:18:41]

is gvr_context neccessary? it doesn't look like it is used here.

[asimjour, 2016/09/22 14:48:36]

Done.

+
+VrControllerManager::~VrControllerManager() {}
+
+void VrControllerManager::OnResume() {
+  VLOG(1) << "Controller-Resume";

[mthiesse, 2016/09/21 17:27:47]

Is there a debug only logging path? Might make sense to use here.

[asimjour, 2016/09/22 14:48:36]

removed unnecessary logs

+  if (vr_controller_)
+    vr_controller_->OnResume();
+}
+
+void VrControllerManager::SetContentViewCore(
+    content::ContentViewCore* content_content_view_core_ptr,

[mthiesse, 2016/09/21 17:27:47]

People don't seem to like the content_content_ naming scheme. I'd suggest
content_cvc_ptr, and ui_cvc_ptr.

[asimjour, 2016/09/22 14:48:36]

Done.

+    content::ContentViewCore* ui_content_view_core_ptr) {
+  vr_content_ptr = content::VrContentViewCore::FromContentViewCore(
+      content_content_view_core_ptr);
+  vr_ui_ptr =
+      content::VrContentViewCore::FromContentViewCore(ui_content_view_core_ptr);
+}
+
+void VrControllerManager::OnPause() {
+  VLOG(1) << "Controller-Pause";
+  if (vr_controller_)
+    vr_controller_->OnPause();
+}
+
+void VrControllerManager::Initialize(gvr_context_* gvr_context) {
+  vr_controller_.reset(new VrController(gvr_context));
+  vr_controller_->Initialize(gvr_context);
+}
+
+VrGesture VrControllerManager::Update() {
+  if (!vr_controller_)
+    return VrGesture();
+  vr_controller_->UpdateState();
+  VrGesture gesture = vr_controller_->DetectGesture();
+  return gesture;
+}
+
+void VrControllerManager::ProcessUpdatedGesture(VrGesture gesture) {

[mthiesse, 2016/09/21 17:27:47]

Why not pass the CVC pointer in here, and not store it in this class?

Then we only need one ProcessUpdatedGesture function.

[asimjour, 2016/09/22 14:48:36]

If we pass the vr_cvc pointer vr_shell need to know about the vr_cvc, which
looks to be unnecessary. We can pass cvc pointer, but then the
ProcessUpdatedGesture would be slightly more complex with getting the
appropriate vr_cvc every time. 
We only have two different vr_cvc, and I don't think that in the near future
there would be any additional cvc, so having two functions looks to be fine.

+  content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,

[mthiesse, 2016/09/21 17:27:47]

You should probably either check that you're not already on the UI thread, or
move the PostTask to the callsite and require this method to be called from the
UI thread.

[asimjour, 2016/09/22 14:48:36]

Done.

+                                   base::Bind(&VrControllerManager::SendGesture,
+                                              this, gesture, vr_content_ptr));
+}
+
+void VrControllerManager::ProcessUpdatedUIGesture(VrGesture gesture) {
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::Bind(&VrControllerManager::SendGesture, this, gesture, vr_ui_ptr));
+}
+
+void VrControllerManager::SendGesture(
+    VrGesture gesture,
+    content::VrContentViewCore* content_view_core_ptr) {
+  int64_t event_time = gesture.start_time;
+  long event_time_milliseconds = (long)(event_time / 1000000);
+
+  if (gesture.type == kGestureTypeScroll) {
+    content_view_core_ptr->SendScrollEvent(
+        event_time_milliseconds, 0.0f, 0.0f, gesture.details.scroll.dx,
+        gesture.details.scroll.dy, gesture.details.scroll.state);
+  } else if (gesture.type == kGestureTypeButtonsChange) {
+    content_view_core_ptr->SendClickEvent(event_time_milliseconds,
+                                          gesture.details.buttons.x,
+                                          gesture.details.buttons.y);
+  } else if (gesture.type == kGestureTypeAngularMove) {
+    content_view_core_ptr->SendMouseMoveEvent(
+        event_time_milliseconds, gesture.details.move.x, gesture.details.move.y,
+        gesture.details.move.type);
+  }
+  return;
+}
+
+bool VrControllerManager::IsTouching() {
+  return vr_controller_->IsTouching();
+}
+
+float VrControllerManager::TouchPosX() {
+  return vr_controller_->TouchPosX();
+}
+
+float VrControllerManager::TouchPosY() {
+  return vr_controller_->TouchPosY();
+}
+
+const gvr_quatf VrControllerManager::Orientation() {

[mthiesse, 2016/09/21 17:27:47]

nit: gvr::Quatf

[asimjour, 2016/09/22 14:48:36]

Done.

+  gvr_quatf orientation = {
+      vr_controller_->Orientation().qx, vr_controller_->Orientation().qy,

[mthiesse, 2016/09/21 17:27:47]

This feels wrong. vr_controller_->Orientation() converts a gvr::Quatf to a
quatf, and this function converts it back to a gvr::Quatf. Why not skip the
conversion altogether?

[asimjour, 2016/09/22 14:48:36]

fixed

+      vr_controller_->Orientation().qz, vr_controller_->Orientation().qw};
+  return orientation;
+}
+
+bool VrControllerManager::IsTouchDown() {
+  return vr_controller_->IsTouchDown();
+}
+
+bool VrControllerManager::IsTouchUp() {
+  return vr_controller_->IsTouchUp();
+}
+
+bool VrControllerManager::ButtonDown(const int32_t button) {
+  return vr_controller_->ButtonDown(button);
+}
+
+bool VrControllerManager::ButtonUp(const int32_t button) {
+  return vr_controller_->ButtonUp(button);
+}
+
+bool VrControllerManager::IsConnected() {
+  return vr_controller_->IsConnected();
+}
+
+}  // namespace vr_shell