Skip to content

WebXR Controls Improvements #1076

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

Closed
BlaiseRideout wants to merge 4 commits into from
Closed

WebXR Controls Improvements #1076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9ab8b20
Replace WebVR integration with WebXR
Dec 18, 2020
4cabd2d
Merge branch 'master' of https://github.com/facebookresearch/habitat-…
Jan 14, 2021
b8f23c9
Merge branch 'master' of https://github.com/facebookresearch/habitat-…
Feb 2, 2021
72ed24b
Add controller input and apply head direction to agent
Feb 2, 2021
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
45 changes: 45 additions & 0 deletions src/esp/bindings_js/bindings_js.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,20 @@ std::map<std::string, ObservationSpace> Simulator_getAgentObservationSpaces(
return spaces;
}

template <class T, typename... Targs>

@Skylion007 Skylion007 Feb 4, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@mosra You are probably the best person to review this part. Do we really not have any better bindings exposed via Magnum?

@mosra mosra Feb 4, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

¯\_(ツ)_/¯

Magnum has no JS bindings on its own. Everything has been happening organically here so far, on an as-needed basis.

@eundersander eundersander Feb 4, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

FYI, I've reached out to Blaise about making some refactors here. The JS code uses Eigen types (vec4f, etc.), perhaps for legacy reasons.

@Skylion007 Skylion007 Feb 4, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@morsa does Magnum::Quaternion really not have a constructor for Vec4f though? Or at least a util for conversion?

@Skylion007 Skylion007 Feb 4, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah nvm, @eundersander posted the answer below.

@mosra mosra Feb 4, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There's a conversion from/to Eigen's Quaternion class, and possibly Eigen itself could have some way to convert to a 4-component vector on its own.

I'm not providing a conversion to a 4-component vector because it's not clear whether it should be xyzw or wxyz -- each math library uses something else, so it's better to be explicit. Which means doing a conversion using data()[i] also might not be 100% safe -- I don't know what order is the vec4f expected to be in and if it matches the order inside Magnum's Quaternion.

static inline auto create(Targs&&... args) {
return std::make_shared<T>(std::forward<Targs>(args)...);
}

Magnum::Quaternion Quaternion_fromVec4f(const vec4f& rot) {
return Magnum::Quaternion(quatf(rot)).normalized();
}

Magnum::Quaternion Quaternion_mul(const Magnum::Quaternion& q1,

@eundersander eundersander Feb 4, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree we need a quaternion multiply. But can we avoid introducing Magnum::Quaternion in the bindings interface? So far, for legacy reasons, we use the eigen vec4f. So I'm suggesting:

vec4f quaternionMultiply(const vec4f& q1, const vec4f& q2) {
  auto product = Magnum::Quaternion(quatf(q1)) * Magnum::Quaternion(quatf(q2));
  return vec4f(product.data()[0], product.data()[1], product.data()[2],
               product.data()[3]);
}

const Magnum::Quaternion& q2) {
return q1 * q2;
}

Observation Sensor_getObservation(Sensor& sensor, Simulator& sim) {
Observation ret;
if (CameraSensor * camera{dynamic_cast<CameraSensor*>(&sensor)})
Expand All @@ -68,6 +82,11 @@ void Sensor_setLocalTransform(Sensor& sensor,
node.setRotation(Magnum::Quaternion(quatf(rot)).normalized());
}

void Agent_setRotation(Agent& agent, const Magnum::Quaternion& rot) {

@eundersander eundersander Feb 4, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we avoid introducing this new binding? How about implementing in JS:

  setAgentRotation(rotAngle) {
    let state = this.sim.getAgentState();
    state.rotation = WebAssembly.Module.eulerToQuaternion([0, rotAngle, 0])
    this.sim.setAgentState(state);
  }

@BlaiseRideout BlaiseRideout Feb 4, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I used that approach originally, but switched to this one because I didn't need to set the whole agent state, so fetching the whole thing and then setting the whole thing when I only need to update one parameter seemed overkill and potentially less performant.

@Skylion007 Skylion007 Feb 6, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, but that former approach is consistent with our Python AgentState APIs. I'd be surprised if it was a performance bottleneck, do you have benchmarks?

SceneNode& node{agent.node()};
node.setRotation(rot);
}

vec3f quaternionToEuler(const quatf& q) {
return q.toRotationMatrix().eulerAngles(0, 1, 2);
}
Expand Down Expand Up @@ -131,6 +150,31 @@ EMSCRIPTEN_BINDINGS(habitat_sim_bindings_js) {
.field("min", &std::pair<vec3f, vec3f>::first)
.field("max", &std::pair<vec3f, vec3f>::second);

em::class_<Magnum::Rad>("Rad").constructor<float>();

em::class_<Magnum::Matrix4>("Matrix4").smart_ptr_constructor(
"Matrix4", &create<Magnum::Matrix4>);

em::class_<Magnum::Vector3>("Vector3")
.constructor<Magnum::Vector3>()
.constructor<float, float, float>()
.function("x", em::select_overload<float&()>(&Magnum::Vector3::x))
.function("y", em::select_overload<float&()>(&Magnum::Vector3::y))
.function("z", em::select_overload<float&()>(&Magnum::Vector3::z))
.class_function("xAxis", &Magnum::Vector3::xAxis)
.class_function("yAxis", &Magnum::Vector3::yAxis)
.class_function("zAxis", &Magnum::Vector3::zAxis);

em::class_<Magnum::Quaternion>("Quaternion")
.constructor<Magnum::Vector3, float>()
.constructor<Magnum::Vector3>()
.function("normalized", &Magnum::Quaternion::normalized)
.function("inverted", &Magnum::Quaternion::inverted)
.function("transformVector", &Magnum::Quaternion::transformVector)
.function("mul", &Quaternion_mul)
.class_function("fromVec4f", &Quaternion_fromVec4f)

@Skylion007 Skylion007 Feb 4, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should be defined as custom embind factory constructor, not a static method.

@eundersander eundersander Feb 9, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm going to push for another direction here. A nice, easy philosophy to apply everywhere here is: the Javascript class should have the same interface as the C++ class. Or a subset that grows as necessary (but remains a subset). My thinking is that @mosra already put some thought into this class's interface so why second-guess. It also avoids confusion for users learning to use both the Javascript and C++ versions of the class.

This means making fromVec4f a free function, called quaternionFromVec4f or similar.

mul is a special case because Javascript doesn't support operator overloading. I'm okay making that a member, since q1.mul(q2) would closely resemble q1 * q2.

@Skylion007 Skylion007 Feb 9, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree with all that except the fact that it should be a static function like above.

@eundersander eundersander Feb 10, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it should be a static function like above.

Can you clarify? which function are we talking about (fromVec4f or mul)? And by static do you mean a static method on the javascript type?

I would argue that Module.Quaternion.fromVec4f (static method on Javascript Quaternion class) is confusing because Magnum::Quaternion::fromVec4f doesn't exist in C++.

If you're advocating that mul should be a static method (not a regular/non-static method), sure, I'm fine with that.

.class_function("rotation", &Magnum::Quaternion::rotation);

em::class_<AgentConfiguration>("AgentConfiguration")
.smart_ptr_constructor("AgentConfiguration",
&AgentConfiguration::create<>)
Expand Down Expand Up @@ -218,6 +262,7 @@ EMSCRIPTEN_BINDINGS(habitat_sim_bindings_js) {
&Agent::getSensorSuite))
.function("getState", &Agent::getState)
.function("setState", &Agent::setState)
.function("setRotation", &Agent_setRotation)

@Skylion007 Skylion007 Feb 4, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So for consistency, rotation is a part of AgentState on the Python side Is it not the on C++ side, @bigbike ?

.function("hasAction", &Agent::hasAction)
.function("act", &Agent::act);

Expand Down
133 changes: 122 additions & 11 deletions src/esp/bindings_js/modules/vr_demo.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,24 @@
// This source code is licensed under the MIT license found in the
// LICENSE file in the root directory of this source tree.

/* global XRWebGLLayer */
/* global XRWebGLLayer, Module */
import WebDemo from "./web_demo";
import { defaultAgentConfig } from "./defaults";

const BUTTON_ID = "vr_button";

const VIEW_SENSORS = ["left_eye", "right_eye"];
const pointToArray = p => [p.x, p.y, p.z, p.w];

///////////
// INPUT //
///////////
const HANDEDNESS = "right";
const REPEAT_TIMEOUT = 500;
const LEFT_RIGHT_AXIS = 2;
const FWD_AXIS = 3;
const VIEW_YAW_STEP = Math.PI * 0.1;

class VRDemo extends WebDemo {
fpsElement;
lastPaintTime;
Expand All @@ -19,6 +30,17 @@ class VRDemo extends WebDemo {
tex = null;
inds = null;

sensorSpecs = null;

prevFwdHeld = false;
prevLeftHeld = false;
prevRightHeld = false;

viewYawOffset = 0;
FWD = Module.Vector3.zAxis(1);
FWD_ANGLE = Math.atan2(this.FWD.z(), this.FWD.x());
DOWN = Module.Vector3.yAxis(-1);

fps = 0;
skipFrames = 60;
currentFramesSkipped = 0;
Expand All @@ -41,6 +63,17 @@ class VRDemo extends WebDemo {
this.exitVR();
}

initSensors() {
const agent = this.simenv.sim.getAgent(this.simenv.selectedAgentId);

this.sensorSpecs = new Array(VIEW_SENSORS.length);
for (var iView = 0; iView < VIEW_SENSORS.length; ++iView) {
const sensor = agent.sensorSuite.get(VIEW_SENSORS[iView]);

this.sensorSpecs[iView] = sensor.specification();
}
}

// Compiles a GLSL shader. Returns null on failure
compileShader(src, type) {
const shader = this.gl.createShader(type);
Expand Down Expand Up @@ -147,6 +180,7 @@ class VRDemo extends WebDemo {
this.gl = document.createElement("canvas").getContext("webgl", {
xrCompatible: true
});
this.initSensors();
this.initGL();
}
this.webXRSession = await navigator.xr.requestSession("immersive-vr", {
Expand Down Expand Up @@ -245,6 +279,87 @@ class VRDemo extends WebDemo {
);
}

handleInput(frame) {
for (let inputSource of frame.session.inputSources) {
if (inputSource.handedness != HANDEDNESS) {
continue;
}
if (inputSource.gamepad) {
const demo = this;

const fwdHeld = inputSource.gamepad.axes[FWD_AXIS] < -0.5;
if (fwdHeld && !this.prevFwdHeld) {
this.task.handleAction("moveForward");
window.setTimeout(function() {
demo.prevFwdHeld = false;
}, REPEAT_TIMEOUT);
}
this.prevFwdHeld = fwdHeld;

const leftHeld = inputSource.gamepad.axes[LEFT_RIGHT_AXIS] > 0.5;
if (leftHeld && !this.prevleftHeld) {
this.viewYawOffset += VIEW_YAW_STEP;
window.setTimeout(function() {
demo.prevLeftHeld = false;
}, REPEAT_TIMEOUT);
}
this.prevLeftHeld = leftHeld;

const rightHeld = inputSource.gamepad.axes[LEFT_RIGHT_AXIS] < -0.5;
if (rightHeld && !this.prevRightHeld) {
this.viewYawOffset -= VIEW_YAW_STEP;
window.setTimeout(function() {
demo.prevRightHeld = false;
}, REPEAT_TIMEOUT);
}
this.prevRightHeld = rightHeld;
}
/*
// 6DoF pose example
if (inputSource.gripSpace) {
const inputPose = frame.getPose(
inputSource.gripSpace,
this.xrReferenceSpace
);
}
*/
}
}

updateHeadPose(pose, agent) {
const headRotation = Module.Quaternion.fromVec4f(
pointToArray(pose.transform.orientation)
);
const pointingVec = headRotation.transformVector(this.FWD);
const pointingAngle =
Math.atan2(pointingVec.z(), pointingVec.x()) - this.FWD_ANGLE;

const agentQuat = Module.Quaternion.rotation(
new Module.Rad(pointingAngle + this.viewYawOffset),
this.DOWN
);
const inverseAgentRot = Module.Quaternion.rotation(
new Module.Rad(-pointingAngle),
this.DOWN
);

agent.setRotation(agentQuat);

for (var iView = 0; iView < pose.views.length; ++iView) {
const view = pose.views[iView];

const sensor = agent.sensorSuite.get(VIEW_SENSORS[iView]);

const pos = pointToArray(view.transform.position).slice(0, -1); // don't need w for position
sensor.setLocalTransform(

@eundersander eundersander Feb 9, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

setLocalTransform takes a vec4f but you're passing a Quaternion. And the scary thing is that this builds and runs without error. I realize now that Emscripten/Embind/Javascript is not doing full type-checking. You can pass any object to setLocalTransform that happens to get storage as a 4-tuple of floats and you get no runtime error/warning.

inverseAgentRot.transformVector(new Module.Vector3(...pos)),
inverseAgentRot.mul(
Module.Quaternion.fromVec4f(pointToArray(view.transform.orientation))
)
);
}
}

drawVRScene(t, frame) {
const session = frame.session;

Expand All @@ -257,25 +372,21 @@ class VRDemo extends WebDemo {
return;
}

const agent = this.simenv.sim.getAgent(this.simenv.selectedAgentId);

this.handleInput(frame);
this.updateHeadPose(pose, agent);

const layer = session.renderState.baseLayer;
this.gl.bindFramebuffer(this.gl.FRAMEBUFFER, layer.framebuffer);

const agent = this.simenv.sim.getAgent(this.simenv.selectedAgentId);

const VIEW_SENSORS = ["left_eye", "right_eye"];
for (var iView = 0; iView < pose.views.length; ++iView) {
const view = pose.views[iView];
const viewport = layer.getViewport(view);
this.gl.viewport(viewport.x, viewport.y, viewport.width, viewport.height);

const sensor = agent.sensorSuite.get(VIEW_SENSORS[iView]);

sensor.setLocalTransform(
pointToArray(view.transform.position).slice(0, -1), // don't need w for position
pointToArray(view.transform.orientation)
);

const texRes = sensor.specification().resolution;
const texRes = this.sensorSpecs[iView].resolution;
const texData = sensor.getObservation(this.simenv.sim).getData();
this.drawTextureData(texRes, texData);
}
Expand Down