Skip to content
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

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

Open
aramadancs opened this issue Jan 20, 2022 · 7 comments
Open

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

aramadancs opened this issue Jan 20, 2022 · 7 comments
Assignees

Comments

@aramadancs
Copy link

aramadancs commented Jan 20, 2022

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
Copy link
Member

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

@aramadancs
Copy link
Author

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
Copy link
Member

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
Copy link
Member

👍 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 ?

@dcoeurjo dcoeurjo assigned kerautret and unassigned troussil Jan 21, 2022
@kerautret
Copy link
Member

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

@dcoeurjo
Copy link
Member

any update @kerautret ? thx

@kerautret
Copy link
Member

Done #1647 with some updates and including the example in tests.

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

No branches or pull requests

4 participants