diff --git a/Cargo.lock b/Cargo.lock index 64e87c24a..4da766abd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4,7 +4,7 @@ version = 4 [[package]] name = "OpenCADStudio" -version = "2026.37.0" +version = "2026.38.0" dependencies = [ "acadrust", "ashpd", diff --git a/Cargo.toml b/Cargo.toml index 7b9768873..6e78631dc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "OpenCADStudio" -version = "2026.37.0" +version = "2026.38.0" edition = "2021" build = "build.rs" default-run = "OpenCADStudio" diff --git a/crates/ocs_plugin_api/src/host.rs b/crates/ocs_plugin_api/src/host.rs index f95c2880a..eff68422c 100644 --- a/crates/ocs_plugin_api/src/host.rs +++ b/crates/ocs_plugin_api/src/host.rs @@ -690,6 +690,36 @@ pub trait HostApi { fn solid_operation(&mut self, _operation: SolidOperation) -> Result { Err("solid operations are not supported by this host".to_owned()) } + /// Add a layer to the active document with full initial properties. + /// If an optional property in `config` is `None`, standard CAD defaults are applied. + /// Returns `None` if the layer already exists or `config.name` is invalid. + fn add_layer(&mut self, config: LayerConfig) -> Option { + let _ = config; + None + } + + /// Modify specified properties of an existing layer in the active document. + /// Properties that are `None` in `config` are left untouched as-is. + /// Returns `false` if the layer does not exist or `config.name` is invalid. + fn modify_layer(&mut self, config: LayerConfig) -> bool { + let _ = config; + false + } +} + +/// Configuration properties for creating or modifying a layer. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, Default)] +pub struct LayerConfig { + pub name: String, + pub color: Option, + pub linetype: Option, + pub lineweight: Option, + pub off: Option, + pub frozen: Option, + pub locked: Option, + pub plottable: Option, + pub transparency: Option, + pub description: Option, } /// Simplified, read-only entity kind exposed by [`DocumentReader`]. diff --git a/crates/ocs_plugin_api/src/ipc/client.rs b/crates/ocs_plugin_api/src/ipc/client.rs index aa5048284..16fd59671 100644 --- a/crates/ocs_plugin_api/src/ipc/client.rs +++ b/crates/ocs_plugin_api/src/ipc/client.rs @@ -490,6 +490,34 @@ impl HostApi for PluginHostApi { Err(error) => Err(error.to_string()), } } + + fn add_layer(&mut self, config: crate::host::LayerConfig) -> Option { + match self.client.request(PluginRequest::AddLayer(config)) { + Ok(PluginResponse::OptHandle(h)) => h, + Ok(other) => { + eprintln!("[plugin] unexpected AddLayer response: {other:?}"); + None + } + Err(e) => { + eprintln!("[plugin] AddLayer request failed: {e}"); + None + } + } + } + + fn modify_layer(&mut self, config: crate::host::LayerConfig) -> bool { + match self.client.request(PluginRequest::ModifyLayer(config)) { + Ok(PluginResponse::Bool(b)) => b, + Ok(other) => { + eprintln!("[plugin] unexpected ModifyLayer response: {other:?}"); + false + } + Err(e) => { + eprintln!("[plugin] ModifyLayer request failed: {e}"); + false + } + } + } } /// Sentinel reader used when the shared-memory view could not be initialized. diff --git a/crates/ocs_plugin_api/src/ipc/protocol.rs b/crates/ocs_plugin_api/src/ipc/protocol.rs index 70a5b72b6..042e0fb2e 100644 --- a/crates/ocs_plugin_api/src/ipc/protocol.rs +++ b/crates/ocs_plugin_api/src/ipc/protocol.rs @@ -140,6 +140,10 @@ pub enum PluginRequest { GetTabId, /// V5: ask the host for the filesystem path of the document in `tab_id`. DocumentPath { tab_id: u64 }, + /// Add a layer to the active document with full initial properties. + AddLayer(crate::host::LayerConfig), + /// Modify specified properties of an existing layer in the active document. + ModifyLayer(crate::host::LayerConfig), /// Read a host-managed setting without nested command dispatch. GetSystemVariable { name: String }, /// Change a host-managed setting without nested command dispatch. @@ -177,6 +181,8 @@ pub enum PluginResponse { TabId(u64), /// V5: filesystem path of the document in the requested tab, if any. DocumentPath(Option), + /// Optional entity handle (e.g. from AddLayer). + OptHandle(Option), SystemVariable(Option), SystemVariableResult(Result), EntityTransactionResult(Result<(), String>), diff --git a/crates/ocs_plugin_api/src/ipc/server.rs b/crates/ocs_plugin_api/src/ipc/server.rs index b20262b02..3b3c39592 100644 --- a/crates/ocs_plugin_api/src/ipc/server.rs +++ b/crates/ocs_plugin_api/src/ipc/server.rs @@ -98,6 +98,8 @@ pub fn handle_plugin_request( DocumentPath { tab_id } => PluginResponse::DocumentPath( host.document_path(tab_id).map(|path| path.into_os_string()), ), + AddLayer(config) => PluginResponse::OptHandle(host.add_layer(config)), + ModifyLayer(config) => PluginResponse::Bool(host.modify_layer(config)), GetSystemVariable { name } => PluginResponse::SystemVariable(host.system_variable(&name)), SetSystemVariable { name, value } => { PluginResponse::SystemVariableResult(host.set_system_variable(&name, value)) diff --git a/crates/ocs_plugin_api/src/ipc/v4/client.rs b/crates/ocs_plugin_api/src/ipc/v4/client.rs index cac660388..43c43e3ba 100644 --- a/crates/ocs_plugin_api/src/ipc/v4/client.rs +++ b/crates/ocs_plugin_api/src/ipc/v4/client.rs @@ -804,6 +804,34 @@ impl HostApi for V4PluginHostApi { Err(error) => Err(error.to_string()), } } + + fn add_layer(&mut self, config: crate::host::LayerConfig) -> Option { + match self.request(PluginRequest::AddLayer(config)) { + Ok(PluginResponse::OptHandle(h)) => h, + Ok(other) => { + eprintln!("[plugin] unexpected AddLayer response: {other:?}"); + None + } + Err(e) => { + eprintln!("[plugin] AddLayer request failed: {e}"); + None + } + } + } + + fn modify_layer(&mut self, config: crate::host::LayerConfig) -> bool { + match self.request(PluginRequest::ModifyLayer(config)) { + Ok(PluginResponse::Bool(b)) => b, + Ok(other) => { + eprintln!("[plugin] unexpected ModifyLayer response: {other:?}"); + false + } + Err(e) => { + eprintln!("[plugin] ModifyLayer request failed: {e}"); + false + } + } + } } #[cfg(test)] diff --git a/docs/automation/README.md b/docs/automation/README.md index 1dcf4a90e..4ce074e9f 100644 --- a/docs/automation/README.md +++ b/docs/automation/README.md @@ -17,6 +17,10 @@ The server provides four tools: - `ocs_execute` performs one operation, an atomic record update, or a sequential batch against the real editor. - `ocs_capture` returns a bounded PNG of the drawing viewport or complete window. +Native builds also accept a second, headless entry point (`OpenCADStudio --serve`, one +JSON request per line) for scripts and CI; the two native channels, their startup dialogs +and their limits are described in [native.md](native.md). + The normal flow is to call `ocs_sessions`, pass its returned `session_id` as `ocs_session_id` to the other tools, read the chosen session with `ocs_read`, then pass the returned document state into `ocs_execute`. For example, an undo request has this shape: ```json @@ -138,7 +142,27 @@ Execute responses use `response_detail: "compact"` by default and return only th For unfamiliar or conditional commands, use `start`, then inspect `state.command` in every response. Its `accepts` array gives the valid MCP input kinds, `options` gives the current tokens, and `input_example` gives the next request shape. Add the current state fields and a new `request_id` to each step. -Every `ocs_execute` request requires a caller-generated `request_id`. Reuse that ID only to retry the identical request after a timeout, together with the same session ID, document ID, expected revision, and selection. Commands report `waiting_input` while more input is required and asynchronous work remains `running` until its real callback finishes. Geometry stays in OpenCADStudio and its geometry kernel. +Every `ocs_execute` request requires a caller-generated `request_id`. Reuse that ID only to retry the identical request after a timeout, together with the same session ID, document ID, expected revision, and selection. Commands report `waiting_input` while more input is required and asynchronous work remains `running` until its real callback finishes. + +A `run` reply also carries two fields that describe what happened to the line: + +* `blocked_by` — `null` when the command finished, otherwise what *this line* + left open and waiting: `"command"` (an interactive command), `"text_editor"` + (the in-place editor opened by the `TEXT` content step), `"mtext_editor"`, or + `"modal:"` for a dialog. A surface that was already open when the line + started (an earlier line's editor, another tab, a startup dialog) is not + attributed to this line, so a caller polling for `completed` is never stuck + waiting on someone else's editor. `blocked_by` is set whenever `status` is + `waiting_input`. +* `unconsumed` — the tokens of the line that no prompt ever asked for, in order. + A fully consumed line reports `[]`; `CIRCLE 0,0 5 9` reports `["9"]` because + the radius step ends the command. Over-typed input is reported instead of + being dropped on the floor. + +When a line ends at the `TEXT` content step (`TEXT 0,0 5 0 hello`) the text is +typed into the in-place editor and committed, so the line behaves exactly as the +same input typed into the GUI command line would: one `Text` entity, nothing +unconsumed, and the command closed rather than left armed for a second line. Geometry stays in OpenCADStudio and its geometry kernel. `ocs_capture` defaults to the drawing viewport and a longest edge of 1600 pixels. Set `scope` to `window` for the full interface or change `max_dimension` between 256 and 4096. @@ -156,3 +180,5 @@ The repeatable live-editor evaluation draws three isolated entities in one batch ```sh python3 docs/automation/mcp_eval.py target/debug/OpenCADStudio ``` + +The web build is driven through two JavaScript functions instead of a process; see [web.md](web.md). diff --git a/docs/automation/native.md b/docs/automation/native.md new file mode 100644 index 000000000..9211f85c6 --- /dev/null +++ b/docs/automation/native.md @@ -0,0 +1,131 @@ +# Driving the native builds + +Every native build has three entry points besides the editor window: the MCP +server described in the [README](README.md) (`OpenCADStudio --mcp`), a headless +JSON-line channel for scripts and CI (`OpenCADStudio --serve`), and a one-shot +converter (`OpenCADStudio --export IN OUT`). The browser entry is described in +[web.md](web.md). + +## Choosing a channel + +| | `--mcp` | `--serve` | +|---|---|---| +| Transport | MCP (JSON-RPC over stdio) with the four tools of the README | one JSON request per line on stdin, one reply per line on stdout; `--port N` listens on `127.0.0.1:N` instead | +| Editor | attaches to a running desktop editor, or starts one when there is none (`ocs_sessions`, `launch_if_none`) | no window; a document session of its own that lives as long as the process | +| Needs a display | yes (`xvfb-run` works on a headless machine; software rendering may raise the GPU warning dialog) | no | +| Needs a writable `HOME` / `XDG_CONFIG_HOME` | yes — the session descriptors live in the user configuration directory; without one `ocs_sessions` answers `No user configuration directory` | no | +| Startup dialogs | those of the editor (below) | none | +| `capture` | `ocs_capture` | not available (`gui_required`): there is no window to capture | +| Good for | an MCP client driving a real editor | batch jobs, CI, agents without a display | + +## `--serve` + +The first line written is a readiness banner, before any request is read: + +```json +{"ok":true,"ready":true,"version":"2026.38"} +``` + +Over `--port N` the banner opens every connection; the listener serves one +client at a time and the document session persists across reconnects. + +Two request styles share the channel, told apart by the `protocol` field: + +| Style | Shape | Operations | +|---|---|---| +| protocol 1 | `{"protocol":1,"request_id":"…","document_id":N,"op":"…"}` — the same requests and replies as `ocs_execute` / `ocs_read`, described in the README | `new`, `open`, `save`, `activate`, `select`, `run`, `start`, `input`, `cancel`, `stop`, `action`, `property`, `set_properties`, `embed_image`, `undo`, `redo`; reads `state`, `capabilities`, `query`, `entities`, `records`, `record_schema`, `layers`, `header`, `properties`, `measure`, `history`, `commands`, `events`, `operation` | +| legacy | `{"op":"…"}` without `protocol` | `new`, `open`, `run`, `entities`, `query`, `records`, `record_schema`, `capabilities`, `layers`, `header`, `select`, `save`, `undo`, `redo` | + +The two styles differ in how a waiting command is treated. A protocol-1 `run` +or `start` while a command is still active is refused with `command_busy`: +continue it with `input` or drop it with `cancel`. A legacy `run` line ends the +waiting command and starts the new one. + +### The legacy `run` line + +`{"op":"run","cmd":"LINE 0,0 10,10"}` feeds the tokens after the command name +to the prompts in order and then presses Enter. The reply is + +```json +{"ok":true,"cmd":"CIRCLE 0,0 5 9","status":"completed","blocked_by":null,"entities":1,"added":1,"unconsumed":["9"]} +``` + +| Field | Meaning | +|---|---| +| `status` | `completed`, or `waiting_input` when the line left something open | +| `blocked_by` | what *this line* left open: `"command"`, `"text_editor"`, `"mtext_editor"`, `"modal:"`, or `null`. A surface that was already open when the line started is not attributed to it | +| `entities`, `added` | entity count after the line, and the difference to before | +| `unconsumed` | tokens no prompt asked for, in order (`["9"]` above: the radius step ended the command); `[]` when everything was consumed | + +`waiting_input` means input is still required, not that nothing happened: +`SPLINE` and `ARC` keep accepting input after the entity exists, and +`zoom_extents` reports it after the view has already moved. + +`TEXT 0,0 5 0 hello world` creates one `Text` entity holding `hello world`: the +tail of the line is typed into the in-place editor, committed, and the command +is closed so it does not stay armed for a second line. A `TEXT` line without +content answers `waiting_input` with `blocked_by: "text_editor"`. + +Known gap (#1390): `MTEXT`, `HATCH` and `POLYGON` answer `completed` with +`added: 0` on this path — they need interactive surfaces (the rich-text editor, +a pattern choice, a boundary pick) that a single line has no tokens to fill. + +## `--mcp` + +The MCP server is a client of the desktop editor: `ocs_sessions` lists the live +editor sessions found in the configuration directory and starts one when none +is running, so several MCP connections can share one editor and its state. + +A fresh profile opens two dialogs before any drawing exists, and both block +document creation: + +| Step | `ocs_sessions` reports | +|---|---| +| right after launch | `modal: "AssocPrompt"` | +| after `close_modal` | `modal: "DonationPrompt"` | +| after a second `close_modal` | `modal: null` | + +While a dialog is open, `{"op":"new"}` answers `ok` but no tab is created. +Close the dialogs first: + +```python +while (info := sessions())["modal"]: + execute({"op": "action", "name": "close_modal", "document_id": info["document_id"]}) +``` + +The donation prompt appears once per application version per profile, so an +automated profile meets it once per version rather than on every run. + +`new` makes the new drawing the active document. Requests carry `document_id`; +take it from the reply's state and pass it on the following requests, as the +README describes. + +## `--export` + +`OpenCADStudio --export IN OUT` loads `IN` and writes `OUT` without a window or +a session. The output format follows the extension: `.dxf` writes DXF, any other +extension writes DWG at the document's own version. The exit code is 0 on +success and 1 when the input cannot be read or the output cannot be written. + +## Interactive steps + +For commands whose answers cannot be written on one line, use `start` and then +`input`. `start` replies with `state.command`, whose `accepts`, `options` and +`input_example` say what the current step wants: + +| Step | Request | Note | +|---|---|---| +| a point | `{"op":"input","kind":"point","point":[x,y,0],"space":"wcs"}` | the point is an array; `space` is `wcs` (default), `ucs` or `relative` | +| a typed value (height, rotation) | `{"op":"input","kind":"text","text":"5"}` | digits go through `text`, not `token`; `{"op":"input","kind":"enter"}` accepts the default | +| a keyword (`J`, `ST`, `C`) | `{"op":"input","kind":"token","text":"C"}` | the payload field is named `text` | +| an object | `{"op":"input","kind":"entity","handle":"HANDLE","point":[x,y,0]}` | `kind: "structure"` for a sub-entity pick, `kind: "selection"` to hand over the current selection | +| the content of `TEXT` | `{"op":"action","name":"text_input","value":"hello"}` then `{"op":"action","name":"text_commit"}` | the command has already ended; the in-place editor owns the input | +| the content of `MTEXT` | `{"op":"action","name":"mtext_insert","value":"hello"}` then `{"op":"action","name":"mtext_commit"}` | the rich-text editor has its own actions; the `TEXT` ones answer `editor_closed` | + +## Native-only operations + +| Operation | `--serve` (protocol 1) | `--mcp` | +|---|---|---| +| measure | `{"protocol":1,"op":"measure","handles":["2A"]}` — kernel-computed length, area, bounds and mass properties; no window needed | `ocs_read` with `op: "measure"` and `parameters.handles` | +| capture | `{"protocol":1,"op":"capture","path":"out.png"}` — needs the editor window, so it answers `gui_required` here | `ocs_capture` with `scope` (`viewport` or `window`) and `max_dimension` | +| `set_properties` | requires `collection`; colours are the serialised enum — `{"Index": 1}` or `"ByLayer"`; a colour name such as `"Red"` is rejected with `invalid_value` | same request through `ocs_execute` | diff --git a/docs/automation/web.md b/docs/automation/web.md new file mode 100644 index 000000000..16a80c622 --- /dev/null +++ b/docs/automation/web.md @@ -0,0 +1,68 @@ +# Driving the web build + +The web build exposes the same control core as `--serve` and `--mcp`, through two +functions on the WebAssembly module. This page covers only what is specific to the +browser; the operations, request envelope and replies are the ones in the +[README](README.md). + +## Reaching the control channel + +The Trunk loader publishes the wasm-bindgen module as `window.wasmBindings`. The +control channel is ready once `window.wasmBindings?.ocs_control_submit` is defined +(the [save smoke test](web_save_smoke.cjs) waits for exactly that). + +| Function | Behaviour | +|---|---| +| `ocs_control_submit(request: string) → string` | Queues one request (a JSON string) and returns a ticket immediately. | +| `ocs_control_take(ticket: string) → string \| undefined` | Returns the reply (a JSON string) once it is ready, `undefined` before that. A reply is removed when taken. | + +Poll `ocs_control_take` until it returns a reply, as the smoke test does: + +```js +async function control(request) { + const api = window.wasmBindings; + const ticket = api.ocs_control_submit(JSON.stringify(request)); + for (let i = 0; i < 600; i++) { + const reply = api.ocs_control_take(ticket); + if (reply) return JSON.parse(reply); + await new Promise((resolve) => setTimeout(resolve, 50)); + } + throw new Error('control timed out'); +} +``` + +Limits: at most 64 requests wait in the queue (a further `submit` is answered with the +`busy` error), and only the newest 64 replies are kept, so take a reply soon after it is +ready. A request that is not valid JSON is answered with `op: "invalid"` and a +`parse_error`. + +## Opening a drawing from bytes + +The web build has no file system, so `open` takes the file's bytes instead of a path: + +```json +{"op":"open","name":"plan.dwg","data_base64":"…"} +``` + +`name` is reduced to its file name and labels the tab and the recent-files entry; +`data_base64` must be valid base64 and non-empty (`invalid_request` otherwise). While an +earlier drawing is still opening the request is answered with `busy`; retry it once +`state` shows the tab. A `path` is resolved against the browser's "recent files" store +rather than a disk, so use `data_base64` for a file the page has not seen. +`data_base64` is rejected on native builds; use `path` there. + +## Startup dialogs + +A dialog can be open when the page is ready. The donation prompt appears once per +application version in each browser profile, so a fresh or disposable profile always +sees it. While a modal is open `state.modal` is set, and `{"op":"new"}` returns `ok` +without creating a tab until the modal is closed. Close it first: + +```js +const state = await control({ op: 'state' }); +if (state.modal) await control({ op: 'action', name: 'close_modal' }); +await control({ op: 'new' }); +``` + +Native builds can show a second dialog at launch (the file-association prompt), so check +`state.modal` again after closing one. diff --git a/src/app/automation.rs b/src/app/automation.rs index fe64eb253..c383ddc96 100644 --- a/src/app/automation.rs +++ b/src/app/automation.rs @@ -352,6 +352,14 @@ impl OpenCADStudio { let i = self.active_tab; let before = self.tabs[i].scene.document.entities().count(); let error_revision = self.command_line.error_revision; + // Surfaces that are already open belong to an earlier line (or + // another tab, or startup). Only what *this* line left open is + // reported as a blocker, otherwise a finished command would keep + // answering `waiting_input` because of someone else's editor and + // a caller polling for `completed` would never get there. + let editor_before = self.text_inline.is_some(); + let mtext_before = self.mtext_editor.is_some(); + let modal_before = self.active_modal.is_some(); if let Err(error) = self.run_headless(&cmd) { return err(error); } @@ -359,12 +367,32 @@ impl OpenCADStudio { return err(self.command_line.last_error.clone().unwrap_or_default()); } let after = self.tabs[i].scene.document.entities().count(); + // A command can finish with an interactive surface still open: + // the in-place text editor (the `TEXT` content step), the MTEXT + // editor, or a modal. Reporting `completed` there hides the fact + // that nothing was committed, so reuse the `waiting_input` + // vocabulary and name the blocker. + let blocked_by: Option = if self.tabs[i].active_cmd.is_some() { + Some("command".to_string()) + } else if let (false, Some(modal)) = (modal_before, self.active_modal.as_ref()) { + Some(format!("modal:{modal:?}")) + } else if !editor_before && self.text_inline.is_some() { + Some("text_editor".to_string()) + } else if !mtext_before && self.mtext_editor.is_some() { + Some("mtext_editor".to_string()) + } else { + None + }; json!({ "ok": true, "cmd": cmd, - "status": if self.tabs[i].active_cmd.is_some() { "waiting_input" } else { "completed" }, + "status": if blocked_by.is_some() { "waiting_input" } else { "completed" }, + "blocked_by": blocked_by, "entities": after, "added": after as i64 - before as i64, + // Tokens no prompt ever asked for: leftover input is + // reported rather than dropped on the floor. + "unconsumed": self.command_line.unconsumed.clone(), }) } "entities" => self.entity_summary(), @@ -814,6 +842,128 @@ mod tests { assert!(app.tabs[i].scene.last_tess_wires.get() > before); } + #[test] + fn batch_text_line_creates_text_and_consumes_every_token() { + let mut app = OpenCADStudio::new_for_test(); + let _ = app.automation_op(r#"{"op":"new"}"#); + + // Regression: the content token used to be dropped once the in-place + // editor took over, so this line created nothing while reporting ok. + let r = app.automation_op(r#"{"op":"run","cmd":"TEXT 0,0 5 0 hello"}"#); + assert_eq!(r["ok"], true); + assert_eq!(r["status"], "completed", "{r}"); + assert_eq!(r["added"], 1, "{r}"); + assert_eq!(r["unconsumed"].as_array().map(|v| v.len()), Some(0), "{r}"); + + let counts = app.automation_op(r#"{"op":"entities"}"#); + assert_eq!(counts["by_type"]["Text"], 1, "{counts}"); + assert_eq!(counts["total"], 1, "no phantom entity: {counts}"); + assert_eq!(assert_one_text(&app), "hello"); + } + + #[test] + fn multi_word_text_keeps_the_spaces() { + let mut app = OpenCADStudio::new_for_test(); + let _ = app.automation_op(r#"{"op":"new"}"#); + + let r = app.automation_op(r#"{"op":"run","cmd":"TEXT 0,0 5 0 hello world"}"#); + assert_eq!(r["status"], "completed", "{r}"); + assert_eq!(r["added"], 1, "{r}"); + assert_eq!(r["unconsumed"].as_array().map(|v| v.len()), Some(0), "{r}"); + let counts = app.automation_op(r#"{"op":"entities"}"#); + assert_eq!(counts["by_type"]["Text"], 1, "{counts}"); + + // The spaces between the words have to survive the token split, and the + // whole tail (not just its first token) has to land in the entity — an + // off-by-one in the consumed index would still pass the checks above. + assert_eq!(assert_one_text(&app), "hello world"); + } + + #[test] + fn a_stray_editor_is_not_hijacked_by_the_next_line() { + let mut app = OpenCADStudio::new_for_test(); + let _ = app.automation_op(r#"{"op":"new"}"#); + + // Line 1 stops at the content step and leaves the editor open. + let r = app.automation_op(r#"{"op":"run","cmd":"TEXT 30,0 5 0"}"#); + assert_eq!(r["blocked_by"], "text_editor", "{r}"); + + // Line 2 must not type its leftover token into that unrelated editor, + // and — since line 2 did not open anything itself — it must not inherit + // line 1's blocker either: a caller that polls for `completed` would + // otherwise never get there. + let r = app.automation_op(r#"{"op":"run","cmd":"CIRCLE 0,0 5 9"}"#); + assert_eq!(r["added"], 1, "{r}"); + assert_eq!(r["unconsumed"][0], "9", "{r}"); + assert_eq!(r["blocked_by"], serde_json::Value::Null, "{r}"); + assert_eq!(r["status"], "completed", "{r}"); + let counts = app.automation_op(r#"{"op":"entities"}"#); + assert_eq!(counts["by_type"]["Circle"], 1, "{counts}"); + assert!( + counts["by_type"].get("Text").is_none(), + "phantom text from stray editor: {counts}" + ); + } + + #[test] + fn run_reports_leftover_tokens_instead_of_dropping_them() { + let mut app = OpenCADStudio::new_for_test(); + let _ = app.automation_op(r#"{"op":"new"}"#); + + // The radius step ends the command, so the extra token is never asked for. + let r = app.automation_op(r#"{"op":"run","cmd":"CIRCLE 0,0 5 9"}"#); + assert_eq!(r["added"], 1, "{r}"); + assert_eq!(r["unconsumed"][0], "9", "{r}"); + + // A fully consumed line reports nothing left over. + let r = app.automation_op(r#"{"op":"run","cmd":"LINE 0,0 10,10"}"#); + assert_eq!(r["unconsumed"].as_array().map(|v| v.len()), Some(0), "{r}"); + } + + #[test] + fn text_without_content_reports_the_open_editor() { + let mut app = OpenCADStudio::new_for_test(); + let _ = app.automation_op(r#"{"op":"new"}"#); + + let r = app.automation_op(r#"{"op":"run","cmd":"TEXT 0,0 5 0"}"#); + assert_eq!(r["status"], "waiting_input", "{r}"); + assert_eq!(r["blocked_by"], "text_editor", "{r}"); + let counts = app.automation_op(r#"{"op":"entities"}"#); + assert_eq!(counts["total"], 0, "{counts}"); + + // The same editor can be finished through the control-surface messages. + let _ = app.update(crate::app::Message::TextInlineInput("hello".into())); + let _ = app.update(crate::app::Message::TextInlineOk); + let counts = app.automation_op(r#"{"op":"entities"}"#); + assert_eq!(counts["by_type"]["Text"], 1, "{counts}"); + } + + #[test] + fn a_command_still_waiting_is_named_as_the_blocker() { + let mut app = OpenCADStudio::new_for_test(); + let _ = app.automation_op(r#"{"op":"new"}"#); + + let r = app.automation_op(r#"{"op":"run","cmd":"LINE"}"#); + assert_eq!(r["status"], "waiting_input", "{r}"); + assert_eq!(r["blocked_by"], "command", "{r}"); + } + + /// The text of the single `Text` entity in the drawing (panics otherwise). + fn assert_one_text(app: &OpenCADStudio) -> String { + let i = app.active_tab; + let texts: Vec = app.tabs[i] + .scene + .document + .entities() + .filter_map(|e| match e { + acadrust::EntityType::Text(t) => Some(t.value.clone()), + _ => None, + }) + .collect(); + assert_eq!(texts.len(), 1, "expected exactly one Text entity, got {texts:?}"); + texts.into_iter().next().unwrap() + } + #[test] fn automation_ops_round_trip() { let mut app = OpenCADStudio::new_for_test(); diff --git a/src/app/command_driver.rs b/src/app/command_driver.rs index 87abf6df6..0e21a8e6e 100644 --- a/src/app/command_driver.rs +++ b/src/app/command_driver.rs @@ -104,6 +104,7 @@ impl OpenCADStudio { self.snapper.from_point = None; self.snapper.clear_tracking(); self.otrack_active = None; + self.otrack_cross = None; self.otrack_kind = None; self.axis_lock_dir = None; self.dyn_user_reshaped = false; @@ -566,6 +567,7 @@ impl OpenCADStudio { /// the headless automation feeder so both behave identically. pub(super) fn run_command_line(&mut self, cmd: &str) -> Task { let i = self.active_tab; + self.command_line.unconsumed.clear(); let tokens: Vec<&str> = cmd.split_whitespace().collect(); if tokens.len() <= 1 { return self.dispatch_command(cmd); @@ -597,18 +599,64 @@ impl OpenCADStudio { /// Feed `tokens[1..]` to the active interactive command as points / option /// keywords, then terminate it as if Enter were pressed. No-op when no /// command is active. + /// + /// Two things happen when the tokens run out mid-way, both needed so a + /// headless caller can tell what actually happened: + /// + /// * If the in-place text editor took over (the `TEXT` content step — the + /// command itself has already ended by then, `active_cmd` is `None`), the + /// remaining tokens are that text: they are typed into the editor and + /// committed, the same messages the control surface's `text_input` / + /// `text_commit` actions dispatch. Without this the tail of the line was + /// silently dropped and `TEXT 0,0 5 0 hi` created nothing at all. + /// * Anything still left over was never claimed by any prompt; it is + /// recorded in [`CommandLine::unconsumed`] instead of vanishing. pub(super) fn finish_active_command(&mut self, tokens: &[String]) -> Task { let i = self.active_tab; if self.tabs[i].active_cmd.is_none() { return Task::none(); } self.last_point = None; + // An editor that is *already* open belongs to someone else (an earlier + // line that stopped at the content step). Only the editor this line + // opens by feeding its own tokens may be handed the tail. + let editor_was_open = self.text_inline.is_some(); let mut tasks = Vec::new(); + // First token is the command verb itself; prompts start consuming after it. + let mut consumed = 1; for tok in &tokens[1..] { if self.tabs[i].active_cmd.is_none() { break; } tasks.push(self.feed_active_cmd(tok)); + consumed += 1; + } + if !editor_was_open && self.text_inline.is_some() && consumed < tokens.len() { + // Fill the editor the same way `Message::TextInlineInput` does, then + // commit *synchronously* so the outcome is observable: dispatching + // `TextInlineOk` as a task would hide whether the entity was really + // created, and the tail token would be counted as consumed either + // way — the caller would have no way to tell "text created" from + // "text silently dropped". + if let Some(editor) = self.text_inline.as_mut() { + editor.value = tokens[consumed..].join(" "); + } + let committed = self.text_inline_commit(); + tasks.push(self.post_editor_closed(committed)); + if committed { + consumed = tokens.len(); + // `TEXT` repeats by design: on a committed string it re-arms for + // the next line (`open_next_line`), which re-opens the editor. A + // batch line is a complete unit, and leaving that repeat armed + // would make the *next* line get eaten as the next text position, + // so end the command here, exactly as Esc does in the GUI. + tasks.push(Task::done(Message::CommandEscape)); + } + // On a failed commit the tokens stay unconsumed (assigned below), so + // the caller still sees the text it passed in. + } + if consumed < tokens.len() { + self.command_line.unconsumed = tokens[consumed..].to_vec(); } tasks.push(self.feed_command(StepInput::Enter)); Task::batch(tasks) @@ -1849,6 +1897,9 @@ impl OpenCADStudio { .active_cmd .as_ref() .is_some_and(|command| command.preserve_commit_layer()); + // Task produced by a command the arm dispatches; it must reach the + // runtime or messages such as a chosen render mode are dropped. + let mut dispatched = Task::none(); match result { CmdResult::OpenAutoConstrainSettings => { self.auto_constrain_saved = Some(self.auto_constrain_settings.clone()); @@ -4754,7 +4805,7 @@ impl OpenCADStudio { self.tabs[i].snap_result = None; self.tabs[i].scene.clear_preview_wire(); self.restore_pre_cmd_tangent(); - let _ = self.dispatch_command(&cmd); + dispatched = self.dispatch_command(&cmd); } CmdResult::Dispatch(cmd) => { // End this interactive front-end, then run the assembled command @@ -4763,7 +4814,7 @@ impl OpenCADStudio { self.tabs[i].snap_result = None; self.tabs[i].scene.clear_preview_wire(); self.restore_pre_cmd_tangent(); - let _ = self.dispatch_command(&cmd); + dispatched = self.dispatch_command(&cmd); } CmdResult::EditTableCell { handle, point } => { // TABLEDIT's pick: end the pick phase and hand (table, point) @@ -7494,16 +7545,15 @@ impl OpenCADStudio { // The rich text canvas owns keyboard editing itself. Leaving the // hidden command input focused would make it consume Left/Right before // the editor can handle them. - if self.mtext_editor.is_some() { - return self.unfocus_widgets(); - } - // The in-place TEXT editor needs keyboard focus on its own field. - if self.text_inline.is_some() { - return iced::widget::operation::focus(iced::widget::Id::new( - super::view::TEXT_INLINE_ID, - )); - } - self.focus_cmd_input() + let focus = if self.mtext_editor.is_some() { + self.unfocus_widgets() + } else if self.text_inline.is_some() { + // The in-place TEXT editor needs keyboard focus on its own field. + iced::widget::operation::focus(iced::widget::Id::new(super::view::TEXT_INLINE_ID)) + } else { + self.focus_cmd_input() + }; + Task::batch([dispatched, focus]) } /// Restore the tangent-snap / ortho state that was in effect before the command started. @@ -10101,3 +10151,42 @@ mod thicken_tests { assert!(app.tabs[i].scene.document.get_entity(source).is_some()); } } + +#[cfg(test)] +mod dispatched_command_task_tests { + use super::*; + use crate::command::CmdResult; + use acadrust::entities::ViewportRenderMode as Mode; + + /// `CmdResult::Dispatch` and `CmdResult::Relaunch` run the assembled line + /// through `dispatch_command`, and the Task it returns must reach the + /// runtime: the render mode a VSCURRENT keyword picker chooses is applied + /// by a message that Task carries. + #[test] + fn dispatch_and_relaunch_keep_the_task_the_command_returns() { + let mut app = OpenCADStudio::new_for_test(); + let _ = app.automation_op(r#"{"op":"new"}"#); + let i = app.active_tab; + + app.tabs[i].render_mode = Mode::Wireframe2D; + let task = app.apply_cmd_result(CmdResult::Dispatch("VSCURRENT GOURAUDSHADED".into())); + app.drive_headless_task(task).unwrap(); + assert_eq!( + app.tabs[i].render_mode, + Mode::GouraudShaded, + "Dispatch dropped the dispatched command's task" + ); + + app.tabs[i].render_mode = Mode::Wireframe2D; + let task = app.apply_cmd_result(CmdResult::Relaunch( + "VSCURRENT GOURAUDSHADED".into(), + Vec::new(), + )); + app.drive_headless_task(task).unwrap(); + assert_eq!( + app.tabs[i].render_mode, + Mode::GouraudShaded, + "Relaunch dropped the dispatched command's task" + ); + } +} diff --git a/src/app/mod.rs b/src/app/mod.rs index 7ede6f889..6714e04df 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -502,6 +502,11 @@ pub(super) struct OpenCADStudio { /// cursor is on a tracking ray. Lets a typed distance place a point along /// the ray from the tracking point (issue #69). `None` when not aligned. otrack_active: Option<(glam::DVec3, glam::DVec3)>, + /// The vector `otrack_active` crosses, when the lock is the meeting of two + /// tracking vectors. Drawn beside the first so the user can see that the + /// point is their intersection; a typed distance still runs along + /// `otrack_active` alone. `None` for a single-ray alignment. (#1313) + otrack_cross: Option<(glam::DVec3, glam::DVec3)>, /// Active OTRACK ray kind, separate from typed-distance geometry. otrack_kind: Option, /// Whether Tangent snap was enabled before a tangent-pick command started. @@ -3873,6 +3878,7 @@ impl OpenCADStudio { annotation_auto_scale: -4, last_saved_config: None, otrack_active: None, + otrack_cross: None, otrack_kind: None, clean_screen: false, quick_properties: false, diff --git a/src/app/plugin_host.rs b/src/app/plugin_host.rs index 6b38b0ca5..3e1c19e7d 100644 --- a/src/app/plugin_host.rs +++ b/src/app/plugin_host.rs @@ -1048,6 +1048,130 @@ impl<'a> HostSession<'a> { pub fn push_error(&mut self, msg: &str) { self.app.command_line.push_error(msg); } + + pub fn add_layer(&mut self, config: ocs_plugin_api::host::LayerConfig) -> Option { + let trimmed = config.name.trim(); + if trimmed.is_empty() { + return None; + } + + let doc = self.document_mut(); + if doc.layers.contains(trimmed) { + return None; + } + + let resolved_lt = match config.linetype { + // A name the drawing does not carry would leave the LAYER record + // pointing at no LTYPE, so it is refused rather than written. + Some(ref lt_name) => match resolve_linetype(doc, lt_name) { + Some(name) => name, + None => return None, + }, + None => "Continuous".to_string(), + }; + + let mut layer = acadrust::tables::Layer::new(trimmed); + let handle = doc.allocate_handle(); + layer.handle = handle; + layer.color = config.color.unwrap_or(acadrust::types::Color::Index(7)); + layer.line_type = resolved_lt; + layer.line_weight = config.lineweight.unwrap_or(acadrust::types::LineWeight::ByLayer); + layer.flags.off = config.off.unwrap_or(false); + if let Some(frz) = config.frozen { + if frz { + layer.freeze(); + } else { + layer.thaw(); + } + } + layer.flags.locked = config.locked.unwrap_or(false); + layer.is_plottable = config.plottable.unwrap_or(true); + layer.transparency = config.transparency.unwrap_or(acadrust::types::Transparency::ByLayer); + layer.description = config.description.unwrap_or_default(); + + let _ = doc.layers.add(layer); + + self.app.tabs[self.tab].dirty = true; + self.app.tabs[self.tab] + .scene + .invalidate_layer_dependencies(&[trimmed.to_string()]); + self.app.refresh_layer_panel(); + self.publish_document_view(); + Some(handle) + } + + pub fn modify_layer(&mut self, config: ocs_plugin_api::host::LayerConfig) -> bool { + let trimmed = config.name.trim(); + if trimmed.is_empty() { + return false; + } + + let resolved_lt = match config.linetype { + Some(ref lt_name) => match resolve_linetype(self.document_mut(), lt_name) { + Some(name) => Some(name), + None => return false, + }, + None => None, + }; + + let doc = self.document_mut(); + let Some(existing) = doc.layers.get_mut(trimmed) else { + return false; + }; + + if let Some(c) = config.color { + existing.color = c; + existing.color_name = None; + existing.book_name = None; + } + if let Some(lt) = resolved_lt { + existing.line_type = lt; + } + if let Some(lw) = config.lineweight { + existing.line_weight = lw; + } + if let Some(off) = config.off { + existing.flags.off = off; + } + if let Some(frz) = config.frozen { + if frz { + existing.freeze(); + } else { + existing.thaw(); + } + } + if let Some(lck) = config.locked { + existing.flags.locked = lck; + } + if let Some(plt) = config.plottable { + existing.is_plottable = plt; + } + if let Some(tr) = config.transparency { + existing.transparency = tr; + } + if let Some(desc) = config.description { + existing.description = desc; + } + + self.app.tabs[self.tab].dirty = true; + self.app.tabs[self.tab] + .scene + .invalidate_layer_dependencies(&[trimmed.to_string()]); + self.app.refresh_layer_panel(); + self.publish_document_view(); + true + } +} + +/// The stored spelling of `name` in the drawing's linetype table, loading the +/// standard linetypes first when it is not there yet. `None` when the drawing +/// cannot supply it: a layer must not reference a linetype that does not exist. +fn resolve_linetype(doc: &mut acadrust::CadDocument, name: &str) -> Option { + let stored = |doc: &acadrust::CadDocument| doc.line_types.get(name).map(|lt| lt.name.clone()); + stored(doc).or_else(|| { + crate::io::linetypes::populate_document(doc); + stored(doc) + }) } /// The stable contract a plugin's `dispatch` sees. Each method forwards to the @@ -1180,6 +1304,12 @@ impl HostApi for HostSession<'_> { fn close_document_view_v4(&mut self, tab_id: u64) { self.close_document_view_v4(tab_id) } + fn add_layer(&mut self, config: ocs_plugin_api::host::LayerConfig) -> Option { + self.add_layer(config) + } + fn modify_layer(&mut self, config: ocs_plugin_api::host::LayerConfig) -> bool { + self.modify_layer(config) + } } /// Bridges a plugin's [`InteractiveCommand`](ocs_plugin_api::host::InteractiveCommand) @@ -6891,4 +7021,63 @@ mod tests { assert_eq!(got.values.len(), 1); assert!(matches!(got.values[0], XDataValue::Integer32(123))); } + + #[test] + fn test_plugin_add_layer_with_defaults_and_modify() { + use ocs_plugin_api::host::LayerConfig; + + let mut app = OpenCADStudio::new_for_test(); + app.tabs[0].is_start = false; + let mut host = HostSession::new(&mut app, 0); + + // 1. Add layer with minimal config (only name specified) + let config = LayerConfig { + name: "ELECTRICAL".to_string(), + ..Default::default() + }; + let handle = host.add_layer(config).expect("should create layer"); + assert_ne!(handle, acadrust::Handle::NULL); + + // Verify defaults were applied + let layer = host.document().layers.get("ELECTRICAL").expect("layer should exist"); + assert_eq!(layer.color, acadrust::types::Color::Index(7)); + assert_eq!(layer.line_type, "Continuous"); + assert_eq!(layer.line_weight, acadrust::types::LineWeight::ByLayer); + assert!(!layer.flags.off); + assert!(!layer.flags.frozen); + assert!(!layer.flags.locked); + assert!(layer.is_plottable); + + // 2. Duplicate add_layer should be rejected (return None) + let dup_config = LayerConfig { + name: "ELECTRICAL".to_string(), + color: Some(acadrust::types::Color::Index(1)), + ..Default::default() + }; + assert!(host.add_layer(dup_config).is_none(), "duplicate layer should return None"); + + // 3. Modify only color and locked; other properties should remain untouched + let mod_config = LayerConfig { + name: "ELECTRICAL".to_string(), + color: Some(acadrust::types::Color::Index(1)), + locked: Some(true), + ..Default::default() + }; + assert!(host.modify_layer(mod_config)); + + let updated = host.document().layers.get("ELECTRICAL").expect("layer should exist"); + assert_eq!(updated.color, acadrust::types::Color::Index(1)); // Modified to red + assert!(updated.flags.locked); // Modified to locked + assert_eq!(updated.line_type, "Continuous"); // Kept as-is + assert_eq!(updated.line_weight, acadrust::types::LineWeight::ByLayer); // Kept as-is + assert!(!updated.flags.off); // Kept as-is + + // 4. Modify nonexistent layer returns false + let non_existent = LayerConfig { + name: "DOES_NOT_EXIST".to_string(), + color: Some(acadrust::types::Color::Index(2)), + ..Default::default() + }; + assert!(!host.modify_layer(non_existent)); + } } diff --git a/src/app/properties.rs b/src/app/properties.rs index c0ed6ce64..b96b37e04 100644 --- a/src/app/properties.rs +++ b/src/app/properties.rs @@ -2683,6 +2683,13 @@ handles={handles_ms:.1} panel={:.1} ribbon={ribbon_ms:.1} tail={:.1} selected={} annotation_scale_handle, ); let mut entity_grips = dispatch::grips(contextual.as_ref()); + if let acadrust::EntityType::Insert(insert) = contextual.as_ref() { + entity_grips = crate::entities::insert::visible_attribute_grips( + &self.tabs[i].scene.document, + insert, + self.tabs[i].scene.annotation_scale, + ); + } if crate::scene::model::solid_history::has_specialized_primitive_properties( &self.tabs[i].scene.document, handle, diff --git a/src/app/update/mod.rs b/src/app/update/mod.rs index 3c30ccf62..ee0ea533b 100644 --- a/src/app/update/mod.rs +++ b/src/app/update/mod.rs @@ -401,6 +401,7 @@ impl OpenCADStudio { { self.snapper.clear_tracking(); self.otrack_active = None; + self.otrack_cross = None; self.otrack_kind = None; } if let Some((started, tab_id, before)) = perf_edit_before { @@ -437,6 +438,7 @@ impl OpenCADStudio { pub(in crate::app) fn reset_tracking_after_point(&mut self) { self.snapper.clear_tracking(); self.otrack_active = None; + self.otrack_cross = None; self.otrack_kind = None; } @@ -4017,6 +4019,7 @@ impl OpenCADStudio { if !self.snapper.otrack_enabled { self.snapper.clear_tracking(); self.otrack_active = None; + self.otrack_cross = None; self.otrack_kind = None; } Task::none() diff --git a/src/app/update/viewport.rs b/src/app/update/viewport.rs index f316976fd..c5279e5a8 100644 --- a/src/app/update/viewport.rs +++ b/src/app/update/viewport.rs @@ -1479,6 +1479,7 @@ impl OpenCADStudio { Some((base, dir)) }); self.otrack_active = otrack_hit.map(|hit| (hit.base, hit.dir)).or(drafting_guide); + self.otrack_cross = otrack_hit.and_then(|hit| hit.cross); self.otrack_kind = otrack_hit.map(|hit| hit.kind); @@ -2190,6 +2191,7 @@ impl OpenCADStudio { None }; self.otrack_active = otrack_hit.map(|h| (h.base, h.dir)); + self.otrack_cross = otrack_hit.and_then(|h| h.cross); self.otrack_kind = otrack_hit.map(|h| h.kind); // Parallel snap: with nothing else snapped or tracked, lock @@ -2208,6 +2210,7 @@ impl OpenCADStudio { (self.last_point, self.snapper.parallel_ref) { self.otrack_active = Some((base, dir.as_dvec3())); + self.otrack_cross = None; } self.tabs[i].snap_result = Some(par); } diff --git a/src/app/view/mod.rs b/src/app/view/mod.rs index 52d4e18cb..dc6679743 100644 --- a/src/app/view/mod.rs +++ b/src/app/view/mod.rs @@ -770,15 +770,26 @@ bg={bg_ms:.1}ms n={view_count}" // The active alignment vector: a dashed guide from the acquired // tracking point through the locked cursor, so the user sees the // extension / tracking line they are snapped to (#219). - let otrack_line: Option<(iced::Point, iced::Point)> = - match (otrack_proj, self.otrack_active) { - (Some((view_rot, eye, ob)), Some((base, _dir))) => { - let b = ost_project(base, view_rot, eye, ob); - let a = ost_project(tab.last_cursor_world, view_rot, eye, ob); - (b.x.is_finite() && a.x.is_finite()).then_some((b, a)) - } - _ => None, - }; + // An intersection lock is the meeting of two tracking vectors, so + // both are drawn — one guide alone hides what the point is (#1313). + let otrack_lines: Vec<(iced::Point, iced::Point)> = match otrack_proj { + Some((view_rot, eye, ob)) => { + let a = ost_project(tab.last_cursor_world, view_rot, eye, ob); + self.otrack_active + .into_iter() + .chain(self.otrack_cross) + .filter_map(|(base, _dir)| { + let b = ost_project(base, view_rot, eye, ob); + (b.x.is_finite() + && b.y.is_finite() + && a.x.is_finite() + && a.y.is_finite()) + .then_some((b, a)) + }) + .collect() + } + None => vec![], + }; // The acquired Parallel-snap reference, marked on its line (#277). let parallel_ref_marker: Option = match (otrack_proj, self.snapper.parallel_ref) { @@ -894,7 +905,7 @@ bg={bg_ms:.1}ms n={view_count}" grip_clip, ucs_icons, ost_points, - otrack_line, + otrack_lines, parallel_ref_marker, // ViewCube hover region matches the drawn cube — gone when hidden. !is_paper && viewcube_visible, diff --git a/src/entities/insert.rs b/src/entities/insert.rs index 37f7dcd7f..5fe92b024 100644 --- a/src/entities/insert.rs +++ b/src/entities/insert.rs @@ -1,22 +1,172 @@ use acadrust::entities::Insert; use acadrust::types::{Matrix3, Transform, Vector3}; -use acadrust::{EntityType, Handle}; +use acadrust::{CadDocument, EntityType, Handle}; use glam::Vec3; use crate::command::EntityTransform; -use crate::entities::common::{edit_angle_prop as edit_angle, edit_prop as edit, parse_f64, ro_prop as ro, square_grip}; +use crate::entities::common::{ + edit_angle_prop as edit_angle, edit_prop as edit, parse_f64, ro_prop as ro, square_grip, +}; +use crate::entities::traits::Grippable; -use crate::scene::model::object::{GripApply, GripDef, PropSection, PropValue, Property}; -use crate::scene::model::wire_model::WireModel; use crate::scene::cache::block_cache; use crate::scene::convert::tessellate; +use crate::scene::model::object::{GripApply, GripDef, PropSection, PropValue, Property}; +use crate::scene::model::wire_model::WireModel; use crate::scene::view::render; +/// Grip ids from here up address the INSERT's attributes — `BASE + i` is +/// `attributes[i]`. Grip 0 stays the block's own insertion point. +const ATTRIBUTE_GRIP_BASE: usize = 1; + +/// Whether an attribute is one the user can pick up and reposition. A constant +/// attribute belongs to the block definition rather than this insert, and a +/// locked position cannot be dragged. Visibility is a separate question: an +/// invisible attribute that ATTMODE 2 displays is grippable while it shows. +fn attribute_is_movable(att: &acadrust::entities::AttributeEntity) -> bool { + !att.flags.constant && !att.lock_position && !att.flags.locked_position +} + +/// A dragged attribute grip, mapped back to the attribute's stored position. +/// The grip is placed on the displayed attribute, which an annotative insert +/// scales about its insertion point; the target has to be unscaled the same +/// way or the attribute lands `scale` times too far and the grip jumps. +pub(crate) fn unscale_attribute_grip( + insert: &Insert, + annotation_scale: f32, + grip_id: usize, + apply: GripApply, +) -> GripApply { + if grip_id < ATTRIBUTE_GRIP_BASE { + return apply; + } + let block_scale = insert_attribute_block_scale( + insert, + annotation_scale, + crate::scene::BlockScalePolicy::FromInsert, + ); + if (block_scale - 1.0).abs() <= 1.0e-6 { + return apply; + } + let origin = glam::DVec3::new(insert.insert_point.x, insert.insert_point.y, insert.insert_point.z); + match apply { + GripApply::Absolute(target) => GripApply::Absolute(origin + (target - origin) / block_scale), + GripApply::Translate(delta) => GripApply::Translate(delta / block_scale), + } +} + +fn attribute_is_render_visible( + document: &CadDocument, + visibility: i16, + attribute: &acadrust::entities::AttributeEntity, +) -> bool { + let layer_visible = document + .layers + .get(&attribute.common.layer) + .is_none_or(|layer| !layer.flags.off && !layer.flags.frozen); + layer_visible + && visibility != 0 + && (visibility == 2 || (!attribute.common.invisible && !attribute.flags.invisible)) +} + +fn insert_attribute_block_scale( + insert: &Insert, + annotation_scale: f32, + scale_policy: crate::scene::BlockScalePolicy, +) -> f64 { + if scale_policy == crate::scene::BlockScalePolicy::FromInsert + && insert + .common + .extended_data + .get_record("AcAnnotativeData") + .is_some() + { + annotation_scale as f64 + } else { + 1.0 + } +} + +fn scale_attribute_for_insert( + insert: &Insert, + attribute: &mut acadrust::entities::AttributeEntity, + block_scale: f64, +) { + if (block_scale - 1.0).abs() <= 1.0e-6 { + return; + } + let insertion = insert.insert_point; + let scale_about = |point: Vector3| { + Vector3::new( + insertion.x + (point.x - insertion.x) * block_scale, + insertion.y + (point.y - insertion.y) * block_scale, + insertion.z + (point.z - insertion.z) * block_scale, + ) + }; + attribute.height *= block_scale; + attribute.insertion_point = scale_about(attribute.insertion_point); + attribute.alignment_point = scale_about(attribute.alignment_point); +} + fn grips(ins: &Insert) -> Vec { // `insert_point` is in the OCS defined by `normal`; the grip must sit at // the world placement, so map it through the OCS. Identity for +Z. let w = Matrix3::arbitrary_axis(ins.normal) * ins.insert_point; - vec![square_grip(0, glam::DVec3::new(w.x, w.y, w.z))] + let mut out = vec![square_grip(0, glam::DVec3::new(w.x, w.y, w.z))]; + // One grip per attribute, so an attribute can be moved off the place the + // block definition put it without dragging the whole block (#1259). + // Attributes are stored beside the block in world space — the same space + // `apply_grip` already translates them in. + // Without the document there is no ATTMODE to consult, so only an + // attribute that is drawn on its own terms gets a grip here; + // `visible_attribute_grips` is the display-aware list. + out.extend(ins.attributes.iter().enumerate().filter_map(|(i, att)| { + if !attribute_is_movable(att) || att.flags.invisible || att.common.invisible { + return None; + } + let grip = Grippable::grips(att).into_iter().next()?; + Some(GripDef { + id: ATTRIBUTE_GRIP_BASE + i, + ..grip + }) + })); + out +} + +pub(crate) fn visible_attribute_grips( + document: &CadDocument, + insert: &Insert, + annotation_scale: f32, +) -> Vec { + let w = Matrix3::arbitrary_axis(insert.normal) * insert.insert_point; + let mut out = vec![square_grip(0, glam::DVec3::new(w.x, w.y, w.z))]; + let visibility = document.header.attribute_visibility; + let block_scale = insert_attribute_block_scale( + insert, + annotation_scale, + crate::scene::BlockScalePolicy::FromInsert, + ); + out.extend( + insert + .attributes + .iter() + .enumerate() + .filter_map(|(i, attribute)| { + if !attribute_is_movable(attribute) + || !attribute_is_render_visible(document, visibility, attribute) + { + return None; + } + let mut attribute = attribute.clone(); + scale_attribute_for_insert(insert, &mut attribute, block_scale); + let grip = Grippable::grips(&attribute).into_iter().next()?; + Some(GripDef { + id: ATTRIBUTE_GRIP_BASE + i, + ..grip + }) + }), + ); + out } fn properties(ins: &Insert) -> Vec { @@ -108,7 +258,14 @@ fn apply_geom_prop(ins: &mut Insert, field: &str, value: &str) { } } -fn apply_grip(ins: &mut Insert, _grip_id: usize, apply: GripApply) { +fn apply_grip(ins: &mut Insert, grip_id: usize, apply: GripApply) { + // An attribute grip moves that attribute alone; the block stays put. + if let Some(index) = grip_id.checked_sub(ATTRIBUTE_GRIP_BASE) { + if let Some(att) = ins.attributes.get_mut(index).filter(|a| attribute_is_movable(a)) { + Grippable::apply_grip(att, 0, apply); + } + return; + } // The grip works in world space, but `insert_point` is stored in the OCS // defined by `normal`. Round-trip through the OCS so dragging a block // whose extrusion direction isn't +Z moves along world axes. Identity OCS @@ -195,45 +352,14 @@ pub(crate) fn insert_attribute_entities( if visibility == 0 || insert.attributes.is_empty() { return Vec::new(); } - let block_scale = if scale_policy - == crate::scene::BlockScalePolicy::FromInsert - && insert - .common - .extended_data - .get_record("AcAnnotativeData") - .is_some() - { - annotation_scale as f64 - } else { - 1.0 - }; - let insertion = insert.insert_point; - let scale_about = |point: Vector3| { - Vector3::new( - insertion.x + (point.x - insertion.x) * block_scale, - insertion.y + (point.y - insertion.y) * block_scale, - insertion.z + (point.z - insertion.z) * block_scale, - ) - }; + let block_scale = insert_attribute_block_scale(insert, annotation_scale, scale_policy); insert .attributes .iter() - .filter(|attribute| { - let layer_visible = document - .layers - .get(&attribute.common.layer) - .is_none_or(|layer| !layer.flags.off && !layer.flags.frozen); - layer_visible - && (visibility == 2 - || (!attribute.common.invisible && !attribute.flags.invisible)) - }) + .filter(|attribute| attribute_is_render_visible(document, visibility, attribute)) .map(|attribute| { let mut attribute = attribute.clone(); - if (block_scale - 1.0).abs() > 1.0e-6 { - attribute.height *= block_scale; - attribute.insertion_point = scale_about(attribute.insertion_point); - attribute.alignment_point = scale_about(attribute.alignment_point); - } + scale_attribute_for_insert(insert, &mut attribute, block_scale); EntityType::AttributeEntity(attribute) }) .collect() @@ -328,3 +454,143 @@ pub(crate) fn append_insert_attribute_wires( } } } + +#[cfg(test)] +mod tests { + use super::*; + use acadrust::entities::AttributeEntity; + use acadrust::tables::Layer; + use acadrust::xdata::ExtendedDataRecord; + + fn attribute(tag: &str, x: f64, y: f64) -> AttributeEntity { + let mut att = AttributeEntity::new(tag.to_string(), format!("{tag}-value")); + att.insertion_point = Vector3::new(x, y, 0.0); + att.alignment_point = Vector3::new(x, y, 0.0); + att + } + + fn block_with(attributes: Vec) -> Insert { + let mut ins = Insert::new("BLOCK", Vector3::new(0.0, 0.0, 0.0)); + ins.attributes = attributes; + ins + } + + fn document_with_layers(layers: Vec) -> CadDocument { + let mut document = CadDocument::new(); + for layer in layers { + document.layers.add_or_replace(layer); + } + document + } + + /// #1259: selecting a block showed only the insertion grip, so there was no + /// handle to pick an attribute up by and nowhere to drop it. + #[test] + fn each_attribute_offers_its_own_grip() { + let ins = block_with(vec![attribute("TAG_A", 3.0, 4.0), attribute("TAG_B", 6.0, 8.0)]); + + let g = grips(&ins); + assert_eq!(g.len(), 3, "insertion point plus one grip per attribute"); + assert_eq!(g[0].world, glam::DVec3::new(0.0, 0.0, 0.0)); + assert_eq!(g[1].world, glam::DVec3::new(3.0, 4.0, 0.0)); + assert_eq!(g[2].world, glam::DVec3::new(6.0, 8.0, 0.0)); + } + + /// Dragging an attribute grip moves that attribute and nothing else — the + /// block and its other attributes stay where they were. + #[test] + fn attribute_grip_moves_only_that_attribute() { + let mut ins = block_with(vec![attribute("TAG_A", 3.0, 4.0), attribute("TAG_B", 6.0, 8.0)]); + + let moved = grips(&ins)[1].id; + apply_grip(&mut ins, moved, GripApply::Absolute(glam::DVec3::new(30.0, 40.0, 0.0))); + + assert_eq!(ins.insert_point, Vector3::new(0.0, 0.0, 0.0)); + assert_eq!(ins.attributes[0].insertion_point, Vector3::new(30.0, 40.0, 0.0)); + assert_eq!(ins.attributes[0].alignment_point, Vector3::new(30.0, 40.0, 0.0)); + assert_eq!(ins.attributes[1].insertion_point, Vector3::new(6.0, 8.0, 0.0)); + } + + /// The insertion grip keeps carrying the attributes along with the block, + /// which is what the attribute grips are an alternative to, not a + /// replacement for (#255). + #[test] + fn insertion_grip_still_carries_the_attributes() { + let mut ins = block_with(vec![attribute("TAG_A", 3.0, 4.0)]); + + apply_grip(&mut ins, 0, GripApply::Translate(glam::DVec3::new(10.0, 0.0, 0.0))); + + assert_eq!(ins.insert_point, Vector3::new(10.0, 0.0, 0.0)); + assert_eq!(ins.attributes[0].insertion_point, Vector3::new(13.0, 4.0, 0.0)); + } + + /// A constant attribute belongs to the block definition and an invisible + /// one is not drawn, so neither gets a grip — and a grip id must still + /// address the attribute it was built from once some are skipped. + #[test] + fn skipped_attributes_neither_grip_nor_shift_the_others() { + let mut constant = attribute("TAG_CONST", 1.0, 1.0); + constant.flags.constant = true; + let mut invisible = attribute("TAG_HIDDEN", 2.0, 2.0); + invisible.flags.invisible = true; + let mut locked = attribute("TAG_LOCKED", 4.0, 4.0); + locked.lock_position = true; + let mut ins = block_with(vec![constant, invisible, locked, attribute("TAG_OK", 5.0, 5.0)]); + + let g = grips(&ins); + assert_eq!(g.len(), 2, "only the movable attribute is gripped: {g:?}"); + assert_eq!(g[1].world, glam::DVec3::new(5.0, 5.0, 0.0)); + + apply_grip(&mut ins, g[1].id, GripApply::Absolute(glam::DVec3::new(50.0, 50.0, 0.0))); + assert_eq!(ins.attributes[3].insertion_point, Vector3::new(50.0, 50.0, 0.0)); + assert_eq!(ins.attributes[0].insertion_point, Vector3::new(1.0, 1.0, 0.0)); + } + + #[test] + fn visible_attribute_grips_match_render_visibility() { + let mut off_layer = Layer::new("OFF"); + off_layer.flags.off = true; + let mut frozen_layer = Layer::new("FROZEN"); + frozen_layer.flags.frozen = true; + let mut document = document_with_layers(vec![off_layer, frozen_layer]); + document.header.attribute_visibility = 1; + + let mut common_invisible = attribute("TAG_COMMON_INVISIBLE", 1.0, 1.0); + common_invisible.common.invisible = true; + let mut off = attribute("TAG_OFF", 2.0, 2.0); + off.common.layer = "OFF".into(); + let mut frozen = attribute("TAG_FROZEN", 3.0, 3.0); + frozen.common.layer = "FROZEN".into(); + let visible = attribute("TAG_VISIBLE", 4.0, 4.0); + let ins = block_with(vec![common_invisible, off, frozen, visible]); + + let grips = visible_attribute_grips(&document, &ins, 1.0); + + assert_eq!(grips.len(), 2, "base grip plus only rendered attributes"); + assert_eq!(grips[1].id, ATTRIBUTE_GRIP_BASE + 3); + assert_eq!(grips[1].world, glam::DVec3::new(4.0, 4.0, 0.0)); + + document.header.attribute_visibility = 0; + assert_eq!( + visible_attribute_grips(&document, &ins, 1.0).len(), + 1, + "ATTMODE=0 should hide every attribute grip", + ); + } + + #[test] + fn visible_attribute_grips_scale_annotative_attributes() { + let document = CadDocument::new(); + let mut insert = block_with(vec![attribute("TAG", 11.0, 10.0)]); + insert.insert_point = Vector3::new(10.0, 10.0, 0.0); + insert + .common + .extended_data + .add_record(ExtendedDataRecord::new("AcAnnotativeData")); + + let grips = visible_attribute_grips(&document, &insert, 2.0); + + assert_eq!(grips.len(), 2); + assert_eq!(grips[1].world, glam::DVec3::new(12.0, 10.0, 0.0)); + } +} diff --git a/src/io/mod.rs b/src/io/mod.rs index 8d0ac888e..646c0d536 100644 --- a/src/io/mod.rs +++ b/src/io/mod.rs @@ -2368,16 +2368,13 @@ fn fix_viewport_status_flags(doc: &mut CadDocument) { } } -/// The acadrust DXF reader still stores Shape rotation directly from group code -/// 50 in degrees, while DWG and our own creation code store radians. Attribute -/// entities and definitions are converted by the reader itself and must not be -/// converted a second time here. +/// The acadrust DXF reader still stores Shape rotation directly from group +/// code 50 in degrees, while DWG and our own creation code store radians. +/// Dimension angles and ATTRIB/ATTDEF rotation are converted inside the +/// reader, so arms for them here would convert twice. fn fix_dxf_dimension_rotations(doc: &mut CadDocument) { for entity in doc.entities_mut() { match entity { - // Dimension angles (rotation / text / oblique) are converted - // degrees->radians inside the acadrust DXF reader now, so a - // dimension arm here would double-convert. EntityType::Shape(s) => { s.rotation = s.rotation.to_radians(); } diff --git a/src/modules/draw/modify/trim.rs b/src/modules/draw/modify/trim.rs index b18573986..f6347ca6e 100644 --- a/src/modules/draw/modify/trim.rs +++ b/src/modules/draw/modify/trim.rs @@ -3810,6 +3810,72 @@ mod tests { other => panic!("expected LwPolyline, got {other:?}"), } } + + /// #1318 repro: a 3D polyline whose first segment is not horizontal has no + /// plan-view shape, so it samples to nothing. The seam test then compared + /// two empty ends as equal and sliced the empty sample from index 1. + #[test] + fn sampling_skips_segments_with_no_plan_shape() { + use acadrust::entities::{Polyline3D, Vertex3DPolyline}; + + let mut pl = Polyline3D::new(); + pl.vertices = vec![ + // Vertical: an upright curve plane, dropped by `entity_curve_xy`. + Vertex3DPolyline::from_xyz(0.0, 0.0, 0.0), + Vertex3DPolyline::from_xyz(0.0, 0.0, 10.0), + // Horizontal, so this one does sample. + Vertex3DPolyline::from_xyz(10.0, 0.0, 10.0), + ]; + + let pts = sample_entity_xy(&EntityType::Polyline3D(pl)); + assert!( + !pts.is_empty(), + "the horizontal segment still has to be sampled" + ); + assert!( + pts.iter().all(|p| p.iter().all(|c| c.is_finite())), + "sampled points stay finite: {pts:?}" + ); + } + + /// The same skip must not swallow a leading segment that does sample, and + /// must keep dropping the duplicated seam vertex between two of them. + #[test] + fn sampling_still_joins_segments_at_their_seam() { + use acadrust::entities::{Polyline3D, Vertex3DPolyline}; + + let mut pl = Polyline3D::new(); + pl.vertices = vec![ + Vertex3DPolyline::from_xyz(0.0, 0.0, 0.0), + Vertex3DPolyline::from_xyz(10.0, 0.0, 0.0), + Vertex3DPolyline::from_xyz(10.0, 10.0, 0.0), + ]; + + let pts = sample_entity_xy(&EntityType::Polyline3D(pl)); + let seam = [10.0, 0.0]; + assert_eq!( + pts.iter().filter(|p| **p == seam).count(), + 1, + "the shared vertex appears once, not twice: {pts:?}" + ); + } + + #[test] + fn preview_sampling_skips_edge_on_polyline2d_arc_segments() { + use acadrust::entities::{Polyline2D, Vertex2D}; + + let mut pl = Polyline2D::new(); + let mut start = Vertex2D::new(Vector3::new(0.0, 0.0, 0.0)); + start.bulge = 1.0; + pl.vertices = vec![start, Vertex2D::new(Vector3::new(2.0, 0.0, 0.0))]; + pl.normal = Vector3::new(1.0, 0.0, 0.0); + + let pts = preview_sample_xy(&EntityType::Polyline2D(pl)); + assert!( + pts.is_empty(), + "edge-on arc segments are skipped instead of panicking: {pts:?}" + ); + } } // ══════════════════════════════════════════════════════════════════════════ @@ -3978,6 +4044,13 @@ fn sample_entity_xy(e: &EntityType) -> Vec<[f64; 2]> { let mut pts: Vec<[f64; 2]> = Vec::new(); for seg in crate::modules::draw::modify::explode::explode_polyline_segments(e) { let sp = sample_entity_xy(&seg); + // A segment with no plan-view shape samples to nothing — an + // edge-on plane has no XY curve, so `entity_curve_xy` declines it. + // Skipping it here also keeps the seam test below from comparing + // two `None`s and then slicing an empty sample (#1318). + if sp.is_empty() { + continue; + } if pts.last() == sp.first() { pts.extend_from_slice(&sp[1..]); } else { @@ -4030,6 +4103,9 @@ fn preview_sample_xy(e: &EntityType) -> Vec<[f64; 2]> { } _ => sample_entity_xy(&seg), }; + if sp.is_empty() { + continue; + } if pts.last() == sp.first() { pts.extend_from_slice(&sp[1..]); } else { diff --git a/src/scene/modify.rs b/src/scene/modify.rs index e4ae6e69c..0cecabf48 100644 --- a/src/scene/modify.rs +++ b/src/scene/modify.rs @@ -1443,6 +1443,17 @@ impl Scene { if self.is_layer_locked(handle) { return; } + // An attribute grip sits on the displayed (annotation-scaled) + // attribute; the stored one is unscaled. + let apply = match self.document.get_entity(handle) { + Some(EntityType::Insert(insert)) => crate::entities::insert::unscale_attribute_grip( + insert, + self.annotation_scale, + grip_id, + apply, + ), + _ => apply, + }; if self.apply_solid_history_grip(handle, grip_id, apply.clone()) { return; } diff --git a/src/scene/page_setup.rs b/src/scene/page_setup.rs index af3926e75..193264d13 100644 --- a/src/scene/page_setup.rs +++ b/src/scene/page_setup.rs @@ -175,7 +175,23 @@ impl Scene { let ObjectType::Dictionary(d) = self.document.objects.get(&dh)? else { return None; }; - d.entries.iter().find(|(k, _)| k == name).map(|(_, h)| *h) + d.entries + .iter() + .find(|(k, _)| k.eq_ignore_ascii_case(name)) + .map(|(_, h)| *h) + } + + /// The dictionary's own spelling of `name` (page-setup names compare + /// case-insensitively, like every named-object dictionary key). + fn page_setup_key(&self, name: &str) -> Option { + let dh = self.plotsettings_dict_handle()?; + let ObjectType::Dictionary(d) = self.document.objects.get(&dh)? else { + return None; + }; + d.entries + .iter() + .find(|(k, _)| k.eq_ignore_ascii_case(name)) + .map(|(k, _)| k.clone()) } /// Create or update the named page setup from `ps` (its `page_name` and @@ -183,7 +199,9 @@ impl Scene { /// are inserted and registered in the dictionary. pub fn page_setup_save(&mut self, name: &str, mut ps: PlotSettings) { let dict_handle = self.ensure_plotsettings_dict(); - ps.page_name = name.to_string(); + // Replacing keeps the entry's stored spelling so the dictionary key + // and the object's page_name stay one name. + ps.page_name = self.page_setup_key(name).unwrap_or_else(|| name.to_string()); ps.owner = dict_handle; if let Some(h) = self.page_setup_handle(name) { ps.handle = h; @@ -206,8 +224,12 @@ impl Scene { let handle = if let Some(ObjectType::Dictionary(d)) = self.document.objects.get_mut(&dict_handle) { - let h = d.entries.iter().find(|(k, _)| k == name).map(|(_, h)| *h); - d.entries.retain(|(k, _)| k != name); + let h = d + .entries + .iter() + .find(|(k, _)| k.eq_ignore_ascii_case(name)) + .map(|(_, h)| *h); + d.entries.retain(|(k, _)| !k.eq_ignore_ascii_case(name)); h } else { None @@ -223,7 +245,8 @@ impl Scene { if old == new || new.trim().is_empty() { return; } - if self.page_setup_handle(new).is_some() { + // A different spelling of the same name is a rename, not a collision. + if !old.eq_ignore_ascii_case(new) && self.page_setup_handle(new).is_some() { return; // name collision } let Some(dict_handle) = self.plotsettings_dict_handle() else { @@ -231,7 +254,7 @@ impl Scene { }; let mut renamed = None; if let Some(ObjectType::Dictionary(d)) = self.document.objects.get_mut(&dict_handle) { - if let Some(e) = d.entries.iter_mut().find(|(k, _)| k == old) { + if let Some(e) = d.entries.iter_mut().find(|(k, _)| k.eq_ignore_ascii_case(old)) { e.0 = new.to_string(); renamed = Some(e.1); } diff --git a/src/scene/parametric_constraints.rs b/src/scene/parametric_constraints.rs index 0dc086c20..0ab068550 100644 --- a/src/scene/parametric_constraints.rs +++ b/src/scene/parametric_constraints.rs @@ -244,6 +244,9 @@ pub(crate) fn set_polyline_vertex( x: f64, y: f64, ) -> bool { + let Some(index) = polyline_vertex_index(entity, index) else { + return false; + }; match entity { acadrust::EntityType::LwPolyline(polyline) => polyline .vertices @@ -800,9 +803,27 @@ pub(crate) fn resolve_point(entity: &acadrust::EntityType, marker: i32) -> Optio if marker < 0 { return None; } - super::dimension_assoc::source_points(entity) - .get(marker as usize) - .copied() + let points = super::dimension_assoc::source_points(entity); + // The closing segment of a closed polyline runs back to vertex 0, so its + // end marker (one past the last vertex) names that vertex — the same wrap + // the solver's `line_segment` applies. + let index = match polyline_vertex_index(entity, marker as usize) { + Some(index) => index, + None => marker as usize, + }; + points.get(index).copied() +} + +/// `marker` as a vertex index of a polyline, wrapping the closing segment's +/// end (one past the last vertex) onto vertex 0 when the polyline is closed. +/// `None` for anything that is not a 2D polyline. +pub(crate) fn polyline_vertex_index(entity: &acadrust::EntityType, marker: usize) -> Option { + let (count, closed) = match entity { + acadrust::EntityType::LwPolyline(polyline) => (polyline.vertices.len(), polyline.is_closed), + acadrust::EntityType::Polyline2D(polyline) => (polyline.vertices.len(), polyline.is_closed()), + _ => return None, + }; + Some(if closed && marker == count && count > 0 { 0 } else { marker }) } /// Below this squared distance (1e-6 world units), two points count as diff --git a/src/scene/parametric_solve.rs b/src/scene/parametric_solve.rs index a0c7a1792..59a5d30ca 100644 --- a/src/scene/parametric_solve.rs +++ b/src/scene/parametric_solve.rs @@ -87,8 +87,16 @@ impl EntityGeom { (EntityGeom::Line(l), 0) => Some(l.p1), (EntityGeom::Line(l), 1) => Some(l.p2), (EntityGeom::Ray(l), 0) => Some(l.p1), - (EntityGeom::Polyline { points, .. }, marker) if marker >= 0 => { - points.get(marker as usize).copied() + (EntityGeom::Polyline { points, closed, .. }, marker) if marker >= 0 => { + // The closing segment's end marker (one past the last vertex) + // is vertex 0 on a closed polyline, as in `line_segment`. + let index = marker as usize; + let index = if *closed && index == points.len() && !points.is_empty() { + 0 + } else { + index + }; + points.get(index).copied() } (EntityGeom::Circle(c), -3) => Some(c.center), (EntityGeom::Arc(a), 0) => Some(a.start), diff --git a/src/snap.rs b/src/snap.rs index 05e9055fb..7b13f8ab1 100644 --- a/src/snap.rs +++ b/src/snap.rs @@ -170,6 +170,7 @@ pub(crate) fn wire_source(wire: &WireModel) -> Option, pub kind: TrackingKind, } @@ -917,19 +924,35 @@ impl Snapper { if sd < r && best_x.as_ref().map_or(true, |(bd, _)| sd < *bd) { // Report an acquired tracking ray (not an auxiliary // last_point ray) as base/dir for typed-distance entry. - let ot = if rays[i].group != POLAR_GROUP && rays[i].group != ORTHO_GROUP { - &rays[i] + let (ot, other) = if rays[i].group != POLAR_GROUP + && rays[i].group != ORTHO_GROUP + { + (&rays[i], &rays[j]) } else { - &rays[j] + (&rays[j], &rays[i]) }; - let t = (x.x - ot.origin.x) * ot.dir.x + (x.y - ot.origin.y) * ot.dir.y; - let dir_out = if t >= 0.0 { ot.dir } else { -ot.dir }; + // Point each ray the way the crossing lies from its own + // origin, so the guide drawn for it runs through the lock + // rather than away from it. + let outward = |ray: &Ray| { + let t = (x.x - ray.origin.x) * ray.dir.x + + (x.y - ray.origin.y) * ray.dir.y; + if t >= 0.0 { + ray.dir + } else { + -ray.dir + } + }; + let dir_out = outward(ot); best_x = Some(( sd, OtrackHit { aligned: x, dir: dir_out, base: ot.origin, + // The vector the reported one crosses. Both are + // drawn, so the intersection reads as one. (#1313) + cross: Some((other.origin, outward(other))), kind: ot.kind, } )); @@ -970,6 +993,8 @@ impl Snapper { aligned, dir: dir_out, base: ray.origin, + // A single-ray alignment has no second vector. + cross: None, kind: ray.kind, }, )); @@ -3660,6 +3685,121 @@ mod ext_tests { assert!(none.is_none(), "no base point → no base→corner alignment"); } + /// #1313: an intersection lock must report both of the vectors it is the + /// crossing of, so the overlay can draw both. Reporting only the one a + /// typed distance runs along leaves the user with a single guide and no + /// sign of what the point actually is. + #[test] + fn intersection_lock_reports_both_crossing_vectors() { + let mut s = Snapper::default(); + s.otrack_enabled = true; + s.osnap_radius_px = 10.0; + // Two acquired corners. With no polar step each offers a horizontal and + // a vertical ray, so their rays cross at (10, 0) and at (0, 5). + let first = DVec3::new(0.0, 0.0, 0.0); + let second = DVec3::new(10.0, 5.0, 0.0); + for corner in [first, second] { + s.tracking_points.push(corner); + s.tracking_dirs.push(Vec::new()); + } + + let view_rot = Mat4::from_scale(Vec3::splat(0.0001)); + let eye = glam::DVec3::ZERO; + let bounds = Rectangle { + x: 0.0, + y: 0.0, + width: 1000.0, + height: 1000.0, + }; + + // Cursor a hair off the crossing of the first corner's horizontal ray + // and the second corner's vertical one. + let crossing = DVec3::new(10.0, 0.0, 0.0); + let hit = s + .otrack_snap( + crossing + DVec3::new(0.02, 0.02, 0.0), + view_rot, + eye, + bounds, + None, + None, + None, + false, + DVec3::X, + DVec3::Y, + ) + .expect("the two rays cross inside the aperture"); + assert!( + (hit.aligned - crossing).length() < 1e-9, + "locked off the crossing: {:?}", + hit.aligned + ); + + let (cross_base, cross_dir) = hit.cross.expect("a crossing reports its second vector"); + let bases = [hit.base, cross_base]; + for corner in [first, second] { + assert!( + bases.iter().any(|b| (*b - corner).length() < 1e-9), + "{corner:?} is not one of the two reported vectors: {bases:?}" + ); + } + + // Each vector runs from its own corner through the crossing, pointing + // at it — the guides are drawn along these. + for (base, dir) in [(hit.base, hit.dir), (cross_base, cross_dir)] { + let off = crossing - base; + assert!( + (off.x * dir.y - off.y * dir.x).abs() < 1e-9, + "the crossing is off the vector from {base:?} along {dir:?}" + ); + assert!( + off.dot(dir) > 0.0, + "vector from {base:?} points away from the crossing" + ); + } + } + + /// The second vector belongs to a crossing alone: a plain single-ray + /// alignment has nothing to cross, and must not draw a second guide. + #[test] + fn single_ray_alignment_reports_no_crossing_vector() { + let mut s = Snapper::default(); + s.otrack_enabled = true; + s.osnap_radius_px = 10.0; + let corner = DVec3::new(10.0, 5.0, 0.0); + s.tracking_points.push(corner); + s.tracking_dirs.push(Vec::new()); + + let view_rot = Mat4::from_scale(Vec3::splat(0.0001)); + let eye = glam::DVec3::ZERO; + let bounds = Rectangle { + x: 0.0, + y: 0.0, + width: 1000.0, + height: 1000.0, + }; + + let hit = s + .otrack_snap( + DVec3::new(60.0, 5.02, 0.0), + view_rot, + eye, + bounds, + None, + None, + None, + false, + DVec3::X, + DVec3::Y, + ) + .expect("the corner's horizontal ray catches the cursor"); + assert!( + hit.cross.is_none(), + "a single-ray alignment reported a crossing vector: {:?}", + hit.cross + ); + } + #[test] fn tangent_points_are_perpendicular_to_the_radius() { let c = Vec3::new(0.0, 0.0, 0.0); diff --git a/src/ui/command_line.rs b/src/ui/command_line.rs index a703c3800..d4259f78c 100644 --- a/src/ui/command_line.rs +++ b/src/ui/command_line.rs @@ -129,6 +129,13 @@ pub struct CommandLine { pub history: Vec, pub error_revision: u64, pub last_error: Option, + /// Tokens from the last multi-token command line that no prompt ever + /// consumed — e.g. a trailing word after an in-place text step, or a typo'd + /// extra argument (`CIRCLE 0,0 5 9`). The headless automation feeder reports + /// them so a caller can tell "part of my line was silently dropped" from + /// "the command ran exactly as typed". Cleared at the start of every + /// `run_command_line`, not persisted. + pub unconsumed: Vec, /// Successfully dispatched commands used for ↑/↓ recall, newest last. /// Stored separately because recall also maintains its own cursor and draft. pub cmd_recall: Vec, @@ -186,6 +193,7 @@ impl Default for CommandLine { history: Vec::new(), error_revision: 0, last_error: None, + unconsumed: Vec::new(), cmd_recall: Vec::new(), recent_commands: Vec::new(), recent_inputs: Vec::new(), diff --git a/src/ui/overlay.rs b/src/ui/overlay.rs index a1c7f2e02..74a828a09 100644 --- a/src/ui/overlay.rs +++ b/src/ui/overlay.rs @@ -962,7 +962,7 @@ pub fn selection_overlay<'a>( grip_clip: Option, ucs_icons: Vec, ost_points: Vec, - otrack_line: Option<(Point, Point)>, + otrack_lines: Vec<(Point, Point)>, parallel_ref_marker: Option, show_viewcube: bool, dividers: Vec, @@ -988,7 +988,7 @@ pub fn selection_overlay<'a>( grip_clip, ucs_icons, ost_points, - otrack_line, + otrack_lines, parallel_ref_marker, show_viewcube, dividers, @@ -1029,11 +1029,12 @@ struct SelectionCanvas { /// entry carries hover/selected (grips). ucs_icons: Vec, ost_points: Vec, - /// Active OTRACK alignment: (acquired tracking point, locked cursor), both - /// in screen space. Drawn as a dashed guide extended a little past the - /// cursor so the extension / tracking line the user snapped to is visible. - /// (#219) - otrack_line: Option<(Point, Point)>, + /// Active OTRACK alignments, each (acquired tracking point, locked cursor) + /// in screen space. Drawn as dashed guides extended a little past the + /// cursor so the extension / tracking line the user snapped to is visible + /// (#219). An intersection lock contributes both of its crossing vectors, + /// so the point reads as their meeting rather than a lone guide (#1313). + otrack_lines: Vec<(Point, Point)>, /// The acquired Parallel-snap reference point (screen), marked with a small /// ∥ glyph so the user sees which line is the parallel reference. (#277) parallel_ref_marker: Option, @@ -2013,7 +2014,7 @@ impl canvas::Program for SelectionCanvas { // real angle from the acquired point through the lock and a little // beyond, dashed so it reads as a construction guide. This covers the // ortho (0°/90°), polar, and edge-extension cases uniformly (#219). - if let Some((base, tip)) = self.otrack_line { + for &(base, tip) in &self.otrack_lines { let dx = tip.x - base.x; let dy = tip.y - base.y; let len = (dx * dx + dy * dy).sqrt();