Skip to content

2 subpass use same depth_stencil attachment as READ_ONLY #948

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

Closed
liufachen opened this issue Jun 2, 2019 · 6 comments
Closed

2 subpass use same depth_stencil attachment as READ_ONLY #948

liufachen opened this issue Jun 2, 2019 · 6 comments
Assignees
Labels
Bug Something isn't working
Milestone

Comments

@liufachen
Copy link

liufachen commented Jun 2, 2019

My vulkan version is 1.1.106.0

The problem is : subpass “1” use color attachment A and subpass “2” use color attachment B, both use the same depth_stencil attachment as VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL.
The validation say:“A dependency between subpasses 1 and 2 must exist but one is not specified”, but Doc say:“If two subpasses use the same attachment in different layouts, and both layouts are read-only, no subpass dependency needs to be specified between those subpasses.”.

My problem is what mistake i made so i need provide a subpass-dependendcy.

I appreciate your time and help, i fought this problem almost 2 days, and hope someone could give me a help and thanks again.

and more information please refer: https://community.khronos.org/t/2-subpass-use-same-depth-stencil-attachment-as-read-only/104058/5

@tobine
Copy link
Contributor

tobine commented Jun 5, 2019

I believe there's a bug in the layers related to this. Specifically in subpass dependency checking we definitely don't account for the case quoted from the spec:

If two subpasses use the same attachment in different layouts, and both layouts are read-only, no subpass dependency needs to be specified between those subpasses.

For this specific bug the attachment is not "in different layouts" it's in the same layout. However, I think this may be a spec issue as I don't believe there should be a subpass dependency if both subpasses access the attachment in read-only layouts regardless of whether the layouts are the same or not.

@liufachen
Copy link
Author

I appreciate for your help and hope this issue will be closed in the feature.

Thank you tobine.

@jzulauf-lunarg
Copy link
Contributor

jzulauf-lunarg commented Jul 18, 2019

@tobine -- did you open spec issue (or propose an MR) for the suspected spec bug? It always makes me nervous to get Validation ahead of the spec in this sense, without at least some grunts of affirmation out of the WG. (not that I disagree, if different read-only layouts don't require dependencies**, then read-only attachments with the same layout certainly don't seem like they should).

@mikes-lunarg

** It strikes me that the implementation has got to be potentially inserting some additional dependencies for the cases were the logical image layouts require actual image layout transitions... but it looks like the spec authors were willing to sign up to that implementation complexity.

@jzulauf-lunarg
Copy link
Contributor

jzulauf-lunarg commented Jul 18, 2019

@tobine -- does the following give enough cover to say that read-only attachments (input or read only depth stencil) don't require subpass dependency w.r.t. other read-only references to the same attachment? Then the "different layouts" section of the spec is then just further clarification that even image layout transitions between read only formats (and image layout transitions can write) don't require them either and not a restriction for only "different layouts".

Care should be taken to avoid a data race here; if any subpasses access attachments with overlapping memory locations, and one of those accesses is a write, a subpass dependency needs to be included between them.

@tobine
Copy link
Contributor

tobine commented Jul 18, 2019

@jzulauf-lunarg That helps but doesn't seem conclusive. I submitted a spec edit and tagged you on it to clarify that no subpass dependency is required between subpasses using the same attachment in any read-only layouts (regardless of if the layouts are the same or not)

oddhack added a commit to KhronosGroup/Vulkan-Docs that referenced this issue Jul 29, 2019
  * Update release number to 117.

Github Issues:

  * Add ename:VK_STENCIL_FACE_FRONT_AND_BACK for naming consistency, and
    alias the old ename:VK_STENCIL_FRONT_AND_BACK for backwards
    compatibility (public issue 991).
  * Fix minor issues with valid usage statements for
    flink:vkCreateFramebuffer, slink:VkFramebufferCreateInfo, and
    slink:VkRenderPassBeginInfo when the `<<VK_KHR_imageless_framebuffer>>`
    extension is enabled (public issue 998).
  * Clarify the subpass dependency requirement in the
    <<renderpass-layout-transitions>> section to eliminate the need for a
    subpass dependency for either the same or different layouts as long as
    they're both read-only (relates to
    KhronosGroup/Vulkan-ValidationLayers#948).

Internal Issues:

  * Document that <<extendingvulkan-compatibility-promotion, backwards
    compatibility aliases are not promoted>> as part of promoting an
    extension (internal issue 1677).
  * Update VK_ANDROID_native_buffer extension to spec version 8 (internal
    issue 1753).
  * Add missing section to the <<VK_KHR_shader_controls_v4_incompatibility,
    VK_KHR_shader_float_controls>> extension appendix describing
    the reason for the breaking API change in version 4 of the extension,
    and correct the version to 4 in `vk.xml` (internal merge request
    3275).
  * Add valid usage statements to slink:VkAccelerationStructureInfoNV
    requiring the ename:VK_BUFFER_USAGE_RAY_TRACING_BIT_NV usage flag for
    buffers used in acceleration structure building.

New Extensions:

  * `<<VK_EXT_line_rasterization>>`
  * `<<VK_EXT_texture_compression_astc_hdr>>`
  * `<<VK_EXT_index_type_uint8>>`
@oddhack
Copy link

oddhack commented Jul 29, 2019

The spec part of this should be addressed in the 1.1.117 spec update. Please close if that's all that needs to be done for this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants