[3.0] Add Khronos StructureType field initializers#2576
Conversation
…need to modify the struct as well
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against c9d23c9 |
Exanite
force-pushed
the
feature/vk-structure-types
branch
from
7a20a21 to
c2c40d6
Compare
|
Verified Windows generation output is the exact same, so not adding an extra commit. |
Exanite
left a comment
Exanite
left a comment
There was a problem hiding this comment.
Self code review done.
| @@ -1,5 +1,6 @@ | |||
| @../../common.rsp | |||
| --define-macro | |||
| VK_ENABLE_BETA_EXTENSIONS | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I did a search over the repo for any missing structure type initializers, and looks like we are good:
| ).Project; | ||
| } | ||
|
|
||
| // Rewrite phase 4 |
There was a problem hiding this comment.
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 StructureTypeMembers within this PR and any metadata we need for the Vulkan struct chaining feature later on. I plan to keep metadata used for Vulkan struct chaining separate from the structure type field initialization code added in this PR.
| "SpecPath": "eng/submodules/vulkan/xml/vk.xml", | ||
| "Namespace": "Silk.NET.Vulkan", | ||
| "StructureTypes": [ | ||
| "VkStructureType" |
There was a problem hiding this comment.
Similar to FlagsTypes immediately below, StructureTypes should let us handle OpenXR easily once we get to that point.
| "VK_KHR_deferred_host_operations+VK_VERSION_1_2", | ||
| ] | ||
| )] | ||
| public AccelerationStructureBuildGeometryInfoKHR() { } |
There was a problem hiding this comment.
Structs that have field initializers need a constructor.
1 participant
Summary of the PR
This adds default field initializers to structures that have structure type members, such as
sTypeforVkStructureTypeinVulkan.Related issues, Discord discussions, or proposals
Original discussion with Perksey: https://discord.com/channels/521092042781229087/1376331581198827520/1395826788101263463
Discord thread for work done during this PR: https://discord.com/channels/521092042781229087/1499891853217103902
Further Comments
Tasks