Support depth only pass, more thorough attachment merge checking.

[DGriffin91]

Depth only passes are useful for depth prepasses and non-vsm shadow maps.

This PR addresses two issues that were blocking using depth only passes.

• Resolver render_area() wasn't taking into account the depth attachment
• When merging passes, if the lhs candidate had less or no color attachments, it would not be evaluated against rhs. Also it would not check if the other attachment was none and Attachment::are_compatible allows one or both of the attachments to be none. I tried addressing this in Attachment::are_compatible but it seems the usage elsewhere needs to be able to allow one of the attachments to be none.

I updated the multipass example with a depth only prepass for testing. This doesn't need to be included in the PR, or could be made optional. Since this example uses the stencil buffer for layering rather than the depth buffer, further adjustments would need to be made for an appropriate example.

[DGriffin91]

Thinking about this more, maybe it would be better to just first check if they have the same total attachment quantity? Assuming that's a requirement anyway.

[DGriffin91]

Was just realizing that some of the checks at the end don't take into account that there may only be color attachments or only be depth attachments. Needs more work.

[DGriffin91]

If it helps at all, you can see what I currently do here (at the bottom)

master...DGriffin91:screen-13:bs13

if lhs_exec.color_attachments.len() != rhs_exec.color_attachments.len() {
    return false;
}
if lhs_exec.depth_stencil_attachment.is_some() != rhs_exec.depth_stencil_attachment.is_some() {
    return false;
}
...
if common_color_attachment && common_depth_attachment {

[attackgoat]

If it helps at all ...

Yes - it very much does. Between this codebase being somewhat sizable and my other projects being my main focus I very much appreciate the effort documenting these issues and the well-labeled commits.

I intend on fixing the merge/re-order correctness and performance using this depth pre-pass example as a test case. I have some ideas that I'd like to push to this branch, it should have no visible API change.

[DGriffin91]

Glad to help! Make whatever changes you'd like. I'm not tied to anything in this PR.

[attackgoat]

I think I have a solution, but I'm going to have to write a few more advanced examples to make sure it handles the edge cases (multi-layer image rendering where subpasses use the same image with different, possibly overlapping, subresource ranges).

In addition to your fixes, the main issue was that render passes were doing memory barriers and image layout transitions for just the first pass worth of accesses.

The depth pre-pass worked fine but the second merged render pass (funky shape PBR + fill background) requested, correctly, access to read/write the depth buffer. This would be good for compute/ray-trace passes but for graphic passes the image layout needs to be first read-only (to load) and then finally write-only (to store). It turns out none of this was actually required at all.

The graphic render passes already specify subpass dependencies to make sure access is correct, and all that needs to be done is the initial image layout of any images needs to be transitioned to whatever the renderpass wants (read for load in this case). I used the recently-added ImageAccess thingy to look at renderpasses that are being started and track the first access to their nodes. It's not as easy as just looking through the renderpass attachments because we also have to "throw away" any other access which the renderpass makes so that secondary work in other stages or graphic passes doesn't incorrectly wait on phantom work the subpass dependencies already waited on.

All examples and everything I've got written for Screen 13 run without validation or synchronization errors now, which is a good start, but I expect to find things to fix yet still.

[attackgoat]

TODO:

• Resolver::lease_render_pass(): Look through secondary passes when figuring out attachments if the attachment is unused in the first pass(es)

[attackgoat]

TL;DR: I had to make a number of fixes and roll-back a few of the merge-check changes (because they were not the issue), but this still needs additional tests and changes.

Notably we don't need to do LHS/RHS + RHS/LHS because the right hand side is always a single un-merged pass. So as long as LHS is compatible with it, it is valid to merge them. In general yes this would be required but the other code holds this invariant to be true and so it is skippable.

fuzzer.rs works again and can be a test bed for anything that needs to run against the validation layers (optimally withVK_LAYER_KHRONOS_synchronization2 enabled in vkconfig/vkconfig-gui, because the "debug" option Screen 13 provides does not enable this layer).

Lots of performance still on the table, but after focusing on correctness and additional tests, validating against anything outside of this repo, this is looking closer to merge to me.

[attackgoat]

The changes to examples/multipass.rs don't affect the rendered output, but they are fun to see when debugging with RenderDoc. It's fine to keep them as an example of the kinds of things that might be done with multi-pass rendering.

Vulkan allows renderpasses to be compatible if the attachments are unused in some subpasses, as long as they're identical (everything except aspect is equal) in the other passes. This is why the merging logic had those conditions allowed, and really it's just an optimization choice that could be made here. Maybe the API in the future allows passes to be scheduled with merge conditions, but for now it's a global and hard-coded choice to merge unless there is no commonality.

The other changes look good to me! If there are additional merge types which cause correctness issues I will help craft new test cases in examples/fuzzer.rs to lock them down. I briefly explored a flag to assert that passes are actually merged, but it seemed a little complicated for this PR so I just manually checked the merging by setting logging to trace level.