Skip to content

Commit 1a0ad7f

Browse files
yutingyemeta-codesync[bot]
authored andcommitted
Fix scalar rasterizer packet iteration (#1569)
Summary: Pull Request resolved: #1569 PyMomentum renderer tests were skipped in OSS CI because the rasterizer crashed when built without SIMD. The failing path was `pymomentum.renderer` calling software rasterization helpers such as `rasterize_spheres`, which eventually iterate rasterizer primitives with `drjit::range<IntP>()`. This works in internal Buck builds because those builds use SIMD compiler flags, so Dr.Jit sees `IntP` as a packed SIMD packet and `drjit::range<IntP>()` produces packet indices as intended. OSS/PyPI-style builds can compile with SIMD disabled, where `IntP` becomes scalar (`Packet<int, 1>`). In that mode `drjit::range<IntP>()` goes through Dr.Jit's recursive static-array range implementation instead of the packed SIMD range path, and the one-dimensional scalar case underflows internally. That produces invalid indices and can crash the renderer. Fix this by adding a rasterizer-local `intPacketRange()` helper: - SIMD builds continue to use `drjit::range<IntP>()`, with a static assertion that `IntP` is actually packed. - Scalar/non-SIMD builds use explicit packet iteration based on `drjit::arange<IntP>()` and a tail mask. Update all rasterizer primitive loops to use the helper, then unskip the affected Linux and macOS PyMomentum renderer tests in OSS CI. The Windows `test_lighting` skip remains because it tracks a separate CMake stack-overflow issue. Reviewed By: cdtwigg Differential Revision: D111828395 fbshipit-source-id: 556184370c9decba521cde0e924b6a0e69330a0d
1 parent 3b45e02 commit 1a0ad7f

6 files changed

Lines changed: 89 additions & 15 deletions

File tree

‎CMakeLists.txt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,13 @@ if(MOMENTUM_BUILD_TESTING OR MOMENTUM_BUILD_PYMOMENTUM)
261261
set(CMAKE_POSITION_INDEPENDENT_CODE ON)
262262
endif()
263263

264+
if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND CMAKE_SYSTEM_PROCESSOR MATCHES "^(aarch64|arm64|ARM64)$")
265+
# GCC emits very noisy psABI notes for arm64 vector and Eigen types after its
266+
# GCC 10 ABI correction. We build the wheel with one toolchain, so these notes
267+
# do not indicate a mixed-ABI problem and only obscure real warnings in CI.
268+
add_compile_options($<$<COMPILE_LANGUAGE:CXX>:-Wno-psabi>)
269+
endif()
270+
264271
if(MSVC)
265272
add_compile_options(/bigobj)
266273
add_compile_options(/utf-8)

‎momentum/rasterizer/image.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ void alphaMatte(Span2f zBuffer, Span3f rgbBuffer, Span<T, 3> tgtImage, float alp
166166
drjit::scatter(tempBuffer.data(), rgba, colIndices);
167167
}
168168

169-
for (const auto [colIndices, colMask] : drjit::range<IntP>(imageWidthDownsampled)) {
169+
for (const auto [colIndices, colMask] : intPacketRange(imageWidthDownsampled)) {
170170
auto rgba = drjit::zeros<Vector4fP>();
171171
// Walk through the columns and sum horizontally
172172
for (int32_t offset = 0; offset < downsampleAmount; ++offset) {

‎momentum/rasterizer/rasterizer.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ void rasterizeLinesImp(
321321

322322
const Vector3f color_drjit = toEnokiVec(color);
323323

324-
for (auto [lineIndices, lineMask] : drjit::range<IntP>(nLines)) {
324+
for (auto [lineIndices, lineMask] : intPacketRange(nLines)) {
325325
const auto lineStart_world =
326326
drjit::gather<Vector3fP>(positions_world.data(), 2 * lineIndices + 0, lineMask);
327327
const auto lineEnd_world =
@@ -407,7 +407,7 @@ void rasterizeCirclesImp(
407407
const int combinedRadiusPixels =
408408
std::clamp<int>(static_cast<int>(std::ceil(combinedRadius)), 1, cameraSimd.imageHeight() - 1);
409409

410-
for (auto [circleIndices, circleMask] : drjit::range<IntP>(nCircles)) {
410+
for (auto [circleIndices, circleMask] : intPacketRange(nCircles)) {
411411
const auto center_world =
412412
drjit::gather<Vector3fP>(positions_world.data(), circleIndices, circleMask);
413413
const auto [center_window, validProj] = cameraSimd.worldToWindow(center_world);
@@ -490,7 +490,7 @@ void rasterizeWireframeImp(
490490

491491
const Vector3f color_drjit = toEnokiVec(color);
492492

493-
for (auto [triangleIndices, triangleMask] : drjit::range<IntP>(nTriangles)) {
493+
for (auto [triangleIndices, triangleMask] : intPacketRange(nTriangles)) {
494494
auto triangles_cur = drjit::gather<Vector3iP>(triangles.data(), triangleIndices, triangleMask);
495495
std::array<Vector3fP, 3> p_tri_window;
496496
IntP::MaskType validTriangles = triangleMask;

‎momentum/rasterizer/rasterizer_triangles.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -562,7 +562,7 @@ void rasterizeMeshImp(
562562
int* vertexIndexBufferPtr = dataOrNull(vertexIndexBuffer);
563563
int* triangleIndexBufferPtr = dataOrNull(triangleIndexBuffer);
564564

565-
for (auto [triangleIndices, triangleMask] : drjit::range<IntP>(nTriangles)) {
565+
for (auto [triangleIndices, triangleMask] : intPacketRange(nTriangles)) {
566566
auto triangles_cur = drjit::gather<Vector3iP>(triangles.data(), triangleIndices, triangleMask);
567567
std::array<Vector3fP, 3> p_tri_window;
568568
Matrix3fP p_tri_eye;
@@ -746,7 +746,7 @@ void rasterizeSplatsImp(
746746
const std::array<Vector3f, 4> quadTextureCoords = {
747747
Vector3f(-1, -1, 0), Vector3f(1, -1, 0), Vector3f(1, 1, 0), Vector3f(-1, 1, 0)};
748748

749-
for (auto [splatIndices, splatMask] : drjit::range<IntP>(nSplats)) {
749+
for (auto [splatIndices, splatMask] : intPacketRange(nSplats)) {
750750
auto position_world = drjit::gather<Vector3fP>(positions_world.data(), splatIndices, splatMask);
751751
auto normal_world = drjit::gather<Vector3fP>(normals_world.data(), splatIndices, splatMask);
752752

‎momentum/rasterizer/utility.h‎

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,14 @@
1010
#include <drjit/array.h>
1111
#include <drjit/fwd.h>
1212
#include <drjit/matrix.h>
13+
#include <drjit/util.h>
1314
#include <mdspan/mdspan.hpp>
1415
#include <momentum/common/exception.h>
1516
#include <momentum/rasterizer/fwd.h>
1617
#include <Eigen/Core>
1718
#include <Eigen/Geometry>
1819
#include <array>
20+
#include <limits>
1921
#include <memory>
2022
#include <sstream>
2123
#include <utility>
@@ -25,7 +27,76 @@ namespace momentum::rasterizer {
2527

2628
using index_t = std::ptrdiff_t;
2729

28-
// Actual implementations
30+
// drjit::range<IntP>() assumes IntP is a packed SIMD type. Use it only in
31+
// SIMD builds; scalar/non-SIMD builds need explicit packet iteration.
32+
#if defined(MOMENTUM_ENABLE_SIMD) && MOMENTUM_ENABLE_SIMD
33+
/// Returns integer packet indices and validity masks for rasterizer primitive loops.
34+
inline auto intPacketRange(index_t count) {
35+
static_assert(IntP::IsPacked, "MOMENTUM_ENABLE_SIMD requires packed Dr.Jit IntP packets.");
36+
return drjit::range<IntP>(count);
37+
}
38+
#else
39+
/// Scalar fallback range yielding IntP index packets and validity masks.
40+
class ScalarIntPacketRange {
41+
public:
42+
/// Creates a range over integer indices in [0, count).
43+
explicit ScalarIntPacketRange(index_t count) : count_(count) {
44+
MT_THROW_IF(
45+
count < 0 || count > std::numeric_limits<int>::max(),
46+
"Packet range count {} is outside the supported int range.",
47+
count);
48+
}
49+
50+
/// Iterator for ScalarIntPacketRange.
51+
class Iterator {
52+
public:
53+
/// Creates an iterator at a packet base offset.
54+
Iterator(index_t base, index_t count) : base_(base), count_(count) {}
55+
56+
/// Returns true when the iterator has not reached the end sentinel.
57+
bool operator!=(const Iterator& other) const {
58+
return base_ != other.base_;
59+
}
60+
61+
/// Advances to the next packet of indices.
62+
Iterator& operator++() {
63+
base_ += static_cast<index_t>(kSimdPacketSize);
64+
return *this;
65+
}
66+
67+
/// Returns the current index packet and a mask for lanes below count.
68+
auto operator*() const {
69+
const IntP indices = IntP(static_cast<int>(base_)) + drjit::arange<IntP>();
70+
const auto mask = indices < IntP(static_cast<int>(count_));
71+
return std::pair{indices, mask};
72+
}
73+
74+
private:
75+
index_t base_;
76+
index_t count_;
77+
};
78+
79+
/// Returns an iterator for the first packet.
80+
Iterator begin() const {
81+
return Iterator(0, count_);
82+
}
83+
84+
/// Returns the sentinel iterator after the final packet.
85+
Iterator end() const {
86+
constexpr index_t kPacketSize = static_cast<index_t>(kSimdPacketSize);
87+
const index_t endBase = ((count_ + kPacketSize - 1) / kPacketSize) * kPacketSize;
88+
return Iterator(endBase, count_);
89+
}
90+
91+
private:
92+
index_t count_;
93+
};
94+
95+
/// Returns integer packet indices and validity masks for scalar rasterizer builds.
96+
inline ScalarIntPacketRange intPacketRange(index_t count) {
97+
return ScalarIntPacketRange(count);
98+
}
99+
#endif
29100

30101
template <std::size_t R>
31102
std::string formatTensorSizes(const std::array<index_t, R>& extents) {

‎pixi.toml‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -328,8 +328,7 @@ build_py = { cmd = "pip install . -v --no-build-isolation", env = { FBXSDK_PATH
328328
""", MOMENTUM_ENABLE_SIMD = "ON", TORCH_CUDA_ARCH_LIST = "5.0;6.0;6.1;7.0;7.5;8.0;8.6;8.9;9.0+PTX" }, depends-on = [
329329
"install_deps",
330330
] }
331-
# TODO(T219163027): Skip TestRendering entirely on Linux — drjit segfaults in scalar SIMD mode (test_lighting, test_rasterize_*, etc.)
332-
test_py = { cmd = "pytest pymomentum/test/ -k 'not TestRendering'", env = { MOMENTUM_MODELS_PATH = "momentum/" }, depends-on = [
331+
test_py = { cmd = "pytest pymomentum/test/", env = { MOMENTUM_MODELS_PATH = "momentum/" }, depends-on = [
333332
"build_py",
334333
] }
335334
doc_py = { cmd = "sphinx-build -a -E -W -b html pymomentum/doc build/python_api_doc", depends-on = [
@@ -356,8 +355,7 @@ build_py = { cmd = "pip install . -v --no-build-isolation", env = { CMAKE_ARGS =
356355
-DBUILD_SHARED_LIBS=OFF \
357356
-DCMAKE_CXX_FLAGS=-fsigned-char
358357
""", MOMENTUM_ENABLE_SIMD = "ON" } }
359-
# TODO(T219163027): Skip TestRendering entirely on Linux — drjit segfaults in scalar SIMD mode
360-
test_py = { cmd = "pytest pymomentum/test/ -k 'not TestRendering'", env = { MOMENTUM_MODELS_PATH = "momentum/" }, depends-on = [
358+
test_py = { cmd = "pytest pymomentum/test/", env = { MOMENTUM_MODELS_PATH = "momentum/" }, depends-on = [
361359
"build_py",
362360
] }
363361

@@ -380,8 +378,7 @@ build_py = { cmd = "pip install . -v", env = { CMAKE_ARGS = """
380378
-DBUILD_SHARED_LIBS=OFF
381379
""", MOMENTUM_ENABLE_FBX_SAVING = "OFF", MOMENTUM_ENABLE_SIMD = "ON" } }
382380
# TODO: Remove -k option, once saving tests are disabled programmatically when Autodesk FBX SDK is unavailable (FBX loading always works via OpenFBX)
383-
# TODO(T219163027): Remove test_lighting skip once drjit::range<IntP> crash in scalar SIMD mode is fixed
384-
test_py = { cmd = "pytest pymomentum/test/ -k 'not ((TestFBXIO and (test_save_motions_with_joint_params or test_save_motions_with_model_params)) or test_lighting)'", env = { MOMENTUM_MODELS_PATH = "momentum/" }, depends-on = [
381+
test_py = { cmd = "pytest pymomentum/test/ -k 'not (TestFBXIO and (test_save_motions_with_joint_params or test_save_motions_with_model_params))'", env = { MOMENTUM_MODELS_PATH = "momentum/" }, depends-on = [
385382
"build_py", "install",
386383
] }
387384
doc_py = { cmd = "sphinx-build -a -E -W -b html pymomentum/doc build/python_api_doc", depends-on = [
@@ -411,8 +408,7 @@ build_py = { cmd = "pip install . -v", env = { CMAKE_ARGS = """
411408
-DBUILD_SHARED_LIBS=OFF
412409
""", MOMENTUM_ENABLE_FBX_SAVING = "OFF", MOMENTUM_ENABLE_SIMD = "ON" } }
413410
# TODO: Remove -k option, once saving tests are disabled programmatically when Autodesk FBX SDK is unavailable (FBX loading always works via OpenFBX)
414-
# TODO(T219163027): Remove test_lighting skip once drjit::range<IntP> crash in scalar SIMD mode is fixed
415-
test_py = { cmd = "pytest pymomentum/test/ -k 'not ((TestFBXIO and (test_save_motions_with_joint_params or test_save_motions_with_model_params)) or test_lighting)'", env = { MOMENTUM_MODELS_PATH = "momentum/" }, depends-on = [
411+
test_py = { cmd = "pytest pymomentum/test/ -k 'not (TestFBXIO and (test_save_motions_with_joint_params or test_save_motions_with_model_params))'", env = { MOMENTUM_MODELS_PATH = "momentum/" }, depends-on = [
416412
"build_py", "install",
417413
] }
418414
doc_py = { cmd = "sphinx-build -a -E -W -b html pymomentum/doc build/python_api_doc", depends-on = [

0 commit comments

Comments
 (0)