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

AABB.intersectSegment returns NaN in hit collider response object #9

Open
mreinstein opened this issue Apr 27, 2020 · 4 comments
Open

Comments

@mreinstein
Copy link

my setup data:

pos = new Point(907.5, 318.5)
half = new Point(7.5, 17.5)

myAabb = new AABB(pos, half)

start = new Point(531, 301)
delta = new Point(16, 0)
result = myAabb.intersectSegment(start, delta)
console.log('result:', result)

/*
prints:

result: Hit {
  collider: AABB {
    pos: Point { x: 907.5, y: 318.5 },
    half: Point { x: 7.5, y: 17.5 }
  },
  pos: Point { x: NaN, y: NaN },
  delta: Point { x: NaN, y: NaN },
  normal: Point { x: 0, y: -1 },
  time: NaN
}
*/

Any ideas what's going on here? When I look at the internal implementation I see nearTimeY is being set to NaN

@mreinstein
Copy link
Author

@noonat I hate to be a pest but, any thoughts on this?

@mreinstein
Copy link
Author

@noonat I figured out the problem. It's possible for nearTimeY to get set to NaN when we attempt to multiply 0 * Infinity (results in NaN)

the fix in #10 is pretty simple, we just check to see if nearTimeY is set to NaN, and if so set it to Infinity

I also added a unit test to reproduce this problem.

@njlr
Copy link

njlr commented Nov 7, 2020

I also ran into this issue. Collision detections fail when delta.x == 0 or delta.y = 0. If they are non-zero then everything works perfectly.

The work-around is to set the near and far values to infinity in such cases. This can lead to false-positives ("ghost collisions") so you also need to add a broad-phase check.

Unfortunately @mreinstein's fix did not work for me; I think some edge-cases are missing.

I got things working in my own code-base. Here is a snippet, which isn't compatible with this repo, but hopefully gives the idea:

// .... 

  if delta.X = 0.0f
  then
    xEntry <- Single.NegativeInfinity
    xExit <- Single.PositiveInfinity
  else
    xEntry <- xInvEntry / delta.X
    xExit <- xInvExit / delta.X

  if delta.Y = 0.0f
  then
    yEntry <- Single.NegativeInfinity
    yExit <- Single.PositiveInfinity
  else
    yEntry <- yInvEntry / delta.Y
    yExit <- yInvExit / delta.Y

// ...

Note the + and - infinity, the difference is important due to comparison checks later.

And this is the extra broad-phase check that removes ghost collisions:

let computeBroadphaseBox (delta : Vector2) (box : AABB) =
  // TODO: Make this more efficient
  let nextBox = { box with Center = box.Center + delta }

  let l = min (AABB.left box) (AABB.left nextBox)
  let r = max (AABB.right box) (AABB.right nextBox)
  let t = min (AABB.top box) (AABB.top nextBox)
  let b = max (AABB.bottom box) (AABB.bottom nextBox)

  {
    Center = box.Center + delta * 0.5f
    HalfSize = Vector2.create (r - l) (b - t) * 0.5f
  }

let sweepAABBWithBroadphase (dynamicBox : AABB) (staticBox : AABB) (delta : Vector2) : SweepResult option =
  let broadphaseBox = computeBroadphaseBox delta dynamicBox

  if aabbCheck broadphaseBox staticBox
  then
    sweepAABB dynamicBox staticBox delta
  else
    None

@mreinstein
Copy link
Author

Unfortunately @mreinstein's fix did not work for me; I think some edge-cases are missing.
I got things working in my own code-base. Here is a snippet, which isn't compatible with this repo

@njlr thanks for providing more details! ❤️

It would be helpful if you can submit test(s) that demonstrate what other broken cases you've discovered, using the actual API that intersect provides. That makes it possible to develop an actionable solution.

The work-around is to set the near and far values to infinity in such cases.
This can lead to false-positives ("ghost collisions")

That makes me a little nervous, because It sounds like this workaround is trading one problem for another. A broadphase collision check shouldn't be required to filter out bad results, that doesn't feel right to me.

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

2 participants