Feature/add depth texture#11
Conversation
WalkthroughThis PR introduces a custom ChangesCustom Exception Infrastructure and Renderer Error Handling
🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly Related PRs
Suggested Labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Infer (1.2.0)src/runtime/core/genericfilemanager.cppsrc/runtime/core/genericfilemanager.cpp:1:10: fatal error: 'core/genericfilemanager.h' file not found ... [truncated 1137 characters] ... clang/18/include" src/runtime/renderer/RHIVulkan.cppIn file included from src/runtime/renderer/RHIVulkan.cpp:1: ... [truncated 2200 characters] ... end_decl.ml" (inlined), line 54, characters 4-52 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. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/runtime/core/segfaultexception.h`:
- Line 1: Add the project-mandated MIT license header to the top of the new
header file src/runtime/core/segfaultexception.h by inserting the standard
multi-line MIT banner used across the repo before the existing `#pragma` once;
ensure the header text matches the exact format used in other .h/.cpp files
(including year and copyright holder) so the file complies with "Include MIT
license header in all source files" policy.
In `@src/runtime/renderer/RHIVulkan.cpp`:
- Around line 318-323: The Vulkan extension enumeration in
RHIImpl::checkDeviceExtensionSupport(VkPhysicalDevice) leaves extensionCount
uninitialized and ignores vkEnumerateDeviceExtensionProperties return values;
change to initialize uint32_t extensionCount = 0, call
vkEnumerateDeviceExtensionProperties and check its VkResult for success before
using extensionCount, bail out (return false or handle error) if the call fails,
then allocate/resize the std::vector<VkExtensionProperties> to extensionCount
and call vkEnumerateDeviceExtensionProperties again while checking its result;
ensure you validate extensionCount (e.g., zero means no extensions) before using
availableExtensions.
- Around line 1025-1026: Wrap calls into the RHI boundary so SegfaultException
cannot escape: in the public RHI API entry points (RHI::init() and
RHI::drawFrame()) catch segfault::core::SegfaultException and convert it to a
safe failure (return false from RHI::init() and return from RHI::drawFrame()
without throwing), logging the error; likewise add try/catch in higher-level
callers (App::drawFrame() and main) where they call
RHIImpl::drawFrame()/RHI::drawFrame() to ensure any remaining
segfault::core::SegfaultException is caught, logged, and translated to a
controlled shutdown path rather than allowing the exception to propagate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3035a2d9-27aa-4221-b5e7-8b6c84bd4f73
📒 Files selected for processing (4)
src/runtime/CMakeLists.txtsrc/runtime/core/genericfilemanager.cppsrc/runtime/core/segfaultexception.hsrc/runtime/renderer/RHIVulkan.cpp
| @@ -0,0 +1,27 @@ | |||
| #pragma once | |||
There was a problem hiding this comment.
Add the required MIT license header.
Line 1 starts a new .h file without the project-mandated MIT license banner.
As per coding guidelines, "Include MIT license header in all source files" for **/*.{h,cpp}.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/runtime/core/segfaultexception.h` at line 1, Add the project-mandated MIT
license header to the top of the new header file
src/runtime/core/segfaultexception.h by inserting the standard multi-line MIT
banner used across the repo before the existing `#pragma` once; ensure the header
text matches the exact format used in other .h/.cpp files (including year and
copyright holder) so the file complies with "Include MIT license header in all
source files" policy.
Source: Coding guidelines
| bool RHIImpl::checkDeviceExtensionSupport(VkPhysicalDevice device_) { | ||
| uint32_t extensionCount; | ||
| vkEnumerateDeviceExtensionProperties(device_, nullptr, &extensionCount, nullptr); | ||
|
|
||
| std::vector<VkExtensionProperties> availableExtensions(extensionCount); | ||
| vkEnumerateDeviceExtensionProperties(device_, nullptr, &extensionCount, availableExtensions.data()); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify all extension-enumeration call sites and whether return values are checked.
rg -nP 'vkEnumerateDeviceExtensionProperties\s*\(' src/runtime/renderer/RHIVulkan.cpp -C3Repository: kimkulling/Segfault
Length of output: 1122
Initialize and validate Vulkan extension-enumeration results in RHIImpl::checkDeviceExtensionSupport(VkPhysicalDevice)
At src/runtime/renderer/RHIVulkan.cpp lines 318-323, extensionCount is left uninitialized and both vkEnumerateDeviceExtensionProperties calls ignore the returned VkResult; if enumeration fails, this can produce an invalid extensionCount and unsafe vector sizing.
🛠️ Proposed fix
bool RHIImpl::checkDeviceExtensionSupport(VkPhysicalDevice device_) {
- uint32_t extensionCount;
- vkEnumerateDeviceExtensionProperties(device_, nullptr, &extensionCount, nullptr);
+ uint32_t extensionCount{};
+ VkResult result = vkEnumerateDeviceExtensionProperties(device_, nullptr, &extensionCount, nullptr);
+ if (result != VK_SUCCESS) {
+ return false;
+ }
std::vector<VkExtensionProperties> availableExtensions(extensionCount);
- vkEnumerateDeviceExtensionProperties(device_, nullptr, &extensionCount, availableExtensions.data());
+ result = vkEnumerateDeviceExtensionProperties(device_, nullptr, &extensionCount, availableExtensions.data());
+ if (result != VK_SUCCESS) {
+ return false;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| bool RHIImpl::checkDeviceExtensionSupport(VkPhysicalDevice device_) { | |
| uint32_t extensionCount; | |
| vkEnumerateDeviceExtensionProperties(device_, nullptr, &extensionCount, nullptr); | |
| std::vector<VkExtensionProperties> availableExtensions(extensionCount); | |
| vkEnumerateDeviceExtensionProperties(device_, nullptr, &extensionCount, availableExtensions.data()); | |
| bool RHIImpl::checkDeviceExtensionSupport(VkPhysicalDevice device_) { | |
| uint32_t extensionCount{}; | |
| VkResult result = vkEnumerateDeviceExtensionProperties(device_, nullptr, &extensionCount, nullptr); | |
| if (result != VK_SUCCESS) { | |
| return false; | |
| } | |
| std::vector<VkExtensionProperties> availableExtensions(extensionCount); | |
| result = vkEnumerateDeviceExtensionProperties(device_, nullptr, &extensionCount, availableExtensions.data()); | |
| if (result != VK_SUCCESS) { | |
| return false; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/runtime/renderer/RHIVulkan.cpp` around lines 318 - 323, The Vulkan
extension enumeration in RHIImpl::checkDeviceExtensionSupport(VkPhysicalDevice)
leaves extensionCount uninitialized and ignores
vkEnumerateDeviceExtensionProperties return values; change to initialize
uint32_t extensionCount = 0, call vkEnumerateDeviceExtensionProperties and check
its VkResult for success before using extensionCount, bail out (return false or
handle error) if the call fails, then allocate/resize the
std::vector<VkExtensionProperties> to extensionCount and call
vkEnumerateDeviceExtensionProperties again while checking its result; ensure you
validate extensionCount (e.g., zero means no extensions) before using
availableExtensions.
| throw SegfaultException("failed to acquire swap chain image!"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify throw/catch balance around public API boundaries.
rg -nP '^\s*(bool|void)\s+RHI::(init|drawFrame)\s*\(' src/runtime/renderer/RHIVulkan.cpp -A120
rg -nP '\bthrow\s+SegfaultException\s*\(' src/runtime/renderer/RHIVulkan.cpp
rg -nP '\bcatch\s*\(' src/runtime/renderer/RHIVulkan.cppRepository: kimkulling/Segfault
Length of output: 8447
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/runtime/renderer/RHIVulkan.cpp"
# Show context around the specific throw sites mentioned in the review
sed -n '1000,1100p' "$file" | cat -n
# Find the RHIImpl drawFrame implementation and show around it
rg -n "drawFrame\\s*\\(" "$file" | head -n 20
# also dump a larger window around the likely drawFrame impl region
sed -n '1600,1750p' "$file" | cat -n
# Confirm whether any catch exists anywhere in this file
rg -n "\\bcatch\\b" "$file" || trueRepository: kimkulling/Segfault
Length of output: 8214
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Find any catches for SegfaultException anywhere in the repo
rg -n --hidden --no-ignore-vcs --glob='!**/node_modules/**' 'catch\s*\(\s*[^)]*SegfaultException' . || true
rg -n --hidden --no-ignore-vcs --glob='!**/node_modules/**' 'SegfaultException' . | head -n 50
# 2) Find call sites of RHI::drawFrame()
rg -n '\.drawFrame\s*\(\s*\)\s*;' src || true
rg -n 'RHI::drawFrame\s*\(' . || true
# 3) Inspect the loop where drawFrame is called most likely (grab a few hits)
# (Use only a small amount of context to avoid huge output)
rg -n '\.drawFrame\s*\(\s*\)\s*;' src -n -C 3 || trueRepository: kimkulling/Segfault
Length of output: 3789
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Find any catch blocks that might intercept exceptions generically
rg -n --hidden --no-ignore-vcs --glob='!**/node_modules/**' '\bcatch\s*\(' src | head -n 200
# 2) Inspect SegfaultException definition
if [ -f src/runtime/core/segfaultexception.h ]; then
nl -ba src/runtime/core/segfaultexception.h | sed -n '1,120p'
fi
# 3) Inspect hello_world drawFrame callsite and surrounding code
nl -ba src/examples/hello_world/hello_world.cpp | sed -n '1,120p'
# 4) Trace where myApp.drawFrame() is implemented (look for "drawFrame()" method in app classes)
rg -n 'drawFrame\s*\(\s*\)' src/runtime src/examples | head -n 200Repository: kimkulling/Segfault
Length of output: 45
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Find any catch blocks that might intercept exceptions
rg -n --hidden --no-ignore-vcs --glob='!**/node_modules/**' '\bcatch\s*\(' src || true | head -n 200
# 2) Inspect SegfaultException definition
if [ -f src/runtime/core/segfaultexception.h ]; then
nl -ba src/runtime/core/segfaultexception.h | sed -n '1,160p'
fi
# 3) Inspect hello_world drawFrame callsite and surrounding code
nl -ba src/examples/hello_world/hello_world.cpp | sed -n '1,200p'
# 4) Locate implementations of drawFrame() methods (likely app delegates)
rg -n '(\b|::)drawFrame\s*\(\s*\)' src/runtime src/examples || true
# 5) Locate main loop / drawFrame routing (look for "mainloop" and "RHI")
rg -n 'mainloop\s*\(\s*\)|RHI::drawFrame|myApp\.drawFrame|drawFrame\s*\(\s*\)\s*;' src/examples src/runtime || trueRepository: kimkulling/Segfault
Length of output: 106
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Find any catch blocks that might intercept exceptions
rg -n --hidden --no-ignore-vcs --glob='!**/node_modules/**' '\bcatch\s*\(' src || true | head -n 200
# 2) Inspect SegfaultException definition
if [ -f src/runtime/core/segfaultexception.h ]; then
cat -n src/runtime/core/segfaultexception.h | sed -n '1,200p'
fi
# 3) Inspect hello_world drawFrame callsite and surrounding code
if [ -f src/examples/hello_world/hello_world.cpp ]; then
cat -n src/examples/hello_world/hello_world.cpp | sed -n '1,220p'
fi
# 4) Locate implementations of drawFrame() methods (avoid matching RHI::drawFrame signature only by including "void ...::drawFrame")
rg -n 'void\s+([A-Za-z_]\w*::)?drawFrame\s*\(' src/runtime src/examples || true | head -n 200
# 5) Locate main loop / routing
rg -n 'mainloop\s*\(\s*\)|myApp\.drawFrame\s*\(\s*\)|RHI::drawFrame\s*\(' src/examples src/runtime || true | head -n 200Repository: kimkulling/Segfault
Length of output: 2325
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file_app="src/runtime/application/app.cpp"
file_apph="src/runtime/application/app.h"
file_rhi="src/runtime/renderer/RHI.h"
file_rhi_impl="src/runtime/renderer/RHIVulkan.cpp"
# Show drawFrame/mainloop implementations with surrounding context
cat -n "$file_app" | sed -n '1,220p'
echo "----"
cat -n "$file_app" | sed -n '220,420p' || true
echo "---- RHI.h ----"
cat -n "$file_rhi" | sed -n '1,140p'
echo "---- RHI::drawFrame call in App::drawFrame ----"
rg -n "drawFrame\\s*\\(\\s*\\)" "$file_app" "$file_apph" || true
rg -n "RHI::drawFrame|mRHI->drawFrame|drawFrame\\s*\\(" "$file_app" -n -C 3 || true
echo "---- try/catch in App:: paths ----"
rg -n "\\btry\\b|\\bcatch\\b" "$file_app" "$file_apph" "$file_rhi_impl" || trueRepository: kimkulling/Segfault
Length of output: 8246
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/runtime/renderer/RHIVulkan.cpp"
# Helper: print a window with nearby function header lines
for start in 180 200 600 660 760 780 800 820 860 880 900 960 980 1000 1010 1030 1060 1080 1100 1120 1140 1160 1200 1230 1300 1450; do
echo "==== lines $start-$((start+80)) ===="
sed -n "${start},$((start+80))p" "$file" | head -n 120
doneRepository: kimkulling/Segfault
Length of output: 50375
🏁 Script executed:
rg -n --hidden --no-ignore-vcs --glob='!**/node_modules/**' '\bcatch\s*\(' src | head -n 50 || trueRepository: kimkulling/Segfault
Length of output: 45
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/runtime/renderer/RHIVulkan.cpp"
# Print the beginning of the file to see namespaces/includes/usings for SegfaultException
sed -n '1,120p' "$file" | cat -n
# Also show the exact RHI::init signature/namespace context (first few lines of the function)
sed -n '1500,1565p' "$file" | cat -nRepository: kimkulling/Segfault
Length of output: 7765
Translate renderer exceptions at the RHI API boundary.
RHIImpl::drawFrame() throws SegfaultException on swapchain acquire/submit/present failures, but neither RHI::drawFrame() (void) nor RHI::init() (bool) catches, and App::drawFrame()/main have no exception handling either—so these exceptions can escape the RHI boundary and terminate the process instead of controlled failure.
Add boundary handling around RHI::init() and RHI::drawFrame() to catch segfault::core::SegfaultException and map to false (and/or a log) / safely return without propagating.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/runtime/renderer/RHIVulkan.cpp` around lines 1025 - 1026, Wrap calls into
the RHI boundary so SegfaultException cannot escape: in the public RHI API entry
points (RHI::init() and RHI::drawFrame()) catch
segfault::core::SegfaultException and convert it to a safe failure (return false
from RHI::init() and return from RHI::drawFrame() without throwing), logging the
error; likewise add try/catch in higher-level callers (App::drawFrame() and
main) where they call RHIImpl::drawFrame()/RHI::drawFrame() to ensure any
remaining segfault::core::SegfaultException is caught, logged, and translated to
a controlled shutdown path rather than allowing the exception to propagate.
Source: Coding guidelines
1 participant
Summary by CodeRabbit