Axismove for touch and vive #2513

machenmusik · 2017-03-22T03:23:41Z

Per discussion on #2423, this PR separates the axismove changes and omits the emulated property.

This PR extends #2413 / #2415 to oculus-touch-controls as thumbstickmoved, and also addresses spurious axismove events discussed in #2423.

machenmusik · 2017-03-22T18:11:49Z

@cmvanb - we decided to close #2423 in favor of this PR, which should have the same benefits

dmarcos · 2017-03-23T01:58:56Z

docs/components/oculus-touch-controls.md

 | Aup                  | A button released.             |
 | Atouchstart          | A button touched.              |
 | Atouchend            | A button no longer touched.    |
+| Achanged             | A button changed.              |


can we make all the event names lower case?

the ones that correspond to buttons A, B, X, and Y are uppercase for clarity because that is the way the button labels are always used, IMO don't think we should lowercase them? worried about namespace collisions too. Note the existing support is the same way...

done and also propagated into #2446

dmarcos · 2017-03-23T02:52:48Z

Our event names don't have uppercase. Can we keep them lower cased?

dmarcos · 2017-03-23T02:53:56Z

we could do abuttonchanged

machenmusik · 2017-03-23T03:20:06Z

buttonchanged is a slightly different but used event; personally not in favor of abuttonup, abuttonchanged, bbuttonchanged, etc. due to being too close to the separately used buttonup, buttonchanged, etc. -- but it is of course technically possible by changing mappings here and also in #2446.

machenmusik · 2017-03-24T15:42:31Z

@dmarcos @ngokevin any more thoughts on this one? hoping to get #2446 in as well, but this one first.

interested in outside perspectives on backward compatibility (whether the event names change is problematic for existing usage) -- I think right now most things use trigger / thumbstick / grip which aren't changing.

dmarcos · 2017-03-24T18:35:53Z

src/components/oculus-touch-controls.js

    'right': {
-      axis0: 'thumbstick',
-      axis1: 'thumbstick',
+      axes: {'thumbstick': [0, 1]},


I prefer to have axis + index instead of an array for consistency with buttons

makes processing harder e.g. you can't just do buttons.forEach ... i would argue we should change buttons to array for consistency instead.

so like axes, buttons could be a keyed map of either singletons or arrays (if aliased)
buttons: { thumbstick: 0, ... }
or if you prefer, one of:
buttons: { 0: 'thumbstick', ...}
or
buttons: ['thumbstick', ...]

really this:
var buttonName = this.mapping[this.data.hand]['button' + id];
should look like
var buttonName = this.mapping[this.data.hand].buttons[id];

more readable IMO

dmarcos · 2017-03-26T01:32:44Z

src/components/oculus-touch-controls.js

+      // touch events aren't happening (touched is stuck true);
+      // synthesize touch events from very low analog values.
+      analogValue = evt.detail.state.value;
+      isPreviousValueEmulatedTouch = this.isEmulatedTouchEvent(this.previousButtonValues[button]);


Touch is going to be supported soon in Firefox. I would prefer to wait for the implementation to land rather hand handling the quirks that we are going to remove soon.

Touch is supported in Nightly, see #2446

Is this touch emulation a good idea from the UX perspective? Can we move it to a separate PR?

that code is not new. it's already in. (if you look more carefully at the changes, the prior early-return is changed so the buttonchanged event fires properly, and some comments are capitalized.)

whether it is still needed depends in part on the Nightly button/axis mapping, which we should probably discuss in #2446

update: the 2017-03-22 build of Nightly makes Touch mapping compatible with Chromium, so #2446 is no longer required. Also, Nightly provides analog value for all buttons, so we do in fact need this emulated touch code (e.g. to make hand-controls work correctly) since Nightly does not otherwise provide capacitive touch events.

I asked our Firefox devs what's the ETA for the touch events. This will be coming soon so I would try to get rid of the hack and wait for Firefox to catch up.

according to @ngokevin there are cases where analog values have very small nonzero values, which may still need functionality similar/identical to this...

We filed bugs about making the threshold configurable in Firefox to address the small values.

I think we can get rid of all the emulated touch logic since Firefox is already supporting touch events.

On oculus touch controllers I believe the touch flag should be independent of the button value since they have a capacitive surface.

dmarcos · 2017-03-27T15:33:11Z

src/components/oculus-touch-controls.js

    var data = this.data;
-    var isPresent = this.isControllerPresent(this.el.sceneEl, GAMEPAD_ID_PREFIX, { hand: data.hand });
+    var isPresent = false;
+    var whichControllers;


You can do in one line var whichControllers = ...

dmarcos · 2017-04-11T23:05:21Z

I'm a bit lost with all these PRs. What does this one includes?

machenmusik · 2017-04-12T04:56:39Z

axismove events similar to #2413 / #2415 for oculus touch (prior commits were vive specific, which frankly we should really avoid where possible in future...)

machenmusik · 2017-04-12T04:57:19Z

This PR extends #2413 / #2415 to oculus-touch-controls as thumbstickmoved, and also addresses spurious axismove events discussed in #2423.

dmarcos · 2017-04-12T18:18:01Z

src/components/daydream-controls.js

    this.addControllersUpdateListener();
-    this.addEventListeners();
+    window.addEventListener('gamepadconnected', this.onGamepadConnectionEvent, false);
+    window.addEventListener('gamepaddisconnected', this.onGamepadConnectionEvent, false);


We can do window.addEventListener('gamepaddisconnected', this.checkIfControllerPresent, false);

see below comment on the everGotGamepadEvent flag

dmarcos · 2017-04-12T18:18:30Z

src/components/daydream-controls.js

-    this.onGamepadConnected = bind(this.onGamepadConnected, this);
-    this.onGamepadDisconnected = bind(this.onGamepadDisconnected, this);
+    this.onAxisMoved = bind(this.onAxisMoved, this);
+    this.onGamepadConnectionEvent = bind(this.onGamepadConnectionEvent, this);


We can get rid of this since we can do window.addEventListener('gamepaddisconnected', this.checkIfControllerPresent, false);

see below comment on the everGotGamepadEvent flag

dmarcos · 2017-04-12T18:18:43Z

src/components/daydream-controls.js

-    // apparent issue with FF Nightly only sending one event and seeing one controller;
-    // this.everGotGamepadEvent = true;
-    // this.removeControllersUpdateListener();
+  onGamepadConnectionEvent: function (evt) {


We can get rid of this method

see below comment on the everGotGamepadEvent flag

dmarcos · 2017-04-12T18:20:27Z

src/components/daydream-controls.js

-    // this.everGotGamepadEvent = true;
-    // this.removeControllersUpdateListener();
+  onGamepadConnectionEvent: function (evt) {
+    this.everGotGamepadEvent = true;


This flag is never checked

you can see why in the comments, but yeah we can remove them for now

actually it is checked in onControllersUpdate (which last I recall is actually fired every tick). so the idea is that if you got gamepad connect/disconnect events, you don't always need to be checking to see when it shows up

We don't have to set the flag to false on gamepaddisconnected?

no because we expect gamepadconnected next time since we're getting events, right? so we don't need to poll every tick

dmarcos · 2017-04-12T18:32:25Z

src/components/oculus-touch-controls.js

-    // apparent issue with FF Nightly only sending one event and seeing one controller;
-    // this.everGotGamepadEvent = true;
-    // this.removeControllersUpdateListener();
+  onGamepadConnectionEvent: function (evt) {


We can get rid of this method and do window.addEventListener('gamepaddisconnected', this.checkIfControllerPresent, false);

I think the question is whether we should restore the check in onControllersUpdate (don't look for touch controllers every tick the controllers list is updated), which would probably be more efficient

I have added the check in onControllersUpdate to match the other controller types.

machenmusik · 2017-04-12T19:34:23Z

summing up recent comments - in daydream-controls, everGotGamepadEvent is in fact used in onControllersUpdate, which is more efficient. oculus-touch-controls was not and so was checking for controller presence every tick, so changed it to stop doing that if everGotGamepadEvent (like the other controllers).

add axes mapping, axismove handler and emulated property, as well as tests add emulated property to docs accommodate temporary Nightly issue where Touch is supported but has lowercased handedness in id string Pass along changed event with button state, using the buttom mapping for convenience. pass along axis changed flags from tracked-controls, and check them in oculus-touch-controls and vive-controls to avoid spurious axis moved events add update comment per discussion on PR fix typo for compatibility, if no changed array for axismove event, treat as changed map to forEach, per discussion on PR remove emulated property per discussion change button names so events are lowercase

…rame into axismove-for-touch-and-vive

machenmusik · 2017-04-14T19:43:11Z

Since #2545 was merged first, I've incorporated the salient aspects of #2588 here now per discussion.

dmarcos · 2017-04-16T16:52:28Z

Thanks!

disable everGotGamepadEvent usage, seeing issues with latest Nightly make sure vive-controls doesn't match tracker bring in line with latest aframevr#2513 work around Nightly bug to get both controllers and trackers working at the same time bring vive-tracker in line with aframevr#2513 fix tests per separate discussion, use controllers list from system not getGamepadsByPrefix bring in line with aframevr#2505 make sure that queryObject values are used even if falsy e.g. index 0 tracked-controls needs to know about hand to filter properly and find vive trackers, given current Nightly bug. since empty string is always changed to default value (even when default is undefined), need to use 'none' to indicate empty-string hand re-clone tracker tests from controls use newly committed vive-tracker model add rotation property, which applies if no rotationOffset and allows full 3DOF rotation correct pose orientation for tracker to be what is generally expected of controllers fix default vive-tracker rotation correction, per discussion updateControllerModel for all tracked controls components; use fixed vive tracker model until published to CDN use new tracker model from CDN add rotation parameter apply rotationOffset the old way, to avoid precision issues add rotation offset test, with EPSILON change sentinel hand value to avoid node test failure zero pivot point update tracked-controls hand docs per discussion on PR fix duplicate play/pause presumably from botched merge

dmarcos reviewed Mar 23, 2017

View reviewed changes

dmarcos reviewed Mar 24, 2017

View reviewed changes

machenmusik force-pushed the axismove-for-touch-and-vive branch from e988b4f to f0b3309 Compare March 24, 2017 20:26

dmarcos reviewed Mar 26, 2017

View reviewed changes

machenmusik mentioned this pull request Mar 27, 2017

Nightly oculus touch #2446

Closed

dmarcos reviewed Mar 27, 2017

View reviewed changes

machenmusik mentioned this pull request Mar 28, 2017

hand-controls support for Nightly with Oculus Touch #2526

Closed

ngokevin added this to the 0.6.0 milestone Mar 30, 2017

ngokevin added the needs review label Apr 6, 2017

machenmusik force-pushed the axismove-for-touch-and-vive branch 3 times, most recently from 19b7e9b to b2f7242 Compare April 7, 2017 20:55

machenmusik mentioned this pull request Apr 8, 2017

Support GearVR 3DOF controller #2545

Merged

dmarcos reviewed Apr 12, 2017

View reviewed changes

dmarcos added needs changes and removed needs review labels Apr 12, 2017

machenmusik force-pushed the axismove-for-touch-and-vive branch from c6ff814 to 16ba6de Compare April 14, 2017 17:56

machenmusik added a commit to chenzlabs/aframe that referenced this pull request Apr 14, 2017

bring gearvr-controls in line with aframevr#2513

985a871

machenmusik force-pushed the axismove-for-touch-and-vive branch from 985a871 to 16ba6de Compare April 14, 2017 17:58

machenmusik and others added 4 commits April 14, 2017 13:59

bring gearvr-controls in line with aframevr#2513

0dd0da0

Merge branch 'master' into axismove-for-touch-and-vive

1f37fce

refactor emitIfAxesChanged into utils

2c3c3a4

Merge branch 'axismove-for-touch-and-vive' of github.com:chenzlabs/af…

4e5c303

…rame into axismove-for-touch-and-vive

machenmusik added a commit to chenzlabs/aframe that referenced this pull request Apr 14, 2017

bring in line with latest aframevr#2513

0744be3

machenmusik added a commit to chenzlabs/aframe that referenced this pull request Apr 14, 2017

bring vive-tracker in line with aframevr#2513

a6dfe4c

dmarcos merged commit 2827f9c into aframevr:master Apr 16, 2017

machenmusik added a commit to chenzlabs/aframe that referenced this pull request Apr 17, 2017

bring in line with latest aframevr#2513

daeef99

machenmusik added a commit to chenzlabs/aframe that referenced this pull request Apr 17, 2017

bring vive-tracker in line with aframevr#2513

629af74

machenmusik added a commit to chenzlabs/aframe that referenced this pull request Apr 18, 2017

bring in line with latest aframevr#2513

d7e548b

machenmusik added a commit to chenzlabs/aframe that referenced this pull request Apr 18, 2017

bring vive-tracker in line with aframevr#2513

1e384fe

machenmusik added a commit to chenzlabs/aframe that referenced this pull request Apr 21, 2017

bring in line with latest aframevr#2513

28e2e75

machenmusik added a commit to chenzlabs/aframe that referenced this pull request Apr 21, 2017

bring vive-tracker in line with aframevr#2513

1133bd0

machenmusik added a commit to chenzlabs/aframe that referenced this pull request Apr 21, 2017

bring in line with latest aframevr#2513

1155bd0

machenmusik added a commit to chenzlabs/aframe that referenced this pull request Apr 21, 2017

bring vive-tracker in line with aframevr#2513

0c42af3

machenmusik added a commit to chenzlabs/aframe that referenced this pull request Apr 24, 2017

bring in line with latest aframevr#2513

e41efab

machenmusik added a commit to chenzlabs/aframe that referenced this pull request Apr 24, 2017

bring vive-tracker in line with aframevr#2513

e8dfb7c

machenmusik added a commit to chenzlabs/aframe that referenced this pull request Apr 25, 2017

bring in line with latest aframevr#2513

676865b

machenmusik added a commit to chenzlabs/aframe that referenced this pull request Apr 25, 2017

bring vive-tracker in line with aframevr#2513

78267ae

machenmusik added a commit to chenzlabs/aframe that referenced this pull request May 25, 2017

bring in line with latest aframevr#2513

4aae118

machenmusik added a commit to chenzlabs/aframe that referenced this pull request May 25, 2017

bring vive-tracker in line with aframevr#2513

dcce0b2

machenmusik added a commit to chenzlabs/aframe that referenced this pull request Jun 22, 2017

bring in line with latest aframevr#2513

ed2d2d3

machenmusik added a commit to chenzlabs/aframe that referenced this pull request Jun 22, 2017

bring vive-tracker in line with aframevr#2513

6b18095

Uh oh!

Axismove for touch and vive #2513

Axismove for touch and vive #2513

Uh oh!

Conversation

machenmusik commented Mar 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

machenmusik commented Mar 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dmarcos commented Mar 23, 2017

Uh oh!

dmarcos commented Mar 23, 2017

Uh oh!

machenmusik commented Mar 23, 2017

Uh oh!

machenmusik commented Mar 24, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

machenmusik Mar 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

machenmusik Mar 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dmarcos commented Apr 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

machenmusik commented Apr 12, 2017

Uh oh!

machenmusik commented Apr 12, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

machenmusik commented Mar 22, 2017 •

edited

Loading

machenmusik Mar 24, 2017 •

edited

Loading

machenmusik Mar 27, 2017 •

edited

Loading

dmarcos commented Apr 11, 2017 •

edited

Loading

machenmusik Apr 12, 2017 •

edited

Loading

machenmusik commented Apr 12, 2017 •

edited

Loading