feat(engine): fill 3 cross-engine feature gaps (clouds, HRTF, directi…#491
Merged
Conversation
…onal streaming)
Cross-checked SparkEngine against modern engines (Unreal 5, Unity, Godot,
Bevy, Wicked, Filament, O3DE, The Forge, Decima) to find genuinely
missing features, then implemented the three remaining gaps:
1. VolumetricCloudSystem (Graphics/VolumetricClouds.{h,cpp}) — Decima-
style ray-marched clouds with two-scale Perlin+Worley noise, 2D
weather map, height gradient per cloud type, two-lobe Henyey-
Greenstein phase, light-march self-shadowing. CPU reference +
GPU feeder (R8 3D + RG8 2D). Wired into GameplayLifecycleShared.
2. HRTFProcessor (Audio/HRTFProcessor.{h,cpp}) — portable analytical
binaural renderer: Woodworth ITD, cos^2 ILD + distance falloff,
one-pole head-shadow low-pass on far ear, per-block gain cross-fade.
No external HRIR dataset required; measured HRIRs can drop in later.
3. Directional predictive streaming (SeamlessAreaManager) — added
StreamingConfig::directionalBias + directionalDotThreshold, new
DirectionalEffectiveDistance() helper reduces radius/sort distance
for areas along the velocity / camera vector (RAGE/Decima style).
23 new tests across 3 files. Full suite: 5614/5614 passing.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: baf5f3fe87
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+384
to
+385
| const float dA = DirectionalEffectiveDistance(areaA.distanceToPlayer, areaA.definition); | ||
| const float dB = DirectionalEffectiveDistance(areaB.distanceToPlayer, areaB.definition); |
There was a problem hiding this comment.
The sort comparator now depends on DirectionalEffectiveDistance, which reads live player state each time it is invoked. Because SetPlayerState() is explicitly allowed from other threads, player velocity/direction can change during std::sort, so the comparator may return inconsistent results for the same pair across calls. That violates std::sort’s strict-weak-ordering requirement and can produce undefined behavior or nondeterministic load ordering under normal multithreaded updates.
Useful? React with 👍 / 👎.
Comment on lines
+336
to
+337
| if (std::abs(dirY) < 1e-4f) | ||
| return -1.0f; |
There was a problem hiding this comment.
The slab intersection helper returns -1 whenever dirY is near zero, which makes both intersections invalid and causes an early return with full transmittance. This drops any ray that is parallel to the cloud layer even if its origin is already between baseAltitude and topAltitude, so horizon-level or lateral cloud rays incorrectly render as cloud-free.
Useful? React with 👍 / 👎.
Contributor
Code Coverage (GCC + lcov)Per-Subsystem Coverage
Total: 50.5% (27707/54912 lines) |
Contributor
❌ CI Error ReportFailed jobs: clang-tidy, linux-clang-Release, macos-Debug, macos-Release Build ErrorsOther errors (2)Compiler Warnings (6)Updated: 2026-04-18T01:22:59Z — this comment is updated in-place, not duplicated. |
check-format job was failing on pre-existing `const &` vs `const&` qualifier spacing in Utils/Expected.h (introduced by 6a6f098 before this branch diverged). Purely cosmetic — no semantic change.
Contributor
Code Coverage (GCC + lcov)Per-Subsystem Coverage
Total: 50.5% (27708/54912 lines) |
Two real bugs flagged by the Codex automated review.
1. SeamlessAreaManager — strict-weak-ordering violation in load sort
`DirectionalEffectiveDistance()` read m_playerPosition / m_playerVelocity
under the player mutex on every comparator call. Because SetPlayerState()
may be invoked from physics/gameplay threads, the player snapshot can
change mid-sort, breaking std::sort's strict-weak-ordering requirement
and producing nondeterministic load ordering or undefined behaviour.
Fix: snapshot position+direction once per Update tick via the new
SnapshotPlayerDirection() helper, build a Candidate{id, priority,
effectiveDistance} array up-front, and sort that array with a pure
comparator that no longer touches live state.
2. VolumetricClouds — horizontal rays starting inside the cloud slab
were silently dropped. The slabDist() helper returned -1 whenever
|dirY| < 1e-4, so a lateral ray (e.g. flying through cumulus at
altitude) reported full transmittance and zero luminance, even when
its origin was clearly between baseAltitude and topAltitude.
Fix: detect the |dirY| ≈ 0 case explicitly. If origin is inside the
slab, march [0, maxDistance]; if outside, early-out as before.
Two new regression tests cover the horizontal-ray paths
(HorizontalRayInsideSlab + HorizontalRayOutsideSlab). Full suite:
5616/5616 passing.
Contributor
Code Coverage (GCC + lcov)Per-Subsystem Coverage
Total: 50.5% (27711/54914 lines) |
build-linux-clang (Release) has been failing with: /usr/bin/ld: ../lib/libJolt.a: error adding symbols: file format not recognized clang++: error: linker command failed with exit code 1 Root cause: actions/cache restores a build/ directory from a previous run. When apt-get upgrades clang / binutils between runs, the cached libJolt.a can end up in a format the current linker no longer recognizes — similar to the existing PCH-vs-unistd_64.h mtime mismatch that this job already handles. Extend the existing stale-artifact cleanup to also scan lib*.a archives. Each one is probed with \`ar t\`; any archive that can't be listed is purged, forcing CMake to rebuild it on the next step. ccache still keeps .o compilation fast. Pre-existing failure (also hit PR #490 on the same job); fixed here.
Two pre-existing failures that were already present on PR #490 / Working: 1. NetworkStack_UDPInitializeSucceeds on Windows `UDPTransport::Initialize()` called ::socket() without calling WSAStartup() — on Windows that fails with WSANOTINITIALISED. The only call to WSAStartup() lived inside `NetworkManager::Initialize()`, so any test or subsystem creating a bare NetworkStack / UDPTransport couldn't open a socket. Fix: add a reference-counted `WinsockScope` RAII helper inside UDPTransport.h (Windows-only). Each UDPTransport takes one reference in Initialize() and releases it in Shutdown(); the first scope calls WSAStartup, the last calls WSACleanup. 2. build-macos (Debug/Release) — same `libJolt.a: error adding symbols: file format not recognized` as linux-clang. apt-get / brew upgrades leave stale archives in actions/cache, which the current ld can't parse. Extended the build step with the same `ar t`-based probe + rm that we just added to linux-clang. Local: 5616/5616 tests pass, including the now-portable NetworkStack_UDPInitializeSucceeds.
Contributor
Code Coverage (GCC + lcov)Per-Subsystem Coverage
Total: 50.5% (27712/54914 lines) |
Previous attempt with `ar t` probe was insufficient — `ar t` only lists archive member names and succeeds on archives whose embedded .o files have wrong object format. The linker then still fails with "file format not recognized" on the stale archive. Simpler and actually-correct fix: delete every lib*.a from the restored build/ directory. CMake re-links them from .o files that come straight out of ccache, so the extra cost is a couple of seconds of archiving. Applied to both build-linux-clang and build-macos.
Contributor
Code Coverage (GCC + lcov)Per-Subsystem Coverage
Total: 50.5% (27712/54914 lines) |
Krilliac
pushed a commit
that referenced
this pull request
Apr 18, 2026
Root cause of the linux-clang-Release / macos-{Debug,Release} linker
failures on PR #491 was NOT a stale CI cache — a fresh local build
reproduced it. Extracting any member of build/lib/libJolt.a showed
LLVM IR bitcode, not ELF. GNU ld / ld64 then rejects the archive with:
libJolt.a: error adding symbols: file format not recognized
clang++: error: linker command failed with exit code 1
Jolt's CMake wrapper was forcing `INTERPROCEDURAL_OPTIMIZATION=ON`
whenever CMAKE_BUILD_TYPE was Release/MinSizeRel/RelWithDebInfo,
regardless of the top-level `ENABLE_LTO` option. The linux-clang CI
passes `-DENABLE_LTO=OFF` expecting native objects everywhere, but
Jolt still produced bitcode — a classic ABI/format mismatch that only
the thin-LTO-aware lld or llvm-ar can resolve.
Fix:
- CMakeLists.txt: gate Jolt's IPO on `ENABLE_LTO AND <release config>`
so the engine and Jolt stay in the same object format.
- build.yml: add `-DENABLE_LTO=OFF` to the macOS job. The job already
unconditionally purges `lib*.a` as a workaround — removing the
underlying mismatch lets that step become a no-op.
Verified locally: after the fix, `ar x libJolt.a AABBTreeBuilder.cpp.o`
yields ELF relocatable instead of LLVM IR bitcode.
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
2 participants
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.
…onal streaming)
VolumetricCloudSystem (Graphics/VolumetricClouds.{h,cpp}) — Decima- style ray-marched clouds with two-scale Perlin+Worley noise, 2D weather map, height gradient per cloud type, two-lobe Henyey- Greenstein phase, light-march self-shadowing. CPU reference + GPU feeder (R8 3D + RG8 2D). Wired into GameplayLifecycleShared.
HRTFProcessor (Audio/HRTFProcessor.{h,cpp}) — portable analytical binaural renderer: Woodworth ITD, cos^2 ILD + distance falloff, one-pole head-shadow low-pass on far ear, per-block gain cross-fade. No external HRIR dataset required; measured HRIRs can drop in later.
Directional predictive streaming (SeamlessAreaManager) — added StreamingConfig::directionalBias + directionalDotThreshold, new DirectionalEffectiveDistance() helper reduces radius/sort distance for areas along the velocity / camera vector (RAGE/Decima style).
23 new tests across 3 files. Full suite: 5614/5614 passing.