Split Winit up into multiple crates

madsmtm commented 7 months ago

Part of https://github.com/rust-windowing/winit/issues/3367, opening to discuss separately.

Winit is designed around a single crate with a specific set of backends, which is great for users that can use that, but there is a need for other backends that are not implemented in Winit today, like GTK. Additionally, the backend-specific extensions that we do have usually constrain some other part of the design.

So it would probably be useful to try to split Winit into multiple smaller crates (though still in the same repository), to ease doing this work. The top-level winit crate would still remain.

Along with this, we'd want a way to write out-of-tree backends, and allow the user to integrate them into their application. This could probably be solved by introducing internal / "backend" traits, and keep a dyn in structures inside the winit crate.

A rough implementation plan could be:

[ ] Create a winit-core crate, which contains the common types that all backends use (like dpi and keyboard types)
[ ] Move the backends out into separate crates, which all depend on winit-core, but still have the main winit crate "merge" these backends with cfgs, not with a trait.
[ ] Make traits in winit-core, that implements the desired functionality, and move each backend to also implement those traits.
[ ] Move the main winit crate to an API that is not cfg-based, but dyn Trait-based.
[ ] Along the way introduce the winit-common crate to have shared functionality between the crates, which will be intended for winit internal usage and help writing new backends.

@kchibisov: I've tried to fairly faithfully reproduce what I felt was the important points from https://github.com/rust-windowing/winit/issues/3367, but please edit this issue description with your own.

amrbashir commented 6 months ago

Is there any interest in splitting dpi types into its own crates? There is a few crates that we maintain at Tauri that use Physical/Logical-Size/Position, and they are duplicated in each crate, currently muda and tray-icon crates but now that wry also needs, it is time to create a new crate.

We would also need to incorporate https://github.com/rust-windowing/winit/issues/2395 as we already included that change in our fork which was implemented based on https://github.com/rust-windowing/winit/pull/2148.

madsmtm commented 6 months ago

Is there any interest in splitting dpi types into its own crates?

Hmm, I guess that actually makes a lot of sense - and it might even be useful for glutin and softbuffer to use PhysicalUnit<NonZeroU32> for their widths and heights?

I've reserved the crate name dpi for now.

amrbashir commented 6 months ago

I'd be happy to help with the migration if there is a repo I can contribute to, or should it live within this repo?

madsmtm commented 6 months ago

I'll bring it up in our meeting on friday, then I'll get back to you with a plan.

madsmtm commented 6 months ago

We talked about it, and don't really think we want to pull in the extra dependency for softbuffer and glutin. That said, if there is interest in a separate crate, then we're fine with maintaining it, though we'd probably want it to stay in this repo, as that's much easier to maintain.

I think we agreed on doing the split like I described in https://github.com/rust-windowing/winit/pull/3455#discussion_r1486544289, have put up https://github.com/rust-windowing/winit/pull/3518 to move the module, then you're free to submit PRs to improve that afterwards.

ten3roberts commented 6 months ago

Hey, thank you for this initiative.

Do you think a split like this would enable us to use winit types (like keycodes) independently from winit and in external APIs. I am finding winits input and event types, especially the KeyCode to be massively beneficial to use in e.g game engine or ui library public APIs rather than re-defining the enum and using into/from.

Would this be a usecase you would be willing to support, I.e; using winit even types in the API (I am ok with breaking changes and refactors, just as long as I can use these in an API) :)

kchibisov commented 5 months ago

Given that the winit-core won't depend on anything else I don't see an issue with using it, if you want so.

n1ght-hunter commented 1 month ago

just wondering if part of this would include a way to access the event loop types and os types in a another crate. example is i am working on tray icon support for iced in a fork of winit. this reuses alot types and event loop from winit so i have built it intergrated into the winit fork. but ideally it would be possible to do this in a separate crate that depends on some of the winit crates that expose more methods. link to the fork showing what is used. https://github.com/iced-rs/winit/compare/v0.29.x...n1ght-hunter:winit:v0.29.x

nicoburns commented 1 month ago

One set of backends this would enable that would be particularly useful are dummy/proxy backends that could be used for:

Automated testing of winit-based applications (where events are synthesized)
Embedding a winit app (or many winit apps!) within a larger app (where events might originally be generated by winit but go through an intermediate level of processing before being passed back into a different winit event loop)

daxpedda commented 1 month ago

just wondering if part of this would include a way to access the event loop types and os types in a another crate. example is i am working on tray icon support for iced in a fork of winit. this reuses alot types and event loop from winit so i have built it intergrated into the winit fork. but ideally it would be possible to do this in a separate crate that depends on some of the winit crates that expose more methods. link to the fork showing what is used. iced-rs/winit@v0.29.x...n1ght-hunter:winit:v0.29.x

We have discussed this before and we do have a plan to allow other crates to offer plugin-like interfaces that users can hook up.

Definitely something we will be investigating in more detail when its time to make the change. Right now its not an issue because WindowEvent still exists and can be passed into various handlers like it works in the current ecosystem.

n1ght-hunter commented 1 month ago

just wondering if part of this would include a way to access the event loop types and os types in a another crate. example is i am working on tray icon support for iced in a fork of winit. this reuses alot types and event loop from winit so i have built it intergrated into the winit fork. but ideally it would be possible to do this in a separate crate that depends on some of the winit crates that expose more methods. link to the fork showing what is used. iced-rs/winit@v0.29.x...n1ght-hunter:winit:v0.29.x

We have discussed this before and we do have a plan to allow other crates to offer plugin-like interfaces that users can hook up.

Definitely something we will be investigating in more detail when its time to make the change. Right now its not an issue because WindowEvent still exists and can be passed into various handlers like it works in the current ecosystem.

is there any public discussions as to why this is? IMO it makes sense for winit to make public its types under a feature flag to allow third party crates to reuse some of the winit logic. especially as winit is the main window library in rust

daxpedda commented 1 month ago

is there any public discussions as to why this is? IMO it makes sense for winit to make public its types under a feature flag to allow third party crates to reuse some of the winit logic. especially as winit is the main window library in rust

I'm not following exactly where you are going with this. With current Winit, I'm unsure how exposing the types (its internals) would make any sense. Mostly their internals are backend specific anyway, so I'm unsure how anybody would make use of it.

For future Winit: we have talked about reusability a lot. There it makes much more sense, because we want to allow external backends and split the crate as discussed in this issue. So internal access is actually a requirement. However we should also make sure that the top-level crate doesn't actually allow internal access, otherwise it would all just be very terrible to use and probably unsafe/unstable.

But if you have a specific suggestion with a specific use-case in mind for current Winit, let us know!

nicoburns commented 1 month ago

@daxpedda A concrete use case I have is for a generic "text editor" library that implements a textbox widget with full support for layout, selection, rendering, and responding to input events. We would like to make use of Winit's keyboard and IME event types, but we don't want the crate to depend on Winit because:

We don't want to bring in any of the backend or actual event loop code
We would like our crate to be usable without Winit (driven by some other input system)

As far as I can see this would work great today if the relevant types were published in their own crate as all the fields are already public.

Code: https://github.com/linebender/parley/pull/88

kchibisov commented 1 month ago

The main problem with such approaches is that you can not really encode everything in single struct types, you usually want to query/build data on demand, because building it costs time.

Though, what I'm saying is mostly for keyboard input, and maybe for that part you can define your own type. The current type in winit, for example, is not covering all the use cases on dekstop, since it's hard to do shortcuts with it.

In general though, you should be able to use types in the future, since the core stuff won't depend on anything, so even if you pull a bit more traits, it'll still be relatively slim to use, so you could just pull the entire thing.

The end layout is anyway outlined in the issue description, and winit-core is the thing with just types. winit will be common glue and provide the same winit experience you have right now, and winit-wayland, etc will be backends implementing the winit-core protocol. Though, backends will have a lot more stuff and you can always downcast to them.

This also means that you can bring whatever you want as long as it'll implement winit-core, which means you may want to depend just on winit-core and require others to implement the subset you want from it.

n1ght-hunter commented 1 month ago

is there any public discussions as to why this is? IMO it makes sense for winit to make public its types under a feature flag to allow third party crates to reuse some of the winit logic. especially as winit is the main window library in rust

I'm not following exactly where you are going with this. With current Winit, I'm unsure how exposing the types (its internals) would make any sense. Mostly their internals are backend specific anyway, so I'm unsure how anybody would make use of it.

For future Winit: we have talked about reusability a lot. There it makes much more sense, because we want to allow external backends and split the crate as discussed in this issue. So internal access is actually a requirement. However we should also make sure that the top-level crate doesn't actually allow internal access, otherwise it would all just be very terrible to use and probably unsafe/unstable.

But if you have a specific suggestion with a specific use-case in mind for current Winit, let us know!

its used in the tray icon code i linked to. many people have wanted tray support directly in winit but it has been closed as not wanted(which im perfectly fine with). but the issue is tray icons are just special windows. they reuse many things from the winit library. basically i have copied the currently style of winit and have a top level struct that abstracts over a platform impl as building a tray window change by platform.

my suggestion is having an advanced feature or whatever that allow libraries to use platform dependent code. as well as in internal types

n1ght-hunter commented 1 month ago

this file shows the windows tray icon i built. https://github.com/n1ght-hunter/winit/blob/v0.29.x/src/platform_impl/windows/tray.rs

which uses these internal types

use crate::{
    dpi::PhysicalPosition,
    error::OsError as RootOsError,
    event::Event,
    platform_impl::platform::{event_loop::ProcResult, WindowId, DEVICE_ID},
    tray::TrayBuilder,
    window::{Icon, WindowId as RootWindowId},
};

use super::{
    event_loop::{runner::EventLoopRunnerShared, DESTROY_MSG_ID},
    util, EventLoopWindowTarget,
};

daxpedda commented 1 month ago

See tray-icon as well.

@n1ght-hunter looking at your tray implementation I don't believe this is a "I don't have access to internal types" issue, you need access to the internal event loop to integrate your tray in.

This seems to be an entirely different issue to me, probably more like https://github.com/rust-windowing/winit/issues/2120.

Let me know if I missed anything!

n1ght-hunter commented 1 month ago

@daxpedda didn't want to overload this issue so moved to discussion https://github.com/rust-windowing/winit/discussions/3835#discussion-6983966

rust-windowing / winit

Split Winit up into multiple crates #3433