optimize tracked controls tick, discovery, and various utils #2943

ngokevin · 2017-08-08T02:25:26Z

Description:

There was some messy/dirty/redundant code. Tested with Vive controllers.

Changes proposed:

Limit the tracked-controls tick to every 500ms. Before it was every 10ms which does not help much since it usually takes more time to render a single frame. Pretty much, it was getting called on every frame. I don't think this polling needs to be called so often.
Don't emit an event and allocate an object every single controller poll. Only emit if the number of controllers change. Before this was emitting every tick, and then possibly handled by 4 controller components at once, for each hand. Save 2 event emissions, 2 object allocations, and 8 event handler calls per tick.
Remove getGamepadsByPrefix util. This was only being used in the tracked controls tick, and tracked controls never used the prefix feature. We call the navigator.gamePads now directly in the controls tick. Previously, we were creating a string and array on each tick, and then re-looping over that array and copying it to another array.
Clean up axismove event abstraction, removing callbacks, and object/array creations.
Don't need to call updateControllersList in the isControllerPresent util. This didn't really do anything since controllers were picked up through the controllersupdate event rather than within here.
Various cleanups include removing unused properties (axisLabels), fixing 95-char lengths, and cleaner comments.

dmarcos · 2017-08-08T02:33:29Z

src/components/gearvr-controls.js

-    this.onButtonUp = function (evt) { self.onButtonEvent(evt.detail.id, 'up'); };
-    this.onButtonTouchStart = function (evt) { self.onButtonEvent(evt.detail.id, 'touchstart'); };
-    this.onButtonTouchEnd = function (evt) { self.onButtonEvent(evt.detail.id, 'touchend'); };
+    this.onButtonDown = function (evt) {


Can we keep the one liners. It makes the code harder to parse?

Some were going over a character limit causing them to wrap. Want me to revert still?

I think it's ok going over the character limit in same cases to avoid contortions and breaking style consistency

dmarcos · 2017-08-08T04:29:43Z

src/components/daydream-controls.js

+    this.onButtonUp = function (evt) {
+      self.onButtonEvent(evt.detail.id, 'up');
+    };
+    this.onButtonTouchStart = function (evt) {


dmarcos · 2017-08-08T04:29:52Z

src/components/daydream-controls.js

+      self.onButtonEvent(evt.detail.id, 'up');
+    };
+    this.onButtonTouchStart = function (evt) {
+      self.onButtonEvent(evt.detail.id, 'touchstart');


yeah i can catch them all if you just note once

though i missed this file xD

dmarcos · 2017-08-08T04:29:59Z

src/components/daydream-controls.js

+    this.onButtonTouchStart = function (evt) {
+      self.onButtonEvent(evt.detail.id, 'touchstart');
+    };
+    this.onButtonTouchEnd = function (evt) {


dmarcos · 2017-08-08T04:31:31Z

src/components/daydream-controls.js

-    buttonMeshes.system = controllerObject3D.getObjectByName('HomeButton_HomeButton_Cylinder.005');
-    buttonMeshes.trackpad = controllerObject3D.getObjectByName('TouchPad_TouchPad_Cylinder.003');
-    // Offset pivot point
+    buttonMeshes.menu = controllerObject3D.getObjectByName(


If needed I think it looks better to break the whole expression to the line below instead of just an argument

i'll just restore to one liners

fernandojsg · 2017-08-08T05:35:59Z

I believe it could be nice to have the tick ms as a parameter, for some applications 500ms it could be way too much

ngokevin · 2017-08-08T06:15:08Z

It just detects new controllers right? Half a second isn't much

fernandojsg · 2017-08-08T06:24:53Z

@ngokevin my fault, I was thinking that the tick was in updating the position/rotation, not just on updating the list, I've checked the coded already O:)

ngokevin · 2017-08-08T20:27:37Z

Updated to save another couple object allocations and to skip type check per tracked controls per tick.

machenmusik · 2017-08-08T22:09:03Z

Has this been checked to ensure that controller pose update rate is unaffected in-VR? I know things have changed a lot since we started these... once upon a time, the only way the controller pose was updated was when it was grabbed by tracked-controls system tick, and then utilized by tracked-controls component tick

machenmusik · 2017-08-08T22:18:02Z

Ok so I think the assumption is that the browser continues to make pose changes to the same controller object, which is still updated unthrottled by tracked-controls component tick, so unless some browser has done wacky things in its implementation, LGTM. Only quirk is if controller goes away, and we won't know it for up to 500ms; need to ensure that dying controller object state doesn't break things before detection.

ngokevin · 2017-08-08T22:20:42Z

Yeah, it'd be pretty noticeable if controllers only moved once every 500ms. The tracked-controls component handles the pose update. The system only handles detection for connected/disconnected so it's fine.

I'll double-check what happens on disconnect.

ngokevin added the needs review label Aug 8, 2017

dmarcos reviewed Aug 8, 2017

View reviewed changes

ngokevin force-pushed the optimizecontrols branch from 325c594 to 4055bc3 Compare August 8, 2017 04:05

dmarcos reviewed Aug 8, 2017

View reviewed changes

ngokevin force-pushed the optimizecontrols branch from 4055bc3 to 13ff155 Compare August 8, 2017 04:30

dmarcos reviewed Aug 8, 2017

View reviewed changes

optimize tracked controls tick and various utils

88fb20b

ngokevin force-pushed the optimizecontrols branch from 13ff155 to 88fb20b Compare August 8, 2017 04:32

reuse position/rotation objects for tracked-controls pose updates

2328c7e

dmarcos merged commit df50647 into aframevr:master Aug 11, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

optimize tracked controls tick, discovery, and various utils #2943

optimize tracked controls tick, discovery, and various utils #2943

Uh oh!

ngokevin commented Aug 8, 2017 •

edited

Loading

dmarcos Aug 8, 2017

ngokevin Aug 8, 2017

dmarcos Aug 8, 2017

ngokevin Aug 8, 2017

dmarcos Aug 8, 2017

dmarcos Aug 8, 2017

ngokevin Aug 8, 2017

ngokevin Aug 8, 2017

dmarcos Aug 8, 2017

dmarcos Aug 8, 2017

ngokevin Aug 8, 2017

fernandojsg commented Aug 8, 2017

ngokevin commented Aug 8, 2017

fernandojsg commented Aug 8, 2017

ngokevin commented Aug 8, 2017

machenmusik commented Aug 8, 2017 •

edited

Loading

machenmusik commented Aug 8, 2017

ngokevin commented Aug 8, 2017

Labels

4 participants

Uh oh!

optimize tracked controls tick, discovery, and various utils #2943

optimize tracked controls tick, discovery, and various utils #2943

Uh oh!

Conversation

ngokevin commented Aug 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fernandojsg commented Aug 8, 2017

ngokevin commented Aug 8, 2017

fernandojsg commented Aug 8, 2017

ngokevin commented Aug 8, 2017

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

Uh oh!

machenmusik commented Aug 8, 2017

ngokevin commented Aug 8, 2017

Labels

4 participants

ngokevin commented Aug 8, 2017 •

edited

Loading

machenmusik commented Aug 8, 2017 •

edited

Loading