mirror of
https://github.com/facebookresearch/pytorch3d.git
synced 2026-08-18 22:05:43 +08:00
a5093b158b
Summary: Fixes https://github.com/facebookresearch/pytorch3d/issues/1934. Two independent bugs in the CPU backend. ### 1. Degenerate-triangle filter discards valid faces `marching_cubes_cpu.cpp`, `marching_cubes.py` `tri.clear()` and `ps.clear()` sit inside the degeneracy check, so the buffers only reset when a triangle is *accepted*. Once a cube's first triangle is degenerate, `ps[0..2]` stay frozen on it, and every subsequent triangle in that cube fails the same stale check and is dropped. Fixed by gating on `ps.size() == 3` and clearing unconditionally. The old code could only ever drop faces, never emit incorrect ones, so this is strictly additive. ### 2. Edge hash computed in float32 `marching_cubes_utils.h` `p[v].x/y/z` hold integral coordinates but are stored as `float`, so `x + y*W + z*W*H` evaluates entirely in float32 before truncating to `int`. float32 is exact only to 2²⁴ − 1 = 16,777,215 — and 256³ maxes out at exactly that value. At 512³ the maximum id is 134,217,727, where float32 spacing is 8, so distinct vertices collide on one id and `uniq_edge_id` merges them. Fixed by widening `W/H/D` to `int64_t` and casting each coordinate before multiplying. Also tightens the stride from `(W + W*H + W*H*D)` to `W*H*D`. Raises the CPU ceiling from 256³ to 1448³. Scope is the CPU path only. `marching_cubes_naive` was never affected (Python ints are arbitrary-precision), and neither was CUDA: `hashVpair` there computes ids in `uint` rather than `float`, so it has no 2²⁴ cliff, and `MarchingCubes` already rejects volumes above 1024³ before the CUDA kernel runs. The new `TORCH_CHECK` bound and the "~1448³" note in the new comments describe `MarchingCubesCpu` only. ### Verification Ellipsoid SDF (0.1, 1, 1), `isolevel=0.0`, identical input tensors on both devices. | Resolution | CUDA V | CUDA F | CPU V | CPU F | Degenerate dropped | | -- | -- | -- | -- | -- | -- | | 32³ | 1,664 | 3,324 | 1,664 | 3,324 | 0 | | 64³ | 7,312 | 14,620 | 7,312 | 14,620 | 0 | | 128³ | 30,168 | 60,332 | 30,168 | 60,332 | 0 | | 256³ | 122,448 | 244,892 | 122,448 | 243,996 | 896 | | 512³ | 491,944 | 983,884 | 491,944 | 976,140 | 7,744 | Machine: Arch Linux, RTX 4080, Ryzen 7 7800X3D Vertex counts now match CUDA exactly at every resolution; 512³ previously produced 176,121. The remaining face gap is entirely degenerate geometry — the CUDA mesh at 512³ contains exactly 7,744 zero-area triangles. ### Tests `test_degenerate_triangle_keeps_later_faces` — a 2×2×2 volume at `isolevel=1` chosen so the cube's four candidate triangles collapse onto its four outside corners: triangles 1 and 4 become degenerate, 2 and 3 stay valid. Pre-fix, the first degeneracy suppresses the rest and the mesh comes back empty; post-fix it is the expected quad. Asserts both `marching_cubes_naive` and the C++ extension. `test_large_grid_edge_ids` — a 2×2×4,200,000 volume (~67MB, ~0.1s) holding 16 isolated interior points on the highest-id grid row, positioned so grid-point ids straddle 2²⁴. Each point cuts exactly four grid edges, so the 64-vertex expectation is derived geometrically rather than copied from output. Pre-fix, a hash collision merges two edges and one vertex is lost. All 26 pre-existing tests in `test_marching_cubes.py` pass **unchanged**. That includes `test_cube_no_duplicate_verts` (`isolevel=1`) and `test_sphere` (`isolevel=64`), which both exercise the degenerate path but whose output is identical before and after the fix — so no existing expectation was edited and `sphere_level64.pickle` does not need regenerating. The imported diff contained no test file. The two tests above were written during import and differ from the tests described in the upstream PR description. *Analysis and write-up done collaboratively with AI, figures from testing are done on my own machine and have been checked.* Pull Request resolved: https://github.com/facebookresearch/pytorch3d/pull/2043 Test Plan: ``` buck2 test fbcode//vision/fair/pytorch3d:tests -- --regex 'test_marching_cubes' ``` `Pass 28. Fail 0.` — 26 pre-existing tests plus the 2 new regression tests. Reverting all three source hunks to their pre-fix state and re-running the same command: both new tests fail (`test_degenerate_triangle_keeps_later_faces` returns an empty mesh instead of 4 verts / 2 faces; `test_large_grid_edge_ids` returns 31 verts instead of 32) and all 26 pre-existing tests still pass. The new tests are therefore pinned to exactly this change, and the change breaks nothing that was already covered. Reviewed By: MichaelRamamonjisoa Differential Revision: D115424433 Pulled By: bottler fbshipit-source-id: 547a260010b94253f52f3a3c223d4c4fa78a7ee6