From 1d648c22568e1407bfbce7a8dae6d0e22b371f89 Mon Sep 17 00:00:00 2001 From: InKryption <59504965+InKryption@users.noreply.github.com> Date: Sat, 27 Nov 2021 00:51:52 +0100 Subject: [PATCH] glfw: Eliminate `Error.InvalidValue` --- glfw/src/Joystick.zig | 6 +++++- glfw/src/Monitor.zig | 11 +++++++---- glfw/src/Window.zig | 44 ++++++++++++++++++++++++++++++++++++------- glfw/src/main.zig | 24 +++++++++++------------ glfw/src/opengl.zig | 11 +++++++---- glfw/src/time.zig | 14 ++++++++++---- glfw/src/vulkan.zig | 2 +- 7 files changed, 79 insertions(+), 33 deletions(-) diff --git a/glfw/src/Joystick.zig b/glfw/src/Joystick.zig index 555690c0..6c87b39e 100644 --- a/glfw/src/Joystick.zig +++ b/glfw/src/Joystick.zig @@ -395,7 +395,11 @@ pub inline fn updateGamepadMappings(gamepad_mappings: [*:0]const u8) Error!void internal_debug.assertInitialized(); _ = c.glfwUpdateGamepadMappings(gamepad_mappings); getError() catch |err| return switch (err) { - Error.InvalidValue => err, // TODO: Evaluate if this is preventable, or if this is like a parsing error which should definitely be returned + // TODO: Maybe return as 'ParseError' here? + // TODO: Look into upstream proposal for GLFW to publicize + // their Gamepad mappings parsing functions/interface + // for a better error message in debug. + Error.InvalidValue => err, else => unreachable, }; } diff --git a/glfw/src/Monitor.zig b/glfw/src/Monitor.zig index 88c15352..9026c2ad 100644 --- a/glfw/src/Monitor.zig +++ b/glfw/src/Monitor.zig @@ -274,12 +274,15 @@ pub inline fn getVideoMode(self: Monitor) Error!VideoMode { /// see also: monitor_gamma pub inline fn setGamma(self: Monitor, gamma: f32) Error!void { internal_debug.assertInitialized(); + + std.debug.assert(!std.math.isNan(gamma)); + std.debug.assert(gamma >= 0); + std.debug.assert(gamma <= std.math.f32_max); + c.glfwSetGamma(self.handle, gamma); getError() catch |err| return switch (err) { - // TODO: Consider whether to assert that 'gamma' is a valid value instead of leaving it to GLFW's error handling. - Error.InvalidValue, - Error.PlatformError, - => err, + Error.InvalidValue => unreachable, // we assert that 'gamma' is a valid value, so this should be impossible + Error.PlatformError => err, else => unreachable, }; } diff --git a/glfw/src/Window.zig b/glfw/src/Window.zig index a99a615b..66f4dd72 100644 --- a/glfw/src/Window.zig +++ b/glfw/src/Window.zig @@ -695,6 +695,20 @@ pub inline fn setSize(self: Window, size: Size) Error!void { /// see also: window_sizelimits, glfw.Window.setAspectRatio pub inline fn setSizeLimits(self: Window, min: Size, max: Size) Error!void { internal_debug.assertInitialized(); + + if (min.width != glfw.dont_care and min.height != glfw.dont_care) { + std.debug.assert(min.width >= 0); + std.debug.assert(min.height >= 0); + } + + if (max.width != glfw.dont_care and max.height != glfw.dont_care) { + std.debug.assert(max.width >= 0); + std.debug.assert(max.height >= 0); + } + + std.debug.assert(min.height <= max.height); + std.debug.assert(min.width <= max.width); + c.glfwSetWindowSizeLimits( self.handle, @intCast(c_int, min.width), @@ -703,9 +717,8 @@ pub inline fn setSizeLimits(self: Window, min: Size, max: Size) Error!void { @intCast(c_int, max.height), ); getError() catch |err| return switch (err) { - Error.InvalidValue, - Error.PlatformError, - => err, + Error.PlatformError => err, + Error.InvalidValue => unreachable, // we assert that 'min' and 'max' contain valid values, both absolutely and relative to each other, so this should be impossible else => unreachable, }; } @@ -738,11 +751,19 @@ pub inline fn setSizeLimits(self: Window, min: Size, max: Size) Error!void { /// see also: window_sizelimits, glfw.Window.setSizeLimits pub inline fn setAspectRatio(self: Window, numerator: usize, denominator: usize) Error!void { internal_debug.assertInitialized(); + + std.debug.assert(numerator != 0); + std.debug.assert(denominator != 0); + + if (numerator != glfw.dont_care and denominator != glfw.dont_care) { + std.debug.assert(numerator > 0); + std.debug.assert(denominator > 0); + } + c.glfwSetWindowAspectRatio(self.handle, @intCast(c_int, numerator), @intCast(c_int, denominator)); getError() catch |err| return switch (err) { - Error.InvalidValue, - Error.PlatformError, - => err, + Error.PlatformError => err, + Error.InvalidValue => unreachable, else => unreachable, }; } @@ -1255,10 +1276,19 @@ pub inline fn getAttrib(self: Window, attrib: Attrib) Error!isize { /// pub inline fn setAttrib(self: Window, attrib: Attrib, value: bool) Error!void { internal_debug.assertInitialized(); + std.debug.assert(switch (attrib) { + .decorated, + .resizable, + .floating, + .auto_iconify, + .focus_on_show, + => true, + else => false, + }); c.glfwSetWindowAttrib(self.handle, @enumToInt(attrib), if (value) c.GLFW_TRUE else c.GLFW_FALSE); getError() catch |err| return switch (err) { - Error.InvalidValue => unreachable, Error.PlatformError => err, + Error.InvalidValue => unreachable, else => unreachable, }; } diff --git a/glfw/src/main.zig b/glfw/src/main.zig index 66f895ea..2fd92f6c 100644 --- a/glfw/src/main.zig +++ b/glfw/src/main.zig @@ -65,10 +65,7 @@ pub inline fn init(hints: InitHints) Error!void { inline for (comptime std.meta.fieldNames(InitHints)) |field_name| { const init_hint = @field(InitHint, field_name); const init_value = @field(hints, field_name); - initHint(init_hint, init_value) catch |err| switch (err) { - Error.InvalidValue => unreachable, - else => unreachable, - }; + initHint(init_hint, init_value); } _ = c.glfwInit(); @@ -170,14 +167,17 @@ const InitHint = enum(c_int) { /// @remarks This function may be called before glfw.init. /// /// @thread_safety This function must only be called from the main thread. -fn initHint(hint: InitHint, value: anytype) Error!void { +fn initHint(hint: InitHint, value: anytype) void { switch (@typeInfo(@TypeOf(value))) { - .Int, .ComptimeInt => c.glfwInitHint(@enumToInt(hint), @intCast(c_int, value)), + .Int, .ComptimeInt => { + std.debug.assert(value == c.GLFW_TRUE or value == c.GLFW_FALSE); + c.glfwInitHint(@enumToInt(hint), @intCast(c_int, value)); + }, .Bool => c.glfwInitHint(@enumToInt(hint), @intCast(c_int, @boolToInt(value))), else => @compileError("expected a int or bool, got " ++ @typeName(@TypeOf(value))), } getError() catch |err| return switch (err) { - Error.InvalidValue => err, + Error.InvalidValue => unreachable, // we assert that 'value' is valid if it is an integer, so this should be impossible else => unreachable, }; } @@ -313,13 +313,13 @@ pub inline fn waitEvents() Error!void { /// see also: events, glfw.pollEvents, glfw.waitEvents pub inline fn waitEventsTimeout(timeout: f64) Error!void { internal_debug.assertInitialized(); + std.debug.assert(!std.math.isNan(timeout)); + std.debug.assert(timeout >= 0); + std.debug.assert(timeout <= std.math.f64_max); c.glfwWaitEventsTimeout(timeout); getError() catch |err| return switch (err) { - // TODO: Consider whether to catch 'GLFW_INVALID_VALUE' from GLFW, or assert that 'timeout' is positive here, in the same manner as GLFW, - // and make its branch unreachable. - Error.InvalidValue, - Error.PlatformError, - => err, + Error.InvalidValue => unreachable, // we assert that 'timeout' is a valid value, so this should be impossible + Error.PlatformError => err, else => unreachable, }; } diff --git a/glfw/src/opengl.zig b/glfw/src/opengl.zig index 324f5293..37fd314f 100644 --- a/glfw/src/opengl.zig +++ b/glfw/src/opengl.zig @@ -131,13 +131,16 @@ pub inline fn swapInterval(interval: isize) Error!void { /// @thread_safety This function may be called from any thread. /// /// see also: context_glext, glfw.getProcAddress -pub inline fn extensionSupported(extension: [*:0]const u8) Error!bool { +pub inline fn extensionSupported(extension: [:0]const u8) Error!bool { internal_debug.assertInitialized(); + + std.debug.assert(extension.len != 0); + std.debug.assert(extension[0] != 0); + const supported = c.glfwExtensionSupported(extension); getError() catch |err| return switch (err) { - Error.NoCurrentContext, - Error.InvalidValue, - => err, + Error.NoCurrentContext => err, + Error.InvalidValue => unreachable, // we assert that 'extension' is a minimally valid value, so this should be impossible else => unreachable, }; return supported == c.GLFW_TRUE; diff --git a/glfw/src/time.zig b/glfw/src/time.zig index ba33893d..02a01ba7 100644 --- a/glfw/src/time.zig +++ b/glfw/src/time.zig @@ -53,12 +53,18 @@ pub inline fn getTime() f64 { /// base time is not atomic, so it needs to be externally synchronized with calls to glfw.getTime. /// /// see also: time -pub inline fn setTime(time: f64) Error!void { +pub inline fn setTime(time: f64) void { internal_debug.assertInitialized(); + + std.debug.assert(!std.math.isNan(time)); + std.debug.assert(time >= 0); + // TODO: Look into why GLFW uses this hardcoded float literal as the maximum valid value for 'time'. + // Maybe propose upstream to name this constant. + std.debug.assert(time <= 18446744073.0); + c.glfwSetTime(time); getError() catch |err| return switch (err) { - // TODO: Consider whether to use GLFW error handling, or assert that 'time' is a valid value - Error.InvalidValue => err, + Error.InvalidValue => unreachable, // we assert that 'time' is a valid value, so this should be impossible else => unreachable, }; } @@ -109,7 +115,7 @@ test "setTime" { try glfw.init(.{}); defer glfw.terminate(); - _ = try glfw.setTime(1234); + _ = glfw.setTime(1234); } test "getTimerValue" { diff --git a/glfw/src/vulkan.zig b/glfw/src/vulkan.zig index 4cc854aa..d7d41668 100644 --- a/glfw/src/vulkan.zig +++ b/glfw/src/vulkan.zig @@ -219,9 +219,9 @@ pub inline fn createWindowSurface(vk_instance: anytype, window: Window, vk_alloc @ptrCast(*c.VkSurfaceKHR, @alignCast(@alignOf(*c.VkSurfaceKHR), vk_surface_khr)), ); getError() catch |err| return switch (err) { + Error.InvalidValue => @panic("Attempted to use window with client api to create vulkan surface."), Error.APIUnavailable, Error.PlatformError, - Error.InvalidValue, => err, else => unreachable, };