Emulate reversed-depth viewports on affected AMD Mac GPUs#2760
Emulate reversed-depth viewports on affected AMD Mac GPUs#2760vkedwardli wants to merge 1 commit into
Conversation
|
Here's a repro used to isolate the issue outside Flycast reversed-depth-repro.zip The test renders overlapping quads using reversed-depth viewport state:
On Radeon Pro 560 through MoltenVK/Metal 2, the failing build shows that reversed Metal viewport depth ( Original failing result on Radeon Pro 560After this PR, the same repro passes the reversed-depth cases by normalizing the Metal viewport depth range and emulating the reversed depth transform in the generated vertex/tessellation-evaluation MSL. Passing result with this PR on Radeon Pro 560 |
| return !positionMemberName.empty(); | ||
| } | ||
|
|
||
| static bool mvkPatchMSLEmulatedReversedDepthViewport(std::string& msl, uint32_t emulatedReversedDepthViewportBufferIndex) { |
There was a problem hiding this comment.
Generally these kinds of things are implemented into SPIRV-Cross, which we then set the configuration for, rather than patching MSL manually, as it seems this could be rather dicey.
There was a problem hiding this comment.
Agreed. I do not think the MSL text patching is the ideal final shape either, I kept it local in this PR mainly to validate the workaround direction. If this Metal2 bug should not be handled by MoltenVK, then we could save time on creating + reviewing cross repo PR.
There was a problem hiding this comment.
If it's indeed broken and there's no simpler workaround, shader handling would make sense I suppose. As far as I know this is a valid thing to do which should work.
There was a problem hiding this comment.
Great, I will move the patch to SPIRV-Cross later.
The current minimal workaround is:
- pass Metal a normal/forward viewport depth range by swapping
znear/zfar - invert clip-space Z in vertex/tessellation-evaluation shaders
- keep the rest of the reversed-Z state unchanged:
- Vulkan viewport semantics from the app’s point of view
- depth compare op, e.g.
GREATERremainsGREATER - depth clear value, e.g. reversed-Z clear
0remains0 - depth test enable/write enable
- depth attachment format
- render pass/load/store behavior
- depth attachment contents/final depth values, logically matching the original reversed viewport
- fragment depth behavior, including
gl_FragCoord.z/gl_FragDepth - renderer draw ordering and OIT logic
| mtlViewports[i].znear = shouldEmulateReversedDepthViewports && isReversedDepthViewport ? 0.0 : viewports[i].minDepth; | ||
| mtlViewports[i].zfar = shouldEmulateReversedDepthViewports && isReversedDepthViewport ? 1.0 : viewports[i].maxDepth; |
There was a problem hiding this comment.
Should be viewports[i].maxDepth : viewports[i].minDepth and viewports[i].minDepth : viewports[i].maxDepth in case you are wondering. but keeping 0.0 / 1.0 seems easier to read for a draft
Avoid passing reversed depth ranges directly to Metal on AMD Mac2 GPUs that do not support Metal 3. Normalize reversed Metal viewports to 0..1, pass a hidden per-draw flag to vertex/tessellation-evaluation shaders, and patch generated MSL to invert clip-space Z dynamically. Disable discarded fragment store checking on the affected path to avoid the helper-thread behavior that breaks OIT rendering.
This PR works around a Metal depth viewport issue seen on AMD Mac2 GPUs, such as Radeon Pro 560
flyinghead/flycast#1172
#955
A working PoC for using non reversed-Z viewport
flyinghead/flycast#2298
On these GPUs, passing a reversed-depth Metal viewport (znear > zfar) can cause depth testing/writes to behave incorrectly. Instead of passing the reversed depth range directly to Metal, MoltenVK now detects the affected hardware path and emulates reversed-depth viewports dynamically.
I acknowledge this appears to be a Metal 2 driver/API issue rather than MoltenVK’s responsibility. I am opening this PR to seek maintainer insight on whether this kind of workaround belongs in MoltenVK, and I tried to keep the implementation as narrowly scoped and least invasive as possible.
znear = 0,zfar = 1.The current SPIR-V/MSL conversion changes are kept local to this PR for discussion. Once the direction is clear, those changes can and probably should be moved into SPIRV-Cross, where the transformation can be implemented with shader semantic context instead of MoltenVK-side MSL text patching.
This is intentionally scoped to the affected hardware path and does not add a public config option.
Known limitation:
Mixed multi-viewport draws where some viewports use reversed depth and others do not, are not fully emulated by this MoltenVK-side patch. Full per-viewport emulation would be better implemented in SPIRV-Cross, where viewport-index output semantics are available during MSL generation.