From d0e83aefa6bf61a6f52048305d0f3172c33810ca Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 23 Oct 2026 17:02:24 +0000 Subject: [PATCH 01/31] chore(deps): bump lyon_tessellation Bumps the cargo-dependencies group with 1 update in the / directory: [lyon_tessellation](https://github.com/nical/lyon). Updates `lyon_tessellation` from 1.0.22 to 1.0.23 - [Release notes](https://github.com/nical/lyon/releases) - [Commits](https://github.com/nical/lyon/commits) --- updated-dependencies: - dependency-name: lyon_tessellation dependency-version: 1.0.23 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: cargo-dependencies ... Signed-off-by: dependabot[bot] --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e5076e82e..266e27ca9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2212,9 +2212,9 @@ dependencies = [ [[package]] name = "lyon_tessellation" -version = "1.0.22" +version = "1.0.23" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "43b8dcf906637ecef61b3c0740c7a4e7f27caeb31257cfac0cc579ce15be6005" +checksum = "fd4872650e70dc907dd4f7ebc449d7d657a0d2658e423b2e724970870bbbf75b" dependencies = [ "float_next_after", "lyon_path", From bbbd9eec512c8a0eedb788fe63c771b16ea01844 Mon Sep 17 00:00:00 2001 From: aiongg <67543387+aiongg@users.noreply.github.com> Date: Sun, 25 Oct 2026 12:06:35 -0400 Subject: [PATCH 02/31] craft-pdf and its PDF backends back at craft-libs 6e7f285 (#2147) The egui 0.37 migration moved craft-pdf, krilla, hayro-syntax and hayro-jbig2 to 7895f5e, 206 commits older, though none of them depends on egui. 6e7f285 keeps krilla patch 13 (non-finite link and page-box geometry refused), craft-libs#282 (URI links only for http(s) and mailto) and craft-libs#255 (CCITT and JPEG 2000 decode bounds). The embedded ICC profiles are identical at both revisions. --- ATTRIBUTION.md | 8 ++++---- Cargo.lock | 8 ++++---- Cargo.toml | 8 ++++---- NOTICE | 4 ++-- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/ATTRIBUTION.md b/ATTRIBUTION.md index 99990cbab..4b9ba5eb5 100644 --- a/ATTRIBUTION.md +++ b/ATTRIBUTION.md @@ -24,10 +24,10 @@ Generated-in-code assets are original and have no file to list: | Asset | Author | Source | Licence | Notes | |---|---|---|---|---| | `epaint_default_fonts::EGUI_ICONS`, `epaint_default_fonts::EMOJI_ICON` (dependency font inputs) | John Slegers | [egui 173cc737](https://github.com/storytold/egui/tree/6d518fc799681571963842f95b55a6b68431b15a/crates/epaint_default_fonts/fonts), [emoji-icon-font](https://github.com/jslegers/emoji-icon-font) | MIT (Copyright (c) 2014 John Slegers; full upstream text in `NOTICE`) | egui-icons.ttf is the five-glyph subset; emoji-icon-font.ttf is the full monochrome fallback. Registered by the production default font definitions; dependency assets unmodified here, no font file copied into this repository. | -| `krilla 0.8.2 / icc/sGrey-v2-magic.icc` (dependency-embedded) | Clinton Ingram | [craft-libs 7895f5e vendor/krilla/icc/sGrey-v2-magic.icc](https://github.com/storytold/craft-libs/blob/7895f5ed79da2f1271b10139cb84d61d4cbc50ca/vendor/krilla/icc/sGrey-v2-magic.icc) (Krilla 0.8.2 upstream 3ffdf0588cf98050aad6edba51ca70162e1fb5b5) | CC0-1.0 | Embedded by the pinned PDF writer (`craft-pdf` 0.1.0, Krilla 0.8.2); unmodified dependency asset, no file copied into this repository. SHA-256 `4110169287e5f1b5149ca9f1da1814d32943905bba8030e849d1a3b729a19481`. | -| `krilla 0.8.2 / icc/sGrey-v4.icc` (dependency-embedded) | Clinton Ingram | [craft-libs 7895f5e vendor/krilla/icc/sGrey-v4.icc](https://github.com/storytold/craft-libs/blob/7895f5ed79da2f1271b10139cb84d61d4cbc50ca/vendor/krilla/icc/sGrey-v4.icc) (Krilla 0.8.2 upstream 3ffdf0588cf98050aad6edba51ca70162e1fb5b5) | CC0-1.0 | Embedded by the pinned PDF writer (`craft-pdf` 0.1.0, Krilla 0.8.2); unmodified dependency asset, no file copied into this repository. SHA-256 `00c0f94e09127520a17dc0e1d9264b5702081d96dbdb1549ee88e3631ce42a9d`. | -| `krilla 0.8.2 / icc/sRGB-v2-magic.icc` (dependency-embedded) | Clinton Ingram | [craft-libs 7895f5e vendor/krilla/icc/sRGB-v2-magic.icc](https://github.com/storytold/craft-libs/blob/7895f5ed79da2f1271b10139cb84d61d4cbc50ca/vendor/krilla/icc/sRGB-v2-magic.icc) (Krilla 0.8.2 upstream 3ffdf0588cf98050aad6edba51ca70162e1fb5b5) | CC0-1.0 | Embedded by the pinned PDF writer (`craft-pdf` 0.1.0, Krilla 0.8.2); unmodified dependency asset, no file copied into this repository. SHA-256 `af4efe28f6d311799865f325ba39184a2e978b113ca74124d60af7de22b105f4`. | -| `krilla 0.8.2 / icc/sRGB-v4.icc` (dependency-embedded) | Clinton Ingram | [craft-libs 7895f5e vendor/krilla/icc/sRGB-v4.icc](https://github.com/storytold/craft-libs/blob/7895f5ed79da2f1271b10139cb84d61d4cbc50ca/vendor/krilla/icc/sRGB-v4.icc) (Krilla 0.8.2 upstream 3ffdf0588cf98050aad6edba51ca70162e1fb5b5) | CC0-1.0 | Embedded by the pinned PDF writer (`craft-pdf` 0.1.0, Krilla 0.8.2); unmodified dependency asset, no file copied into this repository. SHA-256 `c56e1685d888f5edb92fe07f2750f387f8fe8e91b32ff8fb0b56bfbbb9458353`. | +| `krilla 0.8.2 / icc/sGrey-v2-magic.icc` (dependency-embedded) | Clinton Ingram | [craft-libs 6e7f285 vendor/krilla/icc/sGrey-v2-magic.icc](https://github.com/storytold/craft-libs/blob/6e7f28518dd1cb57e1624bdc269c3c14316b13ae/vendor/krilla/icc/sGrey-v2-magic.icc) (Krilla 0.8.2 upstream 3ffdf0588cf98050aad6edba51ca70162e1fb5b5) | CC0-1.0 | Embedded by the pinned PDF writer (`craft-pdf` 0.1.0, Krilla 0.8.2); unmodified dependency asset, no file copied into this repository. SHA-256 `4110169287e5f1b5149ca9f1da1814d32943905bba8030e849d1a3b729a19481`. | +| `krilla 0.8.2 / icc/sGrey-v4.icc` (dependency-embedded) | Clinton Ingram | [craft-libs 6e7f285 vendor/krilla/icc/sGrey-v4.icc](https://github.com/storytold/craft-libs/blob/6e7f28518dd1cb57e1624bdc269c3c14316b13ae/vendor/krilla/icc/sGrey-v4.icc) (Krilla 0.8.2 upstream 3ffdf0588cf98050aad6edba51ca70162e1fb5b5) | CC0-1.0 | Embedded by the pinned PDF writer (`craft-pdf` 0.1.0, Krilla 0.8.2); unmodified dependency asset, no file copied into this repository. SHA-256 `00c0f94e09127520a17dc0e1d9264b5702081d96dbdb1549ee88e3631ce42a9d`. | +| `krilla 0.8.2 / icc/sRGB-v2-magic.icc` (dependency-embedded) | Clinton Ingram | [craft-libs 6e7f285 vendor/krilla/icc/sRGB-v2-magic.icc](https://github.com/storytold/craft-libs/blob/6e7f28518dd1cb57e1624bdc269c3c14316b13ae/vendor/krilla/icc/sRGB-v2-magic.icc) (Krilla 0.8.2 upstream 3ffdf0588cf98050aad6edba51ca70162e1fb5b5) | CC0-1.0 | Embedded by the pinned PDF writer (`craft-pdf` 0.1.0, Krilla 0.8.2); unmodified dependency asset, no file copied into this repository. SHA-256 `af4efe28f6d311799865f325ba39184a2e978b113ca74124d60af7de22b105f4`. | +| `krilla 0.8.2 / icc/sRGB-v4.icc` (dependency-embedded) | Clinton Ingram | [craft-libs 6e7f285 vendor/krilla/icc/sRGB-v4.icc](https://github.com/storytold/craft-libs/blob/6e7f28518dd1cb57e1624bdc269c3c14316b13ae/vendor/krilla/icc/sRGB-v4.icc) (Krilla 0.8.2 upstream 3ffdf0588cf98050aad6edba51ca70162e1fb5b5) | CC0-1.0 | Embedded by the pinned PDF writer (`craft-pdf` 0.1.0, Krilla 0.8.2); unmodified dependency asset, no file copied into this repository. SHA-256 `c56e1685d888f5edb92fe07f2750f387f8fe8e91b32ff8fb0b56bfbbb9458353`. | | `assets/app-icon/hicolor/128x128/apps/ai.storyteller.cadcraft.png` | CADCraft contributors | generated from `assets/app-icon/cadcraft-1024.png` | MIT OR Apache-2.0 | Linux hicolor icon | | `assets/app-icon/hicolor/16x16/apps/ai.storyteller.cadcraft.png` | CADCraft contributors | generated from `assets/app-icon/cadcraft-1024.png` | MIT OR Apache-2.0 | Linux hicolor icon | | `assets/app-icon/hicolor/24x24/apps/ai.storyteller.cadcraft.png` | CADCraft contributors | generated from `assets/app-icon/cadcraft-1024.png` | MIT OR Apache-2.0 | Linux hicolor icon | diff --git a/Cargo.lock b/Cargo.lock index 6ab372714..34b7ffe6d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -961,7 +961,7 @@ source = "git+https://github.com/storytold/craft-libs?rev=82a24b006eefa036b43c09 [[package]] name = "craft-pdf" version = "0.1.0" -source = "git+https://github.com/storytold/craft-libs?rev=7895f5ed79da2f1271b10139cb84d61d4cbc50ca#7895f5ed79da2f1271b10139cb84d61d4cbc50ca" +source = "git+https://github.com/storytold/craft-libs?rev=6e7f28518dd1cb57e1624bdc269c3c14316b13ae#6e7f28518dd1cb57e1624bdc269c3c14316b13ae" dependencies = [ "hayro-syntax", "krilla", @@ -1855,7 +1855,7 @@ checksum = "9f4d0e94ddd48749f06bbe4e5389fb9799a0c45bcaf00495042076ef05e3241a" [[package]] name = "hayro-jbig2" version = "0.3.0" -source = "git+https://github.com/storytold/craft-libs?rev=7895f5ed79da2f1271b10139cb84d61d4cbc50ca#7895f5ed79da2f1271b10139cb84d61d4cbc50ca" +source = "git+https://github.com/storytold/craft-libs?rev=6e7f28518dd1cb57e1624bdc269c3c14316b13ae#6e7f28518dd1cb57e1624bdc269c3c14316b13ae" dependencies = [ "hayro-ccitt", ] @@ -1869,7 +1869,7 @@ checksum = "c75ab947623ef4ccaa7acf0579edf7cbb5a73838e3839a7be73335e522f433a1" [[package]] name = "hayro-syntax" version = "0.7.2" -source = "git+https://github.com/storytold/craft-libs?rev=7895f5ed79da2f1271b10139cb84d61d4cbc50ca#7895f5ed79da2f1271b10139cb84d61d4cbc50ca" +source = "git+https://github.com/storytold/craft-libs?rev=6e7f28518dd1cb57e1624bdc269c3c14316b13ae#6e7f28518dd1cb57e1624bdc269c3c14316b13ae" dependencies = [ "hayro-jbig2", "hayro-jpeg2000", @@ -2215,7 +2215,7 @@ dependencies = [ [[package]] name = "krilla" version = "0.8.2" -source = "git+https://github.com/storytold/craft-libs?rev=7895f5ed79da2f1271b10139cb84d61d4cbc50ca#7895f5ed79da2f1271b10139cb84d61d4cbc50ca" +source = "git+https://github.com/storytold/craft-libs?rev=6e7f28518dd1cb57e1624bdc269c3c14316b13ae#6e7f28518dd1cb57e1624bdc269c3c14316b13ae" dependencies = [ "base64", "bumpalo", diff --git a/Cargo.toml b/Cargo.toml index 293a1b367..8334b1630 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,7 +28,7 @@ cadcraft-mcp = { path = "crates/mcp" } craft-docpath = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "82a24b006eefa036b43c09a67fb9a3fbd17ffe63" } # PDF writing (krilla) for plots, and reading them back in tests. The root patches below pin # craft-pdf's patched backends to the same revision (Cargo ignores dependency-local patches). -craft-pdf = { version = "=0.1.0", git = "https://github.com/storytold/craft-libs", rev = "7895f5ed79da2f1271b10139cb84d61d4cbc50ca", default-features = false } +craft-pdf = { version = "=0.1.0", git = "https://github.com/storytold/craft-libs", rev = "6e7f28518dd1cb57e1624bdc269c3c14316b13ae", default-features = false } # foundation thiserror = "2" @@ -91,9 +91,9 @@ debug = 0 # PDF backends for craft-pdf: pinned to the same reviewed craft-libs revision as craft-pdf itself # (Cargo ignores dependency-local patches). [patch.crates-io] -krilla = { version = "=0.8.2", git = "https://github.com/storytold/craft-libs", rev = "7895f5ed79da2f1271b10139cb84d61d4cbc50ca" } -hayro-syntax = { git = "https://github.com/storytold/craft-libs", rev = "7895f5ed79da2f1271b10139cb84d61d4cbc50ca" } -hayro-jbig2 = { git = "https://github.com/storytold/craft-libs", rev = "7895f5ed79da2f1271b10139cb84d61d4cbc50ca" } +krilla = { version = "=0.8.2", git = "https://github.com/storytold/craft-libs", rev = "6e7f28518dd1cb57e1624bdc269c3c14316b13ae" } +hayro-syntax = { git = "https://github.com/storytold/craft-libs", rev = "6e7f28518dd1cb57e1624bdc269c3c14316b13ae" } +hayro-jbig2 = { git = "https://github.com/storytold/craft-libs", rev = "6e7f28518dd1cb57e1624bdc269c3c14316b13ae" } # Keep the complete egui family and native event bridge on reviewed fork commits. ecolor = { git = "https://github.com/storytold/egui.git", rev = "6d518fc799681571963842f95b55a6b68431b15a" } diff --git a/NOTICE b/NOTICE index 811da09b1..e800c1d45 100644 --- a/NOTICE +++ b/NOTICE @@ -28,9 +28,9 @@ The PDF writer in the pinned craft-pdf 0.1.0 / Krilla 0.8.2 dependency embeds fo Profiles by Clinton Ingram, under CC0-1.0. They are unmodified dependency assets; no profile file is copied into this repository. Per-file sources and SHA-256 hashes are in ATTRIBUTION.md. Source: https://github.com/LaurenzV/krilla/tree/3ffdf0588cf98050aad6edba51ca70162e1fb5b5/crates/krilla/icc -(pinned through https://github.com/storytold/craft-libs/tree/7895f5ed79da2f1271b10139cb84d61d4cbc50ca/vendor/krilla/icc). +(pinned through https://github.com/storytold/craft-libs/tree/6e7f28518dd1cb57e1624bdc269c3c14316b13ae/vendor/krilla/icc). CC0 1.0 Universal: https://creativecommons.org/publicdomain/zero/1.0/ -Pinned licence text: https://github.com/storytold/craft-libs/blob/7895f5ed79da2f1271b10139cb84d61d4cbc50ca/vendor/krilla/icc/LICENSE +Pinned licence text: https://github.com/storytold/craft-libs/blob/6e7f28518dd1cb57e1624bdc269c3c14316b13ae/vendor/krilla/icc/LICENSE The UI's egui-icons.ttf subset and emoji-icon-font.ttf fallback are provided by epaint_default_fonts and registered through egui's default font definitions. They derive from emoji-icon-font by From 0a14fc7d3ae84cf229fcc5733efc9bdebcbdaa22 Mon Sep 17 00:00:00 2001 From: aiongg <67543387+aiongg@users.noreply.github.com> Date: Sun, 25 Oct 2026 12:07:15 -0400 Subject: [PATCH 03/31] macOS menu choices and their shortcuts run at once (#2140) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * macOS menu choices and their shortcuts run at once A menu choice is no window event, so the app ran it only at its next frame for another reason: File ▸ Close (Cmd+W) on an edited drawing showed "Save changes?" and dimmed the floating palettes a second or two late. The menu's event handler now wakes the app. * cargo fmt after merging main --- apps/cadcraft/src/main.rs | 2 +- apps/cadcraft/src/native_menu.rs | 26 +++++++++++++++++++++++--- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/apps/cadcraft/src/main.rs b/apps/cadcraft/src/main.rs index 6c9af80a8..a9bdb343a 100644 --- a/apps/cadcraft/src/main.rs +++ b/apps/cadcraft/src/main.rs @@ -78,7 +78,7 @@ impl eframe::App for App { #[cfg(target_os = "macos")] { if self.native_menu.is_none() && std::env::var_os("CADCRAFT_NO_NATIVE_MENU").is_none() { - self.native_menu = Some(native_menu::NativeMenu::install(&mut self.cad)); + self.native_menu = Some(native_menu::NativeMenu::install(&mut self.cad, ctx)); } if let Some(m) = &mut self.native_menu { m.poll(&mut self.cad, ctx); diff --git a/apps/cadcraft/src/native_menu.rs b/apps/cadcraft/src/native_menu.rs index 523d38a52..1e987870d 100644 --- a/apps/cadcraft/src/native_menu.rs +++ b/apps/cadcraft/src/native_menu.rs @@ -3,6 +3,7 @@ use std::collections::HashMap; use std::str::FromStr; +use std::sync::mpsc; use cadcraft_ui_egui::menus::{self, Entry}; use cadcraft_ui_egui::{CadApp, i18n}; @@ -22,6 +23,7 @@ pub struct NativeMenu { /// The recent files File ▸ Open Recent lists now (it is rebuilt when they change). recent: Vec, counter: usize, + events: mpsc::Receiver, } /// Accelerator for "Cmd+Shift+S" (modifier-less shortcuts stay in the app so typing works). @@ -34,7 +36,15 @@ fn accel(sc: &str) -> Option { impl NativeMenu { /// Build and register the macOS menu (app menu, Settings and the translated command tree); returns the state holding item ids and submenus. - pub fn install(app: &mut CadApp) -> Self { + pub fn install(app: &mut CadApp, ctx: &egui::Context) -> Self { + // A menu choice (or its shortcut, such as Cmd+W) is no window event, so winit would not + // wake the app for it: the handler does, and the choice runs in the next frame. + let (sender, events) = mpsc::channel(); + let wake = ctx.clone(); + MenuEvent::set_event_handler(Some(move |event| { + let _ = sender.send(event); + wake.request_repaint(); + })); let menu = Menu::new(); let mut items = HashMap::new(); let language = app.language(); @@ -70,7 +80,17 @@ impl NativeMenu { submenus.push((title, sub)); } menu.init_for_nsapp(); - Self { _menu: menu, items, submenus, predefined, language, last_refresh: f64::NEG_INFINITY, recent: app.ui.recent_files.clone(), counter } + Self { + _menu: menu, + items, + submenus, + predefined, + language, + last_refresh: f64::NEG_INFINITY, + recent: app.ui.recent_files.clone(), + counter, + events, + } } /// Rebuild File ▸ Open Recent after a drawing was opened or the list was cleared: the native @@ -93,7 +113,7 @@ impl NativeMenu { /// Drain queued menu events into commands (Discord opens externally), rebuild Open Recent, retranslate on a language change and refresh enablement when 300 ms have elapsed. pub fn poll(&mut self, app: &mut CadApp, ctx: &egui::Context) { - while let Ok(ev) = MenuEvent::receiver().try_recv() { + while let Ok(ev) = self.events.try_recv() { if let Some((cmd, _, _)) = self.items.get(ev.id.as_ref()) { if cmd == "ui.discord" { ctx.open_url(egui::OpenUrl::new_tab("https://discord.gg/artcraft")); From 1d9ddd240630230c08463536eaef3b22c64d1f0b Mon Sep 17 00:00:00 2001 From: aiongg <67543387+aiongg@users.noreply.github.com> Date: Sun, 25 Oct 2026 12:07:54 -0400 Subject: [PATCH 04/31] "Save changes?" and Drawing Recovery appear at once (#2144) An egui modal showed a frame late (its first frame only measured it) and then faded in, dim included, over 0.2 s. These alerts answer a key press or menu choice, so they now skip the fade, and their first show reruns the pass with the measured size, which draws them in the first frame presented. --- crates/ui-egui/src/autosave.rs | 2 +- crates/ui-egui/src/closing.rs | 2 +- crates/ui-egui/src/lib.rs | 1 + crates/ui-egui/src/modal.rs | 14 ++++++++ crates/ui-egui/tests/it/instant_modals.rs | 41 +++++++++++++++++++++++ crates/ui-egui/tests/it/main.rs | 1 + 6 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 crates/ui-egui/src/modal.rs create mode 100644 crates/ui-egui/tests/it/instant_modals.rs diff --git a/crates/ui-egui/src/autosave.rs b/crates/ui-egui/src/autosave.rs index e63bf1cbf..d062f69e9 100644 --- a/crates/ui-egui/src/autosave.rs +++ b/crates/ui-egui/src/autosave.rs @@ -150,7 +150,7 @@ pub fn show(app: &mut CadApp, ctx: &egui::Context) { let mut open = None; let mut discard = None; let mut later = false; - egui::Modal::new(egui::Id::unique("cc_drawing_recovery")).show(ctx, |ui| { + crate::modal::instant(ctx, egui::Id::unique("cc_drawing_recovery")).show(ctx, |ui| { ui.set_width(460.0); ui.label(RichText::new("Drawing Recovery").strong()); ui.add_space(4.0); diff --git a/crates/ui-egui/src/closing.rs b/crates/ui-egui/src/closing.rs index 9e4f35ed8..04d526fa2 100644 --- a/crates/ui-egui/src/closing.rs +++ b/crates/ui-egui/src/closing.rs @@ -204,7 +204,7 @@ pub fn prompt(app: &mut CadApp, ctx: &egui::Context) { }; let t = Tokens::get(); let mut choice = None; - egui::Modal::new(egui::Id::unique("cc_close_prompt")).show(ctx, |ui| { + crate::modal::instant(ctx, egui::Id::unique("cc_close_prompt")).show(ctx, |ui| { ui.set_width(360.0); ui.label(RichText::new(crate::tl!("CADCraft")).strong()); ui.add_space(4.0); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index d3799dc36..a157a05aa 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -39,6 +39,7 @@ pub mod icons; pub mod layers; pub mod managers; pub mod menus; +pub mod modal; pub mod opening; pub mod options; pub mod palettes; diff --git a/crates/ui-egui/src/modal.rs b/crates/ui-egui/src/modal.rs new file mode 100644 index 000000000..10c2ea562 --- /dev/null +++ b/crates/ui-egui/src/modal.rs @@ -0,0 +1,14 @@ +//! Alerts that appear in the frame that asks for them. +//! +//! An `egui::Modal` normally shows a frame late and then fades in over `animation_time` +//! (0.2 s): its area's first frame only measures it and draws nothing, and the dim behind it +//! fades with it. An alert that answers a key press or menu choice should be there at once. + +/// A modal with no fade-in. On its first show it asks egui to run the pass again with the +/// measured size, so the first frame presented already draws it. +pub fn instant(ctx: &egui::Context, id: egui::Id) -> egui::Modal { + if egui::AreaState::load(ctx, id).is_none() { + ctx.request_discard("an instant modal's first show only measures it"); + } + egui::Modal::new(id).area(egui::Modal::default_area(id).fade_in(false)) +} diff --git a/crates/ui-egui/tests/it/instant_modals.rs b/crates/ui-egui/tests/it/instant_modals.rs new file mode 100644 index 000000000..7a7797803 --- /dev/null +++ b/crates/ui-egui/tests/it/instant_modals.rs @@ -0,0 +1,41 @@ +//! "Save changes?" is fully drawn, dim included, in the frame of the Close that asks it. + +use cadcraft_engine::Session; +use cadcraft_ui_egui::{CadApp, Services, menus}; +use egui::epaint::Shape; +use egui::vec2; +use egui_kittest::Harness; +use serde_json::json; + +/// The opacity of each painted text containing `needle`. +fn text_opacity(shapes: &[egui::epaint::ClippedShape], needle: &str) -> Vec { + fn walk(shape: &Shape, needle: &str, out: &mut Vec) { + match shape { + Shape::Vec(shapes) => shapes.iter().for_each(|s| walk(s, needle, out)), + Shape::Text(text) if text.galley.text().contains(needle) => out.push(text.opacity_factor), + _ => {} + } + } + let mut out = Vec::new(); + shapes.iter().for_each(|s| walk(&s.shape, needle, &mut out)); + out +} + +#[test] +fn close_prompt_is_fully_drawn_in_the_frame_of_the_menu_choice() { + let mut app = CadApp::new(Session::default(), Services::default()); + app.ui.start_tab = false; + let mut h = Harness::builder().with_size(vec2(1280.0, 800.0)).build_ui_state( + |ui, app: &mut CadApp| { + app.logic(&ui.ctx().clone()); + app.ui(ui); + }, + app, + ); + h.run_steps(5); + h.state_mut().run("line", json!({"points": [[0, 0], [10, 0]]})).unwrap(); + h.run_steps(2); + menus::activate(h.state_mut(), "close"); + h.step(); + assert_eq!(text_opacity(&h.output().shapes, "Save changes to"), [1.0]); +} diff --git a/crates/ui-egui/tests/it/main.rs b/crates/ui-egui/tests/it/main.rs index 2e2451579..5faf3135b 100644 --- a/crates/ui-egui/tests/it/main.rs +++ b/crates/ui-egui/tests/it/main.rs @@ -12,6 +12,7 @@ mod drafting_spacing; mod file_tab_titles; mod gpu_viewport_stencil; mod hidden_command_line; +mod instant_modals; mod lisp_files; mod measure_distance_snapping; mod menubar_hover; From b37b7ceb7b5042a82b4ad90e2795fb78144d3410 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sun, 25 Oct 2026 12:13:04 -0400 Subject: [PATCH 05/31] refactor: replace wildcard imports with explicit ones (S2208, batch_09) (#2046) Co-authored-by: Claude Sonnet 5.5 --- crates/engine/src/cmd/view/named_views.rs | 3 ++- crates/engine/src/cmd/view/vports.rs | 3 ++- crates/geom/src/property_tests.rs | 4 +++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/view/named_views.rs b/crates/engine/src/cmd/view/named_views.rs index ef2750ef2..f88d240db 100644 --- a/crates/engine/src/cmd/view/named_views.rs +++ b/crates/engine/src/cmd/view/named_views.rs @@ -10,7 +10,8 @@ use cadcraft_geom::{Bounds2, Vec2}; use serde_json::{Value, json}; use super::super::qselect::wildcard; -use super::super::*; +use super::super::{CommandSpec, bad, point_param, str_param}; + use crate::{Accept, EngineError, Input, Interactive, Prompt, Result, Session, Step, View}; /// Most names one Delete or list takes. diff --git a/crates/engine/src/cmd/view/vports.rs b/crates/engine/src/cmd/view/vports.rs index 88ee0a6ed..5b95f720a 100644 --- a/crates/engine/src/cmd/view/vports.rs +++ b/crates/engine/src/cmd/view/vports.rs @@ -15,7 +15,8 @@ use cadcraft_geom::Vec2; use serde_json::{Value, json}; use super::super::qselect::wildcard; -use super::super::*; +use super::super::{CommandSpec, bad, find_command, str_param}; + use crate::{Accept, DocState, EngineError, Input, Interactive, ModelTile, ModelTiles, Prompt, Result, Session, Step, View}; /// Most tiles on screen at once. diff --git a/crates/geom/src/property_tests.rs b/crates/geom/src/property_tests.rs index f4ebb55f3..0584c9109 100644 --- a/crates/geom/src/property_tests.rs +++ b/crates/geom/src/property_tests.rs @@ -4,7 +4,9 @@ //! runs a few thousand cheap cases; the known-answer part of the suite works at 1e6–1e7 //! coordinates, where double precision starts to show (geometry-parity.md §Tests). -use crate::*; +use crate::{ + Arc, Bounds2, Circle, Line, Segment, Vec2, circle_circle, intersect_ext, intersect_segments, line_circle, line_line, line_line_infinite, +}; // ---------- generator ---------- From 5ec3a18aa7674e00e2147dcaebd83f0145682249 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:13:07 +0100 Subject: [PATCH 06/31] Escape literal \U+ and \M+ backslashes in the DXF writer (#2050) Found while testing crates/dxf/src/lib.rs:511-524 (escape_unicode) and 226-258 (unescape_unicode, applied in parse_ascii_with_limit at 278) Co-authored-by: Claude Haiku 5.5 --- crates/dxf/src/lib.rs | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/crates/dxf/src/lib.rs b/crates/dxf/src/lib.rs index 82ecf55e8..907c5abf7 100644 --- a/crates/dxf/src/lib.rs +++ b/crates/dxf/src/lib.rs @@ -413,8 +413,7 @@ pub fn write_ascii_as(tags: &[Tag], utf8: bool) -> String { for t in tags { let _ = write!(out, "{:>3}\r\n", t.code); match &t.value { - Value::Str(s) if utf8 => out.extend(s.chars().map(|c| if matches!(c, '\r' | '\n') { ' ' } else { c })), - Value::Str(s) => escape_unicode(s, &mut out), + Value::Str(s) => escape_unicode(s, utf8, &mut out), Value::Real(r) => out.push_str(&format_real(*r)), Value::Int(i) => out.push_str(&i.to_string()), Value::Bool(b) => out.push_str(&i64::from(*b).to_string()), @@ -446,11 +445,7 @@ pub fn write_binary(tags: &[Tag], utf8: bool) -> Vec { v => v.as_str(), }; let mut text = String::with_capacity(s.len()); - if utf8 { - text.extend(s.chars().map(|c| if matches!(c, '\r' | '\n') { ' ' } else { c })); - } else { - escape_unicode(&s, &mut text); - } + escape_unicode(&s, utf8, &mut text); out.extend(text.bytes().filter(|&b| b != 0)); out.push(0); } @@ -507,12 +502,16 @@ pub fn is_binary(bytes: &[u8]) -> bool { /// Escape a string for a pre-R2007 DXF, whose text is in the `$DWGCODEPAGE` code page: ASCII /// stays as it is and every other character becomes `\U+XXXX` (a surrogate pair of escapes -/// above U+FFFF), so the bytes are valid in any code page. Line breaks become spaces. -fn escape_unicode(s: &str, out: &mut String) { - for c in s.chars() { +/// above U+FFFF), so the bytes are valid in any code page; with `utf8` they stay as they are. +/// A backslash that would start a `\U+` or `\M+` sequence is written as `\U+005C` so the reader +/// gives back the literal text. Line breaks become spaces. +fn escape_unicode(s: &str, utf8: bool, out: &mut String) { + let chars: Vec = s.chars().collect(); + for (i, &c) in chars.iter().enumerate() { match c { '\r' | '\n' => out.push(' '), - c if c.is_ascii() => out.push(c), + '\\' if matches!(chars.get(i + 1), Some('U' | 'M')) && chars.get(i + 2) == Some(&'+') => out.push_str("\\U+005C"), + c if utf8 || c.is_ascii() => out.push(c), c => { let mut units = [0u16; 2]; for u in c.encode_utf16(&mut units) { @@ -569,6 +568,19 @@ pub fn records(tags: &[Tag]) -> Vec<(String, Vec)> { mod tests { use super::*; + #[test] + fn literal_escape_text_roundtrips() { + for utf8 in [false, true] { + for text in ["\\U+00E9", "\\M+183A4F", "a\\nb \\P", "\\\\U+00E9", "\\U+005C"] { + let tags = vec![Tag::s(8, text)]; + let back = parse_ascii(&write_ascii_as(&tags, utf8)).unwrap(); + assert_eq!(back, tags, "utf8={utf8} {text}"); + let back = parse_binary(&write_binary(&tags, utf8)).unwrap(); + assert_eq!(back[1..], tags[..], "binary utf8={utf8} {text}"); + } + } + } + #[test] fn ascii_roundtrip() { let tags = vec![ From 097c53bee3203b465e5992dea9b20f2360467fc9 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:13:11 +0100 Subject: [PATCH 07/31] Ignore last vertex bulge in open Polyline::centroid (#2053) Found while testing crates/geom/src/curve.rs:568-581 (Polyline::centroid); compare area() at 533-538 Co-authored-by: Claude Haiku 5.5 --- crates/geom/src/curve.rs | 4 +++- crates/geom/src/tests.rs | 10 ++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/crates/geom/src/curve.rs b/crates/geom/src/curve.rs index 72fe3ac0c..2b799e433 100644 --- a/crates/geom/src/curve.rs +++ b/crates/geom/src/curve.rs @@ -569,7 +569,9 @@ impl Polyline { let (Some(a), Some(b)) = (self.vertices.get(i), self.vertices.get((i + 1) % n)) else { continue }; let (pa, pb) = (a.p - origin, b.p - origin); add(pa.cross(pb) / 2.0, (pa + pb) * (1.0 / 3.0)); - if let Some((arc, ccw)) = bulge_to_arc(a.p, b.p, a.bulge) { + if (i + 1 < n || self.closed) + && let Some((arc, ccw)) = bulge_to_arc(a.p, b.p, a.bulge) + { let t = arc.sweep(); let seg = arc.radius * arc.radius * (t - t.sin()) / 2.0; if seg > 0.0 { diff --git a/crates/geom/src/tests.rs b/crates/geom/src/tests.rs index 8b8a0bad2..a452e610d 100644 --- a/crates/geom/src/tests.rs +++ b/crates/geom/src/tests.rs @@ -331,3 +331,13 @@ fn long_secant_keeps_both_intersections() { assert!(hits[0].0.near(Vec2::new(-2.0, 0.0), 1e-8), "{hits:?}"); assert!(hits[1].0.near(Vec2::new(2.0, 0.0), 1e-8), "{hits:?}"); } + +#[test] +fn open_polyline_centroid_ignores_last_bulge() { + let pts = [Vec2::new(0.0, 0.0), Vec2::new(10.0, 0.0), Vec2::new(10.0, 10.0)]; + let mut pl = Polyline::from_points(&pts, false); + let before = pl.centroid().unwrap(); + pl.vertices[2].bulge = 1.0; + let after = pl.centroid().unwrap(); + assert!(before.near(after, 1e-9), "{before:?} -> {after:?}"); +} From a0a691e1f7e03c890c988993d00e54b91232b481 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:13:14 +0100 Subject: [PATCH 08/31] Reject leading '+' in hex handle and colour parsing (#2067) Found while testing crates/doc/src/entity.rs:21 Co-authored-by: Claude Haiku 5.5 --- crates/doc/src/entity.rs | 15 ++++++++++++++- crates/ui-egui/src/options/mod.rs | 4 ++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/crates/doc/src/entity.rs b/crates/doc/src/entity.rs index 87c7e45b7..0d1f5eb5f 100644 --- a/crates/doc/src/entity.rs +++ b/crates/doc/src/entity.rs @@ -18,7 +18,11 @@ impl Handle { format!("{:X}", self.0) } pub fn parse_hex(s: &str) -> Option { - u64::from_str_radix(s.trim(), 16).ok().map(Handle) + let s = s.trim(); + if !s.bytes().all(|b| b.is_ascii_hexdigit()) { + return None; + } + u64::from_str_radix(s, 16).ok().map(Handle) } } @@ -1477,6 +1481,15 @@ mod grip_tests { use super::*; use cadcraft_geom::PolyVertex; + #[test] + fn handle_parse_hex_accepts_only_hex_digits() { + assert_eq!(Handle::parse_hex("1a"), Some(Handle(0x1A))); + assert_eq!(Handle::parse_hex(" 1A "), Some(Handle(0x1A))); + for bad in ["+1A", "-1A", "", "+", "1 A"] { + assert_eq!(Handle::parse_hex(bad), None, "{bad}"); + } + } + #[test] fn polyline_grips_are_vertices_and_midpoints() { // Grips are each vertex plus the midpoint of each segment; the segment vector is computed diff --git a/crates/ui-egui/src/options/mod.rs b/crates/ui-egui/src/options/mod.rs index 37ee2c404..c8b94c310 100644 --- a/crates/ui-egui/src/options/mod.rs +++ b/crates/ui-egui/src/options/mod.rs @@ -107,7 +107,7 @@ pub fn hex(c: Color32) -> String { /// Parse `"#rrggbb"` (the `#` is optional). pub fn parse_hex(s: &str) -> Option { let h = s.trim().trim_start_matches('#'); - if h.len() != 6 || !h.is_ascii() { + if h.len() != 6 || !h.bytes().all(|b| b.is_ascii_hexdigit()) { return None; } let byte = |i: usize| h.get(i..i + 2).and_then(|b| u8::from_str_radix(b, 16).ok()); @@ -387,7 +387,7 @@ mod tests { let c = Color32::from_rgb(0x12, 0xab, 0xff); assert_eq!(parse_hex(&hex(c)), Some(c)); assert_eq!(parse_hex("FFFFFF"), Some(Color32::WHITE)); - for bad in ["", "#fff", "#gggggg", "#12345678", "#ééé", "#1234567"] { + for bad in ["#+f+0+0", "", "#fff", "#gggggg", "#12345678", "#ééé", "#1234567"] { assert_eq!(parse_hex(bad), None, "{bad}"); } } From 36fcc896f7ebaf8e4edcf92f659546a0798b3b20 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo <103679419+marcoarnulfo@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:13:18 +0100 Subject: [PATCH 09/31] GETVAR CECOLOR reads back as text, as in AutoCAD (follow-up to #2014) (#2069) CECOLOR now reads "BYLAYER", "BYBLOCK", the index as text ("1" after "red") or "RGB:r,g,b" for a true colour, from getvar, LISP and the SETVAR ? list, where it was the ACI number (256, 0, 1, or the nearest index of a true colour). The header still keeps the index and CECOLOR_RGB, so DXF output and new objects are unchanged. SETVAR CECOLOR takes "RGB:r,g,b" too, so a value saved with getvar can be set back, true colour included. --- crates/engine/src/cmd/mod.rs | 2 +- crates/engine/src/cmd/props.rs | 20 ++++++++- crates/engine/src/lisp/tests.rs | 9 ++++ crates/engine/src/sysvars.rs | 4 +- crates/engine/tests/it/setvar_cecolor.rs | 57 +++++++++++++++++++++--- 5 files changed, 83 insertions(+), 9 deletions(-) diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index 74bc7f697..1856c6a15 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -33,7 +33,7 @@ pub mod plotters; mod props; pub use draw2::breakline::BreaklineOptions; pub use props::MatchProps; -pub(crate) use props::set_current_color_value; +pub(crate) use props::{current_color_value, set_current_color_value}; mod qselect; mod selecttools; mod settings; diff --git a/crates/engine/src/cmd/props.rs b/crates/engine/src/cmd/props.rs index c89204c22..0150c3390 100644 --- a/crates/engine/src/cmd/props.rs +++ b/crates/engine/src/cmd/props.rs @@ -431,7 +431,11 @@ fn run_color(s: &mut Session, p: &Value) -> Result { pub(crate) fn set_current_color_value(s: &mut Session, v: &Value) -> Option<()> { // An out-of-range index is refused here, where `color` folds it to ByLayer. let c = match v { - Value::String(t) => Color::parse(t), + // "RGB:r,g,b" is how `getvar` reads a true colour back, so a saved value can be restored. + Value::String(t) => match t.trim().get(..4).filter(|p| p.eq_ignore_ascii_case("rgb:")).and_then(|_| t.trim().get(4..)) { + Some(triple) => Color::parse(triple).filter(|c| matches!(c, Color::True(_))), + None => Color::parse(t), + }, Value::Number(n) => n.as_i64().and_then(|i| Color::parse(&i.to_string())), _ => None, }?; @@ -439,6 +443,20 @@ pub(crate) fn set_current_color_value(s: &mut Session, v: &Value) -> Option<()> Some(()) } +/// `getvar CECOLOR` as AutoCAD gives it, always text: "BYLAYER", "BYBLOCK", the index ("1" for +/// red) or "RGB:r,g,b". +pub(crate) fn current_color_value(d: &cadcraft_doc::Drawing) -> String { + if let Some(rgb) = d.header.get("CECOLOR_RGB").and_then(|v| v.as_i64()).and_then(|v| u32::try_from(v).ok()) { + let cadcraft_color::Rgb(r, g, b) = cadcraft_color::Rgb::from_u32(rgb); + return format!("RGB:{r},{g},{b}"); + } + match Color::from_aci(d.header.i64("CECOLOR", 256) as i16) { + Color::ByLayer => "BYLAYER".into(), + Color::ByBlock => "BYBLOCK".into(), + other => other.to_aci().to_string(), + } +} + /// A true colour keeps its RGB in CECOLOR_RGB (which wins for new objects); any other colour must /// clear a stale one. fn set_current_color(d: &mut cadcraft_doc::Drawing, c: Color) { diff --git a/crates/engine/src/lisp/tests.rs b/crates/engine/src/lisp/tests.rs index 820a9812b..0d8e12ce0 100644 --- a/crates/engine/src/lisp/tests.rs +++ b/crates/engine/src/lisp/tests.rs @@ -455,6 +455,15 @@ fn getvar_drawing_and_session_variables() { assert_eq!(ok(&mut s, "(getvar \"DWGTITLED\")"), "1"); } +/// CECOLOR is a string in AutoCAD, whatever it was set from (#1999). +#[test] +fn getvar_cecolor_is_a_string() { + let mut s = session(); + assert_eq!(ok(&mut s, "(list (getvar \"CECOLOR\") (type (getvar \"CECOLOR\")))"), "(\"BYLAYER\" STR)"); + assert_eq!(ok(&mut s, "(setvar \"CECOLOR\" \"red\") (getvar \"CECOLOR\")"), "\"1\""); + assert_eq!(ok(&mut s, "(setvar \"CECOLOR\" \"10,20,30\") (getvar \"CECOLOR\")"), "\"RGB:10,20,30\""); +} + #[test] fn input_functions_take_supplied_answers() { let mut s = session(); diff --git a/crates/engine/src/sysvars.rs b/crates/engine/src/sysvars.rs index 042185772..e22529aac 100644 --- a/crates/engine/src/sysvars.rs +++ b/crates/engine/src/sysvars.rs @@ -146,6 +146,8 @@ pub fn get(s: &Session, name: &str) -> Option { // Held in radians (`$ANGBASE` in DXF is in degrees); reported in degrees, like POLARANG. "ANGBASE" => json!(s.doc().ok()?.header.f64("ANGBASE", 0.0).to_degrees()), "CONSTRAINTINFER" => json!(i32::from(s.doc().ok()?.parametric.settings.infer)), + // Text, as AutoCAD reads it back; the header keeps the index (and CECOLOR_RGB) for DXF. + "CECOLOR" => json!(crate::cmd::current_color_value(s.doc().ok()?)), // Process-wide font substitution (profile settings, not saved in the drawing). "FONTALT" => json!(cadcraft_fonts::ttf::font_alt()), "FONTFALLBACK" => json!(cadcraft_fonts::ttf::fallback_fonts()), @@ -447,7 +449,7 @@ pub fn list(s: &Session) -> Value { } if let Ok(d) = s.doc() { for (k, v) in &d.header.vars { - if matches!(k.as_str(), "ANGBASE" | "SNAPUNIT" | "GRIDUNIT") { + if matches!(k.as_str(), "ANGBASE" | "SNAPUNIT" | "GRIDUNIT" | "CECOLOR") { m.insert(k.clone(), get(s, k).unwrap_or(Value::Null)); } else if !k.starts_with("CADCRAFT_") { m.insert(k.clone(), serde_json::to_value(v).unwrap_or(Value::Null)); diff --git a/crates/engine/tests/it/setvar_cecolor.rs b/crates/engine/tests/it/setvar_cecolor.rs index a8347ddcb..e37ac2966 100644 --- a/crates/engine/tests/it/setvar_cecolor.rs +++ b/crates/engine/tests/it/setvar_cecolor.rs @@ -1,5 +1,6 @@ //! `setvar CECOLOR` takes what the `color` command takes: "ByLayer", "ByBlock", a colour name or an -//! index (#1999). +//! index (#1999). `getvar CECOLOR` reads it back as AutoCAD does, as text: "BYLAYER", "BYBLOCK", +//! the index ("1" for red) or "RGB:r,g,b". use cadcraft_engine::Session; use cadcraft_engine::doc::color::{Color, Rgb}; @@ -14,22 +15,66 @@ fn new_line_color(s: &mut Session) -> Color { s.doc().unwrap().model.last().unwrap().common.color } +#[test] +fn getvar_cecolor_of_a_new_drawing_is_bylayer() { + let mut s = Session::new(); + assert_eq!(cecolor(&mut s), json!("BYLAYER")); +} + #[test] fn setvar_cecolor_accepts_names() { - for (value, aci) in [("BYLAYER", 256), ("ByLayer", 256), ("BYBLOCK", 0), ("red", 1), ("250", 250)] { + for (value, shown) in [ + ("BYLAYER", "BYLAYER"), + ("ByLayer", "BYLAYER"), + ("256", "BYLAYER"), + ("BYBLOCK", "BYBLOCK"), + ("0", "BYBLOCK"), + ("red", "1"), + ("7", "7"), + ("250", "250"), + ("RGB:10,20,30", "RGB:10,20,30"), + ("10,20,30", "RGB:10,20,30"), + ] { let mut s = Session::new(); s.execute("setvar", &json!({ "name": "CECOLOR", "value": value })).unwrap_or_else(|e| panic!("{value}: {e}")); - assert_eq!(cecolor(&mut s), json!(aci), "{value}"); + assert_eq!(cecolor(&mut s), json!(shown), "{value}"); } } +#[test] +fn getvar_cecolor_after_the_color_command() { + let mut s = Session::new(); + s.execute("color", &json!({ "color": "ByBlock" })).unwrap(); + assert_eq!(cecolor(&mut s), json!("BYBLOCK")); + s.execute("color", &json!({ "color": "10,20,30" })).unwrap(); + assert_eq!(cecolor(&mut s), json!("RGB:10,20,30")); +} + +/// The value `getvar` gives back is one `setvar` takes, so saving and restoring keeps a true colour. +#[test] +fn getvar_cecolor_restores_a_true_colour() { + let mut s = Session::new(); + s.execute("color", &json!({ "color": "10,20,30" })).unwrap(); + let saved = cecolor(&mut s); + s.execute("setvar", &json!({ "name": "CECOLOR", "value": "250" })).unwrap(); + s.execute("setvar", &json!({ "name": "CECOLOR", "value": saved })).unwrap(); + assert_eq!(new_line_color(&mut s), Color::True(Rgb(10, 20, 30))); +} + +#[test] +fn setvar_cecolor_lists_cecolor_as_text() { + let mut s = Session::new(); + s.execute("setvar", &json!({ "name": "CECOLOR", "value": "red" })).unwrap(); + assert_eq!(cadcraft_engine::sysvars::list(&s)["CECOLOR"], json!("1")); +} + #[test] fn setvar_cecolor_name_replaces_a_true_colour() { let mut s = Session::new(); s.execute("color", &json!({ "color": "10,20,30" })).unwrap(); assert_eq!(new_line_color(&mut s), Color::True(Rgb(10, 20, 30))); s.execute("setvar", &json!({ "name": "CECOLOR", "value": "ByLayer" })).unwrap(); - assert_eq!(cecolor(&mut s), json!(256)); + assert_eq!(cecolor(&mut s), json!("BYLAYER")); assert_eq!(new_line_color(&mut s), Color::ByLayer); } @@ -44,8 +89,8 @@ fn setvar_cecolor_number_replaces_a_true_colour() { #[test] fn setvar_cecolor_refuses_what_is_not_a_colour() { let mut s = Session::new(); - for bad in [json!("chartreuse"), json!(257), json!(-1), json!(true)] { + for bad in [json!("chartreuse"), json!(257), json!(-1), json!(true), json!("RGB:"), json!("RGB:300,0,0")] { assert!(s.execute("setvar", &json!({ "name": "CECOLOR", "value": bad })).is_err(), "{bad}"); } - assert_eq!(cecolor(&mut s), json!(256)); + assert_eq!(cecolor(&mut s), json!("BYLAYER")); } From a4a3a342a0306d1fa4272754d1601a46e112c847 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:13:21 +0100 Subject: [PATCH 10/31] Fix format_angle zeroing radians near a full turn at low precision (#2073) Found while testing crates/engine/src/units.rs:124-127 (wrap_full_turn, used at line 106 for AUNITS 3 and at lines 105 and 120) Co-authored-by: Claude Haiku 5.5 --- crates/engine/src/units.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/units.rs b/crates/engine/src/units.rs index 892710890..777e6477c 100644 --- a/crates/engine/src/units.rs +++ b/crates/engine/src/units.rs @@ -136,9 +136,13 @@ pub fn format_angle(a: f64, aunits: i64, auprec: i64) -> String { } } -/// `v` (in `[0, full)`), or 0 when it rounds at `prec` decimals to the same value as a full turn. +/// `v` (in `[0, full)`), or 0 when it rounds at `prec` decimals to the printed full-turn value (only when that value is exact). fn wrap_full_turn(v: f64, full: f64, prec: usize) -> f64 { - if format!("{v:.prec$}") == format!("{full:.prec$}") { 0.0 } else { v } + let full_text = format!("{full:.prec$}"); + // A full turn that cannot be printed exactly (2π) only wraps when `v` is numerically that value. + let exact = full_text.parse::().is_ok_and(|f| f == full); + let reached = exact || full - v < 1e-6; + if reached && format!("{v:.prec$}") == full_text { 0.0 } else { v } } /// Parse a distance typed by the user: decimal, `1'6"`, `1'-6 1/2"`, `6 1/2`, `3/4`, `1e3`. @@ -507,4 +511,11 @@ mod tests { assert_eq!(format_angle(angle, 4, 0), "S 74d E"); assert_eq!(format_angle(angle, 4, 6), "S 73d41'51.00\" E"); } + + #[test] + fn angle_radians_low_precision_not_wrapped() { + assert_eq!(format_angle(6.2, 3, 0), "6r"); + assert_eq!(format_angle(6.26, 3, 1), "6.3r"); + assert_eq!(format_angle(0.0, 3, 1), "0.0r"); + } } From a89be5f2dd73288b74f3b23c7d6cbb4621903802 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo <103679419+marcoarnulfo@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:13:25 +0100 Subject: [PATCH 11/31] A Fix on a derived point (a line's midpoint, an arc's end) holds the position it had before the edit (#2077) --- crates/constraints/src/dof.rs | 2 +- crates/constraints/src/lib.rs | 123 +++++++++++++++++++++- crates/constraints/src/model.rs | 12 +++ crates/constraints/src/tests.rs | 4 +- crates/engine/src/cmd/constraints.rs | 29 +++-- crates/engine/tests/it/fix_holds_grips.rs | 48 +++++++++ 6 files changed, 206 insertions(+), 12 deletions(-) diff --git a/crates/constraints/src/dof.rs b/crates/constraints/src/dof.rs index f0a34b9f8..559452223 100644 --- a/crates/constraints/src/dof.rs +++ b/crates/constraints/src/dof.rs @@ -235,7 +235,7 @@ mod budget_tests { aux_init: Vec::new(), aux: 0, }; - let comp = Comp { model, built: vec![endpoint], weights: vec![1.0; 2], tol: 1e-9 }; + let comp = Comp { model, built: vec![endpoint], weights: vec![1.0; 2], held: Vec::new(), tol: 1e-9 }; assert!(analyse(&comp).dof.is_none()); } } diff --git a/crates/constraints/src/lib.rs b/crates/constraints/src/lib.rs index 81e81fc25..7ee52f0a8 100644 --- a/crates/constraints/src/lib.rs +++ b/crates/constraints/src/lib.rs @@ -81,6 +81,12 @@ pub struct SolveOptions { pub keep: Vec, /// Entities that should take up the change. pub prefer_move: Vec, + /// Points held where they are for this solve, as a Fix would hold them (the grip an edit + /// just put down): see [`edited_points`]. + pub hold: Vec, + /// Where a Fix on a derived point (by constraint id) holds it, instead of where the point is + /// now: see [`moved_anchors`]. + pub anchors: Vec<(u32, Vec2)>, } #[derive(Clone, Debug, Default, PartialEq, Serialize)] @@ -205,12 +211,19 @@ pub(crate) struct Comp { pub(crate) model: Model, pub(crate) built: Vec, weights: Vec, + /// Parameters of the points held by [`SolveOptions::hold`]. + held: Vec, tol: f64, } impl Comp { fn locks(&self, skip: Option) -> Vec { let mut locked = vec![false; self.model.x0.len()]; + for &j in &self.held { + if let Some(l) = locked.get_mut(j) { + *l = true; + } + } for (i, b) in self.built.iter().enumerate() { if Some(i) == skip || b.form != Form::Lock { continue; @@ -346,7 +359,14 @@ pub(crate) fn build_budgeted( for c in group { let target = if c.kind.is_dimensional() { constraint_value(&mut e, c)? } else { 0.0 }; match interpret(c, &mut model, target) { - Ok(b) => built.push(b), + Ok(mut b) => { + if let Form::Anchor(_) = b.form + && let Some(&(_, a)) = opts.anchors.iter().find(|(id, _)| *id == c.id) + { + b.form = Form::Anchor(a); + } + built.push(b); + } Err(msg) => skipped.push((c.id, msg)), } } @@ -393,8 +413,9 @@ pub(crate) fn build_budgeted( for w in weights.iter_mut().skip(geo) { *w = 1e-3; } + let held = opts.hold.iter().filter_map(|r| model.lock_indices(r)).flatten().collect(); let tol = scale_tol(&model.x0); - comps.push(Comp { model, built, weights, tol }); + comps.push(Comp { model, built, weights, held, tol }); } Ok((comps, skipped)) } @@ -445,6 +466,9 @@ pub fn residuals(d: &Drawing) -> Vec<(u32, Option)> { /// A Fix keeps no position of its own: the solver locks the parameters at their current values. /// Without this, an edit that moves a fixed point (a grip drag) would carry the lock along with /// it. Only Fixes that already existed in `before`, on objects of the same shape, are restored. +/// An object held by a Fix on a derived point (a line's midpoint, an arc's end) has nothing to +/// lock: when the edit only moved it (MOVE), it is put back whole, so it is not reshaped to bring +/// the point back. pub fn restore_fixed(d: &mut Drawing, before: &Drawing) -> bool { let refs: Vec = d .constraints @@ -461,10 +485,21 @@ pub fn restore_fixed(d: &mut Drawing, before: &Drawing) -> bool { let mut changed = false; for r in &refs { let same_shape = matches!((now.slot(r.handle), old.slot(r.handle)), (Some(a), Some(b)) if a.shape == b.shape); - let (Some(i), Some(j)) = (now.lock_indices(r), old.lock_indices(r)) else { continue }; if !same_shape { continue; } + let (Some(i), Some(j)) = (now.lock_indices(r), old.lock_indices(r)) else { + if let (Some(a), Some(b)) = (now.slot(r.handle), old.slot(r.handle)) + && translated(&now, &old, r.handle) + && let (Some(dst), Some(src)) = (x.get_mut(a.off..a.off + a.len), old.x0.get(b.off..b.off + b.len)) + { + for (v, &w) in dst.iter_mut().zip(src) { + *v = w; + } + changed = true; + } + continue; + }; for (i, j) in i.into_iter().zip(j) { if let (Some(slot), Some(&v)) = (x.get_mut(i), old.x0.get(j)) && *slot != v @@ -480,6 +515,88 @@ pub fn restore_fixed(d: &mut Drawing, before: &Drawing) -> bool { changed } +/// The parameters of `h`'s points (see [`Model::point_refs`]) in `m`, as (x, y) pairs. +fn point_params(m: &Model, h: Handle) -> Vec<(usize, usize)> { + m.point_refs(h) + .iter() + .filter_map(|r| m.lock_indices(r)) + .flat_map(|ix| ix.as_chunks::<2>().0.iter().map(|&[x, y]| (x, y)).collect::>()) + .collect() +} + +/// Is `h` in `now` its shape in `old` moved by one vector, and nothing else? +fn translated(now: &Model, old: &Model, h: Handle) -> bool { + let (Some(a), Some(b)) = (now.slot(h), old.slot(h)) else { return false }; + let (Some(xa), Some(xb)) = (now.x0.get(a.off..a.off + a.len), old.x0.get(b.off..b.off + b.len)) else { return false }; + let tol = scale_tol(xb) * 10.0; + let pts: Vec<(usize, usize)> = + point_params(old, h).into_iter().filter_map(|(i, j)| Some((i.checked_sub(b.off)?, j.checked_sub(b.off)?))).collect(); + let delta = |(i, j): (usize, usize)| Some(Vec2::new(xa.get(i)? - xb.get(i)?, xa.get(j)? - xb.get(j)?)); + let Some(v) = pts.first().and_then(|&p| delta(p)) else { return false }; + if v.len() <= tol || !pts.iter().all(|&p| delta(p).is_some_and(|d| d.near(v, tol))) { + return false; + } + (0..xb.len()) + .filter(|k| !pts.iter().any(|&(i, j)| *k == i || *k == j)) + .all(|k| matches!((xa.get(k), xb.get(k)), (Some(p), Some(q)) if (p - q).abs() <= tol)) +} + +/// The points of `h` (its ends, centre or vertices) the edit from `before` to `d` moved, when it +/// moved some of them but not all: the grip that was dragged, to pass as [`SolveOptions::hold`] so +/// it stays where it was put and the rest of the object takes up the constraints. Empty when the +/// object kept its points or moved them all. +pub fn edited_points(d: &Drawing, before: &Drawing, h: Handle) -> Vec { + let (now, old) = (Model::build(d, &[h]), Model::build(before, &[h])); + if !matches!((now.slot(h), old.slot(h)), (Some(a), Some(b)) if a.shape == b.shape) { + return Vec::new(); + } + let tol = scale_tol(&old.x0) * 10.0; + let refs = now.point_refs(h); + let moved: Vec = refs + .iter() + .filter(|r| matches!((now.lock_indices(r), old.lock_indices(r)), (Some(i), Some(j)) if i.iter().zip(&j).any(|(&i, &j)| matches!((now.x0.get(i), old.x0.get(j)), (Some(p), Some(q)) if (p - q).abs() > tol)))) + .copied() + .collect(); + if moved.len() == refs.len() { Vec::new() } else { moved } +} + +/// Where `before` had each derived point held by a Fix (a line's midpoint, an arc's end), for the +/// Fixes whose point has moved since, to pass as [`SolveOptions::anchors`]. +/// +/// A Fix on a derived point anchors it where it is when the system is built, and +/// [`restore_fixed`] has no parameters to put back: without this, an edit that moves the point +/// would leave the Fix satisfied at the new place. +pub fn moved_anchors(d: &Drawing, before: &Drawing) -> Vec<(u32, Vec2)> { + let fixes: Vec<(u32, GeomRef)> = d + .constraints + .iter() + .filter(|c| c.kind == ConstraintKind::Fix && before.constraints.contains(c)) + .filter_map(|c| match c.refs.as_slice() { + [r] => Some((c.id, *r)), + _ => None, + }) + .collect(); + if fixes.is_empty() { + return Vec::new(); + } + let handles: Vec = fixes.iter().map(|(_, r)| r.handle).collect(); + let (now, old) = (Model::build(d, &handles), Model::build(before, &handles)); + let tol = scale_tol(&old.x0); + let mut out = Vec::new(); + for (id, r) in fixes { + let same_shape = matches!((now.slot(r.handle), old.slot(r.handle)), (Some(a), Some(b)) if a.shape == b.shape); + if !same_shape || now.lock_indices(&r).is_some() { + continue; + } + if let (Some(p), Some(a)) = (now.point(&now.x0, &r), old.point(&old.x0, &r)) + && !p.near(a, tol) + { + out.push((id, a)); + } + } + out +} + /// True when every interpretable constraint holds at the current geometry. pub fn all_satisfied(d: &Drawing) -> bool { let handles: Vec = d.constraints.iter().flat_map(|c| c.refs.iter().map(|r| r.handle)).collect(); diff --git a/crates/constraints/src/model.rs b/crates/constraints/src/model.rs index ba7e58548..b85afd12f 100644 --- a/crates/constraints/src/model.rs +++ b/crates/constraints/src/model.rs @@ -477,6 +477,18 @@ impl Model { } } + /// The points of an object a Fix locks directly (ends, centre, vertices, insertion point). + pub fn point_refs(&self, h: Handle) -> Vec { + let Some(s) = self.slot(h) else { return Vec::new() }; + let at = |sub: Sub| GeomRef { handle: h, sub }; + match s.shape { + Shape::Line => vec![at(Sub::Start), at(Sub::End)], + Shape::Point => vec![at(Sub::Whole)], + Shape::Circle | Shape::Arc | Shape::Ellipse { .. } | Shape::Insert { .. } => vec![at(Sub::Center)], + Shape::Poly { n, .. } | Shape::Spline { n, .. } => (0..n).filter_map(|i| u32::try_from(i).ok()).map(|i| at(Sub::Vertex(i))).collect(), + } + } + /// Parameters a Fix on this reference locks directly. `None` when the reference is a derived /// point (fixed through a residual instead). pub fn lock_indices(&self, r: &GeomRef) -> Option> { diff --git a/crates/constraints/src/tests.rs b/crates/constraints/src/tests.rs index 6db26c95e..35b73fea2 100644 --- a/crates/constraints/src/tests.rs +++ b/crates/constraints/src/tests.rs @@ -305,7 +305,7 @@ fn keep_option_moves_the_other_entity() { let mut d = Drawing::default(); let a = line(&mut d, (0.0, 0.0), (10.0, 0.0)); let b = line(&mut d, (0.0, 2.0), (10.0, 4.0)); - add(&mut d, con(ConstraintKind::Parallel, &[w(a), w(b)]), &SolveOptions { keep: vec![a], prefer_move: vec![b] }).unwrap(); + add(&mut d, con(ConstraintKind::Parallel, &[w(a), w(b)]), &SolveOptions { keep: vec![a], prefer_move: vec![b], ..Default::default() }).unwrap(); let (a0, a1) = ln(&d, a); assert!(a0.near(Vec2::new(0.0, 0.0), 1e-3) && a1.near(Vec2::new(10.0, 0.0), 1e-3), "{a0:?} {a1:?}"); } @@ -412,7 +412,7 @@ fn glyphs_and_residuals() { }) .unwrap(); assert!(!all_satisfied(&d)); - let r = solve(&mut d, &SolveOptions { keep: vec![], prefer_move: vec![] }).unwrap(); + let r = solve(&mut d, &SolveOptions::default()).unwrap(); assert_eq!(r.moved, vec![a]); assert!(all_satisfied(&d)); } diff --git a/crates/engine/src/cmd/constraints.rs b/crates/engine/src/cmd/constraints.rs index 54af88c80..41ded6d36 100644 --- a/crates/engine/src/cmd/constraints.rs +++ b/crates/engine/src/cmd/constraints.rs @@ -327,7 +327,7 @@ fn add_constraint(s: &mut Session, c: Constraint) -> Result { let keep: Vec = c.refs.first().map(|r| r.handle).into_iter().collect(); let prefer_move: Vec = c.refs.iter().skip(1).map(|r| r.handle).filter(|h| !keep.contains(h)).collect(); let mut d = s.doc()?.clone(); - let id = cs::add(&mut d, c, &cs::SolveOptions { keep, prefer_move }).map_err(err)?; + let id = cs::add(&mut d, c, &cs::SolveOptions { keep, prefer_move, ..Default::default() }).map_err(err)?; *s.doc_mut()? = d; Ok(id) } @@ -1119,7 +1119,7 @@ fn infer_after(s: &mut Session, before: &Drawing) { return; } let mut nd = d.clone(); - let opts = cs::SolveOptions { keep: others, prefer_move: placed }; + let opts = cs::SolveOptions { keep: others, prefer_move: placed, ..Default::default() }; let added = cands.into_iter().take(200).filter(|c| cs::add(&mut nd, c.clone(), &opts).is_ok()).count(); if added > 0 && let Ok(st) = s.state_mut() @@ -1148,7 +1148,8 @@ pub fn after_command(s: &mut Session, before: Option<&Arc>, ok: bool) { let mut d = (*st.doc).clone(); let purged = cs::purge_invalid(&mut d); let restored = cs::restore_fixed(&mut d, before); - let satisfied = cs::all_satisfied(&d); + let anchors = cs::moved_anchors(&d, before); + let satisfied = anchors.is_empty() && cs::all_satisfied(&d); if purged.is_empty() && !restored && satisfied { return; } @@ -1167,9 +1168,25 @@ pub fn after_command(s: &mut Session, before: Option<&Arc>, ok: bool) { keep.push(h); } } - let mut trial = d.clone(); - match cs::solve(&mut trial, &cs::SolveOptions { keep, prefer_move: Vec::new() }) { - Ok(_) => d = trial, + // A grip drag moved some of an object's points: they stay where they were put, and the + // edited objects still move least. When that cannot be solved, the edited objects are + // only kept as a whole, as for any other edit. + let hold: Vec = keep.iter().flat_map(|&h| cs::edited_points(&d, before, h)).collect(); + let mut solved = None; + if !hold.is_empty() { + let mut trial = d.clone(); + let opts = cs::SolveOptions { keep: keep.clone(), prefer_move: Vec::new(), hold, anchors: anchors.clone() }; + solved = cs::solve(&mut trial, &opts).ok().map(|_| trial); + } + let solved = match solved { + Some(t) => Ok(t), + None => { + let mut trial = d.clone(); + cs::solve(&mut trial, &cs::SolveOptions { keep, anchors, ..Default::default() }).map(|_| trial) + } + }; + match solved { + Ok(t) => d = t, Err(e) => msg.push(format!("Constraints could not be satisfied: {e}")), } } diff --git a/crates/engine/tests/it/fix_holds_grips.rs b/crates/engine/tests/it/fix_holds_grips.rs index 9a537964c..515f2c1ef 100644 --- a/crates/engine/tests/it/fix_holds_grips.rs +++ b/crates/engine/tests/it/fix_holds_grips.rs @@ -43,3 +43,51 @@ fn dragging_the_grip_of_a_fixed_end_leaves_it_in_place() { assert!(start.near(Vec2::new(0.0, 0.0), 1e-6), "{start:?}"); assert!(end.near(Vec2::new(10.0, 5.0), 1e-6), "{end:?}"); } + +#[test] +fn a_fixed_midpoint_stays_in_place_when_an_end_grip_is_dragged() { + let mut s = Session::new(); + let a = line(&mut s, [0.0, 0.0], [10.0, 0.0]); + s.execute("gcfix", &json!({ "h1": a.hex(), "p1": "mid" })).unwrap(); + + grip_drag(&mut s, a, Vec2::new(10.0, 0.0), "10,5"); + let (start, end) = ends(&s, a); + assert!(start.mid(end).near(Vec2::new(5.0, 0.0), 1e-6), "the fixed midpoint moved to {:?} ({start:?}, {end:?})", start.mid(end)); + // The dragged end lands where it was put; the line turns about its fixed midpoint. + assert!(end.near(Vec2::new(10.0, 5.0), 1e-6), "{end:?}"); + assert!(start.near(Vec2::new(0.0, -5.0), 1e-6), "{start:?}"); +} + +#[test] +fn moving_an_arc_with_a_fixed_end_leaves_it_whole_and_in_place() { + let mut s = Session::new(); + let r = s.execute("arc", &json!({ "center": [0, 0], "radius": 5, "start": 0, "end": 90 })).unwrap(); + let a = Handle::parse_hex(r["handle"].as_str().unwrap()).unwrap(); + s.execute("gcfix", &json!({ "h1": a.hex(), "p1": "end" })).unwrap(); + + s.execute("move", &json!({ "handles": [a.hex()], "delta": [1, 1] })).unwrap(); + match &s.doc().unwrap().entity(a).unwrap().kind { + EntityKind::Arc(c) => { + assert!(c.center.xy().near(Vec2::new(0.0, 0.0), 1e-6), "{:?}", c.center); + assert!((c.radius - 5.0).abs() < 1e-6, "radius {}", c.radius); + assert!((c.end - std::f64::consts::FRAC_PI_2).abs() < 1e-6, "end at {}", c.end); + } + k => panic!("not an arc: {k:?}"), + } +} + +#[test] +fn dragging_an_end_of_one_of_two_parallel_lines_leaves_its_other_end_in_place() { + let mut s = Session::new(); + let a = line(&mut s, [0.0, 0.0], [10.0, 0.0]); + let b = line(&mut s, [0.0, 2.0], [10.0, 2.0]); + s.execute("gcparallel", &json!({ "h1": a.hex(), "h2": b.hex() })).unwrap(); + + grip_drag(&mut s, a, Vec2::new(10.0, 0.0), "10,5"); + let (start, end) = ends(&s, a); + assert!(end.near(Vec2::new(10.0, 5.0), 1e-6), "{end:?}"); + assert!(start.near(Vec2::new(0.0, 0.0), 1e-3), "the end that was not dragged moved to {start:?}"); + // The other line took up the change. + let (b0, b1) = ends(&s, b); + assert!((b1 - b0).cross(end - start).abs() < 1e-6, "{b0:?} {b1:?}"); +} From 52aae9c1fbe092057d4e716c702506a1f6d6619d Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:13:28 +0100 Subject: [PATCH 12/31] Fix wcmatch returning nil for patterns with many '*' wildcards (#2078) Found while testing crates/engine/src/lisp/builtins.rs:798-800 (glob_inner depth limit; depth is incremented per '*' at line 808) Co-authored-by: Claude Haiku 5.5 --- crates/engine/src/lisp/builtins.rs | 4 ++-- crates/engine/src/lisp/tests.rs | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/lisp/builtins.rs b/crates/engine/src/lisp/builtins.rs index 0fcb22f03..8e89d667c 100644 --- a/crates/engine/src/lisp/builtins.rs +++ b/crates/engine/src/lisp/builtins.rs @@ -797,8 +797,8 @@ fn glob_inner( return Err(()); } *budget -= 1; - if depth > 64 { - return Ok(false); + if depth > 256 { + return Err(()); } let (mut t, mut p) = (0usize, 0usize); while p < pat.len() { diff --git a/crates/engine/src/lisp/tests.rs b/crates/engine/src/lisp/tests.rs index 0d8e12ce0..a48c04eba 100644 --- a/crates/engine/src/lisp/tests.rs +++ b/crates/engine/src/lisp/tests.rs @@ -136,6 +136,10 @@ fn lists_and_strings() { assert_eq!(ok(&mut s, "(wcmatch \"abc\" \"x*,a?c\")"), "T"); assert_eq!(ok(&mut s, "(wcmatch \"abc\" \"~a*\")"), "nil"); assert_eq!(ok(&mut s, "(wcmatch \"b\" \"[a-c]\")"), "T"); + let many = format!("(wcmatch \"{}\" \"{}\")", "a".repeat(80), "*a".repeat(80)); + assert_eq!(ok(&mut s, &many), "T"); + let too_deep = format!("(wcmatch \"{}\" \"{}\")", "a".repeat(400), "*a".repeat(400)); + assert!(eval(&mut s, &too_deep).is_err()); assert_eq!(ok(&mut s, "(vl-string-subst \"X\" \"b\" \"abcb\")"), "\"aXcb\""); assert_eq!(ok(&mut s, "(vl-string-search \"c\" \"abc\")"), "2"); assert_eq!(ok(&mut s, "(read \"(1 2)\")"), "(1 2)"); From 3e6e1e534d0ff12de451482d7bfc87384901dc22 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo <103679419+marcoarnulfo@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:13:31 +0100 Subject: [PATCH 13/31] Constraints: an ellipse solution stored with its axes swapped or mirrored must still meet every constraint (an axis end by index does not) (#2082) --- crates/constraints/src/lib.rs | 25 ++++++++++- crates/constraints/src/model.rs | 58 +++++++++++++++++++++++- crates/constraints/tests/dof.rs | 80 +++++++++++++++++++++++++++++++++ 3 files changed, 160 insertions(+), 3 deletions(-) diff --git a/crates/constraints/src/lib.rs b/crates/constraints/src/lib.rs index 7ee52f0a8..72a05b6a2 100644 --- a/crates/constraints/src/lib.rs +++ b/crates/constraints/src/lib.rs @@ -258,7 +258,30 @@ impl Comp { return Err(SolveError::TooLarge); } let f = |x: &[f64], out: &mut Vec| self.eval(x, out, skip); - Ok(lm::solve(&f, &self.model.x0, &free, &self.weights, self.tol, MAX_ITER)) + let mut o = lm::solve(&f, &self.model.x0, &free, &self.weights, self.tol, MAX_ITER); + // A solution the ellipses would be stored differently from (axes swapped, minor axis + // mirrored) holds only if the constraints still hold on the stored form: a constraint on + // an axis end by index does not. + if o.converged && !self.model.storable(&o.x) { + let mut out = Vec::new(); + match self.model.stored_form(&o.x) { + Some(mut xc) => { + // Contact parameters are positions along the curves: they follow the stored form. + for b in self.built.iter().filter(|b| b.form == Form::Contact) { + for (k, r) in b.refs.iter().enumerate() { + if let (Some(&t), Some(slot)) = (o.x.get(b.aux + k), xc.get_mut(b.aux + k)) { + *slot = self.model.stored_t(&o.x, r.handle, t); + } + } + } + self.eval(&xc, &mut out, skip); + o.converged = lm::max_abs(&out) <= self.tol; + o.x = xc; + } + None => o.converged = false, + } + } + Ok(o) } /// Constraint ids in conflict (each one whose removal makes the rest solvable). fn conflicts(&self, x_failed: &[f64]) -> Vec { diff --git a/crates/constraints/src/model.rs b/crates/constraints/src/model.rs index b85afd12f..165afac8d 100644 --- a/crates/constraints/src/model.rs +++ b/crates/constraints/src/model.rs @@ -32,6 +32,8 @@ pub const MAX_RIGID_POINTS: usize = 256; const MAX_RIGID_MEMBERS: usize = 100_000; /// Shared across every model built for one analysis request. pub(crate) const MAX_RIGID_WORK: usize = 1_000_000; +/// How far past 1 a solved ellipse ratio may land by rounding and still be stored as 1. +const RATIO_TOL: f64 = 1e-9; #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) enum Shape { @@ -516,6 +518,57 @@ impl Model { Some(v) } + /// Can `write_back` store `x` as it is? An ellipse keeps 0 < ratio <= 1, its major axis the + /// longer one: a solution past that is stored with its axes swapped or its minor axis + /// mirrored (see [`Model::stored_form`]), which renumbers the axis ends. + pub fn storable(&self, x: &[f64]) -> bool { + self.slots.iter().all(|s| match s.shape { + Shape::Ellipse { .. } => x.get(s.off + 4).is_some_and(|r| *r > 0.0 && *r <= 1.0 + RATIO_TOL), + _ => true, + }) + } + + /// `x` with every ellipse re-expressed as [`Model::write_back`] stores it (the same curve with + /// the major axis the longer one, a full ellipse's negative ratio made positive), so the + /// constraints can be checked against what would be stored. `None` when an elliptical arc has + /// a negative ratio or an ellipse a zero one. + pub fn stored_form(&self, x: &[f64]) -> Option> { + let mut xc = x.to_vec(); + for s in &self.slots { + let Shape::Ellipse { arc } = s.shape else { continue }; + let o = s.off; + let mut ratio = *x.get(o + 4)?; + if ratio < 0.0 && !arc { + ratio = -ratio; + } + if ratio <= 0.0 { + return None; + } + if ratio > 1.0 + RATIO_TOL { + let major = p2(x, o + 2)?.perp() * ratio; + *xc.get_mut(o + 2)? = major.x; + *xc.get_mut(o + 3)? = major.y; + ratio = 1.0 / ratio; + if arc { + *xc.get_mut(o + 5)? -= std::f64::consts::FRAC_PI_2; + *xc.get_mut(o + 6)? -= std::f64::consts::FRAC_PI_2; + } + } + *xc.get_mut(o + 4)? = ratio; + } + Some(xc) + } + + /// The parameter `stored_form` gives the point at parameter `t` on ellipse `h` (unchanged for + /// any other object): `-t` where a negative ratio was made positive, a quarter turn back where + /// the axes were swapped. + pub fn stored_t(&self, x: &[f64], h: Handle, t: f64) -> f64 { + let Some(s) = self.slot(h) else { return t }; + let (Shape::Ellipse { .. }, Some(&ratio)) = (s.shape, x.get(s.off + 4)) else { return t }; + let (t, ratio) = if ratio < 0.0 { (-t, -ratio) } else { (t, ratio) }; + if ratio > 1.0 + RATIO_TOL { t - std::f64::consts::FRAC_PI_2 } else { t } + } + /// Write solved parameters back. Returns the handles that moved. pub fn write_back(&self, d: &mut Drawing, x: &[f64]) -> Vec { let mut moved = Vec::new(); @@ -584,8 +637,9 @@ impl Model { [a, b] if arc => (*a, *b), _ => (el.start, el.end), }; - // Keep the stored form: the major axis is the longer one. - if ratio > 1.0 { + // Keep the stored form: the major axis is the longer one. `storable` lets a + // solved ratio exceed 1 only by rounding: that is a circle, not a swap. + if ratio > 1.0 + RATIO_TOL { major = major.perp() * ratio; ratio = 1.0 / ratio; if arc { diff --git a/crates/constraints/tests/dof.rs b/crates/constraints/tests/dof.rs index ea78ca932..02fd935c0 100644 --- a/crates/constraints/tests/dof.rs +++ b/crates/constraints/tests/dof.rs @@ -382,3 +382,83 @@ fn vertex_constraints_on_large_fit_spline_do_not_charge_reconstruction() { assert_eq!(report.status, DofStatus::Under); assert_eq!(report.dof, Some(1000)); } + +/// Adds a constraint that the stored ellipse can only hold with its axes renumbered: refused, +/// with the drawing unchanged. +fn refused_on_minor_axis_end(p: (f64, f64)) { + let mut d = Drawing::default(); + let e = ellipse(&mut d, (0.0, 0.0), (4.0, 0.0), 0.5, 0.0, std::f64::consts::TAU); + ok(&mut d, con(K::Fix, &[s(e, Sub::Center)])); + let p = point(&mut d, p); + ok(&mut d, con(K::Fix, &[w(p)])); + let before = d.clone(); + let r = add(&mut d, con(K::Coincident, &[s(e, Sub::Vertex(1)), w(p)]), &SolveOptions::default()); + assert!(matches!(r, Err(SolveError::Conflict { .. })), "{r:?}, ellipse {:?}", ell(&d, e)); + assert_eq!(d, before); +} + +/// Pulling a minor axis end past the major axis length would need a ratio above 1, which the +/// stored ellipse holds only by swapping its axes (and so renumbering their ends). +#[test] +fn minor_axis_end_pulled_past_the_major_axis_is_refused() { + refused_on_minor_axis_end((0.0, 6.0)); +} + +/// Pulling a minor axis end through the centre would need a negative ratio, which the stored +/// ellipse cannot hold: storing its absolute value would mirror the minor axis end. +#[test] +fn minor_axis_end_pulled_through_the_centre_is_refused() { + refused_on_minor_axis_end((0.0, -1.0)); +} + +/// A circle-like ellipse whose solution makes the minor axis the longer one: stored with its axes +/// swapped, which constraints that name no axis end by index do not notice. +fn round_ellipse(d: &mut Drawing, start: f64, end: f64) -> Handle { + let e = ellipse(d, (0.0, 0.0), (4.0, 0.0), 1.0, start, end); + ok(d, con(K::Fix, &[s(e, Sub::Center)])); + e +} + +#[test] +fn tangent_that_swaps_a_round_ellipse_axes_is_kept() { + let mut d = Drawing::default(); + let e = round_ellipse(&mut d, 0.0, std::f64::consts::TAU); + let l = line(&mut d, (-10.0, 4.5), (10.0, 4.5)); + ok(&mut d, con(K::Fix, &[w(l)])); + ok(&mut d, con(K::Tangent, &[w(l), w(e)])); + assert!(ell(&d, e).ratio <= 1.0); + assert!(all_satisfied(&d)); +} + +#[test] +fn point_on_curve_that_swaps_a_round_ellipse_axes_is_kept() { + let mut d = Drawing::default(); + let e = round_ellipse(&mut d, 0.0, std::f64::consts::TAU); + let p = point(&mut d, (0.0, 4.5)); + ok(&mut d, con(K::Fix, &[w(p)])); + ok(&mut d, con(K::Coincident, &[w(p), w(e)])); + assert!(ell(&d, e).ratio <= 1.0); + assert!(all_satisfied(&d)); +} + +#[test] +fn elliptical_arc_start_that_swaps_its_axes_is_kept() { + let mut d = Drawing::default(); + let e = round_ellipse(&mut d, std::f64::consts::FRAC_PI_2, std::f64::consts::PI); + let p = point(&mut d, (0.0, 4.5)); + ok(&mut d, con(K::Fix, &[w(p)])); + ok(&mut d, con(K::Coincident, &[s(e, Sub::Start), w(p)])); + assert!(ell(&d, e).ratio <= 1.0); + assert!(all_satisfied(&d)); +} + +#[test] +fn tangent_circle_that_swaps_a_round_ellipse_axes_is_kept() { + let mut d = Drawing::default(); + let e = round_ellipse(&mut d, 0.0, std::f64::consts::TAU); + let c = circle(&mut d, (0.0, 6.0), 1.5); + ok(&mut d, con(K::Fix, &[w(c)])); + ok(&mut d, con(K::Tangent, &[w(c), w(e)])); + assert!(ell(&d, e).ratio <= 1.0); + assert!(all_satisfied(&d)); +} From 0f60269868463ddba512aada95561786b56cc8f6 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo <103679419+marcoarnulfo@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:13:35 +0100 Subject: [PATCH 14/31] MCP: resend once when the app refuses a token read before --launch opened it (fixes #2087) (#2098) --- crates/mcp/src/backend.rs | 137 +++++++++++++++++++++++++++++++++----- 1 file changed, 120 insertions(+), 17 deletions(-) diff --git a/crates/mcp/src/backend.rs b/crates/mcp/src/backend.rs index 693839335..9d012076a 100644 --- a/crates/mcp/src/backend.rs +++ b/crates/mcp/src/backend.rs @@ -184,17 +184,20 @@ fn is_read_only(method: &str) -> bool { ) } -impl Backend for Remote { - /// Send a control request with the token when available, refreshing it before a safe retry. - /// A mutation that may have reached the server is never resent automatically. - fn call(&mut self, method: &str, params: Value) -> Result { - let id = self.next_id; - self.next_id += 1; +impl Remote { + /// Put the current token (if any) on `req`, replacing the one it carries. + fn set_token(&mut self, req: &mut Value) { let token = self.token().map(str::to_string); - let mut req = json!({"id": id, "method": method, "params": params}); - if let (Some(obj), Some(token)) = (req.as_object_mut(), token) { - obj.insert("token".into(), Value::String(token)); + if let Some(obj) = req.as_object_mut() { + obj.remove("token"); + if let Some(token) = token { + obj.insert("token".into(), Value::String(token)); + } } + } + /// Send `req` and parse the reply, refreshing the token before a safe retry after a transport + /// error. A mutation that may have reached the server is never resent automatically. + fn exchange(&mut self, method: &str, req: &mut Value) -> Result { let reply = match self.roundtrip(&req.to_string()) { Ok(r) => r, Err((e, sent)) => { @@ -208,12 +211,7 @@ impl Backend for Remote { )); } // A restarted server may have rotated its token before dropping the old socket. - if let Some(obj) = req.as_object_mut() { - obj.remove("token"); - if let Some(token) = self.token() { - obj.insert("token".into(), Value::String(token.to_owned())); - } - } + self.set_token(req); self.roundtrip(&req.to_string()).map_err(|(e, _)| { self.token = None; self.conn = None; @@ -221,12 +219,37 @@ impl Backend for Remote { })? } }; - let v: Value = serde_json::from_str(&reply).map_err(|e| format!("bad reply: {e}"))?; + serde_json::from_str(&reply).map_err(|e| format!("bad reply: {e}")) + } +} + +/// An `unauthorized` reply (the app also closes the connection). +fn unauthorized(v: &Value) -> bool { + v.get("error").and_then(Value::as_str).is_some_and(|e| e.starts_with("unauthorized")) +} + +impl Backend for Remote { + /// Send a control request with the token when available. A refused token is read again and the + /// request resent once, only when the app says it did not run it. + fn call(&mut self, method: &str, params: Value) -> Result { + let id = self.next_id; + self.next_id += 1; + let mut req = json!({"id": id, "method": method, "params": params}); + self.set_token(&mut req); + let mut v = self.exchange(method, &mut req)?; + // The token is read before `--launch` opens the app, which then writes a new one (#2087); the + // file can also be left over from an earlier session. Once, so a wrong token can't loop. + if unauthorized(&v) && v.get("state").and_then(Value::as_str) == Some("not-run") { + self.token = None; + self.conn = None; + self.set_token(&mut req); + v = self.exchange(method, &mut req)?; + } if v.get("ok").and_then(Value::as_bool) == Some(true) { Ok(v.get("result").cloned().unwrap_or(Value::Null)) } else { let e = v.get("error").and_then(Value::as_str).unwrap_or("error"); - if e.starts_with("unauthorized") { + if unauthorized(&v) { self.token = None; self.conn = None; } @@ -405,6 +428,7 @@ impl Backend for Headless { mod tests { use super::*; use std::net::TcpListener; + use std::sync::Arc; /// A server that reads one request per connection and closes without replying must not see the request twice. #[test] @@ -479,6 +503,85 @@ mod tests { assert_eq!(remote.call("ping", json!({})).unwrap(), json!("pong")); server.join().unwrap(); } + /// Answer one request per connection like the app's control server: `reply` sees the request + /// and the connection number and says what to send back; the server closes after each reply. + /// Returns how many requests arrived, waiting a short while for each next connection. + fn fake_app(listener: TcpListener, reply: impl Fn(&Value, usize) -> Value + Send + 'static) -> std::thread::JoinHandle { + std::thread::spawn(move || { + listener.set_nonblocking(true).unwrap(); + let mut seen = 0; + loop { + let start = std::time::Instant::now(); + let conn = loop { + match listener.accept() { + Ok((conn, _)) => break Some(conn), + Err(_) if start.elapsed() < Duration::from_millis(500) => std::thread::sleep(Duration::from_millis(10)), + Err(_) => break None, + } + }; + let Some(conn) = conn else { return seen }; + conn.set_nonblocking(false).unwrap(); + let mut line = String::new(); + BufReader::new(conn.try_clone().unwrap()).read_line(&mut line).unwrap(); + let mut out = conn; + writeln!(out, "{}", reply(&serde_json::from_str(&line).unwrap(), seen)).unwrap(); + seen += 1; + } + }) + } + + /// `--launch` reads the token before it opens the app, so the first request carries the token + /// of an earlier session (or none); the app refuses it without running it, and the call is + /// resent once with the token the app has just written (#2087). + #[test] + fn unauthorized_not_run_is_resent_once_with_the_new_token() { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + let old = crate::control_token::issue(addr.port()).unwrap(); + let fresh = Arc::new(std::sync::Mutex::new(String::new())); + let f = fresh.clone(); + let server = fake_app(listener, move |req, n| { + if n == 0 { + assert_eq!(req["token"].as_str(), Some(old.as_str())); + // The app starts and writes its own session's token. + *f.lock().unwrap() = crate::control_token::issue(addr.port()).unwrap(); + json!({"ok": false, "error": "unauthorized: missing or invalid control token", "state": "not-run"}) + } else if req["token"].as_str() == Some(f.lock().unwrap().as_str()) { + json!({"ok": true, "result": "drawing"}) + } else { + json!({"ok": false, "error": "unauthorized: missing or invalid control token", "state": "not-run"}) + } + }); + let mut remote = Remote::lazy(&addr.to_string(), false); + let first = remote.call("engine.execute", json!({"command": "circle", "params": {}})); + let requests = server.join().unwrap(); + crate::control_token::revoke(addr.port()); + assert_eq!(first, Ok(json!("drawing"))); + assert_eq!(requests, 2); + assert_eq!(remote.token.as_deref(), Some(fresh.lock().unwrap().as_str())); + } + + /// A token the app keeps refusing is resent once, not in a loop, and a refusal that doesn't + /// say the request never ran is not resent at all. + #[test] + fn unauthorized_is_resent_at_most_once_and_only_when_not_run() { + for (state, connections) in [(Some("not-run"), 2), (Some("may-have-run"), 1), (None, 1)] { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap().to_string(); + let server = fake_app(listener, move |_, _| { + let mut r = json!({"ok": false, "error": "unauthorized: missing or invalid control token"}); + if let (Some(o), Some(state)) = (r.as_object_mut(), state) { + o.insert("state".into(), json!(state)); + } + r + }); + let mut remote = Remote::lazy(&addr, false); + let err = remote.call("engine.execute", json!({"command": "circle", "params": {}})).unwrap_err(); + assert!(err.starts_with("unauthorized"), "{err}"); + assert_eq!(server.join().unwrap(), connections, "{state:?}"); + assert!(remote.token.is_none() && remote.conn.is_none()); + } + } /// A failed mutation write reports uncertain delivery and does not open a retry connection. #[test] fn failed_mutation_write_is_not_resent() { From 31de470926d8cf100a20534ec0c0915642fb79b8 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:13:39 +0100 Subject: [PATCH 15/31] Fall back to decimal for huge engineering-unit distances (#2114) Found while testing crates/engine/src/units.rs:18-27 (format_distance, LUNITS 3 branch) Co-authored-by: Claude Haiku 5.5 --- crates/engine/src/units.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/crates/engine/src/units.rs b/crates/engine/src/units.rs index 777e6477c..a3018d8f3 100644 --- a/crates/engine/src/units.rs +++ b/crates/engine/src/units.rs @@ -22,6 +22,9 @@ pub fn format_distance(v: f64, lunits: i64, luprec: i64) -> String { let rounded = (v.abs() * scale).round() / scale; let a = if rounded.is_finite() { rounded } else { v.abs() }; let ft = (a / 12.0).floor(); + if ft >= I64_SAFE_MAX { + return format!("{:.*}", prec, v); + } let inch = a - ft * 12.0; format!("{}{}'-{:.*}\"", if neg { "-" } else { "" }, ft as i64, prec, inch) } @@ -360,6 +363,12 @@ pub const DEG: f64 = PI / 180.0; mod tests { use super::*; + #[test] + fn engineering_huge_distance_falls_back_to_decimal() { + assert_eq!(format_distance(1e30, 3, 2), format!("{:.2}", 1e30)); + assert_eq!(format_distance(-1e300, 3, 8), format!("{:.8}", -1e300)); + } + #[test] fn mixed_distance_needs_a_positive_fraction_after_the_whole_number() { assert_eq!(parse_distance("6 1/2"), Some(6.5)); From 0c15f7106c091b00ed4cdb08cb2f6b68cdd8b090 Mon Sep 17 00:00:00 2001 From: Patrik Lermon Date: Sun, 25 Oct 2026 17:13:42 +0100 Subject: [PATCH 16/31] i18n(sv): translate 49 new strings (#2120) Brings the Swedish catalog up to date with upstream main (e86d895). Co-authored-by: Claude Opus 5.5 --- crates/ui-egui/src/i18n/sv.tsv | 49 ++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/crates/ui-egui/src/i18n/sv.tsv b/crates/ui-egui/src/i18n/sv.tsv index 5a2596325..03d51d535 100644 --- a/crates/ui-egui/src/i18n/sv.tsv +++ b/crates/ui-egui/src/i18n/sv.tsv @@ -1397,3 +1397,52 @@ menu Modify Ändra Window < Fönster < X offset (mm) X-förskjutning (mm) Y offset (mm) Y-förskjutning (mm) + AutoLISP (LSP) AutoLISP (LSP) + Units of new drawings Enheter för nya ritningar + Imperial (inches) Anglosaxiskt (tum) + Metric (millimetres) Metriskt (millimeter) + New and the first drawing at startup use these units unless a default template is set (QNEWTEMPLATE). Nya ritningar och den första ritningen vid start använder dessa enheter om ingen standardmall är angiven (QNEWTEMPLATE). + Analyze Usage Analysera användning + Implied Loops Implicita slingor + Angstroms Ångström + Architectural Arkitektonisk + Astronomical Units Astronomiska enheter + Centimeters Centimeter + Decameters Dekameter + Decimal Decimal + Decimal Degrees Decimalgrader + Decimeters Decimeter + Deg/Min/Sec Grad/min/sek + Don't Save Spara inte + Drawing Units Ritningsenheter + Engineering Teknisk + Feet Fot + Fractional Bråk + Gigameters Gigameter + Grads Gon + Hectometers Hektometer + Inches Tum + Insertion scale Insättningsskala + Kilometers Kilometer + Light Years Ljusår + Meters Meter + Microinches Mikrotum + Microns Mikrometer + Miles Engelska mil + Millimeters Millimeter + Mils Tusendels tum + Nanometers Nanometer + Parsecs Parsek + Radians Radianer + Sample output Exempel på utdata + Save changes to {name}? Spara ändringar i {name}? + Scientific Vetenskaplig + Surveyor's Units Lantmäterienheter + Unitless Enhetslös + Units to scale inserted content Enheter för skalning av infogat innehåll + US Survey Feet Lantmäterifot (USA) + US Survey Inch Lantmäteritum (USA) + US Survey Mile Engelsk lantmäterimil (USA) + US Survey Yard Lantmäteriyard (USA) + Yards Yard + Your changes will be lost if you don't save them. Dina ändringar går förlorade om du inte sparar dem. From 422d42411a6004ce9a96825d117a7e914255b127 Mon Sep 17 00:00:00 2001 From: skylar taylor-barrick Date: Sun, 25 Oct 2026 10:13:45 -0600 Subject: [PATCH 17/31] fix: omit empty outlines in complex text shaping (#1960) * fix: filter empty outlines in complex shaping * docs: describe complex shaping and its empty-glyph regression --------- Co-authored-by: skylartaylor <26588250+skylartaylor@users.noreply.github.com> --- crates/fonts/src/complex.rs | 55 ++++++++++++++++++++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/crates/fonts/src/complex.rs b/crates/fonts/src/complex.rs index 0f13b9bd3..a8dd367c3 100644 --- a/crates/fonts/src/complex.rs +++ b/crates/fonts/src/complex.rs @@ -108,7 +108,7 @@ fn shape_chars(primary: Option<&Face>, chars: &[(char, bool, bool)], height: f64 for p in contours.iter_mut().flatten() { *p += Vec2::new(dy * shear, dy); } - out.glyphs.push(contours); + out.push_glyph(contours, None); } let adv = g.advance * k; let (u, o) = flags(g.cluster); @@ -220,6 +220,59 @@ fn shape_run(face: &Face, cs: &[char], r: Range, rtl: bool) -> Vec mod tests { use super::*; + /// A single empty glyph with a positive advance, without relying on installed fonts. + fn empty_glyph_face() -> Face { + let mut head = vec![0_u8; 54]; + head[..4].copy_from_slice(&0x0001_0000_u32.to_be_bytes()); + head[18..20].copy_from_slice(&1000_u16.to_be_bytes()); + let mut hhea = vec![0_u8; 36]; + hhea[..4].copy_from_slice(&0x0001_0000_u32.to_be_bytes()); + hhea[4..6].copy_from_slice(&800_i16.to_be_bytes()); + hhea[6..8].copy_from_slice(&(-200_i16).to_be_bytes()); + hhea[34..36].copy_from_slice(&1_u16.to_be_bytes()); + let tables = [ + (*b"head", head), + (*b"hhea", hhea), + (*b"hmtx", vec![1, 244, 0, 0]), // Advance 500, left side bearing 0. + (*b"maxp", vec![0, 0, 80, 0, 0, 1]), // Version 0.5, one glyph. + ]; + let mut bytes = vec![0_u8; 12 + tables.len() * 16]; + bytes[..4].copy_from_slice(&0x0001_0000_u32.to_be_bytes()); + bytes[4..6].copy_from_slice(&4_u16.to_be_bytes()); + bytes[6..8].copy_from_slice(&64_u16.to_be_bytes()); + bytes[8..10].copy_from_slice(&2_u16.to_be_bytes()); + for (i, (tag, data)) in tables.into_iter().enumerate() { + while !bytes.len().is_multiple_of(4) { + bytes.push(0); + } + let record = 12 + i * 16; + let offset = u32::try_from(bytes.len()).unwrap(); + let len = u32::try_from(data.len()).unwrap(); + bytes[record..record + 4].copy_from_slice(&tag); + bytes[record + 8..record + 12].copy_from_slice(&offset.to_be_bytes()); + bytes[record + 12..record + 16].copy_from_slice(&len.to_be_bytes()); + bytes.extend(data); + } + Face::new(std::sync::Arc::new(bytes), 0).expect("synthetic font parses") + } + + /// Empty complex glyphs retain spacing and underlines without adding geometry or text slots. + #[test] + fn empty_complex_glyph_keeps_advance_without_geometry() { + let face = empty_glyph_face(); + let shaped = shape(Some(&face), " ", 1.0, 1.0, 0.0).expect("shapes empty glyph"); + assert!(shaped.width > 0.0, "the font's advance is retained"); + assert!(shaped.is_empty()); + assert!(shaped.glyphs.is_empty()); + assert!(shaped.text.is_empty()); + + let underlined = shape(Some(&face), "%%u ", 1.0, 1.0, 0.0).expect("shapes decorated empty glyph"); + assert_eq!(underlined.width, shaped.width); + assert!(underlined.glyphs.is_empty()); + assert!(underlined.text.is_empty()); + assert!(!underlined.strokes.is_empty(), "decoration spans survive an empty outline"); + } + #[test] fn stroke_font_rtl_lines_keep_latin_and_digits_in_the_stroke_font() { // With or without installed fonts: the Persian words are fallback glyphs or From 8f7421737ddaede4a3a80fab8ce73aeadb5af701 Mon Sep 17 00:00:00 2001 From: Jonas <80707953+jonascloud@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:13:49 +0100 Subject: [PATCH 18/31] fix(ui): keep layout tabs stable when toggling Tool Sets (#2008) Co-authored-by: cloudj --- crates/ui-egui/src/chrome.rs | 5 +- crates/ui-egui/tests/it/chrome_geometry.rs | 54 +++++++++++++++++++++- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/chrome.rs b/crates/ui-egui/src/chrome.rs index 893379f35..0e79f8444 100644 --- a/crates/ui-egui/src/chrome.rs +++ b/crates/ui-egui/src/chrome.rs @@ -298,7 +298,8 @@ fn visible_tabs(widths: &[f32], active: Option, room: f32) -> std::ops::R start..active + 1 } -/// Status bar: layout tabs on the left, coordinates and drafting toggles on the right. +/// Status bar: layout tabs follow the left-hand layout actions, independent of palette docking. +/// Coordinates and drafting toggles remain on the right. pub fn status_bar(app: &mut CadApp, ui: &mut egui::Ui) { let t = Tokens::get(); egui::Panel::bottom("cc_status").exact_size(26.0).frame(egui::Frame::NONE.fill(t.chrome)).show(ui, |ui| { @@ -328,7 +329,7 @@ pub fn status_bar(app: &mut CadApp, ui: &mut egui::Ui) { } x += 22.0; } - x = x.max(r.left() + if app.ui.show_toolsets { 232.0 } else { 60.0 }); + x += 8.0; // Right side: toggles, right-aligned. let s = app.session.settings.clone(); let anno_all = app.session.doc().is_ok_and(|d| d.anno_all_visible()); diff --git a/crates/ui-egui/tests/it/chrome_geometry.rs b/crates/ui-egui/tests/it/chrome_geometry.rs index ab9c5b8fa..320cabf37 100644 --- a/crates/ui-egui/tests/it/chrome_geometry.rs +++ b/crates/ui-egui/tests/it/chrome_geometry.rs @@ -1,4 +1,5 @@ -//! The real app's painted geometry at native and compact window sizes. +//! The real app's painted geometry at native and compact window sizes. Layout tabs stay anchored +//! to the status bar independently of palette visibility and docking, and remain interactive. use cadcraft_engine::Session; use cadcraft_geom::Vec2; @@ -83,6 +84,57 @@ fn status_labels_never_overlap_at_standard_and_compact_sizes() { assert!(overlaps.is_empty(), "{}", overlaps.join("\n")); } +#[test] +fn layout_tabs_stay_anchored_when_toolsets_toggle_resize_or_move() { + for scale in [1.0, 2.0] { + let mut h = harness(1200.0, 760.0, scale, false); + let tab_rects = + |h: &Harness<'_, CadApp>| ["Model", "Layout1", "Layout2"].map(|name| h.get_by_role_and_label(egui::accesskit::Role::Tab, name).rect()); + let original = tab_rects(&h); + for (command, params) in [ + ("ui.toggle.toolsets", serde_json::json!({})), + ("ui.toggle.toolsets", serde_json::json!({})), + ("ui.dock", serde_json::json!({"operation":"resizeSplit", "path":[], "size":{"FixedFirst":360.0}})), + ("ui.dock", serde_json::json!({"operation":"float", "panel":"toolSets", "rect":[400,120,260,480]})), + ("ui.dock", serde_json::json!({"operation":"move", "panel":"toolSets", "target":"canvas", "zone":"right"})), + ("ui.toggle.toolsets", serde_json::json!({})), + ("ui.toggle.toolsets", serde_json::json!({})), + ("ui.dock.reset", serde_json::json!({})), + ] { + h.state_mut().run(command, params.clone()).unwrap(); + h.run_steps(5); + assert_eq!(tab_rects(&h), original, "{command} {params} at scale {scale} must not move the tabs"); + } + // A fixed but oversized blank reservation would stop the jump without fixing the layout. + let gap = original[0].left() - h.get_by_label("Layout list").rect().right(); + assert!((0.0..=16.0).contains(&gap), "tabs must follow the layout actions, not reserve palette space: {gap}"); + } +} + +#[test] +fn layout_tabs_and_add_button_work_with_toolsets_shown_or_hidden() { + for visible in [true, false] { + let mut h = harness(1400.0, 760.0, 1.0, false); + h.state_mut().run("ui.toggle.toolsets", serde_json::json!({"on":visible})).unwrap(); + h.run_steps(5); + h.get_by_role_and_label(egui::accesskit::Role::Tab, "Layout1").click(); + h.run_steps(3); + assert_eq!(h.state().session.state().unwrap().space, cadcraft_engine::doc::Space::Paper("Layout1".into())); + h.get_by_role_and_label(egui::accesskit::Role::Tab, "Model").click(); + h.run_steps(3); + assert_eq!(h.state().session.state().unwrap().space, cadcraft_engine::doc::Space::Model); + // There are two add buttons; exercise the one between Model and the paper-layout tabs. + let model = h.get_by_role_and_label(egui::accesskit::Role::Tab, "Model").rect(); + h.get_all_by_label("New layout").find(|node| node.rect().left() > model.right()).unwrap().click(); + h.run_steps(3); + assert_eq!(h.state().session.state().unwrap().space, cadcraft_engine::doc::Space::Paper("Layout3".into())); + assert_eq!(h.state().session.doc().unwrap().layouts.len(), 3); + h.get_by_role_and_label(egui::accesskit::Role::Tab, "Layout2").click(); + h.run_steps(3); + assert_eq!(h.state().session.state().unwrap().space, cadcraft_engine::doc::Space::Paper("Layout2".into())); + } +} + #[test] fn compact_toolbar_overflow_keeps_help_reachable() { let mut h = harness(900.0, 560.0, 1.0, false); From 5acec7d0be686899f26a32e10532c9c2bdecce39 Mon Sep 17 00:00:00 2001 From: Nicholas Gower Date: Sun, 25 Oct 2026 09:13:53 -0700 Subject: [PATCH 19/31] PLOTWINDOW: pick both corners even with ORTHO on (#1893) * PLOTWINDOW: pick both corners even with ORTHO on PLOTWINDOW's opposite-corner prompt used Prompt::base, so the canvas bent the picked point onto the nearest axis with ORTHOMODE on: the second corner landed above/beside the first, the window had no width or height, and set_plot_window refused it. The prompt now uses Prompt::corner, the flag ZOOM Window, RECTANG and ARRAYRECT already use to keep a rectangle's opposite corner free of ortho. The first corner sets the base as before, so the rubber band and direct distance entry still work. Regression test drives the interactive machine through snap_cursor with ORTHOMODE on, picks two non-axis-aligned corners and checks the stored window keeps both extents; an ordinary LINE prompt still follows ortho. Fixes #1641 * docs: document the functions touched by #1641 --- crates/engine/src/cmd/plotters.rs | 2 ++ crates/engine/src/prompt.rs | 4 +-- crates/engine/tests/plotters.rs | 49 +++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/plotters.rs b/crates/engine/src/cmd/plotters.rs index 762f65066..a3e1b59f1 100644 --- a/crates/engine/src/cmd/plotters.rs +++ b/crates/engine/src/cmd/plotters.rs @@ -362,6 +362,8 @@ impl Interactive for PlotWindowM { fn name(&self) -> &'static str { "PLOTWINDOW" } + /// Ask for the first corner, then for the opposite one marked as a rectangle corner + /// (`Prompt::corner`), so ORTHO does not bend the pick and the window keeps its area. fn prompt(&self, _s: &Session) -> Prompt { match self.first { None => Prompt::new("Specify first corner", Accept::POINT), diff --git a/crates/engine/src/prompt.rs b/crates/engine/src/prompt.rs index 63883240c..c02551b80 100644 --- a/crates/engine/src/prompt.rs +++ b/crates/engine/src/prompt.rs @@ -41,8 +41,8 @@ pub struct Prompt { /// prompt (a point with no base yet, e.g. the first point of a line). Elsewhere a deferred /// pick arrives as the plain point it was picked at. pub deferred: bool, - /// The base is a rectangle's first corner (ZOOM Window, RECTANG, ARRAYRECT's unit cell): ortho - /// does not bend the opposite corner, so the rectangle keeps its area. + /// The base is a rectangle's first corner (ZOOM Window, PLOTWINDOW, RECTANG, ARRAYRECT's unit + /// cell): ortho does not bend the opposite corner, so the rectangle keeps its area. #[serde(skip_serializing_if = "std::ops::Not::not")] pub corner: bool, /// The grip this prompt stretches (one hot grip at the STRETCH prompt), as (object, grip diff --git a/crates/engine/tests/plotters.rs b/crates/engine/tests/plotters.rs index 43268d38a..81aea1221 100644 --- a/crates/engine/tests/plotters.rs +++ b/crates/engine/tests/plotters.rs @@ -6,6 +6,8 @@ use cadcraft_doc::{Drawing, Space}; use cadcraft_engine::Session; use cadcraft_engine::cmd::file::{IoHooks, base64_decode, set_io}; use cadcraft_engine::cmd::plotters::{PlotterStore, set_plotter_store}; +use cadcraft_engine::snap::tracking::CursorQuery; +use cadcraft_engine::{Input, geom::Vec2}; use serde_json::{Value, json}; /// The plot hook echoes the options it gets. @@ -13,6 +15,7 @@ fn echo_plot(_: &Drawing, _: &Space, opts: &Value) -> Result, String> { Ok(opts.to_string().into_bytes()) } +/// A session with in-memory plotters and I/O hooks, holding one line so layouts have content. fn session() -> Session { set_plotter_store(PlotterStore::memory()); set_io(IoHooks { read: |_, _| Err("no reader in this test".into()), write: |_, _| Err("no writer in this test".into()), plot: Some(echo_plot) }); @@ -22,6 +25,7 @@ fn session() -> Session { s } +/// The device names the PLOTTERMANAGER report lists, in the order it lists them. fn names(s: &mut Session) -> Vec { let r = s.execute("plottermanager", &json!({})).unwrap(); r["devices"].as_array().unwrap().iter().map(|d| d["name"].as_str().unwrap().to_string()).collect() @@ -34,6 +38,8 @@ fn plotted(r: Value) -> Value { } #[test] +/// PLOTTERMANAGER creates, edits, saves, loads and deletes user devices; built-ins are fixed, +/// duplicate names and papers with margins larger than the page are refused. fn manage_devices() { let mut s = session(); let n = names(&mut s); @@ -73,6 +79,8 @@ fn manage_devices() { } #[test] +/// PAGESETUP copies the device's papers and margins into the layout's page, keeps a paper the new +/// device also has, and falls back to the device default when it does not. fn page_setup_takes_a_device_and_its_papers() { let mut s = session(); s.execute("plottermanager", &json!({"action": "new", "name": "Plan Printer", "output": "pdf"})).unwrap(); @@ -110,6 +118,8 @@ fn page_setup_takes_a_device_and_its_papers() { } #[test] +/// PLOT and EXPORTPDF hand the hook the device's output, resolution, paper and margins, and a +/// layout plots with its own page setup; unknown devices and papers are refused. fn plot_uses_the_device() { let mut s = session(); s.execute("plottermanager", &json!({"action": "new", "name": "Sketch PNG", "output": "png", "resolution": 96})).unwrap(); @@ -136,6 +146,7 @@ fn plot_uses_the_device() { } #[test] +/// The typed -PLOTTERMANAGER prompt machine creates a device, adds a paper to it and deletes it. fn typed_plotter_manager() { let mut s = session(); for line in ["-plottermanager", "New", "Typed Plotter", "Svg"] { @@ -155,6 +166,8 @@ fn typed_plotter_manager() { } #[test] +/// Hostile names and values (empty or path-like names, unknown output, huge resolution, negative +/// or enormous paper sizes, oversized loads) are refused instead of panicking. fn hostile_names_and_values_are_refused() { let mut s = session(); let r = s.execute("plottermanager", &json!({"action": "new", "name": "../../etc/passwd"})).unwrap(); @@ -174,6 +187,8 @@ fn hostile_names_and_values_are_refused() { } #[test] +/// PLOTWINDOW keeps both picked corners for the plot dialog, previews the rubber-band rectangle +/// after the first pick, and refuses a window with no area (JSON form included). fn plotwindow_keeps_the_picked_window_for_the_plot_dialog() { let mut s = Session::new(); // Typed: two corners, with the window's rectangle following the cursor after the first. @@ -190,3 +205,37 @@ fn plotwindow_keeps_the_picked_window_for_the_plot_dialog() { assert_eq!(r["window"], json!([[0.0, 0.0], [3.0, 4.0]])); assert!(s.execute("plotwindow", &json!({"p1": [0, 0], "p2": [3, 0]})).is_err()); } + +/// Snap `raw` at the current prompt the way the canvas does, so a test sees the pick ORTHO and +/// object snap would really produce. +fn hover(s: &mut Session, raw: Vec2) -> Vec2 { + let p = s.current_prompt().expect("a prompt"); + let q = CursorQuery { base: p.base, deferred: p.deferred, corner: p.corner, aperture: 1e-6, tol: 1e-6, now: 0.0, shift: false }; + s.snap_cursor(raw, &q).point +} + +#[test] +/// Regression (#1641): with ORTHO on, PLOTWINDOW's opposite corner stays where the cursor is so +/// the window keeps both extents, while an ordinary point prompt still follows ortho. +fn plotwindow_picks_both_corners_with_ortho_on() { + let mut s = Session::new(); + s.execute("setvar", &json!({"name": "OSMODE", "value": 0})).unwrap(); + s.execute("setvar", &json!({"name": "ORTHOMODE", "value": 1})).unwrap(); + s.cmdline("plotwindow").unwrap(); + assert_eq!(s.current_prompt().unwrap().message, "Specify first corner"); + let a = hover(&mut s, Vec2::new(10.0, 20.0)); + s.input(Input::Point(a)).unwrap(); + assert_eq!(s.current_prompt().unwrap().message, "Specify opposite corner"); + let c = hover(&mut s, Vec2::new(40.0, 5.0)); + assert!(c.near(Vec2::new(40.0, 5.0), 1e-9), "the opposite corner is where the cursor is: {c:?}"); + s.input(Input::Point(c)).unwrap(); + assert!(s.running.is_none()); + let w = s.plot_window.expect("the picked window"); + assert!(w.width() > 0.0 && w.height() > 0.0, "the window has both extents: {w:?}"); + assert_eq!((w.min.x, w.min.y, w.max.x, w.max.y), (10.0, 5.0, 40.0, 20.0)); + + // An ordinary point prompt still follows ortho: a line's next point stays on its axis. + s.cmdline("line 0,0").unwrap(); + let c = hover(&mut s, Vec2::new(10.0, 4.0)); + assert!(c.near(Vec2::new(10.0, 0.0), 1e-9), "ortho keeps the line horizontal: {c:?}"); +} From 63a5569970afd4dc037cf7c0f44556c90ef3a73a Mon Sep 17 00:00:00 2001 From: Nicholas Gower Date: Sun, 25 Oct 2026 09:13:56 -0700 Subject: [PATCH 20/31] In-place text editing accepts IME composition and commit (#1905) * ui: accept IME composition and commit in the command line (#1761) The command line is a custom buffer with keyboard focus staying on the drawing, so no egui TextEdit ever owned the IME: egui never published an IME output, winit never called set_ime_allowed(true), and a Windows IME (Microsoft Pinyin, Japanese, Korean) could not even switch on. Even when IME events arrived they were ignored, so in-place single-line TEXT edits (and every other text prompt) accepted ASCII only. - Publish an IME output (command-line rect and caret) while a prompt asks for text, enabling the OS IME; release it when the prompt no longer collects text, so ASCII typing and shortcuts keep their behaviour. - Handle Event::Ime: Preedit is drawn as an underlined composition after the buffer but never saved; Commit inserts the committed string once. Newline/Return commits are ignored like egui's own text fields, and Enter/Escape/submit clear any composition. Tests: a headless command-line test asserts the IME is enabled for a TEXTEDIT prompt, the preedit stays out of the buffer, the commit lands exactly once and the text entity changes; a kittest double-click test edits TEXT in place with IME preedit and commit. * cmdline IME: build the IMEOutput with egui 0.37's constructor and a context generation Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Brandon Thomas Co-authored-by: Claude Opus 5.5 --- crates/ui-egui/src/cmdline.rs | 109 ++++++++++++++++++- crates/ui-egui/tests/it/text_double_click.rs | 21 +++- docs/localization-parity.md | 12 +- 3 files changed, 134 insertions(+), 8 deletions(-) diff --git a/crates/ui-egui/src/cmdline.rs b/crates/ui-egui/src/cmdline.rs index e9e0db8c7..d78561551 100644 --- a/crates/ui-egui/src/cmdline.rs +++ b/crates/ui-egui/src/cmdline.rs @@ -33,6 +33,16 @@ pub struct CmdLine { pub option_rows: Vec<(Rect, String)>, /// Where the Dynamic Input tooltip stays while the options menu is open. pub options_anchor: Option, + /// The IME's composition (pre-edit) text: shown after the buffer while composing, never saved. + pub preedit: String, + /// The command line's editing rectangle and caret (screen) from the last frame, for the IME's + /// composition window (see `enable_ime`). + pub ime_rect: Option<(Rect, Rect)>, + /// The command line's IME context generation: a new one each time it starts collecting text, + /// as egui asks of an IME owner that (re)gains focus. + ime_generation: u64, + /// Whether the command line published an IME context last frame. + ime_on: bool, } /// The most history lines the command line shows above its bar. @@ -247,9 +257,36 @@ pub struct KeyPress { pub plain: bool, } +/// Let the operating system's IME compose into the command line while a prompt asks for text. +/// Nothing owns keyboard focus there (typing goes to the command line from anywhere), so egui +/// never enables the IME by itself: publishing an IME output does it, with the command line's +/// rectangle and caret for the composition window. Without this a Windows IME (Pinyin, Japanese, +/// Korean...) cannot even switch on (#1761). +fn enable_ime(app: &mut CadApp, ctx: &egui::Context) { + if !app.cmd.ime_on { + app.cmd.ime_on = true; + app.cmd.ime_generation = app.cmd.ime_generation.wrapping_add(1); + } + let (rect, cursor_rect) = app.cmd.ime_rect.unwrap_or_else(|| { + let r = ctx.content_rect(); + (r, Rect::from_min_size(pos2(r.left() + 30.0, r.bottom() - 34.0), vec2(1.0, 18.0))) + }); + ctx.output_mut(|o| { + o.ime = Some(egui::output::IMEOutput::new( + egui::Id::unique("cc_cmdline_ime"), + app.cmd.ime_generation, + egui::IMEPurpose::Normal, + rect, + cursor_rect, + )); + }); +} + /// Handle keyboard input destined for the command line (when no text field has focus). pub fn keyboard(app: &mut CadApp, ctx: &egui::Context) { if ctx.egui_wants_keyboard_input() { + app.cmd.preedit.clear(); + app.cmd.ime_on = false; return; } let events = ctx.input(|i| i.events.clone()); @@ -257,6 +294,13 @@ pub fn keyboard(app: &mut CadApp, ctx: &egui::Context) { app.cmd.watch_selection(&selection); let mac = ctx.os() == egui::os::OperatingSystem::Mac; let text_prompt = app.session.current_prompt().is_some_and(|p| p.accept.text && !p.accept.point && !p.accept.number); + if text_prompt { + enable_ime(app, ctx); + } else { + // No prompt is collecting text: a composition left over from one cannot belong here. + app.cmd.preedit.clear(); + app.cmd.ime_on = false; + } options_still_fit(app); for ev in events { // The options menu takes the arrows, Enter and Escape; typing closes it. @@ -297,13 +341,26 @@ pub fn keyboard(app: &mut CadApp, ctx: &egui::Context) { } else if t == " " && !text_prompt { submit(app); } else { + app.cmd.preedit.clear(); app.cmd.buffer.push_str(&t); } } + // IME composition: the pre-edit is shown but stays out of the buffer; the commit is + // the only text that is inserted. Enter/Return commits arrive separately as keys, so + // a newline commit is ignored (as egui's own text fields do) instead of submitting. + egui::Event::Ime(egui::ImeEvent::Preedit { text, .. }) => app.cmd.preedit = text, + egui::Event::Ime(egui::ImeEvent::Commit(text)) => { + app.cmd.preedit.clear(); + if text != "\n" && text != "\r" { + app.cmd.buffer.push_str(&text); + } + } + egui::Event::Ime(_) => {} egui::Event::Key { key, pressed: true, repeat, modifiers, .. } => match key { Key::Enter => submit(app), Key::Escape => { app.cmd.buffer.clear(); + app.cmd.preedit.clear(); app.session.cancel(); } Key::Backspace => { @@ -357,6 +414,7 @@ pub fn keyboard(app: &mut CadApp, ctx: &egui::Context) { } pub fn submit(app: &mut CadApp) { + app.cmd.preedit.clear(); // Grip dimensional input: the typed lengths and angles place the grip. if crate::grip_dyn::submit(app) { return; @@ -493,14 +551,26 @@ pub fn show(app: &mut CadApp, ui: &mut egui::Ui, canvas: Rect) { } } } - let g = p.layout_no_wrap(app.cmd.buffer.clone(), font, t.text); + let g = p.layout_no_wrap(app.cmd.buffer.clone(), font.clone(), t.text); let gw = g.size().x; p.galley(pos2(x, bar.center().y - g.size().y / 2.0), g, t.text); + let mut caret_x = x + gw; + // The IME's composition text sits after the buffer (underlined) until it is committed. + if !app.cmd.preedit.is_empty() { + let pg = p.layout_no_wrap(app.cmd.preedit.clone(), font, t.text); + let pw = pg.size().x; + p.galley(pos2(caret_x, bar.center().y - pg.size().y / 2.0), pg, t.text); + p.line_segment([pos2(caret_x, bar.bottom() - 7.0), pos2(caret_x + pw, bar.bottom() - 7.0)], Stroke::new(1.0, t.accent)); + caret_x += pw; + } // Caret. let blink = (ui.input(|i| i.time) * 2.0).floor() as i64 % 2 == 0; if blink { - p.line_segment([pos2(x + gw + 1.0, bar.top() + 5.0), pos2(x + gw + 1.0, bar.bottom() - 5.0)], Stroke::new(1.0, t.text)); + p.line_segment([pos2(caret_x + 1.0, bar.top() + 5.0), pos2(caret_x + 1.0, bar.bottom() - 5.0)], Stroke::new(1.0, t.text)); } + // Where the IME's composition window follows: the whole command line and the caret in it. + let caret = Rect::from_min_size(pos2(caret_x + 1.0, bar.top() + 5.0), vec2(1.0, h - 10.0)); + app.cmd.ime_rect = Some((bar, caret)); // History chevron. crate::icons::paint( &p, @@ -668,19 +738,52 @@ mod tests { assert_eq!(app.ui.history_lines, 12); } - fn frame(app: &mut CadApp, ctx: &egui::Context, events: Vec) { + fn frame(app: &mut CadApp, ctx: &egui::Context, events: Vec) -> egui::FullOutput { let raw = egui::RawInput { screen_rect: Some(Rect::from_min_size(Pos2::ZERO, vec2(1400.0, 900.0))), events, ..Default::default() }; let mut out = ctx.run_ui(raw, |ui| { app.logic(ui.ctx()); app.ui(ui); }); out.textures_delta.clear(); + out } fn press(app: &mut CadApp, ctx: &egui::Context, at: Pos2, pressed: bool) { frame(app, ctx, vec![Event::PointerButton { pos: at, button: PointerButton::Primary, pressed, modifiers: egui::Modifiers::default() }]); } + #[test] + fn ime_preedit_is_shown_and_its_commit_is_inserted_once() { + use cadcraft_engine::geom::Vec2; + let mut app = CadApp::new(Session::new(), crate::Services::default()); + app.run("text", serde_json::json!({"at": [0, 0], "text": "HELLO", "height": 2.5})).unwrap(); + app.start("textedit"); + app.session.input(Input::Point(Vec2::new(1.0, 1.0))).unwrap(); + assert_eq!(app.session.current_prompt().map(|p| p.message.clone()).as_deref(), Some("Enter new text")); + let ctx = egui::Context::default(); + let out = frame(&mut app, &ctx, Vec::new()); + // The IME is enabled for the command line while it is collecting text. + assert!(out.platform_output.ime.is_some()); + // Composition is shown at the command line but is not part of the text being edited. + frame(&mut app, &ctx, vec![Event::Ime(egui::ImeEvent::Preedit { text: "zhong".into(), active_range_chars: None })]); + assert_eq!(app.cmd.preedit, "zhong"); + assert!(app.cmd.buffer.is_empty()); + // The committed text replaces the pre-edit, exactly once. + frame(&mut app, &ctx, vec![Event::Ime(egui::ImeEvent::Commit("中文".into()))]); + assert!(app.cmd.preedit.is_empty()); + assert_eq!(app.cmd.buffer, "中文"); + let enter = |pressed| Event::Key { key: Key::Enter, physical_key: None, pressed, repeat: false, modifiers: egui::Modifiers::NONE }; + frame(&mut app, &ctx, vec![enter(true), enter(false)]); + let text = app.session.doc().unwrap().model.iter().next().map(|e| match &e.kind { + cadcraft_engine::doc::EntityKind::Text(t) => t.value.clone(), + _ => String::new(), + }); + assert_eq!(text.as_deref(), Some("中文")); + // Back to selecting objects: the next frame lets the IME go, so it can't swallow ASCII typing. + let out = frame(&mut app, &ctx, Vec::new()); + assert!(out.platform_output.ime.is_none()); + } + #[test] fn history_expands_by_click_chevron_f2_and_resizes_by_drag_without_touching_the_drawing() { let mut app = CadApp::new(Session::empty(), crate::Services::default()); diff --git a/crates/ui-egui/tests/it/text_double_click.rs b/crates/ui-egui/tests/it/text_double_click.rs index 782234d10..5cedfbf61 100644 --- a/crates/ui-egui/tests/it/text_double_click.rs +++ b/crates/ui-egui/tests/it/text_double_click.rs @@ -4,7 +4,7 @@ use cadcraft_engine::doc::EntityKind; use cadcraft_engine::geom::Vec2; use cadcraft_engine::{Input, Session}; use cadcraft_ui_egui::{CadApp, Services}; -use egui::{Event, Modifiers, PointerButton, Pos2, vec2}; +use egui::{Event, ImeEvent, Key, Modifiers, PointerButton, Pos2, vec2}; use egui_kittest::Harness; use serde_json::json; @@ -68,6 +68,25 @@ fn double_click_on_text_starts_textedit_for_that_object() { } } +#[test] +fn double_clicked_text_takes_ime_composition_and_commit() { + let mut h = harness(); + double_click(&mut h, Vec2::new(4.0, 1.2)); + assert_eq!(h.state().session.current_prompt().unwrap().message, "Enter new text"); + // Composition is shown, not saved; the commit is inserted exactly once (#1761). + h.event(Event::Ime(ImeEvent::Preedit { text: "zhong".into(), active_range_chars: None })); + h.run_steps(1); + assert_eq!(h.state().cmd.preedit, "zhong"); + assert!(h.state().cmd.buffer.is_empty()); + h.event(Event::Ime(ImeEvent::Commit("中文".into()))); + h.run_steps(1); + assert!(h.state().cmd.preedit.is_empty()); + assert_eq!(h.state().cmd.buffer, "中文"); + h.key_press(Key::Enter); + h.run_steps(1); + assert_eq!(text_at(&h, 0), "中文"); +} + #[test] fn double_click_elsewhere_starts_no_command() { for at in [Vec2::new(30.0, 5.0), Vec2::new(15.0, 30.0)] { diff --git a/docs/localization-parity.md b/docs/localization-parity.md index 4f4964a41..b4ff05f68 100644 --- a/docs/localization-parity.md +++ b/docs/localization-parity.md @@ -1,6 +1,6 @@ # Localization parity -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-19 · **Change:** minor (Brazilian Portuguese row updated and terminology reviewed; previously: Swedish catalog completed with the 95 strings added since; previously: Swedish catalog added: complete catalog, menu headings, native-speaker terminology review; previously: complete current Korean catalog with Hangul font coverage) · **Target:** Autodesk AutoCAD 2027 for Mac (26.0) +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-22 · **Change:** minor (the command line enables the OS IME and handles composition/commit, so single-line TEXT edits accept CJK input; previously: Brazilian Portuguese row updated and terminology reviewed; previously: Swedish catalog completed with the 95 strings added since; previously: Swedish catalog added: complete catalog, menu headings, native-speaker terminology review; previously: complete current Korean catalog with Hangul font coverage) · **Target:** Autodesk AutoCAD 2027 for Mac (26.0) How far CADCraft's interface and drawing text are from AutoCAD's languages. Summarised in [ROADMAP.md](../ROADMAP.md#languages); work items in [gaps.md](gaps.md#localization). @@ -37,9 +37,12 @@ shaping and right-to-left text through TrueType, vertical SHX text styles. lam-alef, ZWNJ, marks; Hebrew), in TEXT and MTEXT including wrapping (#287). Left-to-right lines are still placed one glyph per code point: **no shaping** for Devanagari and other Indic scripts, and **no vertical text**. SHX fonts are not read; our stroke font stands in. -- **Text input:** egui/winit IME. Several upstream IME fixes are queued as PRs (#110–#119: - macOS Korean, emoji picker, Windows caret placement, X11 and Wayland), so CJK input is not yet - reliable on every platform. +- **Text input:** egui/winit IME. The command line — which collects single-line TEXT edits and + other prompt text — now enables the operating system IME while a prompt asks for text and shows + composition (pre-edit) text until it is committed, so Chinese, Japanese and Korean input works + there (#1761); egui's own text fields handle IME themselves. Several upstream IME fixes are + queued as PRs (#110–#119: macOS Korean, emoji picker, Windows caret placement, X11 and Wayland), + so CJK input is not yet reliable on every platform. ## Languages @@ -86,6 +89,7 @@ each language, which needs humans. Translations parallelise fully once the catal | Date | Change | Summary | |---|---|---| +| 2026-10-22 | minor | Command line IME: enabled while a prompt collects text, composition shown but not saved, commit inserted once; single-line TEXT edits accept CJK input (#1761) | | 2026-10-19 | minor | Brazilian Portuguese: row brought up to date (it still read 0%); native-speaker terminology pass on 69 rows (Bind as Integrar, prompt keywords and SPDX kept in English, Thaw Layer, Snap type, Revolve fixed; snap, viewport, grip, offset, layer on/off and multiline wording made consistent with Brazilian usage) | | 2026-10-15 | minor | Greek (`el`) catalog: 1,330 rows, context/plural/locale/font tests; review pending | | 2026-10-14 | minor | Swedish completed: 95 rows for strings added since (Hatch and Gradient dialog, Block Editor, REFEDIT, multileader editing, Options selection settings, MTEXT editor, recent files); 1,330 rows, every key of the Ukrainian reference catalog | From 1c85d339c3e2b0f953ce0f32c4ef8688bc39cd72 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:15:42 +0100 Subject: [PATCH 21/31] Reject negative or out-of-range minutes/seconds in D'" angles (#2116) Found while testing crates/engine/src/units.rs:333-340 (parse_dms) Co-authored-by: Claude Haiku 5.5 Co-authored-by: Brandon Thomas --- crates/engine/src/units.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/units.rs b/crates/engine/src/units.rs index a3018d8f3..0a18a0039 100644 --- a/crates/engine/src/units.rs +++ b/crates/engine/src/units.rs @@ -327,6 +327,12 @@ fn bearing(t: &str) -> Option { Some(deg.to_radians()) } +/// Minutes or seconds of a D'" angle: a number in `0..=60`. +fn dms_part(t: &str) -> Option { + let v: f64 = t.trim().parse().ok()?; + (0.0..=60.0).contains(&v).then_some(v) +} + /// Degrees, `45d`, `45d30'` or `45d30'15"` (a leading `-` negates the whole angle). fn parse_dms(t: &str) -> Option { let (neg, t) = match t.trim().strip_prefix('-') { @@ -339,11 +345,11 @@ fn parse_dms(t: &str) -> Option { if !rest.is_empty() { let (m, rest2) = rest.split_once('\'').unwrap_or((rest, "")); if !m.trim().is_empty() { - deg += m.trim().parse::().ok()? / 60.0; + deg += dms_part(m)? / 60.0; } let s = rest2.trim().trim_end_matches('"'); if !s.is_empty() { - deg += s.parse::().ok()? / 3600.0; + deg += dms_part(s)? / 3600.0; } } let deg = if neg { -deg } else { deg }; @@ -363,6 +369,14 @@ pub const DEG: f64 = PI / 180.0; mod tests { use super::*; + #[test] + fn dms_minutes_and_seconds_must_be_within_0_to_60() { + for t in ["45d-30'", "45d30'-15\"", "45d75'", "45d30'75\"", "n45d-30'e"] { + assert_eq!(parse_angle(t), None, "{t}"); + } + assert!(parse_angle("45d30'15\"").is_some()); + } + #[test] fn engineering_huge_distance_falls_back_to_decimal() { assert_eq!(format_distance(1e30, 3, 2), format!("{:.2}", 1e30)); From cddf09beb2818e3157d51c7c9497df755d99beca Mon Sep 17 00:00:00 2001 From: asimfali Date: Sun, 25 Oct 2026 19:25:21 +0300 Subject: [PATCH 22/31] DXF/DWG: read the drawing's WIPEOUTFRAME from its WIPEOUTVARIABLES object (#908) AutoCAD stores the wipeout frame setting in the drawing's WIPEOUTVARIABLES object (group 70: 0 = frames hidden). CADCraft ignored it and always showed the frames (the default of 1), so masks such as the diamond inside a numbered circle were outlined. The flag is now read into the WIPEOUTFRAME setting. Co-authored-by: simanov Co-authored-by: Claude Sonnet 5.5 --- crates/io/src/dxf_read.rs | 6 ++++++ crates/io/tests/wipeout_frame.rs | 21 +++++++++++++++++++++ 2 files changed, 27 insertions(+) create mode 100644 crates/io/tests/wipeout_frame.rs diff --git a/crates/io/src/dxf_read.rs b/crates/io/src/dxf_read.rs index 6a3ea222d..e71f7725f 100644 --- a/crates/io/src/dxf_read.rs +++ b/crates/io/src/dxf_read.rs @@ -1984,6 +1984,8 @@ struct Objects { layout_views: Vec<(String, (Vec2, f64))>, /// SORTENTSTABLE objects: (owning block record handle, entity handle → sort handle). sortents: Vec<(String, HashMap)>, + /// WIPEOUTVARIABLES: the drawing's WIPEOUTFRAME (0 = frames hidden, 1 = shown). + wipeout_frame: Option, } const MAX_OBJECTS: usize = 1_000_000; @@ -2022,6 +2024,7 @@ impl Objects { "DIMASSOC" if self.dimassocs.len() < MAX_OBJECTS => self.dimassocs.push(tags.to_vec()), "MLEADERSTYLE" if self.mleader_styles.len() < MAX_OBJECTS => self.mleader_styles.push((h, tags.to_vec())), "GROUP" if self.groups.len() < MAX_OBJECTS => self.groups.push((h, tags.to_vec())), + "WIPEOUTVARIABLES" => self.wipeout_frame = T(tags).i(70).filter(|v| (0..=2).contains(v)), "SORTENTSTABLE" if self.sortents.len() < MAX_OBJECTS => { if let Some(t) = sortents_table(tags) { self.sortents.push(t); @@ -2052,6 +2055,9 @@ impl Objects { } fn apply(self, d: &mut Drawing, rx: &Rx) { + if let Some(v) = self.wipeout_frame { + d.header.set_i64("WIPEOUTFRAME", v); + } if let Some(style) = self.xrecord_text(crate::dxf_ext::MLEADER_STYLE_KEY).filter(|s| !s.trim().is_empty()) { d.header.set_str("CMLEADERSTYLE", &style); } diff --git a/crates/io/tests/wipeout_frame.rs b/crates/io/tests/wipeout_frame.rs new file mode 100644 index 000000000..5c2c201c9 --- /dev/null +++ b/crates/io/tests/wipeout_frame.rs @@ -0,0 +1,21 @@ +//! The drawing's WIPEOUTVARIABLES object sets WIPEOUTFRAME: AutoCAD files with hidden wipeout +//! frames (group 70 = 0) must not show them. + +fn dxf(frame: Option) -> String { + let mut s = String::from(" 0\nSECTION\n 2\nENTITIES\n 0\nENDSEC\n 0\nSECTION\n 2\nOBJECTS\n"); + if let Some(f) = frame { + s.push_str(&format!(" 0\nWIPEOUTVARIABLES\n 5\nA1\n100\nAcDbWipeoutVariables\n 70\n{f}\n")); + } + s.push_str(" 0\nENDSEC\n 0\nEOF\n"); + s +} + +#[test] +fn wipeoutvariables_sets_the_frame_flag() { + let frame = |f| cadcraft_io::read_dxf(dxf(f).as_bytes()).unwrap().header.i64("WIPEOUTFRAME", 1); + assert_eq!(frame(Some(0)), 0, "frames hidden"); + assert_eq!(frame(Some(1)), 1); + assert_eq!(frame(Some(2)), 2); + assert_eq!(frame(None), 1, "no object: the default shows frames"); + assert_eq!(frame(Some(7)), 1, "an out-of-range flag is ignored"); +} From 0de051049cd4229c87d25d47def3c15913cfc1d3 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:25:24 +0100 Subject: [PATCH 23/31] Keep associative-array parameters when an xref is loaded (#943) (#966) * Keep associative-array parameters when an xref is loaded (#943) * xref load test: initialise Ctx's new bytes counter Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Brandon Thomas Co-authored-by: Claude Opus 5.5 --- crates/engine/src/cmd/blocks/xref/load.rs | 42 +++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/crates/engine/src/cmd/blocks/xref/load.rs b/crates/engine/src/cmd/blocks/xref/load.rs index efe9143a8..fac289f23 100644 --- a/crates/engine/src/cmd/blocks/xref/load.rs +++ b/crates/engine/src/cmd/blocks/xref/load.rs @@ -420,6 +420,17 @@ fn import(d: &mut Drawing, key: &str, x: &Drawing, cx: &mut Ctx) -> Result<(), S nb.anonymous = b.anonymous; nb.explodable = b.explodable; nb.units = b.units; + // Array parameters keep their kind and spacing; block names follow the block renaming, and + // a source edit in progress in the referenced file is not carried over. + nb.array = b.array.as_ref().map(|a| { + let mut a = a.clone(); + a.source = dep(Sym::Block, &a.source).unwrap_or(a.source); + for r in &mut a.replaced { + r.block = dep(Sym::Block, &r.block).unwrap_or_else(|| r.block.clone()); + } + a.editing = None; + a + }); for e in b.entities.iter() { let e = adopt(d, e); nb.entities.push(e); @@ -532,4 +543,35 @@ mod tests { assert!(!has_prefix("SITE", "site|")); assert!(!has_prefix("é", "ab")); } + + #[test] + fn imported_array_blocks_keep_their_parameters() { + use cadcraft_doc::{ArrayKind, ArrayParams, Block, RectArray, ReplacedItem}; + let rect = RectArray { cols: 3, rows: 2, col_spacing: 5.0, row_spacing: 4.0, axis_angle: 0.0, row_elevation: 0.0 }; + let params = ArrayParams { + source: "*U1".into(), + kind: ArrayKind::Rect(rect), + levels: 1, + level_spacing: 0.0, + replaced: vec![ReplacedItem { i: 1, j: 0, level: 0, block: "ALT".into() }], + editing: Some(Default::default()), + }; + let mut arr = Block::new("*U2"); + arr.array = Some(params); + let mut x = Drawing::default(); + x.blocks.insert("*U2".into(), Arc::new(arr)); + + let mut d = Drawing::default(); + d.blocks.insert("SITE".into(), Arc::new(Block::new("SITE"))); + let mut cx = Ctx { chain: Vec::new(), objects: 0, files: 0, bytes: 0, notes: Vec::new() }; + assert!(import(&mut d, "SITE", &x, &mut cx).is_ok()); + + let got = d.blocks.get("SITE|*U2").and_then(|b| b.array.clone()); + let Some(got) = got else { panic!("array parameters were dropped") }; + assert_eq!(got.source, "SITE|*U1"); + assert_eq!(got.replaced[0].block, "SITE|ALT"); + assert!(got.editing.is_none()); + let ArrayKind::Rect(r) = got.kind else { panic!("not a rectangular array") }; + assert_eq!((r.cols, r.rows), (3, 2)); + } } From 2994b52e0357e026addb82cf019e20a541eb0a19 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:25:28 +0100 Subject: [PATCH 24/31] Read dynamic block element tags from the entity's own xdata (#942) (#967) * Read dynamic block element tags from the entity's own xdata (#942) * dynblock test: an untagged object at index 1 is keyed 1_000_001 Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Brandon Thomas Co-authored-by: Claude Opus 5.5 --- crates/doc/src/dynblock/apply.rs | 46 +++++++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/crates/doc/src/dynblock/apply.rs b/crates/doc/src/dynblock/apply.rs index 70ef484eb..ed5281bf6 100644 --- a/crates/doc/src/dynblock/apply.rs +++ b/crates/doc/src/dynblock/apply.rs @@ -29,16 +29,21 @@ fn tag(code: i32, value: impl Into) -> RawTag { RawTag { code, value: value.into() } } -/// The element tag (`AcDbBlockRepETag` 1071) of object `h`, if it has one. -fn etag(d: &Drawing, h: Handle) -> Option { - let x = &d.foreign.entities.get(&h)?.xdata; +/// The element tag (`AcDbBlockRepETag` 1071) in `x`, if it has one. +fn etag_in(x: &[RawTag]) -> Option { let at = x.iter().position(|t| t.code == 1001 && t.value.eq_ignore_ascii_case(ETAG))?; x.iter().skip(at + 1).take_while(|t| t.code != 1001).find(|t| t.code == 1071).and_then(|t| t.value.trim().parse().ok()) } +/// The element tag of object `e`: its own xdata (where the importer keeps it), else any kept in the +/// foreign data. +fn etag(d: &Drawing, e: &Entity) -> Option { + etag_in(&e.common.xdata).or_else(|| etag_in(&d.foreign.entities.get(&e.handle)?.xdata)) +} + /// Each object of `b` keyed by its element tag, or by its position when it has none. fn by_tag(d: &Drawing, b: &Block) -> Vec<(i64, Handle)> { - b.entities.iter().enumerate().map(|(i, e)| (etag(d, e.handle).unwrap_or(i as i64 + 1_000_000), e.handle)).collect() + b.entities.iter().enumerate().map(|(i, e)| (etag(d, e).unwrap_or(i as i64 + 1_000_000), e.handle)).collect() } /// The file handle of block `name`'s BLOCK_RECORD, as the kept data refers to it. @@ -542,6 +547,12 @@ fn apply( let Some(nh) = copies.get(&e.handle).copied() else { continue }; let mut c: Entity = (**e).clone(); c.handle = nh; + // The copy's own xdata names the copy, not its original (as the kept data below does). + for t in &mut c.common.xdata { + if t.code == 1005 && Handle::parse_hex(&t.value) == Some(e.handle) { + t.value = nh.hex(); + } + } if let Some(v) = shown_copy.get(&e.handle) { c.common.visible = *v; } @@ -964,3 +975,30 @@ fn drop_if_unused(d: &mut Drawing, name: &str) { } } } + +#[cfg(test)] +mod tests { + use super::*; + + fn tagged(h: u64, tag_value: Option<&str>) -> Entity { + let mut e = Entity::new(Handle(h), EntityKind::Line(crate::Line { a: Vec3::ZERO, b: Vec3::ZERO })); + if let Some(v) = tag_value { + e.common.xdata = vec![tag(1001, ETAG), tag(1071, v)]; + } + e + } + + #[test] + fn element_tags_are_read_from_the_entitys_own_xdata() { + let d = Drawing::new_imperial(); + let mut def = Block::new("DEF"); + def.entities.push(tagged(1, Some("10"))); + def.entities.push(tagged(2, Some("20"))); + let mut anon = Block::new("*U1"); + // Reordered, and the first object left out. + anon.entities.push(tagged(5, Some("20"))); + anon.entities.push(tagged(6, None)); + assert_eq!(by_tag(&d, &def), vec![(10, Handle(1)), (20, Handle(2))]); + assert_eq!(by_tag(&d, &anon), vec![(20, Handle(5)), (1_000_001, Handle(6))]); + } +} From d0a1ea526811d12e67135f0f8073c1f600f39a8b Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:25:31 +0100 Subject: [PATCH 25/31] Table commands reject a malformed handle instead of using the selected table (#934) (#970) --- crates/engine/src/cmd/table.rs | 25 +++++++++--- crates/engine/src/cmd/table/cells.rs | 11 +---- crates/engine/tests/table_malformed_handle.rs | 40 +++++++++++++++++++ 3 files changed, 60 insertions(+), 16 deletions(-) create mode 100644 crates/engine/tests/table_malformed_handle.rs diff --git a/crates/engine/src/cmd/table.rs b/crates/engine/src/cmd/table.rs index be156fb1d..f73222866 100644 --- a/crates/engine/src/cmd/table.rs +++ b/crates/engine/src/cmd/table.rs @@ -141,15 +141,28 @@ fn run_table(s: &mut Session, p: &Value) -> Result { Ok(json!({ "handle": h.hex() })) } -/// Modify the table `handle` with `f`. -fn edit(s: &mut Session, p: &Value, id: &str, f: impl FnOnce(&mut Table) -> std::result::Result<(), String>) -> Result { - let h = p - .get("handle") - .and_then(|v| v.as_str().and_then(Handle::parse_hex).or_else(|| v.as_u64().map(Handle))) +/// The table `handle`, or else the selected table. A supplied `handle` that can't be read is an +/// error, never a silent fall back to the selection (like `targets`). +fn table_handle(s: &Session, p: &Value, id: &str) -> Result { + let explicit = match p.get("handle") { + None | Some(Value::Null) => None, + Some(v) => Some( + v.as_str() + .and_then(Handle::parse_hex) + .or_else(|| v.as_u64().map(Handle)) + .ok_or_else(|| bad(id, format!("`handle`: {v} is not an object handle (a hex string such as \"1A2\")")))?, + ), + }; + explicit .or_else(|| { s.selection().into_iter().find(|h| s.doc().ok().and_then(|d| d.entity(*h)).is_some_and(|e| matches!(e.kind, EntityKind::Table(_)))) }) - .ok_or_else(|| bad(id, "`handle` of a table is required"))?; + .ok_or_else(|| bad(id, "`handle` of a table is required")) +} + +/// Modify the table `handle` with `f`. +fn edit(s: &mut Session, p: &Value, id: &str, f: impl FnOnce(&mut Table) -> std::result::Result<(), String>) -> Result { + let h = table_handle(s, p, id)?; let mut t = match s.doc()?.entity(h).map(|e| &e.kind) { Some(EntityKind::Table(t)) => t.clone(), _ => return Err(bad(id, "object is not a table")), diff --git a/crates/engine/src/cmd/table/cells.rs b/crates/engine/src/cmd/table/cells.rs index a2efca4d6..d020758d8 100644 --- a/crates/engine/src/cmd/table/cells.rs +++ b/crates/engine/src/cmd/table/cells.rs @@ -6,7 +6,7 @@ use cadcraft_geom::Vec2; use serde_json::{Value, json}; use super::formula::{self, cell_name, parse_cell_name}; -use super::{MAX_COLS, MAX_ROWS, edit, usize_param}; +use super::{MAX_COLS, MAX_ROWS, edit, table_handle, usize_param}; use crate::cmd::{CommandSpec, bad, bool_or, str_param}; use crate::{Accept, EngineError, Input, Interactive, Prompt, Result, Session, Step}; @@ -524,15 +524,6 @@ fn write_text(path: &str, text: &str) -> Result<()> { } } -fn table_handle(s: &Session, p: &Value, id: &str) -> Result { - p.get("handle") - .and_then(|v| v.as_str().and_then(Handle::parse_hex).or_else(|| v.as_u64().map(Handle))) - .or_else(|| { - s.selection().into_iter().find(|h| s.doc().ok().and_then(|d| d.entity(*h)).is_some_and(|e| matches!(e.kind, EntityKind::Table(_)))) - }) - .ok_or_else(|| bad(id, "`handle` of a table is required")) -} - fn run_export(s: &mut Session, p: &Value) -> Result { let id = "tableexport"; let h = table_handle(s, p, id)?; diff --git a/crates/engine/tests/table_malformed_handle.rs b/crates/engine/tests/table_malformed_handle.rs new file mode 100644 index 000000000..849557494 --- /dev/null +++ b/crates/engine/tests/table_malformed_handle.rs @@ -0,0 +1,40 @@ +//! A malformed `handle` on a table edit command or TABLEEXPORT is an error. It never falls back +//! to the selected table (#934, follow-up to #330). + +use cadcraft_engine::Session; +use cadcraft_engine::doc::{EntityKind, Handle}; +use serde_json::json; + +fn cell_text(s: &Session, h: &str) -> String { + let h = Handle::parse_hex(h).unwrap(); + match &s.doc().unwrap().entity(h).unwrap().kind { + EntityKind::Table(t) => t.cells[0][0].text.clone(), + _ => String::new(), + } +} + +#[test] +fn malformed_table_handle_does_not_edit_the_selected_table() { + let mut s = Session::new(); + let h = s.execute("table", &json!({ "at": [0, 0], "cells": [["a", "b"]], "rowHeight": 1.0, "colWidth": 3.0 })).unwrap()["handle"] + .as_str() + .unwrap() + .to_string(); + s.set_selection(vec![Handle::parse_hex(&h).unwrap()]); + + for bad in [json!("zzz"), json!(""), json!("#1"), json!(-1), json!(true), json!([1])] { + let e = s.execute("table.set", &json!({ "handle": bad, "row": 0, "col": 0, "text": "X" })).expect_err(&format!("handle {bad}")); + assert!(e.to_string().contains("is not an object handle"), "handle {bad}: {e}"); + assert_eq!(cell_text(&s, &h), "a", "handle {bad} must change nothing"); + // TABLEEXPORT resolves its table the same way: no CSV of the selected table either. + let e = s.execute("tableexport", &json!({ "handle": bad })).expect_err(&format!("tableexport handle {bad}")); + assert!(e.to_string().contains("is not an object handle"), "tableexport handle {bad}: {e}"); + } + + // An explicit valid handle and an omitted handle (selection) still work. + s.execute("table.set", &json!({ "handle": h, "row": 0, "col": 0, "text": "Y" })).unwrap(); + assert_eq!(cell_text(&s, &h), "Y"); + s.execute("table.set", &json!({ "row": 0, "col": 0, "text": "Z" })).unwrap(); + assert_eq!(cell_text(&s, &h), "Z"); + assert_eq!(s.execute("tableexport", &json!({})).unwrap()["csv"], "Z,b\r\n"); +} From c3c455fbbf4c4babcb8fb4ea7f6ed7f0c76edf6b Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:25:34 +0100 Subject: [PATCH 26/31] Region: test very long segments against every segment so no crossing is missed (#929) (#975) --- crates/geom/src/region.rs | 61 ++++++++++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 17 deletions(-) diff --git a/crates/geom/src/region.rs b/crates/geom/src/region.rs index 98050cfdc..bef60b530 100644 --- a/crates/geom/src/region.rs +++ b/crates/geom/src/region.rs @@ -60,6 +60,21 @@ struct Graph { edge_src: HashMap<(usize, usize), usize>, } +/// Records the crossing of segments `i` and `j`, if any, as split parameters on both. +fn split_pair(segs: &[(Vec2, Vec2)], splits: &mut [Vec], i: usize, j: usize) { + let (Some(si), Some(sj)) = (segs.get(i), segs.get(j)) else { return }; + let li = Line::new(si.0, si.1); + let lj = Line::new(sj.0, sj.1); + if let Some(x) = line_line(&li, &lj) { + if let Some(v) = splits.get_mut(i) { + v.push(li.param_of(x).clamp(0.0, 1.0)); + } + if let Some(v) = splits.get_mut(j) { + v.push(lj.param_of(x).clamp(0.0, 1.0)); + } + } +} + fn build(polys: &[Vec], tol: f64) -> Option { let mut segs: Vec<(Vec2, Vec2)> = Vec::new(); let mut owner: Vec = Vec::new(); @@ -94,16 +109,13 @@ fn build(polys: &[Vec], tol: f64) -> Option { ) }; let mut grid: HashMap<(usize, usize), Vec> = HashMap::new(); + let mut long: Vec = Vec::new(); for (i, (a, b)) in segs.iter().enumerate() { let (x0, y0) = cell(a.min(*b)); let (x1, y1) = cell(a.max(*b)); if (x1 - x0 + 1) * (y1 - y0 + 1) > 4096 { - // Very long segment: register in a coarse band. - for gx in (x0..=x1).step_by(((x1 - x0) / 64).max(1)) { - for gy in (y0..=y1).step_by(((y1 - y0) / 64).max(1)) { - grid.entry((gx, gy)).or_default().push(i); - } - } + // Very long segment: too many cells to register in; it is tested against every segment below. + long.push(i); continue; } for gx in x0..=x1 { @@ -130,23 +142,24 @@ fn build(polys: &[Vec], tol: f64) -> Option { if i == j || !tested.insert((i.min(j), i.max(j))) { continue; } - let (Some(si), Some(sj)) = (segs.get(i), segs.get(j)) else { continue }; - let li = Line::new(si.0, si.1); - let lj = Line::new(sj.0, sj.1); - if let Some(x) = line_line(&li, &lj) { - if let Some(v) = splits.get_mut(i) { - v.push(li.param_of(x).clamp(0.0, 1.0)); - } - if let Some(v) = splits.get_mut(j) { - v.push(lj.param_of(x).clamp(0.0, 1.0)); - } - } + split_pair(&segs, &mut splits, i, j); if tested.len() > 20_000_000 { return None; } } } } + for &i in &long { + for j in 0..n { + if i == j || !tested.insert((i.min(j), i.max(j))) { + continue; + } + split_pair(&segs, &mut splits, i, j); + if tested.len() > 20_000_000 { + return None; + } + } + } let mut g = Graph { pts: Vec::new(), adj: Vec::new(), src: Vec::with_capacity(n), edge_src: HashMap::new() }; let mut index: HashMap<(i64, i64), usize> = HashMap::new(); let mut vid = |p: Vec2, g: &mut Graph| -> usize { @@ -609,6 +622,20 @@ mod tests { vec![Vec2::new(x0, y0), Vec2::new(x0 + s, y0), Vec2::new(x0 + s, y0 + s), Vec2::new(x0, y0 + s), Vec2::new(x0, y0)] } + #[test] + fn long_segment_crossing_in_unsampled_cell_is_split() { + // A diagonal long enough to skip the grid, and a short segment crossing it in a cell the old coarse sampling missed. + let mut polys = vec![vec![Vec2::new(0.0, 0.0), Vec2::new(1000.0, 1000.0)], vec![Vec2::new(11.0, 12.0), Vec2::new(12.0, 11.0)]]; + for i in 0..17_000usize { + let x = 600.0 + (i % 130) as f64 * 2.0; + let y = 100.0 + (i / 130) as f64 * 2.0; + polys.push(vec![Vec2::new(x, y), Vec2::new(x + 1.0, y)]); + } + let g = build(&polys, 1e-6).unwrap(); + let hit = g.pts.iter().position(|p| p.near(Vec2::new(11.5, 11.5), 1e-6)).unwrap(); + assert_eq!(g.adj[hit].len(), 4); + } + #[test] fn finds_square_around_point() { let l = enclosing_loop(&[square(0.0, 0.0, 10.0)], Vec2::new(5.0, 5.0), 1e-9).unwrap(); From 3f92d57b79cbd3b8716ab9ce4ba47c99c50c0df4 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:25:38 +0100 Subject: [PATCH 27/31] Geometry: circle intersection judges tangency against the smaller circle, so a small circle crossing a huge one gives two points (#926) (#976) --- crates/geom/src/intersect.rs | 22 +++++++++++++++------- crates/geom/src/tests.rs | 23 +++++++++++++++++++++++ 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/crates/geom/src/intersect.rs b/crates/geom/src/intersect.rs index 1451a94bd..653cd61c9 100644 --- a/crates/geom/src/intersect.rs +++ b/crates/geom/src/intersect.rs @@ -62,22 +62,29 @@ pub fn circle_circle(c1: &Circle, c2: &Circle) -> Vec { // Work at the scale of the two circles: fixed absolute tolerances classify // small secants as tangencies, and raw radius squares overflow on large drawings. + // Measure the chord from the smaller circle: from the larger one the half-chord is a + // difference of nearly equal numbers, so a tiny circle crossing a huge one is lost. + let swapped = r2 < r1; + let (from, dir) = if swapped { (c2.center, -(delta / d)) } else { (c1.center, delta / d) }; let scale = d.max(r1).max(r2); let (ds, r1s, r2s) = (d / scale, r1 / scale, r2 / scale); if ds > r1s + r2s || ds < (r1s - r2s).abs() { return Vec::new(); } - let a = ((r1s - r2s) * (r1s + r2s) + ds * ds) / (2.0 * ds); - let h2 = (r1s - a) * (r1s + a); - if h2 < -1e-12 { + let (rs, ro) = if swapped { (r2s, r1s) } else { (r1s, r2s) }; + let a = ((rs - ro) * (rs + ro) + ds * ds) / (2.0 * ds); + let h2 = (rs - a) * (rs + a); + // Tangency is judged against the smaller circle, which sets the size of the chord, with + // room for the rounding in `a` (a few ulps of the unit scale over ds, times 2a in h2). + let tol = (1e-12 * rs * rs).max(8.0 * f64::EPSILON * rs / ds); + if h2 < -tol { return Vec::new(); } - let dir = delta / d; - let p = c1.center + dir * (a * scale); + let p = from + dir * (a * scale); if !p.is_finite() { return Vec::new(); } - if h2 <= 1e-12 { + if h2 <= tol { return vec![p]; } let step = dir.perp() * (h2.sqrt() * scale); @@ -85,7 +92,8 @@ pub fn circle_circle(c1: &Circle, c2: &Circle) -> Vec { if !p1.is_finite() || !p2.is_finite() { return Vec::new(); } - vec![p1, p2] + // Keep the result order independent of which circle was used as the base. + if swapped { vec![p2, p1] } else { vec![p1, p2] } } fn on_arc(a: &Arc, p: Vec2) -> bool { diff --git a/crates/geom/src/tests.rs b/crates/geom/src/tests.rs index a452e610d..915a1071e 100644 --- a/crates/geom/src/tests.rs +++ b/crates/geom/src/tests.rs @@ -119,6 +119,29 @@ fn circle_circle_hits() { assert!(h.iter().all(|p| close(p.x, 4.0) && close(p.y.abs(), 3.0))); } +#[test] +fn small_circle_crossing_a_huge_circle_gives_two_hits() { + // Radii 1e7 and 1 with centres 1e7 apart: the small circle crosses the large one. + let big = Circle::new(Vec2::ZERO, 1e7); + let small = Circle::new(Vec2::new(1e7, 0.0), 1.0); + for hits in [circle_circle(&big, &small), circle_circle(&small, &big)] { + assert_eq!(hits.len(), 2, "{hits:?}"); + for p in &hits { + assert!((p.dist(small.center) - 1.0).abs() < 1e-6, "{p:?} is off the small circle"); + assert!((p.dist(big.center) - 1e7).abs() < 1e-6, "{p:?} is off the big circle"); + } + assert!(hits[0].dist(hits[1]) > 1.0, "{hits:?}"); + } + // Touching from outside and from inside is still one tangent point, in both orders. + for (x, touch) in [(1e7 + 1.0, 1e7), (1e7 - 1.0, 1e7)] { + let small = Circle::new(Vec2::new(x, 0.0), 1.0); + for hits in [circle_circle(&big, &small), circle_circle(&small, &big)] { + assert_eq!(hits.len(), 1, "{hits:?}"); + assert!(hits[0].dist(Vec2::new(touch, 0.0)) < 1e-6, "{hits:?}"); + } + } +} + #[test] fn tiny_circle_secants_and_separated_circles() { let first = Circle::new(Vec2::ZERO, 1e-9); From 2528671106070105c30440f2fc6142f226498f17 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:25:41 +0100 Subject: [PATCH 28/31] Cap HATCH spline edge degree at 10 like the SPLINE reader (#923) (#978) --- crates/io/src/dxf_read.rs | 2 +- crates/io/src/tests.rs | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/crates/io/src/dxf_read.rs b/crates/io/src/dxf_read.rs index e71f7725f..4c797ed9e 100644 --- a/crates/io/src/dxf_read.rs +++ b/crates/io/src/dxf_read.rs @@ -734,7 +734,7 @@ fn hatch(tags: &[Tag], t: &T) -> Option { while let Some(x) = get(i).filter(|x| matches!(x.code, 94 | 73 | 74 | 95 | 96)) { let n = x.i64().clamp(0, 100_000) as usize; match x.code { - 94 => degree = n, + 94 => degree = n.clamp(1, 10), // same cap as the SPLINE reader 95 => nk = n, 96 => nc = n, _ => {} diff --git a/crates/io/src/tests.rs b/crates/io/src/tests.rs index 7b8f38385..d5fe5d2f7 100644 --- a/crates/io/src/tests.rs +++ b/crates/io/src/tests.rs @@ -2139,6 +2139,30 @@ fn hatch_edges_keep_full_circles_splines_and_clockwise_ellipses() { near(hatch_edge_extents(&ellipse(1)), [-5.0, 0.0, 5.0, 2.5]); } +/// Issue #923: a HATCH spline edge caps its degree at 10 like the SPLINE reader does, both when the +/// knots are valid for the stated degree and when the reader falls back to the control polygon. +#[test] +fn hatch_spline_edge_caps_degree_like_spline() { + let control: Vec = (0..13).map(|i| Vec2::new(f64::from(i), f64::from(i % 2) * 10.0)).collect(); + let points: String = control.iter().map(|c| format!("10\n{}\n20\n{}\n", c.x, c.y)).collect(); + let close = "72\n1\n10\n12\n20\n0\n11\n0\n21\n0\n"; + // The degree-10 curve over the same controls, which is what the capped edge must produce. + let want = cadcraft_geom::Spline::from_control(control, 10).tessellate(1e-3); + let (mut lo, mut hi) = (Vec2::new(f64::MAX, f64::MAX), Vec2::new(f64::MIN, f64::MIN)); + for p in &want { + (lo, hi) = (Vec2::new(lo.x.min(p.x), lo.y.min(p.y)), Vec2::new(hi.x.max(p.x), hi.y.max(p.y))); + } + // Degree 12 with 13 controls: a valid knot vector has 26 knots; the second edge gives none. + let knots = format!("{}{}", "40\n0\n".repeat(13), "40\n1\n".repeat(13)); + for (nk, knots) in [(26, knots.as_str()), (0, "")] { + let edge = format!("72\n4\n94\n12\n73\n0\n74\n0\n95\n{nk}\n96\n13\n{knots}{points}97\n0\n{close}"); + let b = hatch_edge_extents(&edge); + for (got, want) in [(b.min.x, lo.x), (b.min.y, lo.y), (b.max.x, hi.x), (b.max.y, hi.y)] { + assert!((got - want).abs() < 1e-6, "extents {b:?}, want {lo:?} {hi:?} (knots {nk})"); + } + } +} + #[test] fn mirrored_text_extrusion_keeps_its_footprint() { // TEXT at OCS (-10,5) under extrusion (0,0,-1) starts at WCS (10,5) and runs towards -x (with From 8343045863dc1ace20ee3e3e378cdac639dbcf94 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:25:44 +0100 Subject: [PATCH 29/31] Fix grid index span overflow with a very large view (#920) (#979) --- crates/ui-egui/src/canvas.rs | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 81c79bbf8..a2812367a 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -692,6 +692,18 @@ fn adaptive(unit: f64, scale: f64, min_px: f64) -> f64 { u } +/// Index ranges of the grid lines inside `wb` for line spacing `minor`, or `None` when there would be +/// 2000 lines or more along an axis. The span is computed with checked arithmetic: a huge view +/// saturates the float-to-integer casts at the i64 limits, and subtracting those would overflow. +fn grid_index_ranges(wb: &Bounds2, minor: f64) -> Option<((i64, i64), (i64, i64))> { + let range = |lo: f64, hi: f64| { + let a = (lo / minor).floor() as i64; + let b = (hi / minor).ceil() as i64; + b.checked_sub(a).filter(|n| *n < 2000).map(|_| (a, b)) + }; + Some((range(wb.min.x, wb.max.x)?, range(wb.min.y, wb.max.y)?)) +} + fn draw_grid(app: &CadApp, p: &egui::Painter, xf: &Xf) { let t = Tokens::get(); let s = app.session.drafting_settings(); @@ -704,11 +716,7 @@ fn draw_grid(app: &CadApp, p: &egui::Painter, xf: &Xf) { let minor = adaptive(unit, xf.scale, 8.0); let major = minor * major_n; let mut lines = Vec::new(); - let x0 = (wb.min.x / minor).floor() as i64; - let x1 = (wb.max.x / minor).ceil() as i64; - let y0 = (wb.min.y / minor).floor() as i64; - let y1 = (wb.max.y / minor).ceil() as i64; - if (x1 - x0) < 2000 && (y1 - y0) < 2000 { + if let Some(((x0, x1), (y0, y1))) = grid_index_ranges(&wb, minor) { for i in x0..=x1 { let x = i as f64 * minor; let is_major = (x / major).round() * major - x == 0.0 || ((x / major) - (x / major).round()).abs() < 1e-6; @@ -2995,3 +3003,16 @@ mod hatch_pick_tests { assert!(app.session.running.is_none(), "{:?}", app.session.running.as_ref().map(|r| r.machine.name())); } } + +#[cfg(test)] +mod grid_range_tests { + use cadcraft_geom::{Bounds2, Vec2}; + + #[test] + fn grid_index_ranges_survive_extreme_views() { + let huge = Bounds2::new(Vec2::new(-1e300, -1e300), Vec2::new(1e300, 1e300)); + assert_eq!(super::grid_index_ranges(&huge, 1.0), None); + let normal = Bounds2::new(Vec2::new(-5.0, 0.0), Vec2::new(5.0, 10.0)); + assert_eq!(super::grid_index_ranges(&normal, 1.0), Some(((-5, 5), (0, 10)))); + } +} From d73e379fd24068d73563f2e895f05bdd471b3a6a Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:25:48 +0100 Subject: [PATCH 30/31] Autosave no longer writes the old host as recovery when the Block Editor's changes can't be composed (#918) (#980) --- crates/engine/src/autosave.rs | 9 +++++-- crates/engine/src/blockedit.rs | 11 +++++---- crates/engine/tests/autosave.rs | 38 +++++++++++++++++++++++++++++ crates/engine/tests/block_editor.rs | 2 +- 4 files changed, 52 insertions(+), 8 deletions(-) diff --git a/crates/engine/src/autosave.rs b/crates/engine/src/autosave.rs index 0904711e4..4da107449 100644 --- a/crates/engine/src/autosave.rs +++ b/crates/engine/src/autosave.rs @@ -208,6 +208,12 @@ impl Autosaver { if minutes == 0 || dir.trim().is_empty() || (!urgent && now - since < f64::from(minutes) * 60.0) { continue; } + // In the Block Editor: the drawing with the editor's unsaved changes in the block. When + // they can't be put there, nothing is written and nothing is marked as covered, so the + // next pass tries again instead of keeping the host without them. + let Ok(doc) = st.recovery_drawing() else { + continue; + }; let path = t .file .clone() @@ -216,8 +222,7 @@ impl Autosaver { t.written = Some(Arc::downgrade(&st.doc)); t.host = Some(Arc::downgrade(st.drawing())); t.since = None; - // In the Block Editor: the drawing with the editor's unsaved changes in the block. - jobs.push(Job::Write { uid: st.uid, path, doc: st.recovery_drawing() }); + jobs.push(Job::Write { uid: st.uid, path, doc }); } jobs } diff --git a/crates/engine/src/blockedit.rs b/crates/engine/src/blockedit.rs index 776909a68..4dab97f1f 100644 --- a/crates/engine/src/blockedit.rs +++ b/crates/engine/src/blockedit.rs @@ -195,12 +195,13 @@ impl DocState { self.block_edit.as_ref().is_some_and(|b| !Arc::ptr_eq(&b.saved, &self.doc)) } /// The drawing to keep for recovery (autosave): with the Block Editor's unsaved changes in the - /// block, so a crash doesn't lose them. - pub fn recovery_drawing(&self) -> Arc { + /// block, so a crash doesn't lose them. An error when the editor's changes can't be put in the + /// block (it would contain itself): the host drawing alone would lose them. + pub fn recovery_drawing(&self) -> Result> { match &self.block_edit { - Some(be) if self.block_unsaved() => be.compose(&self.doc, &be.name).map(Arc::new).unwrap_or_else(|_| be.host.doc.clone()), - Some(be) => be.host.doc.clone(), - None => self.doc.clone(), + Some(be) if self.block_unsaved() => be.compose(&self.doc, &be.name).map(Arc::new), + Some(be) => Ok(be.host.doc.clone()), + None => Ok(self.doc.clone()), } } diff --git a/crates/engine/tests/autosave.rs b/crates/engine/tests/autosave.rs index 0f0ec7da7..da03c0d7d 100644 --- a/crates/engine/tests/autosave.rs +++ b/crates/engine/tests/autosave.rs @@ -322,3 +322,41 @@ fn hooks() { plot: None, }); } + +#[test] +fn a_block_editor_that_cannot_be_composed_is_not_autosaved_as_the_old_host() { + use cadcraft_engine::doc::{Entity, EntityKind, Insert}; + use cadcraft_engine::geom::Vec3; + + let mut s = Session::new(); + let l = s.execute("line", &json!({ "points": [[10, 10], [12, 10]] })).unwrap()["handles"][0].clone(); + s.execute("block", &json!({ "name": "Door", "base": [10, 10], "handles": [l] })).unwrap(); + s.execute("bedit", &json!({ "name": "Door" })).unwrap(); + // An unsaved change in the editor: the block would contain a reference to itself. + let d = s.doc_mut().unwrap(); + let h = d.new_handle(); + let kind = EntityKind::Insert(Insert { + block: "Door".into(), + insert: Vec3::ZERO, + scale: Vec3::new(1.0, 1.0, 1.0), + rotation: 0.0, + attribs: vec![], + cols: 1, + rows: 1, + col_spacing: 0.0, + row_spacing: 0.0, + }); + d.model.push(Entity::new(h, kind)); + assert!(s.state().unwrap().recovery_drawing().is_err()); + + let mut a = Autosaver::new("55-b"); + a.plan(&s, 0.0, 1, "/auto"); + assert!(a.plan(&s, 60.0, 1, "/auto").is_empty(), "no recovery file without the editor's changes"); + assert!(a.plan(&s, 120.0, 1, "/auto").is_empty()); + assert_eq!(a.next_due(120.0, 1), Some(0.0), "still waiting to be written"); + + // Once the reference is gone, the next autosave writes the editor's changes. + let d = s.doc_mut().unwrap(); + d.model.remove(h); + assert_eq!(writes(&a.plan(&s, 180.0, 1, "/auto")).len(), 1); +} diff --git a/crates/engine/tests/block_editor.rs b/crates/engine/tests/block_editor.rs index 3b1404f1f..711bcd796 100644 --- a/crates/engine/tests/block_editor.rs +++ b/crates/engine/tests/block_editor.rs @@ -217,7 +217,7 @@ fn a_file_save_writes_the_drawing_outside_the_editor() { s.execute("bedit", &json!({ "name": "Door" })).unwrap(); line(&mut s, [0.0, 0.0], [0.0, 3.0]); // Autosave keeps the editor's unsaved change in the block. - let rec = s.state().unwrap().recovery_drawing(); + let rec = s.state().unwrap().recovery_drawing().unwrap(); assert_eq!(rec.block("Door").unwrap().entities.len(), 2); assert_eq!(rec.model.len(), 2); From d6e712db41d6a02dd060490c776e1a8c865f0a60 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:25:51 +0100 Subject: [PATCH 31/31] CLI --save, convert and sample write through a staged file so a failed write keeps the previous drawing (#912) (#982) * CLI --save, convert and sample write through a staged file so a failed write keeps the previous drawing (#912) * cli: write through the engine's atomic_write instead of a second copy Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Brandon Thomas Co-authored-by: Claude Opus 5.5 --- apps/cadcraft-cli/src/main.rs | 41 +++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/apps/cadcraft-cli/src/main.rs b/apps/cadcraft-cli/src/main.rs index 32c781ec4..99a8b4282 100644 --- a/apps/cadcraft-cli/src/main.rs +++ b/apps/cadcraft-cli/src/main.rs @@ -226,6 +226,12 @@ fn info(path: &str) -> Result<(), String> { Ok(()) } +/// Write `bytes` to `out` through the engine's atomic writer (an exclusively created sibling file +/// renamed over `out`), so a failed or interrupted write leaves the previous file untouched. +fn write_atomic(out: &str, bytes: &[u8]) -> std::io::Result<()> { + cadcraft_engine::atomic_write::write_atomic(std::path::Path::new(out), bytes) +} + fn export(s: &Session, out: &str) -> Result<(), String> { export_framed(s, out, None) } @@ -233,7 +239,7 @@ fn export(s: &Session, out: &str) -> Result<(), String> { fn export_framed(s: &Session, out: &str, window: Option) -> Result<(), String> { let d = s.doc().map_err(|e| e.to_string())?; let bytes = cadcraft_io::write_framed(d, out, window).map_err(|e| e.to_string())?; - std::fs::write(out, &bytes).map_err(|e| format!("{out}: {e}"))?; + write_atomic(out, &bytes).map_err(|e| format!("{out}: {e}"))?; eprintln!("wrote {out} ({} bytes)", bytes.len()); if cadcraft_engine::cmd::file::drops_unknown(out) { let lost = cadcraft_engine::cmd::file::not_saved(d); @@ -252,7 +258,7 @@ fn sample(args: &[String]) -> Result<(), String> { other => return Err(format!("unknown sample `{other}` (bracket, floorplan)")), }; let bytes = cadcraft_io::write(&d, out).map_err(|e| e.to_string())?; - std::fs::write(out, &bytes).map_err(|e| format!("{out}: {e}"))?; + write_atomic(out, &bytes).map_err(|e| format!("{out}: {e}"))?; eprintln!("wrote {out} ({} bytes)", bytes.len()); Ok(()) } @@ -862,6 +868,37 @@ fn cli() -> ExitCode { mod tests { use super::*; + fn scratch_dir(tag: &str) -> std::path::PathBuf { + let dir = std::env::temp_dir().join(format!("cadcraft-cli-{tag}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).expect("create scratch dir"); + dir + } + + #[test] + fn write_atomic_replaces_file_and_leaves_no_staging_files() { + let dir = scratch_dir("replace"); + let out = dir.join("a.dxf"); + let out = out.to_str().expect("utf-8 path"); + write_atomic(out, b"old").expect("create"); + write_atomic(out, b"new").expect("overwrite"); + assert_eq!(std::fs::read(out).expect("read"), b"new"); + assert_eq!(std::fs::read_dir(&dir).expect("list").count(), 1); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn write_atomic_failure_keeps_destination_and_cleans_up() { + let dir = scratch_dir("fail"); + let out = dir.join("target"); + std::fs::create_dir(&out).expect("dir as destination"); + std::fs::write(out.join("keep"), b"x").expect("seed"); + assert!(write_atomic(out.to_str().expect("utf-8 path"), b"new").is_err()); + assert_eq!(std::fs::read(out.join("keep")).expect("kept"), b"x"); + assert_eq!(std::fs::read_dir(&dir).expect("list").count(), 1); + let _ = std::fs::remove_dir_all(&dir); + } + fn b(x0: f64, y0: f64, x1: f64, y1: f64) -> Bounds2 { Bounds2::new(Vec2::new(x0, y0), Vec2::new(x1, y1)) }