sight-and-light
sight-and-light copied to clipboard
2 fixes: Vertical ray division by zero + Better parallelity check
Hi there.
First of all I'd like to thank you for an awesome tutorial! I love how I can examine and play with the drafts one by one when slowly approaching the final version.
When implementing the visibility script on my own, I found two things that could be enhanced.
1) Vertical ray causing a division by zero
I found out that there is a little bug in your code. The problem is that when you create a strictly vertical ray (r_dx = 0
), you are dividing by zero when calculating the T1
param of an intersection.
// Plug the value of T2 to get T1
T1 = (s_px+s_dx*T2-r_px)/r_dx
How I fixed the code myself is that when the r_dx
is approaching zero, I'm calculating the T1
param from the y
components instead:
T1 = (s_py+s_dy*T2-r_py)/r_dy
As r_dx
and r_dy
cannot be both 0 at the same time (otherwise it wouldn't be a ray, but a point), it fixes the problem quite well.
2) Faster parallelity check
Right now you're using square roots to check if the ray and line segment are parallel.
// Are they parallel? If so, no intersect
var r_mag = Math.sqrt(r_dx*r_dx+r_dy*r_dy);
var s_mag = Math.sqrt(s_dx*s_dx+s_dy*s_dy);
if(r_dx/r_mag==s_dx/s_mag && r_dy/r_mag==s_dy/s_mag){
// Unit vectors are the same.
return null;
}
I dunno how exactly that thing is working, but you don't really need to complicate it that much. Two lines are parallel if and only if their direction vectors are parallel. Vectors are parallel if one can be written as a k
multiple of the other one. Which means:
(a,b) is parallel to (x,y) <=> a/x == b/y <=> a*y == b*x
So an enhanced parallelity check that doesn't need calculating square roots is this:
// If lines are parallel
if (r_dx * s_dy == r_dy * s_dx) {
return null; // they do not intersect
}
@TomTomson, excellent!
seriously, this should really be merged. It's throwing errors for certain kinds of geometry (any with axially aligned bounding boxes for example.)
The author seems to not care about this tutorial. My issue #4 was also ignored completely and there was a PR long ago for division by zero too #2 .
Yet another fix for dividing by zero could be doing this to avoid relying on just x or just y:
T1 = (s_px + s_py+(s_dx + s_dy)*T2-(r_px + r_py))/(r_dx + r_dy)
EDIT: I think my idea causes some subtle bugs too, so the best way and without branching seems to be one from the PR #2 .