VR: Route hover and click events to UI elements.

VR: Route hover and click events to UI elements.

This change maps VR controller input to non-content UI elements. The
content element is treated separately, so that it can handle scrolling
and fling-type input (these are currently not useful to other elements).

BUG=715554
Review-Url: https://codereview.chromium.org/2840183003
Cr-Commit-Position: refs/heads/master@{#467673}
Committed: https://chromium.googlesource.com/chromium/src/+/730e3cee9100751b3fb33c40f58c68e246e483be

[cjgrant, 2017-04-26 14:36:15]

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

[cjgrant, 2017-04-26 14:36:16]

Folks, here's the initial input targeting for native UI elements.  I have a
button base class coming in a follow-on CL.

https://codereview.chromium.org/2840183003/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/ui_elements/ui_element.h (right):

https://codereview.chromium.org/2840183003/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/ui_elements/ui_element.h:108: virtual void
OnMouseEnter();
I used Mouse by convention, but these could be OnHoverEnter/OnHoverLeave, or
something else.  Thoughts?

[mthiesse, 2017-04-26 16:09:50]

https://codereview.chromium.org/2840183003/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/ui_elements/ui_element.h (right):

https://codereview.chromium.org/2840183003/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/ui_elements/ui_element.h:108: virtual void
OnMouseEnter();
On 2017/04/26 14:36:16, cjgrant wrote:
> I used Mouse by convention, but these could be OnHoverEnter/OnHoverLeave, or
> something else.  Thoughts?

Hover enter/leave sound better to me.

https://codereview.chromium.org/2840183003/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/vr_shell_gl.cc (right):

https://codereview.chromium.org/2840183003/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell_gl.cc:776: click_target_element_ =
target_element;
While we're here, can you do buttondown/up events on the UI elements as well?
This can replace the single onClick event and give us more flexibility.

[asimjour1, 2017-04-26 16:16:25]

https://codereview.chromium.org/2840183003/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/vr_shell_gl.cc (right):

https://codereview.chromium.org/2840183003/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell_gl.cc:776: click_target_element_ =
target_element;
On 2017/04/26 16:09:49, mthiesse wrote:
> While we're here, can you do buttondown/up events on the UI elements as well?
> This can replace the single onClick event and give us more flexibility.

It'll also force us to disable more UI elements, and deal with some UX
challenges (like double tap zooming, text selection handles, ...)
So I would say we shouldn't do it now.

[mthiesse, 2017-04-26 16:17:45]

https://codereview.chromium.org/2840183003/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/vr_shell_gl.cc (right):

https://codereview.chromium.org/2840183003/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell_gl.cc:776: click_target_element_ =
target_element;
On 2017/04/26 16:16:25, asimjour1 wrote:
> On 2017/04/26 16:09:49, mthiesse wrote:
> > While we're here, can you do buttondown/up events on the UI elements as
well?
> > This can replace the single onClick event and give us more flexibility.
> 
> It'll also force us to disable more UI elements, and deal with some UX
> challenges (like double tap zooming, text selection handles, ...)
> So I would say we shouldn't do it now.

This would only be for skia elements at the moment, not the content quad.

[cjgrant, 2017-04-26 18:19:42]

On 2017/04/26 16:17:45, mthiesse wrote:
>
https://codereview.chromium.org/2840183003/diff/1/chrome/browser/android/vr_s...
> File chrome/browser/android/vr_shell/vr_shell_gl.cc (right):
> 
>
https://codereview.chromium.org/2840183003/diff/1/chrome/browser/android/vr_s...
> chrome/browser/android/vr_shell/vr_shell_gl.cc:776: click_target_element_ =
> target_element;
> On 2017/04/26 16:16:25, asimjour1 wrote:
> > On 2017/04/26 16:09:49, mthiesse wrote:
> > > While we're here, can you do buttondown/up events on the UI elements as
> well?
> > > This can replace the single onClick event and give us more flexibility.
> > 
> > It'll also force us to disable more UI elements, and deal with some UX
> > challenges (like double tap zooming, text selection handles, ...)
> > So I would say we shouldn't do it now.
> 
> This would only be for skia elements at the moment, not the content quad.

This is why I didn't do it for skia elements.  I'm fine to, and if elements only
handle button-up, we have equivalent behaviour.

[cjgrant, 2017-04-26 21:16:00]

https://codereview.chromium.org/2840183003/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/vr_shell_gl.cc (right):

https://codereview.chromium.org/2840183003/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell_gl.cc:776: click_target_element_ =
target_element;
On 2017/04/26 16:17:45, mthiesse wrote:
> On 2017/04/26 16:16:25, asimjour1 wrote:
> > On 2017/04/26 16:09:49, mthiesse wrote:
> > > While we're here, can you do buttondown/up events on the UI elements as
> well?
> > > This can replace the single onClick event and give us more flexibility.
> > 
> > It'll also force us to disable more UI elements, and deal with some UX
> > challenges (like double tap zooming, text selection handles, ...)
> > So I would say we shouldn't do it now.
> 
> This would only be for skia elements at the moment, not the content quad.

Done as discussed offline.

[mthiesse, 2017-04-26 21:27:10]

lgtm

[cjgrant, 2017-04-27 14:10:33]

The CQ bit was checked by cjgrant@chromium.org

[commit-bot: I haz the power, 2017-04-27 14:10:58]

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-04-27 14:57:53]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493302233414320,
"parent_rev": "591cbec71ef07912c8c7d2b0acc1efb25196d7f1", "commit_rev":
"730e3cee9100751b3fb33c40f58c68e246e483be"}

[commit-bot: I haz the power, 2017-04-27 14:58:11]

Description was changed from

==========
VR: Route hover and click events to UI elements.

This change maps VR controller input to non-content UI elements. The
content element is treated separately, so that it can handle scrolling
and fling-type input (these are currently not useful to other elements).

BUG=715554
==========

to

==========
VR: Route hover and click events to UI elements.

This change maps VR controller input to non-content UI elements. The
content element is treated separately, so that it can handle scrolling
and fling-type input (these are currently not useful to other elements).

BUG=715554

Review-Url: https://codereview.chromium.org/2840183003
Cr-Commit-Position: refs/heads/master@{#467673}
Committed:
https://chromium.googlesource.com/chromium/src/+/730e3cee9100751b3fb33c40f58c...
==========

[commit-bot: I haz the power, 2017-04-27 14:58:12]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/730e3cee9100751b3fb33c40f58c...