warp icon indicating copy to clipboard operation
warp copied to clipboard

Mesh BVH Ray Query Improvements

Open StafaH opened this issue 2 months ago • 9 comments

Description

This PR enhances support for specific mesh ray queries that improve performance in downstream rendering applications.

This PR includes:

  • Improves mesh_query_ray with ordered traversal for more efficient traversal of the mesh BVH.
  • Introduce mesh_query_ray_anyhit which is a faster version of mesh_query_ray that can be used to calculate shadows in rendering applications. Shadow calculations only need to know if there exists any hit in the path of the ray.
  • Adds groups to mesh API to make grouped mesh BVHs, useful for combining meshes especially for multi-world deformables

Before your PR is "Ready for review"

  • [x] All commits are signed-off to indicate that your contribution adheres to the Developer Certificate of Origin requirements
  • [x] Necessary tests have been added
  • [x] Documentation is up-to-date
  • [x] Auto-generated files modified by compiling Warp and building the documentation have been updated (e.g. __init__.pyi, functions.rst)
  • [x] Code passes formatting and linting checks with pre-commit run -a

Summary by CodeRabbit

  • New Features

    • Mesh constructor accepts per-face group indices; ray queries can be scoped to a subtree/root and an any-hit query returns on first intersection.
    • Faster ordered traversal for ray queries.
  • Documentation

    • Docs updated to explain root semantics and group-based traversal.
  • Tests

    • Added tests validating grouped-mesh queries, any-hit behavior, and brute-force cross-checks.
  • Chores

    • Mesh creation and BVH construction paths updated to accept and propagate group information.

✏️ Tip: You can customize this high-level summary in your review settings.

StafaH avatar Nov 13 '25 02:11 StafaH

[!NOTE]

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Walkthrough

Add per-face group support to Mesh (new groups argument and attribute forwarded into native BVH creation); expose mesh_get_group_root(mesh, group_id); extend mesh_query_ray(...) with optional root and add mesh_query_ray_anyhit(..., root); thread groups and root through Python ctypes bindings, native APIs, adjoint stubs, and add tests validating grouped and any‑hit queries.

Changes

Cohort / File(s) Summary
Docs & Stubs
docs/modules/functions.rst, warp/__init__.pyi
Document mesh_get_group_root(), update mesh_query_ray(..., root) signature, add mesh_query_ray_anyhit(..., root), and describe root/group traversal semantics.
Python builtins & types
warp/_src/builtins.py, warp/_src/types.py
Add mesh_get_group_root and mesh_query_ray_anyhit; extend mesh_query_ray with root (default -1); add `groups: array
Python runtime bindings
warp/_src/context.py
Extend ctypes argtypes for wp_mesh_create_host and wp_mesh_create_device to include an extra ctypes.c_void_p groups pointer parameter.
Native headers & API
warp/native/warp.h, warp/native/mesh.h
Insert int* groups parameter into wp_mesh_create_host / wp_mesh_create_device declarations; declare/extend mesh_get_group_root() and root-aware / anyhit mesh query signatures and adjoint variants.
Native implementations
warp/native/mesh.cpp, warp/native/mesh.cu
Accept and forward groups pointer into BVH creation; implement root-aware traversal (initial stack/root handling), add mesh_query_ray_anyhit early-exit path, use temporaries for per-leaf hits, and thread root through query/adjoint wrappers and result structs.
Runtime value-types / wrappers
warp/_src/builtins.py (value-types)
Update MeshQueryRay and wrapper callsites to store/propagate root and adjust overloads/defaults for the new root parameter.
Tests
warp/tests/geometry/test_mesh.py, warp/tests/geometry/test_mesh_query_ray.py
Add kernels and tests validating grouped-mesh construction, root-constrained queries, any-hit behavior, brute-force cross-checks, and register tests.
Changelog
CHANGELOG.md
Document Mesh.groups, mesh_get_group_root(), root-aware mesh query APIs, mesh_query_ray_anyhit, and grouping-related notes.

Sequence Diagram(s)

sequenceDiagram
    participant Py as Python API
    participant Ctx as ctypes runtime
    participant Native as Native API
    participant BVH as BVH builder/traversal

    rect rgba(250,250,255,0.9)
    note over Py,Ctx: Mesh creation with optional per-face groups
    Py->>Ctx: wp_mesh_create_host(points, indices, ..., groups_ptr_or_null, ...)
    Ctx->>Native: wp_mesh_create_host(..., groups_ptr_or_null, ...)
    Native->>BVH: bvh_create_host(..., groups)
    BVH-->>Native: bvh handle
    Native-->>Py: mesh_id
    end

    rect rgba(245,245,245,0.9)
    note over Py,Native: Ray query with optional root / anyhit
    Py->>Native: mesh_query_ray(mesh_id, start, dir, max_t, root=-1)
    alt root == -1
        Native->>BVH: traverse from global root
    else root != -1
        Native->>BVH: traverse from subtree root (mesh_get_group_root)
    end
    opt anyhit variant
        Native->>BVH: stop and return on first hit
    end
    BVH-->>Native: hit info / hit flag
    Native-->>Py: MeshQueryRay struct or bool
    end

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Review focus:
    • ABI/ctypes consistency for the inserted groups pointer across warp/native/warp.h, warp/native/mesh.cpp, warp/native/mesh.cu, and warp/_src/context.py.
    • Correct forwarding and null-vs-pointer handling of groups from Python through ctypes into native BVH creation.
    • BVH traversal changes: root initialization, stack handling, ordered traversal correctness, and anyhit early-exit.
    • Adjoint wrapper updates: ensure root is propagated and signatures match forward APIs.
    • Python-side validation: groups dtype/device/contiguity and length == num_tris.
    • Tests: verify deterministic expectations for grouped/un-grouped meshes and any-hit behavior.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 26.47% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main changes: adding mesh_query_ray_anyhit, group support for mesh BVHs, and new root parameter for mesh ray queries to improve ray-query functionality.
✨ Finishing touches
  • [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Nov 13 '25 02:11 coderabbitai[bot]

Greptile Overview

Greptile Summary

Adds two key enhancements to mesh BVH ray queries for improved rendering performance:

  • mesh_query_ray_anyhit(): New function for shadow ray calculations that returns true immediately upon finding any intersection, avoiding unnecessary traversal after the first hit
  • Group-aware mesh BVHs: Meshes can now be constructed with per-face group IDs, creating subtree structures that enable efficient group-restricted queries via the optional root parameter in mesh_query_ray() and mesh_query_ray_anyhit()

The implementation is clean and well-integrated:

  • C++/CUDA native code properly propagates the groups parameter through mesh and BVH construction
  • Python bindings include proper validation (int32 type, contiguous, correct length)
  • The any-hit implementation correctly returns early without computing full intersection details
  • Comprehensive test coverage validates correctness against brute-force raycast and tests multiple BVH constructor types and leaf sizes
  • Documentation and type stubs are complete and up-to-date

Previous review feedback has been addressed (variable naming conflicts resolved, anyhit is now a separate function, integer division operators fixed).

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The implementation is well-structured, addresses previous review feedback, has comprehensive test coverage validating correctness across multiple configurations, and follows established patterns in the codebase. No security issues, performance regressions, or logical errors were found.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
warp/native/mesh.h 5/5 Adds mesh_query_ray_anyhit() for early-exit ray queries and extends mesh_query_ray() with optional root parameter for group-aware traversal. Variable naming conflicts resolved.
warp/_src/types.py 5/5 Adds groups parameter to Mesh constructor with proper validation and passes it to native mesh creation
warp/_src/builtins.py 5/5 Adds mesh_query_ray_anyhit(), mesh_get_group_root() and extends mesh_query_ray() with optional root parameter
warp/tests/geometry/test_mesh_query_ray.py 5/5 Adds comprehensive tests for grouped meshes, mesh_query_ray_anyhit(), and validates against brute-force raycast

Sequence Diagram

sequenceDiagram
    participant User as User Code
    participant Mesh as wp.Mesh
    participant Types as types.py
    participant Native as mesh.cpp/cu
    participant BVH as BVH
    participant Query as mesh.h

    Note over User,Query: Mesh Creation with Groups
    User->>Mesh: wp.Mesh(points, indices, groups)
    Mesh->>Types: __init__(groups array)
    Types->>Types: Validate groups (int32, contiguous, length=num_tris)
    Types->>Native: wp_mesh_create_host/device(groups ptr)
    Native->>BVH: bvh_create_host/device(groups)
    BVH-->>Native: BVH with group subtrees
    Native-->>User: mesh.id

    Note over User,Query: Ray Query Operations
    User->>Query: wp.mesh_query_ray(mesh.id, start, dir, max_t, root=-1)
    Query->>Query: Initialize stack with root node
    Query->>Query: Traverse BVH from root
    Query->>Query: Test ray-AABB intersections
    Query->>Query: Test ray-triangle intersections
    Query-->>User: Returns closest hit (t, u, v, sign, normal, face)

    Note over User,Query: Any-Hit Query (Shadow Rays)
    User->>Query: wp.mesh_query_ray_anyhit(mesh.id, start, dir, max_t, root=-1)
    Query->>Query: Initialize stack with root node
    Query->>Query: Traverse BVH from root
    Query->>Query: Test ray-AABB intersections
    Query->>Query: Test ray-triangle intersection
    alt Hit Found
        Query-->>User: Return true immediately
    else No Hit
        Query->>Query: Continue traversal
    end

    Note over User,Query: Group-Restricted Query
    User->>Query: wp.mesh_get_group_root(mesh.id, group_id)
    Query->>BVH: bvh_get_group_root(group_id)
    BVH-->>User: root node index
    User->>Query: wp.mesh_query_ray_anyhit(mesh.id, start, dir, max_t, root)
    Query->>Query: Traverse only group subtree
    Query-->>User: Hit result for group only

greptile-apps[bot] avatar Nov 13 '25 02:11 greptile-apps[bot]

Also, please add unit tests for mesh_query_ray_anyhit()

shi-eric avatar Nov 13 '25 23:11 shi-eric

Hi @StafaH, I think this PR looks good. Please fixed the minor issues that Eric and I pointed out and I think we are good to go.

AnkaChan avatar Nov 17 '25 22:11 AnkaChan

Hi @shi-eric, @AnkaChan, I have cleaned up everything, squashed the commits, and added the necessary tests. Let me know if anything else is needed!

StafaH avatar Nov 20 '25 21:11 StafaH

It would also help us if you could add a concise entry to the CHANGELOG.md to mention the new built-in and what it does.

shi-eric avatar Nov 20 '25 23:11 shi-eric

@shi-eric Updated changelog and fixed the tests. I also saw the that the CI was failing, added a fix that should now pass all the build and tests.

StafaH avatar Nov 21 '25 17:11 StafaH

Thanks for the fixes @StafaH! Looks like there's a changelog conflict right now

shi-eric avatar Nov 22 '25 19:11 shi-eric

@shi-eric Branch is rebased and should be good to go

StafaH avatar Nov 24 '25 20:11 StafaH

@StafaH Just enabled clang-format for the codebase as a pre-commit check. You should have some conflicts when rebasing but hopefully they're easy to resolve, then apply clang-format (pre-commit run -a), then fixup back down to a single commit.

shi-eric avatar Dec 01 '25 13:12 shi-eric

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

copy-pr-bot[bot] avatar Dec 02 '25 02:12 copy-pr-bot[bot]

@mmacklin, @AnkaChan I am removing the change for ordered traversal, after more careful consideration, it seems the perf increase was more related to changing out woop for rtcd rather than the traversal itself. I want to investigate further and fix the bottlenecks in woop intersection test first before testing ordered vs unordered.

The PR now adds anyhit and adds groups to the mesh bvh build. Please take a look. I will circle back later with a proper PR for the improvements to the query performance.

@shi-eric Did the rebase and have the pre-commit setup to do the clang format, everything should be good to go. I updated the issue and changelog to reflect the above change in the PR.

StafaH avatar Dec 02 '25 04:12 StafaH

/ok to test cf28d4f14438175f8345e263a3d85e5d8ca51fdc

shi-eric avatar Dec 04 '25 01:12 shi-eric

@shi-eric I notice the doc test is failing, I can try to remove that doc indentation that happened if it's needed.

As an aside is there a way I can stop my build_docs.py from indenting those matrices, it happens every time, and I always have to revert the change in that section of the docs.

StafaH avatar Dec 04 '25 01:12 StafaH

@shi-eric I notice the doc test is failing, I can try to remove that doc indentation that happened if it's needed.

As an aside is there a way I can stop my build_docs.py from indenting those matrices, it happens every time, and I always have to revert the change in that section of the docs.

The formatting that build_docs.py applies should be considered the reference, so if it's modifying the formatting, then I think something else on your system is modifying the indentation first and then build_docs.py is repairing it, if that makes sense.

shi-eric avatar Dec 04 '25 02:12 shi-eric

Makes sense. I fixed the doc issue, all checks should pass now.

StafaH avatar Dec 04 '25 05:12 StafaH

/ok to test 0a017adc3a11577353599e86416f5cf60cec1e7c

shi-eric avatar Dec 04 '25 11:12 shi-eric

Makes sense. I fixed the doc issue, all checks should pass now.

Thanks! Will get this merged shortly.

shi-eric avatar Dec 04 '25 11:12 shi-eric