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

[ios18 support] add new case for Metal Version 3.2 that is on ios 18 -- reroutes MoltenMK to use version 3.1 #2255

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Daij-Djan
Copy link

This adds a new preprocessor macro for xcode16 / ios18 sdk and reroutes the version to 3.1 (additions to MVKDevice are iline with as what we did for other xcode versions in the past)

@CLAassistant
Copy link

CLAassistant commented Jun 14, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@billhollings billhollings left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of this, and for submitting this PR!

reroutes the version to 3.1
...
// We don't explicitly support Metal 3.2 yet

What do you mean by this? Is there a reason why you are not using MTLLanguageVersion3_2? The MVK_XCODE_16 and mvkOSVersionIsAtLeast() guard code should protect against inappropriate build or runtime usage. Have you tried building or running, and are experiencing problems?

In addition, can you also add the equivalent changes to OSSupport.mm?

Thanks.

@Daij-Djan
Copy link
Author

I didnt wanna make any logic changes so I just made it 3.1 -- I didnt test runtime behaviour of moltenVK when I did the change.

@billhollings
Copy link
Contributor

Change it to MTLLanguageVersion3_2 and test whether you can build using Xcode 15, to make sure it doesn't cause a regression break. If that is successful, we should be good.

If you have Xcode 16 up and running, test a build under that as well. If not, it should be good enough to go ahead with MTLLanguageVersion3_2 anyway.

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.

None yet

3 participants