Skip to content
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

Metal validation error when VkPipelineRenderingCreateInfoKHR format is set without attachment #2337

Open
squidbus opened this issue Sep 15, 2024 · 4 comments

Comments

@squidbus
Copy link
Contributor

If you set the formats in a VkPipelineRenderingCreateInfoKHR but don't actually supply a corresponding attachment, you can get the Metal validation errors like the following:

Set Render Pipeline State Validation
For depth attachment, the renderPipelineState pixelFormat must be MTLPixelFormatInvalid, as no texture is set.
For stencil attachment, the renderPipelineState pixelFormat must be MTLPixelFormatInvalid, as no texture is set.

The Vulkan spec states that it is valid to set these to a valid format while still supplying null attachments:

If depthAttachmentFormat, stencilAttachmentFormat, or any element of pColorAttachmentFormats is
VK_FORMAT_UNDEFINED, it indicates that the corresponding attachment is unused within the render pass.
Valid formats indicate that an attachment can be used - but it is still valid to set the attachment to NULL
when beginning rendering.

Based on this it seems to me that MoltenVK should be handling this case since it is within spec, supplying what Metal wants to see to avoid errors.

@billhollings
Copy link
Contributor

but it is still valid to set the attachment to NULL when beginning rendering

How are you accomplishing this?

The pAttachments array in both VkFramebufferCreateInfo and VkRenderPassAttachmentBeginInfo must contain valid VkImageView handles.

On the other hand, MoltenVK correctly handles the use of VK_ATTACHMENT_UNUSED to disable use of the attachment in a render pass or dynamic rendering pass.

You can verify this in the Cube demo included with MoltenVK, by making the following modification in cube.c:

const VkAttachmentReference depth_reference = {
//  .attachment = 1,
    .attachment = VK_ATTACHMENT_UNUSED,
    .layout = VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL,
};

@squidbus
Copy link
Contributor Author

squidbus commented Sep 16, 2024

How are you accomplishing this?

We are using dynamic rendering, the VkRenderingInfo looks like this:

    const vk::RenderingInfo rendering_info = {
        .renderArea =
            {
                .offset = {0, 0},
                .extent = {witdh, height},
            },
        .layerCount = 1,
        .colorAttachmentCount = render_state.num_color_attachments,
        .pColorAttachments = render_state.num_color_attachments > 0
                                 ? render_state.color_attachments.data()
                                 : nullptr,
        .pDepthAttachment = render_state.has_depth ? &render_state.depth_attachment : nullptr,
        .pStencilAttachment = render_state.has_stencil ? &render_state.depth_attachment : nullptr,
    };

According to spec nulls are allowed here: https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkRenderingInfoKHR.html You can see in the validation error conditions that they reference multiple times that the attachment parameters can be NULL. And this code triggers no Vulkan validation errors.

@billhollings
Copy link
Contributor

Ah. I see now. Thanks for the further background.

Odd that Vulkan allows NULL image views only for dynamic rendering.

Hmmm...looks like we'll have to build a workaround, either recompiling and caching the pipeline on the fly (might be glitch-potential), or allocating and caching memoryless temporary textures during the render pass.

To help get this right, can you provide some more context on your use case for this capability, please? Within your app, under what use cases would you want to provide NULL attachments to a pipeline that is expecting to use them?

@squidbus
Copy link
Contributor Author

squidbus commented Sep 17, 2024

I don't think the attachment is expected to actually be used when this triggers, rather I think its an optimization to compile fewer pipelines where the format compiled in is based on the minimal needed state, and the actual attachment has additional checks on whether there is actually an attachment to apply or not. This is for an emulator implementing the interface of another GPU.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants