5

I have an issue in graham scan algorithm when my list have a lot of points, but works every time fine with low amount of points. I made some screenshots :

working : (300 points) working

not working (5000 points) not working

angle calculation:

public static double angle(MyVector3D vec1, MyVector3D vec2)
{
    return Math.Atan2(vec2.Y - vec1.Y, vec2.X - vec1.X) * 180 / Math.PI;

}

sorting points by polar angle depend on max Y point :

//bubblesort
    private void sortList()
    {
        double temp = 0.0;
        MyVector3D tempVector = new MyVector3D();
        for (int i = 1; i < points.Count; i++)
        {
            for (int j = 1; j < points.Count - 1; j++)
            {
                if (angles[j] < angles[j + 1])
                {
                    temp = angles[j + 1];
                    tempVector = points[j + 1];
                    angles[j + 1] = angles[j];
                    points[j + 1] = points[j];
                    angles[j] = temp;
                    points[j] = tempVector;
                }
            }   
        }

ccw method :

private double ccw(MyVector3D vec1, MyVector3D vec2, MyVector3D vec3)
{
    // ccwTest = ((vec2.X - vec1.X) * (vec3.Y - vec1.Y)) - ((vec2.Y - vec1.Y) * (vec3.X - vec1.X));
    return ((vec2.X - vec1.X) * (vec3.Y - vec1.Y)) - ((vec2.Y - vec1.Y) * (vec3.X - vec1.X));
}

Graham scan algorithm :

for (int i = 2; i < points.Count; i++)
    {
        while (ccw(points[M - 1], points[M], points[i]) > 0)
        {
            if (M > 1)
            {
                points.RemoveAt(M);
                M -= 1;
                i--;
            }
            else if (i == points.Count - 1)
            {
                break;
            }

            else
            {
                i += 1;
            }
        }
        //goodPoints.Add(points[M]);
        //listBoxInfos.Items.Add("g" + (int)points[M].X + "," + (int)points[M].Y + "," + 0);
        //listBoxInfos.Items.Add("ccw" + ccwTest);
        M += 1;

    }

I really don't know why my program explode on 800+ points... It's hard to debug, because algorithm works very well with 300,400,500... points.

Ty for informations.

dbc
  • 104,963
  • 20
  • 228
  • 340
Spartan01
  • 66
  • 1
  • 6
  • What is `points` in bubblesort? Shouldn't it start with '0' instead of '1' in `for (...)` loops? – Lanorkin Aug 07 '14 at 19:23
  • It's a list of points on cavas (carry x,y,z(0) coordinates for every point). List of angles is a list of calculated angles between max Y point and others. When I sort the angles, I sort list of points also, then I replace points[0] with max Y point and point[index of max Y point) before graham scan starts. – Spartan01 Aug 07 '14 at 19:29
  • 1
    Looking at the wikipedia pseudocode, I see that you are using ccw>0 instead than ccw <= 0. What happens when ccw == 0? (from what I see you would get ccw == 0 when the 3 points are collinear, which seems to match the fact that your buggy points in the 5000 points image seems to have a 0degree angle with the other linked points) – kugans Aug 07 '14 at 21:16
  • When ccw == 0 the while loop does not execute, that means that the point remains in the list and is a part of convex hull. – Spartan01 Aug 07 '14 at 21:36
  • You're using ints? Doubles? What? – tmyklebu Aug 07 '14 at 22:41
  • 2
    Let points[M-1] be at the same coordinates as points[i]. Then knowing: ccw = ((vec2.X - vec1.X) * (vec3.Y - vec1.Y)) - ((vec2.Y - vec1.Y) * (vec3.X - vec1.X)) with points[M-1] corresponding to vec1 and points[i] corresponding to vec3. Replacing vec3 by vec1 we get: ccw = ((vec2.X - vec1.X) * (vec1.Y - vec1.Y)) - ((vec2.Y - vec1.Y) * (vec1.X - vec1.X)) We can clearly see that this will be == 0. Since you affirmed that if ccw == 0 the points will be kept in the convex hull then this will make part of your hull be two perfectly overlapping lines unless I'm mistaken somewhere. – kugans Aug 08 '14 at 03:07
  • Thanks. I checked the algorithm and you're right kugans. Problem is when ccw==0, but the main problem is how to eliminate points with ccw==0 which are not part of the convex hull and keep those which are, because 3 points in same vector could be part of convex hull or not. Any idea how to fix that ? – Spartan01 Aug 08 '14 at 06:55

3 Answers3

6

The algorithm on Wikipedia is broken. It doesn't handle the case where multiple points are collinear with each other and with the minimum point. For instance, the following test case will fail:

        Vector3[] points3 = new Vector3[] 
        {
            new Vector3( 1, 1, 0),
            new Vector3( 5, 5, 0),
            new Vector3( 3, 3, 0),
            new Vector3( 2, 2, 0),
            new Vector3( 1, 1, 0),
            new Vector3( 1, 10, 0),

        };

The issue is that, when marching along the points, it may be necessary to discard the current point rather than either extending the hull or replacing the last point on the hull, if the point is between the last two points in the hull. (This could only happen if the points were also collinear with the minimum point, otherwise the preceding angle sorting would prevent such double-backs.) There's no logic for that in the psuedocode shown.

I also think the Wikipedia algorithm may be handling floating point roundoff errors badly. In particular checking ccw <= 0 looks problematic.

Here's an attempt to clean up the Wikipedia algorithm. I had to get rid of the (vaguely kludgy) "sentinal point" because, in case all the points are horizontally aligned, it would get chosen essentially randomly:

    public static IList<Vector3> GrahamScanCompute(IList<Vector3> initialPoints)
    {
        if (initialPoints.Count < 2)
            return initialPoints.ToList();

        // Find point with minimum y; if more than one, minimize x also.
        int iMin = Enumerable.Range(0, initialPoints.Count).Aggregate((jMin, jCur) =>
        {
            if (initialPoints[jCur].Y < initialPoints[jMin].Y)
                return jCur;
            if (initialPoints[jCur].Y > initialPoints[jMin].Y)
                return jMin;
            if (initialPoints[jCur].X < initialPoints[jMin].X)
                return jCur;
            return jMin;
        });
        // Sort them by polar angle from iMin, 
        var sortQuery = Enumerable.Range(0, initialPoints.Count)
            .Where((i) => (i != iMin)) // Skip the min point
            .Select((i) => new KeyValuePair<double, Vector3>(Math.Atan2(initialPoints[i].Y - initialPoints[iMin].Y, initialPoints[i].X - initialPoints[iMin].X), initialPoints[i]))
            .OrderBy((pair) => pair.Key)
            .Select((pair) => pair.Value);
        List<Vector3> points = new List<Vector3>(initialPoints.Count);
        points.Add(initialPoints[iMin]);     // Add minimum point
        points.AddRange(sortQuery);          // Add the sorted points.

        int M = 0;
        for (int i = 1, N = points.Count; i < N; i++)
        {
            bool keepNewPoint = true;
            if (M == 0)
            {
                // Find at least one point not coincident with points[0]
                keepNewPoint = !NearlyEqual(points[0], points[i]);
            }
            else
            {
                while (true)
                {
                    var flag = WhichToRemoveFromBoundary(points[M - 1], points[M], points[i]);
                    if (flag == RemovalFlag.None)
                        break;
                    else if (flag == RemovalFlag.MidPoint)
                    {
                        if (M > 0)
                            M--;
                        if (M == 0)
                            break;
                    }
                    else if (flag == RemovalFlag.EndPoint)
                    {
                        keepNewPoint = false;
                        break;
                    }
                    else
                        throw new Exception("Unknown RemovalFlag");
                }
            }
            if (keepNewPoint)
            {
                M++;
                Swap(points, M, i);
            }
        }
        // points[M] is now the last point in the boundary.  Remove the remainder.
        points.RemoveRange(M + 1, points.Count - M - 1);
        return points;
    }

    static void Swap<T>(IList<T> list, int i, int j)
    {
        if (i != j)
        {
            T temp = list[i];
            list[i] = list[j];
            list[j] = temp;
        }
    }

    public static double RelativeTolerance { get { return 1e-10; } }

    public static bool NearlyEqual(Vector3 a, Vector3 b)
    {
        return NearlyEqual(a.X, b.X) && NearlyEqual(a.Y, b.Y);
    }

    public static bool NearlyEqual(double a, double b)
    {
        return NearlyEqual(a, b, RelativeTolerance);
    }

    public static bool NearlyEqual(double a, double b, double epsilon)
    {
        // See here: http://floating-point-gui.de/errors/comparison/
        if (a == b)
        { // shortcut, handles infinities
            return true;
        }

        double absA = Math.Abs(a);
        double absB = Math.Abs(b);
        double diff = Math.Abs(a - b);
        double sum = absA + absB;
        if (diff < 4*double.Epsilon || sum < 4*double.Epsilon)
            // a or b is zero or both are extremely close to it
            // relative error is less meaningful here
            return true;

        // use relative error
        return diff / (absA + absB) < epsilon;
    }

    static double CCW(Vector3 p1, Vector3 p2, Vector3 p3)
    {
        // Compute (p2 - p1) X (p3 - p1)
        double cross1 = (p2.X - p1.X) * (p3.Y - p1.Y);
        double cross2 = (p2.Y - p1.Y) * (p3.X - p1.X);
        if (NearlyEqual(cross1, cross2))
            return 0;
        return cross1 - cross2;
    }

    enum RemovalFlag
    {
        None,
        MidPoint,
        EndPoint
    };

    static RemovalFlag WhichToRemoveFromBoundary(Vector3 p1, Vector3 p2, Vector3 p3)
    {
        var cross = CCW(p1, p2, p3);
        if (cross < 0)
            // Remove p2
            return RemovalFlag.MidPoint;
        if (cross > 0)
            // Remove none.
            return RemovalFlag.None;
        // Check for being reversed using the dot product off the difference vectors.
        var dotp = (p3.X - p2.X) * (p2.X - p1.X) + (p3.Y - p2.Y) * (p2.Y - p1.Y);
        if (NearlyEqual(dotp, 0.0))
            // Remove p2
            return RemovalFlag.MidPoint;
        if (dotp < 0)
            // Remove p3
            return RemovalFlag.EndPoint;
        else
            // Remove p2
            return RemovalFlag.MidPoint;
    }

By the way, your algorithm is order n-squared in a couple places:

  1. The bubblesort is order O(N^2)
  2. Removing points while finding the hull rather than swapping them to the front of the list can be O(N^2) because all subsequent points have to be shifted down.

Let me know if it solves your problems, I've tested it a bit but not completely.

dbc
  • 104,963
  • 20
  • 228
  • 340
  • After some tests, it appears your code seems to fail for some of the points. Consider input points: 0]: {X = 11.581625 Y = -110.983437} [1]: {X = 11.1816254 Y = -108.983437} [2]: {X = 11.88781 Y = -113.115852} [3]: {X = 11.587204 Y = -111.015938} [4]: {X = 12.1884336 Y = -115.215759} [5]: {X = 11.88781 Y = -113.115845} [6]: {X = 12.5794077 Y = -116.863365} [7]: {X = 12.1794081 Y = -115.163368} [8]: {X = 13.0785418 Y = -118.855026} [9]: {X = 12.5785418 Y = -116.855026} [10]: {X = 13.534234 Y = -119.732178} [11]: {X = 13.034234 Y = -118.732178} – Steve Johnson Jan 13 '15 at 15:58
  • Output for the above set of points after the scanning using the code above we get: [0]: {X = 13.534234 Y = -119.732178} [1]: {X = 11.1816254 Y = -108.983437} [2]: {X = 12.1794081 Y = -115.163368} [3]: {X = 12.1884336 Y = -115.215759} [4]: {X = 12.5794077 Y = -116.863365} [5]: {X = 13.034234 Y = -118.732178} [6]: {X = 13.0785418 Y = -118.855026} – Steve Johnson Jan 13 '15 at 15:59
  • Another test case that seems to fail, consider input: [0]: {X = 10.4182844 Y = -111.21611} [1]: {X = 10.0190592 Y = -109.21595} [2]: {X = 10.712142 Y = -113.283806} [3]: {X = 10.4127483 Y = -111.183716} [4]: {X = 11.0115175 Y = -115.383896} [5]: {X = 10.712141 Y = -113.2838} [6]: {X = 11.4204569 Y = -117.136063} [7]: {X = 11.0213022 Y = -115.435867} [8]: {X = 11.9213 Y = -119.144341} [9]: {X = 11.4223957 Y = -117.144066} [10]: {X = 12.4652023 Y = -120.266693} [11]: {X = 11.9662571 Y = -119.266167} – Steve Johnson Jan 13 '15 at 15:59
  • The output for the above points after scanning is as follows: [0]: {X = 12.4652023 Y = -120.266693} [1]: {X = 10.0190592 Y = -109.21595} [2]: {X = 11.0115175 Y = -115.383896} [3]: {X = 11.0213022 Y = -115.435867} [4]: {X = 11.4204569 Y = -117.136063} [5]: {X = 11.4223957 Y = -117.144066} [6]: {X = 11.9213 Y = -119.144341} [7]: {X = 11.9662571 Y = -119.266167} – Steve Johnson Jan 13 '15 at 16:00
  • This screenshot below is a Visual representaion, Top Row coordinates are before scanning. Bottom Row coordinates are after scanning. http://earnestdev.com/universal/tessellation/Problem-Graham-Scan%20-%20Copy.jpg – Steve Johnson Jan 13 '15 at 16:02
  • @SteveJohnson - My results are the same as yours. for your first case, I'm seeing the following hull: http://i.stack.imgur.com/TXLO8.png. For your second case, http://i.stack.imgur.com/so5KD.png. I'm not sure I see the problem. – dbc Jan 13 '15 at 22:52
1

As per this: http://en.wikipedia.org/wiki/Graham_scan and others there are at least 2 issues with your Graham scan algorithm implementation, I think you're 'getting lucky' with the lower numbers:

1) you are incrementing i both in your outer for AND in your else, ie in general you are skipping testing every other point.

2) I'm not convinced by your approach of removing failed points, yes the point is not on the hull 'here' but could be a point on the hull further along, you need to move to swapping these points down or using a stack based approach.

tolanj
  • 3,651
  • 16
  • 30
1

I guess this is getting outside the scope of the comments section so I'll make an answer out of it:

Let points[M-1] be at the same coordinates as points[i].

Then knowing: ccw = ((vec2.X - vec1.X) * (vec3.Y - vec1.Y)) - ((vec2.Y - vec1.Y) * (vec3.X - vec1.X)) with points[M-1] corresponding to vec1 and points[i] corresponding to vec3.

Replacing vec3 by vec1 we get: ccw = ((vec2.X - vec1.X) * (vec1.Y - vec1.Y)) - ((vec2.Y - vec1.Y) * (vec1.X - vec1.X)) We can clearly see that this will be == 0. Since you affirmed that if ccw == 0 the points will be kept in the convex hull then this will make part of your hull be two perfectly overlapping lines unless I'm mistaken somewhere.


Thanks. I checked the algorithm and you're right kugans. Problem is when ccw==0, but the main problem is how to eliminate points with ccw==0 which are not part of the convex hull and keep those which are, because 3 points in same vector could be part of convex hull or not. Any idea how to fix that ?

(you probably want to have a look at dbc's code but here's my answer)

I don't think you want to keep any link with ccw==0 in your convex hull. When ccw==0 you either have a 180deg or 0deg angle between your vectors.

The 0deg case is like what I was talking about at the beginning of my answer (the points don't actually need to overlap, it was easier to demonstrate it that way).

As for the 180deg case, you'll need to make some extra code. I'll quote dbc:

it may be necessary to discard the current point rather than either extending the hull or replacing the last point on the hull, if the point is between the last two points in the hull

If you want to make testing easier you could also extract the position of all the convex hull points and replay with just those.

kugans
  • 629
  • 6
  • 7