Fix `target_voxel_size` Being Ignored by Texture SDF by christophercrouzet · Pull Request #2525 · newton-physics/newton

christophercrouzet · 2026-04-21T21:29:56Z

Description

SDF.create_from_mesh (and Mesh.build_sdf) computes two representations: a sparse NanoVDB grid and a companion texture SDF used by the GL viewer's isomesh pass, hydroelastic broadphase, and texture-based SDF sampling. The docstring promises that target_voxel_size "takes precedence over max_resolution", but that was only honored by the sparse path — the texture path never received target_voxel_size and silently fell back to max_resolution=64. Users parameterizing by world-unit voxel size therefore got a dramatically coarser texture SDF than the sparse grid, with no warning.

This PR forwards target_voxel_size through to create_texture_sdf_from_mesh and gives that function the same precedence policy as the sparse path. The None → 64 fallback is kept inside the texture module so the caller no longer duplicates the ceil/padding math.

Closes #2524

Checklist

New or existing tests cover these changes
The documentation is up to date with these changes
CHANGELOG.md has been updated (if user-facing change)

Test plan

uv run --extra dev -m newton.tests -k test_build_sdf_target_voxel_size_propagates_to_texture
uv run --extra dev -m newton.tests -k test_sdf   # 93 tests, all pass

The new regression test was also verified to fail without the source fix applied.

Bug fix

Steps to reproduce:

Build an SDF from a mesh using target_voxel_size alone (no max_resolution).
Inspect sdf._coarse_texture and sdf._subgrid_texture dimensions.
Observe that they correspond to a 64-resolution grid, not to the requested voxel size.

Minimal reproduction:

import numpy as np
from pxr import Usd

import newton
import newton.examples
import newton.usd

stage = Usd.Stage.Open(newton.examples.get_asset("bunny.usd"))
usd_mesh = newton.usd.get_mesh(stage.GetPrimAtPath("/root/bunny"))
mesh = newton.Mesh(
    np.asarray(usd_mesh.vertices, dtype=np.float32),
    np.asarray(usd_mesh.indices, dtype=np.int32),
)

margin = 0.05
verts = np.asarray(usd_mesh.vertices, dtype=np.float32)
padded_longest = float((verts.max(axis=0) - verts.min(axis=0) + 2.0 * margin).max())
voxel = padded_longest / 384  # equivalent to max_resolution=384

sdf_a = mesh.build_sdf(max_resolution=384, margin=margin)
sdf_b = mesh.build_sdf(target_voxel_size=voxel, margin=margin)

# Before this PR: A -> 297x297x297, B -> 63x63x63 (4.7x coarser per axis)
# After this PR:  both -> 297x297x297
print("A subgrid:", sdf_a._subgrid_texture.width, sdf_a._subgrid_texture.height, sdf_a._subgrid_texture.depth)
print("B subgrid:", sdf_b._subgrid_texture.width, sdf_b._subgrid_texture.height, sdf_b._subgrid_texture.depth)

Summary by CodeRabbit

Bug Fixes
- Fixed target_voxel_size parameter to properly control texture SDF resolution during mesh-to-SDF conversion, ensuring consistent control over both texture and sparse grid resolutions.
Tests
- Added test to verify target_voxel_size parameter propagates correctly to texture resolution calculations.

…2524) When `SDF.create_from_mesh` (and `Mesh.build_sdf`) was called with `target_voxel_size` alone, the companion texture SDF silently fell back to `max_resolution=64`, producing a dramatically coarser secondary representation than the sparse NanoVDB grid.

coderabbitai · 2026-04-21T21:30:08Z

📝 Walkthrough

Walkthrough

This PR fixes a bug where SDF.create_from_mesh was ignoring the target_voxel_size parameter when constructing the texture SDF component. The fix forwards target_voxel_size to create_texture_sdf_from_mesh, which now applies parameter precedence rules consistent with sparse-grid construction.

Changes

Cohort / File(s)	Summary
Documentation `CHANGELOG.md`	Added changelog entry documenting the behavioral fix that `target_voxel_size` now controls texture SDF resolution.
Texture SDF module `newton/_src/geometry/sdf_texture.py`	Modified `create_texture_sdf_from_mesh` signature to accept `target_voxel_size: float \| None = None` and changed `max_resolution` parameter from `int = 64` to `int \| None = None`. Updated cell-size computation to prioritize `target_voxel_size` over `max_resolution` with `64` as final fallback.
SDF utilities module `newton/_src/geometry/sdf_utils.py`	Updated `SDF.create_from_mesh` to forward `target_voxel_size` directly to `create_texture_sdf_from_mesh` and pass `effective_max_resolution` (which may be `None`) instead of a pre-coerced integer, removing the intermediate `res` variable.
Test suite `newton/tests/test_sdf_texture.py`	Added CUDA-only test `test_build_sdf_target_voxel_size_propagates_to_texture` that verifies texture subgrid resolution changes appropriately when `target_voxel_size` is specified and validates the resulting texture grid spacing matches the input voxel size.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Honor target_voxel_size in texture SDF construction #2464: Directly related; both PRs modify create_texture_sdf_from_mesh and SDF.create_from_mesh to forward and apply target_voxel_size parameter changes.
Improve sdf creation performance #2475: Related through texture SDF pipeline modifications; adds parameter plumbing and behavior changes to the same texture SDF construction path.

Suggested reviewers

adenzler-nvidia
mzamoramora-nvidia

🚥 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 directly and clearly describes the main bug fix: forwarding target_voxel_size to the texture SDF path, which was previously being ignored.
Linked Issues check	✅ Passed	All requirements from issue `#2524` are met: target_voxel_size is now forwarded to create_texture_sdf_from_mesh with proper precedence policy, the 64-fallback is retained inside the texture module, and the regression test validates the fix.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to the bug fix: updates to sdf_texture.py and sdf_utils.py APIs, a regression test, and CHANGELOG documentation. No unrelated modifications detected.
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

⚔️ Resolve merge conflicts

Resolve merge conflict in branch ccrouzet/gh-2524-sdf-voxel-size

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

🧹 Nitpick comments (1)

newton/_src/geometry/sdf_utils.py (1)
356-367: Dead fallback on Line 367 after the fix.

With the new semantics, effective_max_resolution is None only when target_voxel_size is provided (otherwise it's max_resolution or 64). In that case _compute_sdf_from_shape_impl ignores max_resolution entirely and uses target_voxel_size, so the if effective_max_resolution is not None else 64 guard on Line 367 never matters. Minor cleanup — you can just pass effective_max_resolution (the impl signature still has a non-None default) or drop the ternary for clarity.
♻️ Suggested simplification
-            target_voxel_size=target_voxel_size,
-            max_resolution=effective_max_resolution if effective_max_resolution is not None else 64,
+            target_voxel_size=target_voxel_size,
+            max_resolution=effective_max_resolution or 64,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@newton/_src/geometry/sdf_utils.py` around lines 356 - 367, The ternary on the
max_resolution argument is dead — compute effective_max_resolution may be None
when target_voxel_size is used, and _compute_sdf_from_shape_impl already handles
a non-None default; replace the expression "effective_max_resolution if
effective_max_resolution is not None else 64" with just
"effective_max_resolution" when calling _compute_sdf_from_shape_impl (update the
call site that passes max_resolution and remove the unnecessary fallback).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@newton/_src/geometry/sdf_utils.py`:
- Around line 356-367: The ternary on the max_resolution argument is dead —
compute effective_max_resolution may be None when target_voxel_size is used, and
_compute_sdf_from_shape_impl already handles a non-None default; replace the
expression "effective_max_resolution if effective_max_resolution is not None
else 64" with just "effective_max_resolution" when calling
_compute_sdf_from_shape_impl (update the call site that passes max_resolution
and remove the unnecessary fallback).

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: d167fde9-731f-4cb8-9188-cdd19b4f5ca5

📥 Commits

Reviewing files that changed from the base of the PR and between b6489fa and 03cf0e0.

📒 Files selected for processing (4)

CHANGELOG.md
newton/_src/geometry/sdf_texture.py
newton/_src/geometry/sdf_utils.py
newton/tests/test_sdf_texture.py

codecov · 2026-04-21T21:50:38Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

adenzler-nvidia · 2026-04-22T07:27:59Z

This looks like a duplicate of #2464 - @christophercrouzet can you check?

nvtw · 2026-04-22T15:16:49Z

I wonder if we should kick out nano vdb based SDFs for collision detection (at least what the CollisionPipeline uses) completely. I kept them so far for unit testing the new(er) texture based SDF against the nano vdb based SDF.

christophercrouzet · 2026-04-22T19:34:20Z

Duplicate.

christophercrouzet requested review from eric-heiden and nvtw April 21, 2026 21:29

christophercrouzet self-assigned this Apr 21, 2026

coderabbitai Bot reviewed Apr 21, 2026

View reviewed changes

christophercrouzet closed this Apr 22, 2026

preist-nvidia added this to Newton Roadmap May 6, 2026

github-project-automation Bot moved this to Done in Newton Roadmap May 6, 2026

preist-nvidia added this to the 1.2 Release milestone May 6, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix `target_voxel_size` Being Ignored by Texture SDF#2525

Fix `target_voxel_size` Being Ignored by Texture SDF#2525
christophercrouzet wants to merge 1 commit into
newton-physics:mainfrom
christophercrouzet:ccrouzet/gh-2524-sdf-voxel-size

christophercrouzet commented Apr 21, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 21, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Uh oh!

coderabbitai Bot left a comment

Uh oh!

codecov Bot commented Apr 21, 2026

Uh oh!

adenzler-nvidia commented Apr 22, 2026

Uh oh!

nvtw commented Apr 22, 2026

Uh oh!

christophercrouzet commented Apr 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

christophercrouzet commented Apr 21, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Test plan

Bug fix

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Apr 21, 2026

Codecov Report

Uh oh!

adenzler-nvidia commented Apr 22, 2026

Uh oh!

nvtw commented Apr 22, 2026

Uh oh!

christophercrouzet commented Apr 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

christophercrouzet commented Apr 21, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 21, 2026 •

edited

Loading