-
Notifications
You must be signed in to change notification settings - Fork 908
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
Winit API redesign #3367
Comments
As I've stated in chat I don't think that a trait based system would be the solution here, as it doesn't solve a number of problems that the current system has:
In the past I’ve proposed replacing the event loop with a "block-on" call that blocks on a future, and then having most of the functionality take place via Many of the problems mentioned are either solved or solvable with I agree with splitting out a types crate and different backend crates, as well as most of the other proposals here. |
I have a demo of such a system here: https://github.com/notgull/project-keter For the most part I've only created the cross-platform integration test harness. However it should demo some of my ideas here. |
That's how macOS actually works. You're getting called with a state. You also setup methods macOS calls to you, so it's just a trait based. You can't really model any
IDETs solve that. You can't even add event without a breaking change now, and no, non-exhaustive is not a solution to any of that.
I mean, you're free to make backend async/await internally, unfortunatelly I don't want to deal with endless |
I like the idea @kchibisov but know very little of the backends. Regarding multithreading, the only thing winit needs in my opinion is the ability to wake the event loop from another thread (i.e. |
Any event loop that can be woken (I.e. any reasonable event loop) can be turned into an With the exception of Windows, in my opinion most of the platforms map better into
Fair enough.
In
It’s possible to prevent the reactor from making progress until some event has happened. I use this in |
While I don't have much direct suggestions in terms of how a future winit api needs to look like, I would like to point out a few things that we've encountered over the past few years of using winit. 🦣 General major API level changesPlease consider that winit is a beacon of the rust ecosystem and any changes made to the public interface of this crate typically end up costing quite a few man-hours downstream in terms of changes us downstream users need to make as well. I know winit doesn't have a 1.0 release at this point, but it's already quite widely used and so the impact of seeminly minor changes have a massive impact. For example in the 0.29 release the PR that changed the Key & KeyCode enum (#3143) ended up costing us quite some time refactoring code we already had and code that was already working and tested in our application; effectively costing us some "busy work" to keep up to date. Obviously this is kind of expected when you want to keep up to date with latest dependencies, so this is definitely not a call to stop all changes to public apis. However, it is an ask to be mindful of the changes proposed and their downstream cost. 📱
|
async/await would be quite challenging to integrate nicely with Bevy, and similarly, would be quite aggressively opposed due to impacts on compile times. |
I am strongly in agreement with @Jasper-Bekkers that winit changes at this point should consider the large amount of downstream work required to adapt to them. Please consider the most minimal changes required to resolve each of the issues listed, I doubt a complete redesign is required. The 0.29 changes were particularly painful for COSMIC and Redox OS, I am now using a forked 0.28 to support things already in 0.29 like Redox support and window resizing with CSDs. I had to backport those changes, quite painfully, because iced has not yet adapted to 0.29. And before such projects have even transitioned, to talk about more breaking changes, fills me with unease. |
Thank you for the feedback here. I think that, if we rewrite the API here, it should be the intention that the new API will be the v1.0 API.
This can be done without linking to
Just because networking primitives have been designed in this way doesn't mean
The current If the overwhelming consensus is "if
This comes at a hidden cost for SDL2: it relies on a number of very fragile hacks that make its Android, Windows and macOS support very precocious. In fact, SDL3 is moving to a controlled callback model similar to what Even this restricted model doesn't work anymore for |
Addressing OP
I totally agree these are issues with the current design that must be addressed one way or another.
Application as a traitGenerally speaking, I'm against the trait based application design, for the same reasons pointed out in #3367 (comment): I would very strongly prefer Winit to be unopinionated. That said, Winit should allow, and maybe also encourage, a higher-level design that is "nice", where a trait based application sounds fantastic. Indeed a separation of low-level, I don't exactly see how the trait based design addresses any of the issues from "Motivation", except the "No way to exclude functionality" for which I believe there are other, possibly less appealing, solutions. OwnershipThe idea to let Winit hold ownership over handles ( So far I can only see two solutions to this problem:
I see this as the biggest factor requiring significant API changes and I don't see how we will bring this unto consensus unless we put significant effort into documenting each backends limitations around this and figure out together how we can proceed. Unless of course we just pick the 1. solution. Forced Event FeedbackThis can obviously be addressed by the trait based system very conveniently. But this could be easily addressed, even though much uglier, with the current API. Again: I'm aware that this might make the API quite ugly, which is why I'm in favor of keeping a very unopinionated and low-level API and having a "nice" high-level API separate (e.g.
|
It really depends what you're doing, but the current design really doesn't work with the way Android expects you to work, and also doesn't work with other stuff. I don't see why you won't be able to do what you did before though, like it doesn't really matter whether you borrow values into the massive closure or whether you put something explicitly in the struct (treat it as closure capture list). I'm also not changing the way current event loop API works, you'd be able to still use
I won't go with async/await either, I'll stick to traits since they work, yes winit will take more control, but not asking for more control is already a big issue since you must create window inside the
It doesn't work for anything more complex sorry, I can't add APIs that are a bit more complex (e.g. popups/subviews/proper dnd). I also have no time and desire to maintain backends in tree that we can't really test. |
I'd also like, but it clearly doesn't work, and I've tried various things over 5 years, and it's just getting worse and worse. But I have an option to just part ways with winit and do something else.
Some handles are provided by event loop and only relevant when it's being executed. It doesn't work already with macOS.
Yes, and that's what I want to avoid, I don't want to deal with any of that, sorry. imagine having
So you want me to pass closures which are limited because they must be Like I tried all of that and gave up because it's a giant mess. You need to develop
I can't have it on Wayland that way, so it doesn't work. Like the only way to remove the batching and keep maintainable code is to do what I do with traits.
It's actually minimal, you just move your event handler into function on a trait, and capture list into structs. Pretty minimal in my opinion.
Why winit should force multithreading on everyone? Why should we have so many |
That's what I want to do as well and then the users may |
As a user of winit in games, the proposed trait API seems inoffensive, and would probably take me about ten minutes to port to. It and the status quo can be implemented in terms of each other, so the amount of disruption has a hard limit. My application code tends to closely resemble the trait pattern in practice anyway, since inlining all your logic into one gigantic callback is a really poor way to structure a project. The prospect for middleware (e.g. GUI toolkits) integration with winit with reduced complexity/buildtime/semver hazard, is appealing, but I think independent of the question of traits vs. mega-callback vs. etc. Similarly, out-of-tree backend support would be cool, but doesn't obviously need to involve changes to the application-facing API. Perhaps these discussions should be separated? I disagree with the suggestion that the proposed application-facing traits are more opinionated. There's very little that you can usefully do outside of a I also disagree that async is particularly costly: like with most Rust features, you pay for what you use. However, it's not obvious why async support must be built-in to be viable, and for maintenance's sake winit must minimize scope creep. |
Yeah, it's separate, but it needs a change in design as in have extension traits in one way or another. The current API kind of wants you to pass event and for some extensions you may want to have a
It does change, because you need
Yeah, that's my point as well, you're doing that already, I just take away the ability to move
Exactly, and that's why I'm going with extensions traits, so backends can independently provide provide special functionality having the |
Having a way to write middleware can be good if you want to have accessibility features or plug extra debugging(but don't want any of that in release builds). I don't see why you shouldn't be able to plug It's just allows the scope and core be limited and clean, while allowing extensibility, and allows to This is included here because the one common way to approach this issue is to develop interfaces (traits), though it's a different set of traits because one part is Application facing and the other is backend facing. But given that backend my need to extend events you can't really do that with the single massive closure, so you need to split and have trait-like callbacks, which leads that you need closure and trait based callbacks at the same time and you'd like need to share state, it's just not nice. |
I don't follow. Can you give a small example? Naively, I'd imagine the core <-> backend interface could be 100% isolated from the core <-> application interface, with the core strongly encapsulating calls into backend traits. This is how Quinn abstracts over async runtimes, for example. |
With the suggested API it's encapsulated yes, with the current API it's not and I don't see how you'd approach that, unless you put event into I think an example is when the backend wants to have a backend specific event for some backend specific functionality, like In the current state of things in winit if you add something like that you'll infect all the backends (see touchpad magnify, or memory warnings on Android/iOS). So it does matter what you pick in core, since you really don't want it to expose backends unless you cast to backend stuff. |
I figured the cost of having rarely-used events in core is comparatively low, but I don't have much context to make a strong judgement there. How would the core pass an event type not known to it from a backend to the application? I'm leery of type-level solutions to this problem, as they tend to complicate portable code which wants to opportunistically leverage platform-specific features, moreso even than feature gates. |
If you know which backend you're using you can have a The other part is requests from the application to core and I'm not entirely sure what to do with that side yet, surely you can also do So I'd say events are fairly simple and requests is the only thing I'm not sure how to plumb correctly. |
Sorry, what? Rust doesn't have C++-style dynamic casts, and trait methods to downcast to a concrete type would have to be defined in core. You could do something with |
Hm, yeah, you'd need something like that https://github.com/bch29/traitcast . Which involves type map to bring RTTI. So the option is to have all extensions listed on |
I see pros in both the trait interface and the async interface, but I can also see why this could be awkward to implement for the users. Winit has two main use cases I would say:
A real time App like a game needs to do much more stuff than just the event loop, there an async/await structure might really be a good solution. For @alice-i-cecile 's comment for bevy, I have to say that the current runner api is a monolithic big thing that controls the whole Normal guis on the other side are more strictly bound to the event loop. In my 5 attempts for a Ui framework I could imagine both a async/await and trait-based api to be used. Im not too familiar with the insides of winit and also not with the old winit .19, but when it's based on a simple poll_events like structure, why couldn't we have both a trait-based and an async/await api, and when it's not too messy we could also for the applications who don't want to upgrade or really prefer the old api, try to also implement that? Never the less, I have some questions to the requested API designs: async/await
trait-basedI forgot ._. |
just fyi How you structure your game engine is also subjective and you clearly can have a let mut event = Vec::new();
loop {
event_loop.pump_events({
event.push(event);
});
for event in events {
// Run
}
} But be aware that such APIs don't work on e.g. ios because they take over the control, but you may not care here. |
But when |
no one said that we're doing async/await, I replied to your |
@Jasper-Bekkers, in neovide I started to work on a direct3d backend with waitable swapchains neovide/neovide#2215, because after numerous of hours/days trying, I could never get windowed opengl to not drop visible frames on Windows. I am still using the winit event loop to drive it though through this https://github.com/neovide/neovide/blob/6e92f27544d3275be4fa9a2135cf8c9e43361034/src/renderer/vsync/vsync_win_swap_chain.rs, which emulates the native winit |
I'd just let it on you so you'd not e.g. crash or anything, you'd just render one extra frame with wrong scale, but that's about it. No sync except that you'd have explicitly confirm the changes to the surface, like all the issues are due to winit doing it for you and not the other way around like it should.
Changes are internal and don't break the user API except that you'll have to move |
PrefaceI'm just summarizing the current discussions and conclusions and adding my 2¢ on top. This might not contain any new information for some, but will try to include the user-perspective in addition to the maintainer one. Current IssuesUser Feedback on EventsThere is currently no good way to supply or require user feedback from events. This would need some return type in the closure, which is hard to accomplish with the current API as not all events require feedback. Currently this is done by providing a writable type in the event, see Internal usage of
|
I've already replied to that, Like #3367 (comment) explicitly address it. The new |
This is indeed outlined in my post.
I have more explicitly compared both approaches now in the post. The point being is that exposing The fast path can still be accessed through There are some tradeoffs for each of those proposals which is why I specifically differentiated them. But these tradeoffs are semantic, the outcome should be the same. Let me know if anything is unclear or if I missed anything. |
The only point that is missing is that we need to model a failure in destructor, which you can't really do when you own the type, such failure is relevant when you have a lot of weak handles, etc. It can also help with cleaning up resources between the run-on-demand runs and in cases where we want to borrow the As for the backends, at least most of them store the window on event loop window target or similar. Also, having calls to accept |
I think you are talking about when the user owned window type actually owns the window state. If we are still talking about why the "No Window Type" proposal might not be necessary, then there is a misunderstanding here. What I'm trying to say is that even without the "No Window Type" proposal, you can let your actual window state live in the event loop, as outlined above.
It is not! What I am saying is that if you take I think I will rename the "No Window Type" proposal to "Direct Window Access". Because this isn't about window ownership, which is covered by "Nothing Outlives the Event Loop". |
My app has several layers that handle winit events. These layers are in different crates, they are like modules that receive the winit events and handle certain parts of the windowing, input, UI, rendering or the game logic. |
@cybersoulK yeah, there will be solution for that, however, keep in mind that some events are |
An update: @daxpedda, @kchibisov and I have discussed this a few times now, and are slowly itching towards an idea for a future This release is meant to be an iterative step that doesn't actually resolve most of the problems outlined here, but instead focuses on providing an upgrade path for our users from the closure-based design, to the trait-based design. The point of this is to gather real-life feedback, and account for missing functionality. The existing closure-based API will still be available (although deprecated) in this version, so if you find that the migration is not possible for some reason or other, we will have a place for you to submit that feedback, and you can continue going about your business with the old API. |
Additionally, as we've agreed to do this work iteratively, I've re-read all the comments on this issue, and attempted to open new, separate issues for each proposed change. The relevant issues are:
I believe I've included the major points from this discussion, but I've definitely missed something, so please look them over, and make sure that the points and ideas you've raised here won't be forgotten. Since this issue has now accomplished its goal of communicating and getting the maintainers on board with a new direction for Winit, I'm going to close it, since it is not really actionable, and the discussion in here is all over the place and impossible to follow. Check out the milestones to follow what work will be done when. |
Discussion summary.
Motivation
We need feedback for events and immediate reactions
The current API doesn't really work when you must get some values from the users
in reply to some events, because users has a clear choice to not do so. In some
cases it could lead to undesired behavior and even crashes. An example of such
thing could be:
Back
button/KeyCode
on Android #2304 (require saying thatyou've handled, otherwise you'll block everything
window creation)
The getting feedback part could be solved vi abuse of
Mutex
but it's not likeyou can force users into doing correct cross-platform code, which actually
behaves the same.
There are more issues due to current massive callback approach, I just picked
few of them.
Monolithic design
In the current form winit is monolithic meaning that if the users want to use
winit they must include the entire crate introducing a lot of dependencies
and increase in compile time.
This is not great since GUIs don't really need all the winit or all the
backends. We also have a situation where you use winit or die, which is also
not great, so having a crate which provides mostly types could benefit.
Other issue of monolithic design is that winit is growing on its backends and
there's a demand to support niche platforms. Unfortunately, we can't provide
good support for all of them with the current maintainers we have. And some
platforms we can't event test reliably (e.g. redox). To solve this
winit should allow writing out-of-tree backends.
Interior mutability mess
Winit historically marks the
Window
asSend + Sync
, while it's actually goodto write multi threaded code, internally it all goes back to event loop thread,
this is true for Windows, Wayland, Web, iOS.
If you even tried contributing into early winit it was event worse on interior
mutabilty and the use of
Mutex
, which were not need in99%
of the time.Objects actually have lifetime
Window
and other objects created while event loop is running actually haslifetime and generally can't be used when the event loop is paused.
This issue is pretty clear with the
run_on_demand
sort of APIs where we mustdestroy everything between the runs.
This issue is usually solved with
lifetimes
orWeak
objects. TheWeak
objects may require some sort of
Upgrade
in the API and doing so inevery call possible is strange. While the
lifetimes
sounds scary here,it's a matter how you look at them, since one option is to give a
reference into the resource owned by winit, thus you can use object
only via the event loop is running.
The key could be some
Id
type and you may fail to get the resource ifit's no longer available.
The relevant issue #2903
No way to exclude functionality
The API surface is huge and we need a reliable extension system, so
the users won't end up with functionality they likely don't need.
One approach could be features, but it'll make the testing really hard, since
you'd need to test every per-mutation, the more natural approach would be
IDETs
. But they don't work with the massive callback design or declerative async API.
Proposed solution
To address the raised issues the proposed solution is being worked on
in https://github.com/rust-windowing/winit-next. It's not even remotely
complete but it should show the vector of development.
The user facing API is based around the
Application
trait, whichmay be extended,
and the event loop handler.
The backend facing APIs uses the traits like
An example could be seen at the winit-next
repo
What is not clear with the current approach
Multithreaded is not clear, but it's actually solvable. For example we may
have a way to enqueue callbacks into event loop thread or have types which
can perform rendering related requests from non-main thread.
However, we won't be able to provide all
Window
APIs on non-main thread, butit is like that on most backends, except X11 and recent Wayland, so having
explicit API to queue callback which will get the relevant state will
work around the same to how it worked before.
We could also have a
Weak<View>
to pass to other threads, though giventhat most drawing libraries
Surface
types areSend
nothing stopsfrom sending the render target and some fences to ensure that the window
is not dropped in-between of the rendering.
Alternatives
Not aware of other options and given that most toolkits(outside of rust) do
generally similar things, it's not like their approach is bad.
What will not change
Fortunately, not everything will change, the things which likely stay
the same will be:
run
APIs, they work fine and there's no general issue withthem.
The text was updated successfully, but these errors were encountered: