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

[Unity plugin] Dynamic heightfield syncing #1268

Merged
merged 10 commits into from
Jan 15, 2024

Conversation

Balint-H
Copy link
Collaborator

@Balint-H Balint-H commented Dec 7, 2023

In this PR the HField component optionally requests a rebuild of the scene during runtime if the Unity terrain changes (with optional rate limit), to allow dynamic deformable environments, (e.g. when resetting an episode of locomotion learning, or to simulate destructible terrain).

I had a go at trying to use model->hfield_data by simply writing changed heightmap pixel values to it (as floats), making sure its at the right address and right number of elements and that scene is properly initialized already. I expected that to be a cleaner and more performant approach, however this was met with crashes. Are there any limitations of writing to hfield_data I'm not aware of that might prevent its use with plugin this way?

This PR also provides edits to the HField geom's components to be a bit more robust to different terrains (taking into account assumptions of a MuJoCo hfield), and provides warnings and tooltips for configuring a terrain so that it matches with the hfield on the MuJoCo side.

Movie_007.mp4

Copy link
Contributor

@erez-tom erez-tom left a comment

Choose a reason for hiding this comment

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

Thanks so much! Could you please remove whitespaces at end of lines?

@Balint-H
Copy link
Collaborator Author

Thanks for spotting that, I believe that should be all the whitespace issues sorted.

if(terrain.transform.parent != transform) Debug.LogWarning($"The terrain of heightfield {transform.name} needs to be parented to the Geom for proper rendering.");
else {
if((terrain.transform.localPosition - new Vector3(-HeightMapLength * HeightMapScale.x / 2, terrain.transform.localPosition.y, -HeightMapWidth * HeightMapScale.z / 2)).magnitude > 0.001) {
Debug.LogWarning($"Terrain of heightfield {transform.name} not aligned with geom. The terrain will be moved to accurately represent the simulated position.");
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems like weird indentation. Should be just 2 spaces.

Can you try to follow the rules in https://google.github.io/styleguide/csharp-style.html#whitespace-rules?
There are also some really long lines here that should be broken up.

@Balint-H Balint-H force-pushed the feature/unity-hmap-dyn branch from 6cf9b5d to d0b505c Compare December 19, 2023 13:55
@Balint-H
Copy link
Collaborator Author

Thanks for the patience, I made an effort now to familiarise myself with formatting tools, and will try to be more consistent from now on.

@yuvaltassa
Copy link
Collaborator

Writing to hfield_data is supposed to work fine. We should try to understand this.

Can you try and make a MWE (in a new issue) which recreates the crash? (Ideally via Python or C, not Unity)

Thanks!

@Balint-H
Copy link
Collaborator Author

Balint-H commented Dec 20, 2023

Thanks for encouraging a further examination Yuval. I tried to recreate a MWE in python and everything worked as I expected with dynamic hfields.

I went back to Unity and tried with the latest version of the plugin and binary (3.1.1) and dynamic heightfields now work as intended! My original try was with 3.0.1, so the issue was possibly related to that. Running the same scripts when I reverted back to 3.0.1 gave the same crashes.

In any case, the hfield_data version runs considerably smoother on 3.1.x, so I'll be sending a PR update in the coming days with that.

… feature/unity-hmap-dyn

Merge with main to keep branch up to date
@Balint-H
Copy link
Collaborator Author

Balint-H commented Dec 21, 2023

With a quick profiling only in editor, writing to hfield_data is approximately 5-10 times faster then recompiling the scene. Now this method will be used if no export path for the hfield png is given for the component (the default), but I haven't removed the scene rebuilding and image exporting approach completely as I see them being useful in case someone is building a hfield scene in Unity and exports it for use with other MuJoCo sim options.

Exporting scenes with the editor window tool works, and multi hfield scenes work too.

@copybara-service copybara-service bot merged commit 3b44092 into google-deepmind:main Jan 15, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants