winit-web: skip_recreation_check added

[rickvanprim]

• Tested on all platforms changed
• Added an entry to the changelog module 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

Tested locally against a web app with two separate Canvases with their own winit+wgpu contexts in a Dioxus web app and unmounting+remounting the components (destroying and recreating everything in the process).

[daxpedda]

The reason this is in place is for consistency between platforms. You can use spawn() to get the desired behavior.

Is there an issue with using spawn() for your use-case?

[kchibisov]

The issue was mentioned in matrix, but basically we mark it for recreation as soon as you create the event loop, but spawn clears only after spawn/etc. Users can not run loops in parallel.

I'd generally lift the restriction on the event loop creation from at least the backend itself.

[daxpedda]

Ah, I see, my bad, I didn't read the OP carefully enough as well.

I'd generally lift the restriction on the event loop creation from at least the backend itself.

Where else can it be enforced then? "at least" implies that it stays somewhere else, or did you mean you want to remove the restriction altogether?

---

I'm still of the same mind here: for consistency we shouldn't allow parallel event loop creation by default. Allowing it for spawn() or adding a backend specific escape hatch is what I believe we should do instead.

[rickvanprim]

How about something like allow_multiple: bool on winit-web::event_loop::PlatformSpecificEventLoopAttributes which could opt out of interacting with EVENT_LOOP_CREATED (both setting and clearing it)?

[daxpedda]

That does sound fine for me.

However, I think I would prefer to guide users towards spawn() if at all possible for non-crossplatform behavior. Do you have a use-case specifically for using run()?

Also while we are at it, whats your use-case for running Winit in parallel?

[rickvanprim]

I'm not using run(), I'm using spawn_app(). I'm still not seeing a spawn() type API that let's me run multiple ApplicationHandlers in parallel. If I'm overlooking a solution that already exists, I'd be happy to switch to that 🙂

My use case is I'm trying to run multiple instances of what amounts to a game client side-by-side embedded in a Dioxus web app (so winit is not the root of the conceptual application) to represent each player's perspective. I could rework my ApplicationHandler type to be long lived and able to add/remove Window/Canvases dynamically as their respective Dioxus components are mounted/unmounted. However being able to isolate the full winit context and run multiple instances feels cleanest and lends itself towards being able to share the ApplicationHandler type between this "editor" usage and the normal "game/engine" usage.

Hopefully that makes sense. I'm happy to provide more info 🙂

[daxpedda]

I'm not using run(), I'm using spawn_app(). I'm still not seeing a spawn() type API that let's me run multiple ApplicationHandlers in parallel. If I'm overlooking a solution that already exists, I'd be happy to switch to that 🙂

Apologies, my suggestion is that instead of adding an escape hatch to run() (via PlatformSpecificEventLoopAttributes), we can allow spawn_app() to run event loops in parallel by default. Right now spawn_app() allows re-creation by default, we should also make it allow parallel event loops by default.

I'm sorry for using the wrong method names, I'm a bit behind on all the renamed stuff.

Hopefully that makes sense. I'm happy to provide more info 🙂

I would recommend you to stick with a single event loop to reduce the overhead. However, the overhead is probably insignificant in comparison to a whole game and in comparison with the advantage of cleaner code.

Sounds cool though and makes sense to me!

[rickvanprim]

Apologies, my suggestion is that instead of adding an escape hatch to run() (via PlatformSpecificEventLoopAttributes), we can allow spawn_app() to run event loops in parallel by default. Right now spawn_app() allows re-creation by default, we should also make it allow parallel event loops by default.

My only concern with that is that there is an overlap there where this "could" snag. In practice this seems pretty unlikely and presumably easy to work around, but the following would fail:

let event_loop_1 = EventLoop::new().unwrap(); // recreation flag set
let event_loop_2 = EventLoop::new().unwrap(); // panic
event_loop_1.spawn_app(App1::default()); // recreation flag reset here
event_loop_2.spawn_app(App2::default());

If you're good with that, I'll update this PR to just clear the flag inside of spawn_app() instead of on drop 🙂 Or however you feel this is best implemented

[daxpedda]

I'm thinking that the best implementation might be to make spawn_app() entirely ignore recreation flags and such instead of resetting it.

---

I was surprised that the current behavior was implemented this way in the first place: resetting the recreation flag instead of just ignoring it. So I went a back to the original PR #2897 and found #2907. Looking at #3311, it is probably best not to proceed here.

I apologize for stringing you along here, I completely forgot about all this. So unless we re-assess our position here, I'm currently in favor of not allowing multiple event loops in parallel.

[rickvanprim]

I'm not sure how handling CloseRequested is at odds with parallel EventLoops? I expect that multiple things on a given webpage could be registering to block navigating away/closing, including one or more EventLoops.

Anyways for my purposes, should I pivot to the long running singleton EventLoop, or wait for others to weigh in?

[kchibisov]

Is it really limited if you create the web loop without going through the top-level event loop? I'm fine with e.g. winit-web to e.g. ignore all of that, but keep the checks around winit::event_loop, thus way we won't really change much.

[rickvanprim]

@kchibisov, assuming you're talking to me, are you suggesting I just directly create a winit-web::event_loop::ActiveEventLoop and call run() on it? Sounds fine to me. Currently run() is only pub(crate) though. Or do you mean something else?

[kchibisov]

I meant to use winit-be::event_loop::EventLoop::new() and using it, and so also using direct methods on the web backend. Like the general limitation of the winit crate should be adjusted together with all the backends to preserve semantics, but backend itself may allow use you want.

[rickvanprim]

Gotcha. Sounds good to me 🙂

[daxpedda]

I'm not sure how handling CloseRequested is at odds with parallel EventLoops? I expect that multiple things on a given webpage could be registering to block navigating away/closing, including one or more EventLoops.

The idea of allowing parallel EventLoops only makes sense to me if they don't interfere. In a case like CloseRequested they do, not necessarily because of blocking the interaction, but if we implement it before Firefox removes the whole B/F cache interaction (see #3311), we need a way to control setting the beforeunload event.

But there are other cases that would interfere in the future. When we change the window/surface model Web can implement more top-level methods, like changing the window title or window size. Moreover, the plan was in the future to replace multiple Windows in Web with subviews. So the only Window you can create on Web is the one covering the screen.

All of these things would significantly interfere with each other. Moreover I'm confident that parallel EventLoops as a feature is a convenience one and won't block anybody from doing anything specific.

---

Anyways for my purposes, should I pivot to the long running singleton EventLoop, or wait for others to weigh in?

I predict you should pivot. But as a contributor I've not contributed to Winit in a long time and I don't want to pretend I've the ultimate say here.

I meant to use winit-be::event_loop::EventLoop::new() and using it, and so also using direct methods on the web backend. Like the general limitation of the winit crate should be adjusted together with all the backends to preserve semantics, but backend itself may allow use you want.

This could be a viable solution. But it needs to be something else then winit_web::event_loop::EventLoop::new() with the interference I described above in mind. Something like SubViewLoop, or until sub views exist, WindowLoop.

[rickvanprim]

@kchibisov shall I close this PR?

[kchibisov]

If you can achieve what you want by calling to winit-web directly, then I think yes?

[rickvanprim]

I cannot achieve that presently. I would need to expose something on winit-web. If you're fine with that, I'll take a shot at updating this PR.

[kchibisov]

I think if the PR is directly against winit-web and not winit-core/etc it could be easier to move with it forward. I'm not web maintainer so can not say much about it, even though, I don't like that we have such recreation stuff for other backends as well, but some really need it, so it's in place for everyone as a lowest denominator.

[rickvanprim]

PR updated. If you don't want this touching winit-core at all, I can get rid of the EventLoopBuilderExtWeb stuff and just directly construct winit-web::event_loop::PlatformSpecificEventLoopAttributes and call winit-web::event_loop::EventLoop::new() with it.

[kchibisov]

PR updated. If you don't want this touching winit-core at all, I can get rid of the EventLoopBuilderExtWeb stuff and just directly construct winit-web::event_loop::PlatformSpecificEventLoopAttributes and call winit-web::event_loop::EventLoop::new() with it.

Yeah, I guess the point is to have it entirely in winit-web.

[rickvanprim]

Just confirming, the usage of this will be:

winit::platform::web::EventLoop::new(&winit::platform::web::PlatformSpecificEventLoopAttributes {
  skip_recreation_check: true,
})?

Instead of the extension trait form:

winit::event_loop::EventLoop::builder().with_skip_recreation_check().build()?

This creates some other friction, because now I have update some method calls like event_loop.create_proxy() to event_loop.window_target().create_proxy() as the platform specific EventLoops evidently expose a slightly different API from the core one.

I've updated the PR removing the extension trait stuff. Let me know if you'd like any other changes 🙂

[daxpedda]

Just making sure my disapproval is not overlooked at a later point.
I hope I made myself clear in #4276 (comment).

[kchibisov]

Isn't it fine if it's a specific backend behavior that you should generally not rely upon, but just how the current backend may work? it's not a winit-core API for that reason, or winit API.

[daxpedda]

Isn't it fine if it's a specific backend behavior that you should generally not rely upon, but just how the current backend may work? it's not a winit-core API for that reason, or winit API.

What does "should generally not rely upon" entail? Do you mean just because its backend specific?

In my explanation I pointed out why parallel event loops will conflict with each other in the future on Web, backend specifically.

[kchibisov]

if in the future you would be able to achieve the desired functionality with the single event loop then I don't see an issue.

I'm not forcing any change here in particular though, just trying to find a compromise, since it's not the first time there's such request for web.

[daxpedda]

if in the future you would be able to achieve the desired functionality with the single event loop then I don't see an issue.

OPs motivation is already achievable with the current API. The requested change is just for convenience. There is no plan to support running event loops in parallel on Web.

I proposed a hypothetical API for parallel event loops on Web in #4276 (comment). However, I believe this would require not-insignificant amount of work and code to maintain. So IMO, unless there is a good use-case, we should not add that feature.

I'm not forcing any change here in particular though, just trying to find a compromise, since it's not the first time there's such request for web.

This request is about parallel event loops not about event loop re-creation, which is already covered with spawn_app(). I'm not aware of other user requests for parallel event loops on Web.

It would be good to know what other use-cases are out there.

[kchibisov]

I think they just don't want to create them in sequence or IIRC the issue is that you can spawn_app if the previous one finished, so it seems like running side by side with spawn_app is not possible?

[daxpedda]

Check #4276 (comment) for the use-case.

But indeed, event loops doesn't support being run in parallel. But there is no use-case for running the event loop in parallel except for simpler implementations, ergo convenience. Multi-window API is harder than ApplicationHandler.