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.

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

Physics usability improvements #231

Merged
merged 9 commits into from
Sep 23, 2019
Merged

Conversation

mosra
Copy link
Collaborator

@mosra mosra commented Sep 20, 2019

Motivation and Context

Sweeps away a bunch of roadblocks I ran into when crafting screenshots for #152.

  • Naming the Bullet-enabling CMake var consistently, turning it into a CMake option so it's discoverable even for people that don't use setup.py at all (i.e., me). Not a user-facing change as the public setup.py variable is still named the same.
  • Using proper directory joining for physics object filenames. This allows absolute filenames to be used inside the JSON configs (i.e., /home/mosra/Data/fair/1199943_collision.glb) without those being turned into ./data//home/mosra/Data/fair/1199943_collision.glb.
  • Printing the physics simulation stats every frame actually slows everything down quite a lot (depending on how slow is your terminal), so not doing that. After that change I got back to 60 FPS in the viewer.
  • Improving the physics config file passing.
    • Use an absolute default path so it doesn't depend on current working directory.
    • Instead of dying on an assert inside rapidjson, check file existence and fail gracefully with a clear message.
    • Use consistent naming scheme for the option.
  • Exiting after an error shouldn't use a success return code.
  • --enable-physics w/o Bullet enabled is most probably an error, so warn on that. People might still want to do this, so not making this a hard error. (Alternatively, --enable-physics could be compiled out when Bullet is not enabled, if that makes more sense for you.)
  • Improving usability of the banana throwing. When all files fail to load, it would crash because of a modulo with zero. And when physics is not enabled, the O key would silently do nothing, which isn't great either.
  • Enable 4x MSAA for the viewer so the imported models are less aliased. Performed well even on my Intel 630 on a 4K screen, and I expect even less perf issues for you all with NVidias :)

How Has This Been Tested

Creating those screenshots was quite a soak test for all the viewer functionality, and it survived.

This allows absolute filenames to be used inside the JSON configs.
…e a lot.

Now you can't *see* it runs 60 FPS but you can *feel* it.
 * Use an absolute default path so it doesn't depend on current working
   directory.
 * Instead of dying on an assert inside rapidjson, check file existence
   and fail gracefully with a clear message.
 * Use consistent naming scheme for the options.
So warn on that. People *might* still want to do this, so not making
this a hard error.
When all files fail to load, it would crash because of a modulo
with zero. When physics is not enabled, the O key would silently do
nothing.
@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Sep 20, 2019
src/esp/gfx/Viewer.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@aclegg3 aclegg3 left a comment

Choose a reason for hiding this comment

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

Thanks, mosra. LGTM :)

<< ":\n---\nPhysics was enabled and Bullet physics engine was "
"specified, but the project is built without Bullet support. "
"Objects added to the scene will be restricted to kinematic updates "
"only. Reinstall with --bullet to enable Bullet dynamics.\n---";
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

with --with-bullet? :)

Looking again, this error was actually there, but so buried among the others that I didn't see it. Now it is nicely loud and clear.

@aclegg3 aclegg3 merged commit 9bf5cc7 into master Sep 23, 2019
@aclegg3 aclegg3 deleted the physics-usability-improvements branch September 23, 2019 16:15
Ram81 pushed a commit to Ram81/habitat-web-sim that referenced this pull request Dec 10, 2020
* Name the Bullet-enabling var consistently, turn it into a CMake option.

* assets: use proper directory joining for physics object filenames.

* This allows absolute filenames to be used inside the JSON configs.

* viewer: printing this every frame actually slows everything down quite a lot.

Now you can't *see* it runs 60 FPS but you can *feel* it.

* viewer: improve the physics config file passing.

 * Use an absolute default path so it doesn't depend on current working
   directory.
 * Instead of dying on an assert inside rapidjson, check file existence
   and fail gracefully with a clear message.
 * Use consistent naming scheme for the options.

* viewer: exiting after an error shouldn't use a success return code.

* viewer: improve usability of the banana throwing.

When all files fail to load, it would crash because of a modulo
with zero. When physics is not enabled, the O key would silently do
nothing.

* viewer: enable at least a bit of MSAA for prettier screenshots.

* Changed no Bullet WARNING in ResourceManager to be more helpful.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants