add smooth moving/looking in viewer by charliecb · Pull Request #1013 · facebookresearch/habitat-sim

charliecb · 2021-01-08T04:47:50Z

Motivation and Context

Previously, the viewer controls to move and look-around moved the agent in discrete steps, and multiple controls could not be used at the same time. To rationalize this, holding "W" in the viewer had the same effect as doing so in a text editor: the agent moves forward once, pauses, and then stutters forward.

To obtain smoother movement and looking-around, this feature does 3 things:

Holding movement keys now results in gliding forward smoothly
Roughly, the rate of movement is independent of frame rate (i.e. it is nearly constant in the world time)
Multiple movement/look-around keys can be handled simultaneously

How Has This Been Tested

Only ad-hoc testing so far. I am concerned that physics may behave slightly differently around pauses (spacebar) and frame-by-frame stepping (.) now, since a minor change was made here. I believe that this was a slight fix, but I don't fully understand the original code, so I could have misinterpreted the situation.

Types of changes

Docs change / refactoring / dependency upgrade
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have completed my CLA (see CONTRIBUTING)
I have added tests to cover my changes.
All new and existing tests passed.

erikwijmans

The downside of this approach is that unless the agent is super fast (like it is now) it has the effect of doing the same thing as disabling sliding, which makes human control much rougher (the agent will get stuck on any little thing). One way around this is to instead of taking lots of little steps, still take the big steps but interpolate the agent between the current state and the next state. Magnum does already have quat slerp for us so it should be fairly straight forward.

jturner65

Added a few change requests. Nice first PR :)

jturner65 · 2021-01-08T12:17:39Z

+    if (keysPressed[KeyEvent::Key::A]) {
+      defaultAgent_->act("moveLeft");
+    }
+    if (keysPressed[KeyEvent::Key::D]) {
+      defaultAgent_->act("moveRight");
+    }
+    if (keysPressed[KeyEvent::Key::S]) {
+      defaultAgent_->act("moveBackward");
+    }
+    if (keysPressed[KeyEvent::Key::W]) {
+      defaultAgent_->act("moveForward");
+    }
+    if (keysPressed[KeyEvent::Key::X]) {
+      defaultAgent_->act("moveDown");
+    }
+    if (keysPressed[KeyEvent::Key::Z]) {
+      defaultAgent_->act("moveUp");
+    }


recAgentLocation() should be called in here, as in the original movement cases. At the end of this section, if any of the movement actions have executed (if the agent's location has changed in any way), recAgentLocation() should be called.

aclegg3

Overall, these changes feel great. They seem to work well with low FPS also. Happy to approve once other concerns are addressed. 👍

aclegg3 · 2021-01-08T16:57:30Z

+    }
+    if (keysPressed[KeyEvent::Key::Down]) {
+      defaultAgent_->act("lookDown");
+    }


These (arrow key turning/looking) feel a bit sluggish compared to the linear movement speed currently.

aclegg3 · 2021-01-08T17:22:03Z

The downside of this approach is that unless the agent is super fast (like it is now) it has the effect of doing the same thing as disabling sliding, which makes human control much rougher (the agent will get stuck on any little thing).

I'm actually not finding this to be the case in practice. If you reduce the moveSensitivity configured here by 10x to 0.008 the sliding still feels good. I think the current approach should be fine for interactive applications.

eundersander · 2021-01-08T18:02:40Z

The downside of this approach is that unless the agent is super fast (like it is now) it has the effect of doing the same thing as disabling sliding, which makes human control much rougher (the agent will get stuck on any little thing).

I'm actually not finding this to be the case in practice. If you reduce the moveSensitivity configured here by 10x to 0.008 the sliding still feels good. I think the current approach should be fine for interactive applications.

I tried this branch just now and I think the sliding feels good. For example, if you walk almost straight at a wall, you slide very slowly, but if you walk almost parallel to the wall (slightly into it), you walk almost full speed.

erikwijmans · 2021-01-08T19:36:42Z

Yeah, seems like a 10x reduction in speed is enough to not make it feel speedy but not enough to effectively disable sliding. Disregard the comment above :)

eundersander · 2021-01-08T22:44:27Z

 }

+void Viewer::moveAndLook(int repetitions) {
+  if (repetitions <= 2) {repetitions = 1;}    // for visual smoothness, for small deviations around 60fps, we default to repetitions=1.


We discussed offline and agreed this isn't necessary, so I expect you'll remove this line.

eundersander · 2021-01-08T22:55:27Z

-    simulator_->stepWorld(1.0 / 60.0);
-    timeSinceLastSimulation = 0.0;
-    simulateSingleStep_ = false;
+  int repetitionsToPerform = timeSinceLastSimulation * 60.0;


Let's rename this "60" magic number: constexpr float agentActionsPerSecond = 60.0f. And lets rename repetitionsToPerform to numAgentActions.

jturner65

Very close. Just one more change, please.

jturner65

Looks good!

aclegg3

LGTM.

Added smooth moving and looking in viewer

559da48

facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Jan 8, 2021

erikwijmans reviewed Jan 8, 2021

View reviewed changes

jturner65 self-requested a review January 8, 2021 12:10

jturner65 requested changes Jan 8, 2021

View reviewed changes

jturner65 reviewed Jan 8, 2021

View reviewed changes

Comment thread src/utils/viewer/viewer.cpp Outdated

aclegg3 reviewed Jan 8, 2021

View reviewed changes

eundersander reviewed Jan 8, 2021

View reviewed changes

Address PR comments

25e6cb2

jturner65 requested changes Jan 9, 2021

View reviewed changes

Comment thread src/utils/viewer/viewer.cpp

Remove redundant recAgentLocation calls

49116d3

jturner65 approved these changes Jan 11, 2021

View reviewed changes

aclegg3 approved these changes Jan 11, 2021

View reviewed changes

charliecb merged commit 3174966 into facebookresearch:master Jan 11, 2021

charliecb deleted the smooth-movement-for-cpp-viewer branch January 11, 2021 22:07

Conversation

charliecb commented Jan 8, 2021

Motivation and Context

How Has This Been Tested

Types of changes

Checklist

Uh oh!

erikwijmans left a comment

Choose a reason for hiding this comment

Uh oh!

jturner65 left a comment

Choose a reason for hiding this comment

Uh oh!

jturner65 Jan 8, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

aclegg3 left a comment

Choose a reason for hiding this comment

Uh oh!

aclegg3 Jan 8, 2021 • 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!

aclegg3 commented Jan 8, 2021

Uh oh!

eundersander commented Jan 8, 2021

Uh oh!

erikwijmans commented Jan 8, 2021

Uh oh!

eundersander Jan 8, 2021

Choose a reason for hiding this comment

Uh oh!

eundersander Jan 8, 2021

Choose a reason for hiding this comment

Uh oh!

jturner65 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jturner65 left a comment

Choose a reason for hiding this comment

Uh oh!

aclegg3 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

aclegg3 Jan 8, 2021 •

edited

Loading