Skip to content

I found an error in the code, it seems to be a bug #309

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

Open
hwh875496 opened this issue Apr 25, 2025 · 1 comment
Open

I found an error in the code, it seems to be a bug #309

hwh875496 opened this issue Apr 25, 2025 · 1 comment

Comments

@hwh875496
Copy link

Within the rayCast function definition in the AABB.ts file, around line 196:
In this section,

for (let f: "x" | "y" = "x"; f !== null; f = (f === "x" ? "y" : null)) {
      if (absD.x < EPSILON) {
        // Parallel.
        if (p[f] < this.lowerBound[f] || this.upperBound[f] < p[f]) {
          return false;
        }
      }
      ...
}

the for loop iterates over both "x" and "y" to analyze the directions along the x-axis and y-axis.
However, at line 209,

if (absD.x < EPSILON) {
  ...
}

it appears that only the value of absD in the x-axis direction is being considered.

I've examined the source code of Box2D, specifically the b2AABB.prototype.RayCast function definition. In it, there are checks for both directions of absD to see if they are less than Number.MIN_VALUE.

b2AABB.prototype.RayCast = function (output, input) {

...

   var absDX = Math.abs(dX);
   var absDY = Math.abs(dY);
   
   if (absDX < Number.MIN_VALUE) {
      if (pX < this.lowerBound.x || this.upperBound.x < pX) return false;
      ...
   }
   
   if (absDY < Number.MIN_VALUE) {
      if (pY < this.lowerBound.y || this.upperBound.y < pY) return false;
      ...
   }

...
}

I'm thinking that in Planck, we should modify line 209 of AABB.ts to if (absD[f] < EPSILON).
I'm not sure if this is the correct approach.

@shakiba
Copy link
Collaborator

shakiba commented Apr 25, 2025

Hmm, yeah seems like a mistake. Do you have an example to reproduce the bug and verify your fix?

And would you be able to make a PR to fix it?

Also I think instead of the for loop it's better to just duplicate the code for x and y.

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