mapbox / delaunator

An incredibly fast JavaScript library for Delaunay triangulation of 2D points
https://mapbox.github.io/delaunator/
ISC License
2.34k stars 141 forks source link

Support flat array input format #17

Closed redblobgames closed 6 years ago

redblobgames commented 6 years ago

My x,y points are in flat arrays (e.g. Float32Array), and I want to run Delaunator on them. I think I have two options:

  1. I construct a dummy array of indices [0, 1, 2, 3, …] and then pass in getX, getY accessors that use the index to look up the value. This feels silly because Delaunator is constructing the ids array which is exactly what I'm passing in.
  2. I convert my flat array to a nested array and don't use getX, getY accessors. This also feels silly because I'm converting my flat array to a nested array and then Delaunator is converting the nested array back to a flat array coords, which is exactly the same as what I started with.

Now, it may be that this doesn't really matter, and I should just convert the data to the format Delaunator needs. But I also wonder if there's a clean way to separate the input-conversion part of the Delaunator() function (ids, coords, points.length) from the part of the code that runs the algorithm, so that I could skip the conversion process.

mourner commented 6 years ago

This makes perfect sense. I'm seeing several options here:

  1. Handle flat array input as a special case in the Delaunator constructor. Probably the easiest option, although I'm not a big fan of polymorphic APIs.
  2. Drop the current point, getX, getY API in favor of just accepting the flat coordinates array. This moves conversion on the user side but seems more honest and minimalistic and fits the way Delaunator works, especially given that flat coords array is immutable.
  3. Change getX and getY signature from (p) => number to (points, index) => number. Flexible but may be not very convenient to use.

I'm inclined towards 2, although it would be a breaking change. cc @mbostock what do you think?

mbostock commented 6 years ago

I would also be inclined towards the second approach!

redblobgames commented 6 years ago

Javascript object constructors can forward to a different function, so you could add a second module export that takes the flat array, and then make the default module export perform the conversion and forward to the flat constructor. Something like this:

module.exports = DelaunatorSimple;
module.exports.raw = Delaunator;

function DelaunatorSimple(a) {
    console.log("Test " + a);
    return new Delaunator("converted:" + a);
}

function Delaunator(b) {
    console.log("Raw " + b);
    this.b = b;
}

Delaunator.prototype = {
    method(x) { console.log("Raw.method " + x); }
};
let Delaunator = require('./foo');

let oldInterface = new Delaunator("nestedarray");
oldInterface.method("old");

let newInterface = new Delaunator.raw("flatarray");
newInterface.method("new");

Existing code will continue to work, and weirdos like me who want to use the flat array constructor would call Delaunator.raw.

mourner commented 6 years ago

@redblobgames we could do that, but I think I still prefer the minimalistic "just accept flat arrays" approach. It forces users to use the most efficient input format, and the API surface is smaller so easier to maintain / reason about.

mbostock commented 6 years ago

I have proposed an implementation in #19. The constructor now takes a flat array:

var delaunator = new Delaunator(coords);

If you want the old behavior, you can use Delaunator.from:

var delaunator = Delaunator.from(points);