Claude/engine roadmap 8 kc2c#458
Merged
Merged
Conversation
Converts the first Tier 2 graphics orphan utility (GTAOEffect.h, 447 lines, zero callers) into an actively wired render feature. GTAO is slotted first in PostProcessPass so ambient occlusion modulates scene lighting at HDR resolution before Bloom extracts highlights. Mirrors the existing 14-effect pattern: inline pixel shader literal in CompileEffectShaders, settings accessor pair, ProcessPass switch case, and a standalone Shaders/HLSL/GTAOPS.hlsl for offline build-time validation. Tests: 8 new CPU-reference cases against the real GTAOEffect class (defaults, init lifecycle, bad-dimensions rejection, open-sky flat depth, zero-depth early-out, wall-occluder AO, denoiser identity, HLSL source shape). Total 4370 tests passing on Linux, 0 failures. https://claude.ai/code/session_01QoefgpfnywL2WtN99Eivrm
Sync the Phase I knowledge entry with the earlier draft that was recovered by the working tree. Content is equivalent — same items, same build results, reworded sections. https://claude.ai/code/session_01QoefgpfnywL2WtN99Eivrm
![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: aacf3d11e9
ℹ️ 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".
| */ | ||
| enum class PostProcessPass | ||
| { | ||
| GTAO, ///< Ground Truth Ambient Occlusion (horizon-based screen-space AO) |
There was a problem hiding this comment.
Adding PostProcessPass::GTAO here introduces a new pass that defaults to disabled (m_passEnabled is zero-initialized) but this commit does not add a way to turn it on through the engine’s existing control path: the only in-repo callers of SetEffectEnabled are the pp_enable/pp_disable command maps in SparkEngine/Source/Core/SubsystemConsoleCommands.cpp (around lines 501-551), and those maps have no gtao entry. In practice, users cannot activate the new pass at runtime, so the feature remains effectively unreachable.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Already addressed in Phase J (commit 262325a). Both pp_enable and pp_disable console command maps in SparkEngine/Source/Core/SubsystemConsoleCommands.cpp now include {"gtao", Graphics::PostProcessPass::GTAO} (lines 502 and 540 on the current branch tip), so users can pp_enable gtao / pp_disable gtao from the console. Phase J added the same entry for the new ssaotemporal pass at the same time.
Generated by Claude Code
Contributor
Code Coverage (GCC + lcov)Per-Subsystem Coverage
Total: 51.4% (20062/39034 lines) |
Wire SSAOTemporal, RenderTargetPool, GPUDebugMarkers, and GPUTimestampQuery into the real PostProcessingPipeline. SSAOTemporal lands as the 16th post-process pass (variance-clipped AO denoiser slotted after GTAO); RenderTargetPool is owned per-pipeline and ticked each frame; every ProcessPass is bracketed by a ScopedGPUEvent for PIX/RenderDoc captures; per-pass GPU timestamps replace std::chrono CPU-side measurements when a D3D11 device is attached. Metrics tables are now static_assert'd against PostProcessPass::Count so future enum additions can't silently desync. pp_enable/pp_disable console commands gain gtao + ssaotemporal keywords. 25 new tests (TestSSAOTemporalFilter, TestPostProcessingPipelinePhaseJ) cover the CPU reference path and the portable accessor surface; full suite 4395 passed, 0 failed on linux-gcc-release. https://claude.ai/code/session_01QoefgpfnywL2WtN99Eivrm
Rewrite the Phase J summary with fuller per-item detail, test coverage lists, and a Phase K candidate roadmap. Content-equivalent but more comprehensive than the original commit. https://claude.ai/code/session_01QoefgpfnywL2WtN99Eivrm
Contributor
Code Coverage (GCC + lcov)Per-Subsystem Coverage
Total: 51.4% (20244/39375 lines) |
… volumes Wire the Tier 2 graphics orphan VolumeSystem.h into PostProcessingPipeline as a live per-pipeline instance. Users create global or local volumes via pipeline->GetVolumeManager(), set the camera position with SetCameraPosition, and on each Process() call the blended VolumeStack (exposure compensation, bloom, color grading, fog) is pushed into the pipeline's effect settings via the new public ApplyVolumeStack() method. Only fields whose overrideState was set by a real volume land on the settings, so hand-authored values survive volume-less frames. K2 fixes a latent bug in VolumeSystem.h: all four Interpolate methods now propagate overrideState = true for every field they lerp, which is the correctness premise ApplyVolumeStack relies on. ColorGradingVolumeComponent::Interpolate was tightened with a local SPARK_VOLUME_BLEND macro to keep the 13-field blend readable. VolumeSystem.h also now explicitly includes <memory> for std::unique_ptr / std::make_unique instead of relying on transitive inclusion from consumers. Adds 22 new tests: 11 VolumeManager CPU oracle tests and 11 PhaseK integration tests against the real pipeline. SparkTests: 4418 passed, 0 failed, 119075 assertions on linux-gcc-release. https://claude.ai/code/session_01QoefgpfnywL2WtN99Eivrm
…hans L1 wires MeshOptimizer::ComputeACMR into LODManager::GenerateLODChain (Windows-only) so every generated LOD chain logs the source mesh's Average Cache Miss Ratio at INFO level. Adds 15 portable tests against Spark::Graphics::MeshOptimizer directly — the previous coverage via TestGraphicsIntegration.cpp was a local reimplementation that never touched the real class, leaving ComputeACMR/OptimizeVertexCache/ OptimizeVertexFetch/GenerateMeshlets effectively untested. L2 wires BVHAccelerator::Build + FrustumQuery into SceneRenderer::CullAndSort as a first-level frustum pre-cull. Before L2 both GraphicsEngine::m_bvhAccelerator and SceneRenderer::m_bvh were declared members with zero callers — pure scaffolding. Now every frame builds a BVH from the live draw commands (synthetic unit AABBs around each world position), runs FrustumQuery against the extracted camera frustum, and short-circuits the existing per-command point-in-clip loop on the result. Survivors still run the precise point test, so no mesh visible to the existing logic disappears. Adds 10 Windows-gated tests covering Build, FrustumQuery culling, rebuild semantics, and a 50-primitive smoke test. SparkTests: 4433 passed, 0 failed, 119228 assertions on linux-gcc-release (+15 from Phase K baseline; the 10 BVH tests only register on Windows). https://claude.ai/code/session_01QoefgpfnywL2WtN99Eivrm
…wAtlas in LightingSystem Wire two Tier 2 graphics orphans as lifecycle-owned members of LightingSystem. ReflectionProbeCache schedules static probes to render once + cache, dynamic probes on a per-frame budget; CachedShadowAtlas maintains separate dynamic + cached sub-atlases and uses a FNV-1a state hash to decide whether a static light's shadow map can be reused. Both orphans are pure CPU so they live outside the SPARK_PLATFORM_WINDOWS guard. Initialize / Shutdown / BeginFrame / EndFrame hooks land on both the Windows real impl and the Linux stub path in LightingSystem.cpp so headless and GPU builds produce consistent state. Update() extracts the camera world position from the inverse view matrix and feeds the probe cache; the shadow atlas frame boundary brackets the existing light culling + shadow map update work. Adds 29 portable tests: 16 TestReflectionProbeCache covers register/ unregister, budget scheduling, static vs dynamic cache, LRU eviction, invalidation, and console status; 13 TestCachedShadowAtlas covers request routing, cache-hit detection via state hash, forceUpdate bypass, invalidation, and dynamic/cached atlas routing. SparkTests: 4461 passed, 0 failed, 1 warned (pre-existing known-flaky), 119289 assertions on linux-gcc-release (+29 tests, +61 assertions from Phase L baseline). https://claude.ai/code/session_01QoefgpfnywL2WtN99Eivrm
…g in PostProcessingPipeline N1 wires RTHandleSystem as a portable per-pipeline member. Unlike most "Graphics utility" orphans this one has no D3D11 dependency — it's pure CPU allocation metadata (reference size, per-handle scale factors, max- history bookkeeping) with format as a raw uint32_t. That lets the member live outside the SPARK_PLATFORM_WINDOWS guard and makes the allocate / resize / dynamic-scale paths fully testable on every CI platform. Initialize follows the pipeline's reference viewport, Resize() forwards the new size via SetReferenceSize so every allocated handle updates its current dimensions, Shutdown clears all handles. N2 wires ConstantBufferRing as a Windows-only per-pipeline member. Initialize allocates a 2 MB dynamic ID3D11Buffer when a device is attached; BeginFrame brackets Process() alongside GPUTimestampQuery and opens a WRITE_DISCARD map; EndFrame unmaps. Per-pass sub-allocation is deferred — the existing m_constantBuffer per-pass Map/Unmap path still runs. Lifecycle-only activation gives future sub-allocation callers a zero-churn landing zone. Exposes portable GetConstantBufferRingCapacity / GetConstantBufferRingPeakUsage accessors that return 0 on non-Windows so UI panels don't special-case the platform. Adds 29 new tests: 14 TestRTHandleSystem + 2 BufferedRTHandle portable CPU tests, 9 Windows-gated TestConstantBufferRing tests covering the uninitialised-state contract and input rejection paths, and 6 portable TestPostProcessingPipelinePhaseN integration tests. SparkTests: 4483 passed, 0 failed, 1 warned (pre-existing known-flaky), 119366 assertions on linux-gcc-release (+77 from Phase M baseline). https://claude.ai/code/session_01QoefgpfnywL2WtN99Eivrm
Wire Spark::Graphics::ShaderVariantSystem as a per-instance member of the Shader class with Initialize/Shutdown hooks on both the Windows real impl and the Linux stub path. Every Shader now carries its own keyword registry, variant key state, and preprocessor-define resolution. New public GetVariantSystem() accessor pair exposes the system to the material / hot-reload / disk-cache layers that already reference it in @see comments. The Shader class lives in the global Spark namespace (not Spark::Graphics), so member declarations + accessor signatures use explicit Spark::Graphics:: qualification — same pattern as Phase M's LightingSystem wiring. Adds 21 portable tests against the real class covering lifecycle, keyword registration (including the kMaxKeywords=64 ceiling), keyword groups with default indices, keyword state management, variant key bit operations, global+material variant resolution, define list generation, ShaderFeature stripping, ShouldCompileVariant feature gating, pre-warming, and console status formatting. Previous coverage in TestGraphicsIntegration.cpp was 3 thin smoke tests against bit math only — the real class lifecycle + registration + resolution paths had zero coverage before Phase O. SparkTests: 4505 passed, 0 failed, 0 warned, 119495 assertions on linux-gcc-release (+21 tests, +129 assertions over Phase N baseline). Even the normally-flaky LoadTest_Severe_EntityFlood passed cleanly. https://claude.ai/code/session_01QoefgpfnywL2WtN99Eivrm
…erialSystem Wire Spark::Graphics::PersistentMaterialCBManager as a per-instance member of Spark::MaterialSystem with Initialize (4096 materials × 256-byte slots = 1 MB shadow buffer) / Shutdown / BeginFrame hooks. Unlike LightingSystem and Shader, MaterialSystem's Initialize and Shutdown bodies are shared across Windows and Linux, so a single hook site at each lifecycle point wires both platforms. Every MaterialSystem now owns a live persistent CB manager with FNV-1a hash-based dirty tracking — materials only land on the GetDirtySlots list when their uploaded data actually changes. BeginFrame() advances the internal frame counter so MaterialCBSlot::lastUpdatedFrame stamps reflect the real render frame index. A future GPU-side companion can read the shadow via GetMaterialData(id) and upload only dirty sub-ranges. New public GetPersistentMaterialCB() accessor pair exposes the manager to downstream material bind / shader compilation / draw command layers. MaterialSystem lives in the global Spark namespace so the member declaration and accessor signatures use explicit Spark::Graphics:: qualification — same pattern as Phases M (LightingSystem) and O (Shader). Adds 19 portable tests covering lifecycle (including 16-byte CB-size alignment), registration (contiguous slot allocation, idempotent duplicates, overflow rejection), update + hash detection (no-op on identical data, dirty on divergence, safe smaller-than-slot writes), dirty list drain, frame-counter stamping, and console status. Full suite: 4523 passed, 0 failed, 1 warned (pre-existing known-flaky LoadTest), 119543 assertions on linux-gcc-release (+19 tests, +48 assertions over Phase O baseline). https://claude.ai/code/session_01QoefgpfnywL2WtN99Eivrm
…er in GraphicsEngine Wire the bundled IDenoiser / SoftwareDenoiser plugin pair as a std::unique_ptr<IDenoiser> member of Spark::Graphics::GraphicsEngine. The SoftwareDenoiser is a joint bilateral filter with optional albedo + normal guide weighting — pure CPU, no external SDK, works on every platform. A future OIDN / OptiX / neural implementation can swap the pointee inside GraphicsEngine::Initialize without any header reshuffling because the member type is the abstract interface. Lifecycle hooks on both the Windows real impl and the Linux stub path (GraphicsEngine.cpp has two full Initialize/Shutdown branches; Phase Q mirrors the denoiser allocation + teardown on both so headless builds see consistent state). Initial settings default to enabled=false so no CPU work runs until a future RT pass toggles it on — the activation is lifecycle-only, matching Phase N's ConstantBufferRing pattern. New public GetDenoiser() / GetDenoiserBackend() accessors expose the denoiser to downstream callers. GetDenoiserBackend() needs a matching body in both Windows and Linux branches of the .cpp file (documented in the Phase Q knowledge entry as a repeatable gotcha). Adds 14 portable tests covering settings defaults, lifecycle, constant-field passthrough, noisy-input variance reduction (using a hash-based pseudo-random pattern instead of a checkerboard, which defeats joint bilateral filtering), optional albedo + normal guides, polymorphic dispatch via the IDenoiser base class, quality presets, temporal settings, and deterministic repeated Execute() calls. One test-framework gotcha fixed: EXPECT_EQ streams operands to std::ostream which doesn't compile for enum class values. Four call sites cast to int to work around it; documented in the knowledge entry for future enum-class tests. SparkTests: 4537 passed, 0 failed, 1 warned (pre-existing known- flaky LoadTest), 120154 assertions on linux-gcc-release (+14 tests, +611 assertions over Phase P baseline — the assertion bump reflects per-pixel checks in the denoise tests). https://claude.ai/code/session_01QoefgpfnywL2WtN99Eivrm
…delete resolved) Audit decision: WIRE, not delete. The April 10 audit flagged UICompositor as "may be superseded by Engine/UI" but a grep across Engine/UI/ found zero references to UICompositor, CompositeOp, BeginComposite, or any related symbols. The two systems are complementary — UISystem manages the widget hierarchy, layout, and input; UICompositor manages the composition stack for nested widget effects (blur, mask, opacity) and pools the intermediate render target metadata those effects need. Activation gives Spark::UI::UISystem a per-instance Spark::Graphics::UICompositor member with lifecycle hooks on Initialize / OnResize / Render. Initialize clamps non-positive screen dimensions to 1 so the compositor has a valid reference size even when the caller passes garbage. OnResize calls Initialize again (it doubles as reset) so the pool drops any in-flight composite levels and re-aligns metadata with the new screen size. Render calls BeginFrame *before* the visibility check so the 10-frame idle reclaim path drains even when the UI is hidden — otherwise SetVisible(false) would leak pool memory indefinitely. Also fixes a latent hidden-include bug: UICompositor.h uses std::remove_if without including <algorithm>. The header had zero direct consumers before Phase R so transitive includes silently saved it; adding UISystem.h as the first real consumer would have failed the build without the fix. One-line include add. Adds 20 portable tests: 13 TestUICompositor covering lifecycle, push/pop, max-stack-depth (8) and max-pool-size (16) limits, same- size pool reuse, different-size pool growth, frame reclaim (15 idle frames drain the pool), mid-frame stack clear, and console status. 7 TestUISystemPhaseR integration tests cover the UISystem lifecycle hooks — Initialize follows, non-positive dimensions clamp, push/pop via the accessor, Render reclaims stale entries across 15 frames, Render clears leftover stacks, OnResize re-initialises, and OnResize handles zero dimensions. SparkTests: 4557 passed, 0 failed, 1 warned (pre-existing known- flaky LoadTest), 120236 assertions on linux-gcc-release (+20 tests, +82 assertions over Phase Q baseline). https://claude.ai/code/session_01QoefgpfnywL2WtN99Eivrm
…icsEngine Wire Spark::Graphics::NoiseGraph as a std::unique_ptr member of GraphicsEngine, default-initialised with a SimplexNode as the output. The 749-line header ships Simplex, Perlin, Cellular, FBM, DomainWarp, and Combiner nodes with runtime SIMD detection (Scalar/SSE2/AVX2, compile-time preprocessor-based so effectively zero-cost), a batch evaluation path that processes 4 samples per iteration, and the NoiseGraph owning container. Lifecycle hooks on both the Windows real impl and the Linux stub path — GraphicsEngine.cpp has two full Initialize/Shutdown branches and Phase S mirrors the allocation + teardown on both so headless builds see the same default Simplex output. The NoiseGraph is owned via unique_ptr because its internal m_outputNode raw pointer aliases into the owned std::vector<std::unique_ptr<NoiseNode>>; heap ownership keeps that alias stable across any future GraphicsEngine copy or std::vector<GraphicsEngine> resize. New public accessors: GetProceduralNoise() returns the live NoiseGraph pointer (terrain / foliage scatter / decoration systems can Evaluate or AddNode through it), and GetProceduralNoiseSIMDLevel() returns the runtime-detected kernel level. Both have matching Windows + Linux bodies (same duplication pattern as Phase Q's GetDenoiserBackend). Adds 28 portable tests covering SIMD detection (level range, name strings, graph-level accessor consistency), Simplex / Perlin / Cellular node determinism and type reporting, FBM octave / lacunarity / gain accessors + determinism, DomainWarp amplitude round-trip + null-child safety, Combiner Add/Multiply/Min/Max ops + SetOp round-trip + null-child safety, batch evaluation consistency (non-multiple-of-4 counts exercise both the unrolled loop and the scalar remainder), FBM batch vs single matching, NoiseGraph empty-graph evaluation, Add/SetOutputNode routing, batch path, and multi-node ownership. SparkTests: 4587 passed, 0 failed, 0 warned (even the normally- flaky LoadTest_Severe_EntityFlood passed cleanly), 120370 assertions on linux-gcc-release (+28 tests, +134 assertions over Phase R baseline). https://claude.ai/code/session_01QoefgpfnywL2WtN99Eivrm
…icsEngine
Wire Spark::Graphics::VCTSystem as a std::unique_ptr member of
GraphicsEngine, initialised with a small 32³ voxel grid (~130 KB)
and enabled=false by default. Closes the last major Tier 2 graphics
orphan catalogued in the April 10 audit — Phases I→T now cover 18
orphans activated on this branch.
Scope surprise: Phase S's knowledge entry parked VCT as "multi-
session scope" assuming it needed a real GI render pipeline. Reading
the source showed VoxelConeTracing.h is fully portable CPU code —
VoxelGrid uses std::vector<uint8_t> for its data + mip chain, and
VCTSystem's four-stage pipeline (BeginVoxelization → Inject →
EndVoxelization → TraceDiffuse/Specular) runs entirely on CPU with
no D3D11, no compute shaders, no external SDK. That made Phase T a
regular-sized activation, not a parked multi-session block.
Lifecycle hooks on both the Windows real impl and the Linux stub
path (GraphicsEngine.cpp has two full Initialize/Shutdown branches).
Default settings set enabled=false so the VCT pipeline does zero
per-frame work until a future GI pass opts in — same "alive but
idle" posture Phase Q's denoiser activation uses. Callers who want
full 128³ resolution (~9 MB) can re-initialise via
GetVCTSystem()->Initialize({.voxelResolution = 128, ...}).
Adds 24 portable tests covering VCTSettings defaults, VoxelGrid
lifecycle + mip count + console status, light injection in-bounds
/ out-of-bounds / clear, cone tracing (empty grid, light
accumulation, wall occlusion, step multiplier), mip chain
idempotency, VCTSystem init/shutdown + default resolution +
settings round-trip, voxelization pipeline (Begin/Inject/End +
intensity scaling), diffuse/specular cone tracing (cone count,
roughness widening, empty grid), console status formatting (ON/OFF),
and reinitialise-growth cycle.
SparkTests: 4611 passed, 0 failed, 0 warned (even the normally-
flaky LoadTest_Severe_EntityFlood passed cleanly), 120441
assertions on linux-gcc-release (+24 tests, +71 assertions over
Phase S baseline).
Tier 2 graphics orphan pool is now exhausted. The Phase T knowledge
entry sketches three roadmap directions for Phase U+:
1. Tier 3 / Tier 4 orphans from the non-Graphics audit entries
2. Quality improvements on I→T "lifecycle only" activations
3. Start a new theme (shader hot-reload, RHI backend parity, or
editor panel activation)
https://claude.ai/code/session_01QoefgpfnywL2WtN99Eivrm
…session Phase T exhausted the Tier 2 graphics orphan pool (18 orphans activated across Phases I→T). This commit documents the Phase U+ plan for direction 3 from the Phase T roadmap: start a new theme now that the Tier 2 pool is closed. The plan sketches three candidate themes in priority order: 1. Theme 3A — Shader hot-reload surface (ShaderHotReload, ShaderDiskCache, ShaderCrossCompiler). Three orphans on one subsystem, all portable, clear activation pattern matching Phase O. Tightest fit — recommended as Phase U / V / W. 2. Theme 3B — RHI backend parity (RHIHandlePool, TransientBufferAllocator activated in Vulkan / D3D12 / OpenGL / NullRHIDevice). Larger scope — 4 phases, many Windows-only tests. Requires per-backend survey before phase planning. 3. Theme 3C — Editor panel activation. Revisits Tier 4 audit entries that Phases A–G didn't touch. Scope depends on a SparkEditor/Source/Panels/ survey against EditorPanelFactory. Plan includes a full "Playbook" section capturing the reusable I→T rhythm: source-read before scoping, portability check, namespace qualification for global-Spark classes, lifecycle hooks on both Windows + Linux branches, default-to-idle for opt-in features, test-against-real-class, EXPECT_EQ enum-class cast workaround, build/test/format cycle, one-phase-per-commit. No code changes in this commit — the plan is deferred to a future session. When resumed, that session should read the Phase T knowledge entry for the current state, then this plan, then pick Phase U (ShaderHotReload) and follow the playbook. https://claude.ai/code/session_01QoefgpfnywL2WtN99Eivrm
Contributor
✅ CI Errors ResolvedAll previously reported errors have been fixed. All builds passing. Last checked: 2026-04-11T10:07:51Z |
Three Phase L tests were failing on the windows-vs2022 (Release) CI job: - BVH_FrustumQuery_CullsBehindCamera - BVH_FrustumQuery_CullsBeyondFarPlane - BVH_Build_HandlesFiftyPrimitives Two root causes, both my fault when authoring the tests: 1. The cull tests assumed BVHAccelerator::FrustumQuery does per-PRIMITIVE culling, but it only does per-LEAF culling. When two primitives share a leaf (default maxLeafSize=4), the leaf's bounding box covers both and the frustum test sees Intersecting → ALL primitives in the leaf are returned. My tests put an in-front primitive AND a behind-camera / past-far-plane primitive in the same BVH, expecting only the in-front one to come back. On Windows (and conceptually on any platform doing the math correctly) both come back because they share a single leaf. The Linux DirectXMath stub apparently produces a slightly different frustum that happened to mask this on the linux-gcc-release build. Fix: split each cull test into a SINGLE-primitive case so the BVH builds a leaf with bounds entirely outside the frustum. The new BVH_FrustumQuery_KeepsInFrontPrimitive test covers the positive side using the same single-primitive pattern. 2. BVH_Build_HandlesFiftyPrimitives put 50 primitives in a 10x5 grid with x ∈ [-4..5]. The right-edge primitive at x=5 has bounds [4.5, 5.5] in x. The frustum right plane sits at ≈ 4.143 (camera at z=-10, 45° vertical FOV, distance 10 from origin). On Windows MSVC Release the math correctly culls the rightmost primitives. The Linux stub apparently produces a slightly more permissive frustum and lets them through. Fix: rescale the grid so all primitives sit comfortably inside the ±4.143 frustum extent. New layout: 10x5 grid centred on the origin with 0.5-step spacing and 0.2 half-extent — max primitive bounds at ±2.45 in x and ±1.2 in y, well inside on every platform. Adds a new comment block at the top of the cull-tests section explaining the per-leaf vs per-primitive distinction so future test authors don't make the same mistake. Linux suite still 4611 / 0 / 0 (BVH tests are #ifdef SPARK_PLATFORM_WINDOWS gated, so this commit doesn't change the Linux test count). Windows CI should now pick up 11 BVH tests (one more than before because the cull test was split into Cull + Keep) and pass cleanly. https://claude.ai/code/session_01QoefgpfnywL2WtN99Eivrm
Coverage is failing with "geninfo: ERROR: no .gcda files found in
build" on commits Phase K+ (I→T orphan activation branch). Earlier
runs on Phase I / J succeeded. The failure mode is deterministic (same
error on 08:36 and 09:38 runs, same 16-minute duration) which rules
out a stale cache as the root cause.
Without direct job log access, I need more visibility into what
SparkTests actually does in the coverage build. This commit adds four
diagnostics to the Test step, right after the SparkTests invocation:
1. The PIPESTATUS[0] exit code (captures the actual SparkTests
return code, not the tee return code after the || true).
2. The last 80 lines of test-output.log (vs the current 5).
3. .gcno file count (shows whether instrumentation was applied).
4. .gcda file count + first 5 paths (shows whether gcov actually
wrote counter data).
These four data points will tell me whether:
- SparkTests crashed at startup (no .gcda, small .gcno count may
or may not exist).
- SparkTests ran but gcov failed to flush (no .gcda, normal .gcno).
- The build cache is corrupted (0 .gcno files despite --coverage
flags).
- A test is crashing mid-run and InstallCrashHandlers's _exit(128
+ sig) is bypassing gcov's atexit flush.
The diagnostics are purely informational — they don't change the
pass/fail behaviour of the coverage step. If the next CI run passes,
the output tells me the root cause; if it fails again, same info +
the existing lcov error.
https://claude.ai/code/session_01QoefgpfnywL2WtN99Eivrm
Phase L's MeshOptimizer_OptimizeVertexFetch_PreservesTriangleMeaning test had an unused, out-of-bounds std::vector lookup that silently read past the end of the `original` vector on Release builds but aborted with SIGABRT on Debug builds with _GLIBCXX_ASSERTIONS (the coverage job's default). The abort happened after the crash handler installed itself, so InstallCrashHandlers's _exit(128 + sig) bypassed gcov's atexit handler and no .gcda files were flushed — which is why the coverage step failed with "geninfo: ERROR: no .gcda files found in build" on every run since Phase L landed. The buggy line was: const Vertex& origV = original[1 + ((i + 2) % 3) - (i == 0 ? 0 : 0)]; For i=0 the index is 1 + 2 - 0 = 3, but `original` only has 3 elements (indices 0..2). The value was never used (the line ended with `(void)origV;` to suppress the unused-variable warning), so the UB was pure dead code that happened to trigger libstdc++'s hardened operator[] bounds check and abort the entire test binary. Fix: delete the dead origV lookup. The remaining assertion in the loop is `EXPECT_TRUE(optV.x >= 0.0f || optV.y >= 0.0f || optV.z >= 0.0f)` which is the actual check the test was written to make. Also reverts the diagnostic workflow patch from 11c51cb — the four diagnostic prints (SparkTests exit status, test-output.log tail, .gcno count, .gcda count, first .gcda paths) did their job and identified the crashing test via the `[ CRASH ]` stack trace, so they can go back to the normal tail -5 output. Full Linux suite remains clean: 4609 passed, 0 failed, 2 warned (pre-existing known-flaky), 120439 assertions on linux-gcc-release. The fixed test also builds cleanly under -D_GLIBCXX_ASSERTIONS (standalone smoke compile). https://claude.ai/code/session_01QoefgpfnywL2WtN99Eivrm
Contributor
Code Coverage (GCC + lcov)Per-Subsystem Coverage
Total: 53% (21716/40951 lines) |
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.
No description provided.