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

Vulkan: Add workarounds for MoltenVK #4202

Merged
merged 37 commits into from Jan 13, 2023
Merged

Conversation

riperiperi
Copy link
Member

@riperiperi riperiperi commented Jan 3, 2023

This PR does the following things:

  • Adds MoltenVK configuration class and provides helpful configurations without manually specifying environment variables.
  • Portability subset workarounds for MoltenVK, based on properties/features:
    • Enables vertex attribute restriding (always 4 byte for now)
    • Enables triangle fan retopology conversion.
    • Enables 3D texture view workaround.
  • Add shader specialization for output attribute types (float/int/uint)
    • This should also prevent validation errors relating to this, though all desktop GPUs were OK with it before.
    • This has been rewritten compared to what is in the macos build so it needs some testing.
  • Adds special treatement for duplicate render attachments when the GPU is detected as TBDR.
    • Fixes some graphical issues in Mario Kart 8 and Zelda BOTW. The error is caused by two copies of the render target existing in tiled memory, where one that was not written may take priority and erase any changes made during the render pass. Generally when this happens, only one render target is enabled/written, so that target is selected and the other is masked out.
  • Add alternate versions of the clear shader for int and uint type targets.
  • Disable null descriptors if they are not available, but Secretly Use Them Anyways on MoltenVK as a Funny Joke.
    • In all seriousness, using null descriptors avoids issues with stale resource usage.
    • Rosetta doesn't seem to care. Nobody is allowed to gaslight me by saying rosetta doesn't work without this.
  • Actually check for individual ASTC formats before saying it is supported. MoltenVK on Rosetta supports the extension, but not the individual formats.
  • Minor additional changes/reordering.

This PR does not include other things mentioned on #4062, such as transform feedback or geometry shader emulation. It also doesn't include A64 JIT, hypervisor or any related workarounds, so things will only really work under Rosetta. The performance enhancement "buffer mirrors" also is not present, which may negatively impact some games. This may be lessened by recent changes to reduce inline updates.

One important thing this doesn't include is any sort of workaround for resource usage errors on MoltenVK. These issues are better explained on #4062, but I really have not had time to make some kind of reproduction for the issue to report it to them. I'd rather we didn't have to add some weird resource tracking to our descriptor set updater just for MoltenVK.

@riperiperi riperiperi added the gpu Related to Ryujinx.Graphics label Jan 3, 2023
@riperiperi
Copy link
Member Author

Note: there seems to be an issue with the attachment format specialization, though it happened randomly after like 4 races in Mario Kart 8 Deluxe. I'm looking into it.

[mvk-error] VK_ERROR_INITIALIZATION_FAILED: Render pipeline compile failed (Error code 3):
output of type uint4 is not compatible with a MTLPixelFormatRG11B10Float color attachement..

@riperiperi riperiperi mentioned this pull request Jan 3, 2023
16 tasks
Ryujinx.Graphics.Vulkan/HardwareCapabilities.cs Outdated Show resolved Hide resolved
Ryujinx.Graphics.Vulkan/HelperShader.cs Show resolved Hide resolved
Ryujinx.Graphics.Vulkan/MoltenVK/MVKConfiguration.cs Outdated Show resolved Hide resolved
Ryujinx.Graphics.Vulkan/Vendor.cs Outdated Show resolved Hide resolved
@riperiperi riperiperi marked this pull request as ready for review January 12, 2023 19:15
@riperiperi
Copy link
Member Author

Screenshot 2023-01-12 at 19 16 39

Ready for review.

Ryujinx.Graphics.Shader/IGpuAccessor.cs Show resolved Hide resolved
Ryujinx.Graphics.Vulkan/FramebufferParams.cs Show resolved Hide resolved
Ryujinx.Graphics.Vulkan/VertexBufferState.cs Show resolved Hide resolved
Ryujinx.Graphics.Vulkan/VulkanRenderer.cs Show resolved Hide resolved
@@ -515,12 +590,13 @@ private unsafe void PrintGpuInformation()

IsAmdWindows = Vendor == Vendor.Amd && RuntimeInformation.IsOSPlatform(OSPlatform.Windows);
IsIntelWindows = Vendor == Vendor.Intel && RuntimeInformation.IsOSPlatform(OSPlatform.Windows);
IsTBDR = IsMoltenVk || Vendor == Vendor.Qualcomm;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That doesn't look right when desktop GPUs will also be included here (AMD, there's also integrated Intel which I think it's also not TBDR)? Also should we include ARM (Mali) GPUs?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how Intel/AMD GPUs on macOS will behave in this situation, so the workaround was enabled there too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I do think other TBDR gpus should be included here, as they will definitely have the issue.

@@ -434,7 +507,9 @@ public unsafe Capabilities GetCapabilities()
GpuVendor,
hasFrontFacingBug: IsIntelWindows,
hasVectorIndexingBug: Vendor == Vendor.Qualcomm,
supportsAstcCompression: features2.Features.TextureCompressionAstcLdr,
needsFragmentOutputSpecialization: IsMoltenVk,
reduceShaderPrecision: IsMoltenVk,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should include all mobile GPUs here too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is enabled because of a weird behaviour with SPIRV-Cross that makes precise much slower. I'm not sure how much effect there will be on other GPUs from this, but it probably won't be anywhere near as extreme.

Copy link
Member

@gdkchan gdkchan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks.

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

Successfully merging this pull request may close these issues.

None yet

5 participants