Incorporate 15 degree laser beam angle on pointer.

Incorporate 15 degree laser beam angle on pointer.

- Point the beam 15 degrees down from where the controller is pointing, for wrist comfort.
- Fix the beam to point directly at the reticle.

BUG=642058
Committed: https://crrev.com/07d8a34ed9c1f4292888b50a0f31797ccd0d39d6
Cr-Commit-Position: refs/heads/master@{#423539}

[cjgrant, 2016-10-05 21:22:02]

cjgrant@chromium.org changed reviewers:
+ bshe@chromium.org, mthiesse@chromium.org

[mthiesse, 2016-10-05 23:29:11]

https://codereview.chromium.org/2392333003/diff/20001/chrome/browser/android/...
File chrome/browser/android/vr_shell/vr_shell.cc (right):

https://codereview.chromium.org/2392333003/diff/20001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_shell.cc:52: // Angle the beam down from the
controller axis, for wrist comfort.
nit: s/Angle/Angle (radians)/ ?

https://codereview.chromium.org/2392333003/diff/20001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_shell.cc:53: static constexpr float
kLaserAngle = 0.26f;
Daydream calls this kErgoAngleOffset, we should probably follow suit for
clarity?

https://codereview.chromium.org/2392333003/diff/20001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_shell.cc:239: gvr::Vec3f beam;
'beam' here is slightly confusing. Maybe ergo_neutral_pose?

https://codereview.chromium.org/2392333003/diff/20001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_shell.cc:534: const auto faces = 4;
Prefer types for simple types? 'int' is even shorter than 'auto' :P

https://codereview.chromium.org/2392333003/diff/20001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_shell.cc:538: auto rot =
QuatFromAxisAngle({0.0f, 0.0f, 1.0f}, angle);
I would prefer types here for clarity, but I'd listen to arguments to the
contrary.

[cjgrant, 2016-10-06 15:05:15]

Description was changed from

==========
Incorporate 15 degree laser beam angle on pointer.

BUG=41413
==========

to

==========
Incorporate 15 degree laser beam angle on pointer.
    
- Point the beam 15 degrees down from where the controller is pointing, for
wrist comfort.
- Fix the beam to point directly at the reticle.
    
BUG=642058
==========

[cjgrant, 2016-10-06 15:07:51]

https://codereview.chromium.org/2392333003/diff/20001/chrome/browser/android/...
File chrome/browser/android/vr_shell/vr_shell.cc (right):

https://codereview.chromium.org/2392333003/diff/20001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_shell.cc:52: // Angle the beam down from the
controller axis, for wrist comfort.
On 2016/10/05 23:29:10, mthiesse wrote:
> nit: s/Angle/Angle (radians)/ ?

Done.

https://codereview.chromium.org/2392333003/diff/20001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_shell.cc:53: static constexpr float
kLaserAngle = 0.26f;
On 2016/10/05 23:29:10, mthiesse wrote:
> Daydream calls this kErgoAngleOffset, we should probably follow suit for
> clarity?

Done.

https://codereview.chromium.org/2392333003/diff/20001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_shell.cc:239: gvr::Vec3f beam;
On 2016/10/05 23:29:10, mthiesse wrote:
> 'beam' here is slightly confusing. Maybe ergo_neutral_pose?

Done.

https://codereview.chromium.org/2392333003/diff/20001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_shell.cc:534: const auto faces = 4;
On 2016/10/05 23:29:10, mthiesse wrote:
> Prefer types for simple types? 'int' is even shorter than 'auto' :P

Done.

https://codereview.chromium.org/2392333003/diff/20001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_shell.cc:538: auto rot =
QuatFromAxisAngle({0.0f, 0.0f, 1.0f}, angle);
On 2016/10/05 23:29:10, mthiesse wrote:
> I would prefer types here for clarity, but I'd listen to arguments to the
> contrary.

Done.  No argument.  :)

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

https://codereview.chromium.org/2392333003/diff/40001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_shell.cc:530: const gvr::Mat4f
beam_direction_mat =
I was recomputing the matrix in the loop (needless).

[bshe, 2016-10-06 15:10:28]

lgtm

[cjgrant, 2016-10-06 15:16:03]

The CQ bit was checked by cjgrant@chromium.org

[commit-bot: I haz the power, 2016-10-06 15:16:13]

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, 2016-10-06 15:24:30]

Description was changed from

==========
Incorporate 15 degree laser beam angle on pointer.
    
- Point the beam 15 degrees down from where the controller is pointing, for
wrist comfort.
- Fix the beam to point directly at the reticle.
    
BUG=642058
==========

to

==========
Incorporate 15 degree laser beam angle on pointer.
    
- Point the beam 15 degrees down from where the controller is pointing, for
wrist comfort.
- Fix the beam to point directly at the reticle.
    
BUG=642058
==========

[commit-bot: I haz the power, 2016-10-06 15:24:31]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-10-06 15:27:01]

Description was changed from

==========
Incorporate 15 degree laser beam angle on pointer.
    
- Point the beam 15 degrees down from where the controller is pointing, for
wrist comfort.
- Fix the beam to point directly at the reticle.
    
BUG=642058
==========

to

==========
Incorporate 15 degree laser beam angle on pointer.

- Point the beam 15 degrees down from where the controller is pointing, for
wrist comfort.
- Fix the beam to point directly at the reticle.

BUG=642058

Committed: https://crrev.com/07d8a34ed9c1f4292888b50a0f31797ccd0d39d6
Cr-Commit-Position: refs/heads/master@{#423539}
==========

[commit-bot: I haz the power, 2016-10-06 15:27:02]

Patchset 3 (id:??) landed as
https://crrev.com/07d8a34ed9c1f4292888b50a0f31797ccd0d39d6
Cr-Commit-Position: refs/heads/master@{#423539}