diff --git a/crates/ui-egui/src/color_picker_ui.rs b/crates/ui-egui/src/color_picker_ui.rs index 27177ceb57..3effb27c4b 100644 --- a/crates/ui-egui/src/color_picker_ui.rs +++ b/crates/ui-egui/src/color_picker_ui.rs @@ -484,7 +484,7 @@ fn color_field(ui: &mut egui::Ui, f: &mut Map) { // Photoshop updates the colour on the press itself, not only when the button is released // (#6937): `is_pointer_button_down_on` is true from the press frame onwards (it covers // dragging too), while `clicked` fires on release and `dragged` only after a move. - if let Some(p) = resp.interact_pointer_pos().filter(|_| resp.is_pointer_button_down_on() || resp.clicked()) { + if let Some(p) = widgets::pick_pos(&resp) { let (x, y) = (((p.x - field.left()) / field.width()).clamp(0.0, 1.0), ((p.y - field.top()) / field.height()).clamp(0.0, 1.0)); let c = field_color(&mode, fz, x, y); let keep = match mode.as_str() { @@ -503,7 +503,7 @@ fn color_field(ui: &mut egui::Ui, f: &mut Map) { let tri = vec![pos2(x, sy), pos2(x - 6.0 * dir, sy - 4.0), pos2(x - 6.0 * dir, sy + 4.0)]; ui.painter().add(egui::Shape::convex_polygon(tri, t.text, Stroke::NONE)); } - if let Some(p) = sresp.interact_pointer_pos().filter(|_| sresp.is_pointer_button_down_on() || sresp.clicked()) { + if let Some(p) = widgets::pick_pos(&sresp) { let z = 1.0 - ((p.y - strip.top()) / strip.height()).clamp(0.0, 1.0); let h = match mode.as_str() { "h" => Some([z * 360.0, hsv[1], hsv[2]]), @@ -864,6 +864,95 @@ mod tests { h.state().get("color").and_then(Value::as_str).unwrap().to_string() } + /// Press at `at` and hold, with no release and no time passing, then run one frame. + fn press(h: &mut Harness<'static, S>, at: egui::Pos2) { + press_button(h, at, egui::PointerButton::Primary, true); + } + + fn press_button(h: &mut Harness<'static, S>, at: egui::Pos2, button: egui::PointerButton, pressed: bool) { + h.event(egui::Event::PointerMoved(at)); + h.event(egui::Event::PointerButton { pos: at, button, pressed, modifiers: egui::Modifiers::NONE }); + h.run_steps(1); + } + + /// #7793: the field and the slider pick on the frame the button goes down, not after the + /// pointer has travelled past egui's click threshold or been held for a while, and follow + /// moves of a few points. + #[test] + fn field_and_slider_pick_on_press_and_follow_small_moves() { + let mut f = Map::new(); + f.insert("color".into(), json!("#336699")); + let mut h = Harness::builder().with_size(vec2(400.0, 300.0)).build_ui_state( + |ui, f: &mut Map| { + ui.horizontal(|ui| { + let o = ui.cursor().min; + let gap = ui.spacing().item_spacing.x; + ui.data_mut(|d| d.insert_temp(egui::Id::new("test-origin"), (o, gap))); + color_field(ui, f); + }); + }, + f, + ); + h.run_steps(2); + let (o, gap) = h.ctx.data(|d| d.get_temp::<(egui::Pos2, f32)>(egui::Id::new("test-origin"))).unwrap(); + // Field: saturation across, brightness down (hue mode). + let at = o + vec2(FIELD * 0.5, FIELD * 0.5); + press(&mut h, at); + let picked = color(&h); + assert_ne!(picked, "#336699", "the press itself picks"); + let hsv = rgb_to_hsb(parse_hex(&picked).unwrap()); + assert!((hsv[1] - 0.5).abs() < 0.02 && (hsv[2] - 0.5).abs() < 0.02, "{picked} {hsv:?}"); + // A 3-point nudge (under the 6-point click threshold) moves the pick. + h.event(egui::Event::PointerMoved(at + vec2(3.0, 0.0))); + h.run_steps(1); + assert_ne!(color(&h), picked, "a small drag adjusts the pick"); + h.event(egui::Event::PointerButton { + pos: at + vec2(3.0, 0.0), + button: egui::PointerButton::Primary, + pressed: false, + modifiers: egui::Modifiers::NONE, + }); + h.run_steps(2); + // Hue slider: a press near the bottom picks a hue near 0 at once. + let before = color(&h); + press(&mut h, o + vec2(FIELD + gap + 9.0, FIELD * 0.95)); + assert_ne!(color(&h), before, "the slider picks on press"); + let hue = rgb_to_hsb(parse_hex(&color(&h)).unwrap())[0]; + assert!(hue < 30.0 || hue > 330.0, "hue {hue}"); + } + + /// Only the primary button picks: a right or middle press (and its release) on the field or + /// the slider leaves the colour alone. + #[test] + fn field_and_slider_ignore_secondary_and_middle_presses() { + let mut f = Map::new(); + f.insert("color".into(), json!("#336699")); + let mut h = Harness::builder().with_size(vec2(400.0, 300.0)).build_ui_state( + |ui, f: &mut Map| { + ui.horizontal(|ui| { + let o = ui.cursor().min; + let gap = ui.spacing().item_spacing.x; + ui.data_mut(|d| d.insert_temp(egui::Id::new("test-origin"), (o, gap))); + color_field(ui, f); + }); + }, + f, + ); + h.run_steps(2); + let (o, gap) = h.ctx.data(|d| d.get_temp::<(egui::Pos2, f32)>(egui::Id::new("test-origin"))).unwrap(); + for at in [o + vec2(FIELD * 0.5, FIELD * 0.5), o + vec2(FIELD + gap + 9.0, FIELD * 0.95)] { + for button in [egui::PointerButton::Secondary, egui::PointerButton::Middle] { + press_button(&mut h, at, button, true); + assert_eq!(color(&h), "#336699", "{button:?} press at {at:?}"); + h.event(egui::Event::PointerMoved(at + vec2(3.0, 0.0))); + h.run_steps(1); + press_button(&mut h, at + vec2(3.0, 0.0), button, false); + h.run_steps(1); + assert_eq!(color(&h), "#336699", "{button:?} drag and release at {at:?}"); + } + } + } + #[test] fn hex_field_shows_uppercase_without_rewriting_the_selected_color() { let h = picker("#12abef"); diff --git a/crates/ui-egui/src/panels.rs b/crates/ui-egui/src/panels.rs index 04b6f1ac59..162cf0911a 100644 --- a/crates/ui-egui/src/panels.rs +++ b/crates/ui-egui/src/panels.rs @@ -4345,9 +4345,8 @@ fn color_field(app: &mut PhotocraftApp, ui: &mut egui::Ui) { } ui.painter().add(mesh); ui.painter().rect_stroke(field, 0.0, Stroke::new(1.0, t.separator), StrokeKind::Outside); - if (fresp.dragged() || fresp.clicked()) - && let Some(p) = fresp.interact_pointer_pos() - { + let field_pick = widgets::pick_pos(&fresp); + if let Some(p) = field_pick { hsva.s = ((p.x - field.left()) / field.width()).clamp(0.0, 1.0); hsva.v = 1.0 - ((p.y - field.top()) / field.height()).clamp(0.0, 1.0); } @@ -4370,18 +4369,18 @@ fn color_field(app: &mut PhotocraftApp, ui: &mut egui::Ui) { m2.add_triangle(a, a + 3, a + 2); } ui.painter().add(m2); - if (sresp.dragged() || sresp.clicked()) - && let Some(p) = sresp.interact_pointer_pos() - { + let strip_pick = widgets::pick_pos(&sresp); + if let Some(p) = strip_pick { hsva.h = 1.0 - ((p.y - strip.top()) / strip.height()).clamp(0.0, 0.9999); } let y = strip.top() + (1.0 - hsva.h) * strip.height(); let tri = vec![pos2(strip.right() + 1.0, y), pos2(strip.right() + 6.0, y - 4.0), pos2(strip.right() + 6.0, y + 4.0)]; ui.painter().add(egui::Shape::convex_polygon(tri, t.text, Stroke::NONE)); - if fresp.dragged() || fresp.clicked() || sresp.dragged() || sresp.clicked() { + if field_pick.is_some() || strip_pick.is_some() { if bg_active { app.session.tools.background = hsva_srgb(hsva); - } else { + } else if app.session.tools.foreground != hsva_srgb(hsva) { + // A held, unmoved press repeats the pick: restyle selected type only on a change. app.session.tools.foreground = hsva_srgb(hsva); crate::type_tool::foreground_changed(app); crate::shape_fill_ui::foreground_changed(app, crate::shape_fill_ui::gesture(ui)); @@ -5000,6 +4999,80 @@ mod color_tests { } } + /// #7793: the Color panel's field and hue strip pick on the frame the button goes down (no + /// release, no time passing) and follow moves smaller than egui's 6-point click threshold. + #[test] + fn color_panel_field_and_hue_strip_pick_on_press() { + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default()); + app.session.tools.foreground = [0.2, 0.4, 0.6, 1.0]; + let mut h = egui_kittest::Harness::builder().with_size(vec2(300.0, 300.0)).build_ui_state( + |ui, app: &mut PhotocraftApp| { + let at = (ui.cursor().min, ui.available_width()); + ui.data_mut(|d| d.insert_temp(egui::Id::new("test-origin"), at)); + color_field(app, ui); + }, + app, + ); + PhotocraftApp::setup_context(&h.ctx, crate::theme::ThemeKind::Studio); + h.run_steps(2); + let (o, w) = h.ctx.data(|d| d.get_temp::<(Pos2, f32)>(egui::Id::new("test-origin"))).unwrap(); + let field_w = w - 38.0 - 14.0 - 16.0; + let press = |h: &mut egui_kittest::Harness<'static, PhotocraftApp>, at: Pos2, pressed: bool| { + h.event(egui::Event::PointerMoved(at)); + h.event(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed, modifiers: egui::Modifiers::NONE }); + h.run_steps(1); + }; + let at = o + vec2(38.0 + 8.0 + field_w * 0.5, 60.0); + press(&mut h, at, true); + let picked = h.state().session.tools.foreground; + assert_ne!(picked, [0.2, 0.4, 0.6, 1.0], "the press itself picks"); + let hsva = srgb_hsva(picked); + assert!((hsva.s - 0.5).abs() < 0.03 && (hsva.v - 0.5).abs() < 0.03, "{hsva:?}"); + h.event(egui::Event::PointerMoved(at + vec2(3.0, 0.0))); + h.run_steps(1); + assert_ne!(h.state().session.tools.foreground, picked, "a small drag adjusts the pick"); + press(&mut h, at + vec2(3.0, 0.0), false); + h.run_steps(1); + // Hue strip: a press a third of the way down is a hue of about 240° at once. + let before = h.state().session.tools.foreground; + press(&mut h, o + vec2(38.0 + 8.0 + field_w + 8.0 + 7.0, 40.0), true); + let now = h.state().session.tools.foreground; + assert_ne!(now, before, "the strip picks on press"); + assert!((srgb_hsva(now).h * 360.0 - 240.0).abs() < 8.0, "{:?}", srgb_hsva(now)); + } + + /// Only the primary button picks: a right or middle press (and its release) on the field or + /// the hue strip leaves the foreground colour alone. + #[test] + fn color_panel_ignores_secondary_and_middle_presses() { + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default()); + app.session.tools.foreground = [0.2, 0.4, 0.6, 1.0]; + let mut h = egui_kittest::Harness::builder().with_size(vec2(300.0, 300.0)).build_ui_state( + |ui, app: &mut PhotocraftApp| { + let at = (ui.cursor().min, ui.available_width()); + ui.data_mut(|d| d.insert_temp(egui::Id::new("test-origin"), at)); + color_field(app, ui); + }, + app, + ); + PhotocraftApp::setup_context(&h.ctx, crate::theme::ThemeKind::Studio); + h.run_steps(2); + let (o, w) = h.ctx.data(|d| d.get_temp::<(Pos2, f32)>(egui::Id::new("test-origin"))).unwrap(); + let field_w = w - 38.0 - 14.0 - 16.0; + let field = o + vec2(38.0 + 8.0 + field_w * 0.5, 60.0); + let strip = o + vec2(38.0 + 8.0 + field_w + 8.0 + 7.0, 40.0); + for at in [field, strip] { + for button in [egui::PointerButton::Secondary, egui::PointerButton::Middle] { + for pressed in [true, false] { + h.event(egui::Event::PointerMoved(at)); + h.event(egui::Event::PointerButton { pos: at, button, pressed, modifiers: egui::Modifiers::NONE }); + h.run_steps(1); + assert_eq!(h.state().session.tools.foreground, [0.2, 0.4, 0.6, 1.0], "{button:?} pressed={pressed} at {at:?}"); + } + } + } + } + /// Click the hex field, select its text and type `text` one key per frame. fn type_hex(h: &mut egui_kittest::Harness<'static, PhotocraftApp>, text: &str) { use egui_kittest::kittest::Queryable; diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index ad5cba2105..1210c6c804 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -3001,6 +3001,16 @@ pub(crate) fn color_swatch(ui: &mut Ui, color: Color32) -> Response { ui.painter().rect_stroke(rect, t.radius_sm, Stroke::new(1.0, t.field_border), StrokeKind::Inside); response } +/// Where a press or drag on a picking surface (a colour field, a hue strip) points, from the +/// frame the button goes down and on every move after it. `dragged()` waits for egui's click +/// threshold (a few points of travel, or a long hold), so a press picked nothing until the +/// pointer had moved away and fine adjustments jumped (#7793). Only the primary button picks: +/// `is_pointer_button_down_on` is true for any button, so a right or middle press is ignored. +pub fn pick_pos(resp: &Response) -> Option { + let held = resp.is_pointer_button_down_on() + && resp.ctx.input(|i| i.pointer.primary_down() && !i.pointer.button_down(egui::PointerButton::Secondary) && !i.pointer.middle_down()); + if held || resp.clicked() { resp.interact_pointer_pos() } else { None } +} /// A colour swatch that only reports clicks (no popup): for colours edited in PhotoCraft's own /// Color Picker dialog. `label` names it for hover text and accessibility. pub fn color_swatch_button(ui: &mut Ui, color: Color32, label: &str) -> Response {