Skip to content

[bugfix] - raycast buffer distance#2466

Merged
aclegg3 merged 2 commits intomainfrom
alex-09_09-raycast_buffer_distance
Sep 9, 2024
Merged

[bugfix] - raycast buffer distance#2466
aclegg3 merged 2 commits intomainfrom
alex-09_09-raycast_buffer_distance

Conversation

@aclegg3
Copy link
Copy Markdown
Contributor

@aclegg3 aclegg3 commented Sep 9, 2024

Motivation and Context

This PR fixes a long-standing raycasting bug related to the behavior of Bullet physics raycasting within the collision margin of a convex shape.

TL;DR: we add a hidden "buffer distance" to the raycast to avoid missing an object from within its margin.

Context:

  • Bullet physics raycasts do not register hits from within convex shapes, neither entering, nor exiting.
  • Each collision shape has a configurable margin distance which is used to pad the collision detection algorithm to better catch upcoming impacts between dynamic shapes and avoid discrete collision issues near boundaries.
  • (a) Raycasts against convex shapes hit the shape, not the margin.
  • (b) Raycast origins within the margin are not considered.
  • Taken together, (a) and (b) result in a narrow region of space near the surface of an object where a raycast will not register the object it is about to hit, though expecting a positive hit distance from the API. This "null" zone causes a number of bugs as we attempt to snap objects onto each other or detect which objects are resting upon others.

Solution:

  • The simple solution is to back-pad the raycast origins and then re-scale the resulting distances such that rays cast within the margin still hit the shape as expected while any hits between the buffer and origin are discarded.

How Has This Been Tested

CI raycast tests still pass as before with expected global hit points and distances, despite the buffer.
Added new unit tests to specifically target this bug and demonstrate the solution.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have completed my CLA (see CONTRIBUTING)
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Sep 9, 2024
hit.rayDistance =
(static_cast<double>(allResults.m_hitFractions[i]) * maxDistance);
(static_cast<double>((allResults.m_hitFractions[i])) - scaledBuffer) *
totalLength / rayLength;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe cache totalLength / rayLength before the loop?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, could do. I feel like the math could be more elegant here, honestly, but I didn't come up with a better version so moving on for now. 😖

Copy link
Copy Markdown
Contributor

@0mdc 0mdc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall. I would consider lowering the buffer distance.

R"(Perform discrete collision detection for the scene. Physics must be enabled. Warning: may break simulation determinism.)")
.def(
"cast_ray", &Simulator::castRay, "ray"_a, "max_distance"_a = 100.0,
"buffer_distance"_a = 0.08,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: 8cm is a pretty big distance for a buffer. Have you explored setting a lower value? (<=0.01 would be good).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value must be greater than the maximum expected collision margin (default is 4cm). I think 0.08 (8 cm) therefore is fine as we are ditching any hits within that distance anyway. FYI, the minimum I'd want to use is 4cm, so I doubled that.

Copy link
Copy Markdown
Contributor

@jturner65 jturner65 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@aclegg3 aclegg3 merged commit 537cdaa into main Sep 9, 2024
@aclegg3 aclegg3 deleted the alex-09_09-raycast_buffer_distance branch September 9, 2024 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Do not delete this pull request or issue due to inactivity.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants