Merge main into feat/artboard-color-labels

crates/ui-egui/src/panels/appearance/tests.rs

@@ -1,4 +1,6 @@
-use super::super::tests_long_popups::{PopupWindow, assert_last_row_scrolls_into_view, assert_short_list_does_not_scroll, fx_menu_ends};
+use super::super::tests_long_popups::{
+    PopupWindow, assert_last_row_scrolls_into_view, assert_popup_stays_put, assert_short_list_does_not_scroll, fx_menu_ends,
+};
 use super::*;
 
 /// A selected rectangle, for the fill row's swatch picker and the Add Effect button.
@@ -81,6 +83,36 @@ fn the_add_effect_popup_scrolls_inside_a_short_window() {
     assert_short_list_does_not_scroll(&mut app_with_a_selected_rectangle(), draw_bottom_bar, fx_button, &first, &last);
 }
 
+/// The Add New Effect popup opened from a button `below` pt above the bottom of a window `height`
+/// pt tall stays at one place on every frame, inside the window.
+fn assert_add_effect_popup_stays_put(height: f32, below: f32) -> Rect {
+    let (first, _) = fx_menu_ends();
+    let draw = move |app: &mut VectorcraftApp, ui: &mut Ui| {
+        ui.add_space((height - below - 24.0).max(0.0));
+        draw_bottom_bar(app, ui);
+    };
+    assert_popup_stays_put(&mut app_with_a_selected_rectangle(), height, draw, fx_button, &first)
+}
+
+/// Opened from a button near the bottom of a short window, the Add New Effect popup stays at one
+/// place instead of being drawn above and below the button on alternate frames.
+#[test]
+fn the_add_effect_popup_stays_put_near_the_bottom_of_a_short_window() {
+    for height in [200.0, 300.0, 400.0, 600.0] {
+        let row = assert_add_effect_popup_stays_put(height, 8.0);
+        assert!(row.bottom() < height - 32.0, "in a {height} pt window the popup opens above the button: {row:?}");
+    }
+}
+
+/// Opened from a button near the top of a window, the popup opens below it and stays there.
+#[test]
+fn the_add_effect_popup_stays_put_below_a_button_near_the_top() {
+    for height in [200.0, 400.0, 900.0] {
+        let row = assert_add_effect_popup_stays_put(height, height - 32.0);
+        assert!(row.top() > 24.0, "in a {height} pt window the popup opens below the button: {row:?}");
+    }
+}
+
 /// Duplicating rows highlights the copies, each right above its original, as the Layers panel
 /// does: the active row's copy becomes active, the anchor's copy the anchor.
 #[test]

crates/ui-egui/src/panels/tests_long_popups.rs

@@ -1,4 +1,5 @@
-//! A headless window for checking that a panel's popup with no fixed length stays inside the window.
+//! A headless window for checking that a panel's popup with no fixed length stays inside the window,
+//! and the tests of menus built on `widgets::menu_scroll` that no panel draws.
 
 use egui::{Event, PointerButton, Pos2, Rect, Ui, vec2};
 
@@ -92,6 +93,70 @@ impl<'a> PopupWindow<'a> {
     }
 }
 
+/// The frames a popup is watched for after it opens.
+const STEADY_FRAMES: usize = 12;
+
+/// In a window `height` pt tall, the popup of the button `pick` finds draws its row `first` at the
+/// same place on every frame it is shown, from the click on, inside the window. With the pointer on
+/// that row, the row's submenu opens and every text of both stays in place too. Returns where the
+/// row is drawn.
+pub(super) fn assert_popup_stays_put(
+    app: &mut VectorcraftApp,
+    height: f32,
+    draw: impl FnMut(&mut VectorcraftApp, &mut Ui),
+    pick: impl FnOnce(&PopupWindow) -> Pos2,
+    first: &str,
+) -> Rect {
+    let opened = Opened { with: PointerButton::Primary, first, hover: first };
+    assert_popup_opened_stays_put(app, height, draw, pick, opened).0
+}
+
+/// How a popup is opened and checked: the mouse button `with` opens it, `first` is its first row,
+/// and the pointer then moves onto the row `hover` to open its submenu.
+struct Opened<'s> {
+    with: PointerButton,
+    first: &'s str,
+    hover: &'s str,
+}
+
+/// [`assert_popup_stays_put`] for a popup opened as `opened` says. Returns where the first row is
+/// drawn and where every text is drawn with the submenu open.
+fn assert_popup_opened_stays_put(
+    app: &mut VectorcraftApp,
+    height: f32,
+    draw: impl FnMut(&mut VectorcraftApp, &mut Ui),
+    pick: impl FnOnce(&PopupWindow) -> Pos2,
+    opened: Opened,
+) -> (Rect, Vec<(String, Rect)>) {
+    let Opened { with, first, hover } = opened;
+    let mut window = PopupWindow::new(app, height, draw);
+    let at = pick(&window);
+    let button = |pressed| Event::PointerButton { pos: at, button: with, pressed, modifiers: Default::default() };
+    window.frame(app, vec![Event::PointerMoved(at), button(true)]);
+    window.frame(app, vec![button(false)]);
+    let mut seen = vec![];
+    for _ in 0..STEADY_FRAMES {
+        seen.extend(window.text(first).map(|row| row.min));
+        window.frame(app, vec![]);
+    }
+    let row = window.text(first).unwrap_or_else(|| panic!("the popup's first row, {first}"));
+    seen.push(row.min);
+    assert!(seen.iter().all(|seen| *seen == row.min), "{first} is drawn at one place on every frame: {seen:?}");
+    assert!(row.top() >= 0.0 && row.bottom() <= height, "{first} at {row:?} is inside the {height} pt window");
+    let popup_texts = window.texts.clone();
+    let over = window.text(hover).unwrap_or_else(|| panic!("the popup's row {hover}"));
+    window.frame(app, vec![Event::PointerMoved(over.center())]);
+    window.frames(app, OPEN_FRAMES);
+    let places = |window: &PopupWindow| window.texts.iter().map(|(text, rect)| (text.clone(), rect.min)).collect::<Vec<_>>();
+    let shown = places(&window);
+    assert!(shown.iter().any(|(text, _)| !popup_texts.iter().any(|(old, _)| old == text)), "the pointer on {hover} opens its submenu");
+    for _ in 0..STEADY_FRAMES {
+        window.frame(app, vec![]);
+        assert_eq!(places(&window), shown, "the popup and its submenu stay in place");
+    }
+    (row, window.texts.clone())
+}
+
 /// The Add Effect popup's first and last rows with no plug-ins: the effect catalog's groups, in order.
 pub(super) fn fx_menu_ends() -> (String, String) {
     let mut groups: Vec<&str> = vec![];
@@ -145,3 +210,111 @@ pub(super) fn assert_short_list_does_not_scroll(
     window.wheel_down(app, before.0.center());
     assert_eq!(shown(&window), before, "a list that fits does not scroll");
 }
+
+/// Rows in each list of [`long_menu`]: more than any window in these tests holds.
+const LONG: usize = 40;
+
+/// A menu of rows `Row 0`, `Row 1`…, each a submenu of as many items.
+fn long_menu(ui: &mut Ui) {
+    menu_of(ui, LONG);
+}
+
+/// A menu of `rows` rows `Row 0`, `Row 1`…, each a submenu of [`LONG`] items.
+fn menu_of(ui: &mut Ui, rows: usize) {
+    crate::widgets::menu_scroll(ui, |ui| {
+        for row in 0..rows {
+            ui.menu_button(format!("Row {row}"), |ui| {
+                crate::widgets::menu_scroll(ui, |ui| {
+                    for item in 0..LONG {
+                        let _ = ui.button(format!("Item {row}.{item}"));
+                    }
+                })
+            });
+        }
+    });
+}
+
+/// A blank document, for windows that draw no panel.
+fn app() -> VectorcraftApp {
+    VectorcraftApp::new(vectorcraft_engine::Session::new(), Default::default())
+}
+
+/// A long menu opened from a button `y` pt down a window stays at one place on every frame, and so
+/// does its submenu, whether it opens below the button, above it, or in a window shorter than it.
+#[test]
+fn a_long_menu_stays_put_wherever_its_button_is() {
+    for height in [160.0, 300.0, 600.0] {
+        for y in [4.0, height / 2.0 - 12.0, height - 28.0] {
+            let draw = move |_: &mut VectorcraftApp, ui: &mut Ui| {
+                ui.add_space(y);
+                let button = ui.button("Open");
+                egui::Popup::menu(&button).show(long_menu);
+            };
+            let open = |window: &PopupWindow| window.text("Open").expect("the button").center();
+            assert_popup_stays_put(&mut app(), height, draw, open, "Row 0");
+        }
+    }
+}
+
+/// A long context menu opened near the top, the middle or the bottom of a short window stays at
+/// one place on every frame, and so does its submenu.
+#[test]
+fn a_long_context_menu_stays_put_wherever_it_opens() {
+    for height in [160.0, 300.0, 600.0] {
+        for y in [4.0, height / 2.0, height - 4.0] {
+            let draw = |_: &mut VectorcraftApp, ui: &mut Ui| {
+                let area = ui.interact(ui.max_rect(), egui::Id::unique("context-area"), egui::Sense::click());
+                area.context_menu(long_menu);
+            };
+            let at = move |_: &PopupWindow| Pos2::new(20.0, y);
+            let opened = Opened { with: PointerButton::Secondary, first: "Row 0", hover: "Row 0" };
+            assert_popup_opened_stays_put(&mut app(), height, draw, at, opened);
+        }
+    }
+}
+
+/// The long submenu of a row near the top of a window opens down from the row's top, and its list
+/// reaches down to the window's bottom less the menu gap.
+#[test]
+fn a_long_submenu_of_a_high_row_reaches_down_to_the_window_bottom() {
+    for height in [300.0, 600.0] {
+        let draw = |_: &mut VectorcraftApp, ui: &mut Ui| {
+            ui.add_space(4.0);
+            let button = ui.button("Open");
+            egui::Popup::menu(&button).show(|ui| menu_of(ui, 6));
+        };
+        let open = |window: &PopupWindow| window.text("Open").expect("the button").center();
+        let mut app = app();
+        let mut window = PopupWindow::new(&mut app, height, draw);
+        let at = open(&window);
+        window.click(&mut app, at);
+        let row = window.text("Row 0").expect("the menu's first row");
+        window.frame(&mut app, vec![Event::PointerMoved(row.center())]);
+        window.frames(&mut app, OPEN_FRAMES);
+        let item = window.text("Item 0.0").expect("the submenu's first item");
+        let areas = window.ctx.memory(|m| m.layer_ids().filter_map(|layer| m.area_rect(layer.id)).collect::<Vec<_>>());
+        let submenu = areas.into_iter().filter(|rect| rect.contains(item.center())).min_by(|a, b| a.area().total_cmp(&b.area()));
+        let submenu = submenu.expect("the submenu's area");
+        assert!(item.top() < row.bottom(), "the submenu opens down from {row:?}: {item:?}");
+        assert!((height - 12.0..=height).contains(&submenu.bottom()), "in a {height} pt window the submenu reaches its bottom: {submenu:?}");
+    }
+}
+
+/// The submenu of a row near the bottom of a window opens up from the row and stays at one place
+/// on every frame.
+#[test]
+fn a_long_submenu_of_a_low_row_opens_up_and_stays_put() {
+    for height in [300.0, 600.0] {
+        let draw = move |_: &mut VectorcraftApp, ui: &mut Ui| {
+            ui.add_space(height - 28.0);
+            let button = ui.button("Open");
+            egui::Popup::menu(&button).show(|ui| menu_of(ui, 6));
+        };
+        let open = |window: &PopupWindow| window.text("Open").expect("the button").center();
+        let opened = Opened { with: PointerButton::Primary, first: "Row 0", hover: "Row 5" };
+        let (_, texts) = assert_popup_opened_stays_put(&mut app(), height, draw, open, opened);
+        let at = |wanted: &str| texts.iter().rfind(|(text, _)| text.trim() == wanted).map(|(_, rect)| *rect);
+        let (row, item) = (at("Row 5").expect("the row"), at("Item 5.0").expect("the submenu's first item"));
+        assert!(item.top() < row.top() - 100.0, "in a {height} pt window the submenu opens up from {row:?}: {item:?}");
+    }
+}

crates/ui-egui/src/widgets.rs

@@ -45,13 +45,13 @@ pub use drag::{
 };
 #[cfg(test)]
 use dropdowns::combo_button_id;
-#[cfg(test)]
-pub(crate) use dropdowns::menu_floor;
 pub(crate) use dropdowns::{combo, combo_menu, combo_menu_field, combo_named, field_focused_in, popover_is_open};
 pub use dropdowns::{
     dropdown, dropdown_named, dropdown_names, dropdown_names_named, dropdown_with, dropdown_with_named, list_scroll_style, menu_room, menu_scroll,
     panel_scroll, popover, set_menu_floor, strip_scroll,
 };
+#[cfg(test)]
+pub(crate) use dropdowns::{menu_floor, menu_room_either};
 pub use labels::{dim_label, dim_name, divider, field_label, label_row, link_label, section_header, section_toggle, section_toggle_name, subheader};
 pub(crate) use labels::{label_field, take_previous_label, text_width};
 pub(crate) use numeric::{ANY, round_to, set_bare_numbers_are_points, typed_unit};
@@ -658,6 +658,27 @@ mod menu_room_tests {
         assert_eq!(menu_room(100.0, 24.0, 92.0), 92.0);
     }
 
+    /// A popup opens below its button while that leaves the minimum height, even when the list
+    /// then scrolls.
+    #[test]
+    fn a_menu_opens_below_while_that_side_has_the_minimum() {
+        assert_eq!(menu_room_either(300.0, 800.0, 980.0), 288.0);
+        assert_eq!(menu_room_either(132.0, 800.0, 980.0), 120.0);
+    }
+
+    /// With less than the minimum below, the list takes the room above, up to the window's height.
+    #[test]
+    fn a_menu_opens_above_when_below_is_too_short() {
+        assert_eq!(menu_room_either(40.0, 500.0, 980.0), 488.0);
+        assert_eq!(menu_room_either(40.0, 500.0, 300.0), 300.0);
+    }
+
+    /// With little room on either side, the list keeps the minimum height.
+    #[test]
+    fn a_menu_keeps_the_minimum_with_little_room_either_side() {
+        assert_eq!(menu_room_either(40.0, 60.0, 980.0), 120.0);
+    }
+
     /// The floor one window records does not reach a menu in another window of the same context.
     #[test]
     fn the_menu_floor_applies_in_the_window_that_recorded_it() {

crates/ui-egui/src/widgets/dropdowns.rs

@@ -284,12 +284,25 @@ pub(crate) fn menu_floor(ctx: &egui::Context) -> f32 {
     ctx.data(|d| d.get_temp::<f32>(id)).unwrap_or_else(|| ctx.content_rect().bottom())
 }
 
+/// The space a menu list leaves between its end and the window's edge or bars.
+const MENU_GAP: f32 = 12.0;
+/// The least height a menu list is given.
+const MENU_MIN_HEIGHT: f32 = 120.0;
+
 /// The height a menu list may take: from `top` down to `floor` less a small gap, at least 120 pt,
 /// and never more than `tallest`, the most that fits in the window.
 pub fn menu_room(floor: f32, top: f32, tallest: f32) -> f32 {
-    const BOTTOM_GAP: f32 = 12.0;
-    const MIN_HEIGHT: f32 = 120.0;
-    (floor - top - BOTTOM_GAP).max(MIN_HEIGHT).min(tallest)
+    (floor - top - MENU_GAP).max(MENU_MIN_HEIGHT).min(tallest)
+}
+
+/// The height a menu list may take when its popup can open below or above the rect it opens from:
+/// `below` and `above` are the heights each side leaves for the list. The list goes below while that
+/// side has room for the minimum height, otherwise on the side with more room. It gets at least
+/// 120 pt and never more than `tallest`.
+pub(crate) fn menu_room_either(below: f32, above: f32, tallest: f32) -> f32 {
+    let (below, above) = (below - MENU_GAP, above - MENU_GAP);
+    let room = if below >= MENU_MIN_HEIGHT { below } else { below.max(above) };
+    room.max(MENU_MIN_HEIGHT).min(tallest)
 }
 
 /// The scroll bar of a menu's list: slim, in a column of its own beside the items, so it never
@@ -298,14 +311,92 @@ pub fn list_scroll_style() -> egui::style::ScrollStyle {
     egui::style::ScrollStyle { bar_width: 6.0, bar_inner_margin: 2.0, ..egui::style::ScrollStyle::solid() }
 }
 
+/// The widget that opened a popup and whether it is a submenu's row (`None` when none was found),
+/// the frame the popup was opened in, and the frame it was last drawn in.
+type Opener = (Option<(egui::Id, bool)>, u64, u64);
+
+/// The frames after a popup opens in which a search that found no opener runs again.
+const OPENER_RETRY_FRAMES: u64 = 3;
+
+/// The widget that opened `popup`: a button whose popup has egui's default id, or a submenu's row,
+/// found among the widgets of this frame and the previous one. Once found, later frames of the same
+/// opening reuse it; a search that finds nothing runs again for the first few frames.
+fn popup_opener(ctx: &egui::Context, popup: egui::Id) -> Option<(egui::Id, bool)> {
+    let key = popup.with("menu-scroll-opener");
+    let frame = ctx.cumulative_frame_nr();
+    let cached = ctx.data(|d| d.get_temp::<Opener>(key)).filter(|&(_, _, seen)| seen + 1 >= frame);
+    let opened = cached.map_or(frame, |(_, opened, _)| opened);
+    let opener = match cached {
+        Some((found, _, _)) if found.is_some() || frame >= opened + OPENER_RETRY_FRAMES => found,
+        _ => {
+            let found = ctx.viewport(|vp| {
+                [&vp.this_pass, &vp.prev_pass].into_iter().flat_map(|pass| pass.widgets.layers()).flat_map(|(_, widgets)| widgets.iter()).find_map(
+                    |w| {
+                        let submenu = egui::containers::menu::SubMenu::id_from_widget_id(w.id) == popup;
+                        (submenu || w.id.with("popup") == popup).then_some((w.id, submenu))
+                    },
+                )
+            });
+            found.filter(|&(widget, submenu)| {
+                submenu || ctx.read_response(widget).is_some_and(|response| egui::Popup::default_response_id(&response) == popup)
+            })
+        }
+    };
+    ctx.data_mut(|d| d.insert_temp::<Opener>(key, (opener, opened, frame)));
+    opener
+}
+
+/// Where the popup or menu that `ui` is drawn in opens from: the rect of the button or row that
+/// opened it, or the point a context menu was opened at. The flag is true for a submenu, which
+/// opens beside its row: down from the row's top, or up from its bottom. While the opening widget
+/// is not drawn, the rect it last had is used. A popup with an id of its own (`Popup::id`) has no
+/// anchor.
+fn popup_anchor(ui: &Ui) -> Option<(Rect, bool)> {
+    let ctx = ui.ctx();
+    let popup = ui.layer_id().id;
+    let root = ui.stack().iter().last()?;
+    if !matches!(root.kind(), Some(egui::UiKind::Menu | egui::UiKind::Popup)) {
+        return None;
+    }
+    if let Some(at) = egui::Popup::position_of_id(ctx, popup) {
+        return Some((Rect::from_pos(at), false));
+    }
+    let (widget, submenu) = popup_opener(ctx, popup)?;
+    let key = popup.with("menu-scroll-anchor");
+    let drawn = ctx.read_response(widget).and_then(|response| egui::PopupAnchor::from(&response).rect(popup, ctx));
+    let rect = match drawn {
+        Some(rect) => {
+            ctx.data_mut(|d| d.insert_temp(key, rect));
+            rect
+        }
+        None => ctx.data(|d| d.get_temp::<Rect>(key))?,
+    };
+    // A submenu opens from its row grown by half the menu's margin, as egui's `SubMenu` places it.
+    let grow = if submenu { egui::Frame::menu(ui.style()).total_margin().sum().y * 0.5 } else { 0.0 };
+    Some((rect.expand2(vec2(0.0, grow)), submenu))
+}
+
 /// The body of a menu or popup list: as tall as its items up to the bottom bars of the window it
 /// opens in, and scrolling only past that, so long menus (Window, Effect, a panel's menu) stay
 /// reachable on small windows and in a floating panel's short OS window. Its scroll bar has a column
 /// of its own, so it never covers the shortcuts or the submenu arrows (#2086, #5347).
+///
+/// The list's height comes from the room below and above the button, row or point the popup opens
+/// from. A popup with an id of its own measures from its own top instead.
 pub fn menu_scroll<R>(ui: &mut Ui, add_contents: impl FnOnce(&mut Ui) -> R) -> R {
     let ctx = ui.ctx();
     let tallest = (ctx.content_rect().height() - egui::Frame::popup(ui.style()).total_margin().sum().y).max(0.0);
-    let room = menu_room(menu_floor(ctx), ui.next_widget_position().y, tallest);
+    let top = ui.next_widget_position().y;
+    // The rows above the list (the popup's margin, a search field) move with the popup.
+    let popup_top = ui.stack().iter().last().map_or(top, |root| root.max_rect.top());
+    let ahead = top - popup_top;
+    let (floor, window_top) = (menu_floor(ctx), ctx.content_rect().top());
+    let room = if let Some((anchor, submenu)) = popup_anchor(ui) {
+        let (down_from, up_to) = if submenu { (anchor.top(), anchor.bottom()) } else { (anchor.bottom(), anchor.top()) };
+        menu_room_either(floor - down_from - ahead, up_to - window_top - ahead, tallest)
+    } else {
+        menu_room(floor, top, tallest)
+    };
     ui.scope(|ui| {
         ui.spacing_mut().scroll = list_scroll_style();
         // The minimum lets the list grow past a popup's default 400 pt size; it still shrinks to

docs/mcp.md

@@ -3109,8 +3109,16 @@ The pattern commands (`object.pattern.make`, `object.pattern.edit`, `object.patt
 `object.repeat.options`) reject a param of the wrong type, such as `"instances": "8"`, `"edit": "false"` or
 `"tileType": 2`, and change nothing. `pattern.options` also rejects an `overlap` that is not an object, and an
 `overlap.h` other than `left` or `right` or an `overlap.v` other than `top` or `bottom`. `object.repeat.options` rejects
-a `center` that is not `[x, y]`, and `object.repeat.radial` rejects a negative `radius`. Omitting a param or giving null
-keeps its default.
+a `center` that is not `[x, y]`, and `object.repeat.radial` rejects a negative `radius`. In `object.pattern.make`,
+`object.pattern.saveCopy`, `object.repeat.radial`, `object.repeat.grid` and `object.repeat.mirror`, omitting a param or
+giving null keeps its default. In `pattern.options` and `object.repeat.options`, omitting a param or giving null keeps
+the pattern's or the repeat's current value, with two exceptions. A brick `tileType` in `pattern.options` or a brick
+`gridType` in `object.repeat.options` with `brickOffset` omitted or null sets the offset to 0.5. While `sizeTileToArt`
+is on, each `pattern.options` call sizes the tile to the art plus `hSpacing` and `vSpacing` again and ignores `width`
+and `height`, so a new `hSpacing` changes the tile's width. `pattern.options` still rejects a non-null `width` or
+`height` that is not a positive number, such as `"width": 0`, and changes nothing. Without a `name`,
+`object.pattern.edit` edits the pattern that fills or strokes a selected object, and `pattern.options` changes the
+pattern being edited; each returns an error when there is no such pattern. `pattern.delete` requires `name`.
 
 ```json
 {"name":"run_command","arguments":{"command":"pattern.options","params":{"showSwatchBounds":true}}}