Add Fisheye model and Rename model class

[DavidTorresOcana]

This PR is motivated by ros-perception/image_pipeline#441

Introduces:

• Add FISHEYE camera model as EQUIDISTANT distortion model: Adds OpenCV 3+ fisheye (Kannala-Brandt) model
• Rename PinholeCameraModel to CameraModel: PinholeCameraModel is misleading. Rename the class and file to a more general name.
  • The class CameraModel does use the distortion model specified in the camera_info, including pinhole, equidistant and rational_polynomial

[furushchev]

@DavidTorresOcana Thanks for the contribution for supporting fisheye camera!
I'm not a maintainer of this repository, so just a comment: it looks better to me if PinholeCameraModel is left as it was without rename and mark it as deprecated instead, and then create another class named CameraModel which is generalized and supports fisheye camera model.
Anyway it is great to support fisheye camera model in ROS by default.
Without it, we always need to use non-standard packages..

[DavidTorresOcana]

@DavidTorresOcana Thanks for the contribution for supporting fisheye camera!
I'm not a maintainer of this repository, so just a comment: it looks better to me if PinholeCameraModel is left as it was without rename and mark it as deprecated instead, and then create another class named CameraModel which is generalized and supports fisheye camera model.
Anyway it is great to support fisheye camera model in ROS by default.
Without it, we always need to use non-standard packages..

Thank you for your comment.

I do not see where Pinhole model class would be used in the official stack but I added it back and a deprecated attribute to it. See f683d0a

[mintar]

All references to FISHEYE should be renamed to EQUIDISTANT, see my comment here:

ros/common_msgs#151 (comment)

That also removes the dependency on ros/common_msgs#151.

[DavidTorresOcana]

All references to FISHEYE should be renamed to EQUIDISTANT, see my comment here:

ros/common_msgs#151 (comment)

That also removes the dependency on ros/common_msgs#151.

Should this be ok?

I tested this with ros-perception/image_pipeline#441 and works ok.

Would be nice to merge into noetic and ROS2 too

[mintar]

Disclaimer: I'm not a maintainer of this repo, so my comments here are just my own personal opinion.

First off, thanks for this PR! There are many incomplete PRs that attempt to implement the fisheye model (e.g., Intermodalics#1, #184, #299), and this one seems to be the best so far. I'm currently in the process of reducing this PR to its essentials and testing it.

Some comments:

• The tests are failing. The travis test fails for unrelated reasons, but you should get the other ones working. For example this one: http://build.ros.org/job/Npr__vision_opencv__ubuntu_focal_amd64/26/console . Maybe it's the IMAGE_GEOMETRY_DECL that you added?
• Since you've renamed the PinholeCameraModel to CameraModel, this PR is almost impossible to review. The diff is just too large. IMO, it would be better to undo this renaming and try to get the diff as small as possible. Then maybe later do the renaming in a different PR, if you really must.
• It would be great if you could add tests for the fisheye model (optional).

[mintar]

Ok, I've dug a bit deeper. I think it's crucial to differentiate between the camera model and the distortion model. For instance, this kalibr wiki page lists the camera and distortion models supported by kalibr (note: "radial-tangential (radtan)" = plumb_bob). The current CameraInfo message only supports the pinhole model, and only allows to change the distortion model. Therefore, what this PR should attempt to do is only adding the equidistant distortion model, and keep the existing pinhole camera model. Therefore, the PinholeCameraModel class should not be renamed to CameraModel.

Next, I think the changes to the project3dToPixel function in this PR should be undone. That function should not take the distortion model into account at all; it's part of the camera model (here: pinhole). Here's the documentation:

vision_opencv/image_geometry/include/image_geometry/pinhole_camera_model.h

Lines 89 to 97 in 0a908b0

	/**
	* \brief Project a 3d point to rectified pixel coordinates.
	*
	* This is the inverse of projectPixelTo3dRay().
	*
	* \param xyz 3d point in the camera coordinate frame
	* \return (u,v) in rectified pixel coordinates
	*/
	cv::Point2d project3dToPixel(const cv::Point3d& xyz) const;

The important point is that the function should project the point to rectified pixel coordinates. I believe cv::fisheye::projectPoints also unrectifies the points, so it shouldn't be used here.

I believe to fully implement a new distortion model, the following functions (and only those) should be modified: rectifyPoint, rectifyImage, unrectifyPoint, unrectifyImage. The last one is not implemented, so that leaves the first three. Here's an overview of the PRs that I could find that implement the equidistant model:

function	#306	#184	#299	Intermodalics#1 (2)
rectifyPoint	yes	no	no	no
rectifyImage (1)	yes	yes	yes	yes
unrectifyPoint	yes	no	no	no

(1) implemented by modifying initRectificationMaps

(2) Intermodalics#1 not only implements the equidistant (aka "fisheye_cv") distortion model, but also the fov model. That would also be great to have here in a later PR.

The table above shows why I think this PR is still the most complete. I think I'll send a simplified PR once I'm done testing. Not that I have a lot of hopes that any PRs in this repo will be merged any time soon... :)

[DavidTorresOcana]

Closing since #358 was finally merged