Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework DnD redux #4079

Merged
merged 24 commits into from
Jan 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
2594b83
feat: DnD changes so far (rebased)
amrbashir Jan 8, 2025
1069dd8
fix: translate xdnd coordinates to root window
valadaptive Jan 8, 2025
b3510c2
feat: DnD example/test
valadaptive Jan 8, 2025
d100b86
fix: make DnD compile and function on macOS
valadaptive Jan 8, 2025
9bf7a4e
fix: make DnD compile and function on Windows
valadaptive Jan 8, 2025
7c293f5
chore: format according to nightly rules
valadaptive Jan 8, 2025
35a972f
docs: expand drag event documentation
valadaptive Jan 8, 2025
36438b1
docs(changelog): better document DnD changes
valadaptive Jan 8, 2025
c5e3575
x11: store dnd.position as pair of i16
valadaptive Jan 8, 2025
a2b150b
appkit: use ProtocolObject<dyn NSDraggingInfo>
valadaptive Jan 8, 2025
5bbbedd
windows: only emit DragEnter if enter_is_valid
valadaptive Jan 10, 2025
3d3f6fb
x11: store translated DnD coords
valadaptive Jan 10, 2025
bafb090
x11: don't emit DragLeave without DragEnter
valadaptive Jan 10, 2025
2873e88
refactor: reword drag events to match pointer ones
valadaptive Jan 10, 2025
991b4c8
feat: add position to DragLeft
valadaptive Jan 10, 2025
dcfb62d
macos: implement position on DragLeft
valadaptive Jan 10, 2025
6b4b84b
refactor: DragDrop -> DragDropped
valadaptive Jan 10, 2025
a4be71e
docs: clarify drag events
valadaptive Jan 10, 2025
df8e5ce
examples/dnd: handle RedrawRequested event
valadaptive Jan 13, 2025
f55a534
x11: DnD code style tweaks
valadaptive Jan 17, 2025
2f589b5
windows: import DnD events at top of file
valadaptive Jan 17, 2025
59dd7ed
windows: drop_handler.enter_is_valid -> valid
valadaptive Jan 17, 2025
2153865
windows: in DnD, always set pdwEffect
valadaptive Jan 17, 2025
731a0ce
Don't rename protocol parameter
madsmtm Jan 28, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions examples/dnd.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
use std::error::Error;

use winit::application::ApplicationHandler;
use winit::event::WindowEvent;
use winit::event_loop::{ActiveEventLoop, EventLoop};
use winit::window::{Window, WindowAttributes, WindowId};

#[path = "util/fill.rs"]
mod fill;
#[path = "util/tracing.rs"]
mod tracing;

fn main() -> Result<(), Box<dyn Error>> {
tracing::init();

let event_loop = EventLoop::new()?;

let app = Application::new();
Ok(event_loop.run_app(app)?)
}

/// Application state and event handling.
struct Application {
window: Option<Box<dyn Window>>,
}

impl Application {
fn new() -> Self {
Self { window: None }
}
}

impl ApplicationHandler for Application {
fn can_create_surfaces(&mut self, event_loop: &dyn ActiveEventLoop) {
let window_attributes =
WindowAttributes::default().with_title("Drag and drop files on me!");
self.window = Some(event_loop.create_window(window_attributes).unwrap());
}

fn window_event(
&mut self,
event_loop: &dyn ActiveEventLoop,
_window_id: WindowId,
event: WindowEvent,
) {
match event {
WindowEvent::DragLeft { .. }
| WindowEvent::DragEntered { .. }
| WindowEvent::DragMoved { .. }
| WindowEvent::DragDropped { .. } => {
println!("{:?}", event);
},
WindowEvent::RedrawRequested => {
let window = self.window.as_ref().unwrap();
window.pre_present_notify();
fill::fill_window(window.as_ref());
},
WindowEvent::CloseRequested => {
event_loop.exit();
},
_ => {},
}
}
}
7 changes: 4 additions & 3 deletions examples/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -541,11 +541,12 @@ impl ApplicationHandler for Application {
info!("Smart zoom");
},
WindowEvent::TouchpadPressure { .. }
| WindowEvent::HoveredFileCancelled
| WindowEvent::DragLeft { .. }
| WindowEvent::KeyboardInput { .. }
| WindowEvent::PointerEntered { .. }
| WindowEvent::DroppedFile(_)
| WindowEvent::HoveredFile(_)
| WindowEvent::DragEntered { .. }
| WindowEvent::DragMoved { .. }
| WindowEvent::DragDropped { .. }
| WindowEvent::Destroyed
| WindowEvent::Moved(_) => (),
}
Expand Down
19 changes: 19 additions & 0 deletions src/changelog/unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,25 @@ changelog entry.
- Rename `VideoModeHandle` to `VideoMode`, now it only stores plain data.
- Make `Fullscreen::Exclusive` contain `(MonitorHandle, VideoMode)`.
- On Wayland, no longer send an explicit clearing `Ime::Preedit` just prior to a new `Ime::Preedit`.
- Reworked the file drag-and-drop API.

The `WindowEvent::DroppedFile`, `WindowEvent::HoveredFile` and `WindowEvent::HoveredFileCancelled`
events have been removed, and replaced with `WindowEvent::DragEntered`, `WindowEvent::DragMoved`,
`WindowEvent::DragDropped` and `WindowEvent::DragLeft`.

The old drag-and-drop events were emitted once per file. This occurred when files were *first*
hovered over the window, dropped, or left the window. The new drag-and-drop events are emitted
once per set of files dragged, and include a list of all dragged files. They also include the
pointer position.

The rough correspondence is:
- `WindowEvent::HoveredFile` -> `WindowEvent::DragEntered`
- `WindowEvent::DroppedFile` -> `WindowEvent::DragDropped`
- `WindowEvent::HoveredFileCancelled` -> `WindowEvent::DragLeft`

The `WindowEvent::DragMoved` event is entirely new, and is emitted whenever the pointer moves
whilst files are being dragged over the window. It doesn't contain any file paths, just the
pointer position.

### Removed

Expand Down
71 changes: 46 additions & 25 deletions src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,28 +175,42 @@ pub enum WindowEvent {
/// The window has been destroyed.
Destroyed,

/// A file is being hovered over the window.
///
/// When the user hovers multiple files at once, this event will be emitted for each file
/// separately.
HoveredFile(PathBuf),

/// A file has been dropped into the window.
///
/// When the user drops multiple files at once, this event will be emitted for each file
/// separately.
///
/// The support for this is known to be incomplete, see [#720] for more
/// information.
///
/// [#720]: https://github.com/rust-windowing/winit/issues/720
DroppedFile(PathBuf),

/// A file was hovered, but has exited the window.
///
/// There will be a single `HoveredFileCancelled` event triggered even if multiple files were
/// hovered.
HoveredFileCancelled,
/// A file drag operation has entered the window.
DragEntered {
/// List of paths that are being dragged onto the window.
paths: Vec<PathBuf>,
/// (x,y) coordinates in pixels relative to the top-left corner of the window. May be
/// negative on some platforms if something is dragged over a window's decorations (title
/// bar, frame, etc).
position: PhysicalPosition<f64>,
Comment on lines +182 to +185
Copy link
Member

Choose a reason for hiding this comment

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

Nit: This extra text with "may be negative" sounds confusing, and probably only relevant on Wayland given that the coordinates are actually in window coordinates, and not surface coordinates. But that's to be resolved in #3891.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By "surface coordinates", do you mean relative to the contents of the window? Because the drag position is currently in surface coordinates, to match the pointer position. Unless I'm misunderstanding.

If you run the dnd example and drag a file over the window's title bar, the y-position will be negative in macOS and Windows. On X11, no drag events are emitted unless you're dragging over the window's contents/surface.

Copy link
Contributor Author

@valadaptive valadaptive Jan 28, 2025

Choose a reason for hiding this comment

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

My understanding is that "window coordinates" are relative to the top-left corner of the window including decorations, and that "surface coordinates" are relative to the top-left corner of the window's contents. Pointer events are in surface coordinates.

DnD events in Windows and X11 were also in surface coordinates in the original PR, so I changed macOS to match those platforms (EDIT: and also the pointer events).

},
/// A file drag operation has moved over the window.
DragMoved {
/// (x,y) coordinates in pixels relative to the top-left corner of the window. May be
/// negative on some platforms if something is dragged over a window's decorations (title
/// bar, frame, etc).
position: PhysicalPosition<f64>,
},
/// The file drag operation has dropped file(s) on the window.
DragDropped {
/// List of paths that are being dragged onto the window.
paths: Vec<PathBuf>,
/// (x,y) coordinates in pixels relative to the top-left corner of the window. May be
/// negative on some platforms if something is dragged over a window's decorations (title
/// bar, frame, etc).
position: PhysicalPosition<f64>,
},
/// The file drag operation has been cancelled or left the window.
DragLeft {
/// (x,y) coordinates in pixels relative to the top-left corner of the window. May be
/// negative on some platforms if something is dragged over a window's decorations (title
/// bar, frame, etc).
///
/// ## Platform-specific
///
/// - **Windows:** Always emits [`None`].
position: Option<PhysicalPosition<f64>>,
},

/// The window gained or lost focus.
///
Expand Down Expand Up @@ -1221,9 +1235,16 @@ mod tests {
with_window_event(Focused(true));
with_window_event(Moved((0, 0).into()));
with_window_event(SurfaceResized((0, 0).into()));
with_window_event(DroppedFile("x.txt".into()));
with_window_event(HoveredFile("x.txt".into()));
with_window_event(HoveredFileCancelled);
with_window_event(DragEntered {
paths: vec!["x.txt".into()],
position: (0, 0).into(),
});
with_window_event(DragMoved { position: (0, 0).into() });
with_window_event(DragDropped {
paths: vec!["x.txt".into()],
position: (0, 0).into(),
});
with_window_event(DragLeft { position: Some((0, 0).into()) });
with_window_event(Ime(Enabled));
with_window_event(PointerMoved {
device_id: None,
Expand Down
68 changes: 53 additions & 15 deletions src/platform_impl/apple/appkit/window_delegate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use objc2::{declare_class, msg_send_id, mutability, sel, ClassType, DeclaredClas
use objc2_app_kit::{
NSAppKitVersionNumber, NSAppKitVersionNumber10_12, NSAppearance, NSAppearanceCustomization,
NSAppearanceNameAqua, NSApplication, NSApplicationPresentationOptions, NSBackingStoreType,
NSColor, NSDraggingDestination, NSFilenamesPboardType, NSPasteboard,
NSColor, NSDraggingDestination, NSDraggingInfo, NSFilenamesPboardType,
NSRequestUserAttentionType, NSScreen, NSToolbar, NSView, NSViewFrameDidChangeNotification,
NSWindow, NSWindowButton, NSWindowDelegate, NSWindowFullScreenButton, NSWindowLevel,
NSWindowOcclusionState, NSWindowOrderingMode, NSWindowSharingType, NSWindowStyleMask,
Expand Down Expand Up @@ -375,19 +375,45 @@ declare_class!(
unsafe impl NSDraggingDestination for WindowDelegate {
/// Invoked when the dragged image enters destination bounds or frame
#[method(draggingEntered:)]
fn dragging_entered(&self, sender: &NSObject) -> bool {
fn dragging_entered(&self, sender: &ProtocolObject<dyn NSDraggingInfo>) -> bool {
trace_scope!("draggingEntered:");

use std::path::PathBuf;

let pb: Retained<NSPasteboard> = unsafe { msg_send_id![sender, draggingPasteboard] };
let pb = unsafe { sender.draggingPasteboard() };
let filenames = pb.propertyListForType(unsafe { NSFilenamesPboardType }).unwrap();
let filenames: Retained<NSArray<NSString>> = unsafe { Retained::cast(filenames) };
let paths = filenames
.into_iter()
.map(|file| PathBuf::from(file.to_string()))
.collect();

filenames.into_iter().for_each(|file| {
let path = PathBuf::from(file.to_string());
self.queue_event(WindowEvent::HoveredFile(path));
});
let dl = unsafe { sender.draggingLocation() };
let dl = self.view().convertPoint_fromView(dl, None);
let position = LogicalPosition::<f64>::from((dl.x, dl.y)).to_physical(self.scale_factor());


self.queue_event(WindowEvent::DragEntered { paths, position });

true
}

#[method(wantsPeriodicDraggingUpdates)]
fn wants_periodic_dragging_updates(&self) -> bool {
trace_scope!("wantsPeriodicDraggingUpdates:");
true
}

/// Invoked periodically as the image is held within the destination area, allowing modification of the dragging operation or mouse-pointer position.
#[method(draggingUpdated:)]
fn dragging_updated(&self, sender: &ProtocolObject<dyn NSDraggingInfo>) -> bool {
trace_scope!("draggingUpdated:");

let dl = unsafe { sender.draggingLocation() };
let dl = self.view().convertPoint_fromView(dl, None);
let position = LogicalPosition::<f64>::from((dl.x, dl.y)).to_physical(self.scale_factor());

self.queue_event(WindowEvent::DragMoved { position });

true
}
Expand All @@ -401,19 +427,24 @@ declare_class!(

/// Invoked after the released image has been removed from the screen
#[method(performDragOperation:)]
fn perform_drag_operation(&self, sender: &NSObject) -> bool {
fn perform_drag_operation(&self, sender: &ProtocolObject<dyn NSDraggingInfo>) -> bool {
trace_scope!("performDragOperation:");

use std::path::PathBuf;

let pb: Retained<NSPasteboard> = unsafe { msg_send_id![sender, draggingPasteboard] };
let pb = unsafe { sender.draggingPasteboard() };
let filenames = pb.propertyListForType(unsafe { NSFilenamesPboardType }).unwrap();
let filenames: Retained<NSArray<NSString>> = unsafe { Retained::cast(filenames) };
let paths = filenames
.into_iter()
.map(|file| PathBuf::from(file.to_string()))
.collect();

filenames.into_iter().for_each(|file| {
let path = PathBuf::from(file.to_string());
self.queue_event(WindowEvent::DroppedFile(path));
});
let dl = unsafe { sender.draggingLocation() };
let dl = self.view().convertPoint_fromView(dl, None);
let position = LogicalPosition::<f64>::from((dl.x, dl.y)).to_physical(self.scale_factor());

self.queue_event(WindowEvent::DragDropped { paths, position });

true
}
Expand All @@ -426,9 +457,16 @@ declare_class!(

/// Invoked when the dragging operation is cancelled
#[method(draggingExited:)]
fn dragging_exited(&self, _sender: Option<&NSObject>) {
fn dragging_exited(&self, info: Option<&ProtocolObject<dyn NSDraggingInfo>>) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For @madsmtm: is this the right way to represent a sender param that may be null? The Swift docs note that for this specific callback, this parameter is nullable.

(As a side node, kinda alarming how the Objective-C docs just don't mention its nullability at all, as far as I can tell. Any idea what's going on there?)

Copy link
Member

Choose a reason for hiding this comment

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

Yes, this is correct, see also the docs in objc2-app-kit. In the future, I plan to make it required to have the same signature as in objc2-app-kit, but haven't gotten around to it yet.

The docs on Apple's website don't mention it, probably to be less verbose, but the header does contain a nullability attribute:

// AppKit.framework/Headers/NSDragging.h
- (void)draggingExited:(nullable id <NSDraggingInfo>)sender;

Copy link
Member

Choose a reason for hiding this comment

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

Nit: Please keep the name as sender.

trace_scope!("draggingExited:");
self.queue_event(WindowEvent::HoveredFileCancelled);

let position = info.map(|info| {
let dl = unsafe { info.draggingLocation() };
let dl = self.view().convertPoint_fromView(dl, None);
LogicalPosition::<f64>::from((dl.x, dl.y)).to_physical(self.scale_factor())
});

self.queue_event(WindowEvent::DragLeft { position } );
}
}

Expand Down
16 changes: 15 additions & 1 deletion src/platform_impl/linux/x11/dnd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::path::{Path, PathBuf};
use std::str::Utf8Error;
use std::sync::Arc;

use dpi::PhysicalPosition;
use percent_encoding::percent_decode;
use x11rb::protocol::xproto::{self, ConnectionExt};

Expand Down Expand Up @@ -45,20 +46,33 @@ pub struct Dnd {
pub type_list: Option<Vec<xproto::Atom>>,
// Populated by XdndPosition event handler
pub source_window: Option<xproto::Window>,
// Populated by XdndPosition event handler
pub position: PhysicalPosition<f64>,
// Populated by SelectionNotify event handler (triggered by XdndPosition event handler)
pub result: Option<Result<Vec<PathBuf>, DndDataParseError>>,
// Populated by SelectionNotify event handler (triggered by XdndPosition event handler)
pub dragging: bool,
}

impl Dnd {
pub fn new(xconn: Arc<XConnection>) -> Result<Self, X11Error> {
Ok(Dnd { xconn, version: None, type_list: None, source_window: None, result: None })
Ok(Dnd {
xconn,
version: None,
type_list: None,
source_window: None,
position: PhysicalPosition::default(),
result: None,
dragging: false,
})
}

pub fn reset(&mut self) {
self.version = None;
self.type_list = None;
self.source_window = None;
self.result = None;
self.dragging = false;
}

pub unsafe fn send_status(
Expand Down
Loading