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

--Add Panoptic/Instance ID support to Semantic sensor #2316

Merged
merged 19 commits into from
Feb 28, 2024

Conversation

jturner65
Copy link
Contributor

@jturner65 jturner65 commented Feb 5, 2024

Motivation and Context

This PR will add the ability to have the semantic sensor produce images containing object ID of the objects rendered per-pixel in the same way that it currently produces semantic ID observations.

Until this PR in habitat-lab is merged, this PR will fail on lab tests due to hard-coded references to the stage ID.

NOTE (side thesis): This PR also refactors many uses of Eigen vec classes to Magnum instead.

How Has This Been Tested

Locally c++ and python tests pass

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.

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Feb 5, 2024
@jturner65 jturner65 force-pushed the SmntcSnsr_InstanceSupport branch from bac2010 to 0160f2a Compare February 5, 2024 20:32
@jturner65 jturner65 requested a review from aclegg3 February 6, 2024 16:20
@jturner65 jturner65 force-pushed the SmntcSnsr_InstanceSupport branch 2 times, most recently from 772bc70 to be0eaaa Compare February 9, 2024 14:41
@jturner65 jturner65 changed the title --[WIP] Add Panoptic/Instance ID support to Semantic sensor --Add Panoptic/Instance ID support to Semantic sensor Feb 9, 2024
@jturner65 jturner65 marked this pull request as ready for review February 9, 2024 18:57
@jturner65 jturner65 force-pushed the SmntcSnsr_InstanceSupport branch 2 times, most recently from d49bd46 to 910f775 Compare February 15, 2024 14:05
@jturner65 jturner65 force-pushed the SmntcSnsr_InstanceSupport branch from 910f775 to 8b83cba Compare February 21, 2024 14:31
@jturner65 jturner65 force-pushed the SmntcSnsr_InstanceSupport branch from 9ffb427 to 13c1310 Compare February 26, 2024 17:26
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.

Quick comments

src/esp/physics/PhysicsManager.cpp Outdated Show resolved Hide resolved
src/esp/physics/PhysicsManager.h Outdated Show resolved Hide resolved
src/esp/physics/bullet/BulletPhysicsManager.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.

Overall good, but one more question about the subtree treatment of the semantic vector.

node->setSceneNodeTags(childNodeTags);
node->setSemanticIDVector(this->getSemanticIDVector());
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this accidentally copy the drawable_id to non-drawable sub nodes?

@jturner65 jturner65 merged commit 415f5ae into main Feb 28, 2024
10 checks passed
@jturner65 jturner65 deleted the SmntcSnsr_InstanceSupport branch February 28, 2024 20:55
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.

3 participants