Add clean pure-C11 port of the lightrt_c BVH binding#3
Merged
Conversation
Guard <thread>/<mutex>/<condition_variable>/<queue> includes and the thread-pool TaskSystem behind #ifndef __EMSCRIPTEN__. Under WASM, provide an inline single-threaded TaskSystem stub (submit/parallelFor run the body directly) and route traverseBatch* / buildRecursive work-stealing through a single-threaded path. No behavior change for native builds. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lightrt_c.c is a standalone C11 reimplementation of the lightrt_c.h API with no dependency on the C++ library (lightrt.hh). It builds its own generic BVH (binned SAH, 16 bins, ported from MMapGenericBVH::buildRecursive) and traverses it with a stack-based front-to-back walk, calling the user's fp64 intersection callback per candidate — preserving the fp32-broadphase / fp64-callback design of the existing C++ wrapper (lightrt_c.cc). Build integration: - CMake gains LIGHTRT_PURE_C_BINDING (default OFF) to select lightrt_c.c over lightrt_c.cc in the library; both expose the same API, so exactly one links. - Project now enables the C language (C11) for the pure-C sources. - New lightrt_c_test target (CTest) compiles lightrt_c.c directly and validates closest-hit results against an O(N) brute-force reference over random spheres. Verified: test passes (0 mismatches, exact t-match), clean under -Wall -Wextra -Wpedantic and ASan+UBSan; edge cases (N=1 leaf-only tree, axis-aligned/zero-dir rays, misses, rebuild, empty scene, NULL callbacks) pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add a `pure_c_binding` boolean option (default false) that swaps lightrt_c.c for lightrt_c.cc in the library, matching CMake's LIGHTRT_PURE_C_BINDING, plus a lightrt_c_test target wired into `meson test`. This also fixes several pre-existing bugs that prevented meson.build from configuring under modern meson (1.10): - enable the C language and set c_std=c11 - 'c++' -> 'cpp' (meson's C++ language id) in add_project_arguments, and pass the flags as a list rather than one combined string - link executables with `link_with:` instead of a positional arg after the `sources:` keyword (positional-after-keyword is a syntax error) - replace the invalid bare `install()` call with install_headers() Verified: both option states configure; the static library archives in the pure-C config (lightrt.cc.o + lightrt_c.c.o); `meson test` passes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a standalone pure-C11 implementation of the lightrt_c.h API (lightrt_c.c) and wires build-system switches (CMake + Meson) to choose between the existing C++ wrapper (lightrt_c.cc) and the new pure-C port. It also adds a C-based correctness test for the binding and includes Emscripten/WASM guards/stubs around the thread-pool TaskSystem.
Changes:
- Add
lightrt_c.c: a self-contained C11 BVH build/traversal + callback intersection backend implementinglightrt_c.h. - Add
tests/test_lightrt_c.cand register it in both CMake and Meson. - Add build options to select the pure-C binding (
LIGHTRT_PURE_C_BINDING/pure_c_binding) and add Emscripten stubs/guards for threading.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
lightrt_c.c |
New pure-C11 implementation of the lightrt_c.h BVH binding. |
tests/test_lightrt_c.c |
New correctness test comparing BVH results vs brute-force analytic sphere hits. |
CMakeLists.txt |
Enables C11, adds option to select binding implementation, adds CTest target for the C binding. |
meson.build |
Adds C language, adds option-controlled binding source selection, fixes Meson configuration issues, adds test executable. |
meson_options.txt |
Adds pure_c_binding Meson option. |
lightrt.hh |
Adds Emscripten guards around thread/queue includes and introduces a single-threaded TaskSystem stub for WASM. |
lightrt.cc |
Adds Emscripten guards around TaskSystem globals and adjusts batch traversal threading logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (num_rays == 0) return; | ||
|
|
||
| #ifdef __EMSCRIPTEN__ | ||
| if (num_threads == 0) num_threads = 1; |
| if (num_rays == 0) return; | ||
|
|
||
| #ifdef __EMSCRIPTEN__ | ||
| if (num_threads == 0) num_threads = 1; |
| if (num_rays == 0) return; | ||
|
|
||
| #ifdef __EMSCRIPTEN__ | ||
| if (num_threads == 0) num_threads = 1; |
Comment on lines
+9808
to
9812
| #ifdef __EMSCRIPTEN__ | ||
| if (num_threads == 0) num_threads = 1; | ||
| #else | ||
| if (num_threads == 0) num_threads = std::thread::hardware_concurrency(); | ||
| if (num_threads < 1) num_threads = 1; |
| uint32_t left = n->u.inner.left; | ||
| uint32_t right = n->u.inner.right; | ||
| uint32_t axis = (n->flags >> 1) & 0x3u; | ||
| if (sp + 2 > LRT_STACK_SIZE) continue; /* guard (very deep tree) */ |
Comment on lines
+79
to
+83
| static unsigned long g_seed = 0x12345678ul; | ||
| static double frand(void) { | ||
| g_seed = g_seed * 6364136223846793005ul + 1442695040888963407ul; | ||
| return (double)((g_seed >> 33) & 0x7fffffff) / (double)0x7fffffff; | ||
| } |
Comment on lines
+91
to
+94
| sphere *spheres = (sphere *)malloc(N * sizeof(sphere)); | ||
| for (unsigned i = 0; i < N; i++) { | ||
| spheres[i].c[0] = frange(-WORLD, WORLD); | ||
| spheres[i].c[1] = frange(-WORLD, WORLD); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
lightrt_c.c— a standalone pure-C11 reimplementation of thelightrt_c.hAPI with no dependency on the C++ library (lightrt.hh). The existinglightrt_c.ccis a thin C++ wrapper overlightrt::MMapGenericBVH; this new file builds and traverses its own BVH entirely in C.Both expose the same
lightrt_c.hAPI, so exactly one is linked into the library (selectable via a build option).What the port does
MMapGenericBVH::buildRecursive— same cost function, axis selection, and median-split fallback. Nodes are preallocated to2·N, so the array never reallocates mid-build.AABB::intersect.doubleso the user's intersection callback can solve analytic surfaces at full precision.Build integration
LIGHTRT_PURE_C_BINDINGoption (defaultOFF) selectslightrt_c.coverlightrt_c.cc. Project now enables the C language at C11. Newlightrt_c_testCTest target compileslightrt_c.cdirectly (no C++).pure_c_bindingoption (defaultfalse) mirroring CMake, pluslightrt_c_testwired intomeson test. Also fixes several pre-existing bugs that preventedmeson.buildfrom configuring under modern meson ('c++'→'cpp', flags-as-list,link_with:, invalid bareinstall(), C language/standard).Also included
TaskSystem(guards<thread>/<mutex>/etc. behind#ifndef __EMSCRIPTEN__with an inline single-threaded stub). No behavior change for native builds.Testing
tests/test_lightrt_c.cbuilds a scene of analytic spheres and cross-checks closest-hit results against an O(N) brute-force reference.tmatch-Wall -Wextra -Wpedanticand ASan + UBSanctest/meson testpass🤖 Generated with Claude Code