Skip to content

make .at type-independent#93

Open
ipa-rmb wants to merge 3 commits into
ipa320:indigo_devfrom
ipa-jcl-cs:indigo_dev
Open

make .at type-independent#93
ipa-rmb wants to merge 3 commits into
ipa320:indigo_devfrom
ipa-jcl-cs:indigo_dev

Conversation

@ipa-rmb

@ipa-rmb ipa-rmb commented Feb 1, 2019

Copy link
Copy Markdown
Contributor
  • fixes Extend image_flip to cope with other image formats #85
  • make .at independent of type and number of channels by creating following methods:
    template void setMatValuePtr(cv::Mat& image, int row, int index, double value);
    void setMatValuePtr(cv::Mat &image, int row, int index, double value);
    template T getMatValuePtr(cv::Mat& image, int row, int index);
    double getMatValuePtr(cv::Mat& image, int row, int index);
  • tried to take care about the intendation ;)

@ipa-rmb ipa-rmb left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Very good, please change the commented smaller items.
Did you test whether the functions with 90/180/270 deg rotations still work and whether they work with e.g. a gray scale single channel image and a color image?

Comment thread cob_image_flip/ros/src/image_flip.cpp Outdated
return true;
}

template <typename T> void ImageFlip::setMatValuePtr(cv::Mat& color_image, int u, int v, double value)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Instead of double value a T value would be more explicit about the usage of the type. If it does not cause problems, please change the function head accordingly, remove type conversion (T) inside the function and add the respective type in the function calls of the other setMatValuePtr function.

void disparityDisconnectCB(const ros::SingleSubscriberPublisher& pub);


template <typename T>void setMatValuePtr(cv::Mat& image, int row, int index, double value);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Brief comments on usage and meaning of the functions, please. It should be noted, e.g., how the row and index coordinates work. Maybe add an example how to compute the index for a multi-channel image (index = number_channels*u+channel_k).

Comment thread cob_image_flip/ros/src/image_flip.cpp Outdated
color_image.ptr<T>(u)[v] = T(value);
}

void ImageFlip::setMatValuePtr(cv::Mat& color_image, int u, int v, double value)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  1. The naming int u, int v is misleading for multi-channel images. Please use int row, int index instead as done in the header file.
  2. Indentation down to line 242 is still spaces and should be changed to tabs as the remainder of the file.

@fmessmer fmessmer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please rebase and finalize

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.

Extend image_flip to cope with other image formats

3 participants