Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement From<u64> for WindowId and vise-versa #2351

Merged
merged 1 commit into from
Jul 2, 2022

Conversation

kchibisov
Copy link
Member

This should help downstream applications to expose WindowId to the end
users via e.g. IPC to control particular windows in multi window
systems.

  • Tested on all platforms changed
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

@kchibisov kchibisov added this to the Version 0.27 milestone Jun 29, 2022
This should help downstream applications to expose WindowId to the end
users via e.g. IPC to control particular windows in multi window
systems.
@maroider
Copy link
Member

maroider commented Jun 29, 2022

I'm not sure if casting to and from u64 is entirely unproblematic here.

Windows' window handles are usize (technically the headers define them as a pointer, but they're not actually pointers). 32-bit Windows installs are admittedly likely quite rare, and I don't believe Windows 11 has a 32-bit edition, but this still feels a bit iffy to me.

It also looks like X11's window IDs are always u32s, X11/Xmd.h defines CARD32 as unsigned int on 64-bit systems, and unsigned long on 32-bit systems, although x11-dl has defined CARD32 as c_ulong, which seems like it would be unsound (or at the very least incorrect) to me, as c_ulong appears to be 8 bytes long on 64-bit Linux.

@kchibisov
Copy link
Member Author

Windows' window handles are usize. 32-bit Windows installs are admittedly likely quite rare, and I don't believe Windows 11 has a 32-bit edition, but this still feels a bit iffy to me.

That's why we use u64, so it can fit any possible value into it? The point is to have ability to convert WindowId to something, send it via some IPC mechanism, and then again receive it and create WindowId out of it to access something like HashMap<WindowId, Window>.

I'm not sure if casting to and from u64 is entirely unproblematic here.

It should be, since you can't do anything with it given that WindowId is an opaque struct for which only checking for identity is stable, it's not like you can build anything out of it nor it's guaranteed to work if you cast it to anything.

@chrisduerr
Copy link
Contributor

Yeah at the very least you should be able to convert to u64. Converting from it might be more controversial but I don't think it should be that big of a deal either.

Copy link
Member

@MarijnS95 MarijnS95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Android, I guess this is okay. Hopefully we can support multiple activities == multiple windows at some point.

@kchibisov kchibisov merged commit cb41c58 into rust-windowing:master Jul 2, 2022
@kchibisov kchibisov deleted the windowid-as-u64 branch July 2, 2022 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants