facebookresearch / habitat-sim

A flexible, high-performance 3D simulator for Embodied AI research.
https://aihabitat.org/
MIT License
2.45k stars 404 forks source link

--[Bugfix] - Compute AO cumulative bbox on creation unconditionally. #2420

Closed jturner65 closed 6 days ago

jturner65 commented 1 week ago

Motivation and Context

We've been computing AO bboxes during creation only if they were specified to use visuals based on link shapes. This PR changes this to always compute cumulative bboxes for loaded AOs. This was not a problem before due to simulator::reconfigure calling reset() which would recompute the cumulative bbox on the scene root node, which would hit the AOs as well, but any AOs that were loaded after reconfigure would be missed.

How Has This Been Tested

Local C++ and Python tests pass.

Types of changes

Checklist

jturner65 commented 6 days ago

This is because the links are not children of the root node on the SceneGraph. When no visual nodes are present, there isn't any content for the link AABB to include. I'm already WIP on writing the correct way to compute these AABBs for the full shape.

The additional code in this PR fixed the problem where newly-added AOs did not have their bboxes computed. (since the hot-loaded objects post-reconfigure were not getting the bbox calc performed)

EDIT: Due to the nature of how the links in AOs are stored in the scene graph, the solution presented in this PR is incomplete, as Alex said.

aclegg3 commented 6 days ago

I'm already WIP on writing the correct way to compute these AABBs for the full shape.

This is an additional change. I'll build on top of this improvement.