Skip to content

Add GetBatchRayIntersectionFromLastStepFeature for CPU lidar#880

Open
apojomovsky wants to merge 18 commits intogazebosim:mainfrom
apojomovsky:feature/cpu-lidar
Open

Add GetBatchRayIntersectionFromLastStepFeature for CPU lidar#880
apojomovsky wants to merge 18 commits intogazebosim:mainfrom
apojomovsky:feature/cpu-lidar

Conversation

@apojomovsky
Copy link

@apojomovsky apojomovsky commented Feb 19, 2026

🎉 New feature

This PR is 1 out of 3 that implement: gazebosim/gz-sensors#26

Summary

Add GetBatchRayIntersectionFromLastStepFeature, a new gz-physics feature that accepts all rays for a sensor in a single call and returns all results at once.

The dartsim implementation bypasses DART's raycast() wrapper, which allocates a temporary btCollisionObject + btShape per ray and calls btCollisionWorld::rayTest() directly.

To reach the underlying btCollisionWorld* without patching DART (where BulletCollisionGroup::getBulletCollisionWorld() is protected), a thin subclass GzBulletCollisionGroup bridges the protected method via inheritance, and GzBulletCollisionDetector::createCollisionGroup() is overridden to produce instances of it.

A batch API was chosen over modifying the existing per-ray GetRayIntersectionFromLastStep to avoid a silent behavior change to a public API, provide independent test coverage, and establish a clean boundary for future batching (e.g. GPU).

Only the Bullet collision detector supports raycasting in dartsim. Non-Bullet detectors (ODE, FCL, native DART) emit a one-time gzwarn and return NaN results.

Test it

colcon build --merge-install --packages-select gz-physics \
  --cmake-args '-DBUILD_TESTING=ON'
colcon test --packages-select gz-physics \
  --ctest-args -R COMMON_TEST_simulation_features_dartsim

Checklist

  • Signed all commits for DCO
  • Added a screen capture or video to the PR description that demonstrates the feature
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • 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.)

Generated-by: Claude Opus 4.6

Disclaimer: The code here was reviewed, tested, and profiled by hand by the author.

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by and Generated-by messages.

@apojomovsky apojomovsky requested a review from ahcorde February 20, 2026 15:56
@apojomovsky apojomovsky changed the base branch from gz-physics9 to main February 20, 2026 20:22
Introduces a batched raycasting API that processes all rays for a sensor
in a single virtual call instead of one call per ray, eliminating the
per-ray overhead that was the primary bottleneck for CPU lidar sensors.

Generated-by: Claude Opus 4.6

Signed-off-by: Alexis Pojomovsky <apojomovsky@gmail.com>
Replicate ODE Classic's dSpaceCollide2 approach in Bullet:
1. Compute the AABB of the entire ray bundle.
2. ONE btBroadphase::aabbTest() call collects candidate objects (C << N).
3. Per-ray: btCollisionWorld::rayTestSingle() against candidates only.

Generated-by: Claude Opus 4.6

Signed-off-by: Alexis Pojomovsky <apojomovsky@gmail.com>
Generated-by: Claude Opus 4.6

Signed-off-by: Alexis Pojomovsky <apojomovsky@gmail.com>
Generated-by: Claude Opus 4.6

Signed-off-by: Alexis Pojomovsky <apojomovsky@gmail.com>
Generated-by: Claude Opus 4.6

Signed-off-by: Alexis Pojomovsky <apojomovsky@gmail.com>
Generated-by: Claude Sonnet 4.6

Signed-off-by: Alexis Pojomovsky <apojomovsky@gmail.com>
Generated-by: Copilot

Signed-off-by: Alexis Pojomovsky <apojomovsky@gmail.com>
Signed-off-by: Alexis Pojomovsky <apojomovsky@gmail.com>
Copy link
Contributor

@j-rivero j-rivero left a comment

Choose a reason for hiding this comment

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

Thanks for the PR !

Not a big expert in physics but let me ask some questions and raise some proposals for the idea:

I somehow miss an API clean way of knowing if there was a hit or not in Ray, it is not documented anyway in the code. I assume that the user needs to check for NaN probably in fraction but I think that NaN has its own problems for some floating point comparisons.

With some help from Claude, how about something like?
https://gist.github.com/j-rivero/5a96bf40b33f0468366dbea82d431ffc

- Make getCollisionWorld() return const and be const-qualified
- Add nullptr guard for btCollisionWorld in startup/teardown phases
- Add bool hit field to RayIntersectionT for clean hit detection
- Cache constraint solver pointer to avoid duplicate getConstraintSolver() call
- Remove redundant CHECK_UNSUPPORTED_ENGINE (feature list already filters)
- Strengthen NaN assertions from isNaN().any() to isNaN().all()

Generated-by: Claude Opus 4.6

Signed-off-by: Alexis Pojomovsky <apojomovsky@gmail.com>
@apojomovsky
Copy link
Author

Thanks for the PR !

Not a big expert in physics but let me ask some questions and raise some proposals for the idea:

I somehow miss an API clean way of knowing if there was a hit or not in Ray, it is not documented anyway in the code. I assume that the user needs to check for NaN probably in fraction but I think that NaN has its own problems for some floating point comparisons.

With some help from Claude, how about something like? https://gist.github.com/j-rivero/5a96bf40b33f0468366dbea82d431ffc

This was actually a great suggestion, thank you. It was implemented as part of my last update, PTAL.

@apojomovsky apojomovsky requested a review from j-rivero February 25, 2026 15:23
@peci1
Copy link
Contributor

peci1 commented Feb 25, 2026

Thanks for doing all the hard work!

How difficult would it be to run the lidar in the more realistic non-zero-scan-duration way? I.e. only capturing a bunch of azimuths at once each timestep and composing the pointcloud from these time-advancing chunks? This would really help with debugging/simulating lidar deskewing nodes...

@apojomovsky
Copy link
Author

apojomovsky commented Feb 26, 2026

Thanks for doing all the hard work!

How difficult would it be to run the lidar in the more realistic non-zero-scan-duration way? I.e. only capturing a bunch of azimuths at once each timestep and composing the pointcloud from these time-advancing chunks? This would really help with debugging/simulating lidar deskewing nodes...

Thanks! Worth noting I did most of this with AI assistance, so take my intuition here with a grain of salt.

The batch API already accepts arbitrary subsets of rays, so in principle you could cast different azimuth slices at different physics steps. The needsRaycast per-step gating is already in place too. So the gz-physics side might not need changes, but I honestly haven't thought through the orchestration and pointcloud assembly side at all. If you want to explore it, the CpuLidar system (that we will hopefuly be merging into gz-sim soon) could be a good place to start.

Exercises the per-ray loop with 7 hits and 5 misses to catch
off-by-one bugs that the 2-ray test cannot expose. Only checks
results.size() and hit bools, no numeric assertions.

Generated-by: Amp <amp@ampcode.com>

Signed-off-by: Alexis Pojomovsky <apojomovsky@gmail.com>
Signed-off-by: Alexis Pojomovsky <apojomovsky@gmail.com>
Move Bullet-specific ray logic from SimulationFeatures.cc into
GzBulletCollisionDetector::BatchRaycast(), behind a virtual method
on the GzCollisionDetector mixin. This allows other detectors
(e.g. ODE) to implement batch raycasting by overriding the same
method.

The default implementation returns false (not supported), and
SimulationFeatures falls back to a warning + NaN results.
Bullet header removed from SimulationFeatures.cc.

Generated-by: Amp <amp@ampcode.com>

Signed-off-by: Alexis Pojomovsky <apojomovsky@gmail.com>
…up to .cc, remove NaN defaults

- Change BatchRaycast to return std::optional<std::vector<GzRayResult>>
  instead of bool + output parameter.
- Move GzBulletCollisionGroup class definition from header to .cc file,
  removing BulletCollisionGroup.hpp include from the header.
- Move warn-once for unsupported detectors into base BatchRaycast.
- Remove NaN default initializers from GzRayResult; set NaN explicitly
  only on the miss path in the Bullet BatchRaycast implementation.
- Use emplace_back in SimulationFeatures.cc conversion loop.

Generated-by: Amp <amp@ampcode.com>

Signed-off-by: Alexis Pojomovsky <apojomovsky@gmail.com>
…t copy

Replace the GzRayResult struct with a using alias to
gz::physics::RayIntersectionT<FeaturePolicy3d>, making the detector
return type identical to BatchRayIntersection. This removes the
per-element conversion loop in GetBatchRayIntersectionFromLastStep,
replacing it with a direct std::move of the result vector.

Generated-by: Amp <amp@ampcode.com>

Signed-off-by: Alexis Pojomovsky <apojomovsky@gmail.com>
When the collision group or btWorld is null, return std::nullopt
instead of a vector of default-constructed results with uninitialized
Eigen fields. The caller already handles std::nullopt by filling
NaN values per REP-117.

Generated-by: Amp <amp@ampcode.com>

Signed-off-by: Alexis Pojomovsky <apojomovsky@gmail.com>
Copy link
Contributor

@j-rivero j-rivero left a comment

Choose a reason for hiding this comment

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

IMHO, is good to go technically. Will require the design and architecture overview from @scpeters or @azeey but it is a great work Alexis.

@github-project-automation github-project-automation bot moved this from Inbox to In review in Core development Mar 12, 2026
j-rivero

This comment was marked as duplicate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

6 participants