ARM-software / vulkan-sdk

Github repository for the Vulkan SDK
Other
228 stars 49 forks source link

Incorrect external dependency in hellotriangle sample #9

Closed zeux closed 6 years ago

zeux commented 7 years ago

This code: https://github.com/ARM-software/vulkan-sdk/blob/master/samples/hellotriangle/hellotriangle.cpp#L211-L226

Has misleading comments and incorrect dstAccessMask. As per offline discussion with @Tobski, we don't need a write-after-read barrier here - the semaphore establishes that - but we need a write-after-write dependency to make sure that the writes to color attachment don't get reordered with the (implied) write operations from the image layout transition. This requires dstAccessMask to be COLOR_ATTACHMENT_OPTIMAL.

We haven't discussed this so I'm less sure about this but it looks like this also needs an external dependency at the end of the pass (so 0 -> EXTERNAL) to establish a read-after-write barrier to make sure that (implied) read operations from the image layout transition wait until the color attachment writes complete.

One day validation layers will check for things like this.

HansKristian-Work commented 7 years ago

Bugs have been filed internally for a while. As far as I can tell the problem is the lack of a proper dstAccessMask (seems like a strange misunderstanding at the time). I suspect most of the samples have this problem.

0 -> EXTERNAL is not needed though, because that kind of barrier is implied by the spec, we get BOTTOM_OF_PIPE dstStages by default, but that's fine because we signal a semaphore in order to go to present, so the default external dependency does the intended thing here.

JoseEmilio-ARM commented 7 years ago

Hi, What do you mean by

This requires dstAccessMask to be COLOR_ATTACHMENT_OPTIMAL

That isn't a valid flag right?

zeux commented 7 years ago

Sorry, this was supposed to say VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT

JoseEmilio-ARM commented 7 years ago

Should have been fixed in https://github.com/ARM-software/vulkan-sdk/commit/b0034a72fec7a59e38206ccabd8e7b024fbd3522