Replace WindowId From/Into u64 with WindowId::into_raw() and from_raw()

[daxpedda]

As there shouldn't really be a need for a user-constructible WindowId, I assume this was used previously because WindowId was passed as a parameter?

I introduced WindowId::to_u64() as a replacement, but I'm not sure if this has any use case.
After the last meeting we decided to add conversion methods called from_raw() and into_raw().

[notgull]

I think having an easy converstion to/from u64 is nice because it lets you easily store the u64 in FFI.

[kchibisov]

Both are used in alarcitty's IPC.

[daxpedda]

@kchibisov @notgull would you mind elaborating on those things?
I have no clue about why Alacritty needs this for IPC or what storing in FFI means exactly.

[kchibisov]

We set a WINDOW_ID in the env variable and then user can use this WINDOW_ID of some window to target it from other window with IPC command (alacritty msg config --window-id WINDOW_ID font.size=10, the point here is that WindowId should be built and passed to HashMap<WindowId, WindowContext> when parsing it from the json payload on the socket.

If you remove the from we probably can use u64 instead(not really nice, since you lose type safety), but we can not do anything without a to for example.

[daxpedda]

We set a WINDOW_ID in the env variable and then user can use this WINDOW_ID of some window to target it from other window with IPC command (alacritty msg config --window-id WINDOW_ID font.size=10, the point here is that WindowId should be built and passed to HashMap<WindowId, WindowContext> when parsing it from the json payload on the socket.

If you remove the from we probably can use u64 instead(not really nice, since you lose type safety), but we can not do anything without a to for example.

Personally I would consider this still as a good tradeoff.
The to_u64() method makes absolute sense to me, but if we allow arbitrary construction from a u64 we just throw type safety out the window (no pun intended). If WindowId is supposed to represent something, we should make sure it does.
These are just my 2¢, so its all very subjective.

But if this is too contentious, which I would totally understand, I would suggest to at least move away from the From implementations to dedicated methods. Same reasons as above: to make it clear to the user that this is an explicit conversion that is not natural; it doesn't produce necessarily valid WindowIds.

Just to clarify: this PR is not important, if we can't come to a happy consensus we should go ahead and close it.

[kchibisov]

Given that WindowId is not tied to the window's lifetime it's not really type safe in any form, so limiting won't provide much benefit here and given that we don't accept WindowId in any form I probably won't change that for now.

On the other side of things I was thinking to make WindowId opaque with a method to get some kind of id() and use the rest to encode some metadata, etc.

[daxpedda]

Given that WindowId is not tied to the window's lifetime it's not really type safe in any form, ...

Its type safe in the sense that you know a WindowId is actually a window ID. Otherwise we could play around with u64s directly as well.

[nicoburns]

Constructing WindowId will be needed for 3rd party backend implementations.

[daxpedda]

Constructing WindowId will be needed for 3rd party backend implementations.

This is a separate matter that we need to address globally.
We need all types to be constructible for any backend to work, not just 3rd party, because all backends will be in their own crate. But we don't want direct winit users to see the internals or constructors of all these types.

[nicoburns]

Another use case for this would be to store the ID as an opaque type somewhere that doesn't otherwise need to depend on Winit. With the functionality I can pass my library code a u64 and get it to pass it back to me when it generates events (e.g. enable IME). It never needs to know that it's a Winit ID, and if I want to integrate it with another system (not winit) then I can.

One pattern I've seen for this that I quite liked is from the slotmap crate which calls these methods as_ffi and from_ffi which really makes it clear that creating the newtype from u64 is only supported if the u64 was created from the newtype in the first place (https://docs.rs/slotmap/latest/slotmap/struct.KeyData.html#method.as_ffi) without actually removing any functionality.

[daxpedda]

One pattern I've seen for this that I quite liked is from the slotmap crate which calls these methods as_ffi and from_ffi which really makes it clear that creating the newtype from u64 is only supported if the u64 was created from the newtype in the first place (https://docs.rs/slotmap/latest/slotmap/struct.KeyData.html#method.as_ffi) without actually removing any functionality.

We discussed this in the meeting and are planning to go down exactly this route!

[madsmtm]

from_ffi / to_ffi

To my eyes, this looks a bit like these can be used in system-level FFI, i.e. as HWND on Windows and *const NSWindow on macOS (which is not the intention). I think from_raw / into_raw are better.

[madsmtm]

@notgull I think we can merge this now? Your review is still marked as "requested changes", but I think your concern has been resolved?