fitEllipse (Bugfix #3165)

Added by Torsten Mehlan about 4 years ago. Updated almost 5 years ago.

Status:Done Start date:2013-07-19
Priority:Normal Due date:
Assignee:Torsten Mehlan % Done:


Category:imgproc, video
Target version:Next Hackathon
Affected version:2.4.6 (latest release) Operating System:Any
Difficulty:Easy HW Platform:Any
Pull request:


The parameter min_eps seems to be choosen too large for some long contours. In some cases the variable "rp2" is quite small. Thus the check for condition "if( rp2 > min_eps )" prevents the code line "rp2 = sqrt(2.0 / rp2);" from being executed. This results in e very small value for the box width at the end of the fitting algorithm. Also the angle is computet wrongly.

std::vector<cv::Point> points;

points.push_back(cv::Point(618,   522));
points.push_back(cv::Point(102, 551));
points.push_back(cv::Point(1, 680));
points.push_back(cv::Point(351, 680));
points.push_back(cv::Point(920, 671));
points.push_back(cv::Point(1911, 664));
points.push_back(cv::Point(2421, 603));
points.push_back(cv::Point(1902, 571));
points.push_back(cv::Point(1116, 549));
points.push_back(cv::Point(897, 552));
cv::RotatedRect box = cv::fitEllipse(cv::Mat(points));
std::cout << "Box has width: " << box.size.width << " height: " << box.size.height << " angle: " << box.angle << std::endl;

Output of this code:
Box has width: 1.37751e-006 height: 145.668 angle: 0

Proposed solution:
Reduce the value of the constant min_eps. Currently it shows up like this: "const double min_eps = 1e-6;" Reduce ist to "const double min_eps = 1e-8;" if there are no further implications to the fitting algorithm.


Associated revisions

Revision 27ed32f8
Added by Steven Puttemans about 4 years ago

Applied bugfix #3165 : Changed min_eps value


Updated by Andrew Senin about 4 years ago

Hello Torsten,

Thank you for posting this PR. It seems this modification does not break the tests. But due to our resource limitations it does not seem possible to do it till the next hackathon. You can speed this up by sending a PR at github ( Usually discussions about ready to merge PR happen to be quicker.

  • Target version changed from 2.4.7 to Next Hackathon
  • Status changed from New to Open

Updated by Steven Puttemans about 4 years ago

I found spare time and didn't think it was worth to spend it useless.
So here is the fix :

Keep those small fixes comming, it is actually easy to transform them in pull request once you get the hang of it.

  • Assignee changed from Vadim Pisarevsky to Torsten Mehlan

Updated by Steven Puttemans almost 5 years ago

Merge complete, fix added!

  • Status changed from Open to Done

Also available in: Atom PDF