Skip to content

Conversation

@Drakulix
Copy link
Member

WIP branch implementing drag&drop between wayland surfaces and xwayland surfaces.

Currently only dragging from wayland -> xwayland works, dnd exclusively between wayland surfaces or xwayland surfaces should not be impacted.

This goes away with the ServerDndGrab, which would've potentially allowed dropping from Xwayland->Wayland but not vise-versa. Instead it moves part of the DnD-code and most importantly the DnDGrab into crate::input::dnd and abstracts over both Sources and potential targets with the new DndFocus-trait. It needs to be implemented for Seat::PointerFocus if one wants to use the DnDGrab.

In theory the new Source-trait should allow to fully replace the ServerDndGrab (if that is even used by any downstream compositor), but I haven't tried that so far, as I have yet to write an implementation of that for Xwayland and not only the wayland-types. (Which is also why this is still a draft.)

However the DnDFocus-trait seems to work out as intended and I would love some feedback on the api design.

This still needs more validation, that it doesn't break clients using older wl_data_device versions and works with clients using older XDND versions than 5.

@Drakulix Drakulix force-pushed the feat/xwayland-dnd branch 2 times, most recently from 510e69f to ee949b1 Compare November 7, 2025 13:16
@Drakulix Drakulix marked this pull request as ready for review November 7, 2025 13:21
@Drakulix
Copy link
Member Author

Drakulix commented Nov 7, 2025

Rebased onto #1854

@Drakulix Drakulix force-pushed the feat/xwayland-dnd branch 2 times, most recently from 7c6283b to 65d32dd Compare November 7, 2025 18:12
@Drakulix
Copy link
Member Author

Drakulix commented Nov 7, 2025

Okay, all bugs should be squashed now, including:

  • Quick mouse movements causing transfers from old windows to be aborted not handled correctly, which caused the source to hang
  • Directly crossing between an X11 and a Wayland window not waiting for mime-types, causing the wayland window to reject the drop
  • Fixing data-device v2 clients
  • Fixing copy/paste from wayland->x11 being broken (and likely was broken for anything but text for a long time)

@ids1024
Copy link
Member

ids1024 commented Nov 7, 2025

When I try to test drag and drop on anvil with WAYLAND_DISPLAY= nautilus, I see issues with the drag icon being offset fairly far away from the cursor. Though these appears to be an existing issue in anvil not affected by he changes here.

@Drakulix
Copy link
Member Author

Drakulix commented Nov 7, 2025

When I try to test drag and drop on anvil with WAYLAND_DISPLAY= nautilus, I see issues with the drag icon being offset fairly far away from the cursor. Though these appears to be an existing issue in anvil not affected by he changes here.

Yes, anvil is not handling moving X11 windows correctly. This PR makes no attempt to fix that, but behaves correctly, if you don't move the window after spawn.

Copy link
Collaborator

@cmeissl cmeissl left a comment

Choose a reason for hiding this comment

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

Okay, so first round was more technical and not so much functionality.
Will try to give it a second look concentrating on dnd functionality tomorrow.

@YaLTeR
Copy link
Contributor

YaLTeR commented Nov 17, 2025

Trying to update niri to this PR.

Some things I've hit:

  1. In WaylandDndGrabHandler::dnd_requested(), when setting the pointer grab, should it be Focus::Clear or Focus::Keep? I'm getting very conflicting evidence 😄
    • the removed lines in the diff (they differ between client and server DnD grab)
    • the new anvil and smallvil examples (they differ) and the cosmic-comp PR
  2. In DndGrabHandler::dropped(), the target is a generic T: DndFocus<Self>. In niri, this can only ever be a WlSurface, but how do I get that WlSurface out? Perhaps some downcasting support is needed?

@Drakulix
Copy link
Member Author

1. In `WaylandDndGrabHandler::dnd_requested()`, when setting the pointer grab, should it be `Focus::Clear` or `Focus::Keep`? I'm getting very conflicting evidence 😄
   
   * the removed lines in the diff (they differ between client and server DnD grab)
   * the new anvil and smallvil examples (they differ) and the cosmic-comp PR

I don't think it makes any difference, as the DnDGrab will continue to send events to the target that started the implicit grab, that was validated for the request. As far as I understand the wayland protocol that is expected and for Xwayland it is necessary for correct behaviour. So the enum-value doesn't really do anything. I changed everything to Focus::Keep though for consistency.

2. In `DndGrabHandler::dropped()`, the `target` is a generic `T: DndFocus<Self>`. In niri, this can only ever be a WlSurface, but how do I get that WlSurface out? Perhaps some downcasting support is needed?

I changed the function a bit to get rid of the generic argument altogether. :)

Copy link
Contributor

@YaLTeR YaLTeR left a comment

Choose a reason for hiding this comment

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

Test niri update: YaLTeR/niri@8ae9590

Some more things I've noticed:

  • After a DnD of a piece of text inside gtk4-demo, the window seemingly stops receiving pointer events until I unfocus and focus it again.

  • In a bunch of places in niri, I test if the grab is a DnD grab, i.e.:

    let is_dnd_grab = grab.as_any().is::<DnDGrab<Self>>();

    With the addition of new generic parameters, it becomes somewhat unwieldy, especially that S: Source parameter that can be several different things (at least WlSurface and WlDataSource).

I don't think it makes any difference, as the DnDGrab will continue to send events to the target that started the implicit grab, that was validated for the request. As far as I understand the wayland protocol that is expected and for Xwayland it is necessary for correct behaviour. So the enum-value doesn't really do anything. I changed everything to Focus::Keep though for consistency.

Thanks.

Actually, apparently I'm checking for pointer focus = None as a condition for triggering the niri hot corner. Previously, during DnD pointer focus was None, but with this PR, pointer focus isn't None during DnD, regardless if I set Focus::Clear or Focus::Keep when creating the DnDGrab. I can add a separate check for this so it's not a big deal, just a behavior change to be noted.

I changed the function a bit to get rid of the generic argument altogether. :)

Thanks, that works better.

@Drakulix
Copy link
Member Author

After a DnD of a piece of text inside gtk4-demo, the window seemingly stops receiving pointer events until I unfocus and focus it again.
[..]
Actually, apparently I'm checking for pointer focus = None as a condition for triggering the niri hot corner. Previously, during DnD pointer focus was None, but with this PR, pointer focus isn't None during DnD, regardless if I set Focus::Clear or Focus::Keep when creating the DnDGrab. I can add a separate check for this so it's not a big deal, just a behavior change to be noted.

Looking at this in more detail, it seems that GTK is unsetting pointer focus on wl_data_device.leave and does not expect wl_pointer events during the drag.

Which leaves us a bit at a conundrum, because ideally it seems like we don't want to set the focus. However Xwayland needs the focus for the grabbing application to generate the necessary dnd-events. Sooo... I added a hack in eb301e5 to force a re-enter on unset. I am not happy with this and suggestions are welcome.

With the addition of new generic parameters, it becomes somewhat unwieldy, especially that S: Source parameter that can be several different things (at least WlSurface and WlDataSource).

Yeah, I am not sure, what would be a good way of solving this. Even with Xwayland not being deployed, what previously was the ServerDndGrab is now also handled by the DndGrab (and I think this unification is actually a good thing), meaning we cannot know, if downstream might add it's own Sources and Targets.

Any suggestions on how to improve this are welcome.

Copy link
Collaborator

@cmeissl cmeissl left a comment

Choose a reason for hiding this comment

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

looks good from my side, but I would prefer to wait for feedback from @YaLTeR in this case for the final go.

@Drakulix Drakulix merged commit fb4fcd1 into master Nov 25, 2025
13 checks passed
@Drakulix Drakulix deleted the feat/xwayland-dnd branch November 25, 2025 16:20
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.

5 participants