-
-
Notifications
You must be signed in to change notification settings - Fork 460
[3.0] Add Khronos StructureType field initializers #2576
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
base: develop/3.0
Are you sure you want to change the base?
Changes from all commits
e722325
965f793
4b9346d
0efa063
fac1d52
b01d322
c2c40d6
c9d23c9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| @../../common.rsp | ||
| --define-macro | ||
| VK_ENABLE_BETA_EXTENSIONS | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TerraFX also does the same: https://github.com/terrafx/terrafx.interop.vulkan/blob/86629a4d0afc1de12d10111b4d1e8bbd4be4363b/generation/settings.rsp#L36 This macro mainly guards a few enum members. Without these enum members, the generated bindings fail to compile. |
||
| TODO_DEFINE_MACROS=HERE | ||
| --headerFile | ||
| ../header.txt | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -428,6 +428,9 @@ | |
| "MixKhronosData": { | ||
| "SpecPath": "eng/submodules/vulkan/xml/vk.xml", | ||
| "Namespace": "Silk.NET.Vulkan", | ||
| "StructureTypes": [ | ||
| "VkStructureType" | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to |
||
| ], | ||
| "FlagsTypes": [ | ||
| "VkFlags", | ||
| "VkFlags64" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -120,6 +120,11 @@ public Dictionary< | |
| /// This was added for Vulkan Flags/FlagBits remappings. | ||
| /// </remarks> | ||
| public Dictionary<string, string> AdditionalTypeRemappings = []; | ||
|
|
||
| /// <summary> | ||
| /// A mapping from struct type to information about the structure type member. | ||
| /// </summary> | ||
| public Dictionary<string, StructureTypeMember> StructureTypeMembers = []; | ||
| } | ||
|
|
||
| /// <summary> | ||
|
|
@@ -146,6 +151,12 @@ public record Configuration | |
| /// </summary> | ||
| public Dictionary<string, string>? TypeMap { get; init; } | ||
|
|
||
| /// <summary> | ||
| /// The structure type enums used by the API. | ||
| /// Eg: VkStructureType for Vulkan | ||
| /// </summary> | ||
| public HashSet<string> StructureTypes { get; init; } = []; | ||
|
|
||
| /// <summary> | ||
| /// The base type used for flags/bitmask enums. | ||
| /// For example, VkFlags and VkFlags64 for Vulkan. | ||
|
|
@@ -342,6 +353,58 @@ .. currentConfig.NonStandardExtensionNomenclature | |
|
|
||
| job.AdditionalTypeRemappings[mapFrom] = mapTo; | ||
| } | ||
|
|
||
| // Gather information about struct structure type enums | ||
| if (currentConfig.StructureTypes.Count != 0) | ||
| { | ||
| foreach ( | ||
| var typeElement in xml.Elements("registry") | ||
| .Elements("types") | ||
| .Elements("type") | ||
| .Where(x => x.Attribute("category")?.Value == "struct") | ||
| ) | ||
| { | ||
| var structName = typeElement.Attribute("name")?.Value; | ||
| if (structName == null) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| foreach (var memberElement in typeElement.Elements("member")) | ||
| { | ||
| var memberType = memberElement.Element("type")?.Value; | ||
| if (memberType == null) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| if (currentConfig.StructureTypes.Contains(memberType)) | ||
| { | ||
| var memberName = memberElement.Element("name")?.Value; | ||
| if (memberName == null) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| var memberValue = memberElement.Attribute("values")?.Value; | ||
| if (memberValue == null) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| job.StructureTypeMembers.Add( | ||
| structName, | ||
| new StructureTypeMember() | ||
| { | ||
| Name = memberName, | ||
| Type = memberType, | ||
| Value = memberValue, | ||
| } | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// <inheritdoc /> | ||
|
|
@@ -402,6 +465,18 @@ public async Task ExecuteAsync(IModContext ctx, CancellationToken ct = default) | |
| ).Project; | ||
| } | ||
|
|
||
| // Rewrite phase 4 | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've decided that it's better to use separate rewriters for everything in MixKhronosData. There is a performance impact of doing this, but it's much more maintainable to keep every transformation independent of the others. The performance impact is roughly 2 seconds for Vulkan so it is very much worth paying. It also does not affect non-Khronos bindings. I also plan to keep any new metadata storage structures separate from existing structures. This is to further keep things independent. Every transformation should scrape their own data (within reason). In particular, what I have in mind relates to |
||
| var rewriter4 = new RewriterPhase4(jobData); | ||
| foreach (var docId in proj.DocumentIds) | ||
| { | ||
| var doc = | ||
| proj.GetDocument(docId) ?? throw new InvalidOperationException("Document missing"); | ||
| proj = doc.WithSyntaxRoot( | ||
| rewriter4.Visit(await doc.GetSyntaxRootAsync(ct)) | ||
| ?? throw new InvalidOperationException("Visit returned null.") | ||
| ).Project; | ||
| } | ||
|
|
||
| // Rename documents to account for FlagBits/Flags differences | ||
| foreach (var docId in proj.DocumentIds) | ||
| { | ||
|
|
@@ -441,6 +516,13 @@ rsp with | |
| return Task.FromResult(rsps); | ||
| } | ||
|
|
||
| internal record struct StructureTypeMember | ||
| { | ||
| public string Name; | ||
| public string Type; | ||
| public string Value; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Contains information about a group of enums. | ||
| /// </summary> | ||
|
|
@@ -1620,11 +1702,7 @@ private class RewriterPhase1(JobData job, ILogger logger) : CSharpSyntaxRewriter | |
| var attributes = SingletonList( | ||
| AttributeList([Attribute(IdentifierName("Transformed"))]) | ||
| ); | ||
|
|
||
| if (groupInfo.NativeName != null) | ||
| { | ||
| attributes = attributes.WithNativeName(groupInfo.NativeName); | ||
| } | ||
| attributes = attributes.WithNativeName(groupInfo.NativeName); | ||
|
|
||
| var baseTypeSyntax = ParseTypeName(baseType); | ||
|
|
||
|
|
@@ -1903,9 +1981,6 @@ private bool TryGetManagedEnumType( | |
| /// <summary> | ||
| /// This rewriter identifies and extracts vendor extension suffixes into [NameSuffix] attributes. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// Yes, this is a 3rd rewriter. | ||
| /// </remarks> | ||
| private class RewriterPhase3(JobData job, Configuration config) : CSharpSyntaxRewriter | ||
| { | ||
| private SyntaxList<AttributeListSyntax> ProcessAndGetNewAttributes( | ||
|
|
@@ -2189,6 +2264,87 @@ public override SyntaxNode VisitMethodDeclaration(MethodDeclarationSyntax node) | |
| ); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// This rewriter adds default field values for structure type members. | ||
| /// </summary> | ||
| private class RewriterPhase4(JobData job) : CSharpSyntaxRewriter | ||
| { | ||
| public override SyntaxNode VisitStructDeclaration(StructDeclarationSyntax node) | ||
| { | ||
| var structNativeName = node.AttributeLists.GetNativeNameOrDefault(node.Identifier); | ||
| if ( | ||
| !job.StructureTypeMembers.TryGetValue(structNativeName, out var structureTypeMember) | ||
| ) | ||
| { | ||
| return node; | ||
| } | ||
|
|
||
| // Structs need to have a constructor if we use field initializers | ||
| var hasConstructor = false; | ||
| var initializerAdded = false; | ||
| var members = new List<MemberDeclarationSyntax>(); | ||
| foreach (var memberNode in node.Members) | ||
| { | ||
| hasConstructor |= memberNode is ConstructorDeclarationSyntax; | ||
| if (memberNode is not FieldDeclarationSyntax memberFieldNode) | ||
| { | ||
| members.Add(memberNode); | ||
| continue; | ||
| } | ||
|
|
||
| var memberNativeName = memberFieldNode.AttributeLists.GetNativeNameOrDefault( | ||
| memberFieldNode.Declaration.Variables.First().Identifier | ||
| ); | ||
|
|
||
| if (memberNativeName != structureTypeMember.Name) | ||
| { | ||
| members.Add(memberNode); | ||
| continue; | ||
| } | ||
|
|
||
| // Don't replace the default value if one is already provided | ||
| if (memberFieldNode.Declaration.Variables.First().Initializer != null) | ||
| { | ||
| return node; | ||
| } | ||
|
|
||
| var initializer = EqualsValueClause( | ||
| MemberAccessExpression( | ||
| SyntaxKind.SimpleMemberAccessExpression, | ||
| IdentifierName(structureTypeMember.Type), | ||
| IdentifierName(structureTypeMember.Value) | ||
| ) | ||
| ); | ||
|
|
||
| initializerAdded = true; | ||
| members.Add( | ||
| memberFieldNode.WithDeclaration( | ||
| memberFieldNode.Declaration.WithVariables( | ||
| [ | ||
| .. memberFieldNode.Declaration.Variables.Select(variable => | ||
| variable.WithInitializer(initializer) | ||
| ), | ||
| ] | ||
| ) | ||
| ) | ||
| ); | ||
| } | ||
|
|
||
| if (initializerAdded && !hasConstructor) | ||
| { | ||
| members.Add( | ||
| ConstructorDeclaration(node.Identifier) | ||
| .WithModifiers(TokenList(Token(SyntaxKind.PublicKeyword))) | ||
| .WithBody(Block()) | ||
| ); | ||
| } | ||
|
|
||
| node = node.WithMembers([.. members]); | ||
|
|
||
| return node; | ||
| } | ||
| } | ||
|
|
||
| [SuppressMessage("ReSharper", "MoveLocalFunctionAfterJumpStatement")] | ||
| internal void ReadGroups(XDocument doc, JobData data, HashSet<string> vendors) | ||
| { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,7 +21,7 @@ public unsafe partial struct AccelerationStructureBuildGeometryInfoKHR | |
| "VK_KHR_deferred_host_operations+VK_VERSION_1_2", | ||
| ] | ||
| )] | ||
| public StructureType SType; | ||
| public StructureType SType = StructureType.AccelerationStructureBuildGeometryInfoKHR; | ||
|
|
||
| [NativeName("pNext")] | ||
| [SupportedApiProfile( | ||
|
|
@@ -132,4 +132,14 @@ public unsafe partial struct AccelerationStructureBuildGeometryInfoKHR | |
| ] | ||
| )] | ||
| public DeviceOrHostAddressKHR ScratchData; | ||
|
|
||
| [SupportedApiProfile( | ||
| "vulkan", | ||
| ["VK_KHR_acceleration_structure"], | ||
| ImpliesSets = [ | ||
| "VK_KHR_deferred_host_operations+VK_VERSION_1_1+VK_EXT_descriptor_indexing+VK_KHR_buffer_device_address", | ||
| "VK_KHR_deferred_host_operations+VK_VERSION_1_2", | ||
| ] | ||
| )] | ||
| public AccelerationStructureBuildGeometryInfoKHR() { } | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Structs that have field initializers need a constructor. |
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a search over the repo for any missing structure type initializers, and looks like we are good:
