-
Notifications
You must be signed in to change notification settings - Fork 453
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
[wiki]Extraneous stage and access flags in depth clear\reuse example #2055
Comments
Seems like the spec changed at some point here, so to synchronize the loadOp = CLEAR it makes sense to only have EARLY_FRAGMENT_TESTS. But fragments later in the render pass might end up running in EARLY or LATE and it may read or write, so I think we technically need to be conservative here. |
@HansKristian-Work Not sure that is healthy to be "conservative" with something that is supposed to be formally described exactly. If that is so, there should be a red triangle with three exclamation marks with a note explaining why it is done in opposition of what the spec implies. Even so, the |
The WG discussed on 2023-02-22. @krOoze's analysis is correct, but we're going to leave the example code as is. We expect that many devs will copy-paste the code into an app and then replace/follow the |
@TomOlson Following the Replacing the Only time this might make sense is dependencies between subpasses. |
Ok, so this section of the spec got tightened up a lot since the examples were written, and I'm inclined to agree with @krOoze here. The only time being conservative would make sense is if you used the NONE ops (i.e. there is no load/store), but those are primarily intended for cases when you aren't going to access the data, so you wouldn't need a barrier 99% of the time anyway. So I am going to update the example just so it's clear that yea, you can rely on the subset of pipeline stages. |
Fixed per above. Assuming that satisfies you @krOoze I'm closing this issue, feel free to reopen if there's any further issues... |
@Tobski Well, the access masks are still extraneous. If we really want it to be robust against load\store op change, I would prefer this nuance to be explicitly stated in the code, such as:
|
We're currently discussing whether write flags are required at all in destination access flags because it seems there's been some confusion; until that's resolved I don't want to touch this, because the resolution there will affect the point you're making, and whether it needs addressing at all; but yea it probably should be a little more explicit about handling the different load ops. |
There are extraneous stage and access bits in the following depth clear\reuse example
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
Load op
VK_ATTACHMENT_LOAD_OP_CLEAR
for depth are performed inVK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT
withVK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT
. Thereforedst
ofVK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT
andVK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT
should be redundant.The text was updated successfully, but these errors were encountered: