upgrade to delaunator@3 #60
Conversation
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).
| <a href="#delaunay_outedges" name="delaunay_outedges">#</a> <i>delaunay</i>.<b>outedges</b> | ||
|
|
||
| The outgoing halfedge indexes as a Int32Array [*e0*, *e1*, *e2*, …]. For each point *i* on the convex hull, *outedges*[*i*] is the halfedge index *e* of the corresponding outgoing halfedge; for other points, the halfedge index is -1. The *outedges* table can be used to traverse the Delaunay triangulation; see also [*delaunay*.neighbors](#delaunay_neighbors). | ||
|
|
There was a problem hiding this comment.
Let's document hullIndex instead maybe?
There was a problem hiding this comment.
I don't think it should be in the public API:
- we have access to the hull if we need to reconstruct it, it's a linear op in time and memory
- we should use neighbors(i) anyway
(Maybe it should be private?)
There was a problem hiding this comment.
I don't know how to do that (ie: keeping this.hullIndex private) in a proper way.
There was a problem hiding this comment.
Ah don't worry about it, let's just leave it there without docs.
There was a problem hiding this comment.
I suggest either adding documentation for hullIndex to the README, or renaming it to _hullIndex to mark it clear that it’s private.
|
Have you considered making a variant of delaunator that uses a mutable api, like I suggested in this delaunator issue? TypedArray allocation tends to be a major bottleneck in these kinds of algorithms, so this is likely to significantly improve performance in Lloyd's Algorithm. Especially if we want to use more points! EDIT: Note, the linked comment contains a link to a gist with a changed |
In this PR I've only focused on bringing delaunator@3 to d3-delaunay. But your approach looks very promising! |
|
Makes sense - plus the proposed mutable variant is not officially part of delaunator yet. I might fork your notebook later to see if it is even worth it (I hope so, the various stippling notebooks could use a performance boost!) |
| <a href="#delaunay_outedges" name="delaunay_outedges">#</a> <i>delaunay</i>.<b>outedges</b> | ||
|
|
||
| The outgoing halfedge indexes as a Int32Array [*e0*, *e1*, *e2*, …]. For each point *i* on the convex hull, *outedges*[*i*] is the halfedge index *e* of the corresponding outgoing halfedge; for other points, the halfedge index is -1. The *outedges* table can be used to traverse the Delaunay triangulation; see also [*delaunay*.neighbors](#delaunay_neighbors). | ||
|
|
There was a problem hiding this comment.
I suggest either adding documentation for hullIndex to the README, or renaming it to _hullIndex to mark it clear that it’s private.
src/delaunay.js
Outdated
| const p = triangles[e % 3 === 2 ? e - 2 : e + 1]; | ||
| if (halfedges[e] === -1 || inedges[p] === -1) inedges[p] = e; | ||
| } | ||
| for (let i = 0; i < hull.length; ++i) { |
| for (let i = 0, h; i < hull.length; ++i) { | ||
| h = hull[i]; | ||
| context[i ? "lineTo" : "moveTo"](points[2 * h], points[2 * h + 1]); | ||
| } |
There was a problem hiding this comment.
I’d prefer the first context.moveTo broken out of the loop (so you don’t need to test i inside the loop). Also pull out n, and then const h = hull[i] * 2; inside the loop.
|
Superseded by #63. (The code is not different but I squashed the commit and did a bit of clean-up.) |
|
@Fil note that you can do this in existing PRs by doing |
|
Or alternatively, we could enable the "Squash and merge" option for PR merge button (currently only "Rebase" is available). Are there specific reasons not to @mbostock? |
fixes #50
Demos:
Breaking changes:
(supersedes #59)