Skip to content

Conversation

@Exanite
Copy link
Contributor

@Exanite Exanite commented Nov 24, 2025

Summary of the PR

This PR focuses on implementing the naming-related improvements identified in #2457.
Depending on the scope of the tasks, some of these tasks may be pushed to another PR.

Important changes:

  • Handle types are now named as -HandleEXT instead of -EXTHandle.
    • Eg: AccelerationStructureHandleKHR
  • Function pointer delegate types are now named as -DelegateEXT instead of EXTDelegate.
    • Eg: pfn struct BufferCallbackSOFT and pfn delegate BufferCallbackDelegateSOFT
  • Functions with data type suffixes following a number such as alGetSourcei64vDirectSOFT are now trimmed and prettified as GetSourcei64vDirectSOFT instead of GetSourcei64VDirectSOFT. Note the capitalization of v. This is because affixes are added after prettification.
  • Vendor suffixes are now kept strictly uppercased.
  • OpenGL vendor suffix trimming is now generalized and can be configured and used for any Khronos API.
    • Notably, TrimEnumMemberImpliedVendors trims enum member vendor suffixes if it matches the enum type vendor suffix.
  • Struct properties are now prettified. They were previously ignored.
  • NativeName is a new attribute that saves the name output by ClangSharp. This for the most part isn't used by the generator, but can be useful for the end user.
  • NameAffix is a new attribute that allows prefixes/suffixes to be declared upfront and reordered. The idea is that mods add the attribute to identifiers and PrettifyNames applies the affixes during trimming and prettification.
  • A large portion of the trimming/PrettifyNames code has been rewritten.
    • NameTrimmer has been kept mostly the same.
    • INameTrimmer.Order is used to order the trimmers instead of the version.
    • PrettifyNames.Visitor's functionality is mostly the same, but also gathers name affix data.
    • MixKhronosData.Trim is almost all replaced by the name affix system. This is implemented in MixKhronosData.RewriterPhase3 (yes, there are 3 rewriters, but this keeps things simpler without adding much time cost).
    • Most usages of tuples have been replaced with record structs.
    • Most nullable collections (eg: List<string>?) have been made non-nullable. Most of these get allocated anyway and it makes the code more maintainable. If there is any performance issue (mainly concerning the Microsoft job), I will take the time to optimize it.
    • Prettification (as in calling the Prettify extension method) is now implemented as another trimmer. Prettification now happens before name conflict resolution and before name affixes are added.
  • Handle structs now have constructors since they were previously not possible to construct (field is readonly).
  • The StripAttributes mod can be used to remove attributes from the final bindings.
    • Currently the following are removed: NativeTypeName, NameAffix, Transformed

Related issues, Discord discussions, or proposals

Previous Vulkan PR (initial bindings generation): #2457
Discord thread: https://discord.com/channels/521092042781229087/1376331581198827520/1442651368207941643

Further Comments

Tasks not part of this PR

These are the tasks from my previous PR. These have been sorted and trimmed down to ensure that this PR remains focused. These may be added to this PR.

If you want to see the original, unmodified set of tasks, please see #2457.

These will also be part of a future PR.

  • Apply BoolType transformation to VkBool32 in structs. They currently are only handled for functions.
  • Add default field values to structs where it makes sense (eg: SType)
  • Ensure SupportedApiProfiles attributes are correct
    • Properly resolve API profiles for Flags/FlagBits types
    • Prefer name from NativeName for resolving API profiles

Future PRs

  • Update Curin's branch to use my new LocationTransformation code. The new renamer should be replaced with this. See the NameUtils.cs and Renamer.cs files on Curin's branch. Renamer.cs should be removed. NameUtils.cs should have the methods updated to use my LocationTransformation code instead (this should show up as a merge conflict).
    • PR opened and ready for review/merge
    • PR merged
    • Old renamer code removed
  • Reimplement the struct chaining API
  • Figure out why the headers are getting mixed up when running multiple jobs at the same time.
    • Seems to be related to TransformFunctions.
  • Figure out why the SDL handle structs are getting named as Silk.NET.SDL.ConditionHandle.gen.cs instead of ConditionHandle.gen.cs when running multiple jobs at the same time.
  • Work on making the generator/mod configs more consistent and user friendly. This should be done before the generator is considered a public API.
  • Consider opening a ClangSharp PR to fix issue with SDL_MAX_SINT64 generation on Linux
  • Consider splitting ExtractNestedTyping and TransformHandles into a new set of Extract- mods.
    • This this mainly for maintainability for ExtractNestedTyping, but for TransformHandles it is useful for AddApiProfiles to be executed after all types are extracted, but not yet transformed.
    • Alternatively, make AddApiProfiles strictly work off of NativeName attributes.
  • Acronym related name improvements discussed here: https://discord.com/channels/521092042781229087/587346162802229298/1446979314330501172
    • Re-read discussion and organize into tasks.
    • Change default acronym threshold to 2 for all bindings
    • Change default acronym threshold to 2 to be the default if not specified in the PrettifyNames config.
    • Handle Mat4X4 case. The "X" should be lowercased.
    • Update existing names to match framework design guidelines
      • EGL as Egl
      • Name overrides in generator config

Current Todos

Prepare to merge.

  • Finish self code review
  • Generate bindings on Windows for consistency
  • Temporarily remove generated files from PR during code review
  • Review the generated bindings as a second code review
  • Consider updating OpenAL submodule and regenerating

Completed Todos

Newest groups at the top. Order within a group is chronological.

Note that I also wrote a summary at the top. The summary is more comprehensive, but this section can be useful getting additional context. Also see my commit descriptions if more context is needed.

INameTrimmer.Order

  • Add priority system to INameTrimmer.
    • I implemented the new name affixer system using trimmers, so this introduced 3 new trimmers in order to separate each stage properly. There is now a new INameTrimmer.Order property and internal TrimmerOrder enum.

Fix misc issues I noticed

  • VkVendorId members are not trimmed properly
  • Figure out why the members of the enum AttribMask in OpenGL are not getting rewritten.
    • Initial investigation shows that the fieldSymbol.ConstantValue is null, but why it is null is unclear. Maybe a Roslyn bug? Example member: DepthBufferBit = unchecked((uint)0x00000100). Note that similar expressions in Vulkan are fine, eg: unchecked((ulong)0x00000002UL). Maybe it is because the literal type and cast type are different.
    • Wait, are these the enums that we generate?
    • Yup, they are. MixKhronosData was outputting subtly invalid type names. I changed it to use ParseTypeName(baseType) instead of IdentifierName(baseType).

Fix issues introduced by name affix system

  • Identify and fix issues introduced by name affixation. Also just identify and fix naming issues in general.
    • Data type trimming in OpenAL seems to be broken. Probably OpenGL as well.
    • Check if I need to reimplement vendor suffix removal for OpenGL. I probably do.
      • Discuss: I decided to change the vendor suffix removal for enum members so that it removes the member suffix if it matches the type suffix. This prevents cases where a member of a different suffix is added, causing all members to have their suffixes restored.
      • Discuss: The above change made it so that the member suffix removal code is usable for Vulkan so I enabled it for Vulkan as well. See PresentModeKHR to see how it looks (the KHR suffixed members have their suffixes removed, but EXT remains the same).
    • Reapply some of my MixKhronosData code removals to the current develop/3.0 branch and see what changes would be made. This is to ensure I don't accidentally remove functionality when deleting code.
  • Ensure other APIs are configured consistently with Vulkan. Eg: Ensure affix priorities are set, etc.

Name affix system

  • Properly trim prefixed/suffixed names, such as PFNVkDebugUtilsMessengerCallbackEXT and BufferTHandle
    • PFNVkDebugUtilsMessengerCallbackEXT prefix
    • PFNVkDebugUtilsMessengerCallbackDelegateEXT suffix
    • AccelerationStructureHandleKHR suffix
    • Extract vendor extensions from names and store into NameSuffix attributes.
    • Revert "hacky" THandle fix.
  • Consider renaming handle types as HandleEXT instead of EXTHandle for readability.
    • This is fairly doable. The above todos track this.

Name metadata

  • Add NativeName attribute
  • Ensure NativeName attribute is added by all mods that generate new types/members
    • Technically not strictly required, but nice from a API consistency standpoint.
    • MixKhronosData
      • Enum members
      • Enum declarations
        • Vulkan (uses FlagBits version of enum names since it is more helpful when googling)
        • Discuss validity in other Khronos APIs (some APIs don't have actual enums)
          • Eg: GLEnum and the other OpenGL enums
    • ExtractNestedTyping
      • Function pointers
      • Others?
    • TransformHandles

Name prettification

Fix bugs from original PR

  • Enums containing negative values should not have an unsigned backing type.

Handle struct improvements

  • Handle structs don't have an easy way of being constructed (field is readonly and no constructor).

This looks like it's from some sort of built-in renamer.
Don't think we'll ever use this since we have a custom renamer.
Not sure why these were not being coerced when the Vulkan PR was merged, but they started getting coerced in this PR.
This technically is a good thing since it means the coerce backing types code is now working as previously intended.
However, the previously intended implementation missed an edge case, which this commit fixes.
This is because the structs were impossible to reasonably construct before.
Technically this is from my update PR into Curin's branch (curin#101), but is effectively the same as backporting the changes from Curin's branch.
Exact commit that I copied the changes from: 0cc698a
VaListTagHandle is an extra handle struct that seems to generate on Linux (and not on Windows) and breaks global prefix determination.
OpenAL: ContextErrorCode seems to always be replaced with ErrorCode, even on Windows. Not sure why. This also happened during the last PR, but I didn't commit it.
Honestly, what I wrote is a complete guess, I'll likely revisit these once I understand them more.
@Exanite Exanite force-pushed the feature/vulkan-bindings-3.0-improvements branch from 77f3ef3 to 9c9fbfc Compare December 6, 2025 18:30
Exanite

This comment was marked as outdated.

@Exanite Exanite marked this pull request as ready for review December 6, 2025 18:48
@Exanite Exanite requested a review from a team as a code owner December 6, 2025 18:48
@Exanite

This comment was marked as outdated.

@Exanite
Copy link
Contributor Author

Exanite commented Dec 6, 2025

Going to revert acronym casing changes as discussed in the Discord before reopening PR for review.

@Exanite Exanite force-pushed the feature/vulkan-bindings-3.0-improvements branch from b06e648 to 53fbdd3 Compare December 6, 2025 23:02
@Exanite Exanite force-pushed the feature/vulkan-bindings-3.0-improvements branch from c6a07d7 to 9bbf8be Compare December 7, 2025 00:18
Copy link
Contributor Author

@Exanite Exanite left a comment

Choose a reason for hiding this comment

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

Self code review complete. Going to mark as ready for review.

Note that I only have the non-generated changes committed.
The generated changes are here: Exanite#19

This is because the generated changes hide the non-generated changes (it goes over the 1k file limit).

Feel free to comment on the linked PR as well.
It will be merged into this PR after the non-generated changes are reviewed.

DO NOT merge the PR until the generated changes are merged, and preferably, reviewed.

+ "conflicts with members of other types.",
responseFile.FilePath,
key
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is from the Microsoft branch

/// Then if no override is found, then the name's affixes are removed,
/// the name is prettified, and the name's affixes are added back.
/// </summary>
private string ApplyPrettifyOnlyPipeline(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is from the Microsoft branch, but heavily modified.

ICulturedStringTransformer nameTransformer)
{
// Check for overrides
foreach (var (nativeName, overriddenName) in nameOverrides)
Copy link
Contributor Author

@Exanite Exanite Dec 7, 2025

Choose a reason for hiding this comment

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

Ideally this code isn't duplicated with the code in Trim(), but to do this performantly requires a decent amount of refactoring.

attributes = attributes.WithNativeName(groupInfo.NativeName);
}

var baseTypeSyntax = ParseTypeName(baseType);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ParseTypeName fixes an issue where IFieldSymbol.ConstantValue was returning null.

@Exanite Exanite marked this pull request as ready for review December 7, 2025 00:45
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