Overv / VulkanTutorial

Tutorial for the Vulkan graphics and compute API
https://vulkan-tutorial.com
Creative Commons Attribution Share Alike 4.0 International
3.06k stars 511 forks source link

Subpass dependency question #308

Closed PatrikTegelberg closed 11 months ago

PatrikTegelberg commented 1 year ago
.srcSubpass = VK_SUBPASS_EXTERNAL;
.dstSubpass = 0;
.srcStageMask = VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT | VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT; 
.srcAccessMask = 0; 
.dstStageMask = VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT | VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT;
.dstAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT;

The tutorial claims that: The [depth stencil] reading happens in the VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT stage and the writing in the VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT.

My question is: What happens to the writes in the late frag test, from the earlier subpass, which can here happen after the depth attachment layout transition? Aren't we potentially clearing/transitioning the depth attachment before the previous pass is done writing to it? Couldn't the previous writes corrupt the depth image after we cleared it in this render pass?

Shouldn't the srcMask be late_frag_test?

krOoze commented 1 year ago

@Overv Bump. Could you address this in some way. It was asked at least three times on StackOverflow. It indeed looks like a bug to me, or needs some deep explanation.

Overv commented 12 months ago

Unfortunately I personally don't have the knowledge to answer this question, so I'm hoping someone from Khronos can pitch in.

krOoze commented 12 months ago

@Overv That's somewhat strange to hear from someone who writes The Tutorial. You shouldn't put out code you yourself don't understand for newbs to mindlessly copy-paste...

But here you go: https://github.com/KhronosGroup/Vulkan-Docs/wiki/Synchronization-Examples#first-render-pass-writes-to-a-depth-attachment-second-render-pass-re-uses-the-same-depth-attachment

The spec seems also relatively clear when these things happen, and what Stages and Access Flags are relevant for Depth Attachment.

Overv commented 12 months ago

@krOoze I know, but you have to keep in mind that I wrote this tutorial when Vulkan had only very recently been released and there were still many unknowns. I haven't used Vulkan to further develop my knowledge since that point due to my career and personal projects heading into a different direction.

krOoze commented 12 months ago

@Overv That's interesting. And maybe worth a disclaimer. It feels your new incoming readers should have that in mind rather than me.

IDK, this specifically was clear from the spec from Nov 2016 going forward.

Anyway, it indeed produces a validation error if sync validation is enabled:

SYNC-HAZARD-WRITE-AFTER-WRITE(ERROR / SPEC): msgNum: 1544472022 - Validation Error: [ SYNC-HAZARD-WRITE-AFTER-WRITE ] Object 0: handle = 0x1e838e13820, type = VK_OBJECT_TYPE_QUEUE; | MessageID = 0x5c0ec5d6 | vkQueueSubmit: Hazard WRITE_AFTER_WRITE for entry 0, VkCommandBuffer 0x1e83c112660[], Recorded access info (recorded_usage: SYNC_IMAGE_LAYOUT_TRANSITION, command: vkCmdBeginRenderPass, seq_no: 1,renderpass: VkRenderPass 0xee647e0000000009[], reset_no: 2). Access info (prior_usage: SYNC_LATE_FRAGMENT_TESTS_DEPTH_STENCIL_ATTACHMENT_WRITE, write_barriers: 0, queue: VkQueue 0x1e838e13820[], submit: 5, batch: 0, batch_tag: 12, command: vkCmdEndRenderPass, seq_no: 3, command_buffer: VkCommandBuffer 0x1e83c090f40[],renderpass: VkRenderPass 0xee647e0000000009[], reset_no: 2). Objects: 1 [0] 0x1e838e13820, type: 4, name: NULL

Proposed change seems to fix it. I am gonna add a PR to that effect, though I don't plan to touch the tutorial text, which if you are not active this long is bound to be a rabbit hole.

Overv commented 11 months ago

IDK, this specifically was clear from the spec from Nov 2016 going forward.

I agree with you that releasing a tutorial that may contain incorrect and/or incomplete information certainly carries some risks and I should have probably been more forthcoming about that, but it is important to understand the context in which it was written.

At the time (July 2016) there were almost no resources available besides the specification, SDK sample code, and an early version of Sascha Willem's examples. Content like the synchronization examples you mentioned was only released much later because there was so much confusion about the topic in the community. The tutorial was very much written at a time where everyone was still kind of trying to figure out how Vulkan worked.

The idea was to make sure that there was at least some kind of half-decent beginner tutorial available as soon as possible, and to maintain it as an open source project that so that the community could contribute corrections and missing information as we collectively gained a better understanding of how Vulkan was supposed to be used.

Now, what didn't exactly go according to plan, is that I personally started lacking time to continue practicing computer graphics and using Vulkan, and that meant that maintaining the tutorial started to solely depend on the community with little oversight from my end. This has caused some mistakes in the tutorial to remain there for a long time because I was no longer able to personally re-review it.

With all of the fixes and contributions from the community over the years, which I'm very thankful for, I mistakenly thought that all of the major mistakes had been ironed out, but that has unfortunately turned out not to be the case and I apologize for that.