fix(shader): recover gracefully from GLSL compile/link failures instead of crashing

[drsnuggles8]

Summary

• A user-authored GLSL compile/link failure was treated as an engineering invariant violation: OpenGLShader logged the error via OLO_CORE_CRITICAL and then called OLO_CORE_VERIFY(false, ...), which OLO_DEBUGBREAK()s in Debug builds — crashing the editor whenever a developer edited a shader into a broken state. Replaced all 5 crash sites (SPIR-V compile, SPIR-V cross-compile, sync GL link, AMD link, AMD per-stage compile) with the existing critical log plus a clean transition to ShaderCompilationStatus::Failed, propagated through both OpenGLShader constructors and Reload() so no further compile/link stage runs on stale data.
• Fixes a second, previously-latent crash this change exposed: the parallel-compile lambdas in CompileOrGetVulkanBinaries/CompileOrGetOpenGLBinaries assigned their result's Stage field after the hasError early-exit check, so a task that never ran left its result default-constructed (Stage=0). Once the fail-fast VERIFY no longer aborted on the first failure, the sequential collect loop reached that never-run result and called GLShaderStageToString(0), hitting its own switch-default assert. Fixed by moving the Stage assignment before the early-exit check in both lambdas.
• Adds ShaderCompileFailureRecoveryTest.cpp, driving real broken GLSL through the compile path and asserting no crash + Failed status + rendererID == 0.
• Documents the ParallelFor result-struct gotcha in docs/agent-rules/cpp-coding-quality.md for future parallel-aggregation refactors.

Closes #568

Test plan

• New ShaderCompileFailureRecoveryTest (3 tests) passes
• Full shader-related test sweep (196 tests across *Shader*, *McpShaderReload*, AssetContentValidity.*) passes
• Manually reproduced the original crash in the editor: broke Renderer2D_Line.glsl with a real syntax error, confirmed the editor now boots through it (logs the error, reaches Failed status, keeps rendering) instead of crashing; restored the shader and confirmed a clean recompile

Summary by CodeRabbit

• Bug Fixes

  • Improved shader compilation and hot-reload reliability, with failed shader stages now correctly stopping further processing and marking the shader as failed.
  • Fixed shader program creation paths to clean up safely after compilation failures, including AMD-specific handling.
  • Added regression coverage for broken shader sources and successful recovery behavior.

• Documentation

  • Expanded C++ guidance for safe parallel result handling and failure reporting.

[coderabbitai]

Warning

Review limit reached

@drsnuggles8, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 43 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 585efd5b-3e45-450d-af6e-800746100be6

📥 Commits

Reviewing files that changed from the base of the PR and between 8e2b458 and 2ed360c.

📒 Files selected for processing (1)

• OloEngine/src/Platform/OpenGL/OpenGLShader.cpp

📝 Walkthrough

Walkthrough

OpenGLShader's Vulkan and OpenGL cross-compilation helpers now return bool instead of void, propagating per-stage compile failures to callers. Constructors, AMD program creation, and Reload() check these results and set m_CompilationStatus to Failed instead of continuing unconditionally. Hard verify/assert calls on link/compile failure are removed, AMD failure paths now clean up partially-created shader objects, and a regression test suite plus a supporting coding-quality doc rule are added.

Changes

Shader compile failure recovery

Layer / File(s)	Summary
Compilation helper contracts OloEngine/src/Platform/OpenGL/OpenGLShader.h	CompileOrGetVulkanBinaries and CompileOrGetOpenGLBinaries are marked [[nodiscard]] with updated documentation on failure/cleanup semantics.
Vulkan binaries compilation returns bool OloEngine/src/Platform/OpenGL/OpenGLShader.cpp	CompileOrGetVulkanBinaries changes from void to bool; stage is recorded before the early-out check; anyStageFailed is tracked during result collection and gates whether reflection runs and true/false is returned.
OpenGL cross-compilation returns bool OloEngine/src/Platform/OpenGL/OpenGLShader.cpp	Mirrors the Vulkan path: stage recorded before hasError early-out, anyStageFailed tracked without a hard verify, and the function returns !anyStageFailed.
Constructors gate program creation on compile success OloEngine/src/Platform/OpenGL/OpenGLShader.cpp	File-based and runtime source-string constructors, plus driver-version conditional logic, check compile results and set m_CompilationStatus = Failed instead of unconditionally calling CreateProgram().
AMD program creation failure cleanup OloEngine/src/Platform/OpenGL/OpenGLShader.cpp	CompileOpenGLBinariesForAmd returns bool, cleans up compiled/attached shaders and clears IDs on failure; CreateProgramForAmd() deletes the program and sets Failed status on that failure.
Hot-reload failure handling OloEngine/src/Platform/OpenGL/OpenGLShader.cpp	Reload() checks the boolean result of CompileOrGetVulkanBinaries and sets Failed status while preserving restore-old-program logic.
Regression tests for compile failure recovery OloEngine/tests/Rendering/PropertyTests/ShaderCompileFailureRecoveryTest.cpp, OloEngine/tests/CMakeLists.txt	New test file validates Failed status, non-ready state, and zero renderer ID for broken vertex/fragment sources, plus a happy-path valid-source test; wired into the test executable.
ParallelFor aggregation coding-quality rule docs/agent-rules/cpp-coding-quality.md	Adds Rule 12 documenting that result-identifying fields must be set before early-exit checks in ParallelFor aggregation lambdas.

Possibly related issues

• Shader compile/link failure crashes the debug editor (OLO_CORE_VERIFY → DEBUGBREAK) instead of degrading gracefully #568: Targets the same OpenGLShader.cpp compile/link failure path, replacing crash-on-failure VERIFY behavior with graceful Failed handling, matching this PR's changes.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: shader compile/link failures now recover gracefully instead of crashing.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[sonarqubecloud]

Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
5.3% Duplication on New Code

See analysis details on SonarQube Cloud