Skip to content

feat: Raycast implementation for ODE#976

Merged
azeey merged 10 commits into
gazebosim:mainfrom
cellumation:main
Jul 15, 2026
Merged

feat: Raycast implementation for ODE#976
azeey merged 10 commits into
gazebosim:mainfrom
cellumation:main

Conversation

@jmachowinski

@jmachowinski jmachowinski commented May 12, 2026

Copy link
Copy Markdown
Contributor

🎉 New feature

Followup to #880

This adds support for ODE for the raycast implementation, and therefore to the CPU-Lidar implementation.

Checklist

  • [x ] Signed all commits for DCO
  • Added a screen capture or video to the PR description that demonstrates the fix (as needed)
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • [x ] All tests passed (See test coverage)
  • Updated Bazel files (if adding new files). Created an issue otherwise.
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers
  • Was GenAI used to generate this PR? If so, make sure to add "Generated-by" to your commits. (See this policy for more info.)

@azeey

azeey commented May 12, 2026

Copy link
Copy Markdown
Contributor

Thanks for the contribution! Could you please fill out the PR template?

@jmachowinski

Copy link
Copy Markdown
Contributor Author

codecheck` passed <- This throws pagewise unrelated warnings...

Note, for the tests, I needed to remove the unsupported 'banana' as this defaulted to ode, and was therefore supported.

@azeey azeey left a comment

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.

Overall, this looks good, but there a couple of issues that need to be addressed. It would also be great if we can update the test to use multiple objects in the line of sight of the ray.

Comment thread dartsim/src/GzCollisionDetector.cc Outdated
Comment thread dartsim/src/GzCollisionDetector.cc Outdated
Comment thread dartsim/src/GzCollisionDetector.cc Outdated
Comment on lines +233 to +242
if(dCollide(ray, other, 1, &contact, sizeof(dContactGeom)) > 0)
{
if(result->mRayHits.empty())
{
setResult(result->mRayHits.emplace_back());
}
else
{
setResult(result->mRayHits.front());
}

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.

From Gemini:

dSpaceCollide2 will call NearCallbackODE for every geometry whose bounding box intersects the ray. By unconditionally overwriting .front(), the raycast will simply return the last object visited by ODE's spatial traversal hierarchy, which is not guaranteed to be the closest object.

I have reproduced the issue by running this on gz-sim/examples/worlds/cpu_lidar_sensor.sdf with the collision_detector changed to ode. As you see in the video below, after moving the cube in front of the cylinder, there are still rays hitting the cylinder. Sometimes, it happens the other way around. I move the cube behind the cylinder and the rays now skip the cylinder and hit the cube.

Screen.Recording.2026-07-01.at.12.27.15.mov

Suggested solution:

Tip

Recommendation: You need to keep track of the minimum distance and only overwrite if the new contact is closer. ODE conveniently populates contact.depth with the distance from the ray origin for ray intersections. You can pass a custom struct via _data instead of just RaycastResult*:

struct OdeRaycastData {
  RaycastResult* result;
  double minDistance = std::numeric_limits<double>::infinity();
};

And in NearCallbackODE:

  OdeRaycastData* data = static_cast<OdeRaycastData*>(_data);
  // ... inside dCollide block:
  if (contact.depth < data->minDistance)
  {
    data->minDistance = contact.depth;
    if(data->result->mRayHits.empty())
      setResult(data->result->mRayHits.emplace_back());
    else
      setResult(data->result->mRayHits.front());
  }

@jmachowinski jmachowinski Jul 3, 2026

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 one is odd, I would have expected to only get the closest hit as I set dGeomRaySetClosestHit.

I fixed it now, note I explicitly computed the sqr dist, as I tried using depth before for the fraction and it gave me not the distance.

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.

Looks like it's not quite fixed on my end. I believe you forgot to update data->curContactDistSqr.

Image

This one is odd, I would have expected to only get the closest hit as I set dGeomRaySetClosestHit.

It is giving you the closest hit for each ray/object pair, but the NearCallabackODE will be called for each object in the scene. In other words, the ray may hit the box multiple times and you'll get the closest of those, but the collision of the ray with the cylinder will be treated as a separate collision and gets its own callback.

@github-project-automation github-project-automation Bot moved this from Inbox to In review in Core development Jul 1, 2026
@jmachowinski

Copy link
Copy Markdown
Contributor Author

@azeey ping

@jmachowinski

Copy link
Copy Markdown
Contributor Author

@azeey Added a test and fixed the issue.

@azeey azeey left a comment

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.

LGTM! Thanks again for the contribution. Just a few minor comments left. Also, would you mind merging from main and fixing the conflicts?

Comment thread test/common_test/simulation_features.cc Outdated
Comment thread test/common_test/simulation_features.cc Outdated
Comment thread test/common_test/simulation_features.cc Outdated
Janosch Machowinski and others added 10 commits July 15, 2026 18:45
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
@azeey
azeey enabled auto-merge (squash) July 15, 2026 17:55
@azeey
azeey merged commit abc3544 into gazebosim:main Jul 15, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants