From 951e9624cb65c15b47288d75018cf7351383c684 Mon Sep 17 00:00:00 2001 From: Raghav Krishna Date: Mon, 9 Nov 2026 18:40:10 +0530 Subject: [PATCH 1/6] Pencil: pressure toggles on the options bar (#11962) --- crates/ui-egui/src/lib.rs | 2 + crates/ui-egui/src/panels/options_bar.rs | 21 +++ .../src/pencil_pressure_toggles_tests.rs | 134 ++++++++++++++++++ 3 files changed, 157 insertions(+) create mode 100644 crates/ui-egui/src/pencil_pressure_toggles_tests.rs diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 0b0bd48329..82bd8a08f6 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -795,6 +795,8 @@ mod cam_no_pixels_alert_tests; #[cfg(test)] mod patch_shift_constrain_tests; #[cfg(test)] +mod pencil_pressure_toggles_tests; +#[cfg(test)] mod shape_bounds_box_tests; #[cfg(test)] diff --git a/crates/ui-egui/src/panels/options_bar.rs b/crates/ui-egui/src/panels/options_bar.rs index 76431408c3..1a0fee434c 100644 --- a/crates/ui-egui/src/panels/options_bar.rs +++ b/crates/ui-egui/src/panels/options_bar.rs @@ -232,11 +232,32 @@ fn brush_specific_options(app: &mut PhotocraftApp, ui: &mut egui::Ui, t: Tokens, let held = app.held_erase; paint_mode_dropdown(ui, "pencil-mode", b, &mut app.ui.tool_options.pencil_clear, held); whole_percent_field(ui, tl!("Opacity"), &mut b.opacity, 1.0..=100.0, if t.pro { 62.0 } else { 66.0 }); + if t.pro { + // Between Opacity and Smoothing, as measured (#11962). + let pressure = pressure_button(ui, b.pressure_opacity, tl!("Always use pressure for opacity"), true); + if pressure.on_hover_text(tl!("Always use Pressure for Opacity. When off, Brush Preset controls pressure")).clicked() { + b.pressure_opacity = !b.pressure_opacity; + } + } opt_label(ui, tl!("Smoothing")); smoothing_field(ui, b, if t.pro { 58.0 } else { 66.0 }); widgets::vline(ui, 22.0); widgets::checkbox(ui, &mut app.ui.tool_options.pencil_auto_erase, tl!("Auto Erase")) .on_hover_text(tl!("Draws background color over foreground colors")); + widgets::vline(ui, 22.0); + if t.pro { + // Between Auto Erase and the Symmetry button the bar adds after this arm (#11962). + let pressure = pressure_button(ui, b.pressure_size, tl!("Always use pressure for size"), true); + if pressure.on_hover_text(tl!("Always use Pressure for Size. When off, Brush Preset controls pressure")).clicked() { + b.pressure_size = !b.pressure_size; + } + } else { + // The other themes show the Brush's labelled switches; the Pencil gets them too. + widgets::toggle(ui, &mut b.pressure_size, tl!("Pressure for Size")) + .on_hover_text(tl!("Always use Pressure for Size. When off, Brush Preset controls pressure")); + widgets::toggle(ui, &mut b.pressure_opacity, tl!("Pressure for Opacity")) + .on_hover_text(tl!("Always use Pressure for Opacity. When off, Brush Preset controls pressure")); + } } Tool::Brush | Tool::Eraser => { brush_preset_chip(ui, b, &mut app.ui); diff --git a/crates/ui-egui/src/pencil_pressure_toggles_tests.rs b/crates/ui-egui/src/pencil_pressure_toggles_tests.rs new file mode 100644 index 0000000000..f357053203 --- /dev/null +++ b/crates/ui-egui/src/pencil_pressure_toggles_tests.rs @@ -0,0 +1,134 @@ +//! #11962: the Pencil's options bar gets the two pressure toggles, editing the Pencil's own +//! brush; the Brush keeps its own. Pro themes show the icon toggles, the others the labelled +//! switches, both with the reference tooltips. + +use egui::vec2; +use egui_kittest::Harness; +use egui_kittest::kittest::{NodeT, Queryable, by}; +use serde_json::json; + +use crate::PhotocraftApp; +use crate::state::Tool; +use crate::theme::ThemeKind; + +fn bar(theme: ThemeKind) -> Harness<'static, PhotocraftApp> { + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default()); + app.run("file.new", json!({"width": 200, "height": 100, "background": "white"})).unwrap(); + app.ui.tool = Tool::Pencil; + let mut h = Harness::builder().with_size(vec2(1600.0, 300.0)).build_ui_state(|ui, app| crate::panels::options_bar(app, ui), app); + PhotocraftApp::setup_context(&h.ctx, theme); + h.run_steps(4); + h +} + +fn description(h: &Harness<'static, PhotocraftApp>, label: &str) -> Option { + h.query_all_by_label(label).next().and_then(|n| n.accesskit_node().description().map(|d| d.to_string())) +} + +const PRO: [(&str, &str); 2] = [ + ("Always use pressure for opacity", "Always use Pressure for Opacity. When off, Brush Preset controls pressure"), + ("Always use pressure for size", "Always use Pressure for Size. When off, Brush Preset controls pressure"), +]; + +const CLASSIC: [(&str, &str); 2] = [ + ("Pressure for Size", "Always use Pressure for Size. When off, Brush Preset controls pressure"), + ("Pressure for Opacity", "Always use Pressure for Opacity. When off, Brush Preset controls pressure"), +]; + +#[test] +fn the_pro_bar_shows_both_toggles_with_the_reference_tooltips() { + let h = bar(ThemeKind::ProMedium); + for (label, tip) in PRO { + assert_eq!(h.query_all_by_label(label).count(), 1, "{label} exists once"); + assert_eq!(description(&h, label).as_deref(), Some(tip), "{label} tooltip"); + } +} + +#[test] +fn the_pro_toggles_edit_the_pencils_own_flags_and_brush_get_sees_them() { + let mut h = bar(ThemeKind::ProMedium); + let flags = |h: &Harness<'static, PhotocraftApp>| (h.state().session.tools.brush.pressure_opacity, h.state().session.tools.brush.pressure_size); + assert_eq!(flags(&h), (false, false), "both off on a fresh Pencil"); + h.get_by_label("Always use pressure for opacity").click(); + h.run_steps(2); + h.get_by_label("Always use pressure for size").click(); + h.run_steps(2); + assert_eq!(flags(&h), (true, true)); + let got = h.state_mut().run("brush.get", json!({})).unwrap(); + assert_eq!(got["pressureOpacity"], json!(true), "brush.get reads the Pencil's flag"); + assert_eq!(got["pressureSize"], json!(true)); +} + +#[test] +fn the_brush_keeps_its_own_pressure_flags() { + let mut h = bar(ThemeKind::ProMedium); + h.get_by_label("Always use pressure for opacity").click(); + h.run_steps(2); + assert!(h.state().session.tools.brush.pressure_opacity, "the Pencil's flag is on"); + // Switch to the Brush and give it the opposite value. + h.state_mut().ui.tool = Tool::Brush; + h.run_steps(4); + h.state_mut().session.tools.brush.pressure_opacity = false; + h.run_steps(2); + // Back to the Pencil: its own value is still on. + h.state_mut().ui.tool = Tool::Pencil; + h.run_steps(4); + assert!(h.state().session.tools.brush.pressure_opacity, "the Pencil's flag came back"); + // And the Brush still has the opposite one. + h.state_mut().ui.tool = Tool::Brush; + h.run_steps(4); + assert!(!h.state().session.tools.brush.pressure_opacity, "the Brush kept its own flag"); +} + +#[test] +fn the_other_themes_show_the_labelled_toggles() { + let mut h = bar(ThemeKind::Classic); + // The switches are unnamed nodes: their tooltips are the descriptions in the bar. + let tips: Vec = + h.query_all(by().predicate(|n| n.description().is_some())).filter_map(|n| n.accesskit_node().description().map(|d| d.to_string())).collect(); + for (label, tip) in CLASSIC { + assert_eq!(h.query_all_by_label(label).count(), 1, "{label} exists once"); + assert!(tips.iter().any(|d| d == tip), "{label} tooltip: {tips:?}"); + } + for (label, tip, expect) in [("Pressure for Size", CLASSIC[0].1, true), ("Pressure for Opacity", CLASSIC[1].1, true)] { + let at = h + .query_all(by().predicate(move |n| n.description().map(|d| d == tip).unwrap_or(false))) + .next() + .unwrap_or_else(|| panic!("no switch for {label}")) + .rect() + .center(); + h.hover_at(at); + h.run(); + h.drag_at(at); + h.run(); + h.drop_at(at); + h.run_steps(2); + let flag = if label.ends_with("Size") { h.state().session.tools.brush.pressure_size } else { h.state().session.tools.brush.pressure_opacity }; + assert_eq!(flag, expect, "{label} toggles the Pencil's flag"); + } +} + +/// Painted width at column `x` on the active layer, through the stroke's row band. +fn width(app: &PhotocraftApp, x: i32, y: i32) -> usize { + let st = app.session.active().expect("a document"); + let layer = st.active_layer.expect("an active layer"); + let s = st.doc.layer(layer).and_then(|l| l.surface()).expect("a raster layer"); + (y - 20..=y + 20).filter(|yy| s.rgba(x, *yy)[3] > 0.5).count() +} + +#[test] +fn pressure_for_size_narrows_a_low_pressure_pencil_stroke() { + let mut h = bar(ThemeKind::ProMedium); + let app = h.state_mut(); + app.run("layer.new.layer", json!({})).unwrap(); + app.run("tools.setBrush", json!({"brush": {"size": 13.0, "pressureSize": true, "smoothing": {"amount": 0.0}}})).unwrap(); + app.run("paint.pencil", json!({"points": [[20.0, 50.0, 0.2], [30.0, 50.0, 0.2], [60.0, 50.0, 1.0]]})).unwrap(); + let (low, high) = (width(app, 22, 50), width(app, 58, 50)); + assert!(low < high, "pressure for size on: low-pressure {low} px, high-pressure {high} px"); + + app.run("layer.new.layer", json!({})).unwrap(); + app.run("tools.setBrush", json!({"brush": {"pressureSize": false}})).unwrap(); + app.run("paint.pencil", json!({"points": [[20.0, 50.0, 0.2], [30.0, 50.0, 0.2], [60.0, 50.0, 1.0]]})).unwrap(); + let (low, high) = (width(app, 22, 50), width(app, 58, 50)); + assert_eq!((low, high), (13, 13), "pressure for size off: every dab is 13 px"); +} From 390a855c1a8bbe407cc9fc2172a0930bf76fcf65 Mon Sep 17 00:00:00 2001 From: Raghav Krishna Date: Mon, 9 Nov 2026 19:01:19 +0530 Subject: [PATCH 2/6] Pencil: tighten the pressure-toggle tests (#11962) --- .../src/pencil_pressure_toggles_tests.rs | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/crates/ui-egui/src/pencil_pressure_toggles_tests.rs b/crates/ui-egui/src/pencil_pressure_toggles_tests.rs index f357053203..b12376d0bc 100644 --- a/crates/ui-egui/src/pencil_pressure_toggles_tests.rs +++ b/crates/ui-egui/src/pencil_pressure_toggles_tests.rs @@ -62,22 +62,26 @@ fn the_pro_toggles_edit_the_pencils_own_flags_and_brush_get_sees_them() { #[test] fn the_brush_keeps_its_own_pressure_flags() { let mut h = bar(ThemeKind::ProMedium); - h.get_by_label("Always use pressure for opacity").click(); - h.run_steps(2); - assert!(h.state().session.tools.brush.pressure_opacity, "the Pencil's flag is on"); - // Switch to the Brush and give it the opposite value. + // Give the Brush its own brush first, so the Pencil's later toggle cannot be "inherited" on + // the first switch and hide a leak. h.state_mut().ui.tool = Tool::Brush; h.run_steps(4); h.state_mut().session.tools.brush.pressure_opacity = false; h.run_steps(2); - // Back to the Pencil: its own value is still on. + + h.state_mut().ui.tool = Tool::Pencil; + h.run_steps(4); + h.get_by_label("Always use pressure for opacity").click(); + h.run_steps(2); + assert!(h.state().session.tools.brush.pressure_opacity, "the Pencil's flag is on"); + + h.state_mut().ui.tool = Tool::Brush; + h.run_steps(4); + assert!(!h.state().session.tools.brush.pressure_opacity, "the Pencil's toggle did not change the Brush"); + h.state_mut().ui.tool = Tool::Pencil; h.run_steps(4); assert!(h.state().session.tools.brush.pressure_opacity, "the Pencil's flag came back"); - // And the Brush still has the opposite one. - h.state_mut().ui.tool = Tool::Brush; - h.run_steps(4); - assert!(!h.state().session.tools.brush.pressure_opacity, "the Brush kept its own flag"); } #[test] @@ -125,6 +129,7 @@ fn pressure_for_size_narrows_a_low_pressure_pencil_stroke() { app.run("paint.pencil", json!({"points": [[20.0, 50.0, 0.2], [30.0, 50.0, 0.2], [60.0, 50.0, 1.0]]})).unwrap(); let (low, high) = (width(app, 22, 50), width(app, 58, 50)); assert!(low < high, "pressure for size on: low-pressure {low} px, high-pressure {high} px"); + assert!((2..=4).contains(&low), "pressure for size on: the 0.2 stroke is about 3 px, got {low}"); app.run("layer.new.layer", json!({})).unwrap(); app.run("tools.setBrush", json!({"brush": {"pressureSize": false}})).unwrap(); From cf93aeecd4728cb13266173fb32e88ec8c837949 Mon Sep 17 00:00:00 2001 From: Raghav Krishna Date: Mon, 9 Nov 2026 19:10:57 +0530 Subject: [PATCH 3/6] Pencil: check both directions of the pressure-flag isolation (#11962) --- .../src/pencil_pressure_toggles_tests.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/crates/ui-egui/src/pencil_pressure_toggles_tests.rs b/crates/ui-egui/src/pencil_pressure_toggles_tests.rs index b12376d0bc..c044398f33 100644 --- a/crates/ui-egui/src/pencil_pressure_toggles_tests.rs +++ b/crates/ui-egui/src/pencil_pressure_toggles_tests.rs @@ -82,6 +82,23 @@ fn the_brush_keeps_its_own_pressure_flags() { h.state_mut().ui.tool = Tool::Pencil; h.run_steps(4); assert!(h.state().session.tools.brush.pressure_opacity, "the Pencil's flag came back"); + + // The mirror: the Brush on, the Pencil off, so a reset to one value cannot pass. + h.state_mut().ui.tool = Tool::Brush; + h.run_steps(4); + h.state_mut().session.tools.brush.pressure_opacity = true; + h.run_steps(2); + h.state_mut().ui.tool = Tool::Pencil; + h.run_steps(4); + h.get_by_label("Always use pressure for opacity").click(); + h.run_steps(2); + assert!(!h.state().session.tools.brush.pressure_opacity, "the Pencil is off"); + h.state_mut().ui.tool = Tool::Brush; + h.run_steps(4); + assert!(h.state().session.tools.brush.pressure_opacity, "the Brush stayed on"); + h.state_mut().ui.tool = Tool::Pencil; + h.run_steps(4); + assert!(!h.state().session.tools.brush.pressure_opacity, "the Pencil stayed off"); } #[test] From 7b8fb4fedc7facba0dd26e76a8232a33573b36cb Mon Sep 17 00:00:00 2001 From: Raghav Krishna Date: Mon, 9 Nov 2026 19:22:50 +0530 Subject: [PATCH 4/6] Pencil: cover pressure size in the isolation test (#11962) --- .../src/pencil_pressure_toggles_tests.rs | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/crates/ui-egui/src/pencil_pressure_toggles_tests.rs b/crates/ui-egui/src/pencil_pressure_toggles_tests.rs index c044398f33..36a03e9090 100644 --- a/crates/ui-egui/src/pencil_pressure_toggles_tests.rs +++ b/crates/ui-egui/src/pencil_pressure_toggles_tests.rs @@ -62,43 +62,50 @@ fn the_pro_toggles_edit_the_pencils_own_flags_and_brush_get_sees_them() { #[test] fn the_brush_keeps_its_own_pressure_flags() { let mut h = bar(ThemeKind::ProMedium); - // Give the Brush its own brush first, so the Pencil's later toggle cannot be "inherited" on + let flags = |h: &Harness<'static, PhotocraftApp>| (h.state().session.tools.brush.pressure_opacity, h.state().session.tools.brush.pressure_size); + let toggle = |h: &mut Harness<'static, PhotocraftApp>, label: &str| { + h.get_by_label(label).click(); + h.run_steps(2); + }; + + // Give the Brush its own brush first, so the Pencil's later toggles cannot be "inherited" on // the first switch and hide a leak. h.state_mut().ui.tool = Tool::Brush; h.run_steps(4); h.state_mut().session.tools.brush.pressure_opacity = false; + h.state_mut().session.tools.brush.pressure_size = false; h.run_steps(2); h.state_mut().ui.tool = Tool::Pencil; h.run_steps(4); - h.get_by_label("Always use pressure for opacity").click(); - h.run_steps(2); - assert!(h.state().session.tools.brush.pressure_opacity, "the Pencil's flag is on"); + toggle(&mut h, "Always use pressure for opacity"); + toggle(&mut h, "Always use pressure for size"); + assert_eq!(flags(&h), (true, true), "the Pencil's flags are on"); h.state_mut().ui.tool = Tool::Brush; h.run_steps(4); - assert!(!h.state().session.tools.brush.pressure_opacity, "the Pencil's toggle did not change the Brush"); - + assert_eq!(flags(&h), (false, false), "the Pencil's toggles did not change the Brush"); h.state_mut().ui.tool = Tool::Pencil; h.run_steps(4); - assert!(h.state().session.tools.brush.pressure_opacity, "the Pencil's flag came back"); + assert_eq!(flags(&h), (true, true), "the Pencil's flags came back"); // The mirror: the Brush on, the Pencil off, so a reset to one value cannot pass. h.state_mut().ui.tool = Tool::Brush; h.run_steps(4); h.state_mut().session.tools.brush.pressure_opacity = true; + h.state_mut().session.tools.brush.pressure_size = true; h.run_steps(2); h.state_mut().ui.tool = Tool::Pencil; h.run_steps(4); - h.get_by_label("Always use pressure for opacity").click(); - h.run_steps(2); - assert!(!h.state().session.tools.brush.pressure_opacity, "the Pencil is off"); + toggle(&mut h, "Always use pressure for opacity"); + toggle(&mut h, "Always use pressure for size"); + assert_eq!(flags(&h), (false, false), "the Pencil is off"); h.state_mut().ui.tool = Tool::Brush; h.run_steps(4); - assert!(h.state().session.tools.brush.pressure_opacity, "the Brush stayed on"); + assert_eq!(flags(&h), (true, true), "the Brush stayed on"); h.state_mut().ui.tool = Tool::Pencil; h.run_steps(4); - assert!(!h.state().session.tools.brush.pressure_opacity, "the Pencil stayed off"); + assert_eq!(flags(&h), (false, false), "the Pencil stayed off"); } #[test] From f41a39276a3f62571d786cb488feaf944141b7c2 Mon Sep 17 00:00:00 2001 From: Raghav Krishna Date: Mon, 9 Nov 2026 19:32:12 +0530 Subject: [PATCH 5/6] Pencil: cover a mixed pressure setting across tool switches (#11962) --- crates/ui-egui/src/pencil_pressure_toggles_tests.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/crates/ui-egui/src/pencil_pressure_toggles_tests.rs b/crates/ui-egui/src/pencil_pressure_toggles_tests.rs index 36a03e9090..edd1a835da 100644 --- a/crates/ui-egui/src/pencil_pressure_toggles_tests.rs +++ b/crates/ui-egui/src/pencil_pressure_toggles_tests.rs @@ -106,6 +106,16 @@ fn the_brush_keeps_its_own_pressure_flags() { h.state_mut().ui.tool = Tool::Pencil; h.run_steps(4); assert_eq!(flags(&h), (false, false), "the Pencil stayed off"); + + // A mixed setting: opacity on, size off, across a switch and back. + toggle(&mut h, "Always use pressure for opacity"); + assert_eq!(flags(&h), (true, false), "mixed: opacity on, size off"); + h.state_mut().ui.tool = Tool::Brush; + h.run_steps(4); + assert_eq!(flags(&h), (true, true), "the Brush still has its own pair"); + h.state_mut().ui.tool = Tool::Pencil; + h.run_steps(4); + assert_eq!(flags(&h), (true, false), "the mixed pair came back unchanged"); } #[test] From 3576f83b5002f7f7ff37471a4e2e617b11dfb2a9 Mon Sep 17 00:00:00 2001 From: Raghav Krishna Date: Mon, 9 Nov 2026 19:44:12 +0530 Subject: [PATCH 6/6] Pencil: check the drawn toggle states after a tool switch (#11962) --- crates/ui-egui/src/pencil_pressure_toggles_tests.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/ui-egui/src/pencil_pressure_toggles_tests.rs b/crates/ui-egui/src/pencil_pressure_toggles_tests.rs index edd1a835da..4a6a2031e4 100644 --- a/crates/ui-egui/src/pencil_pressure_toggles_tests.rs +++ b/crates/ui-egui/src/pencil_pressure_toggles_tests.rs @@ -116,6 +116,10 @@ fn the_brush_keeps_its_own_pressure_flags() { h.state_mut().ui.tool = Tool::Pencil; h.run_steps(4); assert_eq!(flags(&h), (true, false), "the mixed pair came back unchanged"); + // And the bar draws that pair, not just stores it. + let on = |h: &Harness<'static, PhotocraftApp>, label: &str| h.get_by_label(label).accesskit_node().toggled() == Some(egui::accesskit::Toggled::True); + assert!(on(&h, "Always use pressure for opacity"), "the opacity toggle reads as on"); + assert!(!on(&h, "Always use pressure for size"), "the size toggle reads as off"); } #[test]