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 in dynamic rendering with depth attachment and no stencil attachment, but depth-stencil format #2412

Open
squidbus opened this issue Dec 14, 2024 · 1 comment

Comments

@squidbus
Copy link
Contributor

In my code, I have pipelines and dynamic rendering passes where a depth attachment is provided with depth-stencil format, and the stencil attachment is set to nullptr/UNDEFINED, which as far as I can tell is perfectly valid and triggers no Vulkan validation errors:

https://github.com/shadps4-emu/shadPS4/blob/876445faf1b0ef63ddb9d0111e35b74bd31b4a42/src/video_core/renderer_vulkan/vk_scheduler.cpp#L49-L50

https://github.com/shadps4-emu/shadPS4/blob/876445faf1b0ef63ddb9d0111e35b74bd31b4a42/src/video_core/renderer_vulkan/vk_graphics_pipeline.cpp#L256-L257

In a local branch with some other state fixes, I've checked that these are in sync and that when we pass UNDEFINED as as the stencil format, we pass nullptr as the stencil attachment.

However, if our depth attachment happens to be a depth-stencil format, we get the following validation error from Metal:

-[MTLDebugRenderCommandEncoder setRenderPipelineState:]:1616: failed assertion `Set Render Pipeline State Validation
For stencil attachment, the render pipeline's pixelFormat (MTLPixelFormatInvalid) does not match the framebuffer's pixelFormat (MTLPixelFormatDepth32Float_Stencil8).
'

As far as I can tell this is because of this code:

// If the depth/stencil attachment is not in use, but the alternate stencil/depth attachment is,
// and the MTLPixelFormat is usable by both attachments, force the use of the alternate attachment
// for both attachments, to avoid Metal validation errors when a pipeline expects both depth and
// stencil, but only one of the attachments has been provided here.
// Check the MTLPixelFormat of the MVKImage underlying the MVKImageView, to bypass possible
// substitution of MTLPixelFormat in the MVKImageView due to swizzling, or stencil-only access.
const VkRenderingAttachmentInfo* MVKRenderingAttachmentIterator::getAttachmentInfo(const VkRenderingAttachmentInfo* pAtt,
const VkRenderingAttachmentInfo* pAltAtt,
bool isStencil) {
bool useAlt = false;
if ( !(pAtt && pAtt->imageView) && (pAltAtt && pAltAtt->imageView) ) {
MVKImage* mvkImg = ((MVKImageView*)pAltAtt->imageView)->getImage();
useAlt = (isStencil
? mvkImg->getPixelFormats()->isStencilFormat(mvkImg->getMTLPixelFormat())
: mvkImg->getPixelFormats()->isDepthFormat(mvkImg->getMTLPixelFormat()));
}
return useAlt ? pAltAtt : pAtt;
}

My depth attachment is being forced into the stencil attachment due to its format, but since I didn't ask for a stencil attachment and provided UNDEFINED format it is raising an error.

@squidbus
Copy link
Contributor Author

squidbus commented Dec 15, 2024

Note that this is a similar problem space to #2337, which I previously reported, in that it is a problem with how MoltenVK's handling of dynamic rendering attachments does not seem to always line up with Metal expectations.

However in this case the issue is that I'm actually doing what Metal prefers (stencil = nullptr and stencilFormat = UNDEFINED match each other) and MoltenVK is forcing it to be otherwise, where as in that case it was behavior that Vulkan allows but Metal does not (Vulkan allows defined format in pipeline with null dynamic rendering attachment, Metal expects them to always be matching with null attachment being invalid format).

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

No branches or pull requests

1 participant