Skip to content

[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
wants to merge 114 commits into
base: develop/3.0
Choose a base branch
from

Conversation

Exanite
Copy link
Contributor

@Exanite Exanite commented May 23, 2025

Current progress

(Will update this as I work)

Bindings now compile!!!

See bottom of PR for current progress since I'm now actively tracking todos here.

Summary of the PR

This PR primarily adds Vulkan bindings.

Summary of important decisions/changes:

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

Things to know

The change to ExtractNestedTyping and TransformHandles is a breaking change.
Existing binding generator configurations need to have their configurations updated.

Todos

Discuss

  • (Discuss) Structs don't have constructors unlike Silk 2. I don't know if this was an intentional change or if this functionality simply hasn't been properly implemented yet.
  • (Fixed-ish; see below task) I don't think I enabled generation of non-static vtables. All methods are static right now.
    • (Discuss) I enabled the generation of the StaticWrapper vtable, but I'm not sure if this is what we want for Vulkan.
  • Consider transforming bitfield structs into enums where possible
    • Eg: StdVideoEncodeH264SliceHeaderFlags

Attributes

  • Ensure attributes are correct
    • SupportedApiProfiles
    • NativeTypeName

These are not important for the bindings to be used, but are useful.
I might do these in a separate PR.


Trimming

  • The Vk in PFNVkDebugUtilsMessengerCallbackEXT is not trimmed properly
    • This seems similar to the issue we had with TransformHandles. We need to prettify/trim first, then add PFN- to the front.

Before undrafting PR

  • Self code review and make sure all important changes/decisions are documented in the PR summary.
  • Consider updating configs for other bindings because my changes affects them.
    • ExtractNestedTyping and TransformHandles
    • PrettifyNames mod order
  • Update my LocationTransformer code to work with changes made in curin's PR: 0cc49b8#diff-4d93e56d6d07a37ac3577a7fa2dfd0f0649e9ac07912045809c723351c9be56c
    • Curin is now aware of this code and is working on it

Future PRs

In progress

Done

Bools

  • SamplerCreateInfo.UnnormalizedCoordinates, SamplerCreateInfo.CompareEnable, etc uses uint instead of a bool-like type.
    • (Discuss) Is this related to the MaybeBool type? I haven't looked at the rest of Silk 3 yet. If so, then this might be something that can be fixed separately from the PR.
    • The SDL bindings already use MaybeBool, so the functionality already exists. I'll look into how to use it for Vulkan.

Deprecated aliases

  • (Discuss) VK_STRUCTURE_TYPE_SURFACE_CAPABILITIES2_EXT (alias of VK_STRUCTURE_TYPE_SURFACE_CAPABILITIES_2_EXT), VK_KHR_MAINTENANCE1_SPEC_VERSION, etc are deprecated aliases that cause compilation issues due to name conflicts. These are currently manually excluded.
    • These are marked with the deprecated="aliased" attribute in the XML spec
    • Automate the list of type exclusions
    • Decision: I'm going to implement this because these names tend to cause conflicts during name prettification and because there is always an obvious alternative available. This can lead to more upstream breakages, but fixing these errors are trivial. Removing these also reduces confusion about which one to use.

MaxEnum members

  • MaxEnum enum member removal also needs to consider INVALID members (Vulkan Video)
    • This is trickier because we can't just indiscriminately remove the INVALID members when the name contains the string
  • MaxEnum member removal skips over enums not read from the XML
    • This includes both the Vulkan Video enums (there is a XML spec, but we don't read it) and the enums that are empty
      in the Vulkan XML spec (the XML spec is out of date/out of sync)
    • This also impacts the adding of [Flags] attributes.

I ended up making it so that the TransformEnums mod handles MaxEnum removal instead of MixKhronosData. This is because the max enum value pattern applies to C as a whole, rather than just Vulkan or Khronos.

  • (Discuss) MixKhronosData ignores empty enums/groups from the XML. Would it be better to generate empty enums for these instead of completely ignoring them?
    • This is due to the group data only being initialized if there is at least one group member (search for data.Groups[group] = data.Groups.TryGetValue(group, out var groupInfo) in MixKhronosData and read the surrounding code for more context)
    • Because this seems like an implementation detail / oversight to me, I want to make sure that this is intentional.
    • Decided to fix this since it affects the correctness of other transformations.

I also made it so that MixKhronosData does not skip over empty enum groups, which fixes the generation of [Flags] attributes and leads to a few more enums being generated.


Trimming

  • ComponentSwizzle.ComponentSwizzleIdentity, etc is not trimmed properly. The enum name isn't getting trimmed here for some reason.
  • ShaderStageFlags.ShaderStageVertexBit, etc is not trimmed properly
    • Guess: ShaderStageFlags is not a prefix of ShaderStageVertexBit Incorrect. This was the same issue as above.
  • Fix PipelineCompilerControlFlagsAMD.FlagBitsMaxEnumAMD, etc trimming
    • (Discuss) Didn't realize it when I first wrote this, but these are the MaxEnum members. We don't want these do we?
    • Note to self: Fix trimming first, then remove these members. The MaxEnum members are the only ones that are named like this in the headers so we can just remove them. This isn't a trimming issue.
  • Removing the MaxEnum members breaks IndirectStateFlagsNV.FlagFrontfaceBitNV due to no prefixes being shared by member names
    • Exhaustive list: IndirectStateFlagsNV, SemaphoreImportFlags, SemaphoreWaitFlags, SurfaceCounterFlagsEXT

Linux

  • Remove Linux specific Clang workaround in eng/silktouch/vulkan/settings.rsp
    • I needed to add /usr/lib/clang/20/include as an include directory. 20 is the Clang version.
  • Discuss longterm solution for handling the Clang include directory. I know there is UnixStdIncludeResolver
    • I added support for finding the /usr/lib/clang/*/include folder to UnixStdIncludeResolver. It finds all matching folder patterns and takes the latest version.

These are related to the handling of Flags enums.

  • (Fixed) DebugUtilsMessageSeverityFlagsEXT is not marked with the [Flags] attribute.
    • (Discuss) Looks like none of the Flags enums are marked with [Flags]. Does this functionality exist in Silk 3 yet?
      • Doesn't seem like it
    • Implemented by checking KnownBitmask in MixKhronosData.Rewriter.
  • (Discuss) DependencyFlags.None does not exist, but does in Silk 2. None does not exist in the Vulkan headers nor XML spec, so I'm assuming None was added by Silk itself.
    • Also ShaderStageFlags.None
    • I attempted to fix this, but led to some unwanted outputs, such as names not being trimmed properly (does the trimmer check to see if all enum members share a prefix?). Also, knowing when to add in the None = 0 member requires evaluating the value of the existing enum members. This is somewhat difficult because they look like unchecked((ulong)0UL), which isn't trivially comparable to 0.
    • This might fit better in a TransformFlags mod so we can have this behavior in all Silk bindings.
    • It looks like we can use IFieldSymbol.ConstantValue for this
    • Implemented by using semanticModel.GetConstantValue in a new TransformFlags mod.

Original comment: These two issues are Flags/FlagBits specific issues. This disconnect causes ClangSharp to just output ulongs/uints. This is likely something we should fix on our side. We can fix this by looking at the NativeTypeName attribute.

Original comment: Some are trivial, but some NativeTypeNames look like const VkPipelineStageFlags *, such as in SubmitInfo.

  • MemoryBarrier2.SrcStageMask does not use the AccessFlags2 enum and uses ulongs instead.
  • DependencyInfo.DependencyFlags does not use the DependencyFlags enum and uses uint instead.
    • Also ImageSubresourceRange.AspectMask
    • Also CommandPoolCreateInfo.Flags
    • Also FenceCreateInfo.Flags
    • Also ClearAttachment.AspectMask
    • ...etc

Original comment: I recently found code that handles the FlagBits -> Flags mapping in MixKhronosData. I think the issue is that this code doesn't fully work yet so I'm looking into seeing how that code works and fixing that code.

  • Remove code used to generate ClangSharp enum type remappings in sources/SilkTouch/SilkTouch/Mods/MixKhronosData.cs.
  • (Not implemented as originally planned) Automate the generation of ClangSharp enum type remappings
    • Actual implementation: Don't remap using ClangSharp. Use MixKhronosData.Rewriter to replace FlagBits with Flags in identifier names globally.

  • (Fixed) Not sure where VK_LOD_CLAMP_NONE and other constants went.
    • Was due to the fake "API Constants" enum defined in the XML spec. Because of this, the constants were getting removed by MixKhronosData.
  • Consider renaming the Vulkan class to Vk to match Silk 2
    • I decided to go ahead and rename it to Vk

Exanite added 7 commits May 23, 2025 19:11
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)
@Exanite Exanite changed the title Add Vulkan bindings for Silk 3.0 [3.0] Add Vulkan bindings for Silk 3.0 May 25, 2025
Exanite added 10 commits May 26, 2025 08:11
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.
@Exanite Exanite force-pushed the feature/vulkan-bindings-3.0 branch from a117cb2 to 20ef463 Compare May 26, 2025 16:38
@Exanite
Copy link
Contributor Author

Exanite commented May 26, 2025

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:

  • Not enums (API Constants)
  • Are part of a non-core Vulkan header
    • VkFullScreenExclusiveEXT is part of vulkan_win32.h
    • VkDisplacementMicromapFormatNV is part of vulkan_beta.h
  • Or don't exist at all in the headers in the Vulkan-Docs nor Vulkan-Headers repos
fail: Silk.NET.SilkTouch.Mods.MixKhronosData[0] Enum "API Constants" has no base type. Please add TypeMap entry to the configuration. This enum group will be skipped.
fail: Silk.NET.SilkTouch.Mods.MixKhronosData[0] Enum "VkFullScreenExclusiveEXT" has no base type. Please add TypeMap entry to the configuration. This enum group will be skipped.
fail: Silk.NET.SilkTouch.Mods.MixKhronosData[0] Enum "VkFaultLevel" has no base type. Please add TypeMap entry to the configuration. This enum group will be skipped.
fail: Silk.NET.SilkTouch.Mods.MixKhronosData[0] Enum "VkFaultType" has no base type. Please add TypeMap entry to the configuration. This enum group will be skipped.
fail: Silk.NET.SilkTouch.Mods.MixKhronosData[0] Enum "VkFaultQueryBehavior" has no base type. Please add TypeMap entry to the configuration. This enum group will be skipped.
fail: Silk.NET.SilkTouch.Mods.MixKhronosData[0] Enum "VkPipelineMatchControl" has no base type. Please add TypeMap entry to the configuration. This enum group will be skipped.
fail: Silk.NET.SilkTouch.Mods.MixKhronosData[0] Enum "VkSciSyncClientTypeNV" has no base type. Please add TypeMap entry to the configuration. This enum group will be skipped.
fail: Silk.NET.SilkTouch.Mods.MixKhronosData[0] Enum "VkSciSyncPrimitiveTypeNV" has no base type. Please add TypeMap entry to the configuration. This enum group will be skipped.
fail: Silk.NET.SilkTouch.Mods.MixKhronosData[0] Enum "VkPipelineCacheValidationVersion" has no base type. Please add TypeMap entry to the configuration. This enum group will be skipped.
fail: Silk.NET.SilkTouch.Mods.MixKhronosData[0] Enum "VkDisplacementMicromapFormatNV" has no base type. Please add TypeMap entry to the configuration. This enum group will be skipped.

@Exanite Exanite force-pushed the feature/vulkan-bindings-3.0 branch from 20ef463 to e97a9a7 Compare May 26, 2025 19:04
Exanite added 2 commits May 26, 2025 22:21
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
@Exanite Exanite force-pushed the feature/vulkan-bindings-3.0 branch from 54c6350 to 57bde07 Compare May 27, 2025 02:25
Exanite added 30 commits July 11, 2025 00:38
…immer

This also prepares the Rewriter class for a few other changes.
Bit invasive, but since all FlagBits enums are already treated the same way, this ensures that everything is processed consistently. This also avoids the need to tell ClangSharp to remap these enums.
…int/ulongs with types that don't exist in the project
This adds a "None = 0" member to Flags enums if they do not have an equivalent already
Because the MaxEnum member got removed, some enum members had their trimming adjusted as a side effect.

Case 1: Matches Silk 2:
- SamplerYcbcrRange
- SemaphoreImportFlags
- DeviceDiagnosticsConfigFlagsNV
- FenceImportFlags
- VideoDecodeCapabilityFlagsKHR
- VideoEncodeFeedbackFlagsKHR
- ClusterAccelerationStructureAddressResolutionFlagsNV (Doesn't exist in Silk 2)

Case 2: Unintended changes (need to fix):
- IndirectStateFlagsNV
- SemaphoreImportFlags
- SemaphoreWaitFlags
- SurfaceCounterFlagsEXT
Doesn't seem to affect OpenGL bindings generation

This is to ensure all trimming code happens after rewinding, which is useful because I'm thinking of refactoring this code and want to ensure that I can treat all of the trimming code equally.
This is to avoid trimming things like GL_T from OpenGL
This fixes a few cases where the trimmer fails to trim enum member names.
Note that this affects both the Vulkan bindings and the OpenGL bindings.

(cherry picked from commit 2df6cef)
(cherry picked from commit 832c95b)
These only seem to exist for Vulkan currently.

For Vulkan:
These represent misnamed constants that have now been renamed to be consistent with the rest of the Vulkan spec.

For Silk:
These misnamed constants and their corrected variants cause name collisions when names are prettified (thus leading to compile errors) and cause common prefix determination for enum member names to be more pessimistic (thus leading to longer names).

For users:
Removing these aliases can induce more API breaks, but because these are aliases and always have an alternative, API breaks are simple to fix. Removing these aliases also makes it clearer which variant is the correct variant to use.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

1 participant