From 490bb86b33d4cd45d2300584883c96fc3b862ad0 Mon Sep 17 00:00:00 2001 From: Kayshen-X Date: Thu, 14 May 2026 14:52:24 +0800 Subject: [PATCH] feat(shell): wire Document.var_table + canonical loader populates from PenDocument MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the gap between `VariableTable` (commits `62b08b93` / `dbfd2f1a`) and the canonical `.op` loader: opening a file now preserves `.variables` + `.themes` straight onto `Document.var_table` instead of dropping them at the door. - `Document.var_table: VariableTable` field — Default = empty table. Patched 9 test fixtures (`tests_geometry.rs` × 7 + `canvas_viewport.rs` + `layer_panel_tests.rs`) to seed with `VariableTable::default()` alongside `history`. - `pen_doc_adapter::build_var_table(&PenDocument) -> VariableTable` maps `jian_ops_schema::variable::*` → shell-core types: the enums are isomorphic (Color/Number/Boolean/String; Bool/Num/Str; Scalar/Themed). Theme axes copy through directly. - `persistence.rs` open path: when the canonical branch fires, `build_var_table` runs alongside `pen_document_to_payload`. The result is held in a local `Option` because `apply_payload` doesn't know about variables; after it resets the document, the held value is assigned to `doc.var_table`. Round-trip: load → `doc.var_table.find("color-1")` returns the right `Variable`, `doc.var_table.resolve("color-1")` returns the themed value under the current `active_theme` (empty default — picks the `theme = None` entry, mirroring `Variable::resolve`'s fallback). UI for switching `active_theme` (Variables panel) + paint-time `$ref` substitution are the remaining follow-ups for #5. Tests: 184 shell-core + 19 shell-native (no new assertions in this commit; variables algorithm tests landed in `dbfd2f1a`). Desktop builds clean. --- .../openpencil-desktop/src/pen_doc_adapter.rs | 62 +++++++++++++++++++ crates/openpencil-desktop/src/persistence.rs | 13 ++-- crates/openpencil-shell-core/src/document.rs | 3 + .../src/document/mutators.rs | 2 + .../src/document/tests_geometry.rs | 7 +++ .../src/widgets/canvas_viewport.rs | 1 + .../src/widgets/layer_panel_tests.rs | 1 + 7 files changed, 85 insertions(+), 4 deletions(-) diff --git a/crates/openpencil-desktop/src/pen_doc_adapter.rs b/crates/openpencil-desktop/src/pen_doc_adapter.rs index 6ce37da28..d0d2c07ef 100644 --- a/crates/openpencil-desktop/src/pen_doc_adapter.rs +++ b/crates/openpencil-desktop/src/pen_doc_adapter.rs @@ -81,6 +81,68 @@ pub fn pen_document_to_payload(doc: &PenDocument) -> LoadedDoc { } } +/// Copy `PenDocument.variables` + `.themes` into a shell-core +/// `VariableTable`. Caller assigns the result to `Document.var_table` +/// AFTER `apply_payload` (which clears it via Default). Lossless on +/// the supported `VariableDefinition` variants; unknown future +/// `VariableKind`s round-trip via their `Color/Number/Boolean/String` +/// label since the enums are isomorphic. +pub fn build_var_table(doc: &PenDocument) -> openpencil_shell_core::document::VariableTable { + use openpencil_shell_core::document::{ + ThemeAxis, ThemedValue, Variable, VariableKind, VariableScalar, VariableTable, + VariableValue, + }; + let mut out = VariableTable::default(); + if let Some(themes) = &doc.themes { + for (axis_name, values) in themes { + out.themes.push(ThemeAxis { + name: axis_name.clone(), + values: values.clone(), + }); + } + } + if let Some(vars) = &doc.variables { + for (name, def) in vars { + let kind = match def.kind { + jian_ops_schema::variable::VariableKind::Color => VariableKind::Color, + jian_ops_schema::variable::VariableKind::Number => VariableKind::Number, + jian_ops_schema::variable::VariableKind::Boolean => VariableKind::Boolean, + jian_ops_schema::variable::VariableKind::String => VariableKind::String, + }; + let value = match &def.value { + jian_ops_schema::variable::VariableValue::Scalar(s) => { + VariableValue::Scalar(map_scalar(s)) + } + jian_ops_schema::variable::VariableValue::Themed(arr) => VariableValue::Themed( + arr.iter() + .map(|tv| ThemedValue { + value: map_scalar(&tv.value), + theme: tv.theme.clone(), + }) + .collect(), + ), + }; + out.variables.push(Variable { + name: name.clone(), + kind, + value, + }); + } + } + out +} + +fn map_scalar( + s: &jian_ops_schema::variable::VariableScalar, +) -> openpencil_shell_core::document::VariableScalar { + use openpencil_shell_core::document::VariableScalar; + match s { + jian_ops_schema::variable::VariableScalar::Bool(b) => VariableScalar::Bool(*b), + jian_ops_schema::variable::VariableScalar::Num(n) => VariableScalar::Num(*n), + jian_ops_schema::variable::VariableScalar::Str(s) => VariableScalar::Str(s.clone()), + } +} + fn build_page(id: &str, name: &str, roots: &[PenNode], page_idx: usize) -> PagePayload { let mut layout_rects: BTreeMap = BTreeMap::new(); for root in roots { diff --git a/crates/openpencil-desktop/src/persistence.rs b/crates/openpencil-desktop/src/persistence.rs index 233e6172c..fbcaab9ed 100644 --- a/crates/openpencil-desktop/src/persistence.rs +++ b/crates/openpencil-desktop/src/persistence.rs @@ -392,7 +392,11 @@ fn load_from_path(doc: &mut Document, path: &std::path::Path) -> Result<(), Stri // its own Save), then fall back to the canonical `.op` / // `.pen` schema so files from the TS editor, Jian apps, or // anything else emitting the format load through the shared - // parser instead of a private adapter. + // parser instead of a private adapter. Side-channel: when the + // canonical path fires, also harvest `variables` + `themes` into + // a VariableTable for assignment after apply_payload (which + // resets Document state via Default). + let mut canonical_var_table: Option = None; let payload = match serde_json::from_slice::(&bytes) { Ok(p) => p, Err(_) => { @@ -402,9 +406,7 @@ fn load_from_path(doc: &mut Document, path: &std::path::Path) -> Result<(), Stri for w in &loaded.warnings { eprintln!("[open] schema warning: {:?}", w); } - // jian-core's `LayoutEngine` runs inside the adapter, - // so the returned payload already carries absolute - // scene-coord rects on every node. + canonical_var_table = Some(crate::pen_doc_adapter::build_var_table(&loaded.value)); let adapted = crate::pen_doc_adapter::pen_document_to_payload(&loaded.value); adapted.payload } @@ -412,6 +414,9 @@ fn load_from_path(doc: &mut Document, path: &std::path::Path) -> Result<(), Stri let page_count = payload.pages.len(); let node_count: usize = payload.pages.iter().map(|p| count_nodes(&p.children)).sum(); apply_payload(doc, payload)?; + if let Some(tbl) = canonical_var_table { + doc.var_table = tbl; + } let bb = active_page_bbox(doc); eprintln!( "[open] {} pages, {} nodes; content bbox {:?}; viewport pan=({:.1},{:.1}) zoom={:.2}", diff --git a/crates/openpencil-shell-core/src/document.rs b/crates/openpencil-shell-core/src/document.rs index 9f877ddab..6239e9514 100644 --- a/crates/openpencil-shell-core/src/document.rs +++ b/crates/openpencil-shell-core/src/document.rs @@ -272,6 +272,9 @@ pub struct Document { pub ui: UiState, /// Undo / redo stacks. Push BEFORE a transactional mutation. pub history: History, + /// Design variables + theme registry. Populated by the canonical + /// `.op` loader; lookup via `Variable::resolve` / `VariableTable::resolve`. + pub var_table: VariableTable, } /// Document undo / redo stacks. Snapshot = deep copy of the diff --git a/crates/openpencil-shell-core/src/document/mutators.rs b/crates/openpencil-shell-core/src/document/mutators.rs index 319835b22..350066b00 100644 --- a/crates/openpencil-shell-core/src/document/mutators.rs +++ b/crates/openpencil-shell-core/src/document/mutators.rs @@ -48,6 +48,7 @@ impl Document { chat: ChatState::default(), ui: UiState::default(), history: History::default(), + var_table: VariableTable::default(), } } @@ -95,6 +96,7 @@ impl Document { chat: ChatState::default(), ui: UiState::default(), history: History::default(), + var_table: VariableTable::default(), }; debug_assert!( doc.validate().is_ok(), diff --git a/crates/openpencil-shell-core/src/document/tests_geometry.rs b/crates/openpencil-shell-core/src/document/tests_geometry.rs index a7110d149..0bf6ef866 100644 --- a/crates/openpencil-shell-core/src/document/tests_geometry.rs +++ b/crates/openpencil-shell-core/src/document/tests_geometry.rs @@ -472,6 +472,7 @@ fn document_validate_catches_duplicate_node_id() { chat: ChatState::default(), ui: UiState::default(), history: crate::document::History::default(), + var_table: crate::document::VariableTable::default(), }; let result = doc.validate(); assert!(result.is_err()); @@ -506,6 +507,7 @@ fn document_validate_catches_empty_pages() { chat: ChatState::default(), ui: UiState::default(), history: crate::document::History::default(), + var_table: crate::document::VariableTable::default(), }; let result = doc.validate(); assert!(result.is_err()); @@ -530,6 +532,7 @@ fn document_validate_catches_empty_pages() { chat: ChatState::default(), ui: UiState::default(), history: crate::document::History::default(), + var_table: crate::document::VariableTable::default(), }; let err2 = doc2.validate().unwrap_err(); assert!( @@ -559,6 +562,7 @@ fn document_selected_node_scopes_to_active_page() { chat: ChatState::default(), ui: UiState::default(), history: crate::document::History::default(), + var_table: crate::document::VariableTable::default(), }; // Selection is on a non-active page → returns None. assert!(doc.selected_node().is_none()); @@ -588,6 +592,7 @@ fn document_active_page_returns_indexed_page() { chat: ChatState::default(), ui: UiState::default(), history: crate::document::History::default(), + var_table: crate::document::VariableTable::default(), }; assert_eq!(doc.active_page().unwrap().name, "second"); } @@ -605,6 +610,7 @@ fn document_active_page_returns_none_when_index_out_of_range() { chat: ChatState::default(), ui: UiState::default(), history: crate::document::History::default(), + var_table: crate::document::VariableTable::default(), }; assert!(doc.active_page().is_none()); } @@ -657,6 +663,7 @@ fn add_page_returns_none_on_id_overflow() { chat: ChatState::default(), ui: UiState::default(), history: crate::document::History::default(), + var_table: crate::document::VariableTable::default(), }; let mut doc = doc; assert_eq!(doc.add_page(), None); diff --git a/crates/openpencil-shell-core/src/widgets/canvas_viewport.rs b/crates/openpencil-shell-core/src/widgets/canvas_viewport.rs index 88fd8942c..e979cfbaf 100644 --- a/crates/openpencil-shell-core/src/widgets/canvas_viewport.rs +++ b/crates/openpencil-shell-core/src/widgets/canvas_viewport.rs @@ -810,6 +810,7 @@ mod tests { chat: crate::document::ChatState::default(), ui: crate::document::UiState::default(), history: crate::document::History::default(), + var_table: crate::document::VariableTable::default(), }; let viewport = CanvasViewport::from_document(&doc); let mut backend = RecordingBackend::default(); diff --git a/crates/openpencil-shell-core/src/widgets/layer_panel_tests.rs b/crates/openpencil-shell-core/src/widgets/layer_panel_tests.rs index fec54e983..90056c407 100644 --- a/crates/openpencil-shell-core/src/widgets/layer_panel_tests.rs +++ b/crates/openpencil-shell-core/src/widgets/layer_panel_tests.rs @@ -135,6 +135,7 @@ fn from_document_scopes_to_active_page_only() { chat: crate::document::ChatState::default(), ui: crate::document::UiState::default(), history: crate::document::History::default(), + var_table: crate::document::VariableTable::default(), }; let panel = LayerPanel::from_document(&doc); assert_eq!(panel.items.len(), 1);