Rounded corners on arc produces NaNs due to floating point imprecision

[sftrabbit]

This line in src/arc.js assumes that the argument to Math.acos is between -1 and 1, which is not always true due to floating point imprecision, resulting in NaNs in the arc's path and preventing the arc from rendering.

kc = 1 / Math.sin(Math.acos((ax * bx + ay * by) / (Math.sqrt(ax * ax + ay * ay) * Math.sqrt(bx * bx + by * by))) / 2),

I have a fix for this (#91), which introduces clamped version of acos, but I would also very much appreciate if this could also be fixed in d3 v3 (where there is already a d3_acos function to use). I'm not sure if pull requests are being accepted on v3.

[mbostock]

Fixed in #91; thank you.

If you have time, I would appreciate a test to verify this fix as I am unable to reproduce it.

[sftrabbit]

@mbostock Great!

Regarding the test case: unfortunately, because we're currently using v3 (and I'm now going to push moving to v4) and some of the maths above this line has changed and it's difficult to force floating point errors, I only have a case that produces 'NaN's in v3. However, the logic is of course the same - it's occurring because Math.acos isn't clamped. Here's the v3 test case:

d3.svg.arc()
  .innerRadius(230)
  .outerRadius(255)
  .cornerRadius(3)
  .startAngle(0.18479956785822316)
  .endAngle(3.3263922214480157)()

which results in 'MNaN,NaNANaN,NaN 0 1,1 NaN,NaNLNaN,NaNANaN,NaN 0 0,1 NaN,NaNZ'.

[mbostock]

It’s possible this could not have occurred in v4 anyway because it uses epsilon to test for a number of boundary conditions. (Here, the angle of the arc is 3.3263922214480157 - 0.18479956785822316 = 3.1415926535897927 which is approximately π.)

[sftrabbit]

Ah apologies - so either my PR was unnecessary, or the epsilon just happened to fix my case and there are other cases that would be a problem. I suppose it's better to be safe than sorry though, in case there's any possibility (ax * bx + ay * by) / (Math.sqrt(ax * ax + ay * ay) * Math.sqrt(bx * bx + by * by)) falls outside [-1, 1].

[mbostock]

Yep, happy to have this fix. Thank you!