Skip to content

Add drag n drop support to x11#187

Merged
prokopyl merged 11 commits into
RustAudio:masterfrom
BillyDM:x11_drag_n_drop
Jun 12, 2026
Merged

Add drag n drop support to x11#187
prokopyl merged 11 commits into
RustAudio:masterfrom
BillyDM:x11_drag_n_drop

Conversation

@BillyDM

@BillyDM BillyDM commented Apr 1, 2024

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread src/x11/xcb_connection/get_property.rs Outdated
Comment thread src/x11/xcb_connection/get_property.rs Outdated
@prokopyl

Copy link
Copy Markdown
Member

On top of making a merge commit to bring this up to date with main, I made a few fixes:

  • Properly flush the connection every time we send a XdndStatus message, to make sure the source application receives it and keeps sending us position events.
  • Also properly flush the connection when making a XConvertSelection request, to make sure the source application receives it and sends us the SelectionNotify reply.
  • Change the XConvertSelection request logic to only send a request if we haven't requested the data yet, instead of sending a request if we haven't received (or successfully parsed) the data yet. This fixes an issue where multiple requests would be needlessly sent if we received a new position event before the transfer was complete.
  • Track the timestamp of the XdndPosition event from which we made an XConvertSelection request, and match it in the SelectionNotify reply handler, to make sure we aren't reading stale replies.
  • Competely clear the cached drag-n-drop state and data when receiving a new XdndEnter event. This fixes some state/caching issues, but it is still a brittle fix that may cause issues if concurrent (or stale) drag sessions' events reach the application.

For good measure, I've also updated the femtovg example, so that the little orange square that follows the mouse also updates drop DragEnter/Position/Drop events. 🙂

@prokopyl prokopyl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, despite the fixes I've made to make it work on my computer (™), I still have some issues with the current implementation as is.

The biggest of those issues is that, IMO, the current winit X11 DnD implementation that this is based on is very brittle:

  • All of the drag session's state is stored in a bunch of Options, which not only make it a pain to know the current state, but also make it really hard to guard against invalid or unexpected events.
  • The get_property implementation completely blocks the thread waiting for a reply. This can lead to the application hanging if the source application doesn't reply fast enough. We should at least block with a timeout, but ideally this should be asynchronous and let us process other events while waiting for the transfer to complete.
    EDIT: Nevermind, I misunderstood that part. The data transfer is actually done through the server using properties, it is not sent peer-to-peer. So even though this is still synchronous, the only way we could hang is if the X Server doesn't reply, not the source application.

These robustness issues are, IMO, more critical in drag-n-drop than in other parts of the X11 codebase, because we are exchanging messages with an arbitrary application, not the X server itself.
While e.g. Dolphin may be reliable enough, the same cannot be said for other applications (those issues are also mentioned in the spec).

Outside of the robustness of the winit-originating code, there are also a few other things I found in the current implementation (although those are more open questions):

  • Currently, the drop is accepted instantly without first trying to receive or parse the data. This can lead to surprising behavior if an error occurs during parsing/receiving where the user sees that the baseview app accepts the drag but doesn't redraw accordingly, and nothing happens on drop.
  • Right now, the XdndEnter handling code immediately emits a DragEnter event, without waiting to receive position or drop data first. As far as I've tested, this seems inconsistent with other platforms, and forces users to handle the case of not having a reliable position at first, and for drop data to "appear" later.
    I think a more robust approach would be to wait for the data to be received and parsed before notifying the WindowHandler of any DnD event. This is also how the spec suggests handling this, by specifying the "widget" should only be notified when it can "look at" the data.
    This would also fix potential issues where the WindowHandler could "miss" drops if the data isn't received in time.

I can also work on incorporating those changes myself into your PR, instead of asking you to make more back-and-forths, if you're okay with it. 🙂

@BillyDM

BillyDM commented Apr 19, 2024

Copy link
Copy Markdown
Contributor Author

Yeah, I think it would be a good idea to follow how the spec suggests handling it.

I'm okay with you working on it if you're inclined to do so.

@prokopyl

Copy link
Copy Markdown
Member

I just finished rewriting the logic to use a specific enum to keep track of the XDND session state. 🙂

Incidentally I ended up completely rewriting the drag_and_drop file, and there isn't a single line of code remaining from winit, so I removed the attribution and license header from that file. (It's still there on the get_property file though, I haven't touched it.)

This commit should fix all of the issues I raised in my previous review. 🙂

@prokopyl prokopyl changed the title add drag n drop support to x11 Add drag n drop support to x11 Jun 12, 2026
@prokopyl prokopyl merged commit b5f536e into RustAudio:master Jun 12, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants