Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WebXR Input Source: linear velocity #3060

Merged
merged 12 commits into from
Dec 8, 2023
Merged

WebXR Input Source: linear velocity #3060

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
8fa9d54
WebXR input source - add linear velocity
Maksims Apr 1, 2021
e77e169
XrInputSource add docs for getLinearVelocity
Maksims Apr 1, 2021
f7af9e3
revert grip flag
Maksims Apr 1, 2021
184acd9
xr input source linear velocity lerping, for better match to oculus b…
Maksims Apr 2, 2021
b97ee8a
consistency fixes
Maksims Apr 6, 2021
a5340f4
fix
Maksims Apr 6, 2021
efbebef
merge
Maksims Apr 21, 2021
cdc4076
merge
Maksims Nov 14, 2023
5bbd9f7
fixes
Maksims Nov 14, 2023
517ea74
fixes
Maksims Nov 14, 2023
2aeb637
Merge branch 'main' into webxr-input-source-velocity
Maksims Nov 23, 2023
820a845
Edit JSDoc for getLinearVelocity
willeastcott Dec 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 51 additions & 14 deletions src/xr/xr-input-source.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,18 +55,34 @@ class XrInputSource extends EventHandler {
this._ray = new Ray();
this._rayLocal = new Ray();
this._grip = false;
this._gripSpace = xrInputSource.gripSpace;
this._targetRaySpace = xrInputSource.targetRaySpace;
this._hand = null;
this._velocitiesAvailable = false;
this._velocitiesTimestamp = Date.now();

if (xrInputSource.hand)
this._hand = new XrHand(this);

this._localTransform = null;
this._worldTransform = null;
this.vec3A = new Vec3();

this._position = new Vec3();
this._rotation = new Quat();
this._dirtyLocal = true;
this._localPosition = null;
this._localRotation = null;
this._dirtyLocal = true;
this._localTransform = null;
this._worldTransform = null;

if (this._gripSpace) {
this._localPositionLast = new Vec3();
this._localPosition = new Vec3();
this._localRotation = new Quat();
this._localTransform = new Mat4();
this._worldTransform = new Mat4();
this._linearVelocity = new Vec3();
this._linearVelocity2 = new Vec3();
}

this._selecting = false;
this._squeezing = false;
Expand Down Expand Up @@ -185,26 +201,34 @@ class XrInputSource extends EventHandler {
this._hand.update(frame);
} else {
// grip
if (this._xrInputSource.gripSpace) {
var gripPose = frame.getPose(this._xrInputSource.gripSpace, this._manager._referenceSpace);
if (this._gripSpace) {
var gripPose = frame.getPose(this._gripSpace, this._manager._referenceSpace);
if (gripPose) {
if (! this._grip) {
this._grip = true;
this._grip = true;
this._dirtyLocal = true;

this._localTransform = new Mat4();
this._worldTransform = new Mat4();
const now = Date.now();
Maksims marked this conversation as resolved.
Show resolved Hide resolved
const dt = (now - this._velocitiesTimestamp) / 1000;
this._velocitiesTimestamp = now;

this._localPosition = new Vec3();
this._localRotation = new Quat();
}
this._dirtyLocal = true;
this._localPositionLast.copy(this._localPosition);
this._localPosition.copy(gripPose.transform.position);
this._localRotation.copy(gripPose.transform.orientation);

this._velocitiesAvailable = true;
if (this._manager.input.velocitiesSupported && gripPose.linearVelocity) {
this._linearVelocity.copy(gripPose.linearVelocity);
} else if (dt > 0) {
this.vec3A.sub2(this._localPosition, this._localPositionLast).divScalar(dt);
this._linearVelocity.lerp(this._linearVelocity, this.vec3A, 0.15);
}
} else {
this._velocitiesAvailable = false;
}
}

// ray
var targetRayPose = frame.getPose(this._xrInputSource.targetRaySpace, this._manager._referenceSpace);
var targetRayPose = frame.getPose(this._targetRaySpace, this._manager._referenceSpace);
if (targetRayPose) {
this._dirtyRay = true;
this._rayLocal.origin.copy(targetRayPose.transform.position);
Expand Down Expand Up @@ -299,6 +323,19 @@ class XrInputSource extends EventHandler {
return this._localRotation;
}

/**
* @function
* @name XrInputSource#getLinearVelocity
* @description Get the linear velocity (units per second) of input source if it is handheld ({@link XrInputSource#grip} is true). Otherwise it will return null.
* @returns {Vec3|null} The world space linear velocity of handheld input source.
*/
getLinearVelocity() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have RigidBodyComponent#linearVelocity. Should this be a property too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough, but whole XR APIs uses function notation. Also it is only a get function, there is no way to set linear velocity as it is sourced from underlying APIs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then it would just be a getter property. No requirement to have a setter. Functions are fine for getPosition/setPosition etc to mirror the function on GraphNode but generally, we should use properties where it makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to use properties, then all other methods would need to be changed for consistency reasons. Preferring a consistent design for WebXR APIs.

Copy link
Contributor

@willeastcott willeastcott Apr 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The WebXR APIs shouldn't define their own API style. There should be an engine-wide style. If the WebXR APIs don't adhere to that, we can gradually migrate and alias to something that is consistent with the overall engine. Especially while the WebXR spec is in flux.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've used get* style which is in line with Entity and other APIs. And as mentioned before, this is not settable property like in rigidbody's case.
This particular method can be changed to match very specific case of rigidbody.
Then should we change: getOrigin and getDirection of XrInputSource to be similar to Ray, but again mentioning: origin and direction of input source cannot be set by developer, and provided by an API. Also they do call a function inside to update transform, just like GraphNode.getPosition does.

And regarding getPosition and getRotation, should we change them to simply: position and rotation?

To be honest, it is already not consistent within engine, but it is very clear that if it is a property - then it can be set. In XR case, those methods cannot be set, they only provide data.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a large number of gettable but not settable properties in the API. Just check out GraphNode for example:

https://developer.playcanvas.com/en/api/pc.GraphNode.html#children

Read-only properties are 100% OK.

getPosition and getRotation could be aliased to position and rotation maybe. I think it's a little complicated and needs some serious thought.

TBH, we should probably improve Ray and have the XrInputSource provide a Ray instead of separate origin and direction.

Copy link
Contributor Author

@Maksims Maksims Apr 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can definitely start a discussion and do WebXR-wide refactor, with such changes. Some companies already using WebXR for development, and soon we need to consider backwards compatibility.
Development for Oculus Quest is already a commercially viable path.
Also developing for Chrome Android only - is commercially viable too.

if (! this._velocitiesAvailable)
return null;

return this._linearVelocity;
}

/**
* @function
* @name XrInputSource#getOrigin
Expand Down
1 change: 1 addition & 0 deletions src/xr/xr-input.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class XrInput extends EventHandler {
this.manager = manager;
this._session = null;
this._inputSources = [];
this.velocitiesSupported = (window.XRPose && XRPose.prototype.hasOwnProperty('linearVelocity')) || false;

this._onInputSourcesChangeEvt = function (evt) {
self._onInputSourcesChange(evt);
Expand Down
2 changes: 1 addition & 1 deletion src/xr/xr-manager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ class XrManager extends EventHandler {
});
}

update(frame) {
update(frame, dt) {
Maksims marked this conversation as resolved.
Show resolved Hide resolved
if (! this._session) return;

var i, view, viewRaw, layer, viewport;
Expand Down