diff --git a/crates/doc/src/project.rs b/crates/doc/src/project.rs index 6e42aa42..5378b1b8 100644 --- a/crates/doc/src/project.rs +++ b/crates/doc/src/project.rs @@ -485,7 +485,7 @@ fn tilted_circle(plane: &Plane, pts: &[Vec3]) -> Option> { let (c, n, r, closed) = circle3(pts)?; let sn = plane.normal(); let cosang = n.dot(sn).abs(); - if cosang > 1.0 - 1e-12 || cosang < 1e-9 { + if !(1e-9..=1.0 - 1e-12).contains(&cosang) { return None; } if closed { diff --git a/crates/doc/src/sheet.rs b/crates/doc/src/sheet.rs index cdbd6a91..6bee00c8 100644 --- a/crates/doc/src/sheet.rs +++ b/crates/doc/src/sheet.rs @@ -899,7 +899,7 @@ impl SheetBody { return Err(DocError::Invalid("the hem is shorter than its bend".into())); } // A hem folds back over the face it starts from (toward the bottom unless flipped). - let up = !top == flip; + let up = top != flip; let up = !up; self.flanges.push(SheetFlange { parent: panel, diff --git a/crates/engine/src/cmd/sketch_modify.rs b/crates/engine/src/cmd/sketch_modify.rs index 14d4b739..f79c3697 100644 --- a/crates/engine/src/cmd/sketch_modify.rs +++ b/crates/engine/src/cmd/sketch_modify.rs @@ -520,7 +520,7 @@ fn fillet_corner(sk: &mut Sketch, p: &Value, r: f64, cmd: &str) -> Result<(usize let u = (pa - pc).normalized().ok_or_else(|| bad(cmd, "degenerate line"))?; let v = (pb - pc).normalized().ok_or_else(|| bad(cmd, "degenerate line"))?; let half = 0.5 * u.cross(v).atan2(u.dot(v)).abs(); - if half < 1e-6 || half > std::f64::consts::FRAC_PI_2 - 1e-9 { + if !(1e-6..=std::f64::consts::FRAC_PI_2 - 1e-9).contains(&half) { return Err(bad(cmd, "the lines are parallel")); } let d = r / half.tan(); diff --git a/crates/sketch/src/model.rs b/crates/sketch/src/model.rs index d86535fb..2b5a0079 100644 --- a/crates/sketch/src/model.rs +++ b/crates/sketch/src/model.rs @@ -698,10 +698,8 @@ impl Sketch { | Symmetric { l, .. } | PointLineDistance { l, .. } | LinearDiameter { l, .. } => need_line(l)?, - ArcLength { c, .. } => { - if !matches!(self.curves.get(c).map(|c| &c.kind), Some(CurveKind::Arc { .. })) { - return Err(SketchError::WrongKind(name(c), "arc length needs an arc".into())); - } + ArcLength { c, .. } if !matches!(self.curves.get(c).map(|c| &c.kind), Some(CurveKind::Arc { .. })) => { + return Err(SketchError::WrongKind(name(c), "arc length needs an arc".into())); } Smooth { a, b } => { let ends = |i: usize| self.curves.get(i).and_then(|c| c.kind.ends()); @@ -730,10 +728,8 @@ impl Sketch { return Err(SketchError::WrongKind(name(a), "tangent needs a circle or arc".into())); } } - Equal { a, b } => { - if is_line(a) != is_line(b) { - return Err(SketchError::WrongKind(name(b), "equal needs two lines or two circles/arcs".into())); - } + Equal { a, b } if is_line(a) != is_line(b) => { + return Err(SketchError::WrongKind(name(b), "equal needs two lines or two circles/arcs".into())); } Coincident { p, q } if p == q => return Err(SketchError::Invalid("a point is always coincident with itself".into())), _ => {} diff --git a/crates/sketch/src/text.rs b/crates/sketch/src/text.rs index f0f378b0..f1c9d439 100644 --- a/crates/sketch/src/text.rs +++ b/crates/sketch/src/text.rs @@ -74,7 +74,7 @@ pub fn text_geometry(text: &str, at: Vec2, height: f64, angle: f64) -> Result MAX_TEXT_CHARS { return Err(SketchError::TooLarge); } - if !(height.is_finite() && height > 1e-6 && height < 1e6) || !angle.is_finite() || !at.is_finite() { + if !(height.is_finite() && height > 1e-6 && height < 1e6 && angle.is_finite() && at.is_finite()) { return Err(SketchError::Invalid("text height must be positive".into())); } let face = ttf_parser::Face::parse(FONT, 0).map_err(|e| SketchError::Invalid(format!("font: {e}")))?; diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 7f3f83ef..27912c30 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1855,7 +1855,7 @@ pub fn show(app: &mut SolveApp, ctx: &egui::Context) { Err(e) => d.error = Some(e), } } - if !(keep && !cancel) { + if !keep || cancel { crate::dialogs_motion::closed(app, &mut d, applied); } if keep && !cancel { diff --git a/crates/ui-egui/src/dim_view.rs b/crates/ui-egui/src/dim_view.rs index 66382a4c..105529eb 100644 --- a/crates/ui-egui/src/dim_view.rs +++ b/crates/ui-egui/src/dim_view.rs @@ -187,7 +187,7 @@ pub fn show(app: &mut SolveApp, ui: &egui::Ui, painter: &egui::Painter, proj: &P } let tk = Tokens::get(); let is_sel = |id: &str| app.session.selection.iter().any(|s| matches!(s, Sel::SketchConstraint { id: x } if x == id)); - let hover = ui.input(|i| i.pointer.hover_pos()).filter(|p| proj.rect.contains(*p)).and_then(hit); + let hover = ui.input(|i| i.pointer.hover_pos()).filter(|_| ui.rect_contains_pointer(proj.rect)).and_then(hit); let to = |p: Vec2| proj.to_screen(ss.plane.to_world(p)); let font = FontId::proportional(12.0); let centres = chain_centres(sk); diff --git a/crates/ui-egui/src/gizmo.rs b/crates/ui-egui/src/gizmo.rs index 5f1b1eb1..8c211e6a 100644 --- a/crates/ui-egui/src/gizmo.rs +++ b/crates/ui-egui/src/gizmo.rs @@ -255,7 +255,7 @@ pub fn show(ui: &mut egui::Ui, painter: &egui::Painter, proj: &Proj, id: egui::I } let state: Option = ui.data(|d| d.get_temp(id)); let pointer = ui.input(|i| i.pointer.hover_pos()); - let hovered = if state.is_none() { pointer.filter(|p| proj.rect.contains(*p)).and_then(|p| hit(&sh, p)) } else { None }; + let hovered = if state.is_none() { pointer.filter(|_| ui.rect_contains_pointer(proj.rect)).and_then(|p| hit(&sh, p)) } else { None }; let active = state.map(|s| s.handle).or(hovered); let hot = |h: Handle| active == Some(h); let col = |h: Handle, i: usize| if hot(h) { tk.accent } else { axis_color(i) }; diff --git a/crates/ui-egui/src/sketch_tools.rs b/crates/ui-egui/src/sketch_tools.rs index 2f87a9fc..39f02e8f 100644 --- a/crates/ui-egui/src/sketch_tools.rs +++ b/crates/ui-egui/src/sketch_tools.rs @@ -1090,7 +1090,7 @@ fn snap_hint(app: &SolveApp, painter: &egui::Painter, proj: &Proj) { /// Per frame, after the viewport is drawn: constraint glyphs and the text-entry box. pub fn show(app: &mut SolveApp, ui: &egui::Ui, painter: &egui::Painter, proj: &Proj) { SHIFT.with(|c| c.set(ui.input(|i| i.modifiers.shift))); - glyphs(app, painter, proj, ui.input(|i| i.pointer.hover_pos())); + glyphs(app, painter, proj, ui.input(|i| i.pointer.hover_pos()).filter(|_| ui.rect_contains_pointer(proj.rect))); snap_hint(app, painter, proj); combs(app, painter, proj); if DEFERRED.with(|d| d.replace(false)) { diff --git a/crates/ui-egui/src/viewport.rs b/crates/ui-egui/src/viewport.rs index d436dd90..0808c9dd 100644 --- a/crates/ui-egui/src/viewport.rs +++ b/crates/ui-egui/src/viewport.rs @@ -1371,7 +1371,8 @@ pub fn show(app: &mut SolveApp, ui: &mut egui::Ui) { let secondary_down = secondary_down && app.viewport.context_menu.is_none(); // The view cube handles its own clicks; the model underneath must not see them. let cube = Rect::from_center_size(pos2(rect.right() - 80.0, rect.top() + 80.0), vec2(150.0, 150.0)); - let inside = hover.is_some_and(|p| rect.contains(p) && !cube.contains(p)); + // Floating windows and menus own input over the model, including wheel navigation. + let inside = resp.contains_pointer() && hover.is_some_and(|p| !cube.contains(p)); app.viewport.mouse = hover.filter(|_| inside); // Ctrl or Alt held: drawing and dragging don't snap. crate::drag_snap::set_suppressed(mods.ctrl || mods.alt || mods.command); @@ -2086,7 +2087,7 @@ fn view_cube(app: &mut SolveApp, ui: &mut egui::Ui, rect: Rect) { let (r, u, b) = cube_basis(&app.cam); let (wr, wu, _) = app.cam.basis(); let painter = ui.painter_at(rect); - let hover = ui.input(|i| i.pointer.hover_pos()); + let hover = ui.input(|i| i.pointer.hover_pos()).filter(|_| ui.rect_contains_pointer(rect)); let targets = cube_targets(); let shapes = cube_projected(&targets, (r, u, b), c); let hovered = hover.and_then(|h| cube_hit(&shapes, h)); @@ -2534,6 +2535,124 @@ mod tests { } } + #[test] + fn shortcuts_window_wheel_does_not_zoom_the_viewport() { + let ctx = egui::Context::default(); + let mut app = SolveApp::new(solvecraft_engine::Session::default(), crate::Services::default()); + app.viewport.no_pixels = true; + crate::help::run(&mut app, &ctx, "shortcuts"); + let mut time = 0.0; + let mut frame = |app: &mut SolveApp, pointer: Option, wheel: f32| { + time += 1.0 / 60.0; + let mut events = Vec::new(); + if let Some(p) = pointer { + events.push(egui::Event::PointerMoved(p)); + } + if wheel != 0.0 { + events.push(egui::Event::MouseWheel { + unit: egui::MouseWheelUnit::Point, + delta: vec2(0.0, wheel), + modifiers: egui::Modifiers::NONE, + phase: egui::TouchPhase::Move, + }); + } + let _ = ctx.run_ui( + egui::RawInput { + screen_rect: Some(Rect::from_min_size(Pos2::ZERO, vec2(1100.0, 800.0))), + time: Some(time), + events, + ..Default::default() + }, + |ui| { + egui::CentralPanel::default().show(ui, |ui| show(app, ui)); + crate::prefs::show(app, ui.ctx()); + }, + ); + }; + // Let egui establish the floating window's size and hit-test widgets. + for _ in 0..4 { + frame(&mut app, None, 0.0); + } + let window = ctx.memory(|m| m.area_rect(egui::Id::new("sc_prefs"))).unwrap(); + let list = window.center() + vec2(140.0, 0.0); + let height = app.cam.half_height(); + let target = app.cam.target; + frame(&mut app, Some(list), 0.0); + for n in 0..12 { + frame(&mut app, Some(list), if n == 0 { -120.0 } else { 0.0 }); + assert_eq!(app.cam.half_height(), height, "wheel input over shortcuts must stay in the popup"); + assert_eq!(app.cam.target, target); + } + // The window's non-scrolling sidebar must also shield the model. + frame(&mut app, Some(window.left_center() + vec2(30.0, 0.0)), -120.0); + assert_eq!(app.cam.half_height(), height); + // An exposed part of the viewport still accepts wheel navigation with the popup open. + let exposed = pos2(40.0, 700.0); + assert!(!window.contains(exposed)); + frame(&mut app, Some(exposed), 0.0); + frame(&mut app, Some(exposed), -120.0); + assert_ne!(app.cam.half_height(), height); + // Closing the popup returns its former area to the viewport. + app.prefs_window.open = false; + for _ in 0..12 { + frame(&mut app, Some(list), 0.0); + } + let height = app.cam.half_height(); + frame(&mut app, Some(list), -120.0); + assert_ne!(app.cam.half_height(), height); + } + + #[test] + fn floating_dialog_blocks_viewport_hover_and_clicks() { + let mut h = crate::scenario::Harness::new(); + h.call("engine.execute", json!({"command": "solid.box", "params": {"length": 40, "width": 30, "height": 20}})); + h.call("ui.view", json!({"view": "fit"})); + h.app.ui.pick_bodies = true; + let p = h.app.viewport.rect.unwrap().center(); + h.call("ui.move", json!({"x": p.x, "y": p.y})); + assert!(h.app.viewport.hover.is_some(), "the test point must hover over the model"); + h.call("ui.click", json!({"x": p.x, "y": p.y})); + assert!(!h.app.session.selection.is_empty(), "the uncovered model must be selectable"); + h.app.session.selection.clear(); + + h.call("ui.prefs", json!({"open": "Navigation"})); + let window = h.ctx.memory(|m| m.area_rect(egui::Id::new("sc_prefs"))).unwrap(); + assert!(window.contains(p), "the dialog must cover the model at the test point"); + h.call("ui.move", json!({"x": p.x, "y": p.y})); + assert!(h.app.viewport.hover.is_none(), "the dialog must block model highlights"); + assert!(h.app.viewport.mouse.is_none()); + h.call("ui.click", json!({"x": p.x, "y": p.y})); + assert!(h.app.session.selection.is_empty(), "clicks in the dialog must not select the model"); + + h.call("ui.prefs", json!({"button": "cancel"})); + h.call("ui.move", json!({"x": p.x, "y": p.y})); + assert!(h.app.viewport.hover.is_some(), "closing the dialog must restore model highlights"); + h.call("ui.click", json!({"x": p.x, "y": p.y})); + assert!(!h.app.session.selection.is_empty()); + } + + #[test] + fn floating_dialog_blocks_view_cube_highlights_and_clicks() { + let mut h = crate::scenario::Harness::new(); + h.call("ui.resize", json!({"width": 800, "height": 700})); + let p = crate::scenario::handle_at("cube:top").unwrap(); + h.call("ui.move", json!({"x": p.x, "y": p.y})); + assert_eq!(crate::scenario::count_of("cube_highlights"), Some(1.0)); + h.call("ui.prefs", json!({"open": "Navigation"})); + let window = h.ctx.memory(|m| m.area_rect(egui::Id::new("sc_prefs"))).unwrap(); + assert!(window.contains(p), "the floating dialog must cover the cube target"); + let camera = serde_json::to_value(h.app.cam).unwrap(); + h.call("ui.move", json!({"x": p.x, "y": p.y})); + assert_eq!(crate::scenario::count_of("cube_highlights"), Some(0.0)); + h.call("ui.click", json!({"x": p.x, "y": p.y})); + assert_eq!(serde_json::to_value(h.app.cam).unwrap(), camera, "dialog clicks must not turn the view cube"); + h.call("ui.prefs", json!({"button": "cancel"})); + h.call("ui.move", json!({"x": p.x, "y": p.y})); + assert_eq!(crate::scenario::count_of("cube_highlights"), Some(1.0)); + h.call("ui.click", json!({"x": p.x, "y": p.y})); + assert_ne!(serde_json::to_value(h.app.cam).unwrap(), camera); + } + #[test] fn section_outlines_follow_the_cut() { let mut s = solvecraft_engine::Session::default(); diff --git a/crates/ui-egui/tests/scenarios.rs b/crates/ui-egui/tests/scenarios.rs index f0ef2c06..0574bb20 100644 --- a/crates/ui-egui/tests/scenarios.rs +++ b/crates/ui-egui/tests/scenarios.rs @@ -70,9 +70,10 @@ fn angle_dimension_both_interactive_workflows() { fn home_tab_and_file_recent_open_designs() { use serde_json::json; let mut h = Harness::new(); + let path = h.dir.join("recent.solvecraft").to_string_lossy().into_owned(); h.run(&json!([ {"call":"engine.execute","params":{"command":"solid.box","params":{"length":10,"width":10,"height":10}}}, - {"call":"engine.execute","params":{"command":"file.save_as","params":{"path":"$DIR/recent.solvecraft"}}}, + {"call":"engine.execute","params":{"command":"file.save_as","params":{"path":path}}}, {"click":{"handle":"tab:home"}}, {"expect":{"home":true,"documents":1}}, {"click":{"handle":"tab:design:0"}}, @@ -84,7 +85,6 @@ fn home_tab_and_file_recent_open_designs() { {"widget":"Open Recent"} ])) .unwrap(); - let path = h.dir.join("recent.solvecraft").to_string_lossy().into_owned(); h.run(&json!([ {"widget":path}, {"expect":{"home":false,"bodies":1,"documents":3}} diff --git a/xtask/src/layers.rs b/xtask/src/layers.rs index 0a65c1e8..29abb446 100644 --- a/xtask/src/layers.rs +++ b/xtask/src/layers.rs @@ -163,11 +163,10 @@ pub fn check(crates: &[Crate]) -> Vec { match classify(&d.name) { // Unregistered deps are reported on their own entry. None => {} - Some(Class::Testkit) => { - if d.kind != DepKind::Dev { - out.push(Violation::TestkitAsNormalDep { krate: c.name.clone() }); - } + Some(Class::Testkit) if d.kind != DepKind::Dev => { + out.push(Violation::TestkitAsNormalDep { krate: c.name.clone() }); } + Some(Class::Testkit) => {} Some(dc) => { let to = dc.layer().unwrap_or(u8::MAX); if to >= layer && !(to == layer && intra_layer_allowed(&c.name, &d.name)) {