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

Support MoltenVK #1623

Merged
merged 9 commits into from
Sep 9, 2023
Merged

Support MoltenVK #1623

merged 9 commits into from
Sep 9, 2023

Conversation

zvasya
Copy link
Contributor

@zvasya zvasya commented Aug 24, 2023

Fixed loading of the "__Internal" library for iOS, iOS Simulator and MacCatalyst using the GetMainProgramHandle method from Net7.0.
And add package with MoltenVK libraries

@zvasya zvasya changed the title start of Support MoltenVK Support MoltenVK Aug 24, 2023
@zvasya zvasya marked this pull request as ready for review August 24, 2023 07:24
@zvasya zvasya requested a review from a team as a code owner August 24, 2023 07:24
@zvasya
Copy link
Contributor Author

zvasya commented Aug 24, 2023

Working examples can be found here: https://github.com/zvasya/VulkanExample

Updated packages are in the LocalPackages folder.

@zvasya
Copy link
Contributor Author

zvasya commented Aug 24, 2023

@dotnet-policy-service agree

Copy link
Member

@Perksey Perksey left a comment

Choose a reason for hiding this comment

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

This is an amazing contribution I wasn't expecting to see given the state of our iOS support! Thank you so much! There's a few comments mostly relating to process and deployment, and a couple of code comments. Hopefully shouldn't take too much time, again thank you so much for your work here.

Valentin added 2 commits August 29, 2023 12:40
Compile MoltenVK with Nuke
take only necessary libraries into MoltenVK package instead of the full xcframework
Copy link
Member

@Perksey Perksey left a comment

Choose a reason for hiding this comment

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

This is awesome! Please keep it coming, super excited to get Silk.NET.Vulkan working on iOS out of the box.

build/nuke/Native/MoltenVK.cs Outdated Show resolved Hide resolved
Copy link
Member

@Perksey Perksey left a comment

Choose a reason for hiding this comment

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

Good to go after this, thank you so much for your work!

src/Vulkan/Silk.NET.Vulkan/PreventSilkTouchBugAttribute.cs Outdated Show resolved Hide resolved
Co-authored-by: Dylan Perks <11160611+Perksey@users.noreply.github.com>
@zvasya zvasya requested a review from Perksey September 9, 2023 07:57
@HurricanKai
Copy link
Member

I'm probably missing context here - I'm not really surprised that this works, but I'd appreciate if the relevant context is added here or even better the new attribute has some comment to explain why it's necessary.

(Using __Internal works fine for Sdl, right? Why is the attribute not necessary there?)

@zvasya
Copy link
Contributor Author

zvasya commented Sep 9, 2023

I'm probably missing context here - I'm not really surprised that this works, but I'd appreciate if the relevant context is added here or even better the new attribute has some comment to explain why it's necessary.

(Using __Internal works fine for Sdl, right? Why is the attribute not necessary there?)

We discuss this with Perksey at https://discord.com/channels/521092042781229087/607634593201520651/1149730272754020384

In two words:
If I don't add this attribute, the generated file will contain an extra #endif. This is because #if is a trivia in the attributes that are being removed. And #endif is the leading trivia modifier.

And without this condition there is compiler (linking) error for Mac Release. Same error would be if add Silk.Net.SDL package to Mac project.

@Perksey Perksey enabled auto-merge (squash) September 9, 2023 17:02
@Perksey Perksey merged commit fdd0f40 into dotnet:main Sep 9, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants