mirror of
https://github.com/storytold/cadcraft.git
synced 2026-10-24 02:46:51 -06:00
fix(canvas): keep the OS cursor visible over menus and panels (#1879)
* fix(canvas): keep the OS cursor visible over menus and panels (fixes #1701) The drawing area hid the OS cursor with `CursorIcon::None` whenever the canvas response was hovered and the pointer lay inside the canvas rect. egui still reports the canvas as hovered when the pointer is over a menu popup's frame (the padding between its items and its border, and its separators): only widgets claim the hover, so an open menu drawn over the drawing left the canvas "hovered" and the cursor invisible there. The same happened over a dialog's padding or a floating palette over the canvas. Require the topmost layer under the pointer to be the canvas's own layer before treating the pointer as inside the drawing. Then menus, tooltips, dialogs and floating panels keep a visible cursor (and the crosshair stops under them, as over the menu bar already). Tests: `crates/ui-egui/tests/cursor_over_menus.rs` (headless egui_kittest) pins the canvas hiding the cursor, the menu bar and docked palettes keeping it, the Show System Cursor option restoring the arrow off the canvas, and the frame of an open menu over the drawing keeping the cursor: the last case failed before this change. The whole `cadcraft-ui-egui` test suite passes. * docs: document the functions touched by #1701 * fix(canvas): keep the OS cursor visible over canvas-painted menus Selection cycling's rows and Dynamic Input's options rows are menus, but the canvas paints them itself: they share the canvas's layer, so `layer_id_at` still reports the drawing and the OS cursor was hidden over them (the review of #1701 caught it). Exclude their rectangles from the crosshair's cursor contract, leaving the canvas's input and the Dynamic Input tooltip under the pointer untouched (so the list keeps highlighting and a click still picks its row). Tests: two cases in `crates/ui-egui/tests/cursor_over_menus.rs` hover a selection cycling row and a Dynamic Input options row and assert the OS cursor stays visible; the drawing beside the list still hides it. Both failed before this change. * The painted menu frame counts as over the menu (CodeRabbit) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * cargo fmt Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Brandon Thomas <echelon@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Nicholas Gower
co-authored by
Claude Opus 5.5
Brandon Thomas
Claude Opus 5.5
Brandon Thomas
parent
1c7d3f0fae
commit
0299a3eaa0
@@ -1347,6 +1347,14 @@ fn wheel_zoom_points(events: &[egui::Event], opts: &egui::InputOptions, page: f3
|
||||
if points.is_finite() { points } else { 0.0 }
|
||||
}
|
||||
|
||||
/// Draw the drawing area and handle its input (zoom and pan, picking, the crosshair) for this
|
||||
/// frame.
|
||||
///
|
||||
/// It also owns the cursor-visibility contract: the OS cursor is hidden only while the pointer is
|
||||
/// over the drawing area *and* the canvas is the topmost layer under it, so the drawn crosshair
|
||||
/// replaces the pointer over the drawing alone. A menu bar, an open menu (including its frame, not
|
||||
/// just its items), a tooltip or a floating palette above the canvas keeps the normal OS cursor
|
||||
/// visible (#1701), and so do the menus painted on the canvas itself (see [`over_canvas_menu`]).
|
||||
pub fn show(app: &mut CadApp, ui: &mut egui::Ui) {
|
||||
let t = Tokens::get();
|
||||
let full = ui.available_rect_before_wrap();
|
||||
@@ -1381,7 +1389,17 @@ pub fn show(app: &mut CadApp, ui: &mut egui::Ui) {
|
||||
i.pointer.button_double_clicked(egui::PointerButton::Primary),
|
||||
)
|
||||
});
|
||||
let inside = hover_pos.is_some_and(|p| rect.contains(p)) && resp.hovered();
|
||||
// The drawing area alone: a menu popup's frame (between its items and its border), a tooltip
|
||||
// or a floating palette lives on a layer above the canvas, where `hovered()` alone still
|
||||
// counts the canvas. Over those the OS cursor stays visible and the crosshair stays put, as
|
||||
// over the menu bar (#1701).
|
||||
let inside = hover_pos.is_some_and(|p| rect.contains(p) && ui.ctx().layer_id_at(p) == Some(ui.layer_id()) && resp.hovered());
|
||||
// The menus the canvas paints itself (selection cycling's rows, Dynamic Input's options) are
|
||||
// menus too, but they share the canvas's layer: `layer_id_at` reports the drawing under them,
|
||||
// so only their rectangles tell them apart from it ([`over_canvas_menu`]). They are excluded
|
||||
// from the cursor contract below, though not from the canvas's input: a click on a row is
|
||||
// taken by the row.
|
||||
let on_menu = hover_pos.is_some_and(|p| over_canvas_menu(app, p));
|
||||
// Whether Ctrl is held right now. Written every frame (not just while a command prompts), so
|
||||
// previews that read `Session::ctrl_down` always see the current state. The engine
|
||||
// deliberately does not carry modifiers through `Input`, which is UI-agnostic (see PR
|
||||
@@ -2007,7 +2025,7 @@ pub fn show(app: &mut CadApp, ui: &mut egui::Ui) {
|
||||
crate::cmdline::show(app, ui, full);
|
||||
}
|
||||
// Crosshair cursor.
|
||||
if inside {
|
||||
if inside && !on_menu {
|
||||
// Hide the OS cursor under the drawn crosshair, unless assistive tech needs it (#39).
|
||||
ui.ctx().set_cursor_icon(if app.ui.system_cursor { egui::CursorIcon::Crosshair } else { egui::CursorIcon::None });
|
||||
if let Some(hp) = hover_pos {
|
||||
@@ -2019,17 +2037,18 @@ pub fn show(app: &mut CadApp, ui: &mut egui::Ui) {
|
||||
cycle_badge(&painter, hp);
|
||||
}
|
||||
}
|
||||
if app.session.settings.dynmode
|
||||
}
|
||||
if inside
|
||||
&& app.session.settings.dynmode
|
||||
&& let Some(hp) = hover_pos
|
||||
{
|
||||
// A stretched grip shows its dimensions instead of the pointer boxes; a prompt asking
|
||||
// for a distance shows its one field and keeps the prompt and its options.
|
||||
// A stretched grip shows its dimensions instead of the pointer boxes; a prompt asking for
|
||||
// a distance shows its one field and keeps the prompt and its options.
|
||||
let fields = crate::grip_dyn::draw(app, &painter);
|
||||
if !fields || app.session.current_prompt().is_some_and(|p| p.distance) {
|
||||
dynamic_input(app, &painter, hp, fields);
|
||||
}
|
||||
}
|
||||
}
|
||||
// Over the crosshair, so the row under the pointer reads clearly.
|
||||
cycle_list(app, &painter);
|
||||
// No continuous repaint while hovering: input events repaint by themselves. Repainting every
|
||||
@@ -2174,6 +2193,15 @@ fn cycle_badge(p: &egui::Painter, at: Pos2) {
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether `p` lies on a menu the canvas paints itself — the selection cycling list's rows or the
|
||||
/// Dynamic Input options menu's rows. They are menus, so the OS cursor stays visible over them, but
|
||||
/// they share the canvas's layer: `layer_id_at` reports the drawing, and only their rectangles can
|
||||
/// tell them apart from it (#1701).
|
||||
fn over_canvas_menu(app: &CadApp, p: Pos2) -> bool {
|
||||
app.canvas.cycle.as_ref().is_some_and(|c| c.rows.iter().any(|r| r.expand(2.0).contains(p)))
|
||||
|| app.cmd.option_rows.iter().any(|(r, _)| r.expand(2.0).contains(p))
|
||||
}
|
||||
|
||||
/// The selection cycling list beside the click that opened it: a row per object, its type and
|
||||
/// layer, the row under the pointer highlighted; the rows are kept for clicks.
|
||||
fn cycle_list(app: &mut CadApp, p: &egui::Painter) {
|
||||
|
||||
@@ -0,0 +1,203 @@
|
||||
//! The OS cursor is hidden only over the drawing area (#1701): over the menu bar, a menu and the
|
||||
//! docked palettes the normal pointer comes back, so the user can see what they are pointing at.
|
||||
|
||||
use cadcraft_engine::Session;
|
||||
use cadcraft_geom::Vec2;
|
||||
use cadcraft_ui_egui::{CadApp, Services};
|
||||
use egui::accesskit::Role;
|
||||
use egui::vec2;
|
||||
use egui_kittest::Harness;
|
||||
use egui_kittest::kittest::Queryable;
|
||||
use serde_json::json;
|
||||
|
||||
/// A 1280×760 app with the in-window menu bar shown, stepped a few frames so menus and palettes
|
||||
/// have laid out before the test hovers anything.
|
||||
fn harness() -> Harness<'static, CadApp> {
|
||||
let mut app = CadApp::new(Session::new(), Services::default());
|
||||
app.ui.in_window_menu = true;
|
||||
let mut harness = Harness::builder().with_size(vec2(1280.0, 760.0)).build_ui_state(
|
||||
|ui, app: &mut CadApp| {
|
||||
app.logic(&ui.ctx().clone());
|
||||
app.ui(ui);
|
||||
},
|
||||
app,
|
||||
);
|
||||
harness.run_steps(5);
|
||||
harness
|
||||
}
|
||||
|
||||
/// The cursor icon the app asks the platform for in the last frame.
|
||||
fn cursor(harness: &Harness<'_, CadApp>) -> egui::CursorIcon {
|
||||
harness.output().platform_output.cursor_icon
|
||||
}
|
||||
|
||||
/// Point at the middle of the drawing area and settle the frames it takes the canvas to react, so
|
||||
/// `CursorIcon::None` there means the crosshair really is hiding the OS cursor.
|
||||
fn hover_canvas(harness: &mut Harness<'_, CadApp>) {
|
||||
harness.hover_at(egui::pos2(640.0, 400.0));
|
||||
harness.run_steps(3);
|
||||
}
|
||||
|
||||
/// The canvas hides the OS cursor; moving onto a menu title, which sits on a higher layer over no
|
||||
/// drawing, must bring the plain arrow back and must not leave `CursorIcon::None` behind.
|
||||
#[test]
|
||||
fn the_menu_bar_brings_the_os_cursor_back() {
|
||||
let mut harness = harness();
|
||||
hover_canvas(&mut harness);
|
||||
assert_eq!(cursor(&harness), egui::CursorIcon::None, "the drawn crosshair replaces the OS cursor on the canvas");
|
||||
|
||||
// Pointing at a menu title: the crosshair (and the hidden OS cursor) must not leak out of the
|
||||
// drawing area.
|
||||
harness.get_by_role_and_label(Role::Button, "File").hover();
|
||||
harness.run_steps(3);
|
||||
assert_ne!(cursor(&harness), egui::CursorIcon::None, "the menu bar keeps a visible OS cursor");
|
||||
assert_eq!(cursor(&harness), egui::CursorIcon::Default);
|
||||
}
|
||||
|
||||
/// Both parts of an open menu keep the OS cursor visible over the drawing: its items, and the
|
||||
/// frame between them and the border, where no item is hovered. Guards the layer check against
|
||||
/// `hovered()` alone, which still counts the canvas underneath the popup.
|
||||
#[test]
|
||||
fn an_open_menu_keeps_the_os_cursor_visible_over_the_drawing() {
|
||||
let mut harness = harness();
|
||||
let title = harness.get_by_role_and_label(Role::Button, "View");
|
||||
title.click();
|
||||
harness.run_steps(3);
|
||||
|
||||
// A menu item of the open popup lies over the drawing area.
|
||||
harness.get_by_label("Regen All").hover();
|
||||
harness.run_steps(3);
|
||||
assert_ne!(cursor(&harness), egui::CursorIcon::None, "a menu item over the canvas shows the pointer");
|
||||
|
||||
// The menu's frame (between its items and its border) is over the canvas too, but it is no
|
||||
// menu item: the drawing's crosshair must stay under it rather than replace the OS cursor.
|
||||
let popup = harness
|
||||
.ctx
|
||||
.memory(|m| {
|
||||
m.areas()
|
||||
.visible_layer_ids()
|
||||
.into_iter()
|
||||
.filter(|layer| layer.order != egui::Order::Background)
|
||||
.filter_map(|layer| m.area_rect(layer.id).map(|rect| (layer, rect)))
|
||||
.max_by(|(_, a), (_, b)| (a.width() * a.height()).total_cmp(&(b.width() * b.height())))
|
||||
})
|
||||
.expect("the open menu is an area");
|
||||
let frame = egui::pos2(popup.1.right() - 2.0, popup.1.bottom() - 2.0);
|
||||
assert!(harness.state().canvas.rect.is_some_and(|canvas| canvas.contains(frame)), "the menu's frame overlaps the drawing area");
|
||||
assert_eq!(harness.ctx.layer_id_at(frame), Some(popup.0), "the frame point is over the menu, not the drawing");
|
||||
harness.hover_at(frame);
|
||||
harness.run_steps(3);
|
||||
assert_ne!(cursor(&harness), egui::CursorIcon::None, "the menu's frame keeps the OS cursor visible");
|
||||
}
|
||||
|
||||
/// The docked palettes are ordinary (non-floating) panels, still above the canvas in the layer
|
||||
/// stack: leaving the drawing for one restores the OS cursor.
|
||||
#[test]
|
||||
fn docked_palettes_keep_the_os_cursor_visible() {
|
||||
let mut harness = harness();
|
||||
hover_canvas(&mut harness);
|
||||
assert_eq!(cursor(&harness), egui::CursorIcon::None);
|
||||
|
||||
// The Tool Sets palette docked left of the drawing.
|
||||
harness.hover_at(egui::pos2(100.0, 300.0));
|
||||
harness.run_steps(3);
|
||||
assert_ne!(cursor(&harness), egui::CursorIcon::None, "moving to a palette restores the OS cursor");
|
||||
}
|
||||
|
||||
/// With the assistive-technology fallback (`system_cursor`, #39) the canvas asks for the crosshair
|
||||
/// instead of hiding the pointer; off the canvas that request must not follow, so the menu bar
|
||||
/// gets the default arrow rather than a drafting crosshair.
|
||||
#[test]
|
||||
fn show_system_cursor_restores_the_arrow_off_the_canvas() {
|
||||
let mut harness = harness();
|
||||
harness.state_mut().ui.system_cursor = true;
|
||||
hover_canvas(&mut harness);
|
||||
assert_eq!(cursor(&harness), egui::CursorIcon::Crosshair);
|
||||
|
||||
harness.get_by_role_and_label(Role::Button, "File").hover();
|
||||
harness.run_steps(3);
|
||||
assert_eq!(cursor(&harness), egui::CursorIcon::Default, "not the drafting crosshair over the menu bar");
|
||||
}
|
||||
|
||||
/// Two overlapping lines (SELECTIONCYCLING 2), zoomed to fit: a click where they overlap opens the
|
||||
/// selection cycling list beside it.
|
||||
fn cycle_harness() -> Harness<'static, CadApp> {
|
||||
let mut session = Session::new();
|
||||
session.execute("line", &json!({ "points": [[0, 0], [10, 0]] })).unwrap();
|
||||
session.execute("line", &json!({ "points": [[0, 0], [20, 0]] })).unwrap();
|
||||
session.execute("setvar", &json!({ "name": "SELECTIONCYCLING", "value": 2 })).unwrap();
|
||||
session.set_selection(Vec::new());
|
||||
let mut app = CadApp::new(session, Services::default());
|
||||
app.ui.in_window_menu = true;
|
||||
app.canvas.zoom_pending = true;
|
||||
let mut harness = Harness::builder().with_size(vec2(1280.0, 760.0)).build_ui_state(
|
||||
|ui, app: &mut CadApp| {
|
||||
app.logic(&ui.ctx().clone());
|
||||
app.ui(ui);
|
||||
},
|
||||
app,
|
||||
);
|
||||
harness.run_steps(5);
|
||||
harness
|
||||
}
|
||||
|
||||
/// A press and release of the primary button at `at`.
|
||||
fn click(harness: &mut Harness<'_, CadApp>, at: egui::Pos2) {
|
||||
harness.event(egui::Event::PointerMoved(at));
|
||||
harness.run_steps(1);
|
||||
harness.event(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: true, modifiers: egui::Modifiers::NONE });
|
||||
harness.event(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: false, modifiers: egui::Modifiers::NONE });
|
||||
harness.run_steps(2);
|
||||
}
|
||||
|
||||
/// Selection cycling's rows are painted on the canvas, so the layer under them is the drawing, but
|
||||
/// they are a menu: the OS cursor stays visible over a row, while the drawing around it still
|
||||
/// hides the cursor under the crosshair (#1701).
|
||||
#[test]
|
||||
fn a_selection_cycling_row_keeps_the_os_cursor_visible() {
|
||||
let mut harness = cycle_harness();
|
||||
let at = harness.state().canvas.xf.expect("the canvas is laid out").to_screen(Vec2::new(5.0, 0.0));
|
||||
click(&mut harness, at);
|
||||
let rows = harness.state().canvas.cycle_rows();
|
||||
assert_eq!(rows.len(), 2, "the click over both lines lists them");
|
||||
let row = rows[1].0;
|
||||
assert!(harness.state().canvas.rect.is_some_and(|canvas| canvas.contains(row.center())), "the list lies over the drawing");
|
||||
|
||||
// The drawing beside the list still hides the OS cursor.
|
||||
let beside = egui::pos2(row.left() - 4.0, row.center().y);
|
||||
harness.hover_at(beside);
|
||||
harness.run_steps(3);
|
||||
assert_eq!(cursor(&harness), egui::CursorIcon::None, "the canvas beside the list hides the OS cursor");
|
||||
|
||||
// Over the row itself the pointer must come back, and the row keeps highlighting its object.
|
||||
harness.hover_at(row.center());
|
||||
harness.run_steps(3);
|
||||
assert_ne!(cursor(&harness), egui::CursorIcon::None, "a selection cycling row is a menu and keeps a visible OS cursor");
|
||||
assert_eq!(harness.state().canvas.hover, Some(rows[1].1));
|
||||
assert_eq!(harness.state().canvas.cycle_rows().len(), 2, "the list stays open under the pointer");
|
||||
}
|
||||
|
||||
/// Dynamic Input's options menu is painted on the canvas too: over one of its rows the OS cursor
|
||||
/// stays visible (and the menu stays open, so a click can still pick the row).
|
||||
#[test]
|
||||
fn a_dynamic_input_options_row_keeps_the_os_cursor_visible() {
|
||||
let mut harness = harness();
|
||||
hover_canvas(&mut harness);
|
||||
assert_eq!(cursor(&harness), egui::CursorIcon::None, "the canvas hides the OS cursor");
|
||||
|
||||
harness.state_mut().cmdline("circle");
|
||||
harness.run_steps(3);
|
||||
harness.key_press(egui::Key::ArrowDown);
|
||||
harness.run_steps(3);
|
||||
let rows = harness.state().cmd.option_rows.clone();
|
||||
assert!(harness.state().cmd.options.is_some() && rows.len() >= 2, "Down Arrow lists the prompt's options over the canvas: {rows:?}");
|
||||
let row = rows[0].0;
|
||||
assert!(harness.state().canvas.rect.is_some_and(|canvas| canvas.contains(row.center())), "the options menu lies over the drawing");
|
||||
|
||||
harness.hover_at(row.center());
|
||||
harness.run_steps(3);
|
||||
assert_ne!(cursor(&harness), egui::CursorIcon::None, "a Dynamic Input options row is a menu and keeps a visible OS cursor");
|
||||
assert!(harness.state().cmd.options.is_some(), "the menu stays open under the pointer");
|
||||
assert!(harness.state().cmd.option_rows.iter().any(|(r, _)| r.contains(row.center())), "the row is still drawn for clicks");
|
||||
assert!(harness.state().canvas.cursor.is_some(), "the canvas keeps tracking the pointer under the menu, so the value boxes stay put");
|
||||
}
|
||||
Reference in New Issue
Block a user