[PATCH 3/8] winex11.drv: implement DoDragDrop on top of XDND

[Sebastian Lackner]

On 04.07.2015 15:36, Damjan Jovanovic wrote:
> Implements DoDragDrop on top of XDND. While some of DoDragDrop() and
> supporting functions are copied from ole32, they were modified to be
> non-blocking and event-driven.
> 
> The clipboard is patched to allow searching its clipboard formats for
> X11 atoms, export data in X11 format, and forward selection requests
> for the XdndSelection selection to XDND.
> 
> We support XDND version 3 to 5 (although we still need XdndProxy
> support for version 4). XdndPosition messages are rate-limited to at
> most 1 unreplied message and at least every 50 milliseconds apart, so
> that we don't flood slow X11 clients like the spec warns.
> 
> Damjan Jovanovic
> ---
>  dlls/winex11.drv/clipboard.c |  50 +++
>  dlls/winex11.drv/event.c     |   2 +
>  dlls/winex11.drv/x11drv.h    |   7 +
>  dlls/winex11.drv/xdnd.c      | 829 ++++++++++++++++++++++++++++++++++++++++++-
>  4 files changed, 887 insertions(+), 1 deletion(-)

Thanks for working on this. Sorry for the delayed reply, I've been a bit busy
during the last days. Not sure if you have already started improving the
patches, but I'll just comment on this version.

First of all, regarding patch 2, applying such a patch separately doesn't make
much sense. Either the design is accepted (then it can be part of patch 3), or
it needs additional work, but then its questionable if the function can be used
without further modifications.

At first I was planning to give line by line feedback on the patch, but I think
you are more interested in general feedback about the design. One of the main
disadvantages I see in the current approach is that there is a lot of code
duplication in winex11 and ole32. It will get even worse as soon as someone
starts implementing the same for winemac or other future backends. I am aware
that Ken Thomases basically asked for such a solution, but I don't think its the
right approach. There are basically two ways to solve it:

* Either the code in winex11 has to be stripped down a lot. A more minimalistic
solution has better chances to be accepted, and as soon as winemac contains a
similar implementation, the code in ole32 could be removed. When we need a huge
statemachine though, its better to avoid duplication.

* Alternatively, instead of duplicating code, the winex11/winemac backend could
provide a function pointer table or separate exports, with callbacks for things
which can be done natively. Ole32 could use these callbacks to register the
format, query if it dragging was finished, and so on. To avoid polling, the
driver could also post window messages to the "TrackerWindow".

If you need any help, feel free to ask. Also feel free to show proof-of-concept
patches even when they are not finished yet, for example via IRC or wine-devel.

As a side note, it looks like various parts of your patch lack proper locking,
but thats not your fault, its already missing in the existing winex11 code. I'll
see if I can clean up the existing code a bit to simplify the integration of
such a new feature.

Best regards,
Sebastian