-
Notifications
You must be signed in to change notification settings - Fork 424
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
[WIP] - Add viewer.py to Habitat-sim module #1990
base: main
Are you sure you want to change the base?
Conversation
@mosra thoughts on the EGL issue with running from the module? |
self.display_font = text.FontManager().load_and_instantiate("TrueTypeFont") | ||
self.display_font.open_file( | ||
os.path.join(os.path.dirname(__file__), self.sim_settings["font_path"]), | ||
13, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mosra had to replace the relative filepath with a configurable one which can be updated in consumer code. Generally we can't expect a data
folder to be available from conda build, so we should consider how to handle this.
Maybe we should include the font file with the python module source code instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should include the font file with the python module source code instead?
I think that this would make sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that makes sense -- when John did that originally we weren't sure what the use case would be, and hardcoding the relative path to data/
was the most straightforward way.
I'm a strong supporter of this change! A couple of thoughts (from when I was thinking about this a few months ago):
|
Thanks, I agree. I'll start a design doc to help plan which UI features should be core vs. example. 👍 |
A bit of background, just to have the full picture here -- by default, Python loads every module in its own namespace ( The other half of the problem that Habitat uses Magnum compiled statically, and while I eliminated most mutable globals, Magnum has still three left -- for the plugin manager, for logging redirection and for the GL context. Those, with Magnum being compiled statically, get one copy in the A real solution to all these problems would be to have Magnum compiled dynamically, but that was historically not an option as far as I remember. Maybe we could reconsider, it'd definitely fix 98 of 100 recurring nightmares I have :D The EGL error is most probably related to the above -- TL;DR: you'll still need the flag there, at least until Magnum gets compiled dynamically, or a different solution is discovered. |
Motivation and Context
Currently, the interactive sim viewer (
example/viewer.py
) lives in theexamples/
directory is therefore not exposed within thehabitat_sim
python module. As such, it cannot be used with Habitat-sim conda build or imported and extended in Habitat-lab.This PR copies the viewer code into the
src_python
directory and makes minor edits for code style and flaking requirements.See Habitat-lab PR 1099 for example use as external module to extend.
TODO:
Once this is ready for merge we'll update documentation and possibly deprecate the examples/viewer.py.
How Has This Been Tested
With Habitat-sim module installed,
Likely related: we require the following to avoid this issue with the python file run:
possibly the module run is picking things up differently.
Types of changes
Checklist