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

Make winit completely platform-agnostic #2430

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

madsmtm
Copy link
Member

@madsmtm madsmtm commented Aug 13, 2022

Builds upon: #2421.
Related: #2093

A great thing about winit is how it supports a varied number of different platforms. I would like to make it even easier to add support for new platforms. Possible examples include: Mac Catalyst, GNUStep, X11 on macOS, SwiftUI, GTK, KMS/DRM, WinRT, and whatever new thing that vendors may decide we should start using.

We already have code in src/platform_impl/linux/mod.rs to handle the fact that both X11 and Wayland can be present on the same target. While some of the above examples can be bolted on to that existing system, I really feel we need a generic way to handle the cases where more than one platform may be available on a specific target. This PR is an attempt at that.

The basic idea is to take the x11_or_wayland! macro, extend it a bit, and then handle delegation to the desired platform at the top level instead (e.g. inside src/event_loop.rs, src/window.rs, ...).

I haven't implemented this everywhere yet, wanted to get some feedback first, but compare src/platform_impl/linux/mod.rs and src/event_loop.rs in commit e74b801 to get a feel for what this would look like.

In the end, we would end up with the following directory structure:

src/platform_impl/
    android/
    ios/
    macos/
    unix/utils.rs // Shared utility functions between Unix platforms, if needed
    wayland/
    web/
    windows/
    x11/
    mod.rs

(This would also make Wayland and X11 feel a lot less like second-class citizens).

  • Tested on all platforms changed
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation

@madsmtm madsmtm added S - api Design and usability C - needs discussion Direction must be ironed out S - platform parity Unintended platform differences S - maintenance Repaying technical debt labels Aug 13, 2022
@madsmtm madsmtm added this to the Version 0.28 milestone Aug 13, 2022
@kchibisov kchibisov modified the milestones: Version 0.28, Version 0.29.0 Feb 2, 2023
@rockisch
Copy link

rockisch commented Apr 1, 2023

While looking for WinRT support, I one way or another ended up in this PR, and although I understand there are no current plans to do it, would be nice if the new structure supported future work on it.

With that in mind, based on your initial comment, wouldn't it make more sense to call windows folder win32, so that in a possible future winit uses windows in the same vein unix is planned to be used?

On another totally separate note, since this will move quite a lot of files, maybe it would be a good idea to adopt the 'newer module system' for stuff inside platform_impl? At the end of the day it's purely a preference choice, but I feel like more people are growing used to it than using mod.rss.

@thatsofia
Copy link

Isn't WinRT/UWP deprecated now? What's the point in renaming things for a dead platform?

@rockisch
Copy link

rockisch commented Apr 1, 2023

Isn't WinRT/UWP deprecated now? What's the point in renaming things for a dead platform?

...is it? Genuine question, I'm not that into Windows GUI development, so I don't really get what Microsoft's plan is behind the whole 'App SDK' stuff.

It seems like the Windows Store only opened doors for Win32 apps last year tho, so unless they deprecated WinRT in the meantime, I feel like it should still be relevant.

@ids1024
Copy link
Member

ids1024 commented Apr 1, 2023

I think UWP is basically deprecated, "WinRT" isn't, and there's work on better integrating it with win32 stuff? As part of Microsoft's "Project Reunion".

But honestly I don't entirely follow exactly what's going on with these APIs or exactly how Microsoft is using all of these terms.

@thatsofia

This comment was marked as off-topic.

@@ -105,10 +209,72 @@ impl<T> EventLoopBuilder<T> {
if EVENT_LOOP_CREATED.set(()).is_err() {
panic!("Creating EventLoop multiple times is not supported.");
}
// Certain platforms accept a mutable reference in their API.
#[allow(clippy::unnecessary_mut_passed)]

Copy link
Member Author

Choose a reason for hiding this comment

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

Note: We intentionally don't use the platform! macro here, since this is the "starting point" where we may not know the specific platform yet.

@daxpedda
Copy link
Member

We briefly touched this on IRC, but my preference would be to move all backends entirely out of the Winit crate and into separate crates (still a monorepo).

Though I'm not against this PR and don't mind approving it, it's probably pointless if we have consensus on the separate crates idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C - needs discussion Direction must be ironed out S - api Design and usability S - maintenance Repaying technical debt S - platform parity Unintended platform differences
Development

Successfully merging this pull request may close these issues.

6 participants