-
Notifications
You must be signed in to change notification settings - Fork 15
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
Wrong dstStage #19
Comments
Hi @jkoenen, while a layout transition might actually perform a write in reality, from the spec POV it does not. If an implementation does access an image as part of a layout transition, it is responsible for doing any required cache flushes - the user does not have to do anything, which is why no access masks are specified. There's no issue with the code here. The example you've quoted does the same as this test is expecting. |
I'm not sure I follow - I did get a validation error with your code and in the example the dstAccessMask is VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT instead of 0) |
Oh my apologies, I misread the example - been too long since I've looked at this clearly 😬 Let me take another look at this.. |
Okay now I'm very confused. I disagree with the comments I made in the examples (I wrote those too!). So two things - firstly, as a rule of thumb, you only need write access flags in the source and read access flags in the destination - write access in the destination is more or less meaningless, so I'm kind of confused as to why I suggested otherwise.
(From the spec under "Image Layout Transitions"). So as far as I can tell, the example and the validation layer are both wrong here. The 2:1 ratio would make me doubt that usually, but given validation used the examples as source documentation I'm not putting too much stock in it 🙃 Could you raise an issue against the validation layers with details of what you're doing? I'll leave this open until we resolve it... I'll also sort out the sync examples once we figure this out, so no need to raise a separate issue for that. |
Ack, hold up, this problem might run deeper than I thought, don't raise a validation issue yet (If you have nearly written it out feel free to post it though) - but I may need to go get some clarification on the spec here... |
This section in the spec is probably relevant:
I think the issue is that the layout transition only makes the memory available and not visible? |
Yes, the issue is that writes are only supposed to need previous writes available to avoid a data race. However, it seems the spec has been a bit loose on this, and VVL (and my example code) disagrees with what we'd intended. I've raised a spec bug against this internally and we'll see where we land with it... going to have to put a pin in this until that is sorted, because I don't want to change it and then revert it. (Btw thank you for raising this, it's something I wish we had caught sooner!) |
The following test seems to be wrong because the image layout transition is counted as a write operation and therefore it's required to specify VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT in the dstAccessMask.
The problem might be in thsvsGetVulkanImageMemoryBarrier in this block:
I think this code should run after determining the layout to see if a layout transition is specified and then set the dstaccessMask even when the srcAccessMask is 0.
For reference here is this exact synchronization example:
https://github.com/KhronosGroup/Vulkan-Docs/wiki/Synchronization-Examples-(Legacy-synchronization-APIs)#first-draw-samples-a-texture-in-the-fragment-shader-second-draw-writes-to-that-texture-as-a-color-attachment
The text was updated successfully, but these errors were encountered: