-
Notifications
You must be signed in to change notification settings - Fork 0
[3.0] Fix missings [Flags] attribute on certain OpenGL enums; Require explicit typemap for enum base types #30
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: fix/incorrect-native-name-glenum
Are you sure you want to change the base?
Conversation
Exanite
left a comment
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.
Self review complete
| bool KnownBitmask, | ||
| string? ExclusiveVendor, | ||
| string? Namespace | ||
| ); |
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.
This was becoming unreadable, especially since I split KnownBitmask into two separate values (see doc comments for IsDefinitelyBitmask and IsMaybeBitmask on added side of the diff).
| /// <summary> | ||
| /// The base type of the group. | ||
| /// </summary> | ||
| public string? BaseType { get; init; } |
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.
Renamed for clarity.
|
|
||
| // OpenGL-style enums have an uint base type | ||
| var baseType = anyGLStyleGroups ? "uint" : null; | ||
| string? namespaceGroupName = null; |
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 changed this to handle the 3 cases (namespace enum, enum group, and additional GL-style groups) separately instead of combining the namespace enum and enum group cases together. This is a bit easy to read/debug and lets us handle each case more specifically.
| } | ||
| } | ||
|
|
||
| // Create an ungrouped group as well i.e. GLEnum, WGLEnum, etc |
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.
"Ungrouped group" sounds weird. I've been just calling these the namespace enums.
| var topLevelIntentionalExclusion = | ||
| groupName is not null && IsIntentionalExclusion(groupName); | ||
| groupName != null | ||
| && namespaceGroupName == null |
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.
This is a side effect of the "handle namespace enums separately" change above.
This is because groupName used to always be GLEnum and similar before. Now groupName actually contains the enum group name (eg: "SpecialNumbers"), so I added this to keep the behavior the same.
Reverting this change causes the "SpecialNumbers" members to be removed from the generated bindings.
I'd like to handle this a bit better, but I'd like to work on OpenCL and the other Khronos APIs a bit more before that.
| NativeName = nativeName ?? groupName, | ||
| BaseType = baseType, | ||
|
|
||
| IsDefinitelyBitmask = isBitmask, |
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.
If isBitmask is true, this means the XML explicitly states that the enum group is a bitmask. This is different from checking whether an enum member is part of a bitmask group, which is what is handled below in the "Parse enum members" loop.
Note that we can't combine these into one boolean property. There are 3 states (not bitmask, maybe bitmask, and definitely bitmask). I opted for two booleans. An enum also works, but is also unwieldy.
| ? memberGroup with | ||
| { | ||
| KnownBitmask = isBitmask && groupInfo.KnownBitmask, | ||
| IsMaybeBitmask = isBitmask && memberGroup.IsMaybeBitmask, |
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.
This is the original bitmask detection logic for OpenGL-style groups.
Summary of the PR
A short summary of this PR and what it adds/removes/fixes.
Related issues, Discord discussions, or proposals
Links go here.
Further Comments