DGtal-team / DGtal

Digital Geometry Tools and Algorithm Library
https://dgtal.org
GNU Lesser General Public License v3.0
370 stars 115 forks source link

Bug in DGtal::functions::Hull2D::computeHullThickness #1629

Open aramadancs opened 2 years ago

aramadancs commented 2 years ago

The function returns 0 for a convex hull calculated using MelkmanConvexHull. I included the points of the convex hull directly.

#include <DGtal/helpers/StdDefs.h>
#include <DGtal/geometry/tools/Hull2DHelpers.h>

#include <algorithm>
#include <vector>

using namespace DGtal;
using namespace functions;

int main()
{
  std::vector<Z2i::RealPoint> hull {
    {804.56832227024199, -68.471176393526704},
    {804.96020257363512, -69.494933490400683},
    {805.35232247974727, -70.519316530915930},
    {825.15497274857830, -114.34711249674335},
    {828.67425867587508, -121.69593677670120},
    {829.05943325037651, -122.39546351563243},
    {829.42436256177677, -122.73833140123513},
    {827.82353937509288, -118.87280445059109},
    {811.06291230576301, -82.124832029926566},
    {806.83483124583609, -72.890457996792406},
    {804.92531970702396, -68.803808853026638},
    {804.55092650722997, -68.125792709291431},
  };

  std::cout << Hull2D::computeHullThickness(hull.begin(), hull.end(), Hull2D::HorizontalVerticalThickness) << std::endl;

  // just by rotating the vector it works fine.
  // so the problem is related to finding the first pair of edges to calculate the resThickness.
  // but this bug messes up the AlphaThickSegmentComputer calculations.

  std::rotate(hull.begin(), hull.begin() + 1, hull.end());
  std::cout << Hull2D::computeHullThickness(hull.begin(), hull.end(), Hull2D::HorizontalVerticalThickness) << std::endl;

  return 0;
}
dcoeurjo commented 2 years ago

Thx for the bug report ! Let me ping @troussil on this one.

aramadancs commented 2 years ago

This is a patch of a fix I was testing locally. I assume it's related to the tolerance of Pi comparison. Hull2DHelpers.il.patch.txt

troussil commented 2 years ago

Let me ping @kerautret in my turn! I think he wrote the part about thickness computation.

(Note however that MelkmanConvexHull is guaranteed to properly work only with integer points, but not with real points due to possible inexact float comparisons.)

dcoeurjo commented 2 years ago

👍 thx @troussil, I thought it was from you.

Looking at the patch, the angle computation is a bit weird for just a sign of a determinant (even if computed on real numbers). @kerautret ?

kerautret commented 2 years ago

Thanks for their report @aramadancs and the patch ! I look it asap

dcoeurjo commented 2 years ago

any update @kerautret ? thx

kerautret commented 2 years ago

Done https://github.com/DGtal-team/DGtal/pull/1647 with some updates and including the example in tests.