From 6b4691550781e6e687e8d21c0b154c7b3b69392f Mon Sep 17 00:00:00 2001 From: lillith <35337584+0xlillith@users.noreply.github.com> Date: Sun, 11 Oct 2026 00:32:48 +0200 Subject: [PATCH 01/10] docs: align testing guide with the CI profile --- docs/book/src/dev/testing.md | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/docs/book/src/dev/testing.md b/docs/book/src/dev/testing.md index 1be9377e..2e903b54 100644 --- a/docs/book/src/dev/testing.md +++ b/docs/book/src/dev/testing.md @@ -1,7 +1,8 @@ # Test harnesses -`cargo xtask ci` runs everything a pull request must pass: fmt, clippy with `-D warnings`, the -tests in release mode, `xtask layers`, `xtask assets` and the wasm build check. +`cargo xtask ci` runs everything a pull request must pass: fmt, `xtask assets`, +`xtask licences --check`, `xtask layers`, clippy with `-D warnings`, the tests with the `ci` +profile and the wasm build check. | Harness | Where | What it catches | |---|---|---| @@ -34,7 +35,14 @@ A new file in `crates/ui-egui/tests/scenarios/` is picked up automatically. To r before it's fixed, add a `{"pending": "why"}` step; its test is then ignored with that reason, and the fix removes the step. -## Release mode +## CI profile -CI runs the tests with `--release`, because some truck assertions only exist in debug builds -and fail on valid geometry. If a test fails only in debug, say so in the PR. +CI runs the tests with `--profile ci`: light optimisation, line tables, no LTO and debug +assertions off. Some truck debug assertions fail on valid geometry; the `ci` profile disables +them while building faster than `--release`. If a test fails only in a plain debug build, say +so in the PR. + +Run `cargo xtask test` to build with this profile and run the test binaries concurrently, +with doc tests alongside. Arguments after `--` go to each test binary, for example +`cargo xtask test -- shell`. To run the test binaries sequentially instead, use +`cargo test --workspace --profile ci`. From 347230f5d396aac7e437df2d8e6d337df84ac295 Mon Sep 17 00:00:00 2001 From: lillith <35337584+0xlillith@users.noreply.github.com> Date: Sun, 11 Oct 2026 00:41:49 +0200 Subject: [PATCH 02/10] fix: validate fuzz seed ranges without overflow --- apps/solvecraft-cli/src/fuzz.rs | 13 ++++++----- apps/solvecraft-cli/tests/fuzz_range.rs | 31 +++++++++++++++++++++++++ docs/robustness.md | 4 ++++ 3 files changed, 42 insertions(+), 6 deletions(-) create mode 100644 apps/solvecraft-cli/tests/fuzz_range.rs diff --git a/apps/solvecraft-cli/src/fuzz.rs b/apps/solvecraft-cli/src/fuzz.rs index f8fc26b3..7a985f64 100644 --- a/apps/solvecraft-cli/src/fuzz.rs +++ b/apps/solvecraft-cli/src/fuzz.rs @@ -64,6 +64,8 @@ struct Job { pub fn run(args: &[String]) -> Result<(), String> { let from = flag(args, "--from", 0u64); let count = flag(args, "--count", 100u64); + let last = from.checked_add(count.saturating_sub(1)).ok_or("seed range exceeds u64::MAX: reduce --from or --count")?; + let mut seeds = (count > 0).then_some(from..=last).into_iter().flatten().peekable(); let jobs = flag(args, "--jobs", 4usize).clamp(1, 64); let timeout = Duration::from_secs(flag(args, "--timeout", 120u64)); let steps = flag(args, "--steps", 8usize); @@ -74,14 +76,14 @@ pub fn run(args: &[String]) -> Result<(), String> { if let Some(d) = &out_dir { std::fs::create_dir_all(d).map_err(|e| format!("{d}: {e}"))?; } - let mut next = from; let mut running: Vec = Vec::new(); let mut results: Vec = Vec::new(); let t0 = Instant::now(); - while next < from + count || !running.is_empty() { - while running.len() < jobs && next < from + count { + while seeds.peek().is_some() || !running.is_empty() { + while running.len() < jobs { + let Some(seed) = seeds.next() else { break }; let mut cmd = Command::new(&exe); - cmd.args(["fuzz-one", &next.to_string(), "--steps", &steps.to_string()]); + cmd.args(["fuzz-one", &seed.to_string(), "--steps", &steps.to_string()]); if minimise { cmd.arg("--minimise"); } @@ -96,8 +98,7 @@ pub fn run(args: &[String]) -> Result<(), String> { } }); } - running.push(Job { seed: next, child, started: Instant::now(), err: rx, last: String::new() }); - next += 1; + running.push(Job { seed, child, started: Instant::now(), err: rx, last: String::new() }); } std::thread::sleep(Duration::from_millis(50)); let mut i = 0; diff --git a/apps/solvecraft-cli/tests/fuzz_range.rs b/apps/solvecraft-cli/tests/fuzz_range.rs new file mode 100644 index 00000000..60c2ae20 --- /dev/null +++ b/apps/solvecraft-cli/tests/fuzz_range.rs @@ -0,0 +1,31 @@ +//! Seed ranges must not crash or wrap, including at the last representable seed. + +use std::process::Command; + +#[test] +fn overflowing_seed_range_is_an_error_not_a_panic() { + let output = + Command::new(env!("CARGO_BIN_EXE_solvecraft-cli")).args(["fuzz", "--from", "18446744073709551615", "--count", "2"]).output().unwrap(); + assert_eq!(output.status.code(), Some(1)); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("seed range exceeds u64::MAX"), "{stderr}"); + assert!(!stderr.contains("panicked"), "{stderr}"); +} + +#[test] +fn empty_seed_range_at_maximum_is_valid() { + let output = + Command::new(env!("CARGO_BIN_EXE_solvecraft-cli")).args(["fuzz", "--from", "18446744073709551615", "--count", "0"]).output().unwrap(); + assert!(output.status.success(), "{}", String::from_utf8_lossy(&output.stderr)); + assert!(String::from_utf8_lossy(&output.stdout).starts_with("0 seeds, 0 design steps,")); +} + +#[test] +fn maximum_seed_runs_once_without_wrapping() { + let output = Command::new(env!("CARGO_BIN_EXE_solvecraft-cli")) + .args(["fuzz", "--from", "18446744073709551615", "--count", "1", "--steps", "0", "--timeout", "0"]) + .output() + .unwrap(); + assert!(output.status.success(), "{}", String::from_utf8_lossy(&output.stderr)); + assert!(String::from_utf8_lossy(&output.stdout).starts_with("1 seeds,")); +} diff --git a/docs/robustness.md b/docs/robustness.md index 440b96bd..4639aa10 100644 --- a/docs/robustness.md +++ b/docs/robustness.md @@ -11,6 +11,10 @@ solvecraft-cli fuzz --from 0 --count 1000 --jobs 8 --timeout 90 --out runs/ --re solvecraft-cli fuzz-one --minimise # one design, cut down to the steps that matter ``` +`--count` seeds are run starting at `--from`. A range that exceeds `u64::MAX` is +rejected before any jobs or output directories are created. The maximum seed +(`18446744073709551615`) is valid with `--count 1`; `--count 0` runs no seeds. + - **Designs** (`crates/engine/src/fuzz.rs`): a seeded generator starts from a box, an extruded sketch profile closed by an arc (sometimes notched), or a revolved section (partial or full turns, sometimes touching the axis). It then adds up to eight steps: joins and cuts of From 8c1b1478cedff1fd9e0f3cf1d21ef7d8e33b48e4 Mon Sep 17 00:00:00 2001 From: CypherNaugh_0x <9931495+CypherNaught-0x@users.noreply.github.com> Date: Sun, 11 Oct 2026 05:33:50 +0200 Subject: [PATCH 03/10] =?UTF-8?q?Joins=20and=20cuts=20of=20round=20pads,?= =?UTF-8?q?=20bosses=20and=20holes=20across=20a=20floor=20that=20falls=20a?= =?UTF-8?q?=20degree=20or=20two=20no=20longer=20fail:=20the=20B-rep=20resu?= =?UTF-8?q?lts=20were=20right,=20but=20the=20plausibility=20check=20meshed?= =?UTF-8?q?=20them=20without=20refinement,=20and=20where=20the=20shallow?= =?UTF-8?q?=20cut=20trims=20a=20round=20side=20to=20a=20thin=20band=20the?= =?UTF-8?q?=20plain=20mesh=20spans=20it=20with=20triangles=20cutting=20thr?= =?UTF-8?q?ough=20the=20cylinder,=20so=20the=20check=20saw=20missing=20mat?= =?UTF-8?q?erial=20(0.8=20to=201.6=20%=20of=20samples=20misclassified),=20?= =?UTF-8?q?rejected=20the=20result,=20and=20the=20planar=20fallback=20then?= =?UTF-8?q?=20refused=20the=20round=20tool.=20The=20check=20now=20meshes?= =?UTF-8?q?=20the=20operands=20and=20the=20result=20with=20refinement=20on?= =?UTF-8?q?=20curved=20faces=20(as=20measure=20does).=20Test:=20round=5Fpa?= =?UTF-8?q?ds=5Fcrossing=5Fa=5Fshallow=5Ffall=20(a=201=C2=B0=20fall=20and?= =?UTF-8?q?=20a=202=C2=B0=20four-face=20fall=20toward=20a=20drain)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/kernel/src/ops.rs | 9 ++++-- crates/kernel/src/tests.rs | 59 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/crates/kernel/src/ops.rs b/crates/kernel/src/ops.rs index 045f48ba..7b0318a8 100644 --- a/crates/kernel/src/ops.rs +++ b/crates/kernel/src/ops.rs @@ -249,9 +249,12 @@ fn boolean_whole(a: &Body, b: &Body, op: BoolOp) -> Result> { let size = a.size().max(b.size()); // The checks mesh both bodies; where they meet in a region much thinner than they are (a // thin part trimmed by a long one), a step from their size would facet the small faces - // there coarsely enough to fail the membership test on a correct result. + // there coarsely enough to fail the membership test on a correct result. The meshes are + // refined: a round side trimmed to a thin band (a short boss crossing a gently sloping + // floor) is otherwise spanned by triangles cutting through it, which reads as missing + // material. let tol_m = (size * 5e-4).min(meeting_size(a, b) * 2e-3).max(size * 1e-5); - let meshes = (a.tessellate(tol_m).ok(), b.tessellate(tol_m).ok()); + let meshes = (a.tessellate_with(tol_m, true).ok(), b.tessellate_with(tol_m, true).ok()); let (va, vb) = match &meshes { (Some(ma), Some(mb)) => (ma.measure().volume, mb.measure().volume), _ => (volume(a), volume(b)), @@ -260,7 +263,7 @@ fn boolean_whole(a: &Body, b: &Body, op: BoolOp) -> Result> { // Volume bounds, then membership against the operands (catches misclassified pieces), on // one mesh of the result. Returns (plausible, volume). let check = |r: &Body| -> (bool, f64) { - let Ok(mr) = r.tessellate(tol_m) else { return (false, f64::NAN) }; + let Ok(mr) = r.tessellate_with(tol_m, true) else { return (false, f64::NAN) }; let v = mr.measure().volume; let bounds = v > 0.0 && match op { diff --git a/crates/kernel/src/tests.rs b/crates/kernel/src/tests.rs index b65c41a5..56a52453 100644 --- a/crates/kernel/src/tests.rs +++ b/crates/kernel/src/tests.rs @@ -290,6 +290,65 @@ fn moving_a_body_with_an_intersection_curve_edge_ends() { assert!(rel(v0, v1) < 1e-6, "{v0} vs {v1}"); } +/// A trough whose floor falls 1° (or 2° toward a drain, in four faces), and short round pads +/// and bosses crossing the fall. The B-rep results were right, but the plausibility check meshed +/// them without refinement: on a round side trimmed to a thin band by the shallow cut, the plain +/// mesh spans the band with triangles cutting through the cylinder, so the check saw missing +/// material, rejected the result, and the planar fallback refused the round pad. +#[test] +fn round_pads_crossing_a_shallow_fall() { + let hs = |n: Vec3, p: Vec3| { + let n = n.normalized().unwrap(); + HalfSpace { n, d: n.dot(p) } + }; + let walls = |floor: &[HalfSpace]| { + let mut h = vec![ + hs(Vec3::X, Vec3::new(290.0, 0.0, 0.0)), + hs(-Vec3::X, Vec3::new(10.0, 0.0, 0.0)), + hs(Vec3::Y, Vec3::new(0.0, 140.0, 0.0)), + hs(-Vec3::Y, Vec3::new(0.0, 10.0, 0.0)), + hs(Vec3::Z, Vec3::new(0.0, 0.0, 90.0)), + ]; + h.extend_from_slice(floor); + convex_polyhedron(&h).unwrap() + }; + let block = box_solid(Vec3::ZERO, Vec3::new(300.0, 150.0, 80.0)).unwrap(); + // One fall along x at 1°: floor z = 8 + t (x − 10). + let t = 1f64.to_radians().tan(); + let fall = boolean(&block, &walls(&[hs(Vec3::new(t, 0.0, -1.0), Vec3::new(10.0, 0.0, 8.0))]), BoolOp::Cut).unwrap().unwrap(); + let v0 = measure(&fall).unwrap().volume; + for x in [60.0, 100.0] { + // A pad from z = 9 to 12 (its bottom just under the floor, its top 2–3 mm above). + let pad = cylinder(Vec3::new(x, 75.0, 9.0), Vec3::Z, 15.0, 3.0).unwrap(); + let u = boolean(&fall, &pad, BoolOp::Union).unwrap().unwrap(); + // The part of the pad above the floor: the floor under the disc is planar, so its mean + // height is the height at the centre (where the floor is below z = 9 everywhere). + let floor = |x: f64| 8.0 + t * (x - 10.0); + let above = if floor(x - 15.0) >= 9.0 { PI * 225.0 * (12.0 - floor(x)) } else { f64::NAN }; + let v = measure(&u).unwrap().volume; + if above.is_finite() { + assert!(rel(v, v0 + above) < 1e-5, "pad at x {x}: {v} vs {}", v0 + above); + } else { + assert!(v > v0 && v < v0 + PI * 225.0 * 3.0, "pad at x {x}: {v}"); + } + } + // Four faces falling 2° toward a drain point, and a pad over a valley edge and a wall. + let t2 = 2f64.to_radians().tan(); + let d = Vec3::new(240.0, 110.0, 8.0); + let hopper = walls(&[ + hs(Vec3::new(t2, 0.0, -1.0), d), + hs(Vec3::new(-t2, 0.0, -1.0), d), + hs(Vec3::new(0.0, t2, -1.0), d), + hs(Vec3::new(0.0, -t2, -1.0), d), + ]); + let trough = boolean(&block, &hopper, BoolOp::Cut).unwrap().unwrap(); + let pad = cylinder(Vec3::new(270.0, 70.0, 8.0), Vec3::Z, 25.0, 5.0).unwrap(); + let u = boolean(&trough, &pad, BoolOp::Union).unwrap().unwrap(); + // Integrated over the disc (outside the wall at x = 290): about 6517 mm³ above the floor. + let gain = measure(&u).unwrap().volume - measure(&trough).unwrap().volume; + assert!((gain - 6517.0).abs() < 10.0, "{gain}"); +} + #[test] fn planar_booleans_with_coincident_faces() { // L-bracket: a wall flush with three sides of its base. From 75e9472a4718e33fcb3feb83f2781330108ac244 Mon Sep 17 00:00:00 2001 From: GU Junhua Date: Sun, 11 Oct 2026 16:07:46 +0800 Subject: [PATCH 04/10] modified: CONTRIBUTING.md new file: flake.lock new file: flake.nix --- CONTRIBUTING.md | 40 +++++++++++++++ flake.lock | 27 ++++++++++ flake.nix | 133 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 200 insertions(+) create mode 100644 flake.lock create mode 100644 flake.nix diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ee245cdd..f941e55c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -28,6 +28,46 @@ HTTPS or localhost, because it uses the Origin Private File System. Windows from Linux: `cargo xwin build --release --target x86_64-pc-windows-msvc -p solvecraft`. +### NixOS: `nix develop` + +NixOS has no `/usr/lib`, so nothing is on `LD_LIBRARY_PATH` and a plain `cargo run` finds none of +the libraries the app loads at run time: the Wayland ones winit opens the window through, the EGL +loader wgpu renders through, and libdbus-1, which rfd's file dialogs reach xdg-desktop-portal +through (a dialog then never appears and the pick returns no path): + +``` +ERROR solvecraft: winit EventLoopError: ... The wayland library could not be loaded +ERROR eframe::native::run: Exiting because of error: WGPU error: Failed to create surface for any enabled backend: {} +ERROR rfd::backend::xdg_desktop_portal::portal::libdbus: Can't connect to a portal: libdbus-1.so not found +``` + +`nix develop` (the [`flake.nix`](flake.nix) in the repository root) puts the Wayland, X11, +EGL-loader and D-Bus libraries on `LD_LIBRARY_PATH`, so `cargo run --release -p solvecraft` works +from that shell as on any other distribution, file dialogs included. `nix run` builds the app and +wraps the binary the same way. The Mesa drivers stay the system's, from `/run/opengl-driver` +(`hardware.graphics.enable`): that is the driver stack the compositor runs on, and wgpu reaches the +integrated GPU through the EGL loader. The shell deliberately brings no Rust toolchain — the one on +`PATH` keeps its build cache, where a second `rustc` would make every build recompile the workspace +from scratch; `nix develop .#rust` is there for a machine with no Rust at all. + +**Anti-aliasing on the OpenGL backend.** NixOS keeps the Vulkan ICD descriptions +(`/run/opengl-driver/share/vulkan/icd.d`) off the Vulkan loader's search path, so wgpu picks its +OpenGL backend on the integrated GPU. On that backend any multisampling above 1× — the default is +4× — draws a black window: the UI still lays out and the control channel still answers, but every +frame, `ui.screenshot` included, comes out empty. Set **Preferences ▸ Graphics ▸ Anti-aliasing** to +1× and restart (the setting applies on the next start), or write it before the first run, since a +black window is no way to reach the dialog: + +```sh +mkdir -p ~/.local/share/solvecraft +echo '{"msaa": 1}' > ~/.local/share/solvecraft/graphics.json +``` + +`apps/solvecraft/src/main.rs` reads that file (`graphics_path()`, eframe's storage dir) before the +window opens; 2× and 4× are the same black window. The Vulkan backend is no way out of it either: +`libvulkan.so.1` is not on `LD_LIBRARY_PATH`, so `WGPU_BACKEND=vulkan` exits with a +`CreateSurfaceError` instead. + ## How the code is laid out The [README](README.md#architecture) has the layer table. The short version: diff --git a/flake.lock b/flake.lock new file mode 100644 index 00000000..915f6ef3 --- /dev/null +++ b/flake.lock @@ -0,0 +1,27 @@ +{ + "nodes": { + "nixpkgs": { + "locked": { + "lastModified": 1791594108, + "narHash": "sha256-XK4KceonkwD8LuMAt/vNrz982mXJM5jeHAxMwsUXXAQ=", + "owner": "NixOS", + "repo": "nixpkgs", + "rev": "6510408d147d8c5e6d76043e899e9251756f6a75", + "type": "github" + }, + "original": { + "owner": "NixOS", + "ref": "nixos-unstable", + "repo": "nixpkgs", + "type": "github" + } + }, + "root": { + "inputs": { + "nixpkgs": "nixpkgs" + } + } + }, + "root": "root", + "version": 7 +} diff --git a/flake.nix b/flake.nix new file mode 100644 index 00000000..6cab8489 --- /dev/null +++ b/flake.nix @@ -0,0 +1,133 @@ +{ + description = "SolveCraft — native app development shell and package for Linux"; + + inputs.nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable"; + + outputs = + { self, nixpkgs }: + let + systems = [ + "x86_64-linux" + "aarch64-linux" + ]; + forAllSystems = f: nixpkgs.lib.genAttrs systems (system: f nixpkgs.legacyPackages.${system}); + + # Why this shell exists. winit opens the window by dlopen'ing the Wayland client libraries, + # wgpu renders through the EGL loader (libEGL.so.1), and rfd's file dialogs reach + # xdg-desktop-portal through libdbus-1; a plain `cargo run` under NixOS finds none of them, + # because there is no /usr/lib and nothing puts them on LD_LIBRARY_PATH: + # winit EventLoopError: The wayland library could not be loaded + # eframe: WGPU error: Failed to create surface for any enabled backend: {} + # rfd::backend::xdg_desktop_portal::portal::libdbus: Can't connect to a portal: libdbus-1.so not found + # The Mesa drivers themselves — and the EGL vendor / Vulkan ICD descriptions that name them — + # come from the system in /run/opengl-driver (hardware.graphics.enable), not from here, since + # that is the driver stack the compositor runs on. Verified on an Intel LNL iGPU under + # Wayland: wgpu picks the GL backend through this loader and draws. + # The X11 libraries are the ones the app actually uses on a Wayland desktop: with `DISPLAY` + # set and `SOLVECRAFT_WAYLAND` unset, winit runs through XWayland (file drag and drop exists + # only there). The Wayland ones are for `SOLVECRAFT_WAYLAND=1`, and both are needed under + # `xvfb-run` on a machine with no display at all. + runtimeLibs = + pkgs: + (with pkgs; [ + wayland + libxkbcommon + libglvnd + dbus + ]) + ++ (with pkgs; [ + # xorg.libX11 and friends are deprecated aliases in current nixpkgs. + libx11 + libxcursor + libxi + libxrandr + libxrender + libxext + libxcb + ]); + driverLibDir = "/run/opengl-driver/lib"; + libraryPath = pkgs: "${pkgs.lib.makeLibraryPath (runtimeLibs pkgs)}:${driverLibDir}"; + + shellHook = '' + if [ ! -d ${driverLibDir} ]; then + echo "solvecraft: ${driverLibDir} is missing — set hardware.graphics.enable = true so the Mesa drivers and their EGL vendor descriptions exist." >&2 + fi + command -v cc >/dev/null || echo "solvecraft: no C linker on PATH, cargo cannot link — use nix develop .#rust" >&2 + echo "solvecraft: cargo run --release -p solvecraft -- --sample" + ''; + in + { + devShells = forAllSystems (pkgs: { + # The shell to work in. It brings no toolchain of its own, Rust or C: the ones already on + # PATH (a system package, rustup, nixpkgs) keep their build cache and their linker, where a + # second rustc — or mkShell's second gcc, with another glibc behind it — would make every + # `cargo build` produce binaries from somewhere else. + default = pkgs.mkShellNoCC { + name = "solvecraft"; + LD_LIBRARY_PATH = libraryPath pkgs; + inherit shellHook; + }; + + # For a machine with no Rust at all: `nix develop .#rust`. Its own target directory, so + # switching between this toolchain and the system one does not thrash the cache. + rust = pkgs.mkShell { + name = "solvecraft-rust"; + packages = [ + pkgs.cargo + pkgs.rustc + pkgs.rustfmt + pkgs.clippy + ]; + LD_LIBRARY_PATH = libraryPath pkgs; + # Under target/, which .gitignore already covers. + CARGO_TARGET_DIR = "target/nix"; + shellHook = '' + ${shellHook} + echo "solvecraft: $(rustc --version), building into target/nix" + ''; + }; + }); + + packages = forAllSystems (pkgs: { + default = pkgs.rustPlatform.buildRustPackage { + pname = "solvecraft"; + version = (pkgs.lib.importTOML ./Cargo.toml).workspace.package.version; + src = self; + + cargoLock.lockFile = ./Cargo.lock; + # Build and install the app alone: the workspace also holds the CLI, the web app, xtask + # and the crates only the test suites need. + cargoBuildFlags = [ + "-p" + "solvecraft" + ]; + cargoInstallFlags = [ + "-p" + "solvecraft" + ]; + # `cargo xtask ci` is the test gate (it wants a wasm target and a GPU); the sandbox has + # neither. + doCheck = false; + + nativeBuildInputs = [ pkgs.makeWrapper ]; + buildInputs = runtimeLibs pkgs; + + # The app reaches the window through dlopen'd libraries, so the packaged binary needs the + # same LD_LIBRARY_PATH the shell exports. + postFixup = '' + wrapProgram $out/bin/solvecraft --prefix LD_LIBRARY_PATH : "${libraryPath pkgs}" + ''; + + meta = { + description = "Rust-native parametric 3D CAD app"; + homepage = "https://github.com/bherbruck/solvecraft"; + license = with pkgs.lib.licenses; [ + mit + asl20 + ]; + mainProgram = "solvecraft"; + }; + }; + }); + }; +} From 5073f5dd722620107efa72b8a8857e746ed8032f Mon Sep 17 00:00:00 2001 From: CypherNaugh_0x <9931495+CypherNaught-0x@users.noreply.github.com> Date: Sun, 11 Oct 2026 05:25:30 +0200 Subject: [PATCH 05/10] A failed boolean no longer blames "coincident faces on curved bodies" whatever the cause: the exact polygon fallback runs after any failed B-rep boolean (faces that touch, or cross at a shallow angle) and takes planar faces only, but its refusal named coincident faces and curved bodies regardless. It now says which operand is in the way and why: "the tool has 2 curved faces (cylinder)" (kinds: cylinder, cone, sphere, torus, freeform), a face that could not be meshed, a planar face whose mesh is not flat, or a mesh too detailed. Test: planar_fallback_names_the_curved_faces --- crates/engine/src/fuzz.rs | 2 +- crates/kernel/src/polybool.rs | 92 +++++++++++++++++++++++++++++------ crates/kernel/src/tests.rs | 32 ++++++++++++ 3 files changed, 111 insertions(+), 15 deletions(-) diff --git a/crates/engine/src/fuzz.rs b/crates/engine/src/fuzz.rs index a1b20db7..6bbd870c 100644 --- a/crates/engine/src/fuzz.rs +++ b/crates/engine/src/fuzz.rs @@ -82,7 +82,7 @@ fn classify(e: &EngineError) -> Outcome { match e { EngineError::Internal(..) => Outcome::new("panic", msg), EngineError::BadParams { .. } | EngineError::Disabled(..) | EngineError::UnknownCommand(_) => Outcome::new("rejected", msg), - // A failed boolean is a failure even when it says "not supported" (coincident faces on + // A failed boolean is a failure even when it says "not supported" (the planar fallback on // curved bodies): those are the cases to fix. Features the kernel doesn't do yet (a // fillet between curved faces) are limitations. // Picks the generator got wrong (a centroid off its face). diff --git a/crates/kernel/src/polybool.rs b/crates/kernel/src/polybool.rs index 260a35ae..f793a6ce 100644 --- a/crates/kernel/src/polybool.rs +++ b/crates/kernel/src/polybool.rs @@ -199,16 +199,72 @@ impl Node { } } -/// Polygons of a body whose faces are all planar (None otherwise). -fn planar_polys(b: &Body) -> Option> { +/// Why a body's polygons can't be had: in words that finish "the {role} has …" or "the +/// {role}'s …". +#[derive(Debug, PartialEq)] +enum NotPlanar { + /// Faces on curved surfaces, counted by kind. + Curved(std::collections::BTreeMap<&'static str, usize>), + /// Something else (a face that could not be meshed, too many polygons). + Other(String), +} + +impl NotPlanar { + fn describe(&self, role: &str) -> String { + match self { + NotPlanar::Curved(kinds) => { + let n: usize = kinds.values().sum(); + let list: Vec<&str> = kinds.keys().copied().collect(); + format!("the {role} has {n} curved face{} ({})", if n == 1 { "" } else { "s" }, list.join(", ")) + } + NotPlanar::Other(m) => format!("the {role}'s {m}"), + } + } +} + +/// The plane of each face (from its mesh, which must be flat), or what stands in the way: the +/// curved faces by the kind of surface they lie on. +fn face_planes(b: &Body, faces: &[crate::FaceInfo]) -> std::result::Result, NotPlanar> { + use crate::FaceSurface; + let mut curved: std::collections::BTreeMap<&'static str, usize> = std::collections::BTreeMap::new(); + let mut out = Vec::with_capacity(faces.len()); + for f in faces { + if f.area <= 0.0 { + return Err(NotPlanar::Other(format!("face {} could not be meshed", f.index))); + } + match f.plane_normal { + Some(n) => out.push(PlaneEq { n, w: n.dot(f.centroid) }), + None => { + let kind = match crate::face_surface(b, f.index) { + Some(FaceSurface::Cylinder { .. }) => "cylinder", + Some(FaceSurface::Cone { .. }) => "cone", + Some(FaceSurface::Sphere { .. }) => "sphere", + Some(FaceSurface::Torus { .. }) => "torus", + Some(FaceSurface::Plane { .. }) => return Err(NotPlanar::Other(format!("face {} is planar but its mesh is not flat", f.index))), + None => "freeform", + }; + *curved.entry(kind).or_insert(0) += 1; + } + } + } + if curved.is_empty() { Ok(out) } else { Err(NotPlanar::Curved(curved)) } +} + +/// Polygons of a body whose faces are all planar. +fn planar_polys(b: &Body) -> std::result::Result, NotPlanar> { let tol = (b.size() * 1e-3).max(1e-3); - let faces = b.faces(tol).ok()?; - let mesh = b.tessellate(tol).ok()?; - let planes: Vec = faces.iter().map(|f| f.plane_normal.map(|n| PlaneEq { n, w: n.dot(f.centroid) })).collect::>()?; + let unmeshed = |e: KernelError| NotPlanar::Other(format!("faces could not be meshed ({e})")); + let faces = b.faces(tol).map_err(unmeshed)?; + let mesh = b.tessellate(tol).map_err(unmeshed)?; + let planes = face_planes(b, &faces)?; + if mesh.triangles.len() > MAX_POLYS { + return Err(NotPlanar::Other(format!("mesh is too detailed ({} triangles)", mesh.triangles.len()))); + } + let bad_mesh = || NotPlanar::Other("mesh is inconsistent".into()); let mut out = Vec::new(); for (t, fi) in mesh.triangles.iter().zip(&mesh.tri_face) { - let [a, bb, c] = mesh.tri(t)?; - let plane = *planes.get(*fi as usize)?; + let [a, bb, c] = mesh.tri(t).ok_or_else(bad_mesh)?; + let plane = *planes.get(*fi as usize).ok_or_else(bad_mesh)?; // Snap the vertices onto the exact face plane. let snap = |p: Vec3| p - plane.n * (plane.n.dot(p) - plane.w); let mut v = vec![snap(a), snap(bb), snap(c)]; @@ -217,7 +273,7 @@ fn planar_polys(b: &Body) -> Option> { } out.push(Poly { v, plane }); } - (out.len() <= MAX_POLYS).then_some(out) + Ok(out) } /// Polygons of any body: planar faces exactly, curved ones as their triangles (within `tol`). @@ -305,11 +361,19 @@ pub fn faceted_boolean(a: &Body, b: &Mesh, op: BoolOp, tol: f64) -> Result Result> { a.require_brep("a boolean")?; b.require_brep("a boolean")?; - let (Some(pa), Some(pb)) = (planar_polys(a), planar_polys(b)) else { - if let Some(r) = sliced_planar_boolean(a, b, op) { - return r; + let (pa, pb) = match (planar_polys(a), planar_polys(b)) { + (Ok(pa), Ok(pb)) => (pa, pb), + (ra, rb) => { + if let Some(r) = sliced_planar_boolean(a, b, op) { + return r; + } + let why: Vec = + [(ra.err(), "target"), (rb.err(), "tool")].into_iter().filter_map(|(e, role)| e.map(|e| e.describe(role))).collect(); + return Err(KernelError::Failed(format!( + "not supported yet: the exact fallback for where the bodies meet (faces that touch, or cross at a shallow angle) takes planar faces only, and {}", + why.join(" and ") + ))); } - return Err(KernelError::Failed("not supported yet: coincident faces on curved bodies".into())); }; let size = a.size().max(b.size()); polys_boolean(pa, pb, size, op, size * 1e-9) @@ -320,7 +384,7 @@ pub fn planar_boolean(a: &Body, b: &Body, op: BoolOp) -> Result> { /// hole): `a` is split across the slab, the middle piece takes the exact boolean, and the /// pieces outside are joined back on. `None` when no such slab exists. fn sliced_planar_boolean(a: &Body, b: &Body, op: BoolOp) -> Option>> { - let pb = planar_polys(b)?; + let pb = planar_polys(b).ok()?; let size = a.size().max(b.size()); let tol = (a.size() * 1e-3).max(1e-3); let infos = a.faces(tol).ok()?; @@ -391,7 +455,7 @@ fn sliced_planar_boolean(a: &Body, b: &Body, op: BoolOp) -> Option Date: Sun, 11 Oct 2026 05:09:53 +0200 Subject: [PATCH 06/10] A loft gives the same body whatever the order of its sections: the ruled side faces were parameterised from the first section to the last, and a loft cut that touches the body along an edge (a fall lofted from a shelled basin's whole outline down to a drain pad) worked for one order and failed for the other, with an Option::unwrap() panic inside the kernel's intersection or "shell not oriented and closed" from the planar fallback. The loft now builds its sections in one canonical order (first-to-last step with a positive largest component), so either order gives the same, working body. Tests: the basin fall cut, joined and as a new body in both orders and with the pad in every corner (engine), and the kernel loft's volume and cut in both orders, including large-below and planes facing down --- crates/engine/src/cmd/features_more_tests.rs | 48 ++++++++++++++++++++ crates/kernel/src/build.rs | 22 +++++++-- crates/kernel/src/tests.rs | 43 ++++++++++++++++++ 3 files changed, 108 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/cmd/features_more_tests.rs b/crates/engine/src/cmd/features_more_tests.rs index 078464e1..8db8d43e 100644 --- a/crates/engine/src/cmd/features_more_tests.rs +++ b/crates/engine/src/cmd/features_more_tests.rs @@ -137,3 +137,51 @@ fn coil_and_loft_to_a_point() { assert!(rel(volume(&mut s), 400.0 * 30.0 / 3.0) < 1e-6); assert!(s.execute("solid.loft", &json!({"sections": [{"sketch": "Base"}, {"sketch": "Top", "point": "nope"}]})).is_err()); } + +/// A 3 mm shelled basin (100 x 60 x 30, open top) with a fall lofted from its whole outline +/// 10 up down to a drain pad (`pad`: corners, 1 up), in either section order. The loft's top +/// edges lie on the outer walls; the cut used to fail for one order of the sections (an +/// `Option::unwrap()` panic inside the kernel, or "shell not oriented and closed"). +fn basin_fall(pad: [f64; 4], reverse: bool, op: &str) -> (Session, std::result::Result) { + let mut s = Session::default(); + run(&mut s, "solid.box", json!({"length": 100, "width": 60, "height": 30, "corner": [0, 0, 0]})); + run(&mut s, "solid.shell", json!({"faces": [[50, 30, 30]], "thickness": 3})); + run(&mut s, "sketch.create", json!({"plane": {"origin": [0, 0, 10], "x_dir": [1, 0, 0], "y_dir": [0, 1, 0]}, "name": "Rim"})); + run(&mut s, "sketch.rectangle.two_point", json!({"p0": [0, 0], "p1": [100, 60]})); + run(&mut s, "sketch.finish", json!({})); + run(&mut s, "sketch.create", json!({"plane": {"origin": [0, 0, 1], "x_dir": [1, 0, 0], "y_dir": [0, 1, 0]}, "name": "Pad"})); + run(&mut s, "sketch.rectangle.two_point", json!({"p0": [pad[0], pad[1]], "p1": [pad[2], pad[3]]})); + run(&mut s, "sketch.finish", json!({})); + let mut secs = vec![json!({"sketch": "Rim"}), json!({"sketch": "Pad"})]; + if reverse { + secs.reverse(); + } + let r = s.execute("solid.loft", &json!({"sections": secs, "operation": op})).map_err(|e| e.to_string()); + (s, r) +} + +#[test] +fn loft_cut_through_a_shelled_basin_in_either_order() { + // What is left after the cut: the basin less the loft's part of the floor and walls + // (integrated over the height), the same for a pad in any corner. + let want = 41582.028; + for pad in [[80.0, 40.0, 90.0, 50.0], [10.0, 40.0, 20.0, 50.0], [10.0, 10.0, 20.0, 20.0], [80.0, 10.0, 90.0, 20.0]] { + for reverse in [false, true] { + let (mut s, r) = basin_fall(pad, reverse, "cut"); + assert!(r.is_ok(), "{pad:?} reverse {reverse}: {r:?}"); + let v = volume(&mut s); + assert!((v - want).abs() < 1.0, "{pad:?} reverse {reverse}: {v} vs {want}"); + assert_eq!(measure(&mut s)["body_count"], 1); + } + } + // As a new body and joined: a valid solid of positive volume either way. + for op in ["new", "join"] { + let mut vols = Vec::new(); + for reverse in [false, true] { + let (mut s, r) = basin_fall([80.0, 40.0, 90.0, 50.0], reverse, op); + assert!(r.is_ok(), "{op} reverse {reverse}: {r:?}"); + vols.push(volume(&mut s)); + } + assert!(vols[0] > 42948.0 && rel(vols[0], vols[1]) < 1e-6, "{op}: {vols:?}"); + } +} diff --git a/crates/kernel/src/build.rs b/crates/kernel/src/build.rs index f6fc3ba6..e181d0b9 100644 --- a/crates/kernel/src/build.rs +++ b/crates/kernel/src/build.rs @@ -649,7 +649,8 @@ fn resample(lp: &Loop2, n: usize, angles: &[f64]) -> Loop2 { Loop2 { segs } } -/// Ruled loft through two or more planar sections (outer loops only). +/// Ruled loft through two or more planar sections (outer loops only). The sections and the +/// same sections in reverse order give the same body. pub fn loft(sections: &[(Plane, Loop2)]) -> Result { if sections.len() < 2 || sections.len() > 50 { return Err(KernelError::Invalid("a loft needs 2…50 sections".into())); @@ -658,10 +659,21 @@ pub fn loft(sections: &[(Plane, Loop2)]) -> Result { if n == 0 || n > MAX_SEGS { return Err(KernelError::Invalid("empty section".into())); } - // Orient every section counter-clockwise about the first section's normal, then match - // segment counts and starting points to the section with the most segments. - let n0 = sections[0].0.normal(); - let oriented: Vec<(Plane, Loop2)> = sections + // A canonical order, so that reversing the sections changes nothing: the one whose + // first-to-last step has a positive largest component. The side faces' parameterisation + // follows the order, and the boolean worked on one and failed on the other (a cut whose + // loft touches the body along an edge: an internal panic, or "shell not oriented and + // closed"). + let centre = |(p, l): &(Plane, Loop2)| p.to_world(l.centroid()); + let (Some(first), Some(last)) = (sections.first(), sections.last()) else { return Err(KernelError::Invalid("no sections".into())) }; + let step = centre(last) - centre(first); + let big = [step.x, step.y, step.z].into_iter().max_by(|a, b| a.abs().total_cmp(&b.abs())).unwrap_or(0.0); + let ordered: Vec<&(Plane, Loop2)> = if big < 0.0 { sections.iter().rev().collect() } else { sections.iter().collect() }; + // Orient every section counter-clockwise about the first section's normal (`Body::new` + // turns the solid outward), then match segment counts and starting points to the section + // with the most segments. + let n0 = ordered.first().map(|(p, _)| p.normal()).unwrap_or(Vec3::Z); + let oriented: Vec<(Plane, Loop2)> = ordered .iter() .map(|(p, l)| { let l = if p.normal().dot(n0) >= 0.0 { l.ccw() } else { l.ccw().reversed() }; diff --git a/crates/kernel/src/tests.rs b/crates/kernel/src/tests.rs index b65c41a5..61b3dfa6 100644 --- a/crates/kernel/src/tests.rs +++ b/crates/kernel/src/tests.rs @@ -1464,6 +1464,49 @@ fn smooth_loft_through_three_sections() { assert!(v > PI * 64.0 * 30.0 && v < 576.0 * 30.0, "{v}"); } +/// A loft from the basin's whole outline (10 up) down to a drain pad in a corner (1 up), cut +/// from a 3 mm shell: its top edges lie on the outer walls. One order of the sections failed +/// the cut ("shell not oriented and closed", or a panic inside the intersection), the other +/// worked; both orders now build the same body, valid and outward. +#[test] +fn loft_section_order_gives_the_same_body() { + let r = |x0: f64, y0: f64, x1: f64, y1: f64| Loop2::polygon(&[Vec2::new(x0, y0), Vec2::new(x1, y0), Vec2::new(x1, y1), Vec2::new(x0, y1)]); + let at = |z: f64| Plane::new(Vec3::new(0.0, 0.0, z), Vec3::X, Vec3::Y).unwrap(); + let big = (at(10.0), r(0.0, 0.0, 100.0, 60.0)); + let small = (at(1.0), r(80.0, 40.0, 90.0, 50.0)); + let basin = shell(&box_solid(Vec3::ZERO, Vec3::new(100.0, 60.0, 30.0)).unwrap(), &[Vec3::new(50.0, 30.0, 30.0)], 3.0).unwrap(); + // The material inside the loft: the floor from 1 to 3 up, the walls from 3 to 10. + let v0 = measure(&basin).unwrap().volume; + let removed = v0 - 41582.028; + let mut volumes = Vec::new(); + for secs in [vec![big.clone(), small.clone()], vec![small.clone(), big.clone()]] { + let l = loft(&secs).unwrap(); + let m = measure(&l).unwrap(); + assert!(m.volume > 0.0, "{}", m.volume); + volumes.push(m.volume); + let cut = boolean(&basin, &l, BoolOp::Cut).unwrap().unwrap(); + let v = measure(&cut).unwrap().volume; + assert!((v0 - v - removed).abs() < 0.01 * removed, "{v} vs {}", v0 - removed); + let joined = boolean(&basin, &l, BoolOp::Union).unwrap().unwrap(); + assert!(measure(&joined).unwrap().volume > v0); + } + assert!(rel(volumes[0], volumes[1]) < 1e-12, "{volumes:?}"); + // Reversed in z too (large below, small above, and sketched on planes facing down). + let down = |z: f64| Plane::new(Vec3::new(0.0, 0.0, z), Vec3::Y, Vec3::X).unwrap(); + for secs in [ + vec![(at(0.0), r(0.0, 0.0, 100.0, 60.0)), (at(20.0), r(40.0, 20.0, 60.0, 40.0))], + vec![(down(20.0), r(20.0, 40.0, 40.0, 60.0)), (at(0.0), r(0.0, 0.0, 100.0, 60.0))], + ] { + // Prismoid: h/6 · (A0 + 4 Amid + A1). + let want = 20.0 / 6.0 * (6000.0 + 4.0 * 60.0 * 40.0 + 400.0); + let back: Vec<(Plane, Loop2)> = secs.iter().rev().cloned().collect(); + for b in [loft(&secs).unwrap(), loft(&back).unwrap()] { + let v = measure(&b).unwrap().volume; + assert!(rel(v, want) < 1e-6, "{v} vs {want}"); + } + } +} + #[test] fn tangent_holes_pinch_alike() { // Holes at ±15 and ±25 touch along a line (x = ±20); ±30 overlaps ±25. The ±25 wall is cut From c67ad2212956905e8d14f6ec9cf194fde34ac176 Mon Sep 17 00:00:00 2001 From: CypherNaugh_0x <9931495+CypherNaught-0x@users.noreply.github.com> Date: Sun, 11 Oct 2026 05:01:40 +0200 Subject: [PATCH 07/10] A sketch fillet keeps the dimensions of the corner it rounds: filleting the corner of a spout (a vertical riser and a horizontal top, the top's length dimensioned to the corner and the corner's height from the origin) dropped the height dimension with the corner point and left the length on the shortened straight part, so the top came out a fillet radius too long. Like Fusion, a corner that dimensions or constraints measure to now stays as a virtual sharp corner, a free point held on both curves' extensions (two point-on-curve constraints), with those dimensions kept and a line length to the corner turned into a distance from the line's far end to it; chamfers do the same. sketch.fillet and the chamfers report virtual_corners and moved_to_virtual_corner, and the viewport draws a virtual corner as an open square with dashed extension lines from the curves. Tests: the spout keeps its 80 x 100 size and both dimensions still drive it, the virtual corner survives save/load and undo/redo, a fillet with nothing on its corner leaves no point behind, and nearly straight or folded corners with dimensions never panic --- crates/engine/src/cmd/sketch_corner_tests.rs | 87 ++++++++++++ crates/engine/src/cmd/sketch_modify.rs | 132 +++++++++++++++---- crates/engine/src/view.rs | 7 + crates/sketch/src/model.rs | 46 +++++++ crates/ui-egui/src/viewport.rs | 4 + 5 files changed, 250 insertions(+), 26 deletions(-) diff --git a/crates/engine/src/cmd/sketch_corner_tests.rs b/crates/engine/src/cmd/sketch_corner_tests.rs index 344e8fea..b34ea4c3 100644 --- a/crates/engine/src/cmd/sketch_corner_tests.rs +++ b/crates/engine/src/cmd/sketch_corner_tests.rs @@ -96,3 +96,90 @@ fn fillet_and_chamfer_corners() { assert!(solved(&s)); assert!((area(&s) - (a0 - 8.0)).abs() < 1e-6, "{}", area(&s)); } + +/// The ids of the constraints driven by the model parameter `name`. +fn driven_by(s: &Session, name: &str) -> Vec { + sk(s).constraints.iter().filter(|c| c.param.as_deref() == Some(name)).map(|c| c.id.clone()).collect() +} + +/// The spout of a basin: a vertical riser and a horizontal top, the top's length dimensioned +/// to the corner and the corner's height dimensioned from the origin. Filleting the corner used +/// to drop the height and leave the length on the shortened straight part, so the top came out +/// a fillet radius too long. Like Fusion, the corner now stays as a virtual sharp corner (a point +/// on both lines' extensions) and the dimensions measure to it. +#[test] +fn a_fillet_keeps_the_dimensions_of_its_corner_on_a_virtual_sharp() { + let mut s = new_sketch(); + run(&mut s, "sketch.line", json!({"points": [[0, 0], [0, 100], [-80, 100]], "ids": ["riser", "top"]})); + run(&mut s, "sketch.constraint.horizontal_vertical", json!({"line": "riser", "mode": "vertical"})); + run(&mut s, "sketch.constraint.horizontal_vertical", json!({"line": "top", "mode": "horizontal"})); + let height = run(&mut s, "sketch.dimension", json!({"entities": ["origin", "riser.end"], "type": "vertical", "value": 100})); + let height = height["param"].as_str().unwrap().to_string(); + let len = run(&mut s, "sketch.dimension", json!({"entities": ["top"], "value": 80})); + let len = len["param"].as_str().unwrap().to_string(); + let (height_ids, len_ids) = (driven_by(&s, &height), driven_by(&s, &len)); + let f = run(&mut s, "sketch.fillet", json!({"point": "riser.end", "radius": 20})); + assert!(solved(&s), "{f}"); + // Both dimensions are kept (same ids) and reported as moved to the virtual corner. + assert_eq!(driven_by(&s, &height), height_ids); + assert_eq!(driven_by(&s, &len), len_ids); + let corner = f["virtual_corners"][0].as_str().unwrap_or_default().to_string(); + assert!(!corner.is_empty(), "{f}"); + let moved: Vec = f["moved_to_virtual_corner"].as_array().unwrap().iter().filter_map(|v| v.as_str().map(str::to_string)).collect(); + for id in height_ids.iter().chain(&len_ids) { + assert!(moved.contains(id), "{id}: {f}"); + } + // The virtual corner sits where the corner was, and the part keeps its size. + let k = sk(&s); + let vc = k.point(k.point_index(&corner).unwrap()).unwrap(); + assert!(vc.dist(solvecraft_geom::Vec2::new(0.0, 100.0)) < 1e-6, "{vc:?}"); + let top_end = k.point(k.resolve_point("top.end").unwrap()).unwrap(); + assert!(top_end.dist(solvecraft_geom::Vec2::new(-80.0, 100.0)) < 1e-6, "{top_end:?}"); + let (lo, hi) = k.bounds().unwrap(); + assert!((hi.x - lo.x - 80.0).abs() < 1e-6 && (hi.y - lo.y - 100.0).abs() < 1e-6, "{lo:?} {hi:?}"); + // It is drawn with extension lines from both lines' ends to it. + let vcs = k.virtual_corners(); + assert_eq!(vcs.len(), 1, "{vcs:?}"); + assert_eq!(vcs[0].1.len(), 2, "{vcs:?}"); + assert!(vcs[0].1.iter().all(|(a, b)| a.dist(vc) < 1e-9 && (b.dist(vc) - 20.0).abs() < 1e-6), "{vcs:?}"); + // The dimensions still drive the part, measured to the virtual corner. + run(&mut s, "parameters.change", json!({"name": len, "expression": "90 mm"})); + run(&mut s, "parameters.change", json!({"name": height, "expression": "120 mm"})); + assert!(solved(&s)); + let (lo, hi) = sk(&s).bounds().unwrap(); + assert!((hi.x - lo.x - 90.0).abs() < 1e-6 && (hi.y - lo.y - 120.0).abs() < 1e-6, "{lo:?} {hi:?}"); + // It is kept in the design file, and undo and redo take it away and bring it back. + let id = s.active_sketch.unwrap(); + let doc = solvecraft_doc::Document::from_json(&s.doc.to_json()).unwrap(); + assert_eq!(doc.sketch(id).unwrap(), s.doc.sketch(id).unwrap()); + run(&mut s, "edit.undo", json!({})); + run(&mut s, "edit.undo", json!({})); + run(&mut s, "edit.undo", json!({})); + assert!(sk(&s).point_index(&corner).is_none()); + run(&mut s, "edit.redo", json!({})); + assert!(sk(&s).point_index(&corner).is_some()); + // A fillet without anything measuring to its corner leaves no virtual corner behind. + let mut s = new_sketch(); + run(&mut s, "sketch.line", json!({"points": [[10, 10], [10, 100], [-80, 100]], "ids": ["riser", "top"]})); + let n = sk(&s).points.len(); + let f = run(&mut s, "sketch.fillet", json!({"point": "riser.end", "radius": 20})); + assert_eq!(f["virtual_corners"], json!([]), "{f}"); + // Corner gone, two tangent points and the arc's centre added. + assert_eq!(sk(&s).points.len(), n + 2); +} + +/// Lines meeting almost straight or almost folded back: a fillet or chamfer with dimensions on +/// the corner either works or refuses with an error, never a panic. +#[test] +fn a_fillet_of_a_nearly_straight_or_folded_corner_with_dimensions_does_not_panic() { + for (x, y) in [(100.0, 0.05), (100.0, -0.05), (100.0, 1e-9), (-100.0, 0.05), (-100.0, 1e-9)] { + let mut s = new_sketch(); + run(&mut s, "sketch.line", json!({"points": [[0, 0], [100, 0], [100.0 + x, y]], "ids": ["a", "b"]})); + run(&mut s, "sketch.dimension", json!({"entities": ["a"], "value": 100})); + for r in [0.01, 1.0, 50.0] { + let _ = s.execute("sketch.fillet", &json!({"point": "a.end", "radius": r})); + let _ = s.execute("sketch.chamfer.equal_distance", &json!({"point": "a.end", "distance": r})); + } + assert!(sk(&s).points.iter().all(|p| p.pos.is_finite())); + } +} diff --git a/crates/engine/src/cmd/sketch_modify.rs b/crates/engine/src/cmd/sketch_modify.rs index f79c3697..fe57d149 100644 --- a/crates/engine/src/cmd/sketch_modify.rs +++ b/crates/engine/src/cmd/sketch_modify.rs @@ -17,7 +17,7 @@ pub static COMMANDS: &[CommandSpec] = &[ .at("SKETCH", "MODIFY") .icon("sketch_fillet") .enabled(in_sketch) - .params("point: corner shared by two curves (\"l1.end\") | points: [corners] (all rounded alike: one radius, the rest equal) | a, b: two lines or arcs meeting at a corner; radius: expr"), + .params("point: corner shared by two curves (\"l1.end\") | points: [corners] (all rounded alike: one radius, the rest equal) | a, b: two lines or arcs meeting at a corner; radius: expr. A corner that dimensions or constraints measure to stays as a virtual sharp corner on both curves' extensions (`virtual_corners`; the moved ones in `moved_to_virtual_corner`)"), CommandSpec::new("sketch.chamfer.equal_distance", "Equal Distance Chamfer", chamfer_equal) .at("SKETCH", "MODIFY") .icon("sketch_chamfer") @@ -456,8 +456,71 @@ fn reattach(sk: &mut Sketch, l: usize, corner: usize, np: usize) { } } +/// What became of a rounded or cut corner's point: kept as a virtual sharp corner (its id), +/// and the constraints and dimensions that now measure to it. +#[derive(Default)] +struct CornerKept { + virtual_corner: Option, + moved: Vec, +} + +/// Keep corner point `c` of curves `ca`, `cb` (whose corner ends were just moved to `na`, `nb`) +/// when anything measures to it: a constraint or dimension on the point, the length of either +/// line, or it is the origin or still ends another curve. It stays as a virtual sharp corner, +/// held on both curves' extensions (their intersection), and the line lengths become distances +/// from the far end to it, so the part keeps its size, as in Fusion. Otherwise None: the +/// caller removes the point. +fn keep_corner(sk: &mut Sketch, c: usize, [(ca, na), (cb, nb)]: [(usize, usize); 2]) -> Result> { + let used = sk.curves.iter().any(|x| uses(x, c)); + let mut moved: Vec = sk.constraints.iter().filter(|k| k.kind.refers_to_point(c)).map(|k| k.id.clone()).collect(); + for k in sk.constraints.iter_mut() { + let ConstraintKind::Length { l, value } = k.kind else { continue }; + let new_end = if l == ca { + na + } else if l == cb { + nb + } else { + continue; + }; + let Some(CurveKind::Line { a, b }) = sk.curves.get(l).map(|x| &x.kind) else { continue }; + // Same direction as the line, so the dimension keeps its side and text place. + k.kind = if *a == new_end { ConstraintKind::Distance { p: c, q: *b, value } } else { ConstraintKind::Distance { p: *a, q: c, value } }; + moved.push(k.id.clone()); + } + if moved.is_empty() && !used && c != 0 { + return Ok(None); + } + for cu in [ca, cb] { + let k = ConstraintKind::PointOnCurve { p: c, c: cu }; + if !sk.constraints.iter().any(|x| x.kind == k) { + add_c(sk, k)?; + } + } + // A point of its own now: `l1.end` names the line's new end, not the virtual corner. + let virtual_corner = if used || c == 0 { + None + } else { + let id = sk.fresh("corner"); + if let Some(pt) = sk.points.get_mut(c) { + pt.id = id.clone(); + } + Some(id) + }; + Ok(Some(CornerKept { virtual_corner, moved })) +} + /// Points at distances `da` along a and `db` along b from the corner; lines shortened to them. -fn cut_corner(sk: &mut Sketch, la: usize, lb: usize, c: usize, fa: usize, fb: usize, da: f64, db: f64, cmd: &str) -> Result<(usize, usize, Vec2)> { +fn cut_corner( + sk: &mut Sketch, + la: usize, + lb: usize, + c: usize, + fa: usize, + fb: usize, + da: f64, + db: f64, + cmd: &str, +) -> Result<(usize, usize, Vec2, CornerKept)> { let (pc, pa, pb) = (sk.point(c).unwrap_or_default(), sk.point(fa).unwrap_or_default(), sk.point(fb).unwrap_or_default()); if !(da > 1e-9 && db > 1e-9 && da < pc.dist(pa) - 1e-9 && db < pc.dist(pb) - 1e-9) { return Err(bad(cmd, "too large for these lines")); @@ -468,13 +531,14 @@ fn cut_corner(sk: &mut Sketch, la: usize, lb: usize, c: usize, fa: usize, fb: us let tb = sk.add_point(pc + v * db, None)?; reattach(sk, la, c, ta); reattach(sk, lb, c, tb); - let used = sk.curves.iter().any(|x| uses(x, c)); - if !used && c != 0 { + let kept = keep_corner(sk, c, [(la, ta), (lb, tb)])?; + let removed = kept.is_none(); + if removed { sk.remove_points(&[c]); } - // Indices may have shifted when the corner point was removed. - let fix = |i: usize| if !used && c != 0 && i > c { i - 1 } else { i }; - Ok((fix(ta), fix(tb), pc)) + // Indices shift when the corner point was removed. + let fix = |i: usize| if removed && i > c { i - 1 } else { i }; + Ok((fix(ta), fix(tb), pc, kept.unwrap_or_default())) } fn fillet(s: &mut Session, p: &Value) -> Result { @@ -492,9 +556,10 @@ fn fillet(s: &mut Session, p: &Value) -> Result { return Err(bad(cmd, "radius must be positive")); } let pname = doc.new_model_param(&r_expr, "mm"); - let (mut arcs, mut cons) = (Vec::new(), Vec::new()); + let (mut arcs, mut cons, mut kept) = (Vec::new(), Vec::new(), Vec::new()); for q in &corners { - let (arc, mut c) = fillet_corner(sk, q, r, cmd)?; + let (arc, mut c, k) = fillet_corner(sk, q, r, cmd)?; + kept.push(k); // One radius for them all: the first carries the dimension, the rest are equal to it. match arcs.first() { None => c.push(sk.add_constraint(ConstraintKind::Radius { c: arc, value: r }, Some(pname.clone()))?), @@ -503,14 +568,27 @@ fn fillet(s: &mut Session, p: &Value) -> Result { cons.extend(c); arcs.push(arc); } - Ok((ids_of(sk, &arcs), cons, pname)) + Ok((ids_of(sk, &arcs), cons, pname, kept_json(kept))) })?; - Ok(json!({"curves": out.0, "constraints": out.1, "param": out.2, "sketch": info})) + let (vc, moved) = out.3; + Ok(json!({"curves": out.0, "constraints": out.1, "param": out.2, "virtual_corners": vc, "moved_to_virtual_corner": moved, "sketch": info})) +} + +/// The virtual corners kept and the constraints moved to them, for a response. +fn kept_json(kept: Vec) -> (Vec, Vec) { + let vcs = kept.iter().filter_map(|k| k.virtual_corner.clone()).collect(); + let mut moved: Vec = Vec::new(); + for id in kept.into_iter().flat_map(|k| k.moved) { + if !moved.contains(&id) { + moved.push(id); + } + } + (vcs, moved) } /// Round one corner (`point` or `a`, `b`) with an arc of radius `r` tangent to both curves: -/// the arc and the tangent constraints. -fn fillet_corner(sk: &mut Sketch, p: &Value, r: f64, cmd: &str) -> Result<(usize, Vec)> { +/// the arc, the tangent constraints and what became of the corner point. +fn fillet_corner(sk: &mut Sketch, p: &Value, r: f64, cmd: &str) -> Result<(usize, Vec, CornerKept)> { let (la, lb, c, fa, fb) = match corner(sk, p, cmd) { Ok(x) => x, // Not two lines: a corner with an arc. @@ -529,7 +607,7 @@ fn fillet_corner(sk: &mut Sketch, p: &Value, r: f64, cmd: &str) -> Result<(usize } let bis = (u + v).normalized().ok_or_else(|| bad(cmd, "the lines are parallel"))?; let center = pc + bis * (r / half.sin()); - let (ta, tb, _) = cut_corner(sk, la, lb, c, fa, fb, d, d, cmd)?; + let (ta, tb, _, kept) = cut_corner(sk, la, lb, c, fa, fb, d, d, cmd)?; let (qa, qb) = (sk.point(ta).unwrap_or_default(), sk.point(tb).unwrap_or_default()); let ccw = (qa - center).cross(qb - center) > 0.0; let arc = if ccw { @@ -538,13 +616,13 @@ fn fillet_corner(sk: &mut Sketch, p: &Value, r: f64, cmd: &str) -> Result<(usize sk.add_arc(center, qb, qa, [None, Some(tb), Some(ta)], None)? }; let cons = vec![add_c(sk, ConstraintKind::Tangent { a: la, b: arc })?, add_c(sk, ConstraintKind::Tangent { a: lb, b: arc })?]; - Ok((arc, cons)) + Ok((arc, cons, kept)) } /// Fillet a corner where a line meets an arc, or two arcs meet: the arc of radius `r` touching /// both, nearest the corner; both curves are cut back to it. None when the corner is not of /// that kind. -fn round_corner(sk: &mut Sketch, p: &Value, r: f64, cmd: &str) -> Result)>> { +fn round_corner(sk: &mut Sketch, p: &Value, r: f64, cmd: &str) -> Result, CornerKept)>> { let ends = |c: usize| match sk.curves.get(c).map(|c| &c.kind) { Some(CurveKind::Line { a, b }) | Some(CurveKind::Arc { a, b, .. }) => Some((*a, *b)), _ => None, @@ -606,14 +684,15 @@ fn round_corner(sk: &mut Sketch, p: &Value, r: f64, cmd: &str) -> Result Ok(Some((arc, cons, kept))), + None => { + let arc_id = sk.curves.get(arc).map(|c| c.id.clone()).unwrap_or_default(); + sk.remove_points(&[q]); + let arc = sk.curve_index(&arc_id).unwrap_or(arc); + Ok(Some((arc, cons, CornerKept::default()))) + } } - Ok(Some((arc, cons))) } fn chamfer(s: &mut Session, p: &Value, cmd: &str, dists: impl FnOnce(f64) -> Result<(f64, f64)>) -> Result { @@ -623,11 +702,12 @@ fn chamfer(s: &mut Session, p: &Value, cmd: &str, dists: impl FnOnce(f64) -> Res let (u, v) = ((pa - pc).normalized().unwrap_or(Vec2::X), (pb - pc).normalized().unwrap_or(Vec2::Y)); let corner_angle = u.cross(v).atan2(u.dot(v)).abs(); let (da, db) = dists(corner_angle)?; - let (ta, tb, _) = cut_corner(sk, la, lb, c, fa, fb, da, db, cmd)?; + let (ta, tb, _, kept) = cut_corner(sk, la, lb, c, fa, fb, da, db, cmd)?; let l = sk.add_line_pts(ta, tb, None)?; - Ok(ids_of(sk, &[l])) + Ok((ids_of(sk, &[l]), kept_json(vec![kept]))) })?; - Ok(json!({"curves": out, "sketch": info})) + let (curves, (vc, moved)) = out; + Ok(json!({"curves": curves, "virtual_corners": vc, "moved_to_virtual_corner": moved, "sketch": info})) } fn chamfer_equal(s: &mut Session, p: &Value) -> Result { diff --git a/crates/engine/src/view.rs b/crates/engine/src/view.rs index 52b3a675..2fc7e584 100644 --- a/crates/engine/src/view.rs +++ b/crates/engine/src/view.rs @@ -95,6 +95,13 @@ pub fn sketch_lines(sk: &Sketch, plane: &Plane, active: bool, determined: &[bool out.push((seg.polyline(0.02).iter().map(|p| plane.to_world(*p)).collect(), col, c.construction || c.centerline)); } } + // Virtual sharp corners (kept by fillets and chamfers): thin extension lines from the + // curves to the corner, like construction geometry. + for (_, ext) in sk.virtual_corners() { + for (a, b) in ext { + out.push((vec![plane.to_world(a), plane.to_world(b)], colors::SKETCH_CONSTRUCTION, true)); + } + } // 3D curves are in world coordinates already. for w in sk.wires.iter().filter(|w| !w.fit.is_empty() || !sk.view.hide_projected) { // Curves drawn in a 3D sketch look like the sketch's own; included ones are references. diff --git a/crates/sketch/src/model.rs b/crates/sketch/src/model.rs index 2b5a0079..6deaa72a 100644 --- a/crates/sketch/src/model.rs +++ b/crates/sketch/src/model.rs @@ -334,6 +334,10 @@ impl ConstraintKind { LinearDiameter { .. } => "LinearDiameterDimension", } } + /// Does the constraint refer to point `p` (by index)? + pub fn refers_to_point(&self, p: usize) -> bool { + self.indices().0.contains(&p) + } fn indices(&self) -> (Vec, Vec) { use ConstraintKind::*; match *self { @@ -825,6 +829,48 @@ impl Sketch { out } + /// Virtual sharp corners (what a fillet or chamfer leaves where dimensions measured to the + /// corner): points used by no curve, held on two or more lines or arcs (their extensions). + /// Each with the extension segments from the corner to the nearest end of those curves, for + /// drawing. + pub fn virtual_corners(&self) -> Vec<(usize, Vec<(Vec2, Vec2)>)> { + let mut out = Vec::new(); + for (i, p) in self.points.iter().enumerate().skip(1) { + if self.point_used_by_curve(i) { + continue; + } + let on: Vec = self + .constraints + .iter() + .filter_map(|c| match c.kind { + ConstraintKind::PointOnCurve { p, c } if p == i => Some(c), + _ => None, + }) + .filter(|c| matches!(self.curves.get(*c).map(|c| &c.kind), Some(CurveKind::Line { .. } | CurveKind::Arc { .. }))) + .collect(); + if on.len() < 2 { + continue; + } + let mut ext = Vec::new(); + for c in on { + let Some((a, b)) = self.curves.get(c).and_then(|c| c.kind.ends()) else { continue }; + let (Some(pa), Some(pb)) = (self.point(a), self.point(b)) else { continue }; + let near = if p.pos.dist(pa) <= p.pos.dist(pb) { pa } else { pb }; + // A line only needs one when the corner is beyond its ends. + let inside = matches!(self.curves.get(c).map(|c| &c.kind), Some(CurveKind::Line { .. })) && { + let d = pb - pa; + let t = (p.pos - pa).dot(d) / d.dot(d).max(1e-300); + (0.0..=1.0).contains(&t) + }; + if !inside && p.pos.dist(near) > 1e-9 { + ext.push((p.pos, near)); + } + } + out.push((i, ext)); + } + out + } + /// Is curve `c` held in place: fixed or projected (linked). pub fn curve_locked(&self, c: usize) -> bool { self.curves.get(c).is_some_and(|c| c.fixed || c.link.is_some()) diff --git a/crates/ui-egui/src/viewport.rs b/crates/ui-egui/src/viewport.rs index 300f3521..a6aef20a 100644 --- a/crates/ui-egui/src/viewport.rs +++ b/crates/ui-egui/src/viewport.rs @@ -2069,6 +2069,7 @@ fn overlays(app: &SolveApp, painter: &egui::Painter, proj: &Proj) { let Some(ss) = st.sketch(sid) else { return }; let sk = &ss.sketch; let locked = sk.locked_points(); + let virtual_corners: Vec = sk.virtual_corners().into_iter().map(|(i, _)| i).collect(); for (i, p) in sk.points.iter().enumerate() { if !solvecraft_engine::view::sketch_point_visible(sk, i) { continue; @@ -2085,6 +2086,9 @@ fn overlays(app: &SolveApp, painter: &egui::Painter, proj: &Proj) { }; if i == 0 { painter.circle(sp, 3.5, Color32::from_rgb(240, 200, 60), Stroke::new(1.0, t.body_edge)); + } else if virtual_corners.contains(&i) { + // A virtual sharp corner: an open square, it is not on any drawn curve. + painter.rect_stroke(Rect::from_center_size(sp, vec2(6.0, 6.0)), 0.0, Stroke::new(1.2, c), egui::StrokeKind::Middle); } else { painter.rect_filled(Rect::from_center_size(sp, vec2(5.0, 5.0)), 0.0, c); } From 3573b96c7a01baf8b52947567fb91e9d45da05fb Mon Sep 17 00:00:00 2001 From: CypherNaugh_0x <9931495+CypherNaught-0x@users.noreply.github.com> Date: Sun, 11 Oct 2026 10:01:46 +0200 Subject: [PATCH 08/10] fix: shell bodies where four or more planar faces meet at a corner (a floor falling to a drain from the trough edges) by splitting such corners into several joined by short edges instead of refusing because the moved planes miss a common point --- ROADMAP.md | 3 +- crates/kernel/src/offset.rs | 252 +++++++++++++++++++++++++++++++----- crates/kernel/src/tests.rs | 101 +++++++++++++++ 3 files changed, 325 insertions(+), 31 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 001165a6..f40f3247 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -100,7 +100,8 @@ headline counts in-scope tabs only and lists the deferred ones separately. - Free-form curves cut at a tangent touch (no clean crossing) enter that profile as polylines. Sweeps along 3D sketch curves follow their polyline (smooth walls when the path is smooth). - Shell and offset faces work on bodies of planes, cylinders, cones and spheres (convex or not: - filleted boxes, bosses, domes, countersinks); draft on convex planar bodies and on walls (planes, cylinders → cones) between caps; lofts through three or more sections are smooth (B-spline sides through every section), two + filleted boxes, bosses, domes, countersinks; corners where four or more planes meet, such as a + floor falling to a drain, as long as the corner's edges are all convex or all concave); draft on convex planar bodies and on walls (planes, cylinders → cones) between caps; lofts through three or more sections are smooth (B-spline sides through every section), two sections ruled. ## Kernel diff --git a/crates/kernel/src/offset.rs b/crates/kernel/src/offset.rs index 14988ed8..c0f3afd9 100644 --- a/crates/kernel/src/offset.rs +++ b/crates/kernel/src/offset.rs @@ -2,8 +2,10 @@ //! faces). Each face moves along its outward normal by its own distance: a plane is translated, //! a cylinder changes radius about its axis. Every vertex and edge goes to where its faces' //! moved surfaces meet; edges are lines or circular arcs. This is exact when the moved -//! surfaces meet the way the old ones did (prismatic parts, bosses, fillets between walls); -//! anything else is refused, so callers can fall back. +//! surfaces meet the way the old ones did (prismatic parts, bosses, fillets between walls). +//! A corner where more than three planes meet, whose moved planes miss a common point (a +//! floor falling to a drain, a pyramid's apex), becomes several corners joined by short edges. +//! Anything else is refused, so callers can fall back. use std::collections::HashMap; @@ -422,6 +424,143 @@ fn span_step(ns: &[Vec3], s: &[f64]) -> Option { Some(v) } +/// Most faces round one corner that [`split_corner`] takes on. +const MAX_SPLIT_FACES: usize = 48; + +/// The corners in place of a corner where more than three planar faces meet and whose moved +/// planes miss a common point (a hip floor's corner, a pyramid's apex): each new corner with +/// the faces through it. Corners sharing two faces are joined by a new edge on those faces. +/// Only corners whose edges are all convex or all concave: the faces there bound a convex +/// region (the body's side or the outside's), and after the move its corners are the points on +/// three moved planes inside all the others. +fn split_corner( + v: &mt::Vertex, + at: &[usize], + faces: &[mt::Face], + efaces: &HashMap>, + old: &[Surf], + new: &[Surf], + size: f64, +) -> Option)>> { + if at.len() < 4 || at.len() > MAX_SPLIT_FACES { + return None; + } + let p = from_p3(v.point()); + // (face, outward normal, old offset, moved offset). + let planes: Vec<(usize, Vec3, f64, f64)> = at + .iter() + .map(|i| match (old.get(*i)?, new.get(*i)?) { + (Surf::Plane { n, d }, Surf::Plane { d: d2, .. }) => Some((*i, *n, *d, *d2)), + _ => None, + }) + .collect::>()?; + let normal = |fi: usize| planes.iter().find(|x| x.0 == fi).map(|x| x.1); + // Edge kinds round the corner: a face's loop runs counter-clockwise about its outward + // normal, so its inside is to the left; the edge is convex when the face beyond it falls + // away from there. + let mut sign = 0.0f64; + let mut pairs: Vec<(usize, usize)> = Vec::new(); + for &(fi, na, ..) in &planes { + for w in faces.get(fi)?.boundaries() { + for e in w.edge_iter() { + if e.front().id() != v.id() && e.back().id() != v.id() { + continue; + } + let fb = efaces.get(&e.id())?.iter().copied().find(|j| *j != fi)?; + pairs.push((fi, fb)); + let dir = (from_p3(e.back().point()) - from_p3(e.front().point())).normalized()?; + let s = normal(fb)?.dot(na.cross(dir)); + if s.abs() < 1e-9 { + continue; + } + if sign * s < 0.0 { + return None; + } + sign = s.signum(); + } + } + } + if sign == 0.0 { + return None; + } + // Convex: the body's side, n·x ≤ d; concave: the outside's, n·x ≥ d. + let side = -sign; + let merge = size * 1e-6; + let mut nodes: Vec<(Vec3, Vec)> = Vec::new(); + for (ia, a) in planes.iter().enumerate() { + for (ib, b) in planes.iter().enumerate().skip(ia + 1) { + for c in planes.iter().skip(ib + 1) { + let den = a.1.dot(b.1.cross(c.1)); + if den.abs() < 1e-9 { + continue; + } + let x = (b.1.cross(c.1) * a.3 + c.1.cross(a.1) * b.3 + a.1.cross(b.1) * c.3) * (1.0 / den); + if !x.is_finite() || x.dist(p) > size || planes.iter().any(|q| side * (q.1.dot(x) - q.3) > merge) { + continue; + } + let fs = [a.0, b.0, c.0]; + match nodes.iter_mut().find(|(q, _)| q.dist(x) < merge) { + Some((_, nf)) => { + for f in fs { + if !nf.contains(&f) { + nf.push(f); + } + } + } + None => nodes.push((x, fs.to_vec())), + } + } + } + } + // Every face keeps a corner here, and every edge an end. + if planes.iter().any(|q| !nodes.iter().any(|(_, nf)| nf.contains(&q.0))) + || pairs.iter().any(|(x, y)| !nodes.iter().any(|(_, nf)| nf.contains(x) && nf.contains(y))) + { + return None; + } + // Each corner where its planes meet best (least squares, for planes meeting at one point + // to within the merge distance). + for (q, nf) in nodes.iter_mut() { + let (ns, ss): (Vec, Vec) = nf.iter().filter_map(|f| planes.iter().find(|x| x.0 == *f)).map(|x| (x.1, x.3 - x.2)).unzip(); + *q = p + span_step(&ns, &ss)?; + } + Some(nodes) +} + +/// The corners of a split corner (see [`split_corner`]) on face `face`, in order from corner +/// `from` to corner `to`, along the new edges between them. +fn split_path(nodes: &[(Vec3, Vec)], face: usize, from: usize, to: usize) -> Option> { + let joined = |x: usize, y: usize| { + let (Some((_, a)), Some((_, b))) = (nodes.get(x), nodes.get(y)) else { return false }; + a.contains(&face) && b.contains(&face) && a.iter().filter(|f| b.contains(f)).count() >= 2 + }; + // Breadth first; `prev` leads back to `from`. + let mut prev: Vec> = vec![None; nodes.len()]; + let mut queue = std::collections::VecDeque::from([from]); + let mut seen = vec![false; nodes.len()]; + *seen.get_mut(from)? = true; + while let Some(x) = queue.pop_front() { + if x == to { + let mut path = vec![to]; + let mut cur = to; + while let Some(p) = prev.get(cur).copied().flatten() { + path.push(p); + cur = p; + } + path.reverse(); + return Some(path); + } + for y in 0..nodes.len() { + if !seen.get(y).copied().unwrap_or(true) && joined(x, y) { + *seen.get_mut(y)? = true; + *prev.get_mut(y)? = Some(x); + queue.push_back(y); + } + } + } + None +} + /// A line or circular arc through an edge (by samples): `None` for other curves. /// Maximum straight edges per face checked pairwise for crossings. const MAX_CROSSING_EDGES: usize = 2000; @@ -431,7 +570,7 @@ const MAX_CROSSING_EDGES: usize = 2000; fn crossed_loops( faces: &[mt::Face], old: &[Surf], - newpos: &HashMap, + pos: &dyn Fn(&mt::Vertex, &mt::Edge) -> Option, edge_collapsed: &dyn Fn(&mt::Edge) -> bool, size: f64, ) -> Option { @@ -443,32 +582,32 @@ fn crossed_loops( } let (u, w) = (n.any_perp(), n.cross(n.any_perp())); let flat = |p: Vec3| (p.dot(u), p.dot(w)); - // Straight edges as (vertex ids, old ends, new ends) in the plane. - let mut segs: Vec<([mt::VertexID; 2], [(f64, f64); 2])> = Vec::new(); + // Straight edges as (vertex ids, new ends in the plane, the plane's new offset). + let mut segs: Vec<([mt::VertexID; 2], [(f64, f64); 2], f64)> = Vec::new(); for wire in f.absolute_boundaries() { let (mut area_old, mut area_new, mut straight) = (0.0, 0.0, true); for e in wire.edge_iter() { let (v0, v1) = (e.front(), e.back()); - let (Some(q0), Some(q1)) = (newpos.get(&v0.id()), newpos.get(&v1.id())) else { return None }; - let (o0, o1, n0, n1) = (flat(from_p3(v0.point())), flat(from_p3(v1.point())), flat(*q0), flat(*q1)); + let (Some(q0), Some(q1)) = (pos(v0, e), pos(v1, e)) else { return None }; + let (o0, o1, n0, n1) = (flat(from_p3(v0.point())), flat(from_p3(v1.point())), flat(q0), flat(q1)); area_old += o0.0 * o1.1 - o1.0 * o0.1; area_new += n0.0 * n1.1 - n1.0 * n0.1; if arc_mid(e, size * 1e-6) == Some(None) { - segs.push(([v0.id(), v1.id()], [n0, n1])); + segs.push(([v0.id(), v1.id()], [n0, n1], n.dot(q0))); } else { straight = false; } } if straight && area_old.abs() > eps && area_old * area_new <= 0.0 { - return wire.vertex_iter().next().and_then(|v| newpos.get(&v.id()).copied()); + return wire.edge_iter().next().and_then(|e| pos(e.front(), e)); } } if segs.len() > MAX_CROSSING_EDGES { continue; } let side = |a: (f64, f64), c: (f64, f64), p: (f64, f64)| (c.0 - a.0) * (p.1 - a.1) - (c.1 - a.1) * (p.0 - a.0); - for (i, (ia, [a0, a1])) in segs.iter().enumerate() { - for (ib, [b0, b1]) in segs.iter().skip(i + 1) { + for (i, (ia, [a0, a1], d)) in segs.iter().enumerate() { + for (ib, [b0, b1], _) in segs.iter().skip(i + 1) { if ia.iter().any(|v| ib.contains(v)) { continue; } @@ -478,8 +617,7 @@ fn crossed_loops( if ((s0 > tol && s1 < -tol) || (s0 < -tol && s1 > tol)) && ((t0 > tol && t1 < -tol) || (t0 < -tol && t1 > tol)) { let k = t0 / (t0 - t1); let (x, y) = (a0.0 + (a1.0 - a0.0) * k, a0.1 + (a1.1 - a0.1) * k); - let d = n.dot(newpos.get(&ia[0]).copied().unwrap_or_default()); - return Some(u * x + w * y + n * d); + return Some(u * x + w * y + n * *d); } } } @@ -618,6 +756,8 @@ pub(crate) fn offset_body(b: &Body, shift: impl Fn(usize, Vec3) -> f64) -> Resul }) .collect(); let mut newpos: HashMap = HashMap::new(); + // Corners whose moved faces meet at several points (see `split_corner`). + let mut splits: HashMap)>> = HashMap::new(); for v in b.solid.vertex_iter() { let at = vfaces.get(&v.id()).ok_or_else(|| fail("vertex"))?; let p = from_p3(v.point()); @@ -672,10 +812,33 @@ pub(crate) fn offset_body(b: &Body, shift: impl Fn(usize, Vec3) -> f64) -> Resul }); let q = match apex { Some(q) if n.iter().all(|sf| sf.dist(q).abs() < size * 1e-6) => q, - _ => move_point(p, &o, &s, &n, size * 1e-6).ok_or_else(|| fail("a corner whose faces don't meet the same way after the move"))?, + _ => match move_point(p, &o, &s, &n, size * 1e-6) { + Some(q) => q, + None => { + let nodes = (fs == *at) + .then(|| split_corner(&v, at, &faces, &efaces, &old, &new, size)) + .flatten() + .ok_or_else(|| fail("a corner whose faces don't meet the same way after the move"))?; + let q = nodes.first().map(|x| x.0).ok_or_else(|| fail("corner"))?; + splits.insert(v.id(), nodes); + q + } + }, }; newpos.insert(v.id(), q); } + // Which of a split corner's new corners an edge ends at. + let node_of = |v: mt::VertexID, e: mt::EdgeID| -> Option { + let fs = efaces.get(&e)?; + splits.get(&v)?.iter().position(|(_, nf)| fs.iter().all(|i| nf.contains(i))) + }; + // Where an edge's end goes. + let end_pos = |v: &mt::Vertex, e: &mt::Edge| -> Option { + match splits.get(&v.id()) { + Some(nodes) => nodes.get(node_of(v.id(), e.id())?).map(|x| x.0), + None => newpos.get(&v.id()).copied(), + } + }; // An edge turned round: faces crossed over (a wall thinner than the move), which would // leave the body inside out. let edge_collapsed = |e: &mt::Edge| efaces.get(&e.id()).is_some_and(|fs| fs.iter().any(|i| is_collapsed(*i))); @@ -684,8 +847,8 @@ pub(crate) fn offset_body(b: &Body, shift: impl Fn(usize, Vec3) -> f64) -> Resul continue; } let (f0, f1) = (e.absolute_front(), e.absolute_back()); - let (Some(q0), Some(q1)) = (newpos.get(&f0.id()), newpos.get(&f1.id())) else { continue }; - let (old_v, new_v) = (from_p3(f1.point()) - from_p3(f0.point()), *q1 - *q0); + let (Some(q0), Some(q1)) = (end_pos(f0, &e), end_pos(f1, &e)) else { continue }; + let (old_v, new_v) = (from_p3(f1.point()) - from_p3(f0.point()), q1 - q0); if old_v.len() > size * 1e-6 && old_v.dot(new_v) < 0.0 { return Err(fail("not supported yet: the move is larger than a wall or step is thick (faces would cross)")); } @@ -693,7 +856,7 @@ pub(crate) fn offset_body(b: &Body, shift: impl Fn(usize, Vec3) -> f64) -> Resul // A plane's loops crossing after the move: a rim or ledge narrower than the move (the hole // in a top face sliding past its outer edge) turns part of the face inside out, though no // edge turns round. - if let Some(at) = crossed_loops(&faces, &old, &newpos, &edge_collapsed, size) { + if let Some(at) = crossed_loops(&faces, &old, &end_pos, &edge_collapsed, size) { return Err(fail(&format!( "not supported yet: the move is larger than a rim or ledge near [{:.1}, {:.1}, {:.1}] is wide (a face would turn inside out)", at.x, at.y, at.z @@ -725,6 +888,16 @@ pub(crate) fn offset_body(b: &Body, shift: impl Fn(usize, Vec3) -> f64) -> Resul }; verts.insert(v.id(), nv); } + let node_verts: HashMap> = + splits.iter().map(|(id, nodes)| (*id, nodes.iter().map(|(q, _)| builder::vertex(p3(*q))).collect())).collect(); + let vert_at = |v: &mt::Vertex, e: &mt::Edge| -> Option<&mt::Vertex> { + match node_verts.get(&v.id()) { + Some(nv) => nv.get(node_of(v.id(), e.id())?), + None => verts.get(&v.id()), + } + }; + // New edges between a split corner's corners (stored from the lower index). + let mut links: HashMap<(mt::VertexID, usize, usize), mt::Edge> = HashMap::new(); // Old edge → new edge, and whether it runs the other way (merged with its twin). let mut edges: HashMap = HashMap::new(); let mut merged: HashMap<(mt::VertexID, mt::VertexID), mt::Edge> = HashMap::new(); @@ -732,7 +905,7 @@ pub(crate) fn offset_body(b: &Body, shift: impl Fn(usize, Vec3) -> f64) -> Resul if edges.contains_key(&e.id()) { continue; } - let (Some(a), Some(c)) = (verts.get(&e.absolute_front().id()), verts.get(&e.absolute_back().id())) else { return Err(fail("edge")) }; + let (Some(a), Some(c)) = (vert_at(e.absolute_front(), &e), vert_at(e.absolute_back(), &e)) else { return Err(fail("edge")) }; let touches = edge_collapsed(&e); if a == c || from_p3(a.point()).dist(from_p3(c.point())) < size * 1e-9 { if touches { @@ -796,18 +969,37 @@ pub(crate) fn offset_body(b: &Body, shift: impl Fn(usize, Vec3) -> f64) -> Resul if is_collapsed(i) { continue; } - let wires: Vec = f - .absolute_boundaries() - .iter() - .map(|w| { - w.edge_iter() - .filter_map(|e| { - edges.get(&e.id()).map(|(ne, flip)| if (e.front() == e.absolute_front()) != *flip { ne.clone() } else { ne.inverse() }) - }) - .collect::>() - .into() - }) - .collect(); + let mut wires: Vec = Vec::new(); + for w in f.absolute_boundaries() { + let old_edges: Vec = w.edge_iter().cloned().collect(); + let mut wire: Vec = Vec::new(); + for (j, e) in old_edges.iter().enumerate() { + if let Some((ne, flip)) = edges.get(&e.id()) { + wire.push(if (e.front() == e.absolute_front()) != *flip { ne.clone() } else { ne.inverse() }); + } + // At a split corner, along the new edges from this edge's end to the next's. + let v = e.back(); + let (Some(nodes), Some(next)) = (splits.get(&v.id()), old_edges.get((j + 1) % old_edges.len())) else { continue }; + let (Some(from), Some(to)) = (node_of(v.id(), e.id()), node_of(v.id(), next.id())) else { return Err(fail("split corner")) }; + let path = split_path(nodes, i, from, to).ok_or_else(|| fail("split corner"))?; + for pair in path.windows(2) { + let (Some(&x), Some(&y)) = (pair.first(), pair.get(1)) else { continue }; + let key = (v.id(), x.min(y), x.max(y)); + let link = match links.get(&key) { + Some(l) => l.clone(), + None => { + let nv = node_verts.get(&v.id()).ok_or_else(|| fail("split corner"))?; + let (Some(va), Some(vb)) = (nv.get(key.1), nv.get(key.2)) else { return Err(fail("split corner")) }; + let l = builder::line(va, vb); + links.insert(key, l.clone()); + l + } + }; + wire.push(if x < y { link } else { link.inverse() }); + } + } + wires.push(wire.into()); + } let (Some(o), Some(n)) = (old.get(i), new.get(i)) else { return Err(fail("face")) }; let surface = match (o.map_to(n), *o, *n) { (Some(mat), _, _) => mt::Transformed::transformed(&f.surface(), mat), diff --git a/crates/kernel/src/tests.rs b/crates/kernel/src/tests.rs index b65c41a5..13ae1c7f 100644 --- a/crates/kernel/src/tests.rs +++ b/crates/kernel/src/tests.rs @@ -1638,3 +1638,104 @@ fn orientation_mesh_reuses_missing_face_check_at_matching_tolerance() { } } } + +/// A trough whose floor falls about 1° toward a central drain, the fall starting right at the +/// trough edges: four faces meet at each bottom corner (two walls, two fall facets) and at the +/// drain (four facets). A tab on one end makes the body non-convex. The moved planes of four +/// faces don't meet at one point, so each such corner of the cavity becomes two corners joined +/// by a short edge. Checked against the volume integrated by hand. +#[test] +fn shell_where_four_faces_meet_at_a_corner() { + let (w, l, ht, t) = (100.0, 60.0, 40.0, 2.0); + // Drain depth: the long facets fall 1°. + let h = 30.0 * 1f64.to_radians().tan(); + let facet = |n: Vec3, p: Vec3| { + let n = n.normalized().unwrap(); + HalfSpace { n, d: n.dot(p) } + }; + let facets = [ + facet(Vec3::new(0.0, -h, -30.0), Vec3::ZERO), + facet(Vec3::new(0.0, h, -30.0), Vec3::new(0.0, l, 0.0)), + facet(Vec3::new(-h, 0.0, -50.0), Vec3::ZERO), + facet(Vec3::new(h, 0.0, -50.0), Vec3::new(w, 0.0, 0.0)), + ]; + let mut hs = vec![ + HalfSpace { n: -Vec3::X, d: 0.0 }, + HalfSpace { n: Vec3::X, d: w }, + HalfSpace { n: -Vec3::Y, d: 0.0 }, + HalfSpace { n: Vec3::Y, d: l }, + HalfSpace { n: Vec3::Z, d: ht }, + ]; + hs.extend(facets); + let trough = convex_polyhedron(&hs).unwrap(); + let pyramid = w * l * h / 3.0; + assert!(rel(measure(&trough).unwrap().volume, w * l * ht + pyramid) < 1e-9); + let tab = box_solid(Vec3::new(w, 20.0, 20.0), Vec3::new(w + 20.0, 40.0, ht)).unwrap(); + let body = boolean(&trough, &tab, BoolOp::Union).unwrap().unwrap(); + let s = shell(&body, &[Vec3::new(50.0, 30.0, ht)], t).unwrap(); + // The cavity: the walls in by t, above the floor facets moved in by t (midpoint rule; the + // floor is linear between its creases). + let floor = |x: f64, y: f64| facets.iter().map(|f| (f.d - t - f.n.x * x - f.n.y * y) / f.n.z).fold(f64::NEG_INFINITY, f64::max); + let (nx, ny) = (960, 560); + let (dx, dy) = ((w - 2.0 * t) / nx as f64, (l - 2.0 * t) / ny as f64); + let mut cavity = 0.0; + for i in 0..nx { + for j in 0..ny { + cavity += (ht - floor(t + (i as f64 + 0.5) * dx, t + (j as f64 + 0.5) * dy)) * dx * dy; + } + } + let tab_cavity = 20.0 * (20.0 - 2.0 * t) * (20.0 - t); + let want = w * l * ht + pyramid + 8000.0 - cavity - tab_cavity; + let m = measure(&s).unwrap(); + assert!(rel(m.volume, want) < 1e-5, "{} vs {want}", m.volume); +} + +/// The same trough cut as a pocket into a slab (four faces meet at each concave corner of the +/// pocket floor), shelled with its top open. +#[test] +fn shell_of_a_pocket_whose_floor_falls_to_a_drain() { + let (px, py, pw, pl, top, depth, t) = (20.0, 15.0, 100.0, 60.0, 50.0, 30.0, 3.0); + let h = 30.0 * 1f64.to_radians().tan(); + let z0 = top - depth; + let facet = |n: Vec3, p: Vec3| { + let n = n.normalized().unwrap(); + HalfSpace { n, d: n.dot(p) } + }; + let facets = [ + facet(Vec3::new(0.0, -h, -30.0), Vec3::new(px, py, z0)), + facet(Vec3::new(0.0, h, -30.0), Vec3::new(px, py + pl, z0)), + facet(Vec3::new(-h, 0.0, -50.0), Vec3::new(px, py, z0)), + facet(Vec3::new(h, 0.0, -50.0), Vec3::new(px + pw, py, z0)), + ]; + let mut hs = vec![ + HalfSpace { n: -Vec3::X, d: -px }, + HalfSpace { n: Vec3::X, d: px + pw }, + HalfSpace { n: -Vec3::Y, d: -py }, + HalfSpace { n: Vec3::Y, d: py + pl }, + HalfSpace { n: Vec3::Z, d: top + 10.0 }, + ]; + hs.extend(facets); + let pocket = convex_polyhedron(&hs).unwrap(); + let slab = box_solid(Vec3::ZERO, Vec3::new(140.0, 90.0, top)).unwrap(); + let basin = boolean(&slab, &pocket, BoolOp::Cut).unwrap().unwrap(); + let pocket_vol = pw * pl * depth + pw * pl * h / 3.0; + assert!(rel(measure(&basin).unwrap().volume, 140.0 * 90.0 * top - pocket_vol) < 1e-9); + let s = shell(&basin, &[Vec3::new(5.0, 5.0, top)], t).unwrap(); + // The cavity's columns: from t up to the top, or up to the pocket floor moved down by t + // inside the pocket's walls moved out by t. + let floor = |x: f64, y: f64| facets.iter().map(|f| (f.d + t - f.n.x * x - f.n.y * y) / f.n.z).fold(f64::NEG_INFINITY, f64::max); + let (nx, ny) = (1340, 840); + let (dx, dy) = ((140.0 - 2.0 * t) / nx as f64, (90.0 - 2.0 * t) / ny as f64); + let mut cavity = 0.0; + for i in 0..nx { + for j in 0..ny { + let (x, y) = (t + (i as f64 + 0.5) * dx, t + (j as f64 + 0.5) * dy); + let inside = x > px - t && x < px + pw + t && y > py - t && y < py + pl + t; + let roof = if inside { floor(x, y) } else { top }; + cavity += (roof - t) * dx * dy; + } + } + let want = 140.0 * 90.0 * top - pocket_vol - cavity; + let m = measure(&s).unwrap(); + assert!(rel(m.volume, want) < 1e-4, "{} vs {want}", m.volume); +} From adb1e5fec5ae06ce5f7068e3efb1da9f47c595be Mon Sep 17 00:00:00 2001 From: CypherNaugh_0x <9931495+CypherNaught-0x@users.noreply.github.com> Date: Sun, 11 Oct 2026 05:01:44 +0200 Subject: [PATCH 09/10] =?UTF-8?q?timeline.inspect=20returns=20a=20feature'?= =?UTF-8?q?s=20definition=20in=20the=20shape=20timeline.edit's=20set=20tak?= =?UTF-8?q?es,=20so=20agents=20no=20longer=20guess=20field=20names:=20time?= =?UTF-8?q?line.edit=20had=20no=20documented=20format=20({"extent":=20{"di?= =?UTF-8?q?stance":=20=E2=80=A6}}=20worked,=20but=20only=20by=20trying).?= =?UTF-8?q?=20The=20new=20read-only=20command=20(feature:=20id=20or=20name?= =?UTF-8?q?)=20returns=20type,=20definition=20(the=20editable=20fields=20a?= =?UTF-8?q?nd=20their=20current=20values;=20bookkeeping=20and=20large=20pa?= =?UTF-8?q?yloads=20such=20as=20sketch=20geometry=20and=20imported=20STEP?= =?UTF-8?q?=20text=20are=20left=20out=20and=20listed=20under=20omitted),?= =?UTF-8?q?=20inputs=20(label,=20parameter=20name,=20expression,=20unit)?= =?UTF-8?q?=20and=20the=20feature's=20error=20and=20warning.=20timeline.ed?= =?UTF-8?q?it's=20params=20doc=20points=20to=20it,=20and=20the=20book's=20?= =?UTF-8?q?Solid=20features=20chapter=20documents=20the=20edit=20format=20?= =?UTF-8?q?with=20the=20fields=20of=20the=20common=20features.=20Tests:=20?= =?UTF-8?q?inspect,=20edit=20by=20the=20field=20names=20it=20shows,=20insp?= =?UTF-8?q?ect=20again=20(extrude,=20fillet,=20sketch);=20no=20undo=20step?= =?UTF-8?q?;=20hostile=20feature=20arguments=20are=20refused?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/engine/src/cmd/edit.rs | 55 +++++++++++++++++++++++++++++++-- crates/engine/src/tests.rs | 40 ++++++++++++++++++++++++ docs/book/src/guide/features.md | 39 +++++++++++++++++++++++ docs/mcp.md | 5 +++ 4 files changed, 137 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index cdfcd4cc..a4b55195 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -35,8 +35,13 @@ pub static COMMANDS: &[CommandSpec] = &[ .params("feature: id|name → features deleted with it, and features that would fail without it"), CommandSpec::new("timeline.suppress", "Suppress Feature", suppress).params("feature: id|name, suppressed?: bool (default toggles)"), CommandSpec::new("timeline.edit", "Edit Feature", edit_feature).params( - "feature: id|name, set: {fields to change, e.g. {\"extent\": {\"distance\": \"30\"}}} → recomputed: features rebuilt, \ - restored: features whose earlier result for the same inputs was reused", + "feature: id|name, set: {fields to change, merged into the feature's definition as timeline.inspect returns it, \ + e.g. {\"extent\": {\"distance\": \"30\"}} (extrude), {\"angle\": \"90 deg\"} (revolve); \ + expressions are text} → recomputed: features rebuilt, restored: features whose earlier result for the same inputs was reused", + ), + CommandSpec::new("timeline.inspect", "Inspect Feature", inspect_feature).noundo().params( + "feature: id|name → type, definition (the fields timeline.edit's set changes, with their current values), \ + inputs: [{label, param, expression, unit}] (named inputs parameters.change can also set), error, warning", ), CommandSpec::new("select.set", "Select", select_set) .noundo() @@ -188,6 +193,52 @@ fn merge(dst: &mut Value, src: &Value, depth: usize) { } } +/// Fields of a stored feature left out of its definition: bookkeeping timeline.edit recomputes, and +/// the name and suppression, which timeline.rename and timeline.suppress change. +const NOT_EDITABLE: &[&str] = &["id", "type", "name", "suppressed", "param_names", "edge_refs", "edge_names", "face_names"]; + +fn inspect_feature(s: &mut Session, p: &Value) -> Result { + let cmd = "timeline.inspect"; + let id = feature_id(s, p.get("feature"), cmd)?; + let f = s.doc.feature(id).ok_or_else(|| bad(cmd, "feature"))?; + let mut def = serde_json::to_value(f).map_err(|e| EngineError::Other(e.to_string()))?; + let mut omitted = Vec::new(); + if let Some(o) = def.as_object_mut() { + for k in NOT_EDITABLE { + o.remove(*k); + } + // Large payloads that timeline.edit cannot change. + let heavy: &[(&str, &str)] = match &f.kind { + FeatureKind::Sketch { .. } => &[("sketch", "the sketch geometry (sketch.inspect; edit it with the sketch commands)")], + FeatureKind::Import { .. } => &[("step", "the imported STEP text"), ("components", "the imported product tree")], + FeatureKind::MeshImport { .. } => &[("meshes", "the imported meshes")], + _ => &[], + }; + for (k, why) in heavy { + if o.remove(*k).is_some() { + omitted.push(json!({"field": k, "what": why})); + } + } + } + let inputs: Vec = f + .kind + .inputs() + .into_iter() + .enumerate() + .map(|(i, (label, e, k))| { + json!({"label": label, "param": f.param_names.get(i).filter(|n| !n.is_empty()), "expression": e, "unit": k.default_unit()}) + }) + .collect(); + let marker = s.doc.marker.unwrap_or(usize::MAX); + let rolled_back = s.doc.features.iter().position(|x| x.id == id).is_some_and(|i| i >= marker); + let r = s.model.result(id); + Ok(json!({ + "id": f.id, "name": f.name, "type": f.kind.type_name(), "suppressed": f.suppressed, "rolled_back": rolled_back, + "definition": def, "omitted": omitted, "inputs": inputs, + "error": r.and_then(|r| r.error.clone()), "warning": r.and_then(|r| r.warning.clone()), + })) +} + fn edit_feature(s: &mut Session, p: &Value) -> Result { let cmd = "timeline.edit"; let id = feature_id(s, p.get("feature"), cmd)?; diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index e156d671..3c274c8f 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -407,6 +407,46 @@ fn sample_edges_chain_and_faces() { assert!(m.edge_faces.iter().all(|f| !f.is_empty())); } +/// timeline.inspect returns a feature's definition in the shape timeline.edit's `set` takes, so +/// an agent can read the field names instead of guessing them. +#[test] +fn timeline_inspect_shows_what_timeline_edit_changes() { + let mut s = Session::default(); + run(&mut s, "sketch.create", json!({"plane": "XY", "name": "Base"})); + run(&mut s, "sketch.rectangle.two_point", json!({"p0": [0, 0], "p1": [40, 30]})); + run(&mut s, "sketch.finish", json!({})); + run(&mut s, "solid.extrude", json!({"distance": 10, "name": "Slab"})); + run(&mut s, "solid.fillet", json!({"edges": [[20, 0, 10]], "radius": 2, "name": "Round"})); + let i = run(&mut s, "timeline.inspect", json!({"feature": "Slab"})); + assert_eq!(i["type"], "ExtrudeFeature", "{i}"); + assert_eq!(i["definition"]["extent"]["distance"], "10", "{i}"); + assert_eq!(i["definition"]["operation"], "new_body", "{i}"); + assert!(i["definition"].get("type").is_none() && i["definition"].get("param_names").is_none(), "{i}"); + assert_eq!(i["inputs"][0]["label"], "Distance", "{i}"); + assert_eq!(i["inputs"][0]["unit"], "mm", "{i}"); + let d = i["inputs"][0]["param"].as_str().unwrap().to_string(); + // The definition's field names are what timeline.edit takes. + run(&mut s, "timeline.edit", json!({"feature": "Slab", "set": {"extent": {"distance": "12"}}})); + let i = run(&mut s, "timeline.inspect", json!({"feature": i["id"]})); + assert_eq!(i["definition"]["extent"]["distance"], "12", "{i}"); + assert_eq!(i["inputs"][0]["param"], d.as_str(), "{i}"); + let f = run(&mut s, "timeline.inspect", json!({"feature": "Round"})); + assert_eq!(f["definition"]["radius"], "2", "{f}"); + run(&mut s, "timeline.edit", json!({"feature": "Round", "set": {"radius": "3"}})); + assert_eq!(run(&mut s, "timeline.inspect", json!({"feature": "Round"}))["definition"]["radius"], "3"); + // A sketch leaves its geometry out (sketch.inspect has it) and says so. + let k = run(&mut s, "timeline.inspect", json!({"feature": "Base"})); + assert!(k["definition"].get("sketch").is_none() && k["definition"].get("plane").is_some(), "{k}"); + assert_eq!(k["omitted"][0]["field"], "sketch", "{k}"); + // Read-only: no undo step; bad input is refused. + let undo = s.undo.len(); + run(&mut s, "timeline.inspect", json!({"feature": "Slab"})); + assert_eq!(s.undo.len(), undo); + for bad in [json!({}), json!({"feature": "Nope"}), json!({"feature": [1]}), json!({"feature": 1e300}), json!({"feature": null})] { + assert!(s.execute("timeline.inspect", &bad).is_err(), "{bad}"); + } +} + #[test] fn timeline_edits_reresolve_references() { let mut s = Session::default(); diff --git a/docs/book/src/guide/features.md b/docs/book/src/guide/features.md index 555f0561..4250411d 100644 --- a/docs/book/src/guide/features.md +++ b/docs/book/src/guide/features.md @@ -26,5 +26,44 @@ Feature references to faces and edges are stored by name, so an upstream edit th keeps them on the right face (see [Persistent naming](../dev/naming.md)). When a reference can't be found, the feature warns instead of silently picking another face. +## Editing a feature from a script + +`timeline.inspect {feature}` returns a feature's `definition`: the fields `timeline.edit` changes, +with their current values. `timeline.edit {feature, set}` merges `set` into that definition +(objects are merged field by field, anything else replaces the value), rebuilds the feature and +reports `recomputed` and `restored`. The feature's type cannot change, and sketch geometry is +edited with the sketch commands. Expressions are text (`"30"`, `"30 mm"`, `"width / 2"`, +`"45 deg"`); a bare number is a length in the design's units, or degrees for an angle. + +```json +{"command": "timeline.inspect", "params": {"feature": "Slab"}} +→ {"type": "ExtrudeFeature", + "definition": {"sketch": 1, "profiles": {"type": "all"}, "operation": "new_body", + "extent": {"distance": "10", "direction": "positive"}}, + "inputs": [{"label": "Distance", "param": "d1", "expression": "10", "unit": "mm"}], …} +{"command": "timeline.edit", "params": {"feature": "Slab", "set": {"extent": {"distance": "12"}}}} +``` + +`inputs` lists the feature's named inputs. `parameters.change {name: "d1", expression}` sets one +of those directly. Optional fields that are not set do not appear in the definition; they can +still be added through `set`. The fields of the common features: + +| Feature | Fields | +|---|---| +| Extrude | `extent`: {`distance`, `direction`: positive\|negative\|symmetric, `distance2`, `start_offset`, `through_all`, `taper`, `to`, `to_offset`, `from`}; `operation`: new_body\|join\|cut\|intersect; `targets`; `profiles`; `sketch` | +| Revolve | `angle`, `angle2`, `symmetric`, `axis`, `to`, `operation`, `targets`, `profiles`, `sketch` | +| Fillet | `radius`, `edges` (a point on each edge), `body`, `style` | +| Chamfer | `distance`, `distance2` or `angle`, `flip`, `edges`, `body` | +| Box | `length`, `width`, `height`, `corner`, `operation` | +| Cylinder | `radius`, `height`, `base`, `axis`, `operation` | +| Hole | `diameter`, `depth`, `position`, `direction`, `hole` (simple, drilled, counterbore, countersink), `thread`, `to` | +| Shell | `thickness`, `faces`, `direction`, `body`, `tangent_chain` | +| Draft | `angle`, `faces`, `neutral`, `pull`, `body` | +| Offset face | `distance`, `faces`, `body` | +| Pattern | `pattern` (its kind and counts/spacing), `features`, `bodies` | +| Mirror | `plane`, `features`, `bodies`, `combine` | + +For any other feature, `timeline.inspect` shows the fields. + Press **Delete** on anything selected (bodies, faces, sketches, features, planes, components, canvases). It runs as one undoable step and lists the features that would go with it or fail. diff --git a/docs/mcp.md b/docs/mcp.md index 63370d71..d8bb0c49 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -62,6 +62,11 @@ Claude Desktop (`claude_desktop_config.json`), bridged to a running app: | `open` | `path` | Open a `.solvecraft` design, a STEP file (`.step`/`.stp`) as a new design holding its bodies (an Import base feature), or a 3MF/STL file as mesh bodies. | | `save` | `path?` | Save the design (to its current file without `path`). | +To change a feature after it is made, read its definition with `execute` `timeline.inspect` +(`{feature}`: id or name) and change fields with `timeline.edit` (`{feature, set}`; e.g. +`{"extent": {"distance": "30"}}`). The fields per feature type are listed in the book's +*Solid features* chapter. + Resources: `solvecraft://design` (inspect JSON with measurements) and `solvecraft://commands` (the catalog). From 413a1fb727f8c5d258debc3c961665935bae7083 Mon Sep 17 00:00:00 2001 From: bherbruck Date: Sun, 11 Oct 2026 07:12:52 -0400 Subject: [PATCH 10/10] docs: guide agents to show concrete visual context and track integration --- .github/ISSUE_TEMPLATE/bug_report.md | 19 +++++++++++++++++++ .github/pull_request_template.md | 15 +++++++++++++++ AGENTS.md | 19 +++++++++++++++++++ CONTRIBUTING.md | 17 +++++++++++++++++ 4 files changed, 70 insertions(+) create mode 100644 .github/ISSUE_TEMPLATE/bug_report.md create mode 100644 .github/pull_request_template.md diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md new file mode 100644 index 00000000..9c9fd809 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -0,0 +1,19 @@ +--- +name: Bug report +about: Help reproduce and fix a SolveCraft problem +title: '' +labels: '' +assignees: '' +--- + +## Behavior + +What did you expect, and what happened instead? + +## Reproduction + +Include steps, SolveCraft version/commit and relevant environment details. Prefer a small example without private design data. + +## Concrete visual context (when helpful) + +If a screenshot, short demo, diagram or example model view helps make the problem concrete, include it with the reproduction steps, even for kernel, geometry, assembly or backend behavior. Coding agents should run the example and capture the actual result themselves when possible, rather than asking a human for evidence they can produce. Annotate the focus when helpful. Exclude private designs and sensitive details; attach media or use an accessible link, not committed generated artifacts. Problems with no useful visual explanation do not require media. diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 00000000..a346f857 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,15 @@ +## Change + +Describe the problem and resulting behavior. Link the issue when applicable. + +## Validation + +What did you run, and what were the results? Include expected and measured values for geometry changes. + +## Concrete visual context (when helpful) + +If a screenshot, short demo, diagram or before/after model view helps explain what this change means, include it alongside the prose. This includes kernel, geometry, assembly and backend changes, not only UI work. Coding agents should run the example and capture the actual reproduction/result themselves, adding brief steps and annotations where helpful. Use a public-safe example; exclude private designs and sensitive details, and attach media rather than committing generated review artifacts. Changes with no useful visual explanation can omit this section. + +## Integration status + +Record the upstream commit and PR status checked before handoff, any conflicts resolved, and validation after integration. Keep up with upstream merges and monitor this PR while the task is active; rebase before pushing and never force-push. diff --git a/AGENTS.md b/AGENTS.md index dd86ed23..40bf5219 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -71,6 +71,25 @@ People trust SolveCraft with their designs; a crash loses their work. **This out - **Never break wasm**: everything below the apps must build for `wasm32-unknown-unknown` (`cargo xtask wasm`, part of `cargo xtask ci`). +## Showing and following through on changes + +- Give issues and PRs concrete visual context whenever it helps explain what is broken or + changed, including kernel, geometry, assembly and backend work. Coding agents should run + the relevant example and capture a screenshot, short recording, diagram or before/after + model view themselves. Show the actual reproduction/result, annotate the focus when useful, + and pair it with brief expected/actual behavior; do not ask a human for evidence the agent + can produce. Changes with no useful visual explanation do not need media. +- Use a small public-safe example and check for private designs, paths, account details or + other sensitive content before sharing. Attach media to the issue/PR or link to an accessible + recording; do not commit generated screenshots or videos just to provide review evidence. +- Throughout the work and before handoff, check current upstream commits and the relevant + PR's merge status. Keep your branch current with upstream merges, resolve conflicts without + discarding others' work, and re-run validation when integration changes the result. Use + `git pull --rebase origin main` before pushing as above; never force-push. Report the checked + upstream/PR state, any conflicts resolved and the validation performed. If a PR remains open, + keep monitoring it while the task is active and make its pending status clear at handoff; + do not claim it merged without checking. + ## Application releases Development pushes are not application releases. Follow `docs/releasing.md` when asked to prepare or ship a release: choose the version from the actual changes, update Cargo.toml and diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f941e55c..f35be474 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -168,6 +168,23 @@ install. If you work on geometry and want to check against it, ask in an issue. 4. Reference the issue (`Fixes #12`). In the PR, say what you tested and how. For geometry work, include the measured numbers (and the expected ones). +Words alone can leave a change hard to picture. When a screenshot, short demo, diagram or +before/after model view helps explain what is broken or changed, please include that concrete +context in the issue or PR. This applies to kernel, geometry, assembly and backend work as well +as the UI. Coding agents should run the relevant example and capture the actual reproduction +or result themselves, with a brief expected/actual explanation and annotations where useful; +please don't ask a human for evidence the agent can produce. Changes with no useful visual +explanation do not need media. Use a small example without private designs or sensitive +details; attach the media or link to an accessible recording rather than committing generated +review artifacts. + +Keep checking upstream commits and the relevant PR's merge status throughout the work and +before handoff. Integrate upstream merges, resolve conflicts without losing others' changes, +and validate the integrated result. Before pushing, run `git pull --rebase origin main`, re-run +CI if it brought changes, and push without force. Report the upstream/PR state you checked, +any conflicts resolved and validation results. While a task remains active, follow an open +PR through its status changes; at handoff, clearly identify anything still awaiting merge. + Much of SolveCraft so far was written by AI coding agents (Claude) working in parallel, each owning one area, with a human setting direction and reporting bugs. Their working notes are in [AGENTS.md](AGENTS.md). Human contributors are just as welcome. Use whatever tools you like, as