Skip to content

Clean Imgproc module #25001

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

Open
vpisarev opened this issue Feb 12, 2024 · 7 comments
Open

Clean Imgproc module #25001

vpisarev opened this issue Feb 12, 2024 · 7 comments
Labels
cleanup Code cleanup (e.g, drop legacy C-API, legacy unmaintained code) feature
Milestone

Comments

@vpisarev
Copy link
Contributor

vpisarev commented Feb 12, 2024

Describe the feature and motivation

Image processing (imgproc) is one of OpenCV flagship modules, actively used by many people, even in deep-learning era. Most of the stuff it contains is quite useful, but there is some old, obsolete functionality.

Let's clean it up a bit in OpenCV5. Namely:

  • Move old corner detectors to opencv_contrib/ximgproc: cornerMinEigenVal, cornerHarris, cornerEigenValsAndVecs, preCornerDetect. Those functions are all well replaced with goodFeaturesToTrack.
  • Move accumulate*() to opencv_contrib (or to some compatibility header). accumulate() and accumulateWeighted() are already covered by add() and addWeighted(). The other 2 can be replaced with slightly less efficient combination of 2 functions: multiply() and add(). There is no much use for those functions currently.
  • move EMD (Earth mover distance) to opencv_contrib.
  • possibly remove compeltely pyrMeanShiftFiltering. It's very old algorithm for image segmentation that gives very poor quality by today's standards.
  • move moments() and HuMoments() to opencv_contrib.
  • move HoughLines (all variations), HoughCircles(), GeneralizedHoughTransform and LSD (line segment detector) to opencv_contrib. Currently there are significantly, incomparably better deep learning methods to detect lines, circles and other simple objects that with proper optimizations and some tricks can be made as efficient as the traditional methods (or even more efficient when using dedicated accelerators). See https://arxiv.org/pdf/2003.04676.pdf for example
  • not to remove, but we need to normalize color conversion API. The new cvtColor() should take 2 color spaces, input and output, and choose the proper conversion algorithm. It should cover different variations of YUV color space, including subsampled options like 4:2:0, 4:2:2 etc. Color space conversion should also be able to convert the result to proper data type (e.g. FP16 or FP32) with or without scaling. All steps of a complicated color space transformation can likely be fused into a single parallel loop. See also New shiny Imgproc module for OpenCV 5.0 #25012

Additional context

No response

@vpisarev vpisarev added feature cleanup Code cleanup (e.g, drop legacy C-API, legacy unmaintained code) labels Feb 12, 2024
@vpisarev vpisarev added this to the 5.0 milestone Feb 12, 2024
@vpisarev vpisarev mentioned this issue Feb 12, 2024
11 tasks
@crackwitz
Copy link
Contributor

crackwitz commented Feb 12, 2024

May I also suggest completely redoing all the drawing primitive calls?

  • cv::line() doesn't produce lines that are anywhere near the requested thickness

  • Circles are lumpy, as seen in cv::getStructuringElement()

  • No way to draw flat-shaded or textured triangles/polygons in 2D or 3D-orthogonal projection. At least drawing a single textured triangle should be available, without hacks that involve warpAffine or warpPerspective and leave hairline artefacts.

  • No core facility to draw 3D perspective geometry, such as wireframes, single or multiple flat/textured triangles with Z-buffer, perhaps into a Mat that already contains data. It needn't be high performance. It needs to be simple and accessible and always there. All the existing "3D drawing" facilities require external dependencies that don't come with the official opencv-python packages.

  • Drawing calls do support sub-pixel coordinates via shift argument, but I find that inconvenient. There should be overloads that take float scalars or float-typed Mats/vectors/numpy arrays.

@vpisarev
Copy link
Contributor Author

vpisarev commented Feb 12, 2024

@crackwitz, thanks! I have a better implementation of a part of the drawing functions based on the supersampling idea, hopefully that would solve some of the problems.

for 3D visualization we have a pending PR with a lot of new stuff: #20371. Any help in finalizing that PR is greatly appreciated.

regarding overloaded functions with subpixel-accurate coordinates as float's - yes, that should be rather easy to implement.

There should be several subsequent feature requests to improve, not just cleanup, imgproc. They will touch batch processing, parallel implementation of filters etc. You are welcome to submit "drawing" stuff related feature request(s)

@Dhanwanth1803
Copy link
Contributor

@vpisarev I would like to work on this. Can I?

@Dhanwanth1803
Copy link
Contributor

@dkurt Can I work with you guys in this?

@savuor
Copy link
Contributor

savuor commented May 9, 2024

@crackwitz BTW some of that 3D rendering features are already implemented in 5.0: #24459
As for texture rendering, implementing it properly is a quite complicated task. But I successfully simulated it by using texture coordinates as colors and calling remap() afterwards.

@crackwitz
Copy link
Contributor

crackwitz commented May 9, 2024

I am glad for the contribution! Might check it out when I have time.

Doesn't have to be complicated if one simply used OpenGL/Vulkan/🤷‍♂️ for it. I don't think that's too much of a dependency, and can even be kept in an optional module for the optional dependency.

@hmaarrfk
Copy link
Contributor

I didnt see any explicit note about it, and it is somewhat of a subtle point so I wanted to ask:

any thoughts on including options for BT.709 vs BT.601 definitions of "grayscale" and YUV? It would be amazing if this option could be a "first class option" in this revamp!

I wrote up some of my findings in
#18813 (comment)

I would be happy to, given the time, write some documentation from time to time specifying that the current conversion is done with BT.601 so that we can update it as the option gets implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Code cleanup (e.g, drop legacy C-API, legacy unmaintained code) feature
Projects
None yet
Development

No branches or pull requests

6 participants