[Draft] Per-window event handling and enable creation of windows without an event loop for embedded windows

[l0calh05t]

While creating child windows has become possible with platform-specific APIs (#93), use of winit for embedded windows such as plugin windows where external code provides a container window (see #159) remains blocked by the fact that such applications control the main event loop in external code, therefore winit::EventLoop cannot be used in such cases.

Therefore, I suggest adding WindowBuilder.build_without_event_loop (no equivalent Window.new_without_event_loop as this only really makes sense when also using WindowBuilderExtWindows.with_parent_window or other similar platform-specific functions). Furthermore, as lack of an event loop prevents calling the event handler passed to EventLoop.run, I suggest adding Window.set_event_handler to set an optional per-window event handler. This should make it possible to remain compatible with the existing API while gaining the ability to support embedded windows.
An added benefit is that it should make the handling of multi-window applications easier, as the user wouldn't have to perform his own event forwarding depending on window_id.

Currently, this PR is a suggestion for a possible API extension without an implementation. Suggestions for improvements to the API and ideas on how this could be implemented are very welcome.

• Tested on all platforms changed
• cargo fmt has been run on this branch
• 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

[goddessfreya]

Dammit, I forgot to hit submit review.... sorry.

[goddessfreya]

After some pondering, I'm 80% certain this function can not be implemented with X11. Pinging @murarth to investigate this. Surely, no eventsloop = no XConnection, right?

[goddessfreya]

And while we're at it, no EventLoop = no connection to the Wayland display server... I'd be surpized if this is implementable on any of the other platforms.

[murarth]

I haven't done any testing and I'm not totally clear on the details of the proposed API (I skimmed through the comments of the above mentioned issues, but I'm not familiar with the use of VST plugins or any other type of GUI plugin), but I think this could be possible.

While it's true that, presently, the EventLoop is responsible for opening the XConnection, that doesn't necessarily need to be the case.

[goddessfreya]

Ok, but what do you think should be responsible for opening it? Keep in mind, we might bump into this issue right here, but for X11, if we do something too weird: rust-windowing/glutin#1152

[murarth]

It appears there's already a safeguard against this for X11. All XConnection instances are derived from the lazy_static here: https://github.com/rust-windowing/winit/blob/master/src/platform_impl/linux/mod.rs#L48

[goddessfreya]

Right- but the "external code [that] controls the main event loop" might not be run by winit. So, that external code might be using a different X11 display than the one winit wants to use.

[murarth]

True, but that is not an issue unique to winit. Any host program using an X (or Wayland) display server that expects a plugin to create a subwindow to its window must be expected to provide sufficient information to do so. If it doesn't, that would be a problem for any plugin.

The winit-specific element of this would be if, say, a host program provides an Xlib Display* that it expects the plugin to use, then we would need something like a WindowBuilder::from_x_display method to make use of it.

[goddessfreya]

Yes. Which means this function is unfeasible for X11. I suspect for similar reasons it will be unfeasible on every single other platform. This needs to be a platform specific extension like glutin's RawContextExt.

[murarth]

Oh, right. Yes. I agree. (I forgot that we were talking about this specific platform-independent interface.)

[l0calh05t]

Yes. Which means this function is unfeasible for X11. I suspect for similar reasons it will be unfeasible on every single other platform. This needs to be a platform specific extension like glutin's RawContextExt.

This really shouldn't be a platform-specific extension, or rather, if it is there would have to be versions for all major platforms for it to be useful. Embedded/externally hosted windows are definitely possible on x11, windows, and mac. There are VST (and other) plugins for all three.

Looking at https://github.com/steinbergmedia/vstgui/blob/59e9dcfe61e4c4b15c5598ebe7d2e9a52fe6a33d/vstgui/lib/platform/linux/x11platform.cpp it seems that on X11 (but not on Win32 or Cocoa), an own xcb_connect call is issued and an event loop is started. I'm not sure how this is done without breaking the existing host loop (I'm no X11 expert). Maybe the xcb_aux_sync call is important here, or the host is expected to run separate threads for each plugin window. But they certainly don't on Mac or Win32.

True, but that is not an issue unique to winit. Any host program using an X (or Wayland) display server that expects a plugin to create a subwindow to its window must be expected to provide sufficient information to do so. If it doesn't, that would be a problem for any plugin.

The APIs I know mostly just provide a single platform-specific pointer/handle/integer to the parent window. But somehow there are plugins on X11 anyways. So it is not unfeasible.

[goddessfreya]

Presumably this callback should also be static? See https://github.com/rust-windowing/winit/blob/master/src/event_loop.rs#L138

Won't that interfere with run?

What is this function mean to be returning?

[l0calh05t]

I'm not sure about the lifetime. I doubt it should be 'static, as each Window could have its own handler and you don't know a priori how many windows you will have.

Why would it interfere with run?

My initial thought with the bool return value was that events that are handled by an event_handler would no longer be passed to the event handler passed to run, but it doesn't necessarily have to be implemented that way. In the embedded case, the run handler can't really exist anyways - at least not as is.

[Osspial]

I've looked over the API you've proposed, and I'll say that it would be really difficult to implement, on Windows at least. Most of the Windows backend has been designed with the assumption that EventLoop will be there, and getting rid of that assumption will probably involve several painful and difficult-to-maintain changes to the existing codebase.

Anyhow, I read through #159, and #159 (comment) gives the clearest summary of the technical requirements this proposal is trying to solve. With that context in mind, I've got a few questions about how this proposal goes about solving those problems, as well as some suggestions that may make it easier to integrate into our codebase:

a) The current proposal requires stripping out the EventLoop infrastructure completely, which involves scrapping much of Winit's internal structure and the various API conveniences granted by EventLoop. Why is that necessary? Would it be possible to have some sort of EventLoop::run_externally_driven function that doesn't actually drive the event loop, but hooks into an event loop that's already been set up?

b) If we do end up going forward with the API as proposed, how do the two new functions interact with the current EventLoop functionality?

EDIT: Honestly I think that proposing an API here without having a thorough understanding of what each underlying platform needs to implement this is work that will lead nowhere. I'd definitely be interested in adding this functionality (as long as it isn't too much of a maintenance burden on our end), but designing this API-first instead of implementation-first seems backwards, seeing as we don't actually know what the platform-specific requirements are.

[crsaracco]

@l0calh05t are you on the vst-rs discord? Maybe we can take a look at trying this draft idea "internally" first (with actual VSTs as proof that our solution works), wrap our heads around what is exactly needed (and what would be the least painful changeset to winit), and then try an actual proposal afterwards?

I'd be willing to contribute to the Linux side of such an effort.

[l0calh05t]

I've looked over the API you've proposed, and I'll say that it would be really difficult to implement, on Windows at least. Most of the Windows backend has been designed with the assumption that EventLoop will be there, and getting rid of that assumption will probably involve several painful and difficult-to-maintain changes to the existing codebase.

It is entirely possible that the current architecture may be a showstopper here. In principle it is fairly easy to implement on Windows. Usually, an HWND doesn't need to know anything about the event loop driving it. You have the per-HWND GWLP_USERDATA and can attach all information you need for a specific window instance. You would then have a WndProc for your window class that simply fetches the right callback from the user data.

Anyhow, I read through #159, and #159 (comment) gives the clearest summary of the technical requirements this proposal is trying to solve. With that context in mind, I've got a few questions about how this proposal goes about solving those problems, as well as some suggestions that may make it easier to integrate into our codebase:

a) The current proposal requires stripping out the EventLoop infrastructure completely, which involves scrapping much of Winit's internal structure and the various API conveniences granted by EventLoop. Why is that necessary? Would it be possible to have some sort of EventLoop::run_externally_driven function that doesn't actually drive the event loop, but hooks into an event loop that's already been set up?

The intent was to preserve EventLoop as much as possible by making its existence optional. However, the Window itself could no longer rely on the existence of such an object. How would EventLoop::run_externally_driven look like? It would have to return control to the calling thread immediately on most platforms, I believe.

b) If we do end up going forward with the API as proposed, how do the two new functions interact with the current EventLoop functionality?

The intent was to make the existence of EventLoop optional, by providing

1. a way to create a Window without requiring an EventLoop instance
2. a way to handle events passed to a window, without a "global" event handler passed to EventLoop::run,

and to do this without requiring existing users of winit to change how they use the API.

[l0calh05t]

@l0calh05t are you on the vst-rs discord? Maybe we can take a look at trying this draft idea "internally" first (with actual VSTs as proof that our solution works), wrap our heads around what is exactly needed (and what would be the least painful changeset to winit), and then try an actual proposal afterwards?

I'd be willing to contribute to the Linux side of such an effort.

No, I don't use discord at all currently.

I would be willing to look into Windows, but from looking at winit, the current architecture makes things very difficult.

[goddessfreya]

Closing for inactivity.