aleleba/photocraft
1
0
Fork 0
mirror of https://github.com/storytold/photocraft.git synced 2026-10-24 09:57:08 -06:00
Code Issues Packages Projects Releases Wiki Activity

Color pickers: pick on press and follow small drags (#7793) (#7822)

Browse Source 
* Color pickers: pick on press and follow small drags (#7793)

The Color Picker dialog's field and slider, and the Color panel's
field and hue strip, only applied the colour on dragged()/clicked().
egui holds dragged() back until the pointer moves past its click
threshold (~6 pt) or is held a while, so a press picked nothing at
first and small adjustments jumped.

Add widgets::pick_pos, which reads the pointer from the frame the
button goes down (is_pointer_button_down_on), and use it on all four
surfaces. The Color panel now restyles selected type only when the
picked colour changes.

Tests: color_picker_ui::field_and_slider_pick_on_press_and_follow_small_moves,
panels::color_tests::color_panel_field_and_hue_strip_pick_on_press.

* #1 Reverted accidental scoreboard change when regenerating it
#2 Fixed color picker not checking for pointer press, but any mouse click

* Update crates/ui-egui/src/widgets.rs

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Color pickers: rustfmt pick_pos

* test(ui): use main's rgb_to_hsb in the colour picker tests

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Brandon Thomas <echelon@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Hodd
2026-10-23 18:34:41 -04:00
committed by GitHub
co-authored by coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Claude Opus 5.5 Brandon Thomas
parent 74ada29c12
commit d878e38333
3 changed files with 182 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/ui-egui/src/color_picker_ui.rs
+91 -2
View File
@@ -484,7 +484,7 @@ fn color_field(ui: &mut egui::Ui, f: &mut Map<String, Value>) {
// 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<String, Value>) {
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<S>(h: &mut Harness<'static, S>, at: egui::Pos2) {
press_button(h, at, egui::PointerButton::Primary, true);
}
fn press_button<S>(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<String, Value>| {
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<String, Value>| {
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");
crates/ui-egui/src/panels.rs
+81 -8
View File
@@ -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;
crates/ui-egui/src/widgets.rs
+10
View File
@@ -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<Pos2> {
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 {