d3 / d3-hierarchy

2D layout algorithms for visualizing hierarchical data.
https://d3js.org/d3-hierarchy
ISC License
1.14k stars 314 forks source link

Seemingly non-weird ball collection throws an error in d3.packEnclose() #188

Closed esperanc closed 2 years ago

esperanc commented 2 years ago

The following set of balls throws an error:

let weirdBalls = [
  { x: 14.5, y: 48.5, r: 7.585 },
  { x: 9.5, y: 79.5, r: 2.585 },
  { x: 15.5, y: 73.5, r: 8.585 }
] 
d3.packEnclose(weirdBalls)

Note that these do not seem weird in the sense of having centers that are too close or too small/too big radii. I tracked down the error to function encloseBasis3, which has the following conditional assignment:

const r = -(A ? (B + Math.sqrt(B * B - 4 * A * C)) / (2 * A) : C / B);

For this particular data set, A == 4.440892098500626e-16, and my proposed solution is to test for a 'small enough' A :

const r = -(Math.abs(A) > 1e-10
      ? (B + Math.sqrt(B * B - 4 * A * C)) / (2 * A)
      : C / B);

This seems to work in a project which creates quite a few circles, but there is probably a better solution.

mbostock commented 2 years ago

Yes, that looks right to me. encloseBasis3 is generating a circle that doesn’t enclose the three circles (red); the fix you propose looks correct (blue).

Screen Shot 2022-04-02 at 5 12 14 PM

And just for completeness, none of the 2-basis will be correct here.

Screen Shot 2022-04-02 at 5 12 31 PM

https://observablehq.com/d/edc883f49ffd755d