-
Notifications
You must be signed in to change notification settings - Fork 658
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
fix(autoware_image_projection_based_fusion): fix bug of fov checker in pointpainting #9637
base: main
Are you sure you want to change the base?
Conversation
Thank you for contributing to the Autoware project! 🚧 If your pull request is in progress, switch it to draft mode. Please ensure:
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9637 +/- ##
==========================================
+ Coverage 29.57% 29.61% +0.03%
==========================================
Files 1440 1441 +1
Lines 108348 108370 +22
Branches 41384 41389 +5
==========================================
+ Hits 32049 32089 +40
+ Misses 73190 73169 -21
- Partials 3109 3112 +3
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Taekjin LEE <[email protected]>
Signed-off-by: Taekjin LEE <[email protected]>
Signed-off-by: Taekjin LEE <[email protected]>
Signed-off-by: Taekjin LEE <[email protected]>
acd27c5
to
849e529
Compare
perception/autoware_image_projection_based_fusion/src/pointpainting_fusion/node.cpp
Outdated
Show resolved
Hide resolved
if (projected_point.x() < fov_left_.at(image_id)) continue; | ||
if (projected_point.x() > fov_right_.at(image_id)) continue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@technolojin Could you make some change to check fov before calcRawImageProjectedPoint
function to avoid unnecessary projection calculation for points out of FOV?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@badai-nguyen
We do not know until the lens distortion is corrected.
In the case the lens warps the view inside, the more points will be inside of FoV.
In another word, if it comes before the correction, it may miss some points on the edges.
In the case the camera model is the pinhole camera and do not have lens distortion, the previous process p_x > (tan_h_.at(image_id) * p_z)
will be done in the calcRawImageProjectedPoint
and it means it doubles the same process anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well... If the lens distortion is considered when the boundary is set, we may can cut some inefficient process.. let me consider it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved by 355c183
to check distortion effect, additional process is needed and will be implemented by following PRs
This reverts commit 849e529.
Description
When the point is projected to the image space, the point is checked if the point is in the filed-of-view of corresponding camera.
It may cut unnecessary calculation of later roi matching.
However, @a-maumau found that the current implementation of loading camera parameter is done at the initialization and do not get the camera info.
Therefore the checker of the FoV was not done correctly.
This PR will not induce any logic difference.
Special thanks to @a-maumau
Related links
Parent Issue:
How was this PR tested?
Visually confirmed that the given roi and the pointcloud class is corresponding.
Notes for reviewers
None.
Interface changes
None.
Effects on system behavior
None.