-
-
Notifications
You must be signed in to change notification settings - Fork 435
[3.0] Add Vulkan bindings for Silk 3.0 #2457
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
Draft
Exanite
wants to merge
37
commits into
dotnet:develop/3.0
Choose a base branch
from
Exanite:feature/vulkan-bindings-3.0
base: develop/3.0
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This code seems to look for "<feature api=" elements in vk.xml. However, one of the six total elements don't have a "depends" attribute. You can see this by opening https://raw.githubusercontent.com/KhronosGroup/Vulkan-Docs/refs/heads/main/xml/vk.xml and searching for "<feature api=". This is the line that does not have the depends attribute, which causes the assert to fail: <feature api="vulkan,vulkansc" name="VK_VERSION_1_0" number="1.0" comment="Vulkan core API interface definitions"> The other relevant lines do have a depends attribute: <feature api="vulkan,vulkansc" name="VK_VERSION_1_1" number="1.1" depends="VK_VERSION_1_0" comment="Vulkan 1.1 core API interface definitions.">
This commit fixes the allowLeadingDigits overload. The previous similarly named commit fixes the maxLen overload.
This currently doesn't generate anything for some reason (only the enums generate, and that's because of a previous mod)
I'm guessing we don't need this since we don't care about vulkansc. Let me know if we want to keep this defined.
Nothing generates yet. Not sure why.
There are some file names that are incorrect (the enum name inside the file is fine), such as sources/Vulkan/Vulkan/Vulkan/VkBufferCreateFlagBits.gen.cs. They all seem to be enums. I did some light checking and it seems that these incorrectly named enums don't have equivalents in the Enums folder. Some enums have multiple versions, which is expected, but the newer versions tend to be in the Enums folder while the older versions are in the Vulkan folder: For example: sources/Vulkan/Vulkan/Vulkan/VkAccessFlagBits.gen.cs sources/Vulkan/Vulkan/Enums/AccessFlags2.gen.cs sources/Vulkan/Vulkan/Enums/AccessFlags3KHR.gen.cs Interestingly, as I was writing this commit message, I noticed that AccessFlags.gen.cs was named correctly before I added the traverse argument in my previous commits.
Will regenerate using Windows or whichever platform is preferred later for consistency.
a117cb2
to
20ef463
Compare
Putting this here for reference. These are the logs related to the generation of enums during the MixKhronosData mod after the latest commit. The enums specified in these errors either are:
|
20ef463
to
e97a9a7
Compare
Only uint64_t=ulong affects the generation, but the others are included to be slightly more complete. This does not fix the enum backing types though, which also differ between Linux/Windows. See this conversation in the Silk.NET Discord for more information: https://discord.com/channels/521092042781229087/1376331581198827520/1376628539536969949 Relevant ClangSharp issue: dotnet/ClangSharp#574
This is related to dotnet/ClangSharp#601
54c6350
to
57bde07
Compare
Technically this only removes the T, but the effect should be the same. Note the git diff looks a bit weird on this commit. Git seems to think that some of the files were renamed when they were not.
Not sure if doing it this way is correct
This also adds slightly refactors the code for better debuggability (extractedFunctionPointers).
No idea why the using statements change. Enum backing types also change between uint to int. This is fine for mosts cases since the bit width doesn't change, but nevertheless concerning.
This is so that the ExtractNestedTypes mod can also make use of the logic without having to duplicate code. This also adds xml doc comments to all public facing members. Most of these new doc comments are inheritdoc tags. The notable doc comments to review are the comment on the class itself and on GetHandleTypes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Current progress
(Will update this as I work)
Currently investigating how enum and handle types should be handled.
Enum types
Not actively working on.
Handle types
Actively working on.
Perksey and I discussed rewriting ExtractNestedTyping and TransformHandles to use the Roslyn symbol model and to use the symbol renamer. The symbol renamer is currently implemented in a different, unmerged PR so I'll likely wait for that or cherry pick that commit. However, I'll start working on any functionality that does not require the symbol renamer in the meantime.
Summary of the PR
This PR primarily adds Vulkan bindings.
Summary of important decisions/changes:
remap-stdint.rsp
file specific to Vulkan bindings generation to fixuint64_t
mapped tonuint
on Linux (butulong
on Windows) ClangSharp#574This also adds a few fixes to existing code:
Related issues, Discord discussions, or proposals
Initial Discord discussion: https://discord.com/channels/521092042781229087/587346162802229298/1375224188129906778
Discord thread: https://discord.com/channels/521092042781229087/1376331581198827520/1376623544875880589
Further Comments
Before undrafting PR
This tracks smaller things that I might forget about.
sources/SilkTouch/SilkTouch/Mods/MixKhronosData.cs
.eng/silktouch/vulkan/settings.rsp