gorhill / Javascript-Voronoi

A Javascript implementation of Fortune's algorithm to compute Voronoi cells
http://www.raymondhill.net/voronoi/
Other
1.03k stars 166 forks source link

Voronoi Result missing an edge? #16

Closed oderby closed 10 years ago

oderby commented 10 years ago

Not sure if this is a bug or just an undocumented change, but I'm seeing the returned diagram missing the 4th edge for the middle cell in the following example using the latest version of the lib. Using an older version (can start digging to find out exactly which, but I'm guessing it was aad4ca6e97875873054e2ac11760bd8c725af4d0 as that was just before I started using the lib) did not have this problem.

var sites = [{x:2, y:5}, {x:5, y:5},{x:8, y:5}],
    bbox = {xl:0, xr:10, yt:0, yb:10};
var voronoi = new Voronoi();
var diagram = voronoi.compute(sites, bbox);
var polyBoundaries = diagram.cells.map(function(cell) {
    return cell.halfedges.map(function(edge) {
        return edge.getStartpoint();
    });
});
console.log(polyBoundaries);

which prints

[
    [
        {"x": 3.5, "y": 10},
        {"x": 3.5, "y": 0},
        {"x": 0, "y": 0},
        {"x": 0, "y": 10}
    ],
    [
        {"x": 3.5, "y": 0},
        {"x": 3.5, "y": 10},
        {"x": 6.5, "y": 10}
    ],
    [
        {"x": 6.5, "y": 0},
        {"x": 6.5, "y": 10},
        {"x": 10, "y": 10},
        {"x": 10, "y": 0}
    ]
]

The output I expect is

[
    [
        {"x": 0, "y": 0},
        {"x": 0, "y": 10},
        {"x": 3.5, "y": 10},
        {"x": 3.5, "y": 0}
    ],
    [
        {"x": 3.5, "y": 0},
        {"x": 3.5, "y": 10},
        {"x": 6.5, "y": 10},
        {"x": 6.5, "y": 0}
    ],
    [
        {"x": 6.5, "y": 0},
        {"x": 6.5, "y": 10},
        {"x": 10, "y": 10},
        {"x": 10, "y": 0}
    ]
]

If I should be using a different method to extract the points defining the boundary of the cells, let me know.

Note that adding another call to this.closeCells(bbox) at line 1687 properly adds the 4th edge, so I presume some corner case just got created or overlooked in a recent commit.

gorhill commented 10 years ago

You're right, in my fix to avoid infinite loop, I overlooked a possible last segment when walking along the bounding box, in the cases where a side of the bounding box has to be walked twice (at most), while my fix assumed it could be walked at most only once. Thanks for reporting.

gorhill commented 10 years ago

Nah, I have to revise my erroneous conclusion: I see it happening now with #15 fixed. Problem is bad assumption in the code: when closing a cell, the missing edges are not necessarily adjacent, which is what the code is currently assuming.

gorhill commented 10 years ago

Fixed with 95bfbd56272323626a3a517950d28e560810fc33