Skip to content

Update from task a6ea5542-24e3-4e5b-ad34-e79b70444f85 #30

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.

Sign up for GitHub

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

Open
tronpis wants to merge 2 commits into
base: rendergraph-dependency-issues-4b345
Choose a base branch
from
Open

Update from task a6ea5542-24e3-4e5b-ad34-e79b70444f85 #30

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0807c5c
Fix VK_DEPENDENCY_BY_REGION_BIT overuse in RenderGraph barriers
qwen-intl Apr 2, 2026
3ecc116
Update FarmEngine/src/rendering/graph/RenderGraph.cpp
tronpis Apr 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 8 additions & 10 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,33 +1,30 @@
```
# Compiled and binary files
```cpp
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The .gitignore file contains markdown code block syntax (```cpp). This is invalid for a .gitignore file and should be removed. Git will interpret this line as a literal pattern to match.

# Compiled and build artifacts

# Compiled and build artifacts
*.o
*.obj
*.exe
*.dll
*.so
*.a
*.out
build/
dist/

# Dependencies
node_modules/
venv/
.venv/
node_modules/
__pycache__/
.mypy_cache/
.pytest_cache/
target/
.gradle/

# Build artifacts
build/
dist/

# Editor/IDE files
# Editors
.vscode/
.idea/
*.swp
*.swo
*.tmp

# System files
.DS_Store
Expand All @@ -36,8 +33,9 @@ Thumbs.db
.env.local
*.env.*

# Logs
# Logs and temp files
*.log
*.tmp

# Coverage
coverage/
Expand Down
62 changes: 50 additions & 12 deletions FarmEngine/src/rendering/graph/RenderGraph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,16 @@ RenderGraphBuilder& RenderGraphBuilder::addDependency(uint32_t from, uint32_t to
VkPipelineStageFlags srcStage,
VkPipelineStageFlags dstStage,
VkAccessFlags srcAccess,
VkAccessFlags dstAccess) {
VkAccessFlags dstAccess,
VkDependencyFlags dependencyFlags) {
PassDependency dep;
dep.from = from;
dep.to = to;
dep.srcStageMask = srcStage;
dep.dstStageMask = dstStage;
dep.srcAccessMask = srcAccess;
dep.dstAccessMask = dstAccess;
dep.dependencyFlags = dependencyFlags;
explicitDependencies.push_back(dep);
return *this;
}
Expand All @@ -136,7 +138,8 @@ RenderGraphBuilder& RenderGraphBuilder::addResourceDependency(uint32_t from, uin
VkAccessFlags dstAccess,
VkImageLayout oldLayout,
VkImageLayout newLayout,
VkImageAspectFlags aspectMask) {
VkImageAspectFlags aspectMask,
VkDependencyFlags dependencyFlags) {
PassDependency dep;
dep.from = from;
dep.to = to;
Expand All @@ -148,6 +151,7 @@ RenderGraphBuilder& RenderGraphBuilder::addResourceDependency(uint32_t from, uin
dep.oldLayout = oldLayout;
dep.newLayout = newLayout;
dep.aspectMask = aspectMask;
dep.dependencyFlags = dependencyFlags;
explicitDependencies.push_back(dep);
return *this;
}
Expand Down Expand Up @@ -268,6 +272,7 @@ void RenderGraph::compile(RenderGraphBuilder&& builder) {
pendingBarrier.oldLayout = dep.oldLayout;
pendingBarrier.newLayout = dep.newLayout;
pendingBarrier.aspectMask = dep.aspectMask;
pendingBarrier.dependencyFlags = dep.dependencyFlags;

compiledPasses[dep.to].pendingResourceBarriers.push_back(pendingBarrier);
} else {
Expand All @@ -279,7 +284,7 @@ void RenderGraph::compile(RenderGraphBuilder&& builder) {
passDep.dstStageMask = dep.dstStageMask;
passDep.srcAccessMask = dep.srcAccessMask;
passDep.dstAccessMask = dep.dstAccessMask;
passDep.dependencyFlags = VK_DEPENDENCY_BY_REGION_BIT;
passDep.dependencyFlags = dep.dependencyFlags;

compiledPasses[dep.to].passLevelDependencies.push_back(passDep);
}
Expand Down Expand Up @@ -374,8 +379,8 @@ void RenderGraph::recordBarriers(VkCommandBuffer cmd, const CompiledPass& pass,
VkPipelineStageFlags combinedSrcStage = 0;
VkPipelineStageFlags combinedDstStage = 0;

// Combine dependency flags from all pass-level dependencies
VkDependencyFlags combinedDependencyFlags = 0;
// Combine dependency flags from all pass-level dependencies (for memory barriers only)
VkDependencyFlags combinedMemoryDependencyFlags = 0;

// Process pass-level dependencies (emit as VkMemoryBarrier)
std::vector<VkMemoryBarrier> memoryBarriers;
Comment on lines +382 to 386
Copy link
Copy Markdown

@qodo-code-review qodo-code-review Bot Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Per-dependency flags lost 🐞 Bug ≡ Correctness

RenderGraph::recordBarriers bitwise-ORs dependencyFlags across all memory barriers and across all
image barriers, then applies the combined value to a single vkCmdPipelineBarrier call. This makes it
impossible to use different VkDependencyFlags for different dependencies of the same barrier type
because a flag set by one dependency is applied to all barriers in that batch.
Agent Prompt 
### Issue description
`RenderGraph::recordBarriers()` currently merges `VkDependencyFlags` via bitwise-OR across all memory barriers and across all image barriers, then uses that single value for a batched `vkCmdPipelineBarrier()` call. Since `vkCmdPipelineBarrier()` takes one `dependencyFlags` value per call, this causes any flag requested by one dependency to be applied to all barriers in that batch, defeating the intent of the new per-dependency `dependencyFlags` API.

### Issue Context
The builder now accepts `dependencyFlags` per dependency, so the runtime barrier emission should respect heterogeneous flags.

### Fix Focus Areas
- FarmEngine/src/rendering/graph/RenderGraph.cpp[377-482]

### Suggested approach
1. Group barriers by `VkDependencyFlags` value (at least within each barrier type):
   - Build `std::unordered_map<VkDependencyFlags, Group>` where `Group` contains:
     - `std::vector<VkMemoryBarrier> memoryBarriers`
     - `std::vector<VkImageMemoryBarrier> imageBarriers`
     - `VkPipelineStageFlags srcStages`, `dstStages`
2. For each pass-level dependency, append to `groups[dep.dependencyFlags].memoryBarriers` and OR its stage masks into that group's stage masks.
3. For each pending resource barrier, append to `groups[pending.dependencyFlags].imageBarriers` and OR its stage masks into that group's stage masks.
4. Iterate groups and emit **one `vkCmdPipelineBarrier()` per flags value**, using that group’s stage masks (with the existing ALL_COMMANDS fallback per group).

This preserves per-dependency flag semantics while still batching where possible.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Expand All @@ -388,11 +393,13 @@ void RenderGraph::recordBarriers(VkCommandBuffer cmd, const CompiledPass& pass,

combinedSrcStage |= dep.srcStageMask;
combinedDstStage |= dep.dstStageMask;
combinedDependencyFlags |= dep.dependencyFlags;
combinedMemoryDependencyFlags |= dep.dependencyFlags;
}

// Process resource-specific barriers (resolve image at runtime)
// Collect dependency flags from resource barriers separately
std::vector<VkImageMemoryBarrier> imageBarriers;
VkDependencyFlags combinedImageDependencyFlags = 0;
for (const auto& pending : pass.pendingResourceBarriers) {
VkImage image = registry.getImage(pending.resourceName);
const ResourceHandle* res = registry.getResource(pending.resourceName);
Expand Down Expand Up @@ -421,19 +428,50 @@ void RenderGraph::recordBarriers(VkCommandBuffer cmd, const CompiledPass& pass,

combinedSrcStage |= pending.srcStageMask;
combinedDstStage |= pending.dstStageMask;
combinedImageDependencyFlags |= pending.dependencyFlags;
}

// Emit barriers if any exist
if (!memoryBarriers.empty() || !imageBarriers.empty()) {
// Use combined stage masks, or fallback to ALL_COMMANDS if no stages were specified
VkPipelineStageFlags srcStage = combinedSrcStage != 0 ? combinedSrcStage : VK_PIPELINE_STAGE_ALL_COMMANDS_BIT;
VkPipelineStageFlags dstStage = combinedDstStage != 0 ? combinedDstStage : VK_PIPELINE_STAGE_ALL_COMMANDS_BIT;
// Emit barriers - use separate calls when dependency flags differ to avoid
// unintentionally applying flags (like VK_DEPENDENCY_BY_REGION_BIT) to all barrier types
const bool hasMemoryBarriers = !memoryBarriers.empty();
const bool hasImageBarriers = !imageBarriers.empty();
const bool flagsDiffer = combinedMemoryDependencyFlags != combinedImageDependencyFlags;

// Use combined stage masks, or fallback to ALL_COMMANDS if no stages were specified
VkPipelineStageFlags srcStage = combinedSrcStage != 0 ? combinedSrcStage : VK_PIPELINE_STAGE_ALL_COMMANDS_BIT;
VkPipelineStageFlags dstStage = combinedDstStage != 0 ? combinedDstStage : VK_PIPELINE_STAGE_ALL_COMMANDS_BIT;

if (hasMemoryBarriers && hasImageBarriers && flagsDiffer) {
// Emit separate barrier calls to keep dependency flags isolated
vkCmdPipelineBarrier(
cmd,
srcStage,
dstStage,
combinedMemoryDependencyFlags,
static_cast<uint32_t>(memoryBarriers.size()),
memoryBarriers.data(),
0, nullptr,
0, nullptr
);

vkCmdPipelineBarrier(
cmd,
srcStage,
dstStage,
combinedDependencyFlags,
combinedImageDependencyFlags,
0, nullptr,
0, nullptr,
static_cast<uint32_t>(imageBarriers.size()),
imageBarriers.data()
);
} else if (hasMemoryBarriers || hasImageBarriers) {
// Same flags or only one type of barrier - can combine in single call
VkDependencyFlags finalFlags = !memoryBarriers.empty() ? combinedMemoryDependencyFlags : combinedImageDependencyFlags;
vkCmdPipelineBarrier(
cmd,
srcStage,
dstStage,
finalFlags,
static_cast<uint32_t>(memoryBarriers.size()),
memoryBarriers.data(),
0, nullptr,
Expand Down
8 changes: 6 additions & 2 deletions FarmEngine/src/rendering/graph/RenderGraph.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ struct PassDependency {
VkImageLayout oldLayout = VK_IMAGE_LAYOUT_UNDEFINED;
VkImageLayout newLayout = VK_IMAGE_LAYOUT_UNDEFINED;
VkImageAspectFlags aspectMask = VK_IMAGE_ASPECT_COLOR_BIT;
VkDependencyFlags dependencyFlags = 0;
};

class RenderPass {
Expand Down Expand Up @@ -109,7 +110,8 @@ class RenderGraphBuilder {
// in sequential order. Attempting to add a dependency where from >= to will throw an error.
RenderGraphBuilder& addDependency(uint32_t from, uint32_t to,
VkPipelineStageFlags srcStage, VkPipelineStageFlags dstStage,
VkAccessFlags srcAccess, VkAccessFlags dstAccess);
VkAccessFlags srcAccess, VkAccessFlags dstAccess,
VkDependencyFlags dependencyFlags = 0);

// Definir dependencias específicas de recursos (para barreras de imagen)
// Note: The 'from' pass index must be less than the 'to' pass index, as passes execute
Expand All @@ -119,7 +121,8 @@ class RenderGraphBuilder {
VkPipelineStageFlags srcStage, VkPipelineStageFlags dstStage,
VkAccessFlags srcAccess, VkAccessFlags dstAccess,
VkImageLayout oldLayout, VkImageLayout newLayout,
VkImageAspectFlags aspectMask = VK_IMAGE_ASPECT_COLOR_BIT);
VkImageAspectFlags aspectMask = VK_IMAGE_ASPECT_COLOR_BIT,
VkDependencyFlags dependencyFlags = 0);

private:
friend class RenderGraph;
Expand Down Expand Up @@ -172,6 +175,7 @@ class RenderGraph {
VkImageLayout oldLayout;
VkImageLayout newLayout;
VkImageAspectFlags aspectMask;
VkDependencyFlags dependencyFlags = 0;
};

// Pass-level execution dependency info - emitted as VkMemoryBarrier at execution time
Expand Down