Menus: no menu or submenu covers the menu bar (#319) (#337)

crates/ui-egui/src/menu_nav.rs

@@ -51,6 +51,9 @@ pub struct Nav {
     pub(crate) views: Vec<Rect>,
     /// The full height of each level's rows (taller than its view when it scrolls).
     pub(crate) contents: Vec<f32>,
+    /// Per level: the row its open submenu hangs from (level 0 has none). From the frame
+    /// before, since a submenu is drawn while its row is.
+    anchors: Vec<Option<Rect>>,
     /// Scroll the highlighted row of `level` into view when it is next drawn.
     reveal: bool,
     /// Highlight the first enabled row of this level once it has been drawn (a keyboard-opened
@@ -183,6 +186,16 @@ impl Nav {
         self.first_at = Some(level + 1);
     }
 
+    /// Remember the row the submenu of `level` (1 = a top-level menu's submenu) hangs from.
+    pub fn set_anchor(&mut self, level: usize, row: Rect) {
+        if self.anchors.len() <= level {
+            self.anchors.resize(level + 1, None);
+        }
+        if let Some(a) = self.anchors.get_mut(level) {
+            *a = Some(row);
+        }
+    }
+
     /// Handle this frame's navigation keys (after the menus were drawn). `tops` are the menu
     /// bar's popup ids; an activated command lands in `clicked`.
     pub fn keys(&mut self, ctx: &egui::Context, tops: &[Id], clicked: &mut Option<String>) {
@@ -252,6 +265,23 @@ fn pointer_on(ui: &Ui, rect: Rect) -> bool {
     ui.ctx().pointer_hover_pos().is_some_and(|p| rect.contains(p) && ui.ctx().layer_id_at(p) == Some(ui.layer_id()))
 }
 
+/// The height a menu level's rows may take. Every level stays below the menu bar (#319): a
+/// top-level menu hangs from it, and a submenu that is too tall for the window between its row
+/// and the bottom, or (opening upward, as egui does when that fits) between the bar and its row,
+/// scrolls instead of egui sliding it up over the menu bar, where it hid the menu titles.
+/// `anchor` is the submenu's row (unknown on its first frame); `frame` the popup's margins.
+pub fn level_room(screen: Rect, bar_bottom: Option<f32>, depth: usize, anchor: Option<Rect>, frame: f32) -> f32 {
+    let top = bar_bottom.unwrap_or(screen.top()).max(screen.top()) + EDGE;
+    let bottom = screen.bottom() - EDGE;
+    let below_bar = bottom - top;
+    let room = match anchor.filter(|_| depth > 1) {
+        // Downward from the row, or upward from it, whichever leaves more room.
+        Some(row) => (bottom - row.top()).max(row.bottom() - top).min(below_bar),
+        None => below_bar,
+    } - frame;
+    if room.is_finite() { room.max(4.0 * ARROW) } else { 4.0 * ARROW }
+}
+
 /// Draw one level of a menu, bounded by the window: when its rows don't fit they scroll, with
 /// scroll arrows at the top and bottom. `depth` is 1 for a top-level menu.
 pub fn level(ui: &mut Ui, depth: usize, nav: &mut Nav, rows: impl FnOnce(&mut Ui, &mut Nav)) {
@@ -261,11 +291,8 @@ pub fn level(ui: &mut Ui, depth: usize, nav: &mut Nav, rows: impl FnOnce(&mut Ui
     let screen = ctx.content_rect();
     // The menu frame's margin and stroke around the rows.
     let frame = ui.spacing().menu_margin.sum().y + 2.0;
-    // A top-level menu hangs below the menu bar; a submenu may slide up to fit the window.
-    // (From the menu bar, not from where the popup is now: egui slides a popup that doesn't fit.)
-    let top = nav.bar_bottom.unwrap_or(screen.top()) + EDGE;
-    let room = if depth <= 1 { screen.bottom() - top - EDGE } else { screen.height() - 2.0 * EDGE } - frame;
-    let room = room.max(4.0 * ARROW);
+    let anchor = nav.anchors.get(level).copied().flatten();
+    let room = level_room(screen, nav.bar_bottom, depth, anchor, frame);
     let key = ui.id().with(("pc-menu-level", depth));
     // Rows' height from the last frame: does this level overflow?
     let content: Option<f32> = ctx.data(|d| d.get_temp(key));

crates/ui-egui/src/menu_nav_tests.rs

@@ -336,3 +336,77 @@ fn a_short_window_still_scrolls_the_file_menu_to_its_last_row() {
     }
     assert!(visible(&h, 0, last), "the wheel reaches File's last row");
 }
+
+/// Every popup on screen (menus and submenus).
+fn popups(h: &Harness<'static, PhotocraftApp>) -> Vec<egui::Rect> {
+    h.ctx
+        .memory(|m| m.areas().visible_layer_ids())
+        .into_iter()
+        .filter(|l| l.order == egui::Order::Foreground)
+        .filter_map(|layer| h.ctx.memory(|m| m.area_rect(layer.id)))
+        .collect()
+}
+
+/// No popup covers the menu bar or runs off the window, and every menu title stays visible.
+fn check_bar_clear(h: &Harness<'static, PhotocraftApp>, what: &str) {
+    let bar = Nav::current(&h.ctx).bar_bottom.expect("the menu bar's bottom");
+    let screen = h.ctx.content_rect();
+    for r in popups(h) {
+        assert!(r.top() >= bar - 0.5, "{what}: popup {r:?} covers the menu bar (bottom {bar})");
+        assert!(r.bottom() <= screen.bottom() + 0.5, "{what}: popup {r:?} runs off the window {screen:?}");
+    }
+    for title in crate::menus::TOP_MENUS {
+        // The title is the topmost node of that name (a submenu row can share it).
+        let t = h.query_all_by_label(title).map(|n| n.rect()).min_by(|a, b| a.top().total_cmp(&b.top())).expect("menu title");
+        assert!(popups(h).iter().all(|r| !r.intersects(t.shrink(1.0))), "{what}: {title} is covered");
+    }
+}
+
+#[test]
+fn no_menu_or_submenu_covers_the_menu_bar() {
+    // #319: on a 1280 × 720 display (the reporter's, at 1× and 2×, less the title bar) tall menus,
+    // then tall submenus (Image › Adjustments), slid up over the menu bar and hid its titles.
+    let displays = [(1280.0, 703.0, 1.0), (2560.0, 1406.0, 2.0), (1366.0, 768.0, 1.0)];
+    let mut checked = 0;
+    for display in displays {
+        for top in LONGEST {
+            let mut h = harness(display);
+            open(&mut h, top);
+            check_bar_clear(&h, &format!("{display:?} {top}"));
+            // Hover each submenu row in view, top to bottom: rows low in the menu open their
+            // submenus upward.
+            let view = Nav::current(&h.ctx).views.first().copied().unwrap_or(egui::Rect::NOTHING);
+            let subs: Vec<egui::Rect> = rows(&h, 0).iter().filter(|r| r.enabled && r.command.is_none() && view.contains_rect(r.rect)).map(|r| r.rect).collect();
+            for (i, row) in subs.iter().enumerate() {
+                h.hover_at(row.center() + egui::vec2(-20.0, 0.0));
+                h.run_steps(6);
+                if Nav::current(&h.ctx).rows.get(1).is_none_or(Vec::is_empty) {
+                    continue;
+                }
+                checked += 1;
+                check_bar_clear(&h, &format!("{display:?} {top} submenu {i}"));
+            }
+        }
+    }
+    assert!(checked > 30, "checked {checked} submenus");
+}
+
+#[test]
+fn level_room_keeps_every_level_below_the_bar() {
+    let screen = egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(1280.0, 703.0));
+    let (bar, frame) = (Some(30.0), 14.0);
+    let below = 703.0 - EDGE - (30.0 + EDGE) - frame;
+    assert_eq!(level_room(screen, bar, 1, None, frame), below);
+    // A submenu from a row near the top opens downward with the room under it…
+    let row = egui::Rect::from_min_size(egui::pos2(100.0, 80.0), egui::vec2(200.0, 30.0));
+    assert_eq!(level_room(screen, bar, 2, Some(row), frame), 703.0 - EDGE - 80.0 - frame);
+    // …and from a row near the bottom, upward to the bar.
+    let low = egui::Rect::from_min_size(egui::pos2(100.0, 600.0), egui::vec2(200.0, 30.0));
+    assert_eq!(level_room(screen, bar, 2, Some(low), frame), 630.0 - (30.0 + EDGE) - frame);
+    // Never more than the space under the bar, never less than the scroll arrows need.
+    let odd = egui::Rect::from_min_size(egui::pos2(100.0, -50.0), egui::vec2(200.0, 30.0));
+    assert!(level_room(screen, bar, 2, Some(odd), frame) <= below);
+    assert_eq!(level_room(egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(100.0, 40.0)), bar, 1, None, frame), 4.0 * ARROW);
+    assert_eq!(level_room(screen, Some(f32::NAN), 1, None, frame), level_room(screen, None, 1, None, frame));
+    assert_eq!(level_room(egui::Rect::NOTHING, bar, 2, Some(row), frame), 4.0 * ARROW);
+}

crates/ui-egui/src/menus.rs

@@ -855,7 +855,12 @@ fn render_level_rows(ui: &mut egui::Ui, items: &[&MenuItem], depth: usize, click
             let enabled = any_enabled || !child.is_empty();
             ui.add_enabled_ui(enabled, |ui| {
                 nav.row(ui, depth - 1, enabled, None, |ui, nav| {
-                    (ui.menu_button(crate::i18n::tr(lang, name), |ui| render_level(ui, &child, depth + 1, clicked, nav)).response, ())
+                    let r = ui.menu_button(crate::i18n::tr(lang, name), |ui| render_level(ui, &child, depth + 1, clicked, nav));
+                    if r.inner.is_some() {
+                        // Where the submenu hangs from, to keep it below the menu bar (#319).
+                        nav.set_anchor(depth, r.response.rect);
+                    }
+                    (r.response, ())
                 });
             });
             last_was_sep = false;