Implement Drag and Drop for X11

[JohannesLorenz]

This works with zynaddsubfx (both zyn being a source and a target).

Besides DnD, there are two minor commits token from project mruby-zest by fundamental.

[7890]

Hi, IMHO this is an interesting feature.
From what I see every DnD event is handled with callback functions.
A generic question: wouldn't it fit better to handle it with events? Naively speaking, instead of eg. PUGL_FOCUS_IN it would be PUGL_DND_XXYY, to be handled in the app's event loop.
What's the pros/cons of callbacks vs. emitting events?

[JohannesLorenz]

@7890 Some of the DnD callbacks have return values: enums, ints, const char*. E.g. one callback has to return whether the drop is accepted. Another one needs to return a mimetype string. I did not know how this should be mapped to struct events.

Any ideas are welcome, but I think it's not worth to spend much time here as long as this won't be reviewed/merged. @drobilla Can you foresee when you'll have time to review/merge this PR?

[7890]

E.g. one callback has to return whether the drop is accepted

Ah.. yes makes sense, it can't be handled by an event..

I've a simple prototype for pugl view as a target for file drags with the following events:
PUGL_DND_ ENTER, EXIT, DROP_FILE, END
It's much less advanced, things like checking MIME type are left to the receiver of (potential) file URIs. It's limited to the case of dragging file(s) into a view and every file in the list produces a PUGL_DND_DROP_FILE event, including the URI.

I think for a much more advanced DnD integration, the drag-files-from-filemanager-to-view usecase would serve as a nice example usage.

++1 for zynaddsubfx & good luck!

[drobilla]

@JohannesLorenz Honestly, the code is so far from mergeable it's hard to say.

Aside from my hope that this can be done with much less code, and simpler code, there seem to be a few major conceptual things:

• Pugl can't be imposing UX on applications, like "DND works only when a key is held". This is a super weird interaction I don't understand.

• It would need to be implemented for the other platforms as well. At the very least as a prototype so we know it can work.

• Events are the way things are done now, but I am not sure about non-POD events, so maybe some callbacks for things (or methods to get at the data in an event handler) will be necessary here.

This seems like a very hefty project. I'll try to get around to it, but I've been sinking a lot of time into just getting what's currently upstream in shape (and work full time otherwise...), so I really can't say when I might manage. Currently more focused on working towards a solid release and getting everyone based on that...

[7890]

Interesting point on "Pugl can't be imposing UX on applications", makes sense.

If there's interest, I can share my prototype. It's working on all three platforms with events, but only for files. As a side-note, the more advanced one extends the API for one platform only, the more surprises can be expected when trying to make it generic for all platforms.

The sequence of a typical DnD events for prototype is:

Event: DnD enter
Event: Mouse enter at 308.000000,169.000000
Event: DnD drop file 1/2 '///mnt/data/gb1/home/srv/dead.letter'
Event: DnD drop file 2/2 '///mnt/data/gb1/home/srv/diskusage.txt'
Event: DnD end
Event: Focus in

A callback to test if any or all of the files potentially contained in the drag data has technical implications, it must be done early (eg. drag enters view). I find it much simpler to give caller a string as soon as files are dropped and caller can then check if it's a file or a directory, whether its of the expected type etc. totally agnostic from pugl. However it would be possible to add such a callback anytime later.

I don't want to disrupt this PR, just sayin' DnD is interesting for pugl and it can work for files relatively simple. Cheers

Edit: most of the code for X11 was taken from xjadeo project, which seemed compact enough

[drobilla]

A callback to test if any or all of the files potentially contained in the drag data has technical implications, it must be done early (eg. drag enters view). I find it much simpler to give caller a string as soon as files are dropped and caller can then check if it's a file or a directory, whether its of the expected type etc. totally agnostic from pugl. However it would be possible to add such a callback anytime later.

I have no idea what you are trying to get at here.

[7890]

@drobilla I don't understand your question (?)
Whether drag data is checked before or after dropping is a small difference IMHO. If acceptance is checked before dropping, there's some benefits to indicate whether or not the drag will be accepted. If that check is done only after receiving PUGL_DND_DROP_FILE, the user is left uninformed until the drop took place.

[JohannesLorenz]

Aside from my hope that this can be done with much less code, and simpler code

The reason why we have like 14 callbacks and there are "simpler" implementations with only 5 callbacks is that we wanted to be able to both drag from (automating knobs on the host) and drop over zyn (loading presets) and even the combination can be useful (e.g. switching order of effects inside zyn).

For what this PR can do, it's IMO not much code, and taking a quick look at zyn-fusion shows that most of the callbacks are useful.

* Pugl can't be imposing UX on applications, like "DND works only when a key is held".  This is a super weird interaction I don't understand.

Good point. An app may want the user to have 2 buttons pressed for DnD, or even no buttons at all.

I think that can easily be abstrahized. Instead of asking event->key.special == view->dndSourceKeyFunc(view) when a key is pressed and then storing this "dnd-ready" property inside pugl, we let the application keep track of whether it's dnd ready or not, and the code initiating the dnd on mouse press won't ask view->dnd_source_status == PuglDndSourceReady, but instead ask a new callback doesAppWantToStartDnd().

* It would need to be implemented for the other platforms as well.  At the very least as a prototype so we know it _can_ work.

We can still add this later. Other platforms should compile with this patch, they just can't use it. Do we want to make all Linux users wait until we find a Windows and a Mac coder (how seldom is that?) who spends time on such a difficult topic? Keeping this PR open a few more years, making it diverge again? I'm always for keeping PRs minimal and merging them ASAP.

* Events are the way things are done now, but I am not sure about non-POD events, so maybe some callbacks for things (or methods to get at the data in an event handler) will be necessary here.

Not sure what you refer to with non-POD. As mentioned, the problem is that the DnD callbacks must return something. There are ways to do this with events:

1. Let the events carry pointers where the return value will be written (acceptable for me, for now)
2. Always return events to the sender somehow, so the return value can be written into the event (no pointers).
3. Sending events bidirectionally in turn to get answers (not so simple)

What is your preference?

This seems like a very hefty project.

If I'll solve your concerns and rebase it to current master, you just need to click the merge button. I don't require a review for this, I'm using the patch for >1 year now in daily production. So will you hit the merge button then?

[7890]

@JohannesLorenz If a feature is only available for one platform, hoping anyone will implement the same for other platforms anytime later is probably a pipe dream. IMO the fact that most features (very few exceptions) work on all platforms makes 50% of the usefulness of this library. If a major feature isn't available for all platforms, one basically can't use it. Another risk for partial platform support I see in a fragmentation, somebody adds a feature X for OSX, somebody else feature Y for Windows etc. and it becomes a mess loosing the "write once for all" benefit. I wonder how much of an effort it would be to provide it for other platforms.
Out of curiosity how do you solve D&D for your program on other platforms, is it just not available or handled differently?

PS: POD refers to "Plain old data". Just a regular struct or similar holding data.

[drobilla]

For what this PR can do, it's IMO not much code

Always a matter of opinion, I suppose, but as written it more than doubles the size of pugl_x11.c, and adds quite a bit elsewhere. Overall it makes Pugl nearly 1.5 times the size, for only one platform implementation, which is a lot. If doing DnD truly requires this much code, I'd be tempted to say it's just out of scope for Pugl.

I doubt it does, though. It looks like it would be a lot less if the unnecessary stuff was stripped out, but the huge amount of code relative to the size of the project is definitely part of what's made me just not want to deal with it.

We can still add this later.

Yes, but... can we? Does this API actually make sense on the other platforms? I have no idea.

I'm always for keeping PRs minimal and merging them ASAP.

I generally am too... except when it comes to library APIs. I can appreciate that not having things merged is generally frustrating, but I am trying to resolve a lot of the mistakes that were made with Pugl over the years that have caused so much fragmentation and other problems, and I think generally half-assed support where some things maybe-kinda worked on some platform or another was a big one. API instability was another.

Trying to figure out an API that works on all the target platforms is pretty much the task of libraries like Pugl. It sucks sometimes, but that's the work.

I've learned from the LV2 support libraries how things go when this is done well, and learned from Pugl what an absolute nightmare it is when it isn't. Live and learn; I'm done with that nightmare now, and Pugl will be joining the others as an "official" part of the "SDK" with long-term stability guarantees soon.

If I'll solve your concerns and rebase it to current master, you just need to click the merge button.

Not blindly clicking the merge button on whatever is my responsibility as maintainer. Hastily adding sub-par code to Pugl happened far too much in the past, and it made sense in the early days, but that's done. I'm deliberately doing exactly the opposite now.

I don't require a review for this

... yeeeeeeeeeeeeeah, well. I hesitate to even touch this one, but to be completely honest, I'm surprised this was apparently intended as something to be merged as-is. I thought it was more of an RFC thing. It's full of TODOs, debug printfs, weird line noise in comment headers... aside from the general mess, it adds a massive amount of API and code in general. This PR requires the most review out of any in the history of the project, by a wide margin.

That said, sure, address the conceptual concerns and rebase it. I'm happy to do all the mundane cleanup crap myself, but like I said, I really can't promise a timeline on making DnD my life's work for a while.

Anyway, as to the actual substance: I'm not sure what I mean about POD or if it really matters either. Just that all events are currently POD, which seems quite nice, and trucking pointers to stuff around with them seems worth thinking about. I don't think all things need to be events, that's only true in one direction (the system notifying the view about stuff). It is fine to assume a context in the event callback, so it's fine to have puglDndDoThingThatOnlyMakesSenseInADndEventHandler() stuff.

I did recently make the event callback at least return a PuglStatus, but a fancier return type is possible if necessary. It seems like that can cover almost everything though, most return int. That just leaves the KeyFunc thing which needs to go away anyway, and the type/property stuff. My initial feeling is that maybe that stuff is fine as separate functions anyway.

XDnd seems to use the clever approach of sort of re-using the clipboard facilities, maybe that will work nicely here to deal with the actual data as well. As it happens, copy/paste is the other big feature some people are clamoring for... this makes me think that maybe adding that first is the best plan: DnD could be exclusively concerned with events that notify the client about the various situations, and setting/getting data for it would be done through the same API as the clipboard (puglGetClipboardContents(type) or some such thing, possibly with a context parameter or something to distinguish the DnD "clipboard" from the copy-paste-clipboard if necessary).

If you think that general idea makes sense, I might bump a clipboard API up my priority list. There's a PR for that already anyway, and it's a much more manageable piece of work to get done. That would mean we have an API for getting and setting whatever type of data, and this gets reduced to mostly just notification events that DnD stuff of interest has happened. I will have to look into the other platforms a bit to see if this could actually work, but it seems reasonable...

[7890]

Since this library is easily statically linked, long-term API stability is not massively important IMO.
When it becomes a facility of an extended plugin spec ("UI MUST be done with pugl, host MUST provide pugllib"), stability will be very important and benefits will be great.

[JohannesLorenz]

Another risk for partial platform support I see in a fragmentation, somebody adds a feature X for OSX, somebody else feature Y for Windows etc. and it becomes a mess loosing the "write once for all" benefit.

Just for fun discussion. I see that Windows and Mac always get tons of cool native plugins. Now Linux users/open source fans want to have something similar cool, but they can't because the developers want to have it compatible for Windows/Mac. Which no one wants to do because... Windows/Mac already have their cool commercial plugins (and are not that popular for coders). Somehow I find this unfair for Linux users/open source fans. But I'm not insisting on that point. If really necessary, we'll wait until anyone wants to do this porting.

I wonder how much of an effort it would be to provide it for other platforms.

It took me ~100 hours to get this done. I'd expect the same for both other major platforms.

Out of curiosity how do you solve D&D for your program on other platforms, is it just not available or handled differently?

They can implement all the DnD callbacks, compile it all, but the callbacks will never be called.

If doing DnD truly requires this much code, I'd be tempted to say it's just out of scope for Pugl.

Maybe you meant pugl as a simple test thing and never meant for plugins with more functionality than just a few knobs? So if a user needs more complex stuff like a tree widget, file browser, etc, they are asked to write it on their own or use zest (zyn's UI framework). But you can't add DnD to e.g. zest because the X11 events are handled inside pugl. So there are four options:

1. Leave PUGL without any DnD (and maybe other important features that require a lot of code), which makes it unsuited for medium or large plugins
2. Add some kind of minimal DnD. It should at least satisfy zyn though, and I doubt you can remove even 10% of my code...
3. Add a full featured DnD, satisfying plugins that are more than only 4 knobs and a background image.
4. Not decide for anything, which is pretty much point 1.

I doubt it does, though. It looks like it would be a lot less if the unnecessary stuff was stripped out.

Like said, zyn already uses 12 of those 14 callbacks. You could remove 2 and pray no other apps (or zyn again) won't ask you to add them later. Considering pugl_x11.c (the largest code blocks) the XDnD protocol exactly defines what events have to be answered how under which circumstances. So I don't see what parts of this PR can be removed without violating the protocol, but if @7890 can prove their implementation allows zyn to do what it currently can do using this branch (e.g. get test-libversion.c, our pugl adaption working, I can help if there are questions), I'll agree replacing my PR with theirs.

I thought it was more of an RFC thing. It's full of TODOs, debug printfs, weird line noise in comment headers... aside from the general mess, it adds a massive amount of API and code in general. This PR requires the most review out of any in the history of the project, by a wide margin.

There is indeed some noise in this PR, but for many TODOs and debug printfs, it is in such a state because I had questions mailed to you (April 15, 2018) that never got answered. I wouldn't remove valuable debug printfs or TODOs until it's really finished. So after 3 or 4 weeks of no answer I just pushed that PR with the questions still open, hoping it won't get lost and anyone would review it there.

[drobilla]

LV2 is not a Linux plugin spec, it is an open plugin spec. Preventing it from being a Linux-only ghetto is very much intentional, because that severely reduces people's incentive to actually implement it. That said, nobody is blocked by whatever feature not being in upstream Pugl, certainly not with the current situation of it just being copy/pasted around and modified anyway.

Maybe you meant pugl as a simple test thing and never meant for plugins with more functionality than just a few knobs?

I think Pugl should support whatever is necessary for advanced UIs within reason. If something like DnD would bloat the library to several times its size, then yes, I would suggest that a more limited implementation would be a better idea. The amount of blowback I have received for things being in there that somebody somewhere doesn't need (most infamously Cairo) is insane, and hugely toxic.

GLFW gets by with just a single drop callback, for example. Removing the entire concept of being a source seems like a sensible compromise if necessary, that's far less widely useful for things like plugins than being a destination.

So after 3 or 4 weeks of no answer I just pushed that PR with the questions still open, hoping it won't get lost and anyone would review it there.

Me, from that thread, mentioning the biggest conceptual problem with this, which remains:

I'm not sure, the interaction here where dragging is controlled by a
key seems quite application specific (and atypical) so I can't
immediately see how this would influence the code in Pugl itself.
Surely you don't mean Pugl itself would have this behaviour baked in?

The event stuff was also discussed. At the time I was moving to a new country, working full time, and finishing a PhD; so to say I didn't have the time to just do all of this myself would be quite the understatement. The issues, size, and WIPeyness of this PR means it is a lot of work to deal with. You can do that work or not - it's your free time (presumably) and I won't tell you want to do with it. Similarly, though, I don't appreciate being nagged to do a ton of work in mine. As you say, the PR exists for someone to deal with at some point. Here we are, dealing with it. As you may have noticed, I am now dealing with an awful lot of Pugl things after a long stagnant period. If the issues with this get magically addressed and I can truly just "hit the merge button", wonderful. If not, I will deal with it when I deal with it.

If you don't like the standards for Pugl being higher, fair enough. I am convinced that it needs to happen to solve the current situation with UIs, which is an absolute garbage fire. Pugl 1.0 will work well, everywhere, have a thought out, versioned, and minimal API, and be designed to make life easier for people maintaining changes that aren't yet (or ever) suitable for upstream. It will have zero WIP code in it, be well documented, packaged in distributions, and included in the LV2 "SDK", to make it available for people to easily use and put pressure on converging on upstream. This means that I am personally going to have to review and test the entire thing, so, yes, I am not a fan of bloat. Definitely going to have to crack quite few eggs to make that omelette, but it needs to get made. It was a nice idea that total chaos would just magically address everything, but that has clearly failed, so it's full on benevolent dictator time. This is me learning from my mistakes as maintainer, and merging stuff because somebody said it worked for them was very much one of them.

More to the point, I find the above idea about gutting the source part pretty appealing. Why does zyn need this?

[7890]

@JohannesLorenz the prototype I've hacked together won't serve as a replacement of the much more advanced DnD features that your program relies on. I tried to make it simple enough so that there's a chance to implement it everywhere. Even with that, there's a small difference in which events are effectively fired, eg. on Windows, getting the pre-drop events is much harder so I left this out. The most important event is the drop event.

As for how to ship the data, it's just a new type of event with a char[1024] where URI's are memcpy'ed to (not unlike for the text event). Considering there won't be millions of such events this seemed good enough (eg. no pointers shipping around with implications of holding data at different places, possible free() / double free() issues). Of course this is good for just ONE usecase which are file URIs.

@drobilla I think your time will be much better invested to iron out known issues rather than adding new features. Once there is an acceptable multi-platform feature ready enough, it can be considered.
Whether or not to start simple (KISS..) is another decision. It could be one path to start with some DnD optimally covering a common case, and then go from there. This path has the disadvantage that the API must probably be changed if DnD gets more powerful later, since first it would be less generic. The advantage is that it can already be used for the covered usecases.

I'm a few function implementations away from a multi-platform WM feature set. The odd problem I'm tackling currently is this: OSX: remove window title (decoration): view still gets mouse input but no more keyboard events. I've read that canBecomeKey() plays a role in that, pugl has it but it still doesn't work. Why I'm saying this: as long as this isn't solved, the feature isn't useful and a PR would be of little use. My plan is to solve everything possible and then make it available so that partial aspects (like eventually welcomed fullscreen feature) can be ripped out to form a separate, already tested PR. However fullscreen isn't totally agnostic from other WM functions. Alone to make it "restore" nicely, a few WM functions are handy, since the fullscreen procedures differ slightly on platforms. It's a feeling of success when a feature implementation works the first time on a "foreign" platform, I can tell you that :)

Edit: found out the issue for key input. The missing bit was [view->impl->window makeFirstResponder:view->impl->wrapperView]; [view->impl->window makeKeyWindow];

[JohannesLorenz]

I think Pugl should support whatever is necessary for advanced UIs within reason. If something like DnD would bloat the library to several times its size, then yes, I would suggest that a more limited implementation would be a better idea.

You refer to the number of lines from the PR, which is almost only pugl_x11.c. The callbacks are minor. It's not possible to write a valid DnD implementation for both target and destination much simpler than that, because the XDND specs are that complicated. I can't just leave out sending or receiving X11 events because then it's wrong. Which lines of the PR (except debug prints) can I take out and still be valid?

On the other hand, an advanced UI should support bidirectional DnD IMO. So I think it's necessary and minimal to what is necessary.

More to the point, I find the above idea about gutting the source part pretty appealing. Why does zyn need this?

I don't get what you mean with that. What part do you find appealing?

[drobilla]

Which lines of the PR (except debug prints) can I take out and still be valid?

I suppose we'll see, since it seems pretty clear at this point that I'm the one who is going to have to do it...

[drobilla]

On the other hand, an advanced UI should support bidirectional DnD IMO. So I think it's necessary and minimal to what is necessary.

I don't necessarily disagree, but none immediately come to mind, while destination has a bunch of obvious uses in plugins. So I am asking what, concretely, you need this for, to motivate the effort (I don't care for abstract hypotheticals. 7890 here says similar things to advocate for adding a bunch of glaringly inappropriate window management stuff).

I ask these questions because if the answer is stuff that only makes sense for top-level desktop applications, then Pugl is not the appropriate library to use. There are, and always will be, better things for that which aren't constrained by Pugl's embedded purpose and lowest-common-denominator-ism.

I don't get what you mean with that. What part do you find appealing?

Less code, less API, less maintenance burden, less of this kind of discussion, less work.

[JohannesLorenz]

So I am asking what, concretely, you need this for, to motivate the effort

• Dnd as Target: Dragging files/presets from file browser and dropping on zyn (to load them)
• Dnd as Source: Dragging controls from zyn and dropping them over controllers/automation tracks in LMMS (to automate zyn's controls from LMMS)

[drobilla]

Dnd as Source: Dragging controls from zyn and dropping them over controllers/automation tracks in LMMS (to automate zyn's controls from LMMS)

Hm. Never would have thought of that one. What do you use as a payload?

[JohannesLorenz]

Hm. Never would have thought of that one. What do you use as a payload?

We use mime-type "application/x-osc-stringpair" with values like "osc.udp://127.0.0.1:12345/osc/path/to/port".

This is likely not suited for a good standard, it was mostly inventing a mimetype for transferring our OSC paths.

Btw, maybe DnD with zyn as source would not be necessary. E.g. Lv2's ui:touch should allow connecting with first clicking the automation pattern/controller in lmms, then "touching" the control in zyn. Maybe this would have been the easier workflow... 😃

[drobilla]

Oh, right, OSC. My initial thoughts were that this would probably be a good thing to standardize for LV2 some day, but I suppose that's still true.

Touch does seem like a good idea here, but regardless, it's a decent motivating case. It makes more sense to me why you would do that key thing now. That makes no sense at all for typical DnD cases but is necessary for parameters or anything else you drag for other reasons.

[7890]

The payload can be a variant of text in any case. I'd not start to try to drag around fully fleshed binary data.

[JohannesLorenz]

@drobilla Two small commits in this branch are not related to DnD, I just added them because zyn required them: 82a2888 and 179f922 . @fundamental wrote the code, I really don't know what they mean.

I'd like to separate them from this branch. Can you please check if it makes sense to add equivalents of them to master, or if they are even already in there?

[drobilla]

They've been superceded by equivalent/better functionality.

[drobilla]

There is also 4a8cb07 now on my current working branch, which I imagine could be extended to handle the data part of DnD.

[JohannesLorenz]

In this case, I'll open another branch dnd, which won't have those 2 commits.

I'll close the PR soon. This branch will still be kept alive, as it's currently submoduled by zyn.

[falkTX]

Since pugl is getting some love lately, would be nice to solve this, at least for the clipboard support that is a little bit broken.
You have a custom branch to try out some changes, but sadly it is quite outdated now.

Is it possible to get that one updated?
Or maybe just merge it, since main branch has issues and this seems the right time for breakage anyway.

[drobilla]

The dragdrop branch has been rebased on the current master.

I will try to finish it if I have the time/energy, but it's pretty complicated and not something I want to half-ass, since the API that can accommodate it is probably the biggest question. I've still only done one direction (paste/drop) on X11.

The set of types you can rely on is a bit limited, particularly on windows where only URI lists work, but I don't really care about that. If it's possible, it can be added whenever.