Add support for WM_CREATE and WM_COMMAND messages on Windows

[fschutt]

This PR allows to register a custom callback (only on Windows) that can be used to set up application menus / native Win32 menu bars. The reason why it has to be a callback and can't be just a WindowEvent is because the Win32 API isn't thread-safe. If you call the callback from a different thread than the one on which the window was created, Windows will freeze your application on the SetMenu call.

Regular Win32 programming guides want you to register your application menus directly in the winuser::WM_CREATE message. Since we can't expose this directly to the user of this library, the only place to call the callback is directly after the window has been created.

In the callback, it is usual to set up custom IDs for getting a message back when the user has clicked an item. To return this ID to the user of the library, I've added a WindowEvent::Command. Currently this event is available on all platforms, but only gets emitted on Windows. It is debatable if we should put this behind a cfg flag, I've decided not to do that because it makes the event handling code a bit cleaner.

Also note that I've exported the winapi crate. This is done so that libraries can say use winit::winapi instead of having to match their version of winapi to whatever version winit is using. This leads to less version conflicts, when, for example winit uses winapi 0.3, but I want to use winapi 0.4 in my crate. This isn't critical, but it's nicer to re-export it. It's not critical though and could be removed from the PR.

[fschutt]

Note: I am not sure if adding / removing menu items at runtime is thread-safe. Maybe this will need an extra API in the future, but that shouldn't interfere with the current API.

[francesca64]

I wouldn't call this "very simple", since it introduces two unprecedented API changes. Do you mind if I just go ahead and release 0.14.0? Alacritty wouldn't upgrade to 0.13.1 due to a few regressions, so releasing 0.14.0 would allow for a bunch of problems to be fixed over there. Releases are pretty frequent now anyway, and I really don't feel comfortable being under pressure to merge this.

I don't know about exporting winapi. What you're saying sounds valid, but I don't have any experience with the potential ramifications of doing that. @vberger, I know this is a Windows PR, but do you have an opinion on re-exporting dependencies?

I think WindowEvent::Command has to go, since it's very platform-specific. I'd prefer you use an API similar to what was proposed in #450. I also imagine @Osspial might have some thoughts on how winit should support this.

[fschutt]

Alright. I'll think about the event handling API. The reexporting doesn't have large ramifications IMO, but I'll wait for feedback.

[elinorbgr]

@francesca64 I have nothing to say regarding the reexport. It can indeed simplify version handling in general.

However, I agree that WindowEvent::Command does have to go. In general, my understanding was that winit aimed to be kind of a greatest common denominator of all platforms, not a kitchen-sink of all possible uses. And while I am fine with adding platform-specific stuff in the platform-specific extension traits, adding a functionality to the core machinery of winit (here WindowEvent) means imo that we commit to having this functionality available on all platforms. Which in this specific case seems ambitious at least.

Going in the direction of #450, adding the possibility in platform-specific extension traits to add a callback for platform-specific messages not handled by winit seens a good direction to look, I think.

[Osspial]

It looks like the main reason the with_create_callback function needs to exist is that Windows currently spawns a background thread to run the event loop. We're going to be getting rid of that once #459 is resolved, so I'm not sure it's a good idea to pull in an API that's going to be obsoleted so soon after it's introduced. I'm more in favor of adding platform-specific hooks to intercept messages, like in #450, which would handle both this case and any other case we might run in to.

As far as platform-specific WindowEvent variants go, it could make sense to expose a way to deliver platform-specific events (i.e. a WindowEvent::Platform(PlatformEvent) variant that uses a type in the relevant os module), but I don't think they should be top-level variants of WindowEvent. If we keep WindowEvent::Command, we could then choose to expose it through that.

[fschutt]

@Osspial I am in favor of a custom WindowEvent::Custom or WindowEvent::Other which just passes the HWND and the message from the OS along (depending on OS). Let the user of the library handle un-catched events.

The with_create_callback will probably stay this way, even when the background, even when #459 is done. The reason being that you can't easily "inject" custom callbacks in the windows message loop - you can't call self.callback or something like that because in the callback function, you only have access to the HWND, the message and that's it. If it's on a different thread or not is largely not going to matter, because the difficulty is not the threading, it's that you have to know what callback to call once the WM_CREATE message is done.

Or we could say that we put the WM_CREATE message in to the WindowEvent::Other. Since after #459 the thread is the same, that could work, too. Then, but only then, we wouldn't need the callback.

IMO callbacks are a bit unwieldy to handle, so I'd favor a catch-all WindowEvent::Other(PlatformSpecificWindowEvent) or something like that (where the PlatformSpecificWindowEvent holds the platform-native messages and handles), instead of wrangling callbacks (where capturing outer variables could lead to trouble).

So, I'd wait until #459 is done. Because that would resolve at least the inconsistency of WM_CREATE.

[Osspial]

Actually, Windows provides a subclassing API that let's users pass a custom pointer to the windows event function. Using that to provide a function pointer would be pretty easy (see https://msdn.microsoft.com/en-us/library/windows/desktop/bb773183(v=vs.85).aspx#subclassing_v6).

Also, I'd prefer the callback approach in this case. It seems to me that the primary purpose of a callback would be to modify how the window behaves, rather than how the application behaves as a whole, and as such seems out-of-place in the application's main event loop. Putting it in the main loop also complicates how we handle return values, as the windows API attaches some significant semantics to those.

[Osspial]

I'm against re-exporting winapi. winapi 0.3 is structured around exposing the various win32 headers through toggleable feature flags, and if we re-export winapi we're making it so that removing one of those features because our implementation doesn't depend on it is a breaking change. Generally, I feel like a crate should only re-export another crate if the re-exporting crate's public API is inherently dependent on the re-exported crate, which isn't true here; winapi is just an (admittedly very important) implementation detail.

[fschutt]

Closing this because I ran into issues using this in practice.

While yes, you can create menus, there is no way to pass custom data into the on_wm_create callback (which is important to be able to do). With winits current 2-thread design, menus are simply not possible, so I'm closing this in favor of #445 - if the user code and the winit HWND run on the same thread, we can simply use SetMenu(hwnd) from anywhere in the user code without worrying about threading issues.