Replace WindowId From/Into u64 with WindowId::into_raw() and from_raw()#3795
Replace WindowId From/Into u64 with WindowId::into_raw() and from_raw()#3795kchibisov merged 6 commits intorust-windowing:masterfrom
WindowId From/Into u64 with WindowId::into_raw() and from_raw()#3795Conversation
notgull
left a comment
There was a problem hiding this comment.
I think having an easy converstion to/from u64 is nice because it lets you easily store the u64 in FFI.
kchibisov
left a comment
There was a problem hiding this comment.
Both are used in alarcitty's IPC.
|
@kchibisov @notgull would you mind elaborating on those things? |
|
We set a If you remove the |
Personally I would consider this still as a good tradeoff. But if this is too contentious, which I would totally understand, I would suggest to at least move away from the Just to clarify: this PR is not important, if we can't come to a happy consensus we should go ahead and close it. |
|
Given that On the other side of things I was thinking to make |
Its type safe in the sense that you know a |
|
Constructing |
This is a separate matter that we need to address globally. |
|
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 |
We discussed this in the meeting and are planning to go down exactly this route! |
73e5d1b to
1e4ab78
Compare
1e4ab78 to
efebea1
Compare
efebea1 to
e429f56
Compare
WindowId conversions to/from u64WindowId From/Into u64 with WindowId::into_raw() and from_raw()
madsmtm
left a comment
There was a problem hiding this comment.
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.
|
@notgull I think we can merge this now? Your review is still marked as "requested changes", but I think your concern has been resolved? |
resolved, since it's a name change mostly.
As there shouldn't really be a need for a user-constructible
WindowId, I assume this was used previously becauseWindowIdwas passed as a parameter?I introducedWindowId::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()andinto_raw().