-
Notifications
You must be signed in to change notification settings - Fork 0
Update from task 5787d978-9f14-47e6-a307-052b41d15f39 #26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: rendergraphsystemenhancements-6bac1
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,29 +1,66 @@ | ||
| ``` | ||
| # Build artifacts | ||
| # Compiled and binary files | ||
| *.o | ||
| *.obj | ||
| *.a | ||
| *.lib | ||
| *.so | ||
| *.dll | ||
| *.exe | ||
| *.dll | ||
| *.so | ||
| *.a | ||
| *.out | ||
|
|
||
| # CMake build directories | ||
| build/ | ||
| cmake-build-*/ | ||
| CMakeFiles/ | ||
| CMakeCache.txt | ||
| Makefile | ||
| compile_commands.json | ||
|
|
||
| # Dependencies | ||
| vendor/ | ||
| node_modules/ | ||
| venv/ | ||
| .venv/ | ||
| __pycache__/ | ||
| dist/ | ||
| build/ | ||
| target/ | ||
| .gradle/ | ||
|
|
||
| # Logs | ||
| *.log | ||
| # Editors/IDE | ||
| .vscode/ | ||
| .idea/ | ||
| *.swp | ||
| *.swo | ||
|
|
||
| # OS generated files | ||
| # System/Environment | ||
| .DS_Store | ||
| Thumbs.db | ||
| .env | ||
| .env.local | ||
| *.env.* | ||
|
|
||
| # Logs and temp files | ||
| *.log | ||
| *.tmp | ||
|
|
||
| # Coverage | ||
| coverage/ | ||
| htmlcov/ | ||
| .coverage | ||
|
|
||
| # Compressed files | ||
| *.zip | ||
| *.gz | ||
| *.tar | ||
| *.tgz | ||
| *.bz2 | ||
| *.xz | ||
| *.7z | ||
| *.rar | ||
| *.zst | ||
| *.lz4 | ||
| *.lzh | ||
| *.cab | ||
| *.arj | ||
| *.rpm | ||
| *.deb | ||
| *.Z | ||
| *.lz | ||
| *.lzo | ||
| *.tar.gz | ||
| *.tar.bz2 | ||
| *.tar.xz | ||
| *.tar.zst | ||
| ``` | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,10 @@ const ResourceHandle* ResourceRegistry::getResource(const std::string& name) con | |
| if (it == nameToIndex.end()) { | ||
| return nullptr; | ||
| } | ||
| // Defensive bounds check to prevent out-of-bounds access with stale indices | ||
| if (it->second >= resources.size()) { | ||
| return nullptr; | ||
| } | ||
| return &resources[it->second]; | ||
| } | ||
|
|
||
|
|
@@ -123,10 +127,13 @@ RenderGraphBuilder& RenderGraphBuilder::addDependency(uint32_t from, uint32_t to | |
| // ============================================================================ | ||
| // RenderGraph Implementation | ||
| // ============================================================================ | ||
|
|
||
| void RenderGraph::compile(RenderGraphBuilder&& builder) { | ||
| // Copiar recursos y passes | ||
| compiledRegistry.resources = std::move(builder.resources); | ||
|
|
||
| // Limpiar estado previo antes de compilar | ||
| compiledRegistry.nameToIndex.clear(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛑 Logic Error: The |
||
| compiledPasses.clear(); | ||
|
|
||
| compiledPasses.resize(builder.passes.size()); | ||
|
|
@@ -197,9 +204,29 @@ RenderGraphBuilder& RenderGraphBuilder::addDependency(uint32_t from, uint32_t to | |
| // TODO: Implementar análisis de dependencias del grafo | ||
| } | ||
|
|
||
| // Las dependencias entre passes separados se manejan mediante barreras de pipeline | ||
| // registradas durante la ejecución, no mediante VkSubpassDependency | ||
| // VkSubpassDependency solo es válido para subpasses dentro del mismo VkRenderPass | ||
| // Procesar dependencias explícitas y convertirlas en barreras por-pass | ||
| for (const auto& dep : builder.explicitDependencies) { | ||
| if (dep.to < compiledPasses.size()) { | ||
| VkImageMemoryBarrier barrier{}; | ||
| barrier.sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER; | ||
| barrier.srcAccessMask = dep.srcAccessMask; | ||
| barrier.dstAccessMask = dep.dstAccessMask; | ||
| barrier.oldLayout = VK_IMAGE_LAYOUT_UNDEFINED; | ||
| barrier.newLayout = VK_IMAGE_LAYOUT_UNDEFINED; | ||
|
Comment on lines
+214
to
+215
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛑 Logic Error: Image memory barriers are created with both oldLayout and newLayout set to |
||
| barrier.srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED; | ||
| barrier.dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED; | ||
| barrier.image = VK_NULL_HANDLE; // Se establecerá durante la ejecución | ||
| barrier.subresourceRange = {VK_IMAGE_ASPECT_COLOR_BIT, 0, 1, 0, 1}; | ||
|
|
||
| compiledPasses[dep.to].prePassBarriers.push_back(barrier); | ||
|
Comment on lines
+210
to
+221
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This block creates
Since the |
||
| } | ||
|
Comment on lines
+207
to
+222
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1. Null vulkan image barriers RenderGraph::compile() builds VkImageMemoryBarrier objects with image=VK_NULL_HANDLE and UNDEFINED->UNDEFINED layouts, and recordBarriers() submits them unchanged, which will trigger Vulkan validation errors or fail to synchronize passes. This also ignores the dependency’s stage masks and hardcodes VK_IMAGE_ASPECT_COLOR_BIT, breaking depth dependencies (e.g., ShadowMap). Agent Prompt
|
||
| } | ||
| } | ||
|
|
||
| void RenderGraph::build(VkDevice dev, VkExtent2D swapchainExtent) { | ||
| device = dev; | ||
| createRenderPasses(device); | ||
| createFramebuffers(device, swapchainExtent); | ||
| } | ||
|
Comment on lines
+226
to
230
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2. Graph build step unused VkRenderPass/VkFramebuffer creation has been moved into a new RenderGraph::build(), but existing render-pipeline code only calls compile(), so execute() will use VK_NULL_HANDLE renderPass/framebuffer. This will cause vkCmdBeginRenderPass() validation failures/crashes when the graph is executed. Agent Prompt
|
||
|
|
||
| void RenderGraph::createRenderPasses(VkDevice dev) { | ||
|
|
@@ -308,7 +335,25 @@ void RenderGraph::execute(VkCommandBuffer cmd, ResourceRegistry& registry, uint3 | |
| rpBegin.sType = VK_STRUCTURE_TYPE_RENDER_PASS_BEGIN_INFO; | ||
| rpBegin.renderPass = compiled.vkRenderPass; | ||
| rpBegin.framebuffer = compiled.framebuffer; | ||
| rpBegin.renderArea.extent = compiled.extent; | ||
| // Usar el extent del CompiledPass (calculado en createFramebuffers) | ||
| // Si es {0,0}, calcularlo dinámicamente desde los attachments como fallback | ||
| if (compiled.extent.width == 0 || compiled.extent.height == 0) { | ||
| VkExtent2D fallbackExtent = {800, 600}; // Default fallback | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛑 Logic Error: The hardcoded fallback extent of 800x600 is arbitrary and may not match the actual framebuffer dimensions, potentially causing rendering artifacts or validation errors. Since
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| if (!compiled.definition.colorAttachments.empty()) { | ||
| const ResourceHandle* res = compiledRegistry.getResource(compiled.definition.colorAttachments[0]); | ||
| if (res && !res->isSwapchain) { | ||
| fallbackExtent = res->extent; | ||
| } | ||
| } else if (!compiled.definition.depthAttachment.empty()) { | ||
| const ResourceHandle* res = compiledRegistry.getResource(compiled.definition.depthAttachment); | ||
| if (res) { | ||
| fallbackExtent = res->extent; | ||
| } | ||
| } | ||
| rpBegin.renderArea.extent = fallbackExtent; | ||
| } else { | ||
| rpBegin.renderArea.extent = compiled.extent; | ||
| } | ||
|
|
||
| // Clear values - uno por cada attachment (color attachments primero, luego depth) | ||
| std::vector<VkClearValue> clearValues; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
.gitignorefile starts with triple backticks (```), which appear to be a markdown formatting artifact. This line should be removed as it is not a valid git pattern.