Change how the VR reticle distance is determined.

Change how the VR reticle distance is determined.

- Eliminate special-casing of UI elements on the native side.
- Use an invisible, hittable quad as the plane behind content.
- Only UI element hit points within a bounding sphere are allowed.
- Let the UI set the radius of the bounding sphere.

BUG=693154
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2698623007
Cr-Commit-Position: refs/heads/master@{#451802}
Committed: https://chromium.googlesource.com/chromium/src/+/e28e50ecc0bc4077c879abc2d06957de0918e75e

[cjgrant, 2017-02-16 22:43:05]

Description was changed from

==========
Change how the VR reticle distance is determined.

- Eliminate special-casing of UI elements on the native side.
- Use an invisible, hittable quad as the plane behind content.
- Only UI element hit points within a bounding sphere are allowed.
- Let the UI set the radius of the bounding sphere.

BUG=693154
==========

to

==========
Change how the VR reticle distance is determined.

- Eliminate special-casing of UI elements on the native side.
- Use an invisible, hittable quad as the plane behind content.
- Only UI element hit points within a bounding sphere are allowed.
- Let the UI set the radius of the bounding sphere.

BUG=693154
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[cjgrant, 2017-02-16 22:45:19]

cjgrant@chromium.org changed reviewers:
+ joshcarpenter@chromium.org, mthiesse@chromium.org, tiborg@chromium.org

[cjgrant, 2017-02-16 22:53:39]

https://codereview.chromium.org/2698623007/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/ui_scene.h (right):

https://codereview.chromium.org/2698623007/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/ui_scene.h:87: float background_distance_ =
10.0f;
This feels magic-number-ish, but to put a constant into an empty namespace, it
needs to be done in the .cc constructor (which is currently default).  Good
times.

https://codereview.chromium.org/2698623007/diff/1/chrome/browser/resources/vr...
File chrome/browser/resources/vr_shell/vr_shell_ui_scene.js (right):

https://codereview.chromium.org/2698623007/diff/1/chrome/browser/resources/vr...
chrome/browser/resources/vr_shell/vr_shell_ui_scene.js:134: * @param {{r:
number, b: number, g: number, a: number}} color
Note that this works, for future reference.  If the UI's color parameters were
passed in as {a, b, c, x}, this will fail closure.

[tiborg, 2017-02-21 15:25:00]

lgtm

[mthiesse, 2017-02-21 15:40:45]

lgtm

https://codereview.chromium.org/2698623007/diff/20001/chrome/browser/resource...
File chrome/browser/resources/vr_shell/vr_shell_ui.js (right):

https://codereview.chromium.org/2698623007/diff/20001/chrome/browser/resource...
chrome/browser/resources/vr_shell/vr_shell_ui.js:41: /** @const */
this.BACKGROUND_DISTANCE_MULTIPLIER = 2.0;
Multiplier seems strange, probably a constant offset would be better? (2x
distance sounds like it would be too far back too, but I haven't played with it)

[cjgrant, 2017-02-21 16:25:33]

https://codereview.chromium.org/2698623007/diff/20001/chrome/browser/resource...
File chrome/browser/resources/vr_shell/vr_shell_ui.js (right):

https://codereview.chromium.org/2698623007/diff/20001/chrome/browser/resource...
chrome/browser/resources/vr_shell/vr_shell_ui.js:41: /** @const */
this.BACKGROUND_DISTANCE_MULTIPLIER = 2.0;
On 2017/02/21 15:40:45, mthiesse wrote:
> Multiplier seems strange, probably a constant offset would be better? (2x
> distance sounds like it would be too far back too, but I haven't played with
it)

The multiplier approach is trying to make the distance scale with the quad (for
example, if we switch to a huge content quad 10 meters back, but use a constant
offset, our radius may then be 11 meters, which could cut through the content
quad itself).  I think we need to scale with content distance and size. 
However, maybe a better multiplier is root(2) times distance to the backing
plane, such that we transition from backing to sphere at 45 degrees from center,
rather than farther out.

Thoughts?

[mthiesse, 2017-02-21 16:29:17]

On 2017/02/21 16:25:33, cjgrant wrote:
>
https://codereview.chromium.org/2698623007/diff/20001/chrome/browser/resource...
> File chrome/browser/resources/vr_shell/vr_shell_ui.js (right):
> 
>
https://codereview.chromium.org/2698623007/diff/20001/chrome/browser/resource...
> chrome/browser/resources/vr_shell/vr_shell_ui.js:41: /** @const */
> this.BACKGROUND_DISTANCE_MULTIPLIER = 2.0;
> On 2017/02/21 15:40:45, mthiesse wrote:
> > Multiplier seems strange, probably a constant offset would be better? (2x
> > distance sounds like it would be too far back too, but I haven't played with
> it)
> 
> The multiplier approach is trying to make the distance scale with the quad
(for
> example, if we switch to a huge content quad 10 meters back, but use a
constant
> offset, our radius may then be 11 meters, which could cut through the content
> quad itself).  I think we need to scale with content distance and size. 
> However, maybe a better multiplier is root(2) times distance to the backing
> plane, such that we transition from backing to sphere at 45 degrees from
center,
> rather than farther out.
> 
> Thoughts?

sqrt(2) sgtm.

[cjgrant, 2017-02-21 16:45:07]

The CQ bit was checked by cjgrant@chromium.org

[cjgrant, 2017-02-21 16:45:08]

The patchset sent to the CQ was uploaded after l-g-t-m from
mthiesse@chromium.org, tiborg@chromium.org
Link to the patchset: https://codereview.chromium.org/2698623007/#ps40001
(title: "Change background distance multplier to 1.414; move Reload UI button
out of the floor.")

[commit-bot: I haz the power, 2017-02-21 16:46:03]

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-02-21 18:25:35]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1487695507699350,
"parent_rev": "32ab4eebda1a2d68acf74dc0b48b583464b3a2e7", "commit_rev":
"e28e50ecc0bc4077c879abc2d06957de0918e75e"}

[commit-bot: I haz the power, 2017-02-21 18:26:13]

Description was changed from

==========
Change how the VR reticle distance is determined.

- Eliminate special-casing of UI elements on the native side.
- Use an invisible, hittable quad as the plane behind content.
- Only UI element hit points within a bounding sphere are allowed.
- Let the UI set the radius of the bounding sphere.

BUG=693154
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Change how the VR reticle distance is determined.

- Eliminate special-casing of UI elements on the native side.
- Use an invisible, hittable quad as the plane behind content.
- Only UI element hit points within a bounding sphere are allowed.
- Let the UI set the radius of the bounding sphere.

BUG=693154
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2698623007
Cr-Commit-Position: refs/heads/master@{#451802}
Committed:
https://chromium.googlesource.com/chromium/src/+/e28e50ecc0bc4077c879abc2d069...
==========

[commit-bot: I haz the power, 2017-02-21 18:26:19]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/e28e50ecc0bc4077c879abc2d069...