-
Notifications
You must be signed in to change notification settings - Fork 166
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
Voronoi tessellation fails when four cells meet at a point. #15
Comments
Looking into this using your coordinates. Ok, using [-400,+400;-300,+300] for the bounding box I can reproduce the infinite loop. |
The same example, modified slightly: http://bl.ocks.org/mbostock/1402461a872def87504c/fd2052b3094994b789506d0f52aded4d225958ee (crashes) To simplify the test I moved all the points to positive x and y: var vertices = [
[488.000, 426.24997828655780],
[292.000, 239.89177832355410],
[335.500, 97.42857142857147],
[662.625, 97.42857142857143] /* y = 97.42857142857147 fixes crash */
]; And the bounding box is [[40, 40], [920, 460]]. |
Hmm it appears the beachsections making up the beachline for some reasons become unordered, which breaks everything. Now I have to find out why this happens. |
I’m just speculating here, but there are a few places that depend on exact match rather than using epsilon, such as within leftBreakPoint (e.g., rhill-voronoi-core.js:702). I tried using epsilon instead and it caused a different error, so my observation might not be helpful. :\ |
Problem is likely here: https://github.com/gorhill/Javascript-Voronoi/blob/master/rhill-voronoi-core.js#L720-L722 Using your original set of sites, at some point I get:
Which means:
This is the problem with parabola and finite precision arithmetic. This case is likely to happen when using sites which uses all the available arithmetic precision -- hence your nudge worked. Possible solutions:
Edit: Regarding not using epsilon in leftBreakPoint(), this is on purpose, because we want the most accurate result possible in this case -- approximation is undesirable (hence the bug here). |
I don't know if it is acceptable for you while I figure the best fix... In Voronoi.compute(), replacing:
with:
solved the issue. |
Here is a nice name to refer to this bug: "catastrophic cancellation" => http://docs.oracle.com/cd/E19957-01/806-3568/ncg_goldberg.html#700 |
Quantizing the inputs to ε sounds reasonable to me! Especially since it can be done before detecting coincident points, and thus points within some ε will also be considered coincident. |
In case you're looking for another test case exercising this precision bug, I've stumbled across another failure case which appears to be due to the same problem. Running the following with the latest version of the library throws the error var sites = [
{"x":168.77278407280934,"y":243.94568810816415},
{"x":243.77278407280934,"y":168.94568810816415},
{"x":311.2727840728093, "y":101.44568810816413}
],
bbox = {
"xl":86, "xr":384,
"yt":26,"yb":324};
var voronoi = new Voronoi();
voronoi.compute(sites, bbox); If I apply the quantization patch from above, it still fails, but now with the error |
Thanks for reporting, I will look into this tomorrow. |
FWIW, this error does not occur with the D3 port, as shown in bl.ocks.org/1402461a872def87504c: |
var sites = [
{"x":457.5781,"y":49.0705},
{"x":311.669,"y":194.9796},
{"x":386.669,"y":119.9796}
],
bbox = {"xl":232.5781,"xr":529.563,"yt":-25.9295,"yb":271.0554};
var voronoi = new Voronoi();
voronoi.quantizeSites(sites);
voronoi.compute(sites, bbox); Is another case that fails for me (whether or not I call quantizeSites). |
Ok, the "problem" here is the vertex is so far away that the arithmetic in Liang-Barsky breaks when it tries to clip:
Which causes Liang-Barsky's |
Best fix I can think of so far is to constraint "world" domain to
|
95bfbd5 doesn't fix it for me :/ I can confirm that your constraint to 1/epsilon is occurring, but I still get an error of |
Ugh, I made silly changes after testing. But then, fixing the silliness doesn't fix the issue if I use exactly your bbox (originally I just used the default one since I could reproduce the bug with it). |
Even when constraining vertices to within [1/ε,-1/ε] domain on both axes, the Liang-Barsky arithmetic can loose enough precision to fail to connect to one the edges of the bbox. |
I can get your last case to work using |
Ported the library to C# and getting the same error here. Seems to happen no matter what domain (large or small) I use. |
Is the C# port publicly available? Thanks!
|
It will be pretty soon. I'm testing it with Unity first and as soon as this bug is fixed, I'll make it available. |
Err, more... details? Which set of values are causing the errors? Are these values causing the same errors using this js version? I can't investigate this code here without any specifics. |
Sorry, I guess I was expecting you to magically fix all my problems. Please excuse my stupidity. I actually took the time to really understand the clipping method and turns out I only had to put Epsilon to a higher value (0.1 for example) to fit my particular site coordinates. If anyone's interested, the Unity port is here (wait for commit #4 though): https://github.com/jesta88/Unity-Voronoi Thanks for your great work. |
I still would like to know which set of points gave you problem, to find out where the algorithm fails. |
Improved Voronoi creation following gorhill/Javascript-Voronoi#15 Added selection of view. Added lines and styles. Styling for lines depending on depth. Added Explanation and JSon raw display. Use depth for div classes so it is possible to add the CSS definitions. Updated rendered view from filtered view.
I’m not sure the subject is an accurate description of when the error occurs, but this bug is a mirror of d3/d3#1578. A small example that produces the error is:
The expected result is:
However, it crashes when trying to close the cell because none of the four branches are taken to initialize the other end of the border edge.
The text was updated successfully, but these errors were encountered: