Lokathor/gl33

Make GlFns::load_from take FnMut instead of Fn

lunabunn opened this issue · 4 comments

GlFns::load_from currently takes Fn for the *GetProcAddress closure. AFAICT this unnecessarily restricts what the caller can do in their proc address closures, and it should take FnMut instead?

That said, can you demonstrate an actual use case of how this would allow code that isn't currently allowed.

Because function lookups should be a read-only sort of affair. If a function lookup is, by itself, modifying program state, then that's a very bad sign.

That said, can you demonstrate an actual use case of how this would allow code that isn't currently allowed.

To be frank, I have no idea. However, do note that glow hand-edits the signature of load_with (in its phosphorus-generated gl46.rs) to use FnMut because that's what its API takes. The gl crate also takes an FnMut. So the folks working on those crates must have had some reason? (Although I'm inclined to believe they just did it because they could, not for an actual use case)

Well, I wrote the current version of that code actually. I think I was just matching an old version to avoid semver breaking because glow started with gl.

However, I think gl is just being overbroad.

It's not impossible to change this but i dont think I want to without good reason because these loaders should be fully deterministic. (And if you absolutely need to track calls for debugging or whatever you could still use refcell.)