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

Upgrade to delaunator@3. #50

Closed
JobLeonard opened this issue Oct 24, 2018 · 8 comments
Closed

Upgrade to delaunator@3. #50

JobLeonard opened this issue Oct 24, 2018 · 8 comments
Labels
enhancement New feature or request

Comments

@JobLeonard
Copy link

More performance!

See also #49 ;)

@mbostock
Copy link
Member

Yes, I would like to do this, I just haven’t found the time.

@JobLeonard
Copy link
Author

Well, I'm currently looking into it myself, so maybe I can help! I can't promise I'll figure it out though. For now I'm experimenting with it via this notebook instead of cloning the repo.

Here's what I figured out so far:

As-is, it looks like v3 actually discards required information after construction, aside from the changes in logic, so cannot be used. However, if the updateable version I requested is implemented, this becomes possible again (on that note, I guess I should open an issue with a similar request here, or the ability to update won't be used in practice).

That still leaves figuring out the move from the old to the new logic.

Relevant bits of constructor code below

Delaunator V2

    constructor(coords) {

        /* ... */

        // initialize a circular doubly-linked list that will hold an advancing convex hull
        let e = this.hull = insertNode(coords, i0);
        this._hashEdge(e);
        e.t = 0;
        e = insertNode(coords, i1, e);
        this._hashEdge(e);
        e.t = 1;
        e = insertNode(coords, i2, e);
        this._hashEdge(e);
        e.t = 2;

        /* ... */

    }

Where insertNode is:

// create a new node in a doubly linked list
function insertNode(coords, i, prev) {
    const node = {
        i,
        x: coords[2 * i],
        y: coords[2 * i + 1],
        t: 0,
        prev: null,
        next: null,
        removed: false
    };

    if (!prev) {
        node.prev = node;
        node.next = node;

    } else {
        node.next = prev.next;
        node.prev = prev;
        prev.next.prev = node;
        prev.next = node;
    }
    return node;
}

Delaunator V3:

    constructor(coords) {

        /* ... */

        const hullPrev = this.hullPrev = new Uint32Array(n); // edge to prev edge
        const hullNext = this.hullNext = new Uint32Array(n); // edge to next edge
        const hullTri = this.hullTri = new Uint32Array(n); // edge to adjacent triangle

        /* ... */

        // set up the seed triangle as the starting hull
        this.hullStart = i0;
        let hullSize = 3;

        hullNext[i0] = hullPrev[i2] = i1;
        hullNext[i1] = hullPrev[i0] = i2;
        hullNext[i2] = hullPrev[i1] = i0;

        hullTri[i0] = 0;
        hullTri[i1] = 1;
        hullTri[i2] = 2;

        hullHash[this._hashKey(i0x, i0y)] = i0;
        hullHash[this._hashKey(i1x, i1y)] = i1;
        hullHash[this._hashKey(i2x, i2y)] = i2;

        /* ... */

        this.hullPrev = this.hullNext = this.hullTri = null; // get rid of temporary arrays

        /* ... */

    }
@mourner
Copy link
Collaborator

mourner commented Oct 24, 2018

@JobLeonard you only need the delaunator.hull array, which is generated at the end of the constructor:

https://github.com/mapbox/delaunator/blob/c20ef21ef78b0dd9689aa13070010f8a3b5c5a51/index.js#L218-L222

@JobLeonard
Copy link
Author

I feel a bit like an idiot for not noticing that you even explain how to consruct a Voronoi in the explanation page:

https://mapbox.github.io/delaunator/

@mbostock
Copy link
Member

mbostock commented Nov 3, 2018

I’ve tried a few times to upgrade this library to Delaunator v3, and my brain just can’t keep track of the different meaning of the indexes to get it working. Like, the old code:

// For points on the hull, index both the incoming and outgoing halfedges.
let node0, node1 = hull;
do {
  node0 = node1, node1 = node1.next;
  inedges[node1.i] = node0.t;
  outedges[node0.i] = node1.t;
} while (node1 !== hull);

I know I can iterate over the hull array, but I can’t figure out how to compute the equivalents to node.i and node.t in the new data structure.

for (let i = 0, h0, h1 = hull[hull.length - 1]; i < hull.length; ++i) {
  h0 = h1, h1 = hull[i];
  inedges[???] = ???;
  outedges[???] = ???;
}
@mbostock
Copy link
Member

mbostock commented Nov 3, 2018

In particular, I think the new hull data structure doesn’t tell me the halfedge indexes of the convex hull—just the point indexes. So, I don’t think there’s an easy way to initialize inedges[i] with the halfedge index of the hull edge for a point i on the hull.

@mourner
Copy link
Collaborator

mourner commented Nov 3, 2018

All right, will look into this...

@mourner mourner added the enhancement New feature or request label Dec 17, 2018
@mbostock mbostock changed the title Request: upgrade to Delaunator v3 Mar 17, 2019
Fil added a commit that referenced this issue May 21, 2019
fixes #50

note: I had to disable the tests for outeredges, because I'm not using the same outeredges (but they do point to the same nodes).
@Fil
Copy link
Member

Fil commented May 21, 2019

Please test PR #59.

Fil added a commit that referenced this issue Jun 19, 2019
Fix Delaunay and Voronoi for degenerate cases

For 0, 1, 2 points, and for collinear points:
- returns the correct topology (neighbors, find)
- returns the correct Voronoi cells (bounding box, half plane…)
- resorts to deterministic jittering only in the case of multiple collinear points

Fixes:
-  #19
-  #20
-  #50
-  #65
@Fil Fil closed this as completed in bbf0c8e Jun 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
4 participants