From d9e2505b596b9140e321ea9f961bd183d9626f50 Mon Sep 17 00:00:00 2001 From: InKryption Date: Mon, 22 Nov 2021 13:53:40 +0100 Subject: [PATCH] glfw: amend and update various TODOs pertaining to force-init changes --- glfw/src/Cursor.zig | 4 ++-- glfw/src/Joystick.zig | 13 ++++++----- glfw/src/Monitor.zig | 12 +++++------ glfw/src/Window.zig | 43 +++++++++++++++++++++---------------- glfw/src/internal_debug.zig | 2 -- glfw/src/main.zig | 5 +---- glfw/src/vulkan.zig | 2 +- 7 files changed, 41 insertions(+), 40 deletions(-) diff --git a/glfw/src/Cursor.zig b/glfw/src/Cursor.zig index 9334d8ef..3c7f2fb1 100644 --- a/glfw/src/Cursor.zig +++ b/glfw/src/Cursor.zig @@ -84,7 +84,7 @@ pub inline fn createStandard(shape: Shape) Error!Cursor { internal_debug.assertInitialized(); const cursor = c.glfwCreateStandardCursor(@intCast(c_int, @enumToInt(shape))); getError() catch |err| return switch (err) { - // TODO: should be impossible given that only the values in 'Shape' are available, unless the user explicitly gives us a bad value via casting + // should be unreachable given that only the values in 'Shape' are available, unless the user explicitly gives us a bad value via casting Error.InvalidEnum => unreachable, Error.PlatformError => err, else => unreachable, @@ -111,7 +111,7 @@ pub inline fn destroy(self: Cursor) void { internal_debug.assertInitialized(); c.glfwDestroyCursor(self.ptr); getError() catch |err| return switch (err) { - Error.PlatformError => {}, // TODO: See 'todo' in 'internal_debug.zig' concerning 'PlatformError' + Error.PlatformError => std.log.debug("{}: was unable to destroy Cursor.\n", .{ err }), else => unreachable, }; } diff --git a/glfw/src/Joystick.zig b/glfw/src/Joystick.zig index 1c01570e..01978554 100644 --- a/glfw/src/Joystick.zig +++ b/glfw/src/Joystick.zig @@ -311,8 +311,7 @@ pub inline fn getGUID(self: Joystick) Error!?[:0]const u8 { /// @thread_safety This function may be called from any thread. Access is not synchronized. /// /// see also: joystick_userptr, glfw.Joystick.getUserPointer -// TODO: review this function signature -pub inline fn setUserPointer(self: Joystick, Type: anytype, pointer: Type) void { +pub inline fn setUserPointer(self: Joystick, comptime T: type, pointer: *T) void { internal_debug.assertInitialized(); c.glfwSetJoystickUserPointer(self.jid, @ptrCast(*c_void, pointer)); getError() catch unreachable; // Only error 'GLFW_NOT_INITIALIZED' is impossible @@ -329,12 +328,11 @@ pub inline fn setUserPointer(self: Joystick, Type: anytype, pointer: Type) void /// @thread_safety This function may be called from any thread. Access is not synchronized. /// /// see also: joystick_userptr, glfw.Joystick.setUserPointer -// TODO: review this function signature -pub inline fn getUserPointer(self: Joystick, Type: anytype) ?Type { +pub inline fn getUserPointer(self: Joystick, comptime PointerType: type) ?PointerType { internal_debug.assertInitialized(); const ptr = c.glfwGetJoystickUserPointer(self.jid); getError() catch unreachable; // Only error 'GLFW_NOT_INITIALIZED' is impossible - if (ptr) |p| return @ptrCast(Type, @alignCast(@alignOf(Type), p)); + if (ptr) |p| return @ptrCast(PointerType, @alignCast(@alignOf(std.meta.Child(PointerType)), p)); return null; } @@ -460,12 +458,13 @@ pub inline fn isGamepad(self: Joystick) bool { /// @thread_safety This function must only be called from the main thread. /// /// see also: gamepad, glfw.Joystick.isGamepad -// TODO: Consider this; GLFW documentation for this function doesn't list any errors, -// but source code in 'init.c' only appears to return 'GLFW_INVALID_ENUM' and 'GLFW_NOT_INITIALIZED' on error. pub inline fn getGamepadName(self: Joystick) Error!?[:0]const u8 { internal_debug.assertInitialized(); const name_opt = c.glfwGetGamepadName(self.jid); getError() catch |err| return switch (err) { + // TODO: See 'todo' at top of file concerning making 'Joystick' into an enum to make 'InvalidEnum' unreachable + // Note: GLFW documentation doesn't list an error for this function, but source does set 'GLFW_NOT_INITIALIZED' + // and 'GLFW_INVALID_ENUM'. Error.InvalidEnum => err, else => unreachable, }; diff --git a/glfw/src/Monitor.zig b/glfw/src/Monitor.zig index 2021d6f0..9ffe8bd1 100644 --- a/glfw/src/Monitor.zig +++ b/glfw/src/Monitor.zig @@ -98,7 +98,7 @@ const PhysicalSize = struct { /// @thread_safety This function must only be called from the main thread. /// /// see also: monitor_properties -// TODO: Remove error stub +// TODO: Consider whether to retain error here, despite us guaranteeing the absence of 'GLFW_NOT_INITIALIZED' pub inline fn getPhysicalSize(self: Monitor) Error!PhysicalSize { internal_debug.assertInitialized(); var width_mm: c_int = 0; @@ -157,7 +157,7 @@ pub inline fn getContentScale(self: Monitor) Error!ContentScale { /// @thread_safety This function must only be called from the main thread. /// /// see also: monitor_properties -// TODO: Remove error stub +// TODO: Consider whether to retain error here, despite us guaranteeing the absence of 'GLFW_NOT_INITIALIZED' pub inline fn getName(self: Monitor) Error![*:0]const u8 { internal_debug.assertInitialized(); const name = c.glfwGetMonitorName(self.handle); @@ -178,7 +178,7 @@ pub inline fn getName(self: Monitor) Error![*:0]const u8 { /// @thread_safety This function may be called from any thread. Access is not synchronized. /// /// see also: monitor_userptr, glfw.Monitor.getUserPointer -// TODO: Remove error stub +// TODO: Consider whether to retain error here, despite us guaranteeing the absence of 'GLFW_NOT_INITIALIZED' pub inline fn setUserPointer(self: Monitor, comptime T: type, ptr: *T) Error!void { internal_debug.assertInitialized(); c.glfwSetMonitorUserPointer(self.handle, ptr); @@ -197,7 +197,7 @@ pub inline fn setUserPointer(self: Monitor, comptime T: type, ptr: *T) Error!voi /// @thread_safety This function may be called from any thread. Access is not synchronized. /// /// see also: monitor_userptr, glfw.Monitor.setUserPointer -// TODO: Remove error stub +// TODO: Consider whether to retain error here, despite us guaranteeing the absence of 'GLFW_NOT_INITIALIZED' pub inline fn getUserPointer(self: Monitor, comptime T: type) Error!?*T { internal_debug.assertInitialized(); const ptr = c.glfwGetMonitorUserPointer(self.handle); @@ -382,7 +382,7 @@ pub inline fn getAll(allocator: *mem.Allocator) (mem.Allocator.Error)![]Monitor /// @thread_safety This function must only be called from the main thread. /// /// see also: monitor_monitors, glfw.monitors.getAll -// TODO: Remove error stub +// TODO: Consider whether to retain error here, despite us guaranteeing the absence of 'GLFW_NOT_INITIALIZED' pub inline fn getPrimary() Error!?Monitor { internal_debug.assertInitialized(); const handle = c.glfwGetPrimaryMonitor(); @@ -426,7 +426,7 @@ pub const Event = enum(c_int) { /// @thread_safety This function must only be called from the main thread. /// /// see also: monitor_event -// TODO: Remove error stub +// TODO: Consider whether to retain error here, despite us guaranteeing the absence of 'GLFW_NOT_INITIALIZED' pub inline fn setCallback(comptime Data: type, data: *Data, f: ?*const fn (monitor: Monitor, event: Event, data: *Data) void) Error!void { internal_debug.assertInitialized(); if (f) |new_callback| { diff --git a/glfw/src/Window.zig b/glfw/src/Window.zig index 4d5a15fb..a053fdeb 100644 --- a/glfw/src/Window.zig +++ b/glfw/src/Window.zig @@ -72,7 +72,7 @@ pub const InternalUserPointer = struct { /// @thread_safety This function must only be called from the main thread. /// /// see also: window_hints, glfw.Window.hint, glfw.Window.hintString -// TODO: Remove error stub +// TODO: Consider whether to retain error here, despite us guaranteeing the absence of 'GLFW_NOT_INITIALIZED' inline fn defaultHints() Error!void { internal_debug.assertInitialized(); c.glfwDefaultWindowHints(); @@ -258,7 +258,7 @@ pub const Hints = struct { opengl_core_profile = c.GLFW_OPENGL_CORE_PROFILE, }; - // TODO: Remove error stub + // TODO: Consider whether to retain error here, despite us guaranteeing the absence of 'GLFW_NOT_INITIALIZED' and 'GLFW_INVALID_ENUM' fn set(hints: Hints) Error!void { internal_debug.assertInitialized(); inline for (comptime std.meta.fieldNames(Hint)) |field_name| { @@ -416,8 +416,18 @@ pub inline fn create(width: usize, height: usize, title: [*:0]const u8, monitor: if (monitor) |m| m.handle else null, if (share) |w| w.handle else null, ); - // TODO: Consider all the possible errors here, and whether they are actually possible - try getError(); + + getError() catch |err| return switch (err) { + Error.InvalidEnum, + Error.InvalidValue, + => unreachable, // we restrict the set of values to only those which GLFW deems to be valid, so these should never be reachable. + Error.APIUnavailable, + Error.VersionUnavailable, + Error.FormatUnavailable, + Error.PlatformError, + => err, + else => unreachable, + }; return from(handle.?); } @@ -452,7 +462,7 @@ pub inline fn destroy(self: Window) void { // Zig, so by returning an error we'd make it harder to destroy the window properly. So we differ // from GLFW here: we discard any potential error from this operation. getError() catch |err| return switch (err) { - Error.PlatformError => {}, // TODO: Consider this error, and whether it can be guaranteed to not occur somehow + Error.PlatformError => std.log.debug("{}: was unable to destroy Window.\n", .{ err }), else => unreachable, }; } @@ -482,7 +492,7 @@ pub inline fn shouldClose(self: Window) bool { /// synchronized. /// /// see also: window_close -// TODO: Remove error stub +// TODO: Consider whether to retain error here, despite us guaranteeing the absence of 'GLFW_NOT_INITIALIZED' pub inline fn setShouldClose(self: Window, value: bool) Error!void { internal_debug.assertInitialized(); const boolean = if (value) c.GLFW_TRUE else c.GLFW_FALSE; @@ -1102,7 +1112,7 @@ pub inline fn swapBuffers(self: Window) Error!void { /// @thread_safety This function must only be called from the main thread. /// /// see also: window_monitor, glfw.Window.setMonitor -// TODO: Remove error stub +// TODO: Consider whether to retain error here, despite us guaranteeing the absence of 'GLFW_NOT_INITIALIZED' pub inline fn getMonitor(self: Window) Error!?Monitor { internal_debug.assertInitialized(); const monitor = c.glfwGetWindowMonitor(self.handle); @@ -1254,15 +1264,14 @@ pub inline fn getAttrib(self: Window, attrib: Attrib) Error!isize { /// /// see also: window_attribs, glfw.Window.getAttrib /// -// TODO: Maybe enhance type-safety here to avoid 'InvalidEnum' and 'InvalidValue' at runtime alltogether? pub inline fn setAttrib(self: Window, attrib: Attrib, value: bool) Error!void { internal_debug.assertInitialized(); c.glfwSetWindowAttrib(self.handle, @enumToInt(attrib), if (value) c.GLFW_TRUE else c.GLFW_FALSE); getError() catch |err| return switch (err) { Error.InvalidEnum, Error.InvalidValue, - Error.PlatformError, - => err, + => unreachable, + Error.PlatformError => err, else => unreachable, }; } @@ -1282,8 +1291,7 @@ pub inline fn getInternal(self: Window) *InternalUserPointer { /// @thread_safety This function may be called from any thread. Access is not synchronized. /// /// see also: window_userptr, glfw.Window.getUserPointer -// TODO: Review this function signature -pub inline fn setUserPointer(self: Window, Type: anytype, pointer: Type) void { +pub inline fn setUserPointer(self: Window, comptime T: type, pointer: *T) void { internal_debug.assertInitialized(); var internal = self.getInternal(); internal.user_pointer = @ptrCast(*c_void, pointer); @@ -1297,11 +1305,10 @@ pub inline fn setUserPointer(self: Window, Type: anytype, pointer: Type) void { /// @thread_safety This function may be called from any thread. Access is not synchronized. /// /// see also: window_userptr, glfw.Window.setUserPointer -// TODO: Review this function signature -pub inline fn getUserPointer(self: Window, Type: anytype) ?Type { +pub inline fn getUserPointer(self: Window, comptime PointerType: type) ?PointerType { internal_debug.assertInitialized(); var internal = self.getInternal(); - if (internal.user_pointer) |p| return @ptrCast(Type, @alignCast(@alignOf(Type), p)); + if (internal.user_pointer) |p| return @ptrCast(PointerType, @alignCast(@alignOf(std.meta.Child(PointerType)), p)); return null; } @@ -2158,7 +2165,7 @@ fn setDropCallbackWrapper(handle: ?*c.GLFWwindow, path_count: c_int, paths: [*c] /// @thread_safety This function must only be called from the main thread. /// /// see also: path_drop -// TODO: Remove error stub +// TODO: Consider whether to retain error here, despite us guaranteeing the absence of 'GLFW_NOT_INITIALIZED' pub inline fn setDropCallback(self: Window, callback: ?fn (window: Window, paths: [][*:0]const u8) void) Error!void { internal_debug.assertInitialized(); var internal = self.getInternal(); @@ -2727,7 +2734,7 @@ test "setUserPointer" { const T = struct { name: []const u8 }; var my_value = T{ .name = "my window!" }; - window.setUserPointer(*T, &my_value); + window.setUserPointer(T, &my_value); } test "getUserPointer" { @@ -2745,7 +2752,7 @@ test "getUserPointer" { const T = struct { name: []const u8 }; var my_value = T{ .name = "my window!" }; - window.setUserPointer(*T, &my_value); + window.setUserPointer(T, &my_value); const got = window.getUserPointer(*T); std.debug.assert(&my_value == got); } diff --git a/glfw/src/internal_debug.zig b/glfw/src/internal_debug.zig index 0899a90f..74a085e6 100644 --- a/glfw/src/internal_debug.zig +++ b/glfw/src/internal_debug.zig @@ -1,8 +1,6 @@ const std = @import("std"); const zig_builtin = @import("builtin"); -// TODO: Consider if this idea could also be applied to prevent 'PlatformError' - const debug_mode = (zig_builtin.mode == .Debug); var glfw_initialized = if (debug_mode) false else @as(void, {}); pub inline fn toggleInitialized() void { diff --git a/glfw/src/main.zig b/glfw/src/main.zig index 550ddefe..3f39cdce 100644 --- a/glfw/src/main.zig +++ b/glfw/src/main.zig @@ -66,10 +66,7 @@ pub inline fn init(hints: InitHints) Error!void { const init_hint = @field(InitHint, field_name); const init_value = @field(hints, field_name); initHint(init_hint, init_value) catch |err| switch (err) { - // TODO: Consider this; should not be reachable, given that all of the hint tags and hint values - // are coming in from a strict set of predefined values in 'InitHints' and 'InitHint' - error.InvalidValue, - => unreachable, + Error.InvalidValue => unreachable, else => unreachable, }; } diff --git a/glfw/src/vulkan.zig b/glfw/src/vulkan.zig index 7a489f74..c9e4c4bf 100644 --- a/glfw/src/vulkan.zig +++ b/glfw/src/vulkan.zig @@ -24,7 +24,7 @@ const internal_debug = @import("internal_debug.zig"); /// Possible errors include glfw.Error.NotInitialized. /// /// @thread_safety This function may be called from any thread. -// TODO: Remove error stub +// TODO: Consider whether to retain error here, despite us guaranteeing the absence of 'GLFW_NOT_INITIALIZED' pub inline fn vulkanSupported() Error!bool { internal_debug.assertInitialized(); const supported = c.glfwVulkanSupported();