[BUG FIX] Fix camera lookat ignored when entity_idx is set#2614
Open
Lidang-Jiang wants to merge 3 commits intoGenesis-Embodied-AI:mainfrom
Open
[BUG FIX] Fix camera lookat ignored when entity_idx is set#2614Lidang-Jiang wants to merge 3 commits intoGenesis-Embodied-AI:mainfrom
Lidang-Jiang wants to merge 3 commits intoGenesis-Embodied-AI:mainfrom
Conversation
When attaching a camera to an entity via entity_idx, the lookat and up parameters were silently ignored because move_to_attach() used trans_to_T(pos) which only sets translation, discarding orientation. Replace trans_to_T(pos) with pos_lookat_up_to_T(pos, lookat, up) to preserve the camera orientation defined by lookat/up parameters. Fixes Genesis-Embodied-AI#2591 Signed-off-by: Lidang-Jiang <lidangjiang@gmail.com>
duburcqa
reviewed
Mar 28, 2026
duburcqa
reviewed
Mar 28, 2026
duburcqa
reviewed
Mar 28, 2026
duburcqa
reviewed
Mar 28, 2026
- Merge 3 tests into single test_camera_lookat_entity - Remove comments, docstrings, and unused pos_lookat_up_to_T import - Add png_snapshot check for attached camera rendering Signed-off-by: Lidang-Jiang <lidangjiang@gmail.com>
duburcqa
reviewed
Mar 28, 2026
duburcqa
reviewed
Mar 28, 2026
duburcqa
reviewed
Mar 28, 2026
- Remove _blurred_kernel_size (not proven necessary) - Remove surface parameter from sphere entity - Use common_options dict + for loop for camera creation - Use for loop for pair-wise image assertions Signed-off-by: Lidang-Jiang <lidangjiang@gmail.com>
Collaborator
|
Please share the PR on HuggingFace here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
BaseCameraSensor.move_to_attach()silently ignoring thelookatandupparameters when a camera is attached to an entity viaentity_idx.trans_to_T(pos)(translation-only) withpos_lookat_up_to_T(pos, lookat, up)to preserve the full camera orientation.offset_T, and non-regression for detached cameras.Fixes #2591
Root cause
In
BaseCameraSensor.move_to_attach(), when no explicitoffset_Tis provided, the offset transform was computed usingtrans_to_T(pos), which only encodes the position as a translation matrix and discards the rotation derived fromlookat/up. This caused all entity-attached cameras to default to identity rotation regardless of theirlookattarget.Fix: One-line change from
trans_to_T(pos)topos_lookat_up_to_T(pos, lookat, up), which correctly constructs a 4x4 transform encoding both position and orientation.Snapshot update required
This fix changes the rendering output for
test_rasterizer_attached_batchedbecause the attached camera now correctly applies thelookatorientation instead of using identity rotation. The snapshot for this test needs to be regenerated by a maintainer with the reference hardware/GPU setup.Before fix (lookat ignored)
Two cameras attached to the same entity with different
lookattargets produce identical images (mean pixel: 102.3313for both). Thelookatparameter is silently ignored.After fix (lookat correctly applied)
Two cameras with different
lookattargets now produce different images (mean pixel: 20.3333vs50.6091), confirming thelookatparameter is correctly applied.Unit test results (6 passed, 7 skipped)
test_attached_camera_lookat_affects_orientation-- different lookat targets produce different imagestest_attached_camera_lookat_matches_offset_T-- lookat-derived transform matches equivalent explicit offset_Ttest_detached_camera_lookat_still_works-- non-regression: detached cameras still work correctlyTest plan
lookatproduce identical images (before fix)test_attached_camera_lookat_affects_orientation-- PASSEDtest_attached_camera_lookat_matches_offset_T-- PASSEDtest_detached_camera_lookat_still_works-- PASSED (non-regression)test_rasterizer_attached_batchedsnapshot needs regeneration by maintainer (expected due to corrected rendering)