From 57410828482f6a560f89200cc52f484e0f50f0cf Mon Sep 17 00:00:00 2001 From: Stephen Gutekanst Date: Sun, 24 Jul 2022 14:00:33 -0700 Subject: [PATCH] gpu: begin switching from enum(usize) to opaque pointers WebGPU objects, like say `WGPUTexture`, could be represented in one of two ways: 1. As an `enum(usize)` as we were doing previously 2. As an `*opaque` pointer In `webgpu.h` natively, `void*` opaque pointers are used. However, WebGPU objects are not actually pointers: they are just IDs. A concrete example of this is how it is almost always valid to for example pass a `WGPUTexture` that has been freed into the API. Doing so does not result in undefined behavior, instead the implementation considers the texture to be an ID and since it can't find such an ID it knows that texture has been free'd and to generate an error instead of undefined behavior. In this regard, we can say that `enum(usize)` is the more faithful representation. `enum(usize)` is not without issues, though: it cannot effectively represent nullability in Zig, which is used in the `webgpu.h` C ABI to represent _optionality_. `?enum(usize)` is not a valid exported C type, the Zig compiler rejects it. And so in practice to maintain C ABI compatability with descriptor struct types (which we do not want to copy or bitcast), we must represent nullability with a `null` value: ```zig pub const Texture = enum(usize) { _, pub const null: Texture = @intToEnum(Texture, 0); pub const Descriptor = extern struct { next_in_chain: *const ChainedStruct, }; pub fn init() Texture { return Texture.null; } }; ``` The downside to this is that `init` cannot return a clear optional `?Texture`, and instead must return a `Texture` which may be `== .null`. This downside seems significant enough to warrant _not_ going with `enum(usize)` and instead going with `*opaque`: ```zig pub const Texture = *opaque { pub fn init() ?Texture { return null } }; pub const TextureDescriptor = extern struct { next_in_chain: *const ChainedStruct, }; ``` The downside to `*opaque` is that the namespace cannot have types nested below it. `gpu.Texture.Descriptor` becomes `gpu.TextureDescriptor`. Not ideal, but a tradeoff we'll accept to be able to represent optionality with the actual Zig type system. Signed-off-by: Stephen Gutekanst --- gpu/src/instance.zig | 11 +++-------- gpu/src/interface.zig | 9 +++++---- gpu/src/main.zig | 2 +- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/gpu/src/instance.zig b/gpu/src/instance.zig index 96fd43dd..3efe79f8 100644 --- a/gpu/src/instance.zig +++ b/gpu/src/instance.zig @@ -1,12 +1,7 @@ const ChainedStruct = @import("types.zig").ChainedStruct; -pub const Instance = enum(usize) { - _, +pub const Instance = *opaque {}; - // TODO: verify there is a use case for nullable value of this type. - pub const none: Instance = @intToEnum(Instance, 0); - - pub const Descriptor = extern struct { - next_in_chain: *const ChainedStruct, - }; +pub const InstanceDescriptor = extern struct { + next_in_chain: *const ChainedStruct, }; diff --git a/gpu/src/interface.zig b/gpu/src/interface.zig index fbdf8544..263c169b 100644 --- a/gpu/src/interface.zig +++ b/gpu/src/interface.zig @@ -1,8 +1,9 @@ const Instance = @import("instance.zig").Instance; +const InstanceDescriptor = @import("instance.zig").InstanceDescriptor; /// Verifies that a gpu.Interface implementation exposes the expected function declarations. pub fn Interface(comptime Impl: type) type { - assertDecl(Impl, "createInstance", fn (descriptor: *const Instance.Descriptor) callconv(.Inline) Instance); + assertDecl(Impl, "createInstance", fn (descriptor: *const InstanceDescriptor) callconv(.Inline) ?Instance); return Impl; } @@ -17,7 +18,7 @@ pub fn Export(comptime Impl: type) type { _ = Interface(Impl); // verify implementation is a valid interface return struct { // WGPU_EXPORT WGPUInstance wgpuCreateInstance(WGPUInstanceDescriptor const * descriptor); - export fn wgpuCreateInstance(descriptor: *const Instance.Descriptor) Instance { + export fn wgpuCreateInstance(descriptor: *const InstanceDescriptor) ?Instance { return Impl.createInstance(descriptor); } }; @@ -25,9 +26,9 @@ pub fn Export(comptime Impl: type) type { /// A no-operation gpu.Interface implementation. pub const NullInterface = Interface(struct { - pub inline fn createInstance(descriptor: *const Instance.Descriptor) Instance { + pub inline fn createInstance(descriptor: *const InstanceDescriptor) ?Instance { _ = descriptor; - return Instance.none; + return null; } }); diff --git a/gpu/src/main.zig b/gpu/src/main.zig index 35ed0350..fd833ff5 100644 --- a/gpu/src/main.zig +++ b/gpu/src/main.zig @@ -19,7 +19,7 @@ pub const ComputePassEncoder = @import("compute_pass_encoder.zig").ComputePassEn pub const ComputePipeline = @import("compute_pipeline.zig").ComputePipeline; pub const Device = @import("device.zig").Device; pub const ExternalTexture = @import("external_texture.zig").ExternalTexture; -pub const Instance = @import("instance.zig").Instance; +pub usingnamespace @import("instance.zig"); pub const PipelineLayout = @import("pipeline_layout.zig").PipelineLayout; pub const QuerySet = @import("query_set.zig").QuerySet; pub const Queue = @import("queue.zig").Queue;