Skip to content

Fix coordinate indexing in frustum face culling - #2044

Open
tandede wants to merge 1 commit into
facebookresearch:mainfrom
tandede:fix/frustum-culling-coordinate-axis
Open

Fix coordinate indexing in frustum face culling#2044
tandede wants to merge 1 commit into
facebookresearch:mainfrom
tandede:fix/frustum-culling-coordinate-axis

Conversation

@tandede

@tandede tandede commented Aug 24, 2026

Copy link
Copy Markdown

Summary

Fixes the coordinate indexing used by frustum face culling.

face_verts has shape [F, 3, 3], where the last two dimensions are vertex and xyz coordinate respectively. _get_culled_faces previously indexed it as face_verts[:, axis], which selected one vertex from every face instead of selecting the requested coordinate from all three vertices. The subsequent reduction could therefore miss faces fully outside a clipping plane and cull faces that only had one outside vertex.

This change selects the coordinate with face_verts[:, :, axis] and uses all(dim=1) to express the intended condition directly: a face is culled only when all three vertices lie outside the same frustum plane.

The regression test covers all six frustum planes and verifies that a face intersecting the left plane stays visible for three different vertex orders.

Fixes #1936.

Validation

  • The focused regression test passes with all 9 subcases; the pre-fix implementation fails 7 of them.
  • An independent oracle over 4,096 seeded random triangles has zero mismatches with the fix. The previous implementation produces 604 false positives and 603 false negatives on the same inputs.
  • Black, usort, flake8, and git diff --check pass for the changed files.
  • The remaining tests in tests/test_render_meshes_clipped.py require CUDA and cannot run in the local CPU-only PyTorch environment; the focused regression is CPU-only, and the repository CI will cover the CUDA test matrix.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 24, 2026
@meta-codesync

meta-codesync Bot commented Aug 24, 2026

Copy link
Copy Markdown

@bottler has imported this pull request. If you are a Meta employee, you can view this in D117200371.

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

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug found in _get_culled_faces()

1 participant