Skip to content

Commit

Permalink
fix(wl_data_device): drop and leave event handling
Browse files Browse the repository at this point in the history
I was able to recreate the issue in Cosmic, sway, and Pop on wayland (Gnome)

refactor(data-device): merge both cases in leave

Co-authored-by: Kirill Chibisov <[email protected]>

chore(data-device): document that the offer may need to be manually destroyed after leave

cleanup(data-device): example println capitalization
  • Loading branch information
wash2 committed Sep 23, 2023
1 parent 08f6081 commit dc8c4a0
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 17 deletions.
7 changes: 5 additions & 2 deletions examples/data_device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ impl DataDeviceHandler for DataDeviceWindow {
}

fn leave(&mut self, _conn: &Connection, _qh: &QueueHandle<Self>, _data_device: &WlDataDevice) {
println!("data offer left");
println!("Data offer left");
}

fn motion(
Expand Down Expand Up @@ -739,6 +739,7 @@ impl DataDeviceHandler for DataDeviceWindow {
loop_handle.remove(token);
println!("Dropped data: {:?}", String::from_utf8(data.clone()));
offer.finish();
offer.destroy();
state.dnd_offers.push((offer, Vec::new(), None));
return;
} else {
Expand All @@ -754,6 +755,7 @@ impl DataDeviceHandler for DataDeviceWindow {
Err(e) => {
eprintln!("Error reading dropped data: {}", e);
offer.finish();
offer.destroy();
loop_handle.remove(token);

return;
Expand Down Expand Up @@ -851,7 +853,7 @@ impl DataSourceHandler for DataDeviceWindow {
_qh: &QueueHandle<Self>,
_source: &wayland_client::protocol::wl_data_source::WlDataSource,
) {
println!("DROP PERFORMED");
println!("Drop performed");
}

fn dnd_finished(
Expand All @@ -860,6 +862,7 @@ impl DataSourceHandler for DataDeviceWindow {
_qh: &QueueHandle<Self>,
source: &wayland_client::protocol::wl_data_source::WlDataSource,
) {
println!("Finished");
self.drag_sources.retain(|s| s.0.inner() != source);
source.destroy();
}
Expand Down
45 changes: 33 additions & 12 deletions src/data_device_manager/data_device.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
use std::sync::{Arc, Mutex};

use crate::reexports::client::{
event_created_child,
protocol::{
wl_data_device::{self, WlDataDevice},
wl_data_offer::{self, WlDataOffer},
wl_seat::WlSeat,
use std::{
ops::DerefMut,
sync::{Arc, Mutex},
};

use crate::{
data_device_manager::data_offer::DataDeviceOffer,
reexports::client::{
event_created_child,
protocol::{
wl_data_device::{self, WlDataDevice},
wl_data_offer::{self, WlDataOffer},
wl_seat::WlSeat,
},
Connection, Dispatch, Proxy, QueueHandle,
},
Connection, Dispatch, Proxy, QueueHandle,
};

use super::{
Expand All @@ -34,7 +40,8 @@ pub trait DataDeviceHandler: Sized {
fn enter(&mut self, conn: &Connection, qh: &QueueHandle<Self>, data_device: &WlDataDevice);

/// The drag and drop pointer has left the surface and the session ends.
/// The offer will be destroyed.
/// The offer will be destroyed unless it was previously dropped.
/// In the case of a dropped offer, the client must destroy it manually after it is finished.
fn leave(&mut self, conn: &Connection, qh: &QueueHandle<Self>, data_device: &WlDataDevice);

/// Drag and Drop motion.
Expand Down Expand Up @@ -133,9 +140,11 @@ where
Event::Leave => {
// We must destroy the offer we've got on enter.
if let Some(offer) = inner.drag_offer.take() {
offer.destroy()
let data = offer.data::<DataOfferData>().unwrap();
if !data.leave() {
inner.drag_offer = Some(offer);
}
}

// XXX Drop done here to prevent Mutex deadlocks.
drop(inner);
state.leave(conn, qh, data_device);
Expand All @@ -153,6 +162,18 @@ where
state.motion(conn, qh, data_device);
}
Event::Drop => {
if let Some(offer) = inner.drag_offer.take() {
let data = offer.data::<DataOfferData>().unwrap();

let mut drag_inner = data.inner.lock().unwrap();

if let DataDeviceOffer::Drag(ref mut o) = drag_inner.deref_mut().offer {
o.dropped = true;
}
drop(drag_inner);

inner.drag_offer = Some(offer);
}
// XXX Drop done here to prevent Mutex deadlocks.
drop(inner);
// Pass the info about the drop to the user.
Expand Down
40 changes: 37 additions & 3 deletions src/data_device_manager/data_offer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use std::{
sync::{Arc, Mutex},
};

use log::warn;

use crate::reexports::client::{
protocol::{
wl_data_device_manager::DndAction,
Expand Down Expand Up @@ -77,6 +79,10 @@ pub struct DragOffer {
pub source_actions: DndAction,
/// the compositor selected drag action
pub selected_action: DndAction,
/// whether or not the drag has been dropped
pub dropped: bool,
/// whether or not the drag has left
pub left: bool,
}

impl DragOffer {
Expand All @@ -97,7 +103,7 @@ impl DragOffer {
/// This request determines the final result of the drag-and-drop operation.
/// If the end result is that no action is accepted, the drag source will receive wl_data_source.cancelled.
pub fn set_actions(&self, actions: DndAction, preferred_action: DndAction) {
if self.data_offer.version() >= 3 {
if self.data_offer.version() >= 3 && !self.left {
self.data_offer.set_actions(actions, preferred_action);
}
}
Expand All @@ -106,14 +112,21 @@ impl DragOffer {
/// This request may happen multiple times for different mime types, both before and after wl_data_device.drop.
/// Drag-and-drop destination clients may preemptively fetch data or examine it more closely to determine acceptance.
pub fn receive(&self, mime_type: String) -> std::io::Result<ReadPipe> {
receive(&self.data_offer, mime_type)
// When the data device has left, we can't receive unless it was previously dropped.
if !self.left || self.dropped {
receive(&self.data_offer, mime_type)
} else {
Err(std::io::Error::new(std::io::ErrorKind::Other, "offer has left"))
}
}

/// Accept the given mime type, or None to reject the offer.
/// In version 2, this request is used for feedback, but doesn't affect the final result of the drag-and-drop operation.
/// In version 3, this request determines the final result of the drag-and-drop operation.
pub fn accept_mime_type(&self, serial: u32, mime_type: Option<String>) {
self.data_offer.accept(serial, mime_type);
if !self.left {
self.data_offer.accept(serial, mime_type);
}
}

/// Destroy the data offer.
Expand Down Expand Up @@ -258,6 +271,8 @@ impl DataOfferData {
x,
y,
time,
dropped: false,
left: false,
});
}
DataDeviceOffer::Undetermined(o) => {
Expand All @@ -270,6 +285,8 @@ impl DataOfferData {
x,
y,
time,
dropped: false,
left: false,
});
}
}
Expand All @@ -295,6 +312,23 @@ impl DataOfferData {
}
}

pub(crate) fn leave(&self) -> bool {
let mut inner = self.inner.lock().unwrap();
match &mut inner.deref_mut().offer {
DataDeviceOffer::Drag(o) => {
o.left = true;
if !o.dropped {
o.data_offer.destroy();
}
!o.dropped
}
_ => {
warn!("DataDeviceOffer::leave called on non-drag offer");
false
}
}
}

pub(crate) fn as_selection_offer(&self) -> Option<SelectionOffer> {
match &self.inner.lock().unwrap().deref().offer {
DataDeviceOffer::Selection(o) => Some(o.clone()),
Expand Down

0 comments on commit dc8c4a0

Please sign in to comment.