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

Opencv 3 compatibility #564

Merged
merged 4 commits into from
Jul 15, 2020
Merged

Opencv 3 compatibility #564

merged 4 commits into from
Jul 15, 2020

Conversation

clalancette
Copy link
Contributor

This PR reinstates OpenCV 3 compatibility.

While Foxy only supports Ubuntu 20.04 (and hence OpenCV 4), we still strive to maintain Ubuntu 18.04 (which has OpenCV 3). In this case, it is trivial to keep keep image_pipeline working with OpenCV 3, so reintroduce compatibility with it.

Along with this, remove the dependency on GTK3. It fails on Ubuntu 18.04, but it actually doesn't matter since there is no usage of the GTK API anywhere in the code.

It is no longer used at all in image_view.

Signed-off-by: Chris Lalancette <[email protected]>
While Foxy only supports Ubuntu 20.04 (and hence OpenCV 4),
we still strive to maintain Ubuntu 18.04 (which has OpenCV 3).
In this case, it is trivial to keep keep image_pipeline working
with OpenCV 3, so reintroduce compatibility with it.

Signed-off-by: Chris Lalancette <[email protected]>
@@ -46,7 +46,13 @@ void debayer2x2toBGR(
int R, int G1, int G2, int B)
{
typedef cv::Vec<T, 3> DstPixel; // 8- or 16-bit BGR
dst.create(src.rows / 2, src.cols / 2, cv::traits::Type<DstPixel>::value);
#if CV_VERSION_MAJOR >= 3 && CV_VERSION_MINOR >= 2
Copy link
Contributor

@dirk-thomas dirk-thomas Jul 15, 2020

Choose a reason for hiding this comment

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

This condition looks wrong. Version 4.0 / 4.1 would evaluate to FALSE and 4.2 would evaluate to TRUE.

Since on a CMake level we already assert 3.2 or higher I would swap the blocks and just check for CV_VERSION_MAJOR >= 4.

Signed-off-by: Chris Lalancette <[email protected]>
Signed-off-by: Chris Lalancette <[email protected]>
Copy link
Contributor

@dirk-thomas dirk-thomas left a comment

Choose a reason for hiding this comment

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

The build failure on the Jenkins builds is the same as already present on the target branch.

@clalancette
Copy link
Contributor Author

CI failed because camera_calibration test failed. It looks like a fix for that is here: ros-perception/vision_opencv#342 . But its not released yet. I'll see about getting a release done for it.

Suffice it to say, this PR didn't cause that particular failure, so I think this is good to go.

Copy link
Collaborator

@JWhitleyWork JWhitleyWork left a comment

Choose a reason for hiding this comment

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

LGTM too.

@JWhitleyWork JWhitleyWork merged commit 2e0c6d4 into ros-perception:ros2 Jul 15, 2020
@clalancette
Copy link
Contributor Author

Thanks everyone!

@clalancette clalancette deleted the opencv-3-compatibility branch July 15, 2020 21:34
JWhitleyWork pushed a commit that referenced this pull request Jul 15, 2020
* Remove GTK from image_view.

It is no longer used at all in image_view.

Signed-off-by: Chris Lalancette <[email protected]>

* Reinstate OpenCV 3 compatibility.

While Foxy only supports Ubuntu 20.04 (and hence OpenCV 4),
we still strive to maintain Ubuntu 18.04 (which has OpenCV 3).
In this case, it is trivial to keep keep image_pipeline working
with OpenCV 3, so reintroduce compatibility with it.

Signed-off-by: Chris Lalancette <[email protected]>

* Fixes from review.

Signed-off-by: Chris Lalancette <[email protected]>

* One more fix.

Signed-off-by: Chris Lalancette <[email protected]>
JWhitleyWork pushed a commit that referenced this pull request Jul 15, 2020
* Remove GTK from image_view.

It is no longer used at all in image_view.

Signed-off-by: Chris Lalancette <[email protected]>

* Reinstate OpenCV 3 compatibility.

While Foxy only supports Ubuntu 20.04 (and hence OpenCV 4),
we still strive to maintain Ubuntu 18.04 (which has OpenCV 3).
In this case, it is trivial to keep keep image_pipeline working
with OpenCV 3, so reintroduce compatibility with it.

Signed-off-by: Chris Lalancette <[email protected]>

* Fixes from review.

Signed-off-by: Chris Lalancette <[email protected]>

* One more fix.

Signed-off-by: Chris Lalancette <[email protected]>

Co-authored-by: Chris Lalancette <[email protected]>
@JWhitleyWork
Copy link
Collaborator

JWhitleyWork commented Jul 15, 2020

@clalancette Are you doing the new release with this change or do you want me to?

Edit: Same question for Dashing.

@clalancette
Copy link
Contributor Author

@clalancette Are you doing the new release with this change or do you want me to?

Edit: Same question for Dashing.

It would be great if you could. Thanks.

@JWhitleyWork
Copy link
Collaborator

Foxy: ros/rosdistro#25958
Dashing: ros/rosdistro#25959

@clalancette
Copy link
Contributor Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants