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

Potential macOS Metal & vsync improvements #309

Merged

Conversation

ryanmcgrath
Copy link
Collaborator

Reopening this PR given that issues with the Launcher being the culprit were identified and have potential fixes in the pipeline.

Definitely open to more testing, but wanted to open for discussion if nothing else.

This does a few things, detailed below. Tested on the following devices:

2020 M1 MBP, macOS 11.3
2015 Intel i5 MBP, macOS 10.15.7
2020 M1 MBP (8GB), macOS 11.4
2015 MBP, macOS 10.14.6
27inch 5k Retina iMac running 10.14.6, didn't grab year from user
2019 MBP, macOS 10.15.7, didn't grab year from user

On these devices, these patches generally deliver smoother performance with less screen tearing when rendering with Metal. This doesn't fix the odd stuttering for every 8GB M1 Pro, but it has resulted in improved performance for some of them - no idea past this on whether it's hardware or what.

  • Injects a custom CAMetalLayer NSView subclass, which opts in to layer rendering and cuts out the cases where wxWidgets subclass drawRect happens (notably fixes the initial stutter for some users, who reported having it on game start).
  • Explicitly sets vsync to be disabled on the Metal layer before MoltenVK gets ahold of it. For whatever reason, this results in vsync actually being turned off - expecting it to happen on the Vulkan/MoltenVK layer does not appear to work (to be clear/fair: looking through the MoltenVK source, I don't think this is their bug).
  • Has a small change to the Vulkan renderer to cache the rendering layer, which is needed for handling resize and/or potential swapchain recreation. This is needed due to the layer being on a subview, not the main window it's rendering to. This is ifdefd explicitly for Apple/Metal and should not affect or change Vulkan on other platforms, but I of course will fix it if I'm wrong.
  • Enables an env variable (SLP_METAL_DOUBLE_BUFFER) which can be used to tell Metal to use double-buffer instead of triple-buffer behind the scenes. This is only really interesting for iMacs at the moment (Macbooks choke on it), hence why I didn't bother making a UI option for it - mostly want the ability to document it on the wiki and see how it fares for users. This would be possible on a mainline port as well, so I don't think it's going too far into left field either.
  • Opts in to disabling MVK_CONFIG_SYNCHRONOUS_QUEUE_SUBMITS, which we've documented on the wiki and most users who've tried it report it leading to less screen tearing under Vulkan. This setting helps here, but shouldn't be necessary in a mainline port.

Note that the only difference from the original PR is that MVK_CONFIG_SYNCHRONOUS_QUEUE_SUBMITS is ifdef'd out for Playback, as it exhibits a race condition on M1 Macs running under Rosetta 2.

I have zero clue at the moment why this works. MoltenVK sets this
property during initialization of the swapchain, and logging confirms
it's set, but setting the proprty ahead of time seems to actually
disable vsync and result in the expected rendering smoothness.

At worst, this change does nothing that wouldn't be done by MoltenVK
anyway. At best, it seems to provide much smoother render on a number
of tested macOS setups (Intel 2015 MBP/Mojave, Intel 2015 MBP/Catalina, 2020 M1 MBP/Big Sur).
The performance difference for Playback isn't so significant that this makes sense to check for Rosetta 2 and change accordingly.
@ryanmcgrath
Copy link
Collaborator Author

This has been updated to Slippi 2.3.3.

@NikhilNarayana NikhilNarayana merged commit d7d0dad into project-slippi:slippi Dec 31, 2021
vladfi1 pushed a commit to vladfi1/slippi-Ishiiruka that referenced this pull request Jan 1, 2022
* Force CAMetalLayer to disable vsync.

I have zero clue at the moment why this works. MoltenVK sets this
property during initialization of the swapchain, and logging confirms
it's set, but setting the proprty ahead of time seems to actually
disable vsync and result in the expected rendering smoothness.

At worst, this change does nothing that wouldn't be done by MoltenVK
anyway. At best, it seems to provide much smoother render on a number
of tested macOS setups (Intel 2015 MBP/Mojave, Intel 2015 MBP/Catalina, 2020 M1 MBP/Big Sur).

* Inject a custom CALayer rendering target to avoid wxWidgets drawRect

* ENV feature flag this

* Formatting

* Flag MoltenVK ENV variables on Playback

The performance difference for Playback isn't so significant that this makes sense to check for Rosetta 2 and change accordingly.

* Pin ffmpeg in CI due to upstream changes.

* Try 2.8 I guess.
jlambert360 added a commit to jlambert360/Ishiiruka that referenced this pull request Jan 29, 2022
jlambert360 added a commit to jlambert360/Ishiiruka that referenced this pull request Jan 30, 2022
jordan-zilch pushed a commit to jordan-zilch/Ishiiruka that referenced this pull request May 17, 2022
* Force CAMetalLayer to disable vsync.

I have zero clue at the moment why this works. MoltenVK sets this
property during initialization of the swapchain, and logging confirms
it's set, but setting the proprty ahead of time seems to actually
disable vsync and result in the expected rendering smoothness.

At worst, this change does nothing that wouldn't be done by MoltenVK
anyway. At best, it seems to provide much smoother render on a number
of tested macOS setups (Intel 2015 MBP/Mojave, Intel 2015 MBP/Catalina, 2020 M1 MBP/Big Sur).

* Inject a custom CALayer rendering target to avoid wxWidgets drawRect

* ENV feature flag this

* Formatting

* Flag MoltenVK ENV variables on Playback

The performance difference for Playback isn't so significant that this makes sense to check for Rosetta 2 and change accordingly.

* Pin ffmpeg in CI due to upstream changes.

* Try 2.8 I guess.
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

Successfully merging this pull request may close these issues.

2 participants