-
Notifications
You must be signed in to change notification settings - Fork 45
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
Add overridable attributes to all types (defaulting to empty) #182
Conversation
while you're at it; you may consider adding a WGPU_NULLABLE attribute too? |
I'll add |
+1 to nullable <3 Yes, this patch looks perfect. |
97ec8e0
to
785b47d
Compare
PTAL again, I had to bundle a couple changes that would have been annoying to split up. |
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.
LGTM w/ non-blocking comments
|
||
typedef struct WGPUComputePassTimestampWrite { | ||
WGPUQuerySet querySet; | ||
uint32_t queryIndex; | ||
WGPUComputePassTimestampLocation location; | ||
} WGPUComputePassTimestampWrite; | ||
} WGPUComputePassTimestampWrite WGPU_STRUCTURE_ATTRIBUTE; | ||
|
||
typedef struct WGPUConstantEntry { | ||
WGPUChainedStruct const * nextInChain; | ||
char const * key; |
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.
we might also want a NONNULL for all of the pointers that aren't nullable?
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.
That's would be a lot of noise, but maybe? What do others think of adding a WGPU_NONNULL
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.
#define WGPU_FUNCTION_ATTRIBUTE | ||
#endif | ||
#if !defined(WGPU_NULLABLE) | ||
#define WGPU_NULLABLE |
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.
Do we want the header to use the clang extension (and maybe other extensions) by default if available, or just leave this for people who want to take advantage of it?
typedef void (*WGPUProc)(void) WGPU_FUNCTION_ATTRIBUTE; | ||
typedef void (*WGPUQueueWorkDoneCallback)(WGPUQueueWorkDoneStatus status, void * userdata) WGPU_FUNCTION_ATTRIBUTE; | ||
typedef void (*WGPURequestAdapterCallback)(WGPURequestAdapterStatus status, WGPUAdapter adapter, char const * message, void * userdata) WGPU_FUNCTION_ATTRIBUTE; | ||
typedef void (*WGPURequestDeviceCallback)(WGPURequestDeviceStatus status, WGPUDevice device, char const * message, void * userdata) WGPU_FUNCTION_ATTRIBUTE; | ||
|
||
typedef struct WGPUChainedStruct { | ||
struct WGPUChainedStruct const * next; |
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.
These should be nullable
- Also adds WGPU_NULLABLE instead of the /* nullable */ comments - Also adds WGPUFeatureName_Float32Filterable - Also adds struct forward declarations and move funtion pointers typedefs before the struct definitions, so that struct can contain function pointers. Fixes webgpu-native#179 Fixes webgpu-native#119
Rebased past #181, let's do the rest as followup |
@litherum PTAL at the second commit, would this work for you?