[1/3] winex11: Select for XRandR screen change events and implement an event handler.

[Sam Edwards]

On 05/10/2013 09:11 AM, Alexandre Julliard wrote:
> Sam Edwards <cfsworks at gmail.com> writes:
>
>> Yeah, I don't call it until patch 2 in the series. This patch just
>> introduces it without calling it, which does cause a warning. (I hope
>> this doesn't violate the "atomic patches" rule.)
> It does, you can't add dead code (or warnings for that matter).
So, if I were to give this patch a try 2, I'd have to call the event 
handler somewhere (probably right after selecting for the resize event)? 
I'll keep that in mind for the future. :)

>>> But no, you can't select for events on the gdi display; actually I don't
>>> think you want to do it this way at all.
>> So gdi_display should not receive events? I'm guessing this has
>> something to do with the fact that Wine opens one Xlib Display per
>> thread, so the XRandR resize events need to be pulled down on one of
>> the thread-local X connections, rather than the master
>> gdi_display. (I'm still not sure why Wine does it this way. Are
>> certain versions of Xlib not thread-safe?)
> Most window events need to be handled in the thread that owns the
> window. Pulling them from a global connection in a different thread
> would only cause extra context switches and potential deadlocks.
This is kind of a special case because RRScreenChangeNotify is a root 
window event, so there's no "thread that owns the window" there (unless 
Wine has some internal representation for the X11 root window, and a 
thread that owns that representation, that I'm not aware of).

>>> Handling external resizes
>>> should be done in the desktop process, and most likely involves the
>>> wineserver too.
>> The event handler does call X11DRV_resize_desktop, which messages the
>> desktop process with the new size. But, I can see how it would be
>> cleaner to put the event checker in the same process as well, since
>> that way the XRandR resize doesn't have to get processed once for
>> every process on the system.
>>
>> How would this involve the wineserver, though? It looks like we
>> already have the infrastructure for synchronizing screen_width and
>> screen_height changes from one process to another.
> Not really, only the process that triggered the change gets properly
> updated at the moment. The Mac driver tries to do it by broadcasting a
> message, but that doesn't update processes that currently don't own a
> window. There are also race conditions, and various issues with window
> surfaces and DCE regions. Resizing the desktop is a tricky problem.
Aha; this is why the wineserver is necessary: So that applications that 
haven't created any windows can receive the update.

> As far as XRandr is concerned, at this point you could probably simply
> cache the mode and don't bother to update except when the app triggers a
> change, since that's what we do for the rest of the desktop parameters
> anyway.
>
Thanks for the suggestion. That sounds like it has a much smaller 
impact, so I think I'll use that approach instead.

Until then, I'll withdraw this patch.

Thanks,
Sam