Skip to content
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

2 fixes: Vertical ray division by zero + Better parallelity check #3

Open
tomasiser opened this issue Jan 4, 2015 · 3 comments
Open

Comments

@tomasiser
Copy link

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
}
@idlesync
Copy link

@TomTomson, excellent!

@mreinstein
Copy link

seriously, this should really be merged. It's throwing errors for certain kinds of geometry (any with axially aligned bounding boxes for example.)

@FRex
Copy link

FRex commented Nov 8, 2017

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 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants