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

Split out platform-independent types into a new winit-core crate #3455

Closed
wants to merge 17 commits into from

Conversation

notgull
Copy link
Member

@notgull notgull commented Feb 5, 2024

This PR implements the first item of #3433 by splitting the platform-independent types from winit into a new crate: winit-core. It contains mostly types, traits and structures from winit that are platform independent. As part of this commit, I made a handful of changes to certain types (most notably Event) to make them more platform-independent.

A brief summary of the changes made:

  • dpi and keyboard modules are copied over wholesale.
  • The errors in the error module that don't involve OsError are copied as well.
  • Some types from the window module are copied verbatim. WindowId is now a newtype over u64 instead of a platform-specific value. ActivationToken is copied and given methods to make it useful.
  • Event is given a new generic parameter to describe platform-specific key information. InnerSizeWriter is given methods to allow for instantiation. Aside from this, most things from the event module are copied verbatim.
  • AsyncRequestSerial and ControlFlow are copied from event_loop. AsyncRequestSerial::get is made public.

Aside from this, the biggest changes are just moving everything to winit-core and then re-exporting it.

I've formatted this PR as a series of changes in order to make reviewing easier. @rust-windowing/winit Please review this PR quickly. It will be difficult to rebase on top of new changes and it'll take me the whole day to remake it.

Closes #2759

  • Tested on all platforms changed
    • Ran cargo check on all platforms to make sure it builds there.
    • Did not actually run example programs on all platforms.
  • 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

@notgull
Copy link
Member Author

notgull commented Feb 5, 2024

@jackpot51 Any insight into why the Redox CI is failing with __errno not found errors?

@jackpot51
Copy link
Member

@notgull not sure, it may be trying to compile with the host compiler instead of a cross compiler?

@notgull
Copy link
Member Author

notgull commented Feb 5, 2024

It appears that the only failures are the Redox failure (is that new?) and what appears to be a bug in cargo-apk me forgetting to pass -p to cargo-apk. Both of which are unrelated to this PR.

@notgull
Copy link
Member Author

notgull commented Feb 5, 2024

No, the Redox failure is new. It looks like Redox can't compile run-wasm.

@kchibisov
Copy link
Member

Why it compiles it in the first place?

@notgull
Copy link
Member Author

notgull commented Feb 5, 2024

Why it compiles it in the first place?

Because with the new workspace layout it tries to compile everything instead of just winit

This will allow us to split off crates from the main winit crate.

Signed-off-by: John Nunley <[email protected]>
As winit no longer owns some of the types this requires some adjustments
in the Wayland backend.

Signed-off-by: John Nunley <[email protected]>
This commit moves WindowId to winit-core and replaces its inner type
with a platform-agnostic `u64`. This required some changes to many of
the backend in order to accommodate this new system.

Signed-off-by: John Nunley <[email protected]>
This requires a similar restructuring as with WindowId.

Signed-off-by: John Nunley <[email protected]>
This requires a lot of restructuring on the backend side, as we need to
accommodate for backends not being able to construct these types
implicitly. In addition, we also need to expose some new APIs on these
types to make them usable.

Several breaking changes occurred in this commit.

Signed-off-by: John Nunley <[email protected]>
Signed-off-by: John Nunley <[email protected]>
- Update CHANGELOG
- Add documentation to items I forgot to document
- Trait implementations

Signed-off-by: John Nunley <[email protected]>
Signed-off-by: John Nunley <[email protected]>
Signed-off-by: John Nunley <[email protected]>
Signed-off-by: John Nunley <[email protected]>
Signed-off-by: John Nunley <[email protected]>
Comment on lines -86 to -89
/// There is no way to detect which device that performed a certain event in
/// UIKit (i.e. you can't differentiate between different external keyboards,
/// or wether it was the main touchscreen, assistive technologies, or some
/// other pointer device that caused a touch event).
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to retain this comment in some shape or form, it is nice to know that device IDs will never be supported on iOS.

@@ -683,7 +665,7 @@ pub struct RawKeyEvent {

/// Describes a keyboard input targeting a window.
#[derive(Debug, Clone, Eq, PartialEq, Hash)]
pub struct KeyEvent {
pub struct KeyEvent<Extra> {
Copy link
Member

@madsmtm madsmtm Feb 12, 2024

Choose a reason for hiding this comment

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

I dislike this design, and think we need to discuss further how to do this sort of thing. See also #3064.

So maybe refrain from moving the event structures in this PR?

[workspace]
members = [
"run-wasm",
"winit",
Copy link
Member

@madsmtm madsmtm Feb 12, 2024

Choose a reason for hiding this comment

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

I think it's a mistake to move winit to a subdirectory, at least yet, and it also complicates this PR unnecessarily. There is especially value in having the top-level examples directory.

Let's use the include field instead to specify which files should be included in the winit package. The include field isn't needed, Cargo automatically doesn't include subcrates.

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.

Split parts of winit into separate crates
4 participants