From 1788753f6d1ebb1fdc574ca09ec825069d882af9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Sat, 26 Sep 2026 16:38:22 -0400 Subject: [PATCH] Media: a shaped corner follows the pointer up and down; the fitted crop is told once With an aspect, a corner drag sizes the crop by whichever side the pointer moved further, so a drag straight up or down resizes it too. The upload tells the host the fitted crop once per file, whether the host keeps it or not; a host that dropped it had set off a loop of reports. The annotator's Escape stops only when it lets a chosen mark go. --- TASKS.md | 1 + src/media/annotate.rs | 13 ++++--- src/media/crop.rs | 43 +++++++++++++++++++++-- src/media/tests/edit.rs | 76 ++++++++++++++++++++++++++++++++--------- src/media/upload.rs | 17 ++++++--- 5 files changed, 124 insertions(+), 26 deletions(-) diff --git a/TASKS.md b/TASKS.md index ad20822..3d9c0ba 100644 --- a/TASKS.md +++ b/TASKS.md @@ -309,3 +309,4 @@ Per-component lines live in `tasks/`. Tags there: | T25f | 2 | PASS | | | T26a | 1 | FAIL | the viewer's picture had no id, so an animated picture never left its first frame | | T26a | 2 | PASS | | +| T26b | 1 | FAIL | a shaped crop's corner ignored the pointer's move up or down | diff --git a/src/media/annotate.rs b/src/media/annotate.rs index 3ea60cb..3e41be4 100644 --- a/src/media/annotate.rs +++ b/src/media/annotate.rs @@ -407,10 +407,15 @@ impl RenderOnce for ImageAnnotator { match (stroke.key.as_str(), command, stroke.modifiers.shift) { ("z", true, back) => keyed.step(!back, window, cx), ("delete" | "backspace", false, _) => keyed.remove(window, cx), - ("escape", false, _) => keyed.hand.update(cx, |hand, cx| { - hand.chosen = None; - cx.notify(); - }), + ("escape", false, _) => { + let chose = keyed.hand.update(cx, |hand, cx| { + cx.notify(); + hand.chosen.take().is_some() + }); + if !chose { + return; + } + } (key, false, false) => { match TOOLS.iter().find(|(_, each, ..)| *each == key) { Some((tool, ..)) => keyed.pick(*tool, cx), diff --git a/src/media/crop.rs b/src/media/crop.rs index fb338eb..6b940fa 100644 --- a/src/media/crop.rs +++ b/src/media/crop.rs @@ -72,7 +72,7 @@ pub(crate) fn grip_at(crop: Crop, at: (f32, f32), reach: (f32, f32), shaped: boo } } -/// The crop after `grip` moves by `by`, in shares, kept inside the picture with sides of at least `least`. With `shape`, a width over height in shares, only corners resize and the box keeps that shape, anchored at the opposite corner. +/// The crop after `grip` moves by `by`, in shares, kept inside the picture with sides of at least `least`. With `shape`, a width over height in shares, only corners resize and the box keeps that shape, anchored at the opposite corner, sized by whichever side the pointer moved further. pub(crate) fn dragged( crop: Crop, grip: Grip, @@ -108,7 +108,13 @@ pub(crate) fn dragged( if let Some(shape) = shape { let tall = if grip.top { y2 } else { 1.0 - y1 }; let wide = if grip.left { x2 } else { 1.0 - x1 }; - let w = (x2 - x1) + let (across, down) = (x2 - x1, (y2 - y1) * shape); + let asked = if (across - crop.w).abs() >= (down - crop.w).abs() { + across + } else { + down + }; + let w = asked .max(least * shape.max(1.0)) .min(tall * shape) .min(wide); @@ -252,6 +258,39 @@ mod tests { assert_eq!(dragged(HALF, edge, (0.0, -0.1), 0.05, Some(1.0)), HALF); } + #[test] + fn a_shaped_corner_follows_the_pointer_up_and_down_too() { + let bottom_right = Grip { + right: true, + bottom: true, + ..Grip::default() + }; + let down = dragged(HALF, bottom_right, (0.0, 0.2), 0.05, Some(1.0)); + assert!( + near( + down, + Crop { + w: 0.7, + h: 0.7, + ..HALF + } + ), + "straight down grows it: {down:?}" + ); + let up = dragged(HALF, bottom_right, (0.0, -0.2), 0.05, Some(1.0)); + assert!( + near( + up, + Crop { + w: 0.3, + h: 0.3, + ..HALF + } + ), + "straight up shrinks it: {up:?}" + ); + } + #[test] fn a_new_shape_takes_the_largest_centered_box_inside() { let wide = fitted(HALF, 2.0); diff --git a/src/media/tests/edit.rs b/src/media/tests/edit.rs index ff51aa4..cc93fb5 100644 --- a/src/media/tests/edit.rs +++ b/src/media/tests/edit.rs @@ -1,8 +1,8 @@ use std::path::PathBuf; use gpui::{ - Context, Entity, IntoElement, Modifiers, MouseButton, ParentElement, Pixels, Point, Render, - Styled, TestAppContext, VisualTestContext, Window, div, point, px, + Context, Entity, InteractiveElement, IntoElement, Modifiers, MouseButton, ParentElement, + Pixels, Point, Render, Styled, TestAppContext, VisualTestContext, Window, div, point, px, }; use super::{picture, press, settle, setup}; @@ -161,10 +161,11 @@ fn a_cropper_with_no_handler_takes_no_tab(cx: &mut TestAppContext) { assert!(cx.update(|window, cx| window.focused(cx)).is_none()); } -/// An upload over a picked picture, and how often it was emptied. +/// An upload over a picked picture, the crops it heard, kept or not, and how often it was emptied. struct Uploading { picked: Option, crop: Option, + keeps: bool, heard: usize, removed: usize, } @@ -177,7 +178,10 @@ impl Render for Uploading { .aspect(Some(1.0)) .on_crop(move |crop, _, cx| { cropped.update(cx, |view, cx| { - (view.crop, view.heard) = (Some(crop), view.heard + 1); + view.heard += 1; + if view.keeps { + view.crop = Some(crop); + } cx.notify(); }) }) @@ -198,6 +202,7 @@ fn a_picked_picture_crops_from_a_fitted_box_and_remove_empties_it(cx: &mut TestA let (view, cx) = cx.add_window_view(|_, _| Uploading { picked, crop: None, + keeps: true, heard: 0, removed: 0, }); @@ -230,32 +235,63 @@ fn a_picked_picture_crops_from_a_fitted_box_and_remove_empties_it(cx: &mut TestA assert_eq!(view.read_with(cx, |view, _| view.removed), 1); } +#[gpui::test] +fn a_host_that_keeps_no_crop_still_hears_the_fitted_one_once(cx: &mut TestAppContext) { + setup(cx); + let picked = Some(picture("ignored", 300, 100)); + let (view, cx) = cx.add_window_view(|_, _| Uploading { + picked, + crop: None, + keeps: false, + heard: 0, + removed: 0, + }); + settle(cx); + for _ in 0..3 { + view.update(cx, |_, cx| cx.notify()); + settle(cx); + } + assert_eq!(view.read_with(cx, |view, _| view.heard), 1); +} + /// An annotator 400 wide over a picture twice as wide as tall, and its marks. struct Marking { path: PathBuf, marks: Vec, + escapes: usize, } impl Render for Marking { fn render(&mut self, _: &mut Window, cx: &mut Context) -> impl IntoElement { - let owner = cx.entity(); - div().w(px(400.0)).child( - ImageAnnotator::new("marks", self.path.clone(), 2.0, self.marks.clone()).on_change( - move |marks, _, cx| { - owner.update(cx, |view, cx| { - view.marks = marks.to_vec(); - cx.notify(); - }) - }, - ), - ) + let (owner, page) = (cx.entity(), cx.entity()); + div() + .w(px(400.0)) + .on_key_down(move |event, _, cx| { + if event.keystroke.key == "escape" { + page.update(cx, |view, _| view.escapes += 1); + } + }) + .child( + ImageAnnotator::new("marks", self.path.clone(), 2.0, self.marks.clone()).on_change( + move |marks, _, cx| { + owner.update(cx, |view, cx| { + view.marks = marks.to_vec(); + cx.notify(); + }) + }, + ), + ) } } fn marking(marks: Vec, cx: &mut TestAppContext) -> (Entity, &mut VisualTestContext) { setup(cx); let path = picture("marks", 200, 100); - let (view, cx) = cx.add_window_view(|_, _| Marking { path, marks }); + let (view, cx) = cx.add_window_view(|_, _| Marking { + path, + marks, + escapes: 0, + }); cx.update(|window, _| window.activate_window()); settle(cx); (view, cx) @@ -299,11 +335,19 @@ fn select_and_delete_remove_a_mark_and_undo_brings_it_back(cx: &mut TestAppConte let middle = spot("image-annotator", (0.5, 0.5), cx); cx.simulate_click(middle, Modifiers::none()); settle(cx); + press("escape", cx); + let escapes = view.read_with(cx, |view, _| view.escapes); + assert_eq!(escapes, 0, "Escape lets the chosen pin go and stops there"); + cx.simulate_click(middle, Modifiers::none()); + settle(cx); press("backspace", cx); assert!( view.read_with(cx, |view, _| view.marks.is_empty()), "the pin is gone" ); + press("escape", cx); + let escapes = view.read_with(cx, |view, _| view.escapes); + assert_eq!(escapes, 1, "with nothing chosen, Escape passes to the page"); press("cmd-z", cx); assert_eq!(view.read_with(cx, |view, _| view.marks.clone()), [pin]); } diff --git a/src/media/upload.rs b/src/media/upload.rs index 7cac689..0ef0f87 100644 --- a/src/media/upload.rs +++ b/src/media/upload.rs @@ -17,6 +17,13 @@ use crate::{ type OnPath = Rc; +/// What the upload has said about a file: its fitted crop to the host, its failure to the log. +#[derive(Default)] +struct Told { + fitted: bool, + failed: bool, +} + /// A picture to send: a drop zone until one is chosen, then the picture to crop, its name, Replace and Remove. Only pictures are taken; the picture's shape comes from the file. The host keeps the file and its crop; with none yet, the upload reports the largest centered box of its aspect once the picture opens. #[derive(IntoElement)] pub struct ImageUpload { @@ -80,10 +87,10 @@ impl RenderOnce for ImageUpload { }; let decoded = window.use_asset::(&Resource::Path(path.clone().into()), cx); - let failed = window.use_keyed_state( + let told = window.use_keyed_state( (self.id.clone(), path.to_string_lossy().into_owned()), cx, - |_, _| false, + |_, _| Told::default(), ); let name = path .file_name() @@ -102,15 +109,17 @@ impl RenderOnce for ImageUpload { _ => None, }; if let (Some((_, fitted)), None, Some(on_crop)) = (opened, self.crop, self.on_crop.clone()) + && !told.read(cx).fitted { log::info!("image upload: {name} opened, crop fitted to {fitted:?}"); + told.update(cx, |told, _| told.fitted = true); window.defer(cx, move |window, cx| on_crop(fitted, window, cx)); } if let Some(Err(error)) = &decoded - && !*failed.read(cx) + && !told.read(cx).failed { log::error!("image upload: {name} can't be opened: {error}"); - failed.update(cx, |failed, _| *failed = true); + told.update(cx, |told, _| told.failed = true); } let theme = cx.theme(); let colors = theme.colors.clone();