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 option for orthographic projections to sim depth cameras #4

Merged
merged 2 commits into from
Jan 6, 2023

Conversation

thomascent
Copy link

Summary

When checking whether two objects overlap it's useful to be able to render their depth map using an orthographic projection. This PR adds that functionality to gl_depth_sim!

To do that, I just added an enum for whether the camera was a perspective or othrographic projection type and modified the projection matrix accordingly

@thomascent thomascent added the Minor Update New feature that doesn't impact the existing API. Should update the MINOR version. label May 18, 2022
@thomascent thomascent marked this pull request as ready for review May 18, 2022 12:53
Copy link

@Yuki-cpp Yuki-cpp left a comment

Choose a reason for hiding this comment

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

Prefer enum classes to straight up enums.
Additionally, setting up explicit values to enum fields isn't useful if those values don't have a specific meaning.

gl_depth_sim/include/gl_depth_sim/camera_properties.h Outdated Show resolved Hide resolved
gl_depth_sim/include/gl_depth_sim/camera_properties.h Outdated Show resolved Hide resolved
gl_depth_sim/src/gl_depth_sim/sim_depth_camera.cpp Outdated Show resolved Hide resolved
gl_depth_sim/src/gl_depth_sim/sim_depth_camera.cpp Outdated Show resolved Hide resolved
Co-authored-by: Leo <35804463+Yuki-cpp@users.noreply.github.com>
@thomascent
Copy link
Author

@Yuki-cpp , makes sense, thanks for the review!

@thomascent thomascent requested a review from Yuki-cpp May 19, 2022 02:44
Copy link
Member

@danielcranston danielcranston left a comment

Choose a reason for hiding this comment

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

Nice feature!

Can we extend the test (https://github.com/ascentai/gl_depth_sim/blob/master/gl_depth_sim/test/utest.cpp) in some way to cover this? I have no idea if those values are correct.

Also, it seems CI is not run on PRs?

@danielcranston danielcranston merged commit 526e07b into master Jan 6, 2023
@danielcranston danielcranston deleted the orthographic branch January 6, 2023 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Minor Update New feature that doesn't impact the existing API. Should update the MINOR version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants