Merge PR #6263: fix(ui): Paths panel footer: cut-off hover highlight and scroll jiggle (part of #6238)

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 {

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;

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");
+}

crates/ui-egui/src/widgets.rs

@@ -183,6 +183,9 @@ pub fn panel_footer<R>(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]