Change how camera intrinsics are used for creation and update #1624
+118
−64
Conversation
This file contains 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
|
@pascal-roth here is that camera intrinsic PR I was talking about. |
jtigue-bdai
requested review from
Mayankm96,
jsmith-bdai,
kellyguo11,
pascal-roth and
Dhoeller19
as code owners
pascal-roth
reviewed
Jan 9, 2025
Comment on lines
+211
to
214
| The intrinsic matrix is used to set the following parameters to the USD camera: | ||
|
|
||
| - ``focal_length``: The focal length of the camera. | ||
| - ``horizontal_aperture``: The horizontal aperture of the camera. |
There was a problem hiding this comment.
This is not the case, isn't it? The focal_length is not set over the intrinsic matrix but just over the width. We should rephrase that to prevent confusion
jtigue-bdai
commented
Jan 15, 2025
| @@ -129,7 +130,8 @@ def from_intrinsic_matrix( | |||
| width: Width of the image (in pixels). | |||
| height: Height of the image (in pixels). | |||
| clipping_range: Near and far clipping distances (in m). Defaults to (0.01, 1e6). | |||
| focal_length: Perspective focal length (in cm). Defaults to 24.0 cm. | |||
| focal_length: Perspective focal length (in cm) used to calculate pixel size. Defaults to None. If None | |||
| focal_length will be calculated 1 / width. | |||
There was a problem hiding this comment.
@pascal-roth Sorry that was a mistype. pixel_size will be 1/width instead of calculated from focal_length. We actually shouldn't need focal length because the intrinsic matrix has all the info we need. I left focal length in there so it doesn't break backwards compatibility.
Suggested change
| focal_length will be calculated 1 / width. | |
| pixel_size will be calculated 1 / width. |
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.
Description
This PR changes how the camera intrinsic matrix:
Fixes # (issue)
Type of change
Screenshots
Please attach before and after screenshots of the change if applicable.
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there