Vulkan-docs: Using a read-only attachment in a shader binding

Created on 11 Aug 2020  路  11Comments  路  Source: KhronosGroup/Vulkan-Docs

We are trying to use a depth image for both depth testing (without writes) and sampling from the shader. We are doing that by ensuring the DEPTH_STENCIL_READ_ONLY_OPTIMAL layout, and simply by binding a descriptor set with the sampled image binding for the same depth used in this renderpass (with a single sub-pass). This appears to work on desktops just fine, and also a similar workload is permitted on Metal and D3D12 fwiw. No Vulkan validation layer complaints are seen either.

However, it came to my attention that the specification has a VU against that:

Image subresources used as attachments in the current render pass must not be accessed in any way other than as an attachment by this command

So I'm a bit confused about which side the bug is on: the spec, or our application and LunarG validation layers?

If we are doing it wrong, would there be a right way to do this? Maybe the depth binding has to be done via an "input attachment", in which case the depth would be in two different attachments?

question

Most helpful comment

@NicolBolas Assumably, if both attachment and non-attachment accesses are read-only, then nothing prevents it? The only problem here are Subpass Layout Transitions, which technically are a potential write access. But if the layout stays the same the whole Render Pass, then it should theoretically be ok.

All 11 comments

Incidentally this came up from a different source recently, and there's an internal issue opened and under active discussion about it. The spec does forbid it currently, but we're investigating whether we can state specific conditions under which it can be relaxed.

Doing this as a depth attachment is allowed and expected to work. For some applications that works well, for others (especially API-on-API translation, depending on the source API) it can be pretty cumbersome.

@critsec

Wait: how is this "expected to work"? In a TBR for example, the depth buffer is just tile data, right? The texels outside of a tile may not even exist yet. Subpasses aren't supposed to force a full dump of TBR data to memory; that's what ending render passes is for.

It seems to me that what is "expected to work" is to be able to use the depth attachment as an input attachment. I mean, that's why input attachments were invented, right? To allow a fragment shader to read from the texel that was written by a previous subpass without having to break up subpasses.

Isn't the point of a render pass to specify an abstraction under which a TBR can (hypothetically) preserve attached images within tile data without paging anything out to main memory? That is, they could hypothetically execute all of the rendering commands for one tile, then execute them all for another, etc, without having any one tile wait on the data from another tile. And thus, all operations that would make that impossible (like reading arbitrarily from attached images) is forbidden within a render pass.

@NicolBolas Assumably, if both attachment and non-attachment accesses are read-only, then nothing prevents it? The only problem here are Subpass Layout Transitions, which technically are a potential write access. But if the layout stays the same the whole Render Pass, then it should theoretically be ok.

@krOoze

Assumably, if both attachment and non-attachment accesses are read-only, then nothing prevents it?

How can you read data from tiles that haven't executed yet? Remember: the render pass architecture is intended to allow rasterization generated by a later subpass to execute even though rasterization from a previous subpass hasn't entirely finished yet, so long as what hasn't finished is from distinct tiles. That is, an implementation of a renderpass ought to be able to do all of the geometry stuff for basically the entire render pass, then, for each tile, execute all of the rasterization on that tile. Hypothetically, at least; individual hardware may or may not be able to handle this for every workload.

This allows all attachment data to (again hypothetically) remain within a tile and only be pushed to GPU memory once the render pass is over. This is why input attachments have the specific limitation of only ever fetching from the fragment's current position: that position is (again hypothetically) in a tile's data and not outside of the tile somewhere.

If you can read from data generated by a different tile, then you can't do this. So if you're going to allow it, you need some kind of hard barrier built into the render pass architecture that says "all tiles must have finished rasterization up to this point, and all rasterization products must be in actual GPU accessible memory at this point in time". I don't believe any form of dependency in the current render pass system specifies this.

Well, except for the obvious: ending the render pass. In fact, that's what "ending the render pass" is for. That's why you can't do everything within a render pass.

Now yes, for a non-TBR renderer, you might be able to allow it. But if you're going to break the render pass system into what can work for TBRs and what can't, it'd be better to just make render passes optional (or more specifically, allow TBRs to make them required) and just allow non-TBR hardware to work without them and their limitations.

How can you read data from tiles that haven't executed yet?

You can if the depth attachment is readonly (without subpass transitions) for the duration of the render pass: usage as an attachment will load in tile memory, and usage a sampled texture will fetch the same data again from VRAM. I believe that's why @critsec said:

The spec does forbid it currently, but we're investigating whether we can state specific conditions under which it can be relaxed.

@kvark was asking about this in the context of WebGPU's depthReadOnly on the render pass descriptor (note that WebGPU render passes have only a single subpass). depthReadOnly is important for many things, for example to do local ray-tracing in the depth buffer for contact shadows (sampling path) while at the same time using the same depth buffer for occluding the light volumes (attachment path).

@NicolBolas Because the original non-tiled VkImage still exists (or is the same thing in case of non-tiled architectures). Problem with render pass attachments is that you have potentially two copies, which need to be kept consistent and which can pollute each other as they load and store between themselves. That's why attachment and non-attachment use at the same time is forbidden. But if both are kept the same (read-only access in both attachment and non-attachment use), then maintaining consistency is not a problem, and it could theoretically be allowed.

You can if the depth attachment is readonly (without subpass transitions) for the duration of the render pass: usage as an attachment will load in tile memory, and usage a sampled texture will fetch the same data again from VRAM.

OK, that's fair. But if you're going to do it, it has to be a property of the render pass attachment itself, not a transitory layout property. Otherwise, every transition into this layout will provoke tiles to fully resolve and store their data to the image's memory. And since people are already using this layout (for depth test and input attachment uses), that could be bad for their performance.

Overall, it shouldn't be something invisible; it's got to be something that's part of Vulkan itself. You have to ask for it, and VU rules should be able to exist that you've specified it incorrectly. It could be an attachment description flag: "VK_ATTACHMENT_DESCRIPTION_IS_IMMUTABLE_BIT" or somesuch.

@NicolBolas IDK, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL and VK_IMAGE_LAYOUT_DEPTH_READ_ONLY_OPTIMAL are already allowed in both attachments and non-attachments. Only thing that remains is to allow it to be used in both at the same time for the same aspect? What is the perceived performance problem here? There would be no transitions allowed, as transition is a write access. So no premature pass flushes needed.

VU rules should be able to exist that you've specified it incorrectly

That's perhaps a good idea though. Make a flag that prevents you to change layouts, and prevents to specify a clear op?

@krOoze

IDK, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL and VK_IMAGE_LAYOUT_DEPTH_READ_ONLY_OPTIMAL are already allowed in both attachments and non-attachments. Only thing that remains is to allow it to be used in both at the same time for the same aspect?

For depth attachments, it is logically possible to read from it as an attachment and as a shader value without writing to it. You can do depth tests without doing depth writes.

But there is no equivalent read-only operation for color attachments. Yes, you can have pipeline mask off writes to the attachment, but if you do that, then you're not doing anything with the attachment, reading or writing. Since there's no read-only operation for color attachments, it doesn't make sense for immutable attachments to be able to be used as color attachments.

That's perhaps a good idea though. Make a flag that prevents you to change layouts, and prevents to specify a clear op?

That's why I suggested making it an attachment description. But layout transitions wouldn't be altogether forbidden; layout transitions other than the first and last would be forbidden.

The main thing the immutability switch does is ensure that the state of the image data when it is loaded into a tile cannot differ from the state of the image data in GPU-accessible memory. So it is conceptually OK to transition into some layout for the duration of the render pass (the first subpass that uses the attachment can have a different layout from initialLayout), and you can transition to some layout after the render pass (the last subpass that uses the attachment can have a different layout from finalLayout), but every use of the attachment within the render pass must have the same layout.

So if we were to specify a more complete list of VUs, if VkAttachmentDescription::flags for an attachment includes VK_ATTACHMENT_DESCRIPTION_IS_IMMUTABLE_BIT:

  • VkAttachmentDescription::loadOp must be VK_ATTACHMENT_LOAD_OP_LOAD. (neither clearing nor discarding makes sense if the attachment is immutable).
  • All VkAttachmentReference::layouts which reference an immutable attachment must use the same layout as all other VkAttachmentReferences.
  • The only valid VkAttachmentReference::layouts are the READ_ONLY depth/stencil layouts.

But there is no equivalent read-only operation for color attachments.

Yes, that is why I avoided talking about that one. The Input Attachment seems more obvious case.

Though to paraphraze B. Stroustrup, you know you have a great language if it is used in ways that surprise you. To play the devil's advocate, there might be sneaky ways the color attachment is read. Extensions might change something, so it matters. pResolveAttachments require the color attachment and read it. There might be some conditional rendering going on, that formally requires two color attachments, but writes to only one. And what if you write the R channel, but B channel is read-only? Blending technically reads the color attachment (and could read all channels, but write only some channels). You never know how will it be used...

Also for better or worse, people like null and dummy parameters in the API. It alows them to duct-tape stuff together, even if it does not necessarily make sense from performance perspective.

layout transitions other than the first and last would be forbidden.

Not sure that would work. I think an obvious optimization is to make the initial layout transition be a fused operation with the load op. That means the original image in the general purpose memory might remain untransitioned, and so you have coherency problem.

Equally, the final layout transition would not work. Keep in mind the tiler streams tile-by-tile when doing store op. This means you could be storing (writing) a transitioned image data, while the same coordinates still may be sampled later (in non-attachment use) in a different tile, corrupting the results.

This seems to have been fixed in https://github.com/KhronosGroup/Vulkan-Docs/commit/f90136facacd25f016e523064f03713bdfe1b22d which is Vulkan 1.X.160. Can we close the issue?

Was this page helpful?
0 / 5 - 0 ratings