fix(graphics): enable OpenGL rendering on Linux with backend fallback#462
Merged
Conversation
Three issues prevented the OpenGL backend from rendering on Linux: 1. RHI backend selection had no fallback — if Vulkan (preferred on Linux) failed to initialize, the engine went straight to headless NullRHI. Now iterates available backends until one succeeds. 2. GLSwapChain treated all Linux paths as headless — always created an FBO and Present() only called glFlush(). Now detects windowed mode via windowHandle, uses the default framebuffer (FBO 0), and calls SDL_GL_SwapWindow for actual screen presentation. 3. SetRenderTargets called glDrawBuffers(GL_COLOR_ATTACHMENT0) on the default framebuffer, which is a GL error. Now uses GL_BACK for FBO 0. Also fixes GLSwapChain::Resize to properly handle both windowed (no-op, default framebuffer resizes with the window) and headless (recreate FBO) modes. 5305 tests pass, 0 regressions. https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
![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: cfadfd5490
ℹ️ 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
+314
to
+319
| if (desc.windowHandle != nullptr) | ||
| { | ||
| // Windowed mode: SDL2 created the GL context and owns the window. | ||
| // Render to the default framebuffer (FBO 0) and use SDL_GL_SwapWindow. | ||
| m_windowed = true; | ||
| m_sdlWindow = desc.windowHandle; |
There was a problem hiding this comment.
This marks any non-null windowHandle as windowed and later calls SDL_GL_SwapWindow, but GLDevice::Initialize still uses the SPARK_EGL_SUPPORT path to create and keep a headless EGL pbuffer context (not the SDL window context). In Linux builds where EGL is enabled and SDL is used (a common default), rendering goes to the pbuffer while Present() swaps the SDL window, so frames are not actually presented (and swap-interval/swap calls can fail each frame). The windowed path should only be enabled when the current GL context is actually the SDL window context, otherwise remain in headless-FBO mode or rebind the correct context.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Fixed in commit cb5b210. The EGL bootstrap now mirrors the GLX branch's detect-and-reuse pattern: if eglGetCurrentContext() != EGL_NO_CONTEXT, we reuse the host's (SDL2's) existing EGL display/surface/context, set m_ownsEglContext = false, and skip the pbuffer creation. Shutdown() now only tears down the EGL resources when we own them, matching the existing m_ownsGLXContext logic. The windowed path + SDL_GL_SwapWindow will now operate on the correct GL context.
Generated by Claude Code
…anager Adds critical-path integration tests for three systems that previously had zero orchestration coverage: - TestRHIBridgeIntegration.cpp (18 tests): lifecycle, backend fallback, headless frame management, resource creation, shader cache, capabilities - TestNetworkManagerIntegration.cpp (14 tests): init/shutdown lifecycle, state queries, update ticks, server start/stop, console integration - TestSystemManagerIntegration.cpp (11 tests): execution order, enable/ disable, system lookup, world interaction with entity creation All tests run headless (NullRHIDevice, loopback sockets). 5348 tests pass. https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
…ngine lifecycle) Continues critical-path integration test coverage for systems that previously had zero orchestration tests: - TestAssetPipelineIntegration.cpp (16 tests): lifecycle on Linux (accepts nullptr device), asset type detection for all formats, AssetCache construction/queries/LRU, hot-reload toggle - TestMaterialSystemIntegration.cpp (14 tests): Material object creation, PBR property get/set/validation edge cases, render state management, PersistentMaterialCBManager register/update/dirty tracking - TestEngineLifecycle.cpp (11 tests): full GraphicsEngine init→tick→ shutdown via NullRHI, subsystem availability, RHI device access, RHIBridge standalone resource lifecycle 5389 tests pass, 0 failures, 0 regressions. https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
Validates the FPS module's core gameplay orchestration: GameMode lifecycle (initialize with Deathmatch/Survival/TeamDM rules), player management (add/remove/team assignment), scoring (kill recording, multi-kill tracking), and spawn point management. Also adds a knowledge entry documenting the full session: OpenGL rendering fix + 94 integration tests across 7 critical systems (RHIBridge, NetworkManager, SystemManager, AssetPipeline, MaterialSystem, GraphicsEngine, GameMode). Test count: 5305 → 5399. https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
Contributor
✅ CI Errors ResolvedAll previously reported errors have been fixed. All builds passing. Last checked: 2026-04-13T17:02:04Z |
…ine tests Two changes that advance the OpenGL rendering pipeline: 1. Shader class now stores compiled GLSL source text after RHI compilation on Linux (m_compiledVertexSource / m_compiledPixelSource). Previously, the compiled bytecode was discarded — shaders compiled to GLSL but the result was never available for pipeline state creation. Callers can now access the source via GetCompiledVertexSource/PixelSource(). 2. TestGLSLPipelineIntegration.cpp (12 tests) validates the full GLSL pipeline through the RHI: shader compilation, passthrough verification, device shader creation, pipeline state linking, full draw pipeline (vertex buffer + index buffer + PSO + DrawIndexed), cross-compilation HLSL→GLSL, and shader factory utilities. 5410 tests pass, 0 failures. https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
Completes the GLSL rendering pipeline by connecting the Shader class to the RHI device on Linux. Previously, shaders compiled to GLSL but the result was discarded — SetShaders() was a no-op, and constant buffer updates were silently dropped. Changes: - Shader::Initialize() acquires the RHI device from the bridge - CreateRHIPipelineIfReady() lazily builds IRHIShader + IRHIPipelineState from the stored compiled GLSL source when both VS and PS are available - SetShaders() now binds the RHI pipeline state and constant buffers to the command list - UpdatePerFrameConstants/UpdatePerObjectConstants() now write data to RHI constant buffers via device->UpdateBuffer() - CreateConstantBuffers() creates RHI dynamic constant buffers for per-frame (binding 0) and per-object (binding 1) data - Shutdown() releases all RHI resources This means the full path is now connected: LoadVertexShader → compile GLSL → store source → CreateRHIShader → CreatePipelineState → SetShaders binds PSO → UpdateConstants writes UBOs → DrawIndexed renders geometry 5409 tests pass, 0 failures. https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
…er tests Fixed a critical bug where Shader::LoadVertexShader/LoadPixelShader always failed on GLSL files: the RHI compile options used Auto target backend which resolved to HLSL, producing a GLSL→HLSL cross-compilation path that is unsupported. The fix detects .glsl/.vert/.frag file extensions and explicitly sets sourceLanguage=GLSL, targetLanguage=GLSL, targetBackend=OpenGL. Also discovered that SUCCEEDED() macro is broken on 64-bit Linux: HRESULT is 'long' (8 bytes), so E_FAIL (0x80004005) is positive — SUCCEEDED() returns true for failure codes. Tests now use EXPECT_EQ(hr, S_OK). Added 7 new tests loading real production GLSL shaders (BasicVS.glsl, BasicPS.glsl, all 14 GLSL files) through both the RHI directly and the Shader class, verifying source storage and compilation. Updated knowledge entry to cover full session scope (6 engine fixes + 113 tests). 5416 tests pass, 0 failures. https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
Three platform-level fixes that were silently hiding errors across the Linux build: 1. **HRESULT was broken on 64-bit Linux**: PlatformTypes.h defined it as 'long' (8 bytes on x86_64), so error codes like E_FAIL (0x80004005) had the high bit clear and were positive — making SUCCEEDED() return true for failure codes everywhere. Every error path tested with SUCCEEDED() was silently swallowing failures. Fixed by using int32_t to match the Windows 32-bit ABI. All 5417 tests still pass, confirming no regressions. 2. **CompileShader Auto fallback defaulted to HLSL**: RHIFactory::CompileShader resolved targetLanguage=Auto by falling through to HLSL when the backend was unknown, producing GLSL→HLSL cross-compilation paths that are unsupported. Fixed by evaluating source language first and using it as the target fallback (passthrough). GLSL shaders now compile through the RHI even when callers don't set an explicit backend. 3. **Silent Initialize() failures in Linux graphics path**: Denoiser, VCTSystem, and Shader constant-buffer creation all returned bool/HRESULT without any logging on failure. Added SPARK_LOG_WARN/SPARK_LOG_ERROR at each failure site so later breakages can be diagnosed from logs. These three bugs compounded each other: the HRESULT bug made SUCCEEDED(LoadVertexShader) return true for GLSL→HLSL failures, the CompileShader default made every GLSL load fail, and the missing init logging meant the engine silently proceeded in a broken state. 5416 tests pass, 0 failures. https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
TestHResultPlatform.cpp locks in the HRESULT platform fix with regression tests for the type identity, all error code values, SUCCEEDED/FAILED macros, and mutual exclusion. If HRESULT is ever regressed back to 'long' on 64-bit Linux, these tests catch it immediately. Also added SPARK_LOG diagnostics to the silent initialization sites the audit identified: - TextureSystemLinux.cpp: 4 CreateFromData calls for default textures (white/black/normal/noise) now log on failure - LightingSystemLinux.cpp: CachedShadowAtlas::Initialize now logs on failure, matching the Windows path - SparkEngineLinux.cpp: NeuralInferenceEngine::Initialize now logs on failure instead of silently continuing with a broken instance 5440 tests pass, 0 failures. https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
Both subsystems were created in GraphicsEngineLinux::Initialize but never had Initialize() called on them — so the shadow cache, probe cache, and asset pipeline all operated with zero capacity and rejected every request. The Windows path correctly initialized them, but the Linux path only constructed them. Only LightingSystem and AssetPipeline are wired because their Linux Initialize() accepts nullptr device/context. TextureSystem and MaterialSystem still assert non-null, so they remain uninitialized in headless mode and must be wired later via SetDevice paths. Added two regression tests in TestEngineLifecycle.cpp: - LightingSystem_InitializedWithShadowCache: verifies GetCachedShadowAtlas().IsInitialized() returns true after engine init. Before this commit, it returned false. - AssetPipeline_InitializedAfterEngineInit: smoke test that Initialize ran without crashing. 5442 tests pass, 0 failures. https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
Both Linux Initialize() paths had spurious null-device asserts even though their bodies don't actually touch the device pointer — all real GPU work is behind #ifdef SPARK_PLATFORM_WINDOWS. The asserts blocked headless initialization, leaving the engine with null default textures (white/black/normal) and uninitialized material state. Fixes: 1. TextureSystemLinux.cpp: removed ASSERT_NOT_NULL; the code path calls CreateFromData(nullptr, 4, nullptr) — the stored device pointer is never dereferenced. 2. MaterialSystem.cpp: moved SPARK_EXPECTS inside the existing SPARK_PLATFORM_WINDOWS guard, matching the already-guarded body. 3. GraphicsEngineLinux.cpp: wired m_textureSystem->Initialize and m_materialSystem->Initialize into the init chain, bringing headless init to parity with the Windows path (now 4 subsystems: texture, material, lighting, asset pipeline — all four previously silent). Two new regression tests in TestEngineLifecycle.cpp: - TextureSystem_DefaultTexturesCreated: verifies GetWhiteTexture(), GetBlackTexture(), GetNormalTexture() return non-null after engine init. Before this commit they were all null. - MaterialSystem_InitializedAfterEngineInit: smoke test for MaterialSystem::Initialize running to completion on Linux. 5444 tests pass, 0 failures. https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
Auto-generated timestamp update from docs/update-all-docs.sh. https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
PostProcessingPipeline was the last unwired subsystem in GraphicsEngineLinux::Initialize. Its Initialize() already handled null device gracefully (the CPU-side temporal filter, volume manager, and RT handle system run on every platform; the D3D11-backed orphans are guarded by #ifdef SPARK_PLATFORM_WINDOWS + m_device checks), so wiring it up just required adding the call. Before this commit, Process() on Linux would early-out silently because m_initialized stayed false — every post-process effect (16 passes) was skipped without any log indication. Added PostProcessingPipeline::IsInitialized() accessor and a regression test that verifies m_initialized is true after engine init. Five subsystems are now fully wired in headless mode: TextureSystem, MaterialSystem, LightingSystem, AssetPipeline, PostProcessingPipeline. 5445 tests pass, 0 failures. https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
Two more subsystems wired up in GraphicsEngineLinux::Initialize: 1. LightManager — pure CPU tile binning and shadow atlas slot tracking. Initialize(width, height, 16) builds the tile grid. Without it, GetTilesX/Y returned 0 and any tile-based light culling would silently skip every light. 2. UpscalingSystem — on Linux, CreateGPUResources() is a no-op that returns true unconditionally, so Initialize(nullptr, nullptr, w, h) succeeds and the system tracks render/display resolution on the CPU side. Seven subsystems are now fully initialized in headless mode on Linux: TextureSystem, MaterialSystem, LightingSystem, AssetPipeline, PostProcessingPipeline, LightManager, UpscalingSystem. Regression test: EngineLifecycle_LightManager_InitializedWithTileGrid verifies GetTilesX() > 0 and GetTilesY() > 0 after engine init. Before this commit, both were 0. 5446 tests pass, 0 failures. https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
GraphicsEngine::Shutdown on Linux was just resetting unique_ptrs, skipping the explicit Shutdown() methods on subsystems that need them. Also fixed a duplicate m_postProcessing.reset() call. Added explicit Shutdown() calls in reverse init order for all 7 wired subsystems: UpscalingSystem, LightManager, PostProcessingPipeline, AssetPipeline, LightingSystem, MaterialSystem, TextureSystem. Matches the Windows path pattern. 5447 tests pass, 0 failures. https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
GraphicsEngine::BeginFrame and EndFrame on Linux were only calling the RHI bridge and VRAMBudgetMonitor — every other subsystem with an Update()/Process() method was silently stalled. The audit identified three critical gaps: 1. AssetPipeline::Update never called → async load callbacks queued but never processed; streaming broken. 2. LightingSystem::Update never called → shadow cache BeginFrame/ EndFrame never advanced; probe cache frame counter stalled. 3. PostProcessingPipeline::Process never called → all 16 post-process passes (Bloom, DoF, Tonemapping, ColorGrading, etc.) silently skipped. Wired BeginFrame to call AssetPipeline::Update and LightingSystem::Update with a nominal 1/60s delta (matches the frame cadence). Wired EndFrame to call PostProcessingPipeline::Process before rhi.bridge.EndFrame(). Regression test EngineLifecycle_BeginEndFrame_InvokesSubsystemUpdates runs 3 full frame cycles to exercise the new update paths without crashing. Before the fix, these paths were dead code. 5447 tests pass, 0 failures. https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
New test GLSLPipeline_ShaderClass_WithGraphicsEngine_CreatesRHIPipeline validates the full integration through the global RHI state: GraphicsEngine::Initialize → bridge + RHI device setup Shader::Initialize → acquires IRHIDevice from the singleton LoadVertexShader(BasicVS.glsl) → compiles GLSL, stores source LoadPixelShader(BasicPS.glsl) → compiles GLSL, stores source SetShaders() → creates VS + PS + PSO via device->CreateShader() GetRHIPipelineState() → returns non-null Previously, a standalone RHIBridge in the test wasn't visible to Shader::Initialize (which reads the global LinuxRHIState singleton). This test uses GraphicsEngine, which owns the global state, so the pipeline creation path runs end-to-end. 5448 tests pass, 0 failures. https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
Updated project-priorities-session-2026-04-12.md with the full scope of the session: 18 commits across 7 phases, fixing OpenGL rendering infrastructure, GLSL shader pipeline, HRESULT platform bug, silent subsystem init chain, Shutdown symmetry, and per-frame Update wiring. Full Linux headless init parity with Windows now achieved: - 7 subsystems fully initialized (TextureSystem, MaterialSystem, LightingSystem, AssetPipeline, PostProcessingPipeline, LightManager, UpscalingSystem) - 3 per-frame Update methods wired (AssetPipeline::Update, LightingSystem::Update, PostProcessingPipeline::Process) - 7 explicit Shutdown() calls in reverse-init order 5449 tests pass, 0 failures. Session test delta: 5305 → 5449 (+144). https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
AudioEngine::Update was never called from the main loop on either
platform — a pre-existing bug affecting both Windows and Linux.
UpdateSources() (advance source state machine, stop finished sources)
and Update3DAudio() (spatialization, distance attenuation) never ran.
Added SPARK_GUARDED_UPDATE("Audio", ...) calls in:
- SparkEngineLinux.cpp::TickFrame (SDL2 windowed main loop)
- SparkEngineLinux.cpp headless fallback loop
- SparkEngineWindows.cpp headless main loop
- SparkEngineWindows.cpp windowed main loop
All four main loop variants now pump g_audioEngine->Update(dt) each
frame. The wiki documentation at Troubleshooting.md:317 already
asserted that this was required ("Ensure AudioEngine::Update() is
called every frame") but no caller existed.
5448 tests pass, 0 failures.
https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
…ead) The ShaderHotReload singleton is pumped via Shader::HotReloadShaders(), but that method is only called when m_shader is non-null — and m_shader is never instantiated on either Windows or Linux. The entire hot-reload pump was dead code. File changes on disk would never be detected at runtime. Fixed by calling ShaderHotReload::GetInstance().Update(dt) directly from GraphicsEngine::BeginFrame on both platforms, bypassing the vestigial m_shader member. The singleton's internal poll-interval gating (default 0.5s) prevents this from becoming a per-frame hot path. 5448 tests pass, 0 failures. https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
TemporalEffects was created only on Windows; Linux never constructed it, leaving m_temporalEffects null. The class's CPU-side state (jitter, history, motion vectors) can run without a D3D11 device — SetDevice() is a separate opt-in for the GPU path. Changes: - GraphicsEngineLinux::Initialize creates m_temporalEffects and calls Initialize(width, height) to set up the CPU state. - BeginFrame now calls TemporalEffects::Update(dt) to advance m_totalTime. - Shutdown calls TemporalEffects::Shutdown() in reverse-init order. - Added GraphicsEngine::GetTemporalEffects() public accessor. - EngineLifecycle_TemporalEffects_InitializedAfterEngineInit regression test verifies IsInitialized() is true after engine init. Before this commit, the accessor returned nullptr. Eighth subsystem fully wired in headless mode on Linux: TextureSystem, MaterialSystem, LightingSystem, AssetPipeline, PostProcessingPipeline, LightManager, UpscalingSystem, TemporalEffects. 5449 tests pass, 0 failures. https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
…0th subsystems) Two more subsystems that were created only on Windows: 1. ShadowAtlas — pure CPU allocation tracker and tile LRU. Initialize takes (atlasSize, minTileSize), no device needed. On Linux it was left as nullptr, so any downstream shadow tile allocation would silently skip. 2. ScreenSpaceEffects — CPU-side generates SSAO kernel and noise texture on Initialize(width, height). GPU resource creation is a stub until SetDevice is called. On Linux it was nullptr, making SSAO/SSR/ contact-shadows silently unavailable. Both are now created and initialized in GraphicsEngineLinux::Initialize and properly shut down in reverse-init order. Regression tests: - ShadowAtlas_CreatedAfterEngineInit verifies GetShadowAtlas() != nullptr - ScreenSpaceEffects_CreatedAfterEngineInit verifies GetScreenSpaceEffects() != nullptr Ten subsystems now fully wired in headless mode on Linux: TextureSystem, MaterialSystem, LightingSystem, AssetPipeline, PostProcessingPipeline, LightManager, UpscalingSystem, TemporalEffects, ShadowAtlas, ScreenSpaceEffects. 5451 tests pass, 0 failures. https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
TerrainRenderer has a Linux-specific no-device Initialize() overload (the header guards #ifdef SPARK_PLATFORM_WINDOWS around the D3D11 variant). Its CPU tile LRU + heightfield sampling state runs without a GPU. On Linux it was nullptr, so any terrain streaming would silently no-op. Added GetTerrainRenderer() accessor and wired Initialize/Shutdown in GraphicsEngineLinux. Regression test verifies the pointer is non-null after engine init. Eleven subsystems now fully wired in headless mode. https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
Two fixes, one new feature: 1. RHIBridge::Resize dereferenced m_swapChain unconditionally — but on headless/NullRHI initialization, m_swapChain is null. Any Resize call in headless mode would crash or hang on the null pointer. Added an early return that tracks the new dimensions but skips GPU resource recreation when the swap chain doesn't exist. 2. GraphicsEngine::Resize on Linux only resized the RHI bridge — it didn't propagate the new dimensions to wired subsystems. Now calls Resize on PostProcessingPipeline, TemporalEffects, ScreenSpaceEffects, and LightManager so they recompute their internal state (render targets, history buffers, tile grids). 3. EngineLifecycle_Resize_PropagatesToSubsystems verifies LightManager::GetTilesX/Y returns the expected values for 1920x1080: 1920/16=120 tiles X, (1080+15)/16=68 tiles Y. Before this commit, the values stayed at 80x45 because Resize only touched the bridge. 5453 tests pass, 0 failures. https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
Two fixes flagged by CI/codex on PR #462: 1. **codex P1 (OpenGLDevice.cpp)**: EGL bootstrap always created a fresh pbuffer context, even when SDL2 had already made a window GL context current. The result: windowed mode code path calls SDL_GL_SwapWindow on the SDL window, but rendering goes to the unrelated EGL pbuffer context — frames never reach the screen. Mirror the GLX branch's detect-and-reuse pattern: if eglGetCurrentContext() != EGL_NO_CONTEXT, reuse the existing EGL display/surface/context and set m_ownsEglContext = false. Shutdown() now only tears down the context when we own it, matching the existing GLX ownership logic. 2. **build-windows-vs2022 (Debug)**: TestHResultPlatform.cpp only included Core/PlatformTypes.h for HRESULT, but that header is entirely guarded by #ifndef SPARK_PLATFORM_WINDOWS — on Windows, HRESULT comes from <windows.h>. Added a platform-conditional include so the test compiles on both Windows (via windows.h) and Linux/macOS (via PlatformTypes.h). 5453 tests pass on Linux, 0 failures. https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
When Vulkan Lavapipe is installed on the build machine, RHIBridge::Initialize
would pick Vulkan as the recommended backend, succeed in selecting the
software device ("llvmpipe (LLVM 20.1.2, 256 bits)"), then hang
indefinitely because there is no presentation surface to bind to. Tests
that called GraphicsEngine::Initialize(nullptr) hung forever.
Two-layer fix:
1. RHIBridge::Initialize now checks windowHandle == nullptr at the top
of the function. If null, the requested backend is replaced with
GraphicsBackend::None — GPU backends require a presentation surface
to function, so headless callers must stay on NullRHIDevice
regardless of what was requested.
2. GraphicsEngine::Initialize on Linux now passes
GraphicsBackend::None directly when hWnd is null, instead of asking
GetRecommendedBackend() (which would pick Vulkan when Lavapipe is
present). The bridge-level guard above is the safety net; this
second layer makes the intent explicit and avoids the recommended-
backend log line for headless callers.
Tests now run in ~3 minutes instead of hanging forever on Vulkan.
5453 pass, 0 failed.
https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
…rnings
Fixes the Windows VS2022 (Debug) + Code Coverage CI failures reported
by the CI Error Report bot:
**EditorApplicationLinux.cpp + EditorApplicationWindows.cpp**
Both files defined EditorApplication member methods (CreateMainWindow,
InitializeGraphics, InitializeImGui, Run, ProcessMessages, Render,
WindowProc, Shutdown, etc.) at file scope — but EditorApplication is
declared in `namespace SparkEditor` in EditorApplication.h. The compiler
saw these as free functions in the global namespace, producing 47 errors
of the form "EditorApplication has not been declared" / "EditorConfig
does not name a type" / "m_hwnd was not declared in this scope".
Local Linux builds hid this because Dear ImGui is not available in
this env, so SparkEditor is skipped entirely. The CI containers have
ImGui, so they actually compile these files — but build-linux-gcc uses
ccache so earlier successful builds masked the error. The coverage job
forces a rebuild (--coverage flag invalidates ccache) and exposed the
bug, same for Windows VS2022 Debug.
Fix: wrap both .cpp bodies in `namespace SparkEditor { ... }`, so all
member definitions resolve against the class declaration. Added the
missing `<chrono>` and `<iostream>` includes to the Linux file.
**CI compiler warnings cleanup**
- SparkEngineWindows.cpp:257 — dangling backslash at end of // comment
triggered a -Wcomment "multi-line comment" warning.
- TestRHIBridgeIntegration.cpp:118, TestMaterialSystemIntegration.cpp:121,
TestEntityPresetManagerPhaseEE.cpp:121 — unsigned `size() >= 0` checks
were always-true. Replaced with `(void)value; EXPECT_TRUE(true);` to
preserve intent (just verify the call doesn't crash).
- TestNetworkManagerIntegration.cpp:133 — unused `stats` local removed.
https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
Follow-up to a334c83. Wrapping the files in namespace SparkEditor exposed a separate pre-existing bug: both platform .cpp files use SPARK_GUARDED_UPDATE but neither includes "Core/FaultIsolation.h" that defines the macro. Without the include, the preprocessor leaves `SPARK_GUARDED_UPDATE("EditorConsole", "Editor", { console.Update(); });` as an undeclared identifier call — the third argument `{ console.Update(); }` is parsed as a brace-enclosed init list that breaks the enclosing function body, cascading into 20+ "X was not declared in this scope" errors for the rest of the function. Added `#include "Core/FaultIsolation.h"` to both platform files, matching what EditorApplication.cpp already does. SparkEditor's CMake target has `${CMAKE_SOURCE_DIR}/SparkEngine/Source` as an include dir, so the path resolves correctly. https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
…ws.cpp Follow-up to efffce1. The Windows platform file uses std::cout and std::cerr on 9 lines but never included <iostream>. Linux builds transitively picked it up via another header (non-portable), but MSVC enforces explicit includes — producing: EditorApplicationWindows.cpp(84,14): error C2039: 'cout': is not a member of 'std' EditorApplicationWindows.cpp(84,14): error C2065: 'cout': undeclared identifier ...and 14 more similar errors on cout/cerr usages. Added <iostream> alongside the <chrono> I already added in a334c83. https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
Last remaining pre-existing compile error from the SparkEditor namespace fix series. Both platform files reference EditorWindowManager::GetInstance().Shutdown() in their shutdown path but neither includes "EditorWindowManager.h". The earlier commits (a334c83, efffce1, e2b3735) fixed 49 of the 50 errors the CI Error Report bot flagged; this fixes the last one. https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
…public
Two more pre-existing SparkEditor bugs surfaced by the namespace
wrapping work:
1. **s_instance internal linkage**: EditorApplication.cpp declared
`static EditorApplication* s_instance = nullptr;` as a file-local
static — internal linkage, unreachable from other translation units.
EditorApplicationWindows.cpp::WindowProc references `s_instance`
but lives in a different TU, so MSVC VS2022 (Debug) reports:
error C2065: 's_instance': undeclared identifier
Previously hidden because the Windows file wasn't compiling at all
due to the namespace issue (fixed in a334c83); now that it compiles
to the WindowProc body, it hits this scope error.
Fix: promoted `s_instance` to a public static member of
EditorApplication (declared in the header, defined out-of-class in
EditorApplication.cpp). WindowProc now accesses it via
`EditorApplication::s_instance`.
2. **DrawVec3Control private**: InspectorPanel declared the static
helper in its `private:` section, but
InspectorComponentRenderers_Reflected.cpp calls it externally as
`InspectorPanel::DrawVec3Control(...)`. GCC (coverage job) reports:
error: 'static void SparkEditor::InspectorPanel::DrawVec3Control(...)'
is private within this context
Fix: moved the declaration to the `public:` section with a comment
explaining why (reflection-based external component renderers need
to reuse it).
Both are pre-existing bugs independent of my OpenGL/HRESULT work.
The namespace wrapper exposed them by letting compilation actually
reach the affected functions.
https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
Contributor
Code Coverage (GCC + lcov)Per-Subsystem Coverage
Total: 53.9% (26251/48728 lines) |
The Windows VS2022 Release ctest run crashed with:
[ CRASH ] AssetPipeline_InitializeShutdown_Linux_Succeeds (EXCEPTION)
Assert::Fail (Source/Utils/Assert.cpp:163)
AssetPipeline::Initialize (AssetPipelineWindows.cpp:45)
test_AssetPipeline_InitializeShutdown_Linux_Succeeds (TestAssetPipelineIntegration.cpp:27)
Root cause: `SPARK_REQUIRE_NOT_NULL` is always active (not debug-only
like `ASSERT`), so passing nullptr for the D3D11 device aborts the
test process on Windows regardless of build config. Three test files
exercise the Linux headless path with null device/context/window:
1. **TestAssetPipelineIntegration.cpp** — 10 `Initialize(nullptr, nullptr)`
calls. AssetPipelineWindows.cpp:45 aborts on the first one.
2. **TestEngineLifecycle.cpp** — 21 `engine.Initialize(nullptr)` calls.
GraphicsEngineWindows::Initialize returns E_INVALIDARG cleanly, but
several tests then use `EXPECT_EQ(hr, S_OK)` and subsequent calls
to subsystem accessors would fail or crash.
3. **TestGLSLPipelineIntegration.cpp** — `Shader::LoadVertexShader`/
`LoadPixelShader` calls route to ShaderCompilationWindows.cpp:92
which has `SPARK_REQUIRE_NOT_NULL(m_device)`. Since the Shader is
initialized with a null device in the tests, this aborts.
Fix: wrap the entire body of each file in `#ifndef _WIN32`. These
tests are explicitly Linux-headless smoke tests ("Linux" literally
appears in the failing test name). Windows coverage of the same
subsystems would need a different test that creates a real D3D11
device first — out of scope for this PR.
TestRHIBridgeIntegration.cpp was checked and is safe on Windows: all
its `Initialize(nullptr, ..., GraphicsBackend::None, ...)` calls
route to NullRHIDevice, which works on any platform.
TestMaterialSystemIntegration.cpp was checked too — it never calls
`MaterialSystem::Initialize` (only tests PersistentCB + Material
construction paths that don't require a device).
https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5
Contributor
Code Coverage (GCC + lcov)Per-Subsystem Coverage
Total: 53.9% (26258/48728 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.
Three issues prevented the OpenGL backend from rendering on Linux:
RHI backend selection had no fallback — if Vulkan (preferred on Linux) failed to initialize, the engine went straight to headless NullRHI. Now iterates available backends until one succeeds.
GLSwapChain treated all Linux paths as headless — always created an FBO and Present() only called glFlush(). Now detects windowed mode via windowHandle, uses the default framebuffer (FBO 0), and calls SDL_GL_SwapWindow for actual screen presentation.
SetRenderTargets called glDrawBuffers(GL_COLOR_ATTACHMENT0) on the default framebuffer, which is a GL error. Now uses GL_BACK for FBO 0.
Also fixes GLSwapChain::Resize to properly handle both windowed (no-op, default framebuffer resizes with the window) and headless (recreate FBO) modes.
5305 tests pass, 0 regressions.
https://claude.ai/code/session_01RfQPA8wL2qYsv2tbxowvF5