glfw: Force init before use of init dependent functions

[InKryption]

Fixes #61.

This is very rough, just about barely working, and doesn't look to pretty, but tests pass on my machine now, so may as well put it up here.
That said, in doing this, I've started to dig my teeth into the error handling, and I've realized that a lot of errors in GLFW are less like errors, and more like band-aids over C's weak type system; the vast majority of instances of 'InvalidEnum' and 'InvalidValue' can be handled "implicitly" by Zig's type system.
This may take a while though.

• By selecting this checkbox, I agree to license my contributions to this project under the license(s) described in the LICENSE file, and I have the right to do so or have received permission to do so by an employer or client I am producing work for whom has this right.

[InKryption]

Note: this wasn't all done by hand. A lot of this was repetitive, so I made handy use of regex and Search-Replace in VSCode, so there's bound to be plenty of not-so-natural-looking pieces.

[slimsag]

I've realized that a lot of errors in GLFW are less like errors, and more like band-aids over C's weak type system; the vast majority of instances of 'InvalidEnum' and 'InvalidValue' can be handled "implicitly" by Zig's type system.

Totally agree with this, it's worth doing.

On the broader topic of GLFW error handling, hopefully you've also seen this: https://github.com/hexops/mach/tree/main/glfw#a-warning-about-error-handling

I'm not sure if there is a better way to represent GLFW errors, because they can be so "soft" or "hard" in different circumstances - but I wish there was, because as it stands error handling with GLFW is really easy to get subtly wrong

[slimsag]

Really liking this 👍 only thing that's left here I think is the question around what to do with platform errors, we'll need to either sort that out or remove all the TODOs associated with that before merging I think

[InKryption]

Yeah, after having actually looked through some of the source where 'GLFW_PLATFORM_ERROR' is set, it would be nigh-impossible to solve with this approach. Might still stew over a solution though.

There are still a fair few more TODO's which I'd like your input on.
In particular, what are your thoughts in regards to enum-ifying 'Joystick'? GLFW docs say they only support up to 16 joysticks (only 16 joystick IDs available), and the source re-inforces this by both asserting that any passed joystick ID be lteq to 16 (and gteq 0), conceptually mapping 'Joystick' to an exhaustive enum.

And also would like your thoughts on the functions that only have a potential for having the error 'GLFW_NOT_INITIALIZED' set that now should never return errors. We could remove the error altogether, but as you pointed out, if they decided to add any more errors to those functions, that would mean ABI incompatibility for us.
Though, now that I think about it, does GLFW have any guarantees about functions and the errors they can set? If not, then what I just said would technically apply to any function, meaning in the extreme, we'd have to normalize errors for every part of the API.

[slimsag]

On the topic of platform errors, I should note why I am so hesitant about trying to remove those. Back in 2015 with the Go GLFW bindings I led an effort to eliminate the possibility of those being returned but we ultimately got clarification from the author of GLFW that even in e.g. system misconfiguration cases, platform errors could be returned glfw/glfw#450

In particular, what are your thoughts in regards to enum-ifying 'Joystick'? GLFW docs say they only support up to 16 joysticks (only 16 joystick IDs available), and the source re-inforces this by both asserting that any passed joystick ID be lteq to 16 (and gteq 0), conceptually mapping 'Joystick' to an exhaustive enum.

Fully agree with this. I actually thought I had enumified these, but I guess not. One important note, the enum value should be c_int I think to keep interoperability with other libraries that expect to be able to receive a Joystick ID.

And also would like your thoughts on the functions that only have a potential for having the error 'GLFW_NOT_INITIALIZED' set that now should never return errors. We could remove the error altogether, but as you pointed out, if they decided to add any more errors to those functions, that would mean ABI incompatibility for us.

Though, now that I think about it, does GLFW have any guarantees about functions and the errors they can set? If not, then what I just said would technically apply to any function, meaning in the extreme, we'd have to normalize errors for every part of the API.

I think in cases where we have eliminated all documented errors the GLFW function could return, we should indeed remove the error return.

GLFW's guarantees around what errors a function may generate are somewhat aspirational, somewhat concrete. We faced the same issue with Go GLFW bindings back in the day, and you can see here that GLFW's author suggests the documentation today is generally correct glfw/glfw#361 (comment)

I think that there will definitely be cases where GLFW changes the errors a function may generate in a minor or patch release version, for example if using a new Wayland API to improve support for something they really have no choice. That said, I am content with us being generally aspirational here as well and trying to focus just on the documented errors and, when a breakage does change on the GLFW side, us also breaking ABI compatibility and either documenting such small changes can occur or releasing a new major version each time.

I'd rather have a nice API that sometimes require users update their ABI usage, than have an API that can return everything under the sun.

[InKryption]

On the topic of platform errors, I should note why I am so hesitant about trying to remove those. Back in 2015 with the Go GLFW bindings I led an effort to eliminate the possibility of those being returned but we ultimately got clarification from the author of GLFW that even in e.g. system misconfiguration cases, platform errors could be returned glfw/glfw#450

That's unfortunate to hear, but good to know. Maybe farther in the future, mach could have patches over GLFW to fall more in line with Zig's error handling philosophy; but that would be a ways off.

Fully agree with this. I actually thought I had enumified these, but I guess not. [...]

I went ahead and did the enum-ification, though I opted to just make the member 'jid' into an enum, to avoid having to fix all the references to 'Joystick' as a struct file. Maybe that's its own smaller PR.

[...]
I'd rather have a nice API that sometimes require users update their ABI usage, than have an API that can return everything under the sun.

I share the sentiment wholeheartedly, though your comment concerning error-denormalization made me think ABI was a bigger concern than I had realized (not to say that it isn't a concern at all now). But taking that into account, I went ahead and removed the error union returns from functions that shouldn't return any errors. I may have missed some, but unsure if you want to block the PR on tracking those down (I'm happy to drudge around).

[slimsag]

That's unfortunate to hear, but good to know. Maybe farther in the future, mach could have patches over GLFW to fall more in line with Zig's error handling philosophy; but that would be a ways off.

I think this dips into "rewrite GLFW" territory 😅 I'd refocus that effort on seeing if we could improve GLFW upstream, and steer clear of patching GLFW on our side as much as reasonably possible (UB is a clear exception here.)

your comment concerning error-denormalization made me think ABI was a bigger concern than I had realized

Sorry that was ambiguous.. When I wrote "might be worth discussing if this is a good idea or not" I really did mean discuss :D I just hadn't thought through the problem enough to think if that was a good idea or not.

Ideally, we can keep PRs on the smaller side in general - I'm super happy with more iterative development (even if that means littering TODOs throughout the code that we address in follow up PRs), primarily because that makes it easier to have these discussions.

e.g. ideally a PR titled this way focuses on one problem (force init before use of init dependent functions) and all other work ends up in other PRs so we can discuss merits of changes individually.

Anywho, this PR looks pretty great in its current state to me - if you want to mark it as ready for review I'm happy to merge now. If you want to, feel free to rebase and I'll merge all the commits - otherwise I'll squash merge to eliminate the merge commit.

[InKryption]

Squashing sounds good to me.

[slimsag]

Seriously, thanks so much for the work you are doing :)