From 50db12b169ba9067a5845415fcf29fa038badd8c Mon Sep 17 00:00:00 2001 From: Sebastian <106036+schoeller@users.noreply.github.com> Date: Wed, 14 Oct 2026 20:57:13 +0200 Subject: [PATCH] refactor(scene): reference-only designation becomes a scene-scoped set (API v9) Per the upstream review of the XDATA-based storage: the host no longer writes its own state into the saved file. The marker record under the host-owned OCS APPID, the on-first-use APPID registration and the verbatim-EED rewrite handling are removed; the single source of truth is a scene-scoped in-memory set of entity handles (Scene::reference_only) that dies with the drawing tab, so a reopened file starts undesignated and the owning plugin re-marks on the document-opened notification. All behaviour sites are unchanged, driven by the set: the wire stamp moves from an XDATA read to a set lookup threaded through the tessellation pipelines (tessellate_entity and the dimension text path take the handle set as a plain parameter), and the pick filters, selection retains and update/erase refusals read Scene::is_reference_only. Undo keeps the entity-write contract: set mutations are recorded as explicit first-touch before/after values on each history delta (the parametric- constraints channel pattern). A designation inside a plugin push_undo scope joins it, a scope-less change is one step of its own, an idempotent re-mark records nothing, erasing a designated entity drops the set entry with the entity, and undo/redo restores entity and designation together. The round-trip test is inverted: reference_only_designation_is_not_written_ to_dwg_or_dxf now asserts the marker appears in neither format's bytes, that no OCS APPID or marker record survives a reload, and that the designation stays live in memory. --- crates/ocs_plugin_api/src/host.rs | 15 +- docs/plugin-host-model-completion-plan.md | 30 +++ docs/plugin-host-model.md | 27 ++- src/app/document.rs | 24 ++ src/app/history.rs | 89 ++++++- src/app/plugin_host.rs | 233 +++++++++++++------ src/app/record_api.rs | 40 ++-- src/entities/dimension.rs | 14 ++ src/scene/cache/block_cache.rs | 4 + src/scene/convert/tess.rs | 18 +- src/scene/dimension_assoc.rs | 2 +- src/scene/mod.rs | 75 +++++- src/scene/parametric_solve.rs | 4 +- src/scene/pipeline/circle_gpu.rs | 2 + src/scene/reference_only.rs | 268 ++++++++++++---------- src/scene/selection.rs | 9 + src/scene/view/render.rs | 1 + 17 files changed, 623 insertions(+), 232 deletions(-) diff --git a/crates/ocs_plugin_api/src/host.rs b/crates/ocs_plugin_api/src/host.rs index 65e9dd4e7..55aded21e 100644 --- a/crates/ocs_plugin_api/src/host.rs +++ b/crates/ocs_plugin_api/src/host.rs @@ -1189,12 +1189,21 @@ pub trait HostApi { /// plugin keeps full write access through this API: `update_entity`, /// [`update_entities_transaction`](HostApi::update_entities_transaction), /// `remove_entity` and the XDATA record methods keep working, so derived - /// display geometry can be refreshed in place. The designation survives - /// DWG/DXF save and reload. + /// display geometry can be refreshed in place. + /// + /// The designation is a scene-scoped, in-memory set keyed by entity + /// handle: it is not written to the saved DWG/DXF (the host stores no + /// state of its own in the file), it lives and dies with the drawing tab, + /// and a reloaded drawing starts undesignated — the owning plugin + /// re-marks its entities when the document opens, which makes the + /// in-memory designation fully equivalent to a persisted one for its use + /// case. /// /// Fails when the entity does not exist or is on a locked layer (the same /// policy as the XDATA record methods). A call that would not change the - /// current designation succeeds without recording an undo step. + /// current designation succeeds without recording an undo step — and + /// without publishing a document change, so idempotent re-marks cannot + /// loop with the notification dispatch. /// /// Undo composition — the entity-write contract: a change performed /// inside a plugin-opened [`HostApi::push_undo`] scope joins that scope, diff --git a/docs/plugin-host-model-completion-plan.md b/docs/plugin-host-model-completion-plan.md index 7ded4cf43..e190c2a73 100644 --- a/docs/plugin-host-model-completion-plan.md +++ b/docs/plugin-host-model-completion-plan.md @@ -400,3 +400,33 @@ and `box_selections_skip_reference_only_wires`; the record surface test `entities_records_expose_the_reference_only_designation`; and the IPC round-trip `reference_only_request_and_response_roundtrip`. Like the XRecord API, a Python mapping is future work. + +**Reference-only rework: scene-scoped storage (2026-10-14, API v9).** +Supersedes the storage decision above after the upstream review: the designation is no longer +written to the file at all. The XDATA marker under the host-owned `OCS` APPID (and the APPID +registration, the verbatim-EED rewrite handling and the storage unit tests built on it) is removed; +the single source of truth is now a scene-scoped in-memory set of entity handles +(`Scene::reference_only`, `scene::reference_only`), deliberately not serialized — the host writes +nothing of its own into the saved DWG/DXF, the set dies with the drawing tab, and a reopened file +starts undesignated. Every behaviour site is unchanged and driven by the set: the wire +`tessellate_entity` stamp (threaded through the tessellation pipelines as a plain handle-set +parameter), the pick-side wire filters, the selection-gather retains, the update/erase refusals +and the MCP `entities` boolean. Undo keeps the entity-write contract through an explicit +designation channel on each history delta (the `parametric_constraints` pattern): a marking inside +a plugin-opened `push_undo` scope joins it, a scope-less change is one step of its own, an +idempotent re-mark records nothing, erasing a designated entity drops the set entry in the same +step, and undo/redo restores entity and designation together. The round-trip test is inverted: +`reference_only_designation_is_not_written_to_dwg_or_dxf` now asserts the marker appears in +neither format's bytes, that no `OCS` APPID or marker record survives a reload, and that the +designation stays live in memory. Evidence: the set-storage tests +`marking_and_unmarking_report_and_track_membership`, `removing_a_handle_forgets_it`, +`retain_present_drops_entries_of_absent_entities`, `scene_membership_is_guarded_by_entity_presence` +and `tessellated_wires_stamp_from_the_designation_set`; the host tests +`reference_only_refusals_record_no_undo_step`, +`reference_only_designation_composes_undo_like_entity_writes`, +`reference_only_designation_is_not_written_to_dwg_or_dxf`, +`erasing_a_designated_entity_undoes_designation_with_it`, +`user_paths_refuse_and_plugin_paths_write_reference_only_entities`, +`select_all_visible_skips_reference_only_entities`, +`entities_records_expose_the_reference_only_designation`; the unchanged pick tests; and the IPC +round-trip `reference_only_request_and_response_roundtrip` (unchanged). diff --git a/docs/plugin-host-model.md b/docs/plugin-host-model.md index 2c1cd2bec..c33159a56 100644 --- a/docs/plugin-host-model.md +++ b/docs/plugin-host-model.md @@ -123,13 +123,23 @@ re-designation that would not change the state succeeds without recording an undo step. Undo composition is the entity-write contract: a change inside a plugin-opened `push_undo` scope joins it, so an operation composed of several writes is a single undo step under the plugin's label; with no scope open a -change is one undo step of its own. The designation survives DWG and DXF -save/reload, stored as a marker value in an XDATA record under the host-owned -`OCS` application namespace — plugins register their own APPIDs through their -manifest, so the two cannot collide. Out of scope today: entities nested -inside block instances (the block cache rebuilds instance wires without the -flag) and a Python mapping. The MCP `entities` read exposes the flag as a -`reference_only` boolean on every entity record. +change is one undo step of its own. + +**Storage is a scene-scoped in-memory set, not a file format.** The host +writes nothing of its own into the saved DWG/DXF — no APPID registration, no +XDATA marker on the entity — so a saved drawing never depends on host state +to load correctly. The set lives and dies with the drawing tab: closing a +tab forgets its designations, and a freshly opened drawing starts +undesignated. Because the designation is per handle, an in-place refresh +(clone → edit → replace) keeps it by construction. Undo keeps the contract +through an explicit designation channel on each history step (the same +pattern the parametric-constraint directory uses): undo/redo restores the +designation together with the entity it belongs to — erasing a designated +entity inside a scope, then undoing, brings back the entity *and* its +designation. Out of scope today: entities nested inside block instances +(the block cache rebuilds instance wires without the flag) and a Python +mapping. The MCP `entities` read exposes the flag as a `reference_only` +boolean on every entity record. ### Usage from a plugin @@ -169,6 +179,9 @@ Notes for plugin authors: no undo step. - The designation travels with the entity: a clone → edit → replace refresh keeps it, so a plugin marks its display data once and refreshes freely. +- The designation is scene state, not file state: it does not survive + DWG/DXF save and reload — a plugin re-marks its display data when the + document opens, and idempotent re-marks keep that loop finite. - Rendering and object snap are unaffected; grips, picking and the built-in write paths are the only observers. - Plugins that read the document through the IPC client (the diff --git a/src/app/document.rs b/src/app/document.rs index bf0b130b7..ced420528 100644 --- a/src/app/document.rs +++ b/src/app/document.rs @@ -766,6 +766,7 @@ impl HistorySnapshot { .saturating_add(d.named_parameters.as_ref().map_or(0, |(before, after)| { before.len().saturating_add(after.len()).saturating_mul(128) })) + .saturating_add(d.reference_designations.len().saturating_mul(16)) .saturating_add(d.label.len()), HistorySnapshot::ObjectVisibility(v) => v .before @@ -818,6 +819,12 @@ pub(super) struct DeltaSnapshot { /// `document.objects`, so it needs its own channel here rather than /// riding along with `entities`/`structure`. Almost always empty. pub(super) parametric_constraints: Vec, + /// Reference-only designations (API v9) this same command changed — set + /// state on `Scene` (see `scene::reference_only`), recorded as explicit + /// first-touch before/afters so undo/redo restores the designation with + /// the entity it belongs to (e.g. a plugin scope that both erases and + /// re-designates). Almost always empty. + pub(super) reference_designations: Vec, /// Drawing-wide parameter table changed by this transaction. pub(super) named_parameters: Option<( crate::scene::named_parameters::ParameterTable, @@ -892,6 +899,17 @@ pub(super) struct ParametricConstraintsEntryDelta { pub(super) after: crate::scene::parametric_constraints::ParametricConstraintSet, } +/// One reference-only designation's before/after value within an entity delta +/// (API v9). The designation set lives on `Scene`, not in the document, so +/// the value rides its own channel — symmetric like every other delta field, +/// undo applies the before value, redo the after value. +#[derive(Clone)] +pub(super) struct ReferenceDesignationDelta { + pub(super) handle: Handle, + pub(super) before: bool, + pub(super) after: bool, +} + #[derive(Default)] pub(super) struct HistoryState { pub(super) undo_stack: Vec, @@ -907,6 +925,12 @@ pub(super) struct PendingHistorySnapshot { pub(super) dirty_before: bool, pub(super) structure_before: CadDocument, pub(super) recorder: Arc, + /// First-touch before-values of reference-only designations (API v9) + /// mutated inside this plugin scope (see `scene::reference_only`). Set + /// mutations are not document writes, so the scope closes by reading the + /// set's current state as the after-image and keeping entries whose value + /// actually flipped. + pub(super) reference_designations_before: Vec<(Handle, bool)>, /// Kept as an undo step even when nothing changed. pub(super) keep_empty: bool, } diff --git a/src/app/history.rs b/src/app/history.rs index 01b919f14..d40509e7e 100644 --- a/src/app/history.rs +++ b/src/app/history.rs @@ -1,8 +1,8 @@ use super::{ document::{ DeltaSnapshot, HistorySnapshot, ObjectEntryDelta, ObjectVisibilitySnapshot, - ParametricConstraintsEntryDelta, PendingHistorySnapshot, StructureSnapshot, - TableEntryDelta, + ParametricConstraintsEntryDelta, PendingHistorySnapshot, ReferenceDesignationDelta, + StructureSnapshot, TableEntryDelta, }, OpenCADStudio, }; @@ -226,6 +226,7 @@ impl OpenCADStudio { active_layer: None, structure: (!objects.is_empty()).then_some(StructureSnapshot::Objects(objects)), parametric_constraints: Vec::new(), + reference_designations: Vec::new(), named_parameters: None, label: label.into(), }; @@ -313,8 +314,26 @@ impl OpenCADStudio { let selected_after: Vec = self.tabs[i].scene.selected.iter().copied().collect(); let dirty_after = self.tabs[i].dirty; let active_layer_after = self.tabs[i].active_layer.clone(); + // The designation set is scene state mutated through the plugin API: + // a scope that (un)marked entities carries the first-touch before + // values here; the after value is whatever the set holds now, and + // only untouched-value entries drop out (an idempotent re-mark never + // got recorded in the first place). + let reference_designations: Vec<_> = pending + .reference_designations_before + .into_iter() + .filter_map(|(handle, before)| { + let after = self.tabs[i].scene.reference_only.is_marked(handle); + (before != after).then_some(ReferenceDesignationDelta { + handle, + before, + after, + }) + }) + .collect(); if !pending.keep_empty && entities.is_empty() + && reference_designations.is_empty() && !structure_changed && pending.selected_before == selected_after && pending.dirty_before == dirty_after @@ -335,6 +354,7 @@ impl OpenCADStudio { structure: structure_changed .then_some(StructureSnapshot::Full(pending.structure_before)), parametric_constraints: Vec::new(), + reference_designations, named_parameters: None, label: pending.label, }; @@ -375,10 +395,33 @@ impl OpenCADStudio { dirty_before, structure_before, recorder, + reference_designations_before: Vec::new(), keep_empty: false, }); } + /// Record the pre-mutation value of a reference-only designation (API v9) + /// into the open plugin pending scope — first touch wins. No-op when no + /// scope is open: like the XRecord methods, a designation change must open + /// its own scope before mutating, so by the time this runs a scope either + /// covers it or the caller owns one. + pub(super) fn record_reference_designation_before_pending( + &mut self, + i: usize, + handle: Handle, + before: bool, + ) { + if let Some(pending) = self.tabs[i].history.pending.as_mut() { + if !pending + .reference_designations_before + .iter() + .any(|(recorded, _)| *recorded == handle) + { + pending.reference_designations_before.push((handle, before)); + } + } + } + /// [`Self::push_undo_snapshot`] for a command that is an undo step even /// when it changes nothing (UNDO and U then name it and undo nothing). pub(super) fn push_undo_mark(&mut self, i: usize, label: impl Into) { @@ -466,6 +509,7 @@ impl OpenCADStudio { active_layer: None, structure: Some(StructureSnapshot::Layers(entries)), parametric_constraints: Vec::new(), + reference_designations: Vec::new(), named_parameters: None, label: pending.label, }; @@ -523,6 +567,7 @@ impl OpenCADStudio { active_layer: None, structure: Some(StructureSnapshot::TextStyles(entries)), parametric_constraints: Vec::new(), + reference_designations: Vec::new(), named_parameters: None, label: pending.label, }; @@ -580,6 +625,7 @@ impl OpenCADStudio { active_layer: None, structure: Some(StructureSnapshot::DimStyles(entries)), parametric_constraints: Vec::new(), + reference_designations: Vec::new(), named_parameters: None, label: pending.label, }; @@ -645,6 +691,7 @@ impl OpenCADStudio { active_layer: None, structure: Some(StructureSnapshot::Objects(entries)), parametric_constraints: Vec::new(), + reference_designations: Vec::new(), named_parameters: None, label: pending.label, }; @@ -680,6 +727,7 @@ impl OpenCADStudio { object_handles, }), parametric_constraints: Vec::new(), + reference_designations: Vec::new(), named_parameters: None, label: "STYLE".to_string(), }; @@ -761,8 +809,13 @@ impl OpenCADStudio { pending.label ); } - let (entity_before, object_before, parametric_constraints_before, named_parameters_before) = - rec.into_recorded_images(); + let ( + entity_before, + object_before, + parametric_constraints_before, + named_parameters_before, + reference_designations_before, + ) = rec.into_recorded_images(); let entities: Vec<(Handle, Option>, Option>)> = entity_before .into_iter() @@ -808,6 +861,20 @@ impl OpenCADStudio { let after = self.tabs[i].scene.named_parameters().clone(); (before != after).then_some((before, after)) }); + // Designations recorded by `remove_entities`'s delta path (the + // first-touch before values) close symmetric with the set's current + // state as the after image. + let reference_designations: Vec = reference_designations_before + .into_iter() + .filter_map(|(handle, before)| { + let after = self.tabs[i].scene.reference_only.is_marked(handle); + (before != after).then_some(ReferenceDesignationDelta { + handle, + before, + after, + }) + }) + .collect(); let selected_after = self.tabs[i].scene.selected.iter().copied().collect(); let dirty_after = self.tabs[i].dirty; let mut structure = pending.structure_before.map(StructureSnapshot::Full); @@ -844,6 +911,7 @@ impl OpenCADStudio { active_layer: None, structure, parametric_constraints, + reference_designations, named_parameters, label: pending.label, }; @@ -998,8 +1066,19 @@ impl OpenCADStudio { } } } - let changes = self.tabs[i].scene.apply_entity_delta(&d.entities, undo); + let mut changes = self.tabs[i].scene.apply_entity_delta(&d.entities, undo); let scene = &mut self.tabs[i].scene; + // Designations: restore the chosen side's value on the scene-owned set + // (the entity images above already applied, so a restored entity is + // present before its designation is). Each actual flip joins the + // returned per-handle change list so the deferred cache reseed and + // geometry bump re-stamp that entity's wires. + for entry in &d.reference_designations { + let value = if undo { entry.before } else { entry.after }; + if scene.set_reference_only_membership(entry.handle, value) { + changes.push((entry.handle, crate::scene::ChangeKind::Modified)); + } + } if !d.parametric_constraints.is_empty() { scene.bump_constraints_epoch(); } diff --git a/src/app/plugin_host.rs b/src/app/plugin_host.rs index 39718d7dd..a108a4f6f 100644 --- a/src/app/plugin_host.rs +++ b/src/app/plugin_host.rs @@ -519,6 +519,28 @@ impl<'a> HostSession<'a> { Ok(()) } + /// Erase through the plugin path while keeping reference-only designations + /// undoable with the erase (API v9). The designation set is scene state, + /// not a document write: erasing drops the set entry in + /// `Scene::remove_entities`, so its first-touch before-image must join the + /// open plugin scope here — otherwise undoing the erase would restore the + /// entity without its designation. No-op for undesignated handles and when + /// no scope is open (then the erase follows the same no-bookkeeping + /// contract as any other entity write). + fn erase_entities_recording_designations(&mut self, handles: &[Handle]) { + for &handle in handles { + if self.app.tabs[self.tab] + .scene + .reference_only + .is_marked(handle) + { + self.app + .record_reference_designation_before_pending(self.tab, handle, true); + } + } + self.app.tabs[self.tab].scene.erase_entities_plugin(handles); + } + /// Delete the entity with `handle`, keeping the scene's render caches in /// sync. Returns `false` when the entity is absent or on a locked layer /// (which `erase_entities` refuses to remove). @@ -548,9 +570,7 @@ impl<'a> HostSession<'a> { .document() .get_entity(handle) .map(|entity| entity.common().owner_handle); - self.app.tabs[self.tab] - .scene - .erase_entities_plugin(&[handle]); + self.erase_entities_recording_designations(&[handle]); let removed = self.document().get_entity(handle).is_none(); if removed { // The document keeps a block record's member list separately from the @@ -1140,9 +1160,7 @@ impl<'a> HostSession<'a> { return Err("the result could not be added".into()); } if delete_source { - self.app.tabs[self.tab] - .scene - .erase_entities_plugin(&[source]); + self.erase_entities_recording_designations(&[source]); self.publish_document_view(); } Ok(handle) @@ -1405,9 +1423,7 @@ impl<'a> HostSession<'a> { self.push_undo("Delete layer"); self.document_mut().layers.remove(&name); if !on_layer.is_empty() { - self.app.tabs[self.tab] - .scene - .erase_entities_plugin(&on_layer); + self.erase_entities_recording_designations(&on_layer); } self.finish_layer_change(&[name]); Ok(handle) @@ -2262,9 +2278,7 @@ impl<'a> HostSession<'a> { } } if erase_originals { - self.app.tabs[self.tab] - .scene - .erase_entities_plugin(&entities); + self.erase_entities_recording_designations(&entities); } self.finish_block_change(); Ok(handle) @@ -3134,62 +3148,64 @@ impl<'a> HostSession<'a> { /// Mark (or unmark) an entity as reference-only (API v9, additive): /// plugin-owned display geometry the built-in edit paths treat like a /// locked-layer entity, while the owning plugin keeps full write access - /// through this session. The designation survives DWG/DXF save and reload. + /// through this session. The designation is a scene-scoped in-memory set + /// keyed by handle: nothing is written to the saved DWG/DXF, the set dies + /// with the tab, and a reloaded drawing starts undesignated — the owning + /// plugin re-marks its entities when the document opens, which makes the + /// in-memory designation fully equivalent to a persisted one for its use + /// case. /// /// Every refusal — absent entity, locked layer — is decided before the /// undo scope is touched, so a failure changes nothing. A call that would /// not change the current designation succeeds without recording an undo - /// step. Undo composition: a change inside a plugin-opened `push_undo` - /// scope joins it (see [`HostSession::ensure_xrecord_undo_scope`]); a - /// scope-less call is one undo step of its own. + /// step, without bumping the geometry epoch and without publishing — the + /// idempotence that terminates the re-mark loop driven by the + /// document-changed dispatch. Undo composition: a change inside a + /// plugin-opened `push_undo` scope joins it (see + /// [`HostSession::ensure_xrecord_undo_scope`]); a scope-less call is one + /// undo step of its own. The set is scene state, not a document write, so + /// the undo delta restores it through the explicit designation channel + /// (`DeltaSnapshot::reference_designations`). pub fn set_reference_only( &mut self, handle: Handle, reference_only: bool, ) -> Result<(), String> { // Refusals precede any change (and any undo step). - let Some(entity) = self.document().get_entity(handle) else { + if self.document().get_entity(handle).is_none() { return Err(format!("entity {handle:?} does not exist")); - }; + } if self.app.tabs[self.tab].scene.is_layer_locked(handle) { return Err(format!("entity {handle:?} is on a locked layer")); } - if crate::scene::reference_only::entity_is_reference_only(entity) == reference_only { + if self.app.tabs[self.tab].scene.is_reference_only(handle) == reference_only { return Ok(()); } let own_scope = self.ensure_xrecord_undo_scope("Set Reference-Only"); - // Register the host APPID before the marker is written so the record's - // application is resolvable on save. - if reference_only { - self.ensure_app_id(crate::scene::reference_only::REFERENCE_ONLY_APP_ID); - } - let app_handle = self - .document() - .app_ids - .get(crate::scene::reference_only::REFERENCE_ONLY_APP_ID) - .map(|app| app.handle.value()); - let changed = self - .document_mut() - .get_entity_mut(handle) - .map(|entity| { - let changed = - crate::scene::reference_only::set_entity_reference_only(entity, reference_only); - // Drop stale verbatim EED for the host app so the fresh - // marker — not pre-edit bytes captured on a prior read — - // wins on the next save. - if changed { - if let Some(app_handle) = app_handle { - entity - .common_mut() - .extended_data - .raw_dwg_eed - .retain(|(a, _)| *a != app_handle); - } - } - changed - }) - .unwrap_or(false); - debug_assert!(changed, "designation state was checked before the mutation"); + // First-touch before-image for the pending scope's designation + // channel, recorded before the set changes (see + // `Scene::record_undo_reference_designation_before`). + let before = self.app.tabs[self.tab] + .scene + .reference_only + .is_marked(handle); + self.app + .record_reference_designation_before_pending(self.tab, handle, before); + self.app.tabs[self.tab] + .scene + .set_reference_only_membership(handle, reference_only); + debug_assert!( + self.app.tabs[self.tab] + .scene + .reference_only + .is_marked(handle) + == reference_only, + "designation state was checked before the mutation" + ); + // The wires carry the click-through stamp baked at tessellation, so a + // designation change must rebuild them: bump_geometry clears the + // tessellation memos and resident wire sets exactly like an entity + // edit would. self.bump_geometry(); self.set_dirty(); self.publish_document_view(); @@ -13491,9 +13507,7 @@ step('undo_move', lambda: M.move([u], (0, 0, 0), (7, 0, 0))) .get_mut("0") .map(|layer| layer.unlock()); assert!( - !crate::scene::reference_only::entity_is_reference_only( - host.document().get_entity(handle).unwrap() - ), + !host.app.tabs[0].scene.is_reference_only(handle), "refusals must not partially apply the designation" ); assert_eq!( @@ -13526,7 +13540,7 @@ step('undo_move', lambda: M.move([u], (0, 0, 0), (7, 0, 0))) // Undo reverts the unmark, redo re-applies it. drop(host); app.undo_steps(1); - let mut host = HostSession::new(&mut app, 0); + let host = HostSession::new(&mut app, 0); assert!(host.app.tabs[0].scene.is_reference_only(handle)); drop(host); app.redo_steps(1); @@ -13549,7 +13563,7 @@ step('undo_move', lambda: M.move([u], (0, 0, 0), (7, 0, 0))) // Undoing the plugin operation reverts it whole. app.undo_steps(1); - let mut host = HostSession::new(&mut app, 0); + let host = HostSession::new(&mut app, 0); assert!( host.app.tabs[0] .scene @@ -13565,32 +13579,101 @@ step('undo_move', lambda: M.move([u], (0, 0, 0), (7, 0, 0))) } #[test] - fn reference_only_survives_dxf_and_dwg_round_trips() { + fn reference_only_designation_is_not_written_to_dwg_or_dxf() { + let mut app = OpenCADStudio::new_for_test(); + app.tabs[0].is_start = false; + let mut host = HostSession::new(&mut app, 0); + let handle = host.add_entity(reference_test_line(10.0, 0.0, Handle::NULL)); + host.set_reference_only(handle, true).unwrap(); + // The designation lives in the scene's in-memory set only. + assert!(host.app.tabs[0].scene.is_reference_only(handle)); + + // The host writes nothing of its own into the document: no APPID + // registration, no marker XDATA on the designated entity. + assert!(host.document().app_ids.get("OCS").is_none()); + assert!(host + .document() + .get_entity(handle) + .expect("designated entity") + .common() + .extended_data + .get_record("OCS") + .is_none()); + + let contains_marker = + |bytes: &[u8]| bytes.windows(15).any(|window| window == b"REFERENCE_ONLY"); + + // DWG: the marker string appears nowhere in the saved bytes, and a + // reopened file carries neither the host APPID nor the marker record. + let dwg = codec::DwgWriter::write_to_vec(host.document()).unwrap(); + assert!( + !contains_marker(&dwg), + "DWG save must not contain the designation marker" + ); + let reloaded = crate::io::load_bytes("reference-only.dwg", dwg).unwrap(); + assert!(reloaded.app_ids.get("OCS").is_none()); + assert!(reloaded + .get_entity(handle) + .expect("the line survives the round trip") + .common() + .extended_data + .get_record("OCS") + .is_none()); + assert!( + !contains_marker(&codec::DwgWriter::write_to_vec(&reloaded).unwrap()), + "a re-save of the reopened file stays marker-free" + ); + + // DXF: same contract for the text format. + let dxf = codec::DxfWriter::new(host.document()) + .write_to_vec() + .unwrap(); + assert!( + !contains_marker(&dxf), + "DXF save must not contain the designation marker" + ); + let reloaded = crate::io::load_bytes("reference-only.dxf", dxf).unwrap(); + assert!(reloaded.app_ids.get("OCS").is_none()); + assert!(reloaded + .get_entity(handle) + .expect("the line survives the round trip") + .common() + .extended_data + .get_record("OCS") + .is_none()); + } + + #[test] + fn erasing_a_designated_entity_undoes_designation_with_it() { let mut app = OpenCADStudio::new_for_test(); app.tabs[0].is_start = false; let mut host = HostSession::new(&mut app, 0); let handle = host.add_entity(reference_test_line(10.0, 0.0, Handle::NULL)); host.set_reference_only(handle, true).unwrap(); - let flagged = |document: &codec::CadDocument| { - document - .get_entity(handle) - .is_some_and(crate::scene::reference_only::entity_is_reference_only) - }; - // The host APPID the marker lives under travels with the record. - assert!(host.document().app_ids.get("OCS").is_some()); + // The plugin erase path removes a designated entity inside a scope: + // entity and set entry leave together. + host.push_undo("CVI: replace the underlying model"); + host.remove_entity(handle); + drop(host); + app.finish_pending_history(0); + let host = HostSession::new(&mut app, 0); + assert!(host.app.tabs[0].scene.document.get_entity(handle).is_none()); + assert!(!host.app.tabs[0].scene.is_reference_only(handle)); + drop(host); - let dwg = codec::DwgWriter::write_to_vec(host.document()).unwrap(); - let reloaded = crate::io::load_bytes("reference-only.dwg", dwg).unwrap(); - assert!(flagged(&reloaded), "DWG save/reopen keeps the designation"); - assert!(reloaded.app_ids.get("OCS").is_some()); - - let dxf = codec::DxfWriter::new(host.document()) - .write_to_vec() - .unwrap(); - let reloaded = crate::io::load_bytes("reference-only.dxf", dxf).unwrap(); - assert!(flagged(&reloaded), "DXF save/reopen keeps the designation"); - assert!(reloaded.app_ids.get("OCS").is_some()); + // Undoing the erase restores the entity AND its designation: the set + // mutation rode the scope as an explicit designation channel. + app.undo_steps(1); + let host = HostSession::new(&mut app, 0); + assert!( + host.app.tabs[0].scene.document.get_entity(handle).is_some(), + "undo restored the erased entity" + ); + assert!( + host.app.tabs[0].scene.is_reference_only(handle), + "undo restored the designation together with its entity" + ); } #[test] diff --git a/src/app/record_api.rs b/src/app/record_api.rs index 53e63a807..af7d10cfb 100644 --- a/src/app/record_api.rs +++ b/src/app/record_api.rs @@ -226,7 +226,7 @@ fn record( value } -fn entity_record(entity: &codec::EntityType) -> Result { +fn entity_record(entity: &codec::EntityType, reference_only: bool) -> Result { let (kind, properties) = enum_parts(entity)?; let mut value = record( "entities", @@ -245,7 +245,8 @@ fn entity_record(entity: &codec::EntityType) -> Result { "record_type".into(), json!(crate::entities::names::dxf_name(entity)), ); - let reference_only = crate::scene::reference_only::entity_is_reference_only(entity); + // Scene-scoped designation (API v9): the set membership of this entity's + // handle, resolved by the caller from the live scene. object.insert("reference_only".into(), json!(reference_only)); Ok(value) } @@ -307,12 +308,13 @@ fn map_records( .collect() } -fn collection_records( - document: &codec::CadDocument, - collection: &str, -) -> Result, String> { +fn collection_records(scene: &crate::scene::Scene, collection: &str) -> Result, String> { + let document = &scene.document; match collection { - "entities" => document.entities().map(entity_record).collect(), + "entities" => document + .entities() + .map(|entity| entity_record(entity, scene.is_reference_only(entity.common().handle))) + .collect(), "objects" => { let mut objects: Vec<_> = document.objects.iter().collect(); objects.sort_by_key(|(handle, _)| handle.value()); @@ -1387,7 +1389,7 @@ impl OpenCADStudio { } let definitions = referenced_definitions(&root_type); let runtime_schema = collection.and_then(|collection| { - collection_records(&self.tabs[self.active_tab].scene.document, collection) + collection_records(&self.tabs[self.active_tab].scene, collection) .ok()? .into_iter() .find(|record| { @@ -1431,20 +1433,20 @@ impl OpenCADStudio { } pub(super) fn record_query(&self, request: &Value) -> Value { - let document = &self.tabs[self.active_tab].scene.document; + let scene = &self.tabs[self.active_tab].scene; let Some(collection) = request["collection"].as_str() else { return self.record_capabilities(); }; let mut records = Vec::new(); if collection == "all" { for (name, _) in COLLECTIONS { - match collection_records(document, name) { + match collection_records(scene, name) { Ok(mut values) => records.append(&mut values), Err(error) => return failure("serialization_failed", error), } } } else { - match collection_records(document, collection) { + match collection_records(scene, collection) { Ok(values) => records = values, Err(error) => return failure("unknown_collection", error), } @@ -1712,14 +1714,18 @@ mod tests { Vector3::new(10.0, 0.0, 0.0), ), )); - fn mark(app: &mut OpenCADStudio, i: usize, handle: Handle, on: bool) { - if let Some(entity) = app.tabs[i].scene.document.get_entity_mut(handle) { - crate::scene::reference_only::set_entity_reference_only(entity, on); - } + // The designation is plugin-mutated scene state: mark through the + // same host API a plugin uses, so the read surface reflects the + // live set rather than any stored marker. + fn mark(app: &mut OpenCADStudio, handle: Handle, on: bool) { + let i = app.active_tab; + crate::app::plugin_host::HostSession::new(app, i) + .set_reference_only(handle, on) + .expect("mark through the plugin host API"); } // The read surface reports the designation alongside the record. - mark(&mut app, i, handle, true); + mark(&mut app, handle, true); let records = app.record_query(&json!({"collection":"entities"})); assert_eq!(records["count"], 1); assert_eq!( @@ -1744,7 +1750,7 @@ mod tests { assert_eq!(refused["code"], json!("reference_only")); // Clearing the designation is reported too. - mark(&mut app, i, handle, false); + mark(&mut app, handle, false); let records = app.record_query(&json!({"collection":"entities"})); assert_eq!(records["records"][0]["reference_only"], json!(false)); } diff --git a/src/entities/dimension.rs b/src/entities/dimension.rs index 07f2292b6..fd021e0f2 100644 --- a/src/entities/dimension.rs +++ b/src/entities/dimension.rs @@ -3598,6 +3598,7 @@ fn apply_dimension_jog( } pub trait DimensionTess { + #[allow(clippy::too_many_arguments)] fn tessellate( &self, document: &CadDocument, @@ -3611,10 +3612,12 @@ pub trait DimensionTess { bg_color: [f32; 4], view_aabb: Option<[f32; 4]>, world_per_pixel: Option, + reference_only: &rustc_hash::FxHashSet, ) -> Vec; } impl DimensionTess for Dimension { + #[allow(clippy::too_many_arguments)] fn tessellate( &self, document: &CadDocument, @@ -3628,6 +3631,7 @@ impl DimensionTess for Dimension { bg_color: [f32; 4], view_aabb: Option<[f32; 4]>, world_per_pixel: Option, + reference_only: &rustc_hash::FxHashSet, ) -> Vec { tessellate_dimension_inner( document, @@ -3642,10 +3646,12 @@ impl DimensionTess for Dimension { bg_color, view_aabb, world_per_pixel, + reference_only, ) } } +#[allow(clippy::too_many_arguments)] fn tessellate_dimension_inner( document: &CadDocument, handle: Handle, @@ -3662,6 +3668,7 @@ fn tessellate_dimension_inner( bg_color: [f32; 4], view_aabb: Option<[f32; 4]>, world_per_pixel: Option, + reference_only: &rustc_hash::FxHashSet, ) -> Vec { if let Dimension::Arc(arc) = dim { if let Some((local_arc, normal)) = arc_dimension_in_ocs(arc) { @@ -3678,6 +3685,7 @@ fn tessellate_dimension_inner( bg_color, None, world_per_pixel, + reference_only, ); for wire in &mut wires { map_wire_ocs_to_wcs(wire, normal); @@ -3700,6 +3708,7 @@ fn tessellate_dimension_inner( bg_color, None, world_per_pixel, + reference_only, ); for wire in &mut wires { map_wire_ocs_to_wcs(wire, normal); @@ -3721,6 +3730,7 @@ fn tessellate_dimension_inner( bg_color, None, world_per_pixel, + reference_only, ); for wire in &mut wires { map_wire_ocs_to_wcs(wire, normal); @@ -4420,6 +4430,7 @@ fn tessellate_dimension_inner( view_aabb, world_per_pixel, text_color, + reference_only, ); for mut w in text_wires { w.name = name.clone(); @@ -4437,6 +4448,7 @@ fn tessellate_dimension_inner( view_aabb, world_per_pixel, text_color, + reference_only, ); for mut w in tol_wires { w.name = name.clone(); @@ -8890,6 +8902,7 @@ mod linear_transform_tests { [0.0, 0.0, 0.0, 1.0], None, None, + &rustc_hash::FxHashSet::default(), ); let mut points = Vec::new(); for wire in &wires { @@ -9221,6 +9234,7 @@ mod layout_parity_tests { [0.0, 0.0, 0.0, 1.0], None, None, + &rustc_hash::FxHashSet::default(), ); let mut points = Vec::new(); let mut fills = 0; diff --git a/src/scene/cache/block_cache.rs b/src/scene/cache/block_cache.rs index a7d2e648f..6e409e0a2 100644 --- a/src/scene/cache/block_cache.rs +++ b/src/scene/cache/block_cache.rs @@ -772,6 +772,9 @@ fn tessellate_sub_local( // top-level entities, applied per-defn. let wires_out = if let EntityType::Dimension(dimension) = sub { use crate::entities::dimension::DimensionTess; + // Block-definition members never carry the reference-only + // designation (it targets top-level geometry — see the API v9 + // known-limit note), so the block cache tessellates with an empty set. dimension.tessellate( doc, h, @@ -784,6 +787,7 @@ fn tessellate_sub_local( bg_color, None, None, + &HashSet::default(), ) } else { tessellate::tessellate( diff --git a/src/scene/convert/tess.rs b/src/scene/convert/tess.rs index a2e5e384a..87f2630b3 100644 --- a/src/scene/convert/tess.rs +++ b/src/scene/convert/tess.rs @@ -473,6 +473,7 @@ fn section_symbol_wires( /// so it picks up the standard text LOD ladder (baseline / greek / full), /// then re-color the returned wires with the dimension's resolved text colour /// (so DIMCLRT / DIMSTYLE colours win over the synthetic Text's defaults). +#[allow(clippy::too_many_arguments)] pub(crate) fn tessellate_entity_dim_text( document: &codec::CadDocument, selected: &HashSet, @@ -483,6 +484,7 @@ pub(crate) fn tessellate_entity_dim_text( view_aabb: Option<[f32; 4]>, world_per_pixel: Option, text_color: [f32; 4], + reference_only: &HashSet, ) -> Vec { let mut wires = tessellate_entity( document, @@ -496,6 +498,7 @@ pub(crate) fn tessellate_entity_dim_text( view_aabb, world_per_pixel, false, + reference_only, ); for w in &mut wires { // Synth dim text carries no real entity colour — paint everything @@ -508,6 +511,7 @@ pub(crate) fn tessellate_entity_dim_text( } wires } +#[allow(clippy::too_many_arguments)] pub(crate) fn tessellate_entity( document: &codec::CadDocument, selected: &HashSet, @@ -520,6 +524,7 @@ pub(crate) fn tessellate_entity( view_aabb: Option<[f32; 4]>, world_per_pixel: Option, paper_space: bool, + reference_only: &HashSet, ) -> Vec { let mut wires = tessellate_entity_inner( document, @@ -533,6 +538,7 @@ pub(crate) fn tessellate_entity( view_aabb, world_per_pixel, paper_space, + reference_only, ); let layer_plottable = document @@ -550,8 +556,10 @@ pub(crate) fn tessellate_entity( // Reference-only entities (API v9) render and snap normally but must not // be picked by the built-in edit paths; the pick-side wire filters in // `scene::pick::hit_test` and the selection-gather retains consume the - // flag. - if crate::scene::reference_only::entity_is_reference_only(e) { + // flag. The stamp is driven by the scene's in-memory designation set — + // outside `Scene` this stays pure data, so the parallel tessellation + // workers only need the handle set, not scene access. + if reference_only.contains(&e.common().handle) { for wire in &mut wires { wire.reference_only = true; } @@ -559,6 +567,7 @@ pub(crate) fn tessellate_entity( wires } +#[allow(clippy::too_many_arguments)] fn tessellate_entity_inner( document: &codec::CadDocument, selected: &HashSet, @@ -577,6 +586,10 @@ fn tessellate_entity_inner( // Retained through recursive INSERT expansion; PSLTSCALE itself is applied // by the viewport's GPU uniform so it never changes resident wire content. _paper_space: bool, + // The scene's reference-only designation set (API v9): drives the + // `WireModel::reference_only` stamp for dimensions' synthesis paths that + // bypass the wrapper's post-stamp. + reference_only: &HashSet, ) -> Vec { let contextual = crate::scene::annotative::entity_for_annotation_context( document, @@ -1029,6 +1042,7 @@ fn tessellate_entity_inner( bg_color, view_aabb, world_per_pixel, + reference_only, ); for w in &mut wires { w.aci = aci; diff --git a/src/scene/dimension_assoc.rs b/src/scene/dimension_assoc.rs index c51da0c15..bbb40ae90 100644 --- a/src/scene/dimension_assoc.rs +++ b/src/scene/dimension_assoc.rs @@ -2279,7 +2279,7 @@ mod tests { scene.sync_diameter_association_angle(dimension); let recording = scene.take_undo_recording().expect("undo recording"); - let (_, objects, _, _) = recording.into_recorded_images(); + let (_, objects, _, _, _) = recording.into_recorded_images(); assert!(objects .iter() .any(|(handle, before)| *handle == association && before.is_some())); diff --git a/src/scene/mod.rs b/src/scene/mod.rs index 779a46109..d4ccd2271 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -82,6 +82,10 @@ pub(crate) use convert::tess::{ /// Keep construction-history curves in the same per-entity tessellation memo /// as their parent, so navigation never rebuilds them on every frame. +/// +/// `reference_only` is the scene's designation set: its membership decides +/// whether the produced wires carry the `WireModel::reference_only` click- +/// through stamp (see `scene::reference_only`). #[allow(clippy::too_many_arguments)] pub(crate) fn tessellate_entity( document: &CadDocument, @@ -95,6 +99,7 @@ pub(crate) fn tessellate_entity( view_aabb: Option<[f32; 4]>, world_per_pixel: Option, paper_space: bool, + reference_only: &HashSet, ) -> Vec { let mut wires = convert::tess::tessellate_entity( document, @@ -108,6 +113,7 @@ pub(crate) fn tessellate_entity( view_aabb, world_per_pixel, paper_space, + reference_only, ); if matches!(entity, EntityType::Solid3D(_) | EntityType::Surface(_)) { let handle = entity.common().handle; @@ -159,6 +165,7 @@ pub(crate) fn tessellate_entity( view_aabb, world_per_pixel, paper_space, + reference_only, ) }; for wire in &mut construction { @@ -758,6 +765,13 @@ pub struct UndoRecording { >, parametric_constraints_order: Vec, named_parameters_before: Option, + /// First-touch before-images of reference-only designations (API v9) + /// changed by the command — almost always empty. The designation set + /// lives on `Scene`, not in the document, so like + /// `parametric_constraints_before` it needs its own channel. Erasing a + /// designated entity drops its set entry here so undo restores entity + /// and designation together. + reference_designations_before: Vec<(Handle, bool)>, poisoned: bool, } @@ -773,6 +787,7 @@ impl UndoRecording { && self.object_order.is_empty() && self.parametric_constraints_order.is_empty() && self.named_parameters_before.is_none() + && self.reference_designations_before.is_empty() } /// Consume the recording directories in deterministic first-touch @@ -790,6 +805,7 @@ impl UndoRecording { parametric_constraints::ParametricConstraintSet, )>, Option, + Vec<(Handle, bool)>, ) { let entities = self .order @@ -810,11 +826,13 @@ impl UndoRecording { .map(|before| (scope, before)) }) .collect(); + let reference_designations = std::mem::take(&mut self.reference_designations_before); ( entities, objects, parametric_constraints, self.named_parameters_before, + reference_designations, ) } @@ -2062,6 +2080,11 @@ pub struct Scene { pub selected_constraint: Option, /// Session-only ISOLATEOBJECTS / HIDEOBJECTS state. Never written to DWG/DXF. pub object_isolation: ObjectIsolationState, + /// Scene-scoped reference-only designations (API v9): plugin-owned + /// display geometry the built-in edit paths refuse. In-memory only, + /// keyed by handle, lives and dies with this scene — never written to + /// DWG/DXF (see `scene::reference_only`). + pub(crate) reference_only: reference_only::ReferenceOnlyDesignation, /// Entity handles temporarily removed from the base render while an /// interactive grip preview draws their live replacement. /// Separate from object isolation so a grip can never activate the @@ -2694,6 +2717,7 @@ impl Scene { selected_order: Vec::new(), selected_constraint: None, object_isolation: ObjectIsolationState::default(), + reference_only: Default::default(), preview_hidden: HashSet::default(), command_preview_hidden: HashSet::default(), refedit_keep: None, @@ -3341,6 +3365,26 @@ impl Scene { } } + /// Record a reference-only designation's value before its first change + /// within the open delta recording (API v9). The designation set lives on + /// `Scene`, outside the document, so it needs its own channel — like + /// the parametric-constraint directory above — for undo to restore the + /// designation together with the entity it belongs to. First touch wins. + pub(crate) fn record_undo_reference_designation_before(&mut self, handle: Handle) { + if let Some(recording) = self.undo_recording.as_mut() { + if !recording + .reference_designations_before + .iter() + .any(|(recorded, _)| *recorded == handle) + { + let before = self.reference_only.is_marked(handle); + recording + .reference_designations_before + .push((handle, before)); + } + } + } + /// Flag the open recording as touching non-entity state (a new layer, a /// group edit, a `*D` block record) that a pure-entity delta cannot /// restore. The app's per-command predicate keeps this from firing. @@ -3398,6 +3442,13 @@ impl Scene { } } + // History replay can apply an entity removal the designation channels + // predate (an "added" side rolled back). Sweep so the set never keeps + // an entry for an absent entity; exact designations come right after + // this from the delta's own channel. + if !self.reference_only.is_empty() { + self.reference_only.retain_present(&self.document); + } if !changes.is_empty() { self.invalidate_dependency_index(); } @@ -7250,6 +7301,7 @@ impl Scene { None, None, anno_scale_override.is_some(), + self.reference_only.handles(), ); // Solid B-rep corners snap via the mesh edge list (their wire // path is an empty shell). Enrich before the memo/assembly split @@ -9153,9 +9205,15 @@ impl Scene { /// locked-layer entity while its owning plugin keeps write access through /// the plugin host API. Absent entities are never reference-only. pub fn is_reference_only(&self, handle: Handle) -> bool { - self.document - .get_entity(handle) - .is_some_and(reference_only::entity_is_reference_only) + self.reference_only.is_marked(handle) && self.document.get_entity(handle).is_some() + } + + /// Insert or remove `handle` in the scene-scoped designation set, + /// returning whether membership changed. Pure storage: no epoch bump, no + /// undo bookkeeping — those belong to the callers (the plugin host API + /// mutation and the history channels). + pub(crate) fn set_reference_only_membership(&mut self, handle: Handle, on: bool) -> bool { + self.reference_only.set(handle, on) } /// The name of the locked layer `handle` sits on, if any (for messages). @@ -11347,6 +11405,9 @@ impl Scene { // `world_offset` subtraction even though `current_layout != "Model"`. // Decide based on the block being tessellated, not the layout. let is_model_block = block_handle == self.model_space_block_handle(); + // The reference-only designation set drives the wire stamp; captured + // as a shared ref like `doc`/`sel` so the rayon workers can read it. + let reference_only_set = self.reference_only.handles(); let bg = if self.current_layout == "Model" { self.bg_color } else { @@ -11470,6 +11531,7 @@ impl Scene { view_aabb, wpp, paper, + reference_only_set, ); (e.common().handle, Arc::new(w)) }) @@ -11509,6 +11571,7 @@ impl Scene { view_aabb, wpp, paper, + reference_only_set, ) }) .collect(); @@ -11543,6 +11606,7 @@ impl Scene { view_aabb, wpp, paper, + reference_only_set, )), Ok(None) => Some(Vec::new()), Err(()) => None, @@ -12563,6 +12627,7 @@ vis_index={:.1} visible_probe={:.1}", None, None, paper, + self.reference_only.handles(), ); // Same solid-corner snaps as the resident paths, so one-off consumers // (overlay changed-wires, edit previews) agree with hit-test/snap. @@ -14048,7 +14113,7 @@ mod delta_undo_tests { let handle = scene.add_entity(EntityType::RasterImage(image)); let rec = scene.take_undo_recording().unwrap(); assert!(!rec.is_poisoned()); - let (entities, objects, _parametric_constraints, _named_parameters) = + let (entities, objects, _parametric_constraints, _named_parameters, _designations) = rec.into_recorded_images(); assert_eq!(entities.len(), 1); assert_eq!(entities[0].0, handle); @@ -14073,7 +14138,7 @@ mod delta_undo_tests { scene.erase_entities(&[h1]); let rec = scene.take_undo_recording().unwrap(); assert!(!rec.is_poisoned()); - let (entities, objects, _parametric_constraints, _named_parameters) = + let (entities, objects, _parametric_constraints, _named_parameters, _designations) = rec.into_recorded_images(); assert_eq!(entities.len(), 1); assert_eq!(entities[0].0, h1); diff --git a/src/scene/parametric_solve.rs b/src/scene/parametric_solve.rs index 58290abeb..ca72e56f7 100644 --- a/src/scene/parametric_solve.rs +++ b/src/scene/parametric_solve.rs @@ -4502,7 +4502,7 @@ mod tests { .take_undo_recording() .expect("an undo recording should still be open"); - let (entities, _objects, _parametric_constraints, _named_parameters) = + let (entities, _objects, _parametric_constraints, _named_parameters, _designations) = recording.into_recorded_images(); let touched: std::collections::HashSet<_> = entities.iter().map(|(h, _)| *h).collect(); assert!( @@ -4559,7 +4559,7 @@ mod tests { let recording = scene .take_undo_recording() .expect("an undo recording should still be open"); - let (_entities, _objects, parametric_constraints, _named_parameters) = + let (_entities, _objects, parametric_constraints, _named_parameters, _designations) = recording.into_recorded_images(); assert_eq!( parametric_constraints.len(), diff --git a/src/scene/pipeline/circle_gpu.rs b/src/scene/pipeline/circle_gpu.rs index 8e3d8e06b..b2c8285e2 100644 --- a/src/scene/pipeline/circle_gpu.rs +++ b/src/scene/pipeline/circle_gpu.rs @@ -588,6 +588,7 @@ mod tests { let h = doc.add_entity(donut).unwrap(); let entity = doc.get_entity(h).unwrap(); let selected = rustc_hash::FxHashSet::default(); + let reference_only = rustc_hash::FxHashSet::default(); let wires = crate::scene::convert::tess::tessellate_entity( &doc, &selected, @@ -600,6 +601,7 @@ mod tests { None, None, false, + &reference_only, ); assert_eq!( wires.len(), diff --git a/src/scene/reference_only.rs b/src/scene/reference_only.rs index 7ecfe93c9..24a82d098 100644 --- a/src/scene/reference_only.rs +++ b/src/scene/reference_only.rs @@ -7,143 +7,181 @@ //! plugin keeps full write access through the plugin host API, so it can //! refresh derived geometry in place. //! -//! The designation persists as a marker value inside an XDATA record under a -//! host-owned APPID, so it round-trips DWG and DXF with the entity and never -//! collides with plugin APPID namespaces (plugins register their own -//! application names through their manifest). +//! Storage is a scene-scoped in-memory set of entity handles — the host +//! writes nothing of its own into the saved DWG/DXF: no APPID registration, +//! no XDATA marker on the entity, no host-owned state a saved file depends +//! on. The set lives and dies with the scene (per drawing tab): closing a +//! tab forgets its designations, and a freshly opened drawing starts +//! undesignated. The owning plugin re-marks its entities when the host +//! dispatches the document-opened notification (API v9's +//! `BuiltinPlugin::on_notification` surface), which makes the in-memory +//! designation fully equivalent to a persisted one for its use case. +//! +//! Undo keeps the entity-write contract: a designation change performed +//! inside a plugin-opened `push_undo` scope joins that scope, a scope-less +//! change is one undo step of its own, and undo/redo restores the +//! designation with the entities it belongs to. Because the set is not part +//! of the document, set mutations are recorded as explicit first-touch +//! before-images on the history delta (see +//! `DeltaSnapshot::reference_designations`), the same channel pattern +//! `parametric_constraints` uses. -use codec::entities::EntityType; -use codec::xdata::{ExtendedDataRecord, XDataValue}; +use codec::types::Handle; +use rustc_hash::FxHashSet; -/// Host-owned APPID carrying the designation. The name belongs to the host -/// alone; plugin records live under the plugin's own registered APPIDs. -pub const REFERENCE_ONLY_APP_ID: &str = "OCS"; - -/// Marker value (DXF group code 1000) that flags an entity reference-only. -/// Human-readable so an exported file explains itself. -const REFERENCE_ONLY_MARKER: &str = "REFERENCE_ONLY"; - -/// True when `value` is the reference-only marker. -fn is_marker(value: &XDataValue) -> bool { - matches!(value, XDataValue::String(text) if text.as_str() == REFERENCE_ONLY_MARKER) +/// The in-memory designation set owned by a [`Scene`](crate::Scene). +/// +/// Deliberately tiny: a plain set of handles plus the operations the scene, +/// the plugin host and the history channels use. Not `Serialize` — the set +/// is never written anywhere and never leaves the process. +#[derive(Debug, Default, Clone, PartialEq)] +pub struct ReferenceOnlyDesignation { + handles: FxHashSet, } -/// True when `entity` carries the reference-only designation. Tolerant read: -/// a record without the marker (foreign or damaged content) is not -/// reference-only. -pub fn entity_is_reference_only(entity: &EntityType) -> bool { - let Some(record) = entity - .common() - .extended_data - .get_record(REFERENCE_ONLY_APP_ID) - else { - return false; - }; - record.values.iter().any(is_marker) -} - -/// Set (or clear) the designation on `entity`, returning whether the entity -/// changed. Only the marker value is added or removed; foreign values under -/// the host APPID are preserved verbatim, and a record left without values is -/// dropped so a cleared flag leaves no empty record behind. -pub fn set_entity_reference_only(entity: &mut EntityType, reference_only: bool) -> bool { - if entity_is_reference_only(entity) == reference_only { - return false; +impl ReferenceOnlyDesignation { + /// Raw membership (by handle) — the caller guarantees the entity exists, + /// or accepts a stale entry; [`Scene::is_reference_only`] does the + /// existential check. + pub(crate) fn is_marked(&self, handle: Handle) -> bool { + self.handles.contains(&handle) } - let xd = &mut entity.common_mut().extended_data; - if reference_only { - let mut record = xd - .get_record(REFERENCE_ONLY_APP_ID) - .cloned() - .unwrap_or_else(|| ExtendedDataRecord::new(REFERENCE_ONLY_APP_ID)); - record - .values - .push(XDataValue::String(REFERENCE_ONLY_MARKER.to_owned())); - xd.upsert_record(record); - } else { - let Some(mut record) = xd.get_record(REFERENCE_ONLY_APP_ID).cloned() else { - return false; - }; - record.values.retain(|value| !is_marker(value)); - if record.values.is_empty() { - xd.remove_record(REFERENCE_ONLY_APP_ID); + + /// Insert or remove `handle`, returning whether the membership changed. + /// Idempotent: re-marking a designated handle reports `false` and touches + /// nothing, which is what terminates the re-mark loop driven by the + /// document-changed notification dispatch. + pub(crate) fn set(&mut self, handle: Handle, on: bool) -> bool { + if on { + self.handles.insert(handle) } else { - xd.upsert_record(record); + self.handles.remove(&handle) } } - true + + /// Forget `handle` — called when its entity is erased so the set never + /// leaks an entry for a removed entity. The undo channels restore the + /// entry together with the entity on undo. + pub(crate) fn remove(&mut self, handle: Handle) { + self.handles.remove(&handle); + } + + /// The marked handles, for the tessellation pipelines (wire stamping) + /// and the history channels. + pub(crate) fn handles(&self) -> &FxHashSet { + &self.handles + } + + /// True when nothing is designated. + pub(crate) fn is_empty(&self) -> bool { + self.handles.is_empty() + } + + /// Drop entries whose entities no longer exist. The undo/redo channels + /// model the exact designation state, so this is only a defensive sweep + /// after history replay (an entity the chosen delta side removed must + /// never stay designated through a stale entry). + pub(crate) fn retain_present(&mut self, document: &codec::CadDocument) { + self.handles + .retain(|handle| document.get_entity(*handle).is_some()); + } } #[cfg(test)] mod tests { use super::*; - use codec::entities::Line; + use codec::entities::{EntityType, Line}; - fn entity() -> EntityType { - EntityType::Line(Line::new()) + fn handle(value: u64) -> Handle { + Handle::new(value) } #[test] - fn marking_flags_and_unmarking_clears() { - let mut entity = entity(); - assert!(!entity_is_reference_only(&entity)); - assert!(set_entity_reference_only(&mut entity, true)); - assert!(entity_is_reference_only(&entity)); - // Idempotent: a second mark changes nothing and reports it. - assert!(!set_entity_reference_only(&mut entity, true)); - assert!(set_entity_reference_only(&mut entity, false)); - assert!(!entity_is_reference_only(&entity)); - assert!(!set_entity_reference_only(&mut entity, false)); - // A cleared flag leaves no empty record behind. - assert!(entity - .common() - .extended_data - .get_record(REFERENCE_ONLY_APP_ID) - .is_none()); + fn marking_and_unmarking_report_and_track_membership() { + let mut designation = ReferenceOnlyDesignation::default(); + assert!(!designation.is_marked(handle(7))); + assert!(designation.set(handle(7), true)); + assert!(designation.is_marked(handle(7))); + // Idempotent: a second mark changes nothing and reports it — the + // property that terminates re-mark loops. + assert!(!designation.set(handle(7), true)); + assert!(designation.set(handle(7), false)); + assert!(!designation.is_marked(handle(7))); + assert!(!designation.set(handle(7), false)); + assert!(designation.handles().is_empty()); } #[test] - fn foreign_values_under_the_host_app_id_survive() { - let mut entity = entity(); - let mut record = ExtendedDataRecord::new(REFERENCE_ONLY_APP_ID); - record.values.push(XDataValue::Integer16(7)); - entity.common_mut().extended_data.add_record(record); - // A record without the marker is not reference-only. - assert!(!entity_is_reference_only(&entity)); - assert!(set_entity_reference_only(&mut entity, true)); - let record = entity - .common() - .extended_data - .get_record(REFERENCE_ONLY_APP_ID) - .expect("record exists"); - assert_eq!( - record.values, - vec![ - XDataValue::Integer16(7), - XDataValue::String(REFERENCE_ONLY_MARKER.to_owned()), - ] - ); - assert!(set_entity_reference_only(&mut entity, false)); - let record = entity - .common() - .extended_data - .get_record(REFERENCE_ONLY_APP_ID) - .expect("record survives"); - assert_eq!(record.values, vec![XDataValue::Integer16(7)]); + fn removing_a_handle_forgets_it() { + let mut designation = ReferenceOnlyDesignation::default(); + designation.set(handle(7), true); + designation.remove(handle(7)); + assert!(!designation.is_marked(handle(7))); + // Removing an unmarked handle is a no-op. + designation.remove(handle(8)); + assert!(designation.is_empty()); } #[test] - fn other_apps_are_untouched() { - let mut entity = entity(); - assert!(set_entity_reference_only(&mut entity, true)); - let mut foreign = ExtendedDataRecord::new("CVIFC"); - foreign.values.push(XDataValue::String("road".to_owned())); - entity.common_mut().extended_data.add_record(foreign); - assert!(set_entity_reference_only(&mut entity, false)); - assert!(entity - .common() - .extended_data - .get_record("CVIFC") - .is_some_and(|record| record.values.len() == 1)); + fn retain_present_drops_entries_of_absent_entities() { + let mut document = codec::CadDocument::new(); + let live = document.add_entity(EntityType::Line(Line::new())).unwrap(); + let mut designation = ReferenceOnlyDesignation::default(); + designation.set(live, true); + designation.set(handle(999_999), true); + designation.retain_present(&document); + assert!(designation.is_marked(live)); + assert!(!designation.is_marked(handle(999_999))); + } + + #[test] + fn scene_membership_is_guarded_by_entity_presence() { + let mut scene = crate::scene::Scene::new(); + let handle = scene + .document + .add_entity(EntityType::Line(Line::new())) + .unwrap(); + assert!(!scene.is_reference_only(handle)); + assert!(scene.set_reference_only_membership(handle, true)); + assert!(scene.is_reference_only(handle)); + // A stale set entry alone never designates: absent entities are not + // reference-only even if the set still lists them. + scene.set_reference_only_membership(Handle::new(999_999), true); + assert!(!scene.is_reference_only(Handle::new(999_999))); + } + + #[test] + fn tessellated_wires_stamp_from_the_designation_set() { + let document = codec::CadDocument::new(); + let line = EntityType::Line(Line::new()); + let handle = line.common().handle; + let selected: rustc_hash::FxHashSet = Default::default(); + let none: rustc_hash::FxHashSet = Default::default(); + let mut designated: rustc_hash::FxHashSet = Default::default(); + designated.insert(handle); + let context = crate::scene::cache::block_cache::BlockCache::default(); + for designated in [&none, &designated] { + let wires = crate::scene::tessellate_entity( + &document, + &selected, + None, + [0.0, 0.0, 0.0, 1.0], + 1.0, + None, + &line, + Some(&context), + None, + None, + false, + designated, + ); + assert!(!wires.is_empty()); + assert!( + wires + .iter() + .all(|wire| wire.reference_only == designated.contains(&handle)), + "every wire carries the set's stamp for its entity" + ); + } } } diff --git a/src/scene/selection.rs b/src/scene/selection.rs index 95dc55353..d533f57d2 100644 --- a/src/scene/selection.rs +++ b/src/scene/selection.rs @@ -1143,6 +1143,12 @@ impl Scene { let before = self.document.objects.get(&manager_handle).cloned(); self.record_undo_object_before(manager_handle, before); } + // The designation set is scene state, not document state: a + // designated entity's set entry joins the same first-touch + // recording so undo restores entity and designation together. + if self.reference_only.is_marked(h) { + self.record_undo_reference_designation_before(h); + } } self.delete_solid_history(h); if let Some(settings_handle) = settings_handle { @@ -1161,6 +1167,9 @@ impl Scene { } self.remember_removed_cache_categories(h); self.document.remove_entity_arc(h); + // The set entry dies with its entity (an absent entity is never + // reference-only). Undo restores it through the recorded channel. + self.reference_only.remove(h); selection_changed |= self.selected.remove(&h); self.selected_order.retain(|selected| *selected != h); if self.hover_highlight == Some(h) { diff --git a/src/scene/view/render.rs b/src/scene/view/render.rs index 9cb39f5c5..a87a65475 100644 --- a/src/scene/view/render.rs +++ b/src/scene/view/render.rs @@ -4163,6 +4163,7 @@ impl Scene { None, None, content_viewport, + self.reference_only.handles(), ); for wire in &mut context_wires { wire.color = tint;