Skip to content

Honor target_voxel_size in texture SDF construction#2464

Merged
adenzler-nvidia merged 3 commits into
newton-physics:mainfrom
devshahofficial:fix/texture-sdf-target-voxel-size
Apr 20, 2026
Merged

Honor target_voxel_size in texture SDF construction#2464
adenzler-nvidia merged 3 commits into
newton-physics:mainfrom
devshahofficial:fix/texture-sdf-target-voxel-size

Conversation

@devshahofficial

@devshahofficial devshahofficial commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #2407 — the texture SDF path silently ignored target_voxel_size and always fell back to the default max_resolution=64, causing the texture SDF to disagree with the sparse SDF (which did honor the requested voxel size) and producing visually-mismatched collision debug meshes. The documented precedence is that target_voxel_size takes priority over max_resolution when both are provided.

Root cause

In newton/_src/geometry/sdf_utils.py::SDF.create_from_mesh:

effective_max_resolution = 64 if max_resolution is None and target_voxel_size is None else max_resolution
...
res = effective_max_resolution if effective_max_resolution is not None else 64
create_texture_sdf_from_mesh(..., max_resolution=res, ...)

When the user passed only target_voxel_size=0.1, effective_max_resolution ended up None, res fell through to 64, and target_voxel_size was dropped on the floor before reaching the texture-SDF builder. The same pattern existed in newton/_src/sim/builder.py for primitive shapes.

Fix

  1. newton/_src/geometry/sdf_texture.pycreate_texture_sdf_from_mesh now accepts an optional target_voxel_size and derives max_resolution from it using ceil(max_padded_extent / target_voxel_size), rounded up to a multiple of 8 to match the tiled-SDF builder convention. This mirrors the conversion already done in the sparse path in _compute_sdf_from_shape_impl (sdf_utils.py:778-787), so the two paths now agree on resolution for any given target_voxel_size. max_resolution is now int | None with a default of None; the prior behavior (64 when both are unset) is preserved.

  2. newton/_src/geometry/sdf_utils.pySDF.create_from_mesh now forwards target_voxel_size into the texture call instead of dropping it.

  3. newton/_src/sim/builder.py — the primitive-mesh path at the hydroelastic call site now forwards sdf_target_voxel_size into the texture call. ShapeConfig.configure_sdf(target_voxel_size=...) was already plumbed through self.shape_sdf_target_voxel_size but discarded at the final call.

Behavior

Before (on this branch without the fix):

vox: 0.2    -> texture_blocks: 504
vox: 0.1    -> texture_blocks: 504
vox: 0.05   -> texture_blocks: 504

After:

vox: 0.2    -> texture_blocks: 1
vox: 0.1    -> texture_blocks: 8
vox: 0.05   -> texture_blocks: 27

(Matches the reporter's observation in #2407.)

Test plan

Added four regression tests in newton/tests/test_sdf_texture.py:

  • test_texture_sdf_target_voxel_size_scales — sweeping target_voxel_size across (0.2, 0.1, 0.05) produces a strictly-increasing number of texture blocks on a unit cube. Would have failed on main (identical counts).
  • test_texture_sdf_target_voxel_size_takes_precedence — passing both max_resolution=8 and target_voxel_size=0.05 produces more blocks than max_resolution=8 alone, confirming target_voxel_size wins.
  • test_mesh_build_sdf_target_voxel_size_propagates_to_texture — end-to-end path through the public Mesh.build_sdf(target_voxel_size=...) API, validating that SDF.texture_block_coords actually scales (the exact symptom reported in [BUG] Texture SDF generation ignores target_voxel_size precedence #2407).
  • test_create_texture_sdf_from_mesh_validates_target_voxel_size — invalid inputs (0.0, negative) raise ValueError with a clear message.
  • All four tests registered via add_function_test against get_cuda_test_devices(), consistent with the rest of test_sdf_texture.py; they auto-skip on non-CUDA environments.
  • ruff check / ruff format --check pass on all four modified files (my changes introduce no new lint errors).
  • Verified all existing call sites to create_texture_sdf_from_mesh still work — the signature change is backwards-compatible (max_resolution became int | None with the same 64 default preserved in the no-args path).
  • Full CUDA test run — I was not able to run the CUDA-gated SDF tests locally (arm64 macOS, no NVIDIA GPU). Relying on CI. Happy to iterate on any failures.

Fixes #2407

Summary by CodeRabbit

  • New Features

    • Added a target_voxel_size option to control texture SDF resolution with input validation and sensible defaults; it takes precedence when provided and limits max resolution.
    • Mesh-to-texture SDF generation now respects the new resolution control end-to-end.
  • Tests

    • Added regression tests validating resolution scaling, precedence behavior, propagation, and invalid-input handling.

)

The texture SDF path silently ignored `target_voxel_size` and always
fell back to the default `max_resolution=64`, causing the texture SDF
to disagree with the sparse SDF (which honored the requested voxel
size) and producing visually-mismatched collision debug meshes. The
documented precedence is that `target_voxel_size` takes priority over
`max_resolution` when both are provided.

- `create_texture_sdf_from_mesh` now accepts an optional
  `target_voxel_size` and derives `max_resolution` from it, rounding
  up to a multiple of 8 to match the tiled SDF builders. This mirrors
  the conversion already done in the sparse path in
  `_compute_sdf_from_shape_impl`.
- `SDF.create_from_mesh` and the primitive-mesh path in
  `ModelBuilder` now forward `target_voxel_size` into the texture
  call instead of dropping it.
- Added four regression tests in `test_sdf_texture.py`: voxel-size
  scaling, precedence over `max_resolution`, end-to-end propagation
  through `Mesh.build_sdf`, and input validation.

Fixes newton-physics#2407

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@linux-foundation-easycla

linux-foundation-easycla Bot commented Apr 15, 2026

Copy link
Copy Markdown

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: devshahofficial / name: Dev Shah (06a44d1, 42a9f0c)
  • ✅ login: devshahofficial / name: devshahofficial (1fb4cfc)

@coderabbitai

coderabbitai Bot commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: bfdcda90-c97a-4977-b86d-9f47ca77d4fd

📥 Commits

Reviewing files that changed from the base of the PR and between 1fb4cfc and 06a44d1.

📒 Files selected for processing (1)
  • newton/_src/sim/builder.py
✅ Files skipped from review due to trivial changes (1)
  • newton/_src/sim/builder.py

📝 Walkthrough

Walkthrough

Create_texture_sdf_from_mesh gained a new optional target_voxel_size parameter and now derives an effective max_resolution from it (with validation and 8-voxel alignment). The parameter is propagated through SDF creation call sites and four GPU-only regression tests were added to validate behavior and input checks.

Changes

Cohort / File(s) Summary
Core SDF Texture API
newton/_src/geometry/sdf_texture.py
Signature changed to accept `max_resolution: int
Parameter Propagation
newton/_src/geometry/sdf_utils.py, newton/_src/sim/builder.py
Forwarded target_voxel_size through the SDF creation path: SDF.create_from_mesh now passes target_voxel_size to create_texture_sdf_from_mesh; simulation builder forwards sdf_target_voxel_size when constructing primitive meshes.
Test Coverage
newton/tests/test_sdf_texture.py
Added four GPU-only regression tests: scaling with decreasing target_voxel_size, precedence of target_voxel_size over max_resolution, end-to-end propagation from Mesh.build_sdf, and validation that invalid target_voxel_size values raise ValueError.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Honor target_voxel_size in texture SDF construction' directly and clearly summarizes the main change—implementing proper support for the target_voxel_size parameter in texture-based SDF generation.
Linked Issues check ✅ Passed The PR successfully addresses all coding requirements from issue #2407: derives max_resolution from target_voxel_size with proper validation, rounds to multiples of 8 for tiled SDF alignment, propagates target_voxel_size through all call sites, and adds comprehensive regression tests demonstrating scaling, precedence, propagation, and input validation.
Out of Scope Changes check ✅ Passed All changes directly support the objective of honoring target_voxel_size in texture SDF construction: signature updates with validation logic, parameter propagation through call sites, and targeted regression tests—with no extraneous modifications.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@newton/_src/geometry/sdf_utils.py`:
- Around line 398-407: Add a CHANGELOG.md entry under the [Unreleased] → Fixed
section noting that the target_voxel_size parameter now influences texture-SDF
resolution; mention that target_voxel_size is forwarded to
create_texture_sdf_from_mesh (used by the texture SDF code path) and that this
changes how max_resolution is derived for texture SDFs so users know the
externally visible behavior change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 321d94be-d24c-4115-aea6-81c14752bc07

📥 Commits

Reviewing files that changed from the base of the PR and between b6a8799 and 1fb4cfc.

📒 Files selected for processing (4)
  • newton/_src/geometry/sdf_texture.py
  • newton/_src/geometry/sdf_utils.py
  • newton/_src/sim/builder.py
  • newton/tests/test_sdf_texture.py

Comment thread newton/_src/geometry/sdf_utils.py

@devshahofficial devshahofficial left a comment

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.

@eric-heiden can you review? Thanks!

@nvtw nvtw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, thanks for your contribution

@codecov

codecov Bot commented Apr 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 76.92308% with 3 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
newton/_src/geometry/sdf_texture.py 76.92% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

@nvtw

nvtw commented Apr 17, 2026

Copy link
Copy Markdown
Member

Let's merge here first and then I'll take care of fixing potential merge conflicts in #2475

@nvtw nvtw enabled auto-merge April 20, 2026 05:46
@nvtw nvtw added this pull request to the merge queue Apr 20, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Apr 20, 2026
@adenzler-nvidia adenzler-nvidia added this pull request to the merge queue Apr 20, 2026
Merged via the queue into newton-physics:main with commit c3cca91 Apr 20, 2026
25 checks passed
preist-nvidia added a commit to preist-nvidia/newton that referenced this pull request May 4, 2026
The pre-release audit found user-facing commits that landed since v1.1.0
without an [Unreleased] entry. Backfill five of them here. Kamino-related
commits (newton-physics#2280, newton-physics#2517, newton-physics#2554, newton-physics#2575) are intentionally omitted: Kamino is
still alpha and not currently tracked in CHANGELOG.

- Changed: contact-conversion overhead reduction in SolverMuJoCo
  ([newton-physicsGH-2393](newton-physics#2393))
- Fixed: target_voxel_size ignored on the texture-SDF path
  ([newton-physicsGH-2464](newton-physics#2464))
- Fixed: Newton-to-mujoco-warp material-combination mismatch
  ([newton-physicsGH-2439](newton-physics#2439))
- Fixed: eq_objtype mismatch on joint equality / mimic constraints in
  SolverMuJoCo ([newton-physicsGH-2459](newton-physics#2459))
- Fixed: _tiled_sum_kernel launch-dim handling under warp-lang 1.13
  ([newton-physicsGH-2546](newton-physics#2546))
@preist-nvidia preist-nvidia added this to the 1.2 Release milestone May 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[BUG] Texture SDF generation ignores target_voxel_size precedence

4 participants