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.

Sign up for GitHub

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

Jump to bottom

Leave to YARP the pixel code (channel order) conversion aligned with OpenCV (BGRx) #44

Merged
merged 3 commits into from
Apr 17, 2022
Merged

Leave to YARP the pixel code (channel order) conversion aligned with OpenCV (BGRx) #44

merged 3 commits into from
Apr 17, 2022

Conversation

nunoguedelha
Copy link
Collaborator

Implements #43 .

@nunoguedelha nunoguedelha self-assigned this Apr 15, 2022
@nunoguedelha
Copy link
Collaborator Author

This is a first attempt to use yarp::sig::toCvMat function for converting the pixel code for aligning it with the default channel order used by OpenCV.

There is a problem with the definition of the map yarpCode2toCvMat: for adding another line with a different pixel code (e.g. yarp::sig::PixelMono), I would need to have a generic type x in the declaration of the map, which accepts both yarp::sig::PixelMono and yarp::sig::PixelRgb (like a "union"). Same applies to all the other lines in yarpCode2toCvMat still commented. Using a parent class of ImageOf, i.e. Image doesn't seem to work.

@nunoguedelha nunoguedelha mentioned this pull request Apr 15, 2022
Closed
@nunoguedelha
Use yarp::sig::Image::copy instead of cv::toCvMat which simplifie…
d8bd494 
…s the implementation

- This allows to avoid having to select a specialisation of `yarp::cv::toCvMat` depending on the pixel code, and instead, always use the specialization `yarp::cv::toCvMat<yarp::sig::PixelBgr>` respective to the destination image.
- The drawback is that we loose the alpha channel, as per the actual implementation of `yarp::sig::Image::copy`.
@nunoguedelha
Use PixelBgra code instead of PixelBgr accounting for eventual alpha …
cb1c162 
…channel
@nunoguedelha nunoguedelha marked this pull request as ready for review April 15, 2022 21:30
@nunoguedelha nunoguedelha requested a review from traversaro April 15, 2022 21:31
@nunoguedelha
Copy link
Collaborator Author

CC @S-Dafarra @RiccardoGrieco

@nunoguedelha
Copy link
Collaborator Author

There is a problem with the definition of the map yarpCode2toCvMat: for adding another line with a different pixel code (e.g. yarp::sig::PixelMono), I would need to have a generic type x in the declaration of the map, which accepts both yarp::sig::PixelMono and yarp::sig::PixelRgb (like a "union"). Same applies to all the other lines in yarpCode2toCvMat still commented. Using a parent class of ImageOf, i.e. Image doesn't seem to work.

This problem was avoided. Refer to #43 .

traversaro
traversaro approved these changes Apr 16, 2022
View reviewed changes
Copy link
Member

@traversaro traversaro left a comment

Choose a reason for hiding this comment

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

Cool!

@nunoguedelha nunoguedelha merged commit 08c9c2c into master Apr 17, 2022
@nunoguedelha
Copy link
Collaborator Author

Side note:
I selected "Squash and merge" but it kept the old commits and created a new merge commit, just as if I had chosen "Create new merge commit".
CC @traversaro

@nunoguedelha nunoguedelha deleted the refact/i43-use-yarp-toCvMat-pixel-code-conversion branch April 17, 2022 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@traversaro traversaro traversaro approved these changes

Assignees

@nunoguedelha nunoguedelha

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nunoguedelha @traversaro