https://bugs.winehq.org/show_bug.cgi?id=2155
Rémi Bernon rbernon@codeweavers.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |rbernon@codeweavers.com
--- Comment #25 from Rémi Bernon rbernon@codeweavers.com --- Created attachment 68356 --> https://bugs.winehq.org/attachment.cgi?id=68356 Patch series addressing several focus inconsistencies.
I spent some time digging into focus issues lately and I believe this issue has several root causes.
First of all of course, Wine currently doesn't implement any mechanism to activate other windows on the X11 side. This is implemented by the staged patch, but I believe it can then trigger other focus problems that Wine suffers from.
With or without the patch, the root issues are:
1) I believe there's a race condition in the SetForegroundWindow / SetActiveWindow implementation. When multiple threads are involved, SetActiveWindow requests from other threads are only applied when the thread checks its messages, possibly overwriting changes that were done by the thread itself.
2) The asynchronous nature of the focus handling code: Wine applies the Win32 focus state changes immediately, but processes -- and applies -- the results of the X11 activation requests only later, during window message processing.
3) Windows allows hidden windows to be focused and foreground, X11 doesn't allow unmapped windows to be. This is a fundamental difference and it is pretty hard to solve, we currently implement hidden windows as unmapped X11 windows, as it is probably the only way to have truly invisible windows, but it makes it impossible to keep the X11 state consistent in that case.
The solutions are either to map hidden windows, using some other mechanism to hide them as much as possible, bottom-most attribute being perhaps the least brittle. Or, as a possible mitigation, force change X11 focus when hidden window is supposed to be focused, so that X11 will send focus change events when it changes back to another window.
4) SetForegroundWindow is not supposed to always succeed, although Wine always pretend it does. The _NET_ACTIVE_WINDOW request also not always succeed, but as we can't wait for its possible result, it is hard to return a reliable success status.
This is especially the case with the staged patch, that sends activation requests to be applied later, but it is also the case with user focus changes. Having the X11 events processed and applied only when message processing can trigger a race condition where X11 related focus changes will overwrite the already executed Win32 focus changes, possibly ending up in an inconsistent state.
Also, because of 1), when focus events come on several threads, SetActiveWindow messages coming from other threads may be applied at the same as the X11 events are processed and applied, making things even more inconsistent.
The attached patch series should help fixing these issues. First of all, it fixes the incorrect message processing order in 1).
Then it should try to make 2) work better by tracking X11 focus globally in explorer.exe, so that we don't rely on application processing their messages anymore, and by discarding old SetForegroundWindow requests based on their timestamp.
Last, it also implements a similar feature as the staged _NET_ACTIVE_WINDOW patch to send X11 activation requests, but it adds a mitigation for 3) and 4), where SetForegroundWindow returns failure if X11 focus is not on another Wine window, and where SetActiveWindow forcefully unsets X11 focus when hidden window is activated. It is still not completely matching Windows behavior, as SetForegroundWindow should probably return FALSE in more cases, but the details are a little bit more complicated and it may also break some currently working cases.
I'm currently trying to upstream the first patches, but the way internal messages gets discarded needed some discussion. The whole series also could benefit from more extensive testing.
This is also probably going to conflict with the rawinput patch series, as there are changes in the XInput2 code to get Focus messages with timestamps if possible. I'll attach rebased versions next in the eventuality this goes into staging.