diff --git a/crates/ui-egui/src/dock.rs b/crates/ui-egui/src/dock.rs index 0c25ed910b..820433c4c0 100644 --- a/crates/ui-egui/src/dock.rs +++ b/crates/ui-egui/src/dock.rs @@ -120,9 +120,12 @@ impl Group { } /// Tabs that lay out their own scrolling list and footer, or size themselves to the group - /// (the Navigator's preview grows with it, as in Photoshop: #2726). + /// (the Navigator's preview grows with it, as in Photoshop: #2726). Channels and Paths are + /// Layers' siblings and do the same: wrapped in the dock's scroll area as well, the two nested + /// areas shared the wheel, and where the outer content overflowed by one physical pixel (at + /// 125 % scaling for some window heights) the footer and list moved a pixel as you scrolled (#6238). pub fn scrolls_itself(self, tab: usize) -> bool { - matches!((self, tab), (Group::Layers, 0) | (Group::History, 0) | (Group::History, 1) | (Group::Navigator, 0)) + matches!((self, tab), (Group::Layers, 0..=2) | (Group::History, 0) | (Group::History, 1) | (Group::Navigator, 0)) } pub(crate) fn tab_mut(self, tabs: &mut DockTabs) -> &mut usize { diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 6f54ed1616..35e0e37417 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -3073,6 +3073,8 @@ mod warp_text_dialog_tests; #[cfg(test)] mod blend_dropdown_wheel_tests; +#[cfg(test)] +mod paths_scroll_tests; #[cfg(test)] mod options_bar_mode_wheel_tests; diff --git a/crates/ui-egui/src/paths_scroll_tests.rs b/crates/ui-egui/src/paths_scroll_tests.rs new file mode 100644 index 0000000000..924e6e6290 --- /dev/null +++ b/crates/ui-egui/src/paths_scroll_tests.rs @@ -0,0 +1,69 @@ +//! Scrolling the Paths panel moves only its list (#6238). + +use egui::vec2; +use egui_kittest::{Harness, kittest::Queryable}; +use serde_json::json; + +use crate::PhotocraftApp; + +fn harness(paths: usize, size: egui::Vec2, ppp: f32) -> Harness<'static, PhotocraftApp> { + let mut h = Harness::builder().with_size(size).with_pixels_per_point(ppp).with_max_steps(64).build_eframe(move |cc| { + PhotocraftApp::setup_context(&cc.egui_ctx, Default::default()); + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default()); + app.run("file.new", json!({"width": 400, "height": 300})).unwrap(); + for i in 0..paths { + let x = (i % 8) as i32 * 40; + app.run("select.rect", json!({"x": x, "y": 20, "width": 30, "height": 30})).unwrap(); + app.run("select.toWorkPath", json!({"tolerance": 2})).unwrap(); + app.run("path.rename", json!({"name": "work", "to": format!("Path {}", i + 1)})).unwrap(); + } + // Layers | Channels | Paths: the Paths tab. + app.ui.dock_tabs.layers = 2; + app + }); + h.run_steps(8); + h +} + +/// Scrolling the path list moves the rows and nothing else: the footer's buttons stay where they +/// are. The Paths panel lays out its own list and footer, but the dock also wrapped it in a +/// scroll area; at 125 % scaling and some window heights that outer area overflowed by one +/// physical pixel, took part of the wheel and moved the footer about a pixel down and back up. +#[test] +fn scrolling_the_paths_list_does_not_move_the_footer() { + // Heights that moved it at 125 % (two bands of window heights), and ones that never did. + for (ppp, height) in [(1.25, 520.0), (1.25, 900.0), (1.25, 960.0), (1.0, 900.0), (1.5, 900.0)] { + let mut h = harness(30, vec2(1440.0, height), ppp); + let footer = |h: &Harness<'_, PhotocraftApp>| h.query_by_label("Delete path").map(|r| r.rect().min.y); + let rest = footer(&h).unwrap_or_else(|| panic!("the footer shows at {ppp}x, {height} high")); + let first_row = h.query_by_label("Path 1").unwrap().rect(); + h.hover_at(first_row.center()); + h.run_steps(2); + for i in 0..40 { + // Down to the end of the list and past it, then back up. + let dir = if i < 30 { -1.0 } else { 1.0 }; + h.event(egui::Event::MouseWheel { + unit: egui::MouseWheelUnit::Line, + delta: vec2(0.0, dir), + phase: egui::TouchPhase::Move, + modifiers: Default::default(), + }); + h.run_steps(1); + assert_eq!(footer(&h), Some(rest), "{ppp}x, {height} high, notch {i}: the footer stayed put"); + } + // The list itself did scroll (the first row is out of view or has moved). + let moved = h.query_by_label("Path 1").map(|r| r.rect().min.y) != Some(first_row.min.y); + assert!(moved || h.query_by_label("Path 1").is_none(), "{ppp}x, {height} high: the list scrolled"); + } +} + +/// Channels, Paths and Layers are siblings with the same own-scroll layout, so none is wrapped in +/// the dock's scroll area. +#[test] +fn the_layers_group_tabs_all_scroll_themselves() { + use crate::dock::Group; + for tab in 0..=2 { + assert!(Group::Layers.scrolls_itself(tab), "Layers group tab {tab}"); + } + assert!(!Group::Layers.scrolls_itself(3), "a tab that does not exist is not special"); +} diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index c973d710db..968b8d39e1 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -183,6 +183,9 @@ pub fn panel_footer(ui: &mut Ui, add: impl FnOnce(&mut Ui) -> R) -> R { ui.painter().line_segment([pos2(r.left() - m.left as f32, y), pos2(r.right() + m.right as f32, y)], Stroke::new(1.0, t.separator)); let bar = Rect::from_min_size(pos2(r.left(), r.top() + 1.0), vec2(r.width(), FOOTER_BAR)); let mut bar_ui = ui.new_child(egui::UiBuilder::new().max_rect(bar).layout(egui::Layout::right_to_left(egui::Align::Center))); + // The bar runs on over the body's bottom margin, past where the body's own clip ends: without + // this its buttons were cut off there, a 26 px hover highlight drawn about 21 px tall. + bar_ui.set_clip_rect(bar_ui.clip_rect().union(bar)); bar_ui.spacing_mut().item_spacing.x = 2.0; add(&mut bar_ui) } @@ -1980,6 +1983,32 @@ mod tests { } use egui_kittest::{Harness, kittest::Queryable}; + /// A panel footer's buttons are drawn whole. The bar runs on over the body's bottom margin, so a + /// body that clips where its content ends (as the panels do) cut the bottom off every button: + /// the hover highlight of a 26 px button was about 21 px tall in the Paths, Layers and Channels + /// panels (#6238). + #[test] + fn panel_footer_buttons_are_not_clipped_by_the_body() { + let mut h = Harness::new_ui_state( + |ui, s: &mut Option<(egui::Rect, egui::Rect, egui::Rect)>| { + // A body exactly as tall as the footer takes, clipped there. + let body = egui::Rect::from_min_size(ui.max_rect().min, egui::vec2(220.0, super::footer_height(ui))); + let mut body_ui = ui.new_child(egui::UiBuilder::new().max_rect(body)); + body_ui.set_clip_rect(body); + super::panel_footer(&mut body_ui, |bar| { + let b = crate::icons::button(bar, "square", 26.0, false, "Button"); + *s = Some((b.rect, bar.clip_rect(), body)); + }); + }, + None, + ); + crate::PhotocraftApp::setup_context(&h.ctx, crate::theme::ThemeKind::Studio); + h.run(); + let (button, clip, body) = h.state().expect("the footer drew its button"); + assert!(button.bottom() > body.bottom(), "the bar really overruns the body, or this test proves nothing: {button:?} vs {body:?}"); + assert!(clip.contains_rect(button), "the whole button is inside the clip: {button:?} not in {clip:?}"); + } + /// #3789: a plain text field is one boxed, outline-free input whose text-selection colour stays /// local, so it isn't recoloured for the rest of the frame (selected rows, slider trails). #[test]