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

[3rdParty] Update MoltenVK to 1.3.236 & set MSL Fastmath to On Demand #13035

Merged
merged 6 commits into from
Dec 9, 2022

Conversation

shinra-electric
Copy link
Contributor

List of fixes and changes here.

Testing:
Check that the macOS CI build still works

@shinra-electric shinra-electric changed the title [3rdParty] Update MoltenVK to 1.2.236 [3rdParty] Update MoltenVK to 1.3.236 Dec 9, 2022
@shinra-electric
Copy link
Contributor Author

shinra-electric commented Dec 9, 2022

Error message:

/private/var/folders/xx/vl5f934s6k927z1vyyl9cxth0000gn/T/cirrus-ci-build/rpcs3/Emu/RSX/VK/vkutils/device.cpp:154:33: error: assigning to 'MVKConfigFastMath' from incompatible type 'bool'
                        mvk_config.fastMathEnabled = !(g_cfg.video.disable_msl_fast_math.get());
                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

According to MVK this option is an enum, not a bool:

/** Identifies when Metal shaders will be compiled with the fast math option. */
typedef enum MVKConfigFastMath {
	MVK_CONFIG_FAST_MATH_NEVER     = 0,  /**< Metal shaders will never be compiled with the fast math option. */
	MVK_CONFIG_FAST_MATH_ALWAYS    = 1,  /**< Metal shaders will always be compiled with the fast math option. */
	MVK_CONFIG_FAST_MATH_ON_DEMAND = 2,  /**< Metal shaders will be compiled with the fast math option, unless the shader includes execution modes that require it to be compiled without fast math. */
	MVK_CONFIG_FAST_MATH_MAX_ENUM  = 0x7FFFFFFF
} MVKConfigFastMath;

Not sure what would be preferable, but I think NEVER for off, and ON_DEMAND for on?
It seems to be better than ALWAYS

fastMathEnabled now has three options:
NEVER = 0
ALWAYS = 1 
ON_DEMAND = 2

On demand seems better, since it will use fast math except for shaders that are incompatible.
@shinra-electric shinra-electric changed the title [3rdParty] Update MoltenVK to 1.3.236 [WIP] [3rdParty] Update MoltenVK to 1.3.236 Dec 9, 2022
@shinra-electric shinra-electric changed the title [WIP] [3rdParty] Update MoltenVK to 1.3.236 [3rdParty] Update MoltenVK to 1.3.236 & set MSL Fastmath to On Demand Dec 9, 2022
@shinra-electric
Copy link
Contributor Author

This is working now.

The GUI option is Off and On Demand. Having Always as an option seems a bit pointless.

However, I'm wondering if using On Demand by default makes the settings option useless? Should it just be removed from the GUI?

Or alternatively we could change the option from an on/off toggle to a dropdown with all three options?

Feedback appreciated.

@Megamouse Megamouse requested a review from kd-11 December 9, 2022 15:41
@Megamouse Megamouse added OS: macOS Driver: MoltenVK Open-source macOS Vulkan driver labels Dec 9, 2022
kd-11
kd-11 previously approved these changes Dec 9, 2022
@kd-11
Copy link
Contributor

kd-11 commented Dec 9, 2022

This is working now.

The GUI option is Off and On Demand. Having Always as an option seems a bit pointless.

However, I'm wondering if using On Demand by default makes the settings option useless? Should it just be removed from the GUI?

Or alternatively we could change the option from an on/off toggle to a dropdown with all three options?

Feedback appreciated.

I haven't read through the moltenVK code to figure out why this option is present. As for usefulness, its pretty much pointless as we have a global "shader quality" config as well that does a similar thing.

@Megamouse Megamouse merged commit 809e880 into RPCS3:master Dec 9, 2022
@shinra-electric shinra-electric deleted the update_MVK branch December 9, 2022 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Driver: MoltenVK Open-source macOS Vulkan driver OS: macOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants