Merge branch 'feat/finder-drop-pointer' into feat/drop-loads-place-cursor

# Conflicts:
#	crates/ui-egui/src/app/input.rs

apps/designcraft/src/pointer.rs

@@ -11,6 +11,33 @@ pub fn screen_position() -> Option<egui::Pos2> {
     // AppKit's origin is the bottom left of the first screen, the one with the menu bar.
     let height = NSScreen::screens(mtm).firstObject()?.frame().size.height;
     let at = NSEvent::mouseLocation();
-    let (x, y) = (at.x, height - at.y);
+    from_appkit(at.x, at.y, height)
+}
+
+/// A point in AppKit's global coordinates (origin at the bottom left of the main display, the one
+/// with the menu bar, y up) in winit's (origin at its top left, y down), given that display's
+/// height. Only the main display's height: winit flips window positions the same way
+/// (`flip_window_screen_coordinates`, against `CGMainDisplayID`), whatever display the window is
+/// on, so a display left of or above the main one gives negative coordinates in both.
+fn from_appkit(x: f64, y: f64, main_height: f64) -> Option<egui::Pos2> {
+    let y = main_height - y;
     (x.is_finite() && y.is_finite()).then(|| egui::pos2(x as f32, y as f32))
 }
+
+#[cfg(test)]
+mod tests {
+    use super::from_appkit;
+
+    #[test]
+    fn appkit_points_flip_against_the_main_display_only() {
+        // The main display is 1000 points high: its top left is (0, 0), its bottom left (0, 1000).
+        assert_eq!(from_appkit(0.0, 1000.0, 1000.0), Some(egui::pos2(0.0, 0.0)));
+        assert_eq!(from_appkit(200.0, 100.0, 1000.0), Some(egui::pos2(200.0, 900.0)));
+        // A second display above the main one, and one to its left: negative, as winit reports a
+        // window there.
+        assert_eq!(from_appkit(300.0, 1200.0, 1000.0), Some(egui::pos2(300.0, -200.0)));
+        assert_eq!(from_appkit(-800.0, 500.0, 1000.0), Some(egui::pos2(-800.0, 500.0)));
+        assert_eq!(from_appkit(f64::NAN, 0.0, 1000.0), None);
+        assert_eq!(from_appkit(0.0, 0.0, f64::INFINITY), None);
+    }
+}

crates/ui-egui/src/app.rs

@@ -206,7 +206,7 @@ impl DesignApp {
         // must not replay shortcuts or dropped files from stale input.
         if ctx.current_pass_index() == 0 {
             #[cfg(not(target_arch = "wasm32"))]
-            self.take_pending_drops(&ctx);
+            self.take_pending_drops();
             menus::shortcuts(self, &ctx);
         }
         let t0 = now_ms();

crates/ui-egui/src/app/input.rs

@@ -59,10 +59,13 @@ impl DesignApp {
                 _ => None,
             });
             let generation = self.input_generation();
+            // A drop without pointer events (dragged in from another application) asks the
+            // system now, while the pointer is still where the files were let go.
+            let system = if pointer.is_none() && !raw.dropped_files.is_empty() { self.system_pointer(ctx, raw) } else { None };
             for file in std::mem::take(&mut raw.dropped_files) {
                 let path = file.path().to_string_lossy().to_string();
                 if !path.is_empty() {
-                    self.pending_drops.push(PendingDrop { path, pointer, generation });
+                    self.pending_drops.push(PendingDrop { path, pointer, system, generation });
                 }
             }
         }
@@ -144,9 +147,9 @@ impl DesignApp {
 
     /// Open or place the files dropped since the last UI pass, at the pointer. Files dropped
     /// without pointer events (dragged in from another application) are placed where the system
-    /// says the pointer is, so one dropped on a frame still fills it.
+    /// said the pointer was when they arrived, so one dropped on a frame still fills it.
     #[cfg(not(target_arch = "wasm32"))]
-    pub(crate) fn take_pending_drops(&mut self, ctx: &egui::Context) {
+    pub(crate) fn take_pending_drops(&mut self) {
         // Sample the generation once for the whole batch: placing the first valid file must not
         // suppress later files that belong to the same captured batch.
         let current = self.input_generation();
@@ -155,23 +158,19 @@ impl DesignApp {
             return;
         }
         // Dropped together, they go together: several images load the place cursor at once. Each
-        // file keeps the pointer it was dropped at.
-        let system = drops.iter().any(|d| d.pointer.is_none()).then(|| self.system_pointer(ctx)).flatten();
-        self.open_dropped_batch(drops.into_iter().map(|d| (d.path, d.pointer.or(system))).collect());
+        // file keeps the pointer it was dropped at, or the system's from when it arrived.
+        self.open_dropped_batch(drops.into_iter().map(|d| (d.path, d.pointer.or(d.system))).collect());
     }
 
     /// Where the pointer is in the window, asked of the system ([`crate::Services::pointer`]),
     /// for files dropped without pointer events: dragged in from another application, they bring
-    /// none. `None` when the system doesn't say or the window's place isn't known.
-    pub(crate) fn system_pointer(&self, ctx: &egui::Context) -> Option<egui::Pos2> {
+    /// none. Converted with the window's place in `raw` (that input's own viewport info, else the
+    /// last pass's). `None` when the system doesn't say or the window's place isn't known.
+    #[cfg(not(target_arch = "wasm32"))]
+    fn system_pointer(&self, ctx: &egui::Context, raw: &egui::RawInput) -> Option<egui::Pos2> {
         let at = (self.services.pointer.as_ref()?)()?;
-        // The system's points are unscaled; egui's (the window's rect too) are divided by the
-        // UI scale.
-        let zoom = ctx.zoom_factor();
-        let at = if zoom.is_finite() && zoom > 0.0 { at / zoom } else { at };
-        let inner = ctx.input(|i| i.viewport().inner_rect)?;
-        let local = at - inner.min.to_vec2();
-        (local.x.is_finite() && local.y.is_finite()).then_some(local)
+        let inner = raw.viewports.get(&raw.viewport_id).and_then(|v| v.inner_rect).or_else(|| ctx.input(|i| i.viewport().inner_rect))?;
+        screen_to_window(at, ctx.zoom_factor(), inner)
     }
 
     /// The current state an injected batch belongs to.
@@ -221,3 +220,12 @@ impl DesignApp {
         self.control_input_frame.store(false, std::sync::atomic::Ordering::Relaxed);
     }
 }
+
+/// A point on the screen in the system's unscaled points, in the window whose content is `inner`
+/// (egui's points, divided by the UI scale `zoom`). `None` when the result isn't finite.
+#[cfg(not(target_arch = "wasm32"))]
+fn screen_to_window(at: egui::Pos2, zoom: f32, inner: egui::Rect) -> Option<egui::Pos2> {
+    let at = if zoom.is_finite() && zoom > 0.0 { at / zoom } else { at };
+    let local = at - inner.min.to_vec2();
+    (local.x.is_finite() && local.y.is_finite()).then_some(local)
+}

crates/ui-egui/src/lib.rs

@@ -111,6 +111,10 @@ struct InputGeneration {
 struct PendingDrop {
     path: String,
     pointer: Option<egui::Pos2>,
+    /// Where the system said the pointer was in the window when this drop's raw input arrived,
+    /// for a drop that brought no pointer events ([`crate::Services::pointer`]). Each drop keeps
+    /// its own, converted with the window's place and the UI scale of that input.
+    system: Option<egui::Pos2>,
     generation: InputGeneration,
 }
 

crates/ui-egui/src/services.rs

@@ -47,6 +47,7 @@ pub struct Services {
     /// The pointer's place on the screen, asked when a file is dropped: a file dragged in from
     /// another application brings no pointer events with it (winit reports none during a drag on
     /// macOS), so without it a drop can't tell which frame it landed on. `None`: a drop without
-    /// pointer events loads the place cursor.
+    /// pointer events fills no frame: an image loads the place cursor, and other files land in the
+    /// middle of the view (`canvas::drop_point`).
     pub pointer: Option<PointerFn>,
 }

crates/ui-egui/src/tests.rs

@@ -909,6 +909,15 @@ fn a_graphic_dropped_on_a_frame_fills_it() {
     assert!(matches!(d.item(designcraft_doc::ItemId(frame)).unwrap().content, designcraft_doc::Content::Graphic(_)));
 }
 
+/// The raw input of a drop without pointer events, with the window's content at `inner`.
+#[cfg(not(target_arch = "wasm32"))]
+fn pointerless_drop(path: &std::path::Path, inner: egui::Rect) -> egui::RawInput {
+    let mut raw = egui::RawInput { screen_rect: Some(egui::Rect::from_min_size(egui::Pos2::ZERO, inner.size())), ..Default::default() };
+    raw.viewports.insert(egui::ViewportId::ROOT, egui::ViewportInfo { inner_rect: Some(inner), ..Default::default() });
+    raw.dropped_files.push(dropped_file(path));
+    raw
+}
+
 /// A file dragged in from the Finder brings no pointer events (winit sends none during a drag on
 /// macOS): the drop asks the system where the pointer is, so one dropped on a frame fills it.
 #[cfg(not(target_arch = "wasm32"))]
@@ -921,13 +930,13 @@ fn a_file_dropped_without_pointer_events_fills_the_frame_under_the_system_pointe
         let window = egui::vec2(300.0, 200.0);
         app.services.pointer = Some(Box::new(move || Some(((inside + window).to_vec2() * zoom).to_pos2())));
         let path = dropped_png("drop-system-pointer");
-        app.pending_drops.push(crate::PendingDrop { path: path.to_string_lossy().into_owned(), pointer: None, generation: app.input_generation() });
         let ctx = egui::Context::default();
         ctx.set_zoom_factor(zoom);
-        let rect = egui::Rect::from_min_size(egui::Pos2::ZERO + window, egui::vec2(1400.0, 900.0));
-        let mut raw = egui::RawInput { screen_rect: Some(egui::Rect::from_min_size(egui::Pos2::ZERO, rect.size())), ..Default::default() };
-        raw.viewports.insert(egui::ViewportId::ROOT, egui::ViewportInfo { inner_rect: Some(rect), ..Default::default() });
-        ctx.run_ui(raw, |ui| app.take_pending_drops(ui.ctx())).textures_delta.clear();
+        // The zoom takes effect from the next pass, as it has by the time the system drops a file.
+        ctx.run_ui(egui::RawInput::default(), |_| {}).textures_delta.clear();
+        let mut raw = pointerless_drop(&path, egui::Rect::from_min_size(egui::Pos2::ZERO + window, egui::vec2(1400.0, 900.0)));
+        app.raw_input_hook(&ctx, &mut raw);
+        ctx.run_ui(raw, |_| app.take_pending_drops()).textures_delta.clear();
         let _ = std::fs::remove_file(&path);
         assert!(app.pending_drops.is_empty());
         assert!(app.session.loaded.is_none(), "nothing waits in the place cursor at {zoom}");
@@ -940,6 +949,65 @@ fn a_file_dropped_without_pointer_events_fills_the_frame_under_the_system_pointe
     }
 }
 
+/// A drop without pointer events keeps the place the system gave when its input arrived,
+/// converted with that input's window: the pointer and the window moving before the UI pass that
+/// places it don't move the drop.
+#[cfg(not(target_arch = "wasm32"))]
+#[test]
+fn a_pointerless_drop_keeps_the_system_pointer_of_its_own_input() {
+    let (mut app, frame, inside, outside) = app_with_drop_frame();
+    let at = std::rc::Rc::new(std::cell::Cell::new(inside));
+    let reported = at.clone();
+    app.services.pointer = Some(Box::new(move || Some(reported.get())));
+    let ctx = egui::Context::default();
+    let window = egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(1400.0, 900.0));
+    let path = dropped_png("drop-own-input");
+    let mut raw = pointerless_drop(&path, window);
+    app.raw_input_hook(&ctx, &mut raw);
+    // Before the pass that places it, the window moves and the pointer leaves the frame; a later
+    // input with no drop doesn't ask the system again.
+    let moved = window.translate(egui::vec2(50.0, 40.0));
+    at.set(outside + moved.min.to_vec2());
+    let mut later = egui::RawInput { screen_rect: Some(egui::Rect::from_min_size(egui::Pos2::ZERO, moved.size())), ..Default::default() };
+    later.viewports.insert(egui::ViewportId::ROOT, egui::ViewportInfo { inner_rect: Some(moved), ..Default::default() });
+    app.raw_input_hook(&ctx, &mut later);
+    ctx.run_ui(later, |_| app.take_pending_drops()).textures_delta.clear();
+    let _ = std::fs::remove_file(&path);
+    let d = &app.session.doc().unwrap().doc;
+    assert_eq!(d.spreads[0].items.len(), 1, "no new object");
+    let designcraft_doc::Content::Graphic(g) = &d.item(designcraft_doc::ItemId(frame)).unwrap().content else {
+        panic!("the file fills the frame it was dropped on")
+    };
+    assert!(d.assets[&g.asset].name.contains("drop-own-input"));
+}
+
+/// A filled compound path (Object > Compound Paths > Make) is a frame a dropped image fills, as
+/// in InDesign, through its ring and not its hole.
+#[test]
+fn a_graphic_dropped_on_a_compound_path_fills_it() {
+    let (mut app, frame, _, _) = app_with_drop_frame();
+    let hole = app.run("frame.create", json!({"rect": [66, 66, 106, 106]})).unwrap()["id"].as_u64().unwrap();
+    assert_eq!(app.run("object.makeCompoundPath", json!({"ids": [frame, hole]})).unwrap()["id"].as_u64(), Some(frame));
+    app.run("object.fill", json!({"ids": [frame], "swatch": "[Black]"})).unwrap();
+    let (view, rect) = (app.view().unwrap(), app.canvas_rect.unwrap());
+    let st = app.session.active().unwrap();
+    let layout = designcraft_tools::CanvasLayout::new(&st.doc, false);
+    let slot = layout.slot(designcraft_doc::SpreadRef::Doc(0)).unwrap();
+    let screen = |x, y| canvas::Xf::new(rect, view).to_screen(slot.to_canvas(designcraft_geom::Point::new(x, y)));
+    // On the ring, between the outer edge (36) and the hole (66); the hole's middle is no frame.
+    let ring = screen(50.0, 50.0);
+    assert!(canvas::drop_frame(&app, Some(screen(86.0, 86.0))).is_none(), "the hole");
+    let path = dropped_png("drop-compound");
+    let r = app.open_dropped_at(&path.to_string_lossy(), Some(ring));
+    let _ = std::fs::remove_file(&path);
+    assert_eq!(r.unwrap()["id"].as_u64(), Some(frame), "the compound path under the pointer is filled");
+    let d = &app.session.doc().unwrap().doc;
+    assert_eq!(d.spreads[0].items.len(), 1, "no new object");
+    let it = d.item(designcraft_doc::ItemId(frame)).unwrap();
+    assert!(matches!(it.content, designcraft_doc::Content::Graphic(_)));
+    assert_eq!(it.path.subpaths.len(), 2, "it keeps its hole");
+}
+
 #[test]
 fn a_locked_frame_is_not_a_drop_target() {
     let (mut app, frame, inside, _) = app_with_drop_frame();