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

Triangles orientation are not counter clockwise and differences in code to Mapbox's delaunator #38

Closed
lightalchemist opened this issue Nov 23, 2021 · 1 comment

Comments

@lightalchemist
Copy link

lightalchemist commented Nov 23, 2021

The triangles produced by the code in the master branch seems to be oriented in clockwise order whereas Mapbox's delaunator library advertises the orientation produced by code to be counterclockwise. Since this is a port of the latter, and that the counterclockwise order is more typical, I was just wondering if this is a bug or is it intentional?

For e.g., running the binary compiled from basic.cpp using the test data

/* x0, y0, x1, y1, ... */
std::vector<double> coords = {-1, 1, 1, 1, 1, -1, -1, -1};

produces triangles

Triangle points: [[-1.000000, 1.000000], [1.000000, 1.000000], [1.000000, -1.000000]]
Triangle points: [[1.000000, -1.000000], [-1.000000, -1.000000], [-1.000000, 1.000000]]

The 3 points of each triangle for this case is clearly in a clockwise order.

I compared the code to the JS code in the Mapbox's repository and I noticed 2 key differences:

inline bool orient(
    const double px,
    const double py,
    const double qx,
    const double qy,
    const double rx,
    const double ry) {

    // Current code. This seems to be a bug
    // return (qy - py) * (rx - qx) - (qx - px) * (ry - qy) < 0.0; 

    // orient2dfast from https://github.com/mourner/robust-predicates/blob/master/src/orient2d.js
    // The above library appears to be a port of Prof Jonathan Shewchuk's robust predicate library
    // Also see Prof Shewchuk's page http://www.cs.cmu.edu/~quake/robust.html
    return ((py - ry) * (qx - rx) - (px - rx) * (qy - ry)) < 0.0;
}

Another difference is in the legalize method

            // edge swapped on the other side of the hull (rare); fix the halfedge reference
            if (hbl == INVALID_INDEX) {
                std::size_t e = hull_start;
                do {
                    if (hull_tri[e] == bl) {
                        hull_tri[e] = a;
                        break;
                    }

                    // Current code
                    // e = hull_next[e];

                    // Code in Mapbox's JS library
                    e = hull_prev[e];
                } while (e != hull_start);
            }

However, making the above 2 changes still did not fix the orientation.

Am I missing something? Or is the origin at the top-left corner instead of the bottom-left?

@lightalchemist
Copy link
Author

lightalchemist commented Nov 25, 2021

The changes to the method orient are unnecessary as on closer inspection they are equivalent.
The second difference concerning changing hull_next to hull_prev in legalize is addressed in #32 .
Lastly, the origin appears to be at the top-left corner. Closing as no changes needed.

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

1 participant