metal: match pipeline attachment formats to the view#635
Open
benface wants to merge 1 commit into
Open
Conversation
5615845 to
7ea7ce8
Compare
Owner
|
While it does make sense that the view surface doesn't have stencil and the depth, setting the format into MTLPixelFormatInvalid feels wrong to me. Maybe we can just remove setDepthAttachmentPixelFormat and setStencilAttachmentPixelFormat calls for MTKView? |
The Metal backend unconditionally configured every render pipeline
state for two color attachments (`for i in 0..2`) plus depth and
stencil at `Depth32Float_Stencil8`, while MTKView by default exposes
only color attachment 0 and `depthStencilPixelFormat = Invalid`. Metal
validation fires on the first draw:
failed assertion `Set Render Pipeline State Validation
For color attachment 1, the renderPipelineState pixelFormat must
be MTLPixelFormatInvalid, as no texture is set.
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.
Configure only color attachment 0 (the backend has no MRT path) and
mirror the view's `depthStencilPixelFormat` onto the pipeline's depth
+ stencil formats — but only when the view actually has one. When
MTKView reports `Invalid`, skip the `set*AttachmentPixelFormat:` calls
so the descriptor keeps its `Invalid` defaults instead of having that
sentinel passed in explicitly.
Adds `MTLPixelFormat::Invalid = 0` to the binding (Apple's `MTLPixelFormatInvalid`).
Contributor
Author
|
Good catch — pushed a v2: instead of passing |
7ea7ce8 to
c251bbb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The Metal backend unconditionally configures every render pipeline state for two color attachments (
for i in 0..2) plus depth + stencil atDepth32Float_Stencil8. MTKView by default exposes only color attachment 0 anddepthStencilPixelFormat = MTLPixelFormatInvalid. With Metal validation on, the first draw asserts:Hit this trying to switch a 2D macroquad app to the Metal backend on an iPhone 17 simulator (iOS 27 beta).
Fix
Two changes in
graphics/metal.rs::new_pipeline:for i in 0..1) — miniquad's Metal backend doesn't expose multiple render targets, so attachment 1 should remainInvalid.depthStencilPixelFormatand pass it through to the pipeline's depth + stencil attachment formats —MTLPixelFormatInvalidwhen MTKView has no depth/stencil, which is the default.After this, the pipeline state's attachment formats match whatever the encoder actually has bound, and Metal validation accepts every draw.
Tested on
iPhone 17 simulator on iOS 27 beta — Metal validation crash gone, app launches and renders into the simulator.