-
Notifications
You must be signed in to change notification settings - Fork 598
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
Rectify image if any distortion coefficients are non-zero #118
Conversation
error = cv::sum(diff_image)[0]; | ||
EXPECT_EQ(error, 0); | ||
|
||
// debug |
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.
This adds an extra dependency on highgui so I guess you can remove it right ?
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.
I can remove it and the #include <opencv2/highgui/highgui.hpp>
above.
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.
It's gone now.
looks good overall, please take the comments into account, that just makes less code to review :) And please squash your commits too once done. Thx. |
…xed comment wording slightly for pull request ros-perception#118
2c284a4
to
ce51a40
Compare
…stortion coefficients to see if rectification ought to be done
Made those changes, and squashed. |
} | ||
|
||
cv::Mat rectified_image; | ||
cv::Mat diff_image; |
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.
this is useless now right ?
…stortion coefficients to see if rectification ought to be done
Deleted unused diff image mat. |
Thx ! |
Rectify image if any distortion coefficients are non-zero
Thix closes #117 |
…ach-end-of-non-void Return default value to prevent missing return warning.
Unit tests included.
For #117