Add named parameters, UI scale setting, OPTIONS/OP dispatch, and help manual

Named parameters (docs/named_parameters_design.md, stages 3-4):
- ParameterTable (src/scene/named_parameters.rs): a document-wide table of
  named formulas, each resolved through a shared expression evaluator with
  cross-parameter references, cycle detection, and division-by-zero/parse
  error reporting. SketchConstraint::driving_param becomes DrivingValue
  (Literal | Named) instead of a bare f64, resolved at solve time by
  sketch_solve::build_constraint via Scene::named_parameters.
- Persistence (src/scene/named_parameters_persist.rs): same lazy XRecord
  model sketch_persist.rs already established, synced around open/save.
  Bumps the sketch-constraint XRecord's own format version (1 -> 2) since
  DrivingValue's shape doesn't line up with a version-1 blob's bytes.
- PARAMETERS command and its editor (src/ui/window/named_parameters.rs):
  add/remove/edit rows, live validation, duplicate-name detection: Apply
  defines the table and re-solves every constraint scope with a
  named-parameter-driven constraint (command_driver.rs). Distance/Angle
  constraint prompts (modules/draw/constrain/value.rs) now accept either a
  literal or an existing parameter's name, mirroring how the constraint's
  own value prompt already worked.
- Undo scope-down carried over from this project's own established pattern
  (constraint-system stage 9): the geometry an Apply moves is one undo
  step, but the parameter table edit itself has no undo entry yet — a
  documented follow-up, not a silent gap.

UI scale setting (src/app/settings.rs, app/mod.rs, ui/window/options.rs):
  a whole-interface scale percentage (50-200%), wired directly into iced's
  own per-window scale_factor hook rather than per-panel font-size
  plumbing, so it applies immediately to every panel, dialog, and the
  ribbon at once. Persisted alongside the existing lineweight-display-scale
  setting.

OPTIONS/OP command dispatch (app/commands/view.rs, app/commands/mod.rs):
  OPTIONS was registered for autocomplete but had no dispatch arm — the
  Start-tab welcome screen's own button was the only way to reach it.
  Found live-testing the UI-scale setting, unrelated to that feature
  itself. "OP" is AutoCAD's own short alias.

help/: a new self-contained static user-help manual (manual.md as the
authored source, tools/build_help.py splits it into the committed static
HTML pages plus search index) — not yet wired into the application.

64 new/updated tests across the named-parameters and constraint-prompt
changes.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
This commit is contained in:
Felix RiestraandClaude Sonnet 5 committed 2026-09-09 13:50:08 +02:00
1 parent e2d39b049d
commit 7cddf31bcb
47 files changed
+4136 -52

No files matched your search

+14
View File
@@ -36,6 +36,8 @@ mod group_layer;
mod layout;
mod limits;
mod modify;
pub mod named_parameters;
mod named_parameters_persist;
mod mspace;
mod page_setup;
mod paper;
@@ -1934,6 +1936,17 @@ pub struct Scene {
/// touches directly, materialized into `document.objects` only right
/// before a save and read back right after a load.
pub(crate) sketch_constraints: Vec<sketch_constraints::SketchConstraintSet>,
/// Document-wide named-parameter/expression table (`docs/
/// named_parameters_design.md`). One table per document (not per-scope,
/// unlike `sketch_constraints` — a parameter is meant to be shared
/// across the whole drawing), lives on `Scene` for the same reason
/// `sketch_constraints` does: co-located with the `document: CadDocument`
/// it's synced against. Synced with a single document-level
/// `OCS_NAMED_PARAMETERS` XRecord (hung off `document.header.
/// named_objects_dict_handle`, not a per-`BlockRecord` one) around
/// open/save (`named_parameters_persist.rs`, same "lazy" model as
/// `sketch_persist.rs`).
pub(crate) named_parameters: named_parameters::ParameterTable,
/// Tessellated block definitions in block-local coords, keyed by render
/// background and block epoch. Model and Paper adapt black/white colours
/// differently; retaining both variants prevents a full block rebuild on
@@ -2167,6 +2180,7 @@ impl Scene {
dependency_index_cache: RefCell::new(None),
associative_hatch_source_cache: RefCell::new(None),
sketch_constraints: Vec::new(),
named_parameters: named_parameters::ParameterTable::new(),
has_associative_centers: std::cell::Cell::new(None),
block_defn_cache: RefCell::new(HashMap::default()),
entity_index_cache: RefCell::new(None),
+898
View File
@@ -0,0 +1,898 @@
//! Named, formula-capable document parameters — data model.
//!
//! Design: `docs/named_parameters_design.md`. This is stage 1 of that doc's
//! §4 staged plan: the parameter table type plus a small expression
//! parser/evaluator, unit-tested in isolation — no document/UI integration
//! yet (that's stage 2's XRecord persistence, mirroring
//! [`super::sketch_persist`], and stage 3's
//! `SketchConstraint::driving_param` wiring).
//!
//! A [`Parameter`] is a name plus a formula: either a bare literal (`12.5`)
//! or an expression referencing other parameters by name (`2 * hole_dia +
//! 1.5`). A [`ParameterTable`] holds every parameter for one document (per
//! the design doc's §2: one table per document, not per-[`super::
//! sketch_constraints::SketchScope`]) and resolves each one's numeric value,
//! rejecting a circular reference at the moment it would be introduced
//! rather than at evaluation time — the design doc's explicit requirement
//! ("A referencing B referencing A must be rejected, not silently
//! infinite-loop or panic"), and the same real-time-rejection behavior a
//! spreadsheet gives for a circular formula.
//!
//! No existing workspace dependency parses expressions (the design doc's §2
//! flags checking `evalexpr`/`meval` before hand-rolling one). Hand-rolled
//! here instead: cycle detection needs the set of names an expression
//! references *before* it's evaluated, which means owning the parse tree —
//! treating evaluation as a black box, the way a generic expression-eval
//! crate is normally used, would just mean re-deriving that same dependency
//! information by other means. [`Expr::refs`] *is* that dependency
//! information, not extra work spent duplicating what such a crate computes
//! internally anyway.
use super::Scene;
use serde::{Deserialize, Serialize};
use std::collections::HashMap;
use std::fmt;
/// A parsed formula. Ordinary arithmetic plus [`Expr::Ref`] (a reference to
/// another parameter by name — what makes this a *named*-parameter system
/// rather than a plain calculator) and [`Expr::Call`] (a function call).
/// `Call` is in the grammar now so it doesn't need a rewrite if/when more
/// functions are wanted later (design doc §5 open question 2 leaves the
/// exact v1 function set undecided); only a small, conservative builtin set
/// is actually wired up for now (see `call_builtin`).
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub enum Expr {
Number(f64),
Ref(String),
Neg(Box<Expr>),
Add(Box<Expr>, Box<Expr>),
Sub(Box<Expr>, Box<Expr>),
Mul(Box<Expr>, Box<Expr>),
Div(Box<Expr>, Box<Expr>),
Pow(Box<Expr>, Box<Expr>),
Call(String, Vec<Expr>),
}
impl Expr {
/// Every parameter name this expression reads, deduplicated in
/// first-seen order. This is the entire basis for cycle detection
/// (`ParameterTable::set`): one dependency edge per name returned here.
fn refs(&self, out: &mut Vec<String>) {
match self {
Expr::Number(_) => {}
Expr::Ref(name) => {
if !out.contains(name) {
out.push(name.clone());
}
}
Expr::Neg(a) => a.refs(out),
Expr::Add(a, b) | Expr::Sub(a, b) | Expr::Mul(a, b) | Expr::Div(a, b) | Expr::Pow(a, b) => {
a.refs(out);
b.refs(out);
}
Expr::Call(_, args) => {
for arg in args {
arg.refs(out);
}
}
}
}
}
/// Everything that can go wrong parsing, defining, or resolving a
/// parameter. Every case is reported back to the caller (and, in later
/// stages, the UI) rather than panicking — matching this codebase's
/// established pattern of isolating a bad derived value instead of letting
/// it take down an unrelated computation (e.g. dangling-constraint handling
/// in `sketch_solve.rs`).
#[derive(Debug, Clone, PartialEq)]
pub enum ParamError {
/// A formula's text doesn't parse. The `String` is a short, human-
/// readable reason, not a machine-parseable code.
Parse(String),
/// Not a valid identifier, or collides with a builtin function name
/// (`is_reserved_name`).
InvalidName(String),
/// Accepting this formula would close a circular reference right now.
/// The path reads start-to-finish, e.g. `["a", "b", "a"]` for `a = b`,
/// `b = a`.
Cycle(Vec<String>),
UnknownFunction(String),
WrongArgCount { function: String, expected: usize, found: usize },
/// Raised by `resolve`/`resolve_all`, not `set` — a formula may
/// legitimately reference a parameter that doesn't exist *yet* (order of
/// definition shouldn't matter, mirroring a spreadsheet); this only
/// surfaces once something actually tries to read the missing value.
UndefinedReference(String),
DivisionByZero,
}
impl fmt::Display for ParamError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
ParamError::Parse(msg) => write!(f, "formula error: {msg}"),
ParamError::InvalidName(name) => write!(f, "'{name}' is not a valid parameter name"),
ParamError::Cycle(path) => write!(f, "circular reference: {}", path.join(" -> ")),
ParamError::UnknownFunction(name) => write!(f, "unknown function '{name}'"),
ParamError::WrongArgCount { function, expected, found } => {
write!(f, "{function}() expects {expected} argument(s), found {found}")
}
ParamError::UndefinedReference(name) => write!(f, "'{name}' is not defined"),
ParamError::DivisionByZero => write!(f, "division by zero"),
}
}
}
impl std::error::Error for ParamError {}
/// The v1 builtin function set — deliberately small (design doc §5 open
/// question 2 defers the full surface). Also doubles as the reserved-word
/// list `is_reserved_name` checks a parameter name against, so a formula
/// like `sqrt(x)` is never ambiguous between "call the builtin" and
/// "multiply by a parameter named sqrt".
const BUILTIN_FUNCTIONS: &[&str] = &["sqrt", "abs", "pow", "min", "max"];
pub fn is_reserved_name(name: &str) -> bool {
BUILTIN_FUNCTIONS.contains(&name)
}
/// A valid parameter name: starts with a letter or underscore, followed by
/// letters/digits/underscores. (Design doc §5 open question 5.)
pub fn is_valid_name(name: &str) -> bool {
let mut chars = name.chars();
match chars.next() {
Some(c) if c.is_alphabetic() || c == '_' => {}
_ => return false,
}
chars.all(|c| c.is_alphanumeric() || c == '_')
}
/// A dimensional constraint's driving value (design doc §2's own suggested
/// shape for `SketchConstraint::driving_param`, stage 3 of the named-
/// parameters staged plan): either a literal number typed directly, or a
/// reference to a named parameter resolved through a [`ParameterTable`] at
/// solve time. Lives here rather than in `sketch_constraints.rs` since
/// resolving `Named` needs `ParameterTable`, which this module owns.
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub enum DrivingValue {
Literal(f64),
Named(String),
}
impl DrivingValue {
/// Resolves to a number: a literal resolves to itself (never fails); a
/// named reference resolves through `table`, which can fail the same
/// ways any [`ParameterTable::resolve`] call can (undefined reference,
/// division by zero — a cycle should be unreachable here since `set`
/// already refuses to create one).
pub fn resolve(&self, table: &ParameterTable) -> Result<f64, ParamError> {
match self {
DrivingValue::Literal(value) => Ok(*value),
DrivingValue::Named(name) => table.resolve(name),
}
}
}
impl From<f64> for DrivingValue {
fn from(value: f64) -> Self {
DrivingValue::Literal(value)
}
}
fn call_builtin(name: &str, args: &[f64]) -> Result<f64, ParamError> {
fn check_arity(name: &str, args: &[f64], expected: usize) -> Result<(), ParamError> {
if args.len() != expected {
return Err(ParamError::WrongArgCount { function: name.to_string(), expected, found: args.len() });
}
Ok(())
}
match name {
"sqrt" => {
check_arity(name, args, 1)?;
Ok(args[0].sqrt())
}
"abs" => {
check_arity(name, args, 1)?;
Ok(args[0].abs())
}
"pow" => {
check_arity(name, args, 2)?;
Ok(args[0].powf(args[1]))
}
"min" => {
check_arity(name, args, 2)?;
Ok(args[0].min(args[1]))
}
"max" => {
check_arity(name, args, 2)?;
Ok(args[0].max(args[1]))
}
_ => Err(ParamError::UnknownFunction(name.to_string())),
}
}
#[derive(Debug, Clone, PartialEq)]
enum Token {
Number(f64),
Ident(String),
Plus,
Minus,
Star,
Slash,
Caret,
LParen,
RParen,
Comma,
}
fn tokenize(src: &str) -> Result<Vec<Token>, ParamError> {
let chars: Vec<char> = src.chars().collect();
let mut tokens = Vec::new();
let mut i = 0;
while i < chars.len() {
let c = chars[i];
if c.is_whitespace() {
i += 1;
continue;
}
match c {
'+' => {
tokens.push(Token::Plus);
i += 1;
}
'-' => {
tokens.push(Token::Minus);
i += 1;
}
'*' => {
tokens.push(Token::Star);
i += 1;
}
'/' => {
tokens.push(Token::Slash);
i += 1;
}
'^' => {
tokens.push(Token::Caret);
i += 1;
}
'(' => {
tokens.push(Token::LParen);
i += 1;
}
')' => {
tokens.push(Token::RParen);
i += 1;
}
',' => {
tokens.push(Token::Comma);
i += 1;
}
_ if c.is_ascii_digit() || c == '.' => {
let start = i;
let mut seen_dot = c == '.';
i += 1;
while i < chars.len() {
let d = chars[i];
if d.is_ascii_digit() {
i += 1;
} else if d == '.' && !seen_dot {
seen_dot = true;
i += 1;
} else {
break;
}
}
// Scientific notation: 1e-3, 2.5E10. Only consumed when a
// digit (optionally signed) actually follows the e/E, so a
// bare trailing "e" is left for the identifier lexer instead
// (not reachable today since no builtin/constant is named
// "e", but keeps this branch from eating an identifier that
// happens to start with it, e.g. a formula like `2 * ecc`).
if i < chars.len() && (chars[i] == 'e' || chars[i] == 'E') {
let mut j = i + 1;
if j < chars.len() && (chars[j] == '+' || chars[j] == '-') {
j += 1;
}
if j < chars.len() && chars[j].is_ascii_digit() {
while j < chars.len() && chars[j].is_ascii_digit() {
j += 1;
}
i = j;
}
}
let text: String = chars[start..i].iter().collect();
let value: f64 = text.parse().map_err(|_| ParamError::Parse(format!("invalid number '{text}'")))?;
tokens.push(Token::Number(value));
}
_ if c.is_alphabetic() || c == '_' => {
let start = i;
i += 1;
while i < chars.len() && (chars[i].is_alphanumeric() || chars[i] == '_') {
i += 1;
}
tokens.push(Token::Ident(chars[start..i].iter().collect()));
}
other => return Err(ParamError::Parse(format!("unexpected character '{other}'"))),
}
}
Ok(tokens)
}
struct Parser<'a> {
tokens: &'a [Token],
pos: usize,
}
impl<'a> Parser<'a> {
fn peek(&self) -> Option<&Token> {
self.tokens.get(self.pos)
}
fn bump(&mut self) -> Option<Token> {
let t = self.tokens.get(self.pos).cloned();
if t.is_some() {
self.pos += 1;
}
t
}
fn expect(&mut self, expected: Token) -> Result<(), ParamError> {
match self.bump() {
Some(t) if t == expected => Ok(()),
Some(t) => Err(ParamError::Parse(format!("expected {expected:?}, found {t:?}"))),
None => Err(ParamError::Parse(format!("expected {expected:?}, found end of formula"))),
}
}
// Precedence, low to high: expr (+ -) > term (* /) > unary (-) > power (^) > primary.
// `^` binds tighter than unary minus on its left operand but is
// right-associative on its own exponent (`parse_unary` recursing back
// into `parse_power` covers `2^-2` == `2^(-2)`, and `2^3^2` ==
// `2^(3^2)`).
fn parse_expr(&mut self) -> Result<Expr, ParamError> {
let mut lhs = self.parse_term()?;
loop {
match self.peek() {
Some(Token::Plus) => {
self.pos += 1;
lhs = Expr::Add(Box::new(lhs), Box::new(self.parse_term()?));
}
Some(Token::Minus) => {
self.pos += 1;
lhs = Expr::Sub(Box::new(lhs), Box::new(self.parse_term()?));
}
_ => break,
}
}
Ok(lhs)
}
fn parse_term(&mut self) -> Result<Expr, ParamError> {
let mut lhs = self.parse_unary()?;
loop {
match self.peek() {
Some(Token::Star) => {
self.pos += 1;
lhs = Expr::Mul(Box::new(lhs), Box::new(self.parse_unary()?));
}
Some(Token::Slash) => {
self.pos += 1;
lhs = Expr::Div(Box::new(lhs), Box::new(self.parse_unary()?));
}
_ => break,
}
}
Ok(lhs)
}
fn parse_unary(&mut self) -> Result<Expr, ParamError> {
match self.peek() {
Some(Token::Minus) => {
self.pos += 1;
Ok(Expr::Neg(Box::new(self.parse_unary()?)))
}
Some(Token::Plus) => {
self.pos += 1;
self.parse_unary()
}
_ => self.parse_power(),
}
}
fn parse_power(&mut self) -> Result<Expr, ParamError> {
let base = self.parse_primary()?;
if let Some(Token::Caret) = self.peek() {
self.pos += 1;
let exponent = self.parse_unary()?;
return Ok(Expr::Pow(Box::new(base), Box::new(exponent)));
}
Ok(base)
}
fn parse_primary(&mut self) -> Result<Expr, ParamError> {
match self.bump() {
Some(Token::Number(n)) => Ok(Expr::Number(n)),
Some(Token::Ident(name)) => {
if matches!(self.peek(), Some(Token::LParen)) {
self.pos += 1;
let mut args = Vec::new();
if !matches!(self.peek(), Some(Token::RParen)) {
loop {
args.push(self.parse_expr()?);
if matches!(self.peek(), Some(Token::Comma)) {
self.pos += 1;
} else {
break;
}
}
}
self.expect(Token::RParen)?;
Ok(Expr::Call(name, args))
} else {
Ok(Expr::Ref(name))
}
}
Some(Token::LParen) => {
let inner = self.parse_expr()?;
self.expect(Token::RParen)?;
Ok(inner)
}
Some(other) => Err(ParamError::Parse(format!("unexpected token {other:?}"))),
None => Err(ParamError::Parse("unexpected end of formula".to_string())),
}
}
}
/// Parses a formula's text into an [`Expr`], without evaluating or
/// resolving any reference. `ParameterTable::set` is the usual entry point;
/// exposed standalone so a UI stage can validate-as-you-type without
/// touching the table.
pub fn parse(source: &str) -> Result<Expr, ParamError> {
let tokens = tokenize(source)?;
if tokens.is_empty() {
return Err(ParamError::Parse("empty formula".to_string()));
}
let mut parser = Parser { tokens: &tokens, pos: 0 };
let expr = parser.parse_expr()?;
if parser.pos != tokens.len() {
return Err(ParamError::Parse(format!("unexpected trailing token {:?}", tokens[parser.pos])));
}
Ok(expr)
}
/// One named parameter: a name plus its formula, both the original typed
/// text (`source`, for redisplay/re-editing) and the parsed [`Expr`]
/// (`expr`, for evaluation).
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct Parameter {
pub name: String,
pub source: String,
pub expr: Expr,
}
/// Every named parameter for one document (design doc §2: per-document, not
/// per-[`super::sketch_constraints::SketchScope`] — a block/xref does not
/// get its own independent table; open question 4 leaves whether that's the
/// final answer, but it's this stage's working assumption).
///
/// `parameters` is intentionally not `pub` (unlike the sibling
/// `SketchConstraintSet::constraints`): this table's entire value is that
/// every stored formula is already known to be parse-valid and cycle-free,
/// an invariant only `set`/`remove` maintain. A direct external push could
/// silently violate it.
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
pub struct ParameterTable {
parameters: Vec<Parameter>,
}
impl ParameterTable {
pub fn new() -> Self {
Self::default()
}
pub fn is_empty(&self) -> bool {
self.parameters.is_empty()
}
pub fn len(&self) -> usize {
self.parameters.len()
}
/// Insertion order — the order parameters were first defined in, stable
/// across edits to existing ones. The natural listing order for a
/// future parameters panel.
pub fn iter(&self) -> impl Iterator<Item = &Parameter> {
self.parameters.iter()
}
pub fn get(&self, name: &str) -> Option<&Parameter> {
self.parameters.iter().find(|p| p.name == name)
}
pub fn contains(&self, name: &str) -> bool {
self.get(name).is_some()
}
fn index_of(&self, name: &str) -> Option<usize> {
self.parameters.iter().position(|p| p.name == name)
}
/// Defines a new parameter or redefines an existing one's formula (one
/// upsert operation — there is no separate "must not already exist"
/// entry point; a future UI stage that wants add-vs-rename semantics can
/// check `contains` itself first).
///
/// Rejects: an invalid identifier or one colliding with a builtin
/// function name (`is_valid_name`/`is_reserved_name`), a formula that
/// fails to parse, and — the design doc's explicit requirement — a
/// formula that would close a circular reference right now. A reference
/// to a parameter that doesn't exist *yet* is not rejected (order of
/// definition shouldn't matter, mirroring a spreadsheet); it only
/// surfaces as [`ParamError::UndefinedReference`] when [`Self::resolve`]
/// actually needs that value.
pub fn set(&mut self, name: &str, source: &str) -> Result<(), ParamError> {
if !is_valid_name(name) || is_reserved_name(name) {
return Err(ParamError::InvalidName(name.to_string()));
}
let expr = parse(source)?;
self.check_no_cycle(name, &expr)?;
match self.index_of(name) {
Some(i) => {
self.parameters[i].source = source.to_string();
self.parameters[i].expr = expr;
}
None => self.parameters.push(Parameter { name: name.to_string(), source: source.to_string(), expr }),
}
Ok(())
}
/// Removes a parameter by name. Returns whether one was actually
/// removed. Deliberately does not check whether another parameter's
/// formula still references `name` — that becomes an
/// [`ParamError::UndefinedReference`] for the referencing parameter the
/// next time it's resolved, the same graceful-degradation treatment an
/// unresolvable reference gets everywhere else in this module, rather
/// than a special "can't delete, still in use" rule this stage doesn't
/// need to invent (a future UI stage may still want to warn before
/// deleting — that's a UI-layer decision, not a data-layer one).
pub fn remove(&mut self, name: &str) -> bool {
match self.index_of(name) {
Some(i) => {
self.parameters.remove(i);
true
}
None => false,
}
}
/// Would accepting `new_expr` as `name`'s formula close a circular
/// reference right now? Walks the dependency graph from each of
/// `new_expr`'s direct references, following each visited parameter's
/// *existing* stored formula, looking for a path back to `name`. A
/// reference to a name not currently in the table just terminates that
/// branch — no cycle through something that doesn't exist yet.
fn check_no_cycle(&self, name: &str, new_expr: &Expr) -> Result<(), ParamError> {
let mut direct_refs = Vec::new();
new_expr.refs(&mut direct_refs);
for start in &direct_refs {
let mut visiting = Vec::new();
if let Some(mut path) = self.find_path(start, name, &mut visiting) {
let mut full = vec![name.to_string()];
full.append(&mut path);
return Err(ParamError::Cycle(full));
}
}
Ok(())
}
/// Depth-first search from `current` to `target`, following each
/// visited parameter's existing formula's references. Returns the path
/// (starting with `current`) if `target` is reachable.
fn find_path(&self, current: &str, target: &str, visiting: &mut Vec<String>) -> Option<Vec<String>> {
if current == target {
return Some(vec![current.to_string()]);
}
if visiting.iter().any(|v| v == current) {
// A cycle that doesn't involve `target` shouldn't be reachable
// (`set` refuses to create one), but bail out rather than loop
// forever if that invariant is ever violated some other way
// (e.g. a foreign/hand-edited file deserialized straight into a
// table without going through `set`).
return None;
}
let param = self.get(current)?;
visiting.push(current.to_string());
let mut refs = Vec::new();
param.expr.refs(&mut refs);
for r in &refs {
if let Some(mut rest) = self.find_path(r, target, visiting) {
let mut path = vec![current.to_string()];
path.append(&mut rest);
visiting.pop();
return Some(path);
}
}
visiting.pop();
None
}
/// Resolves one parameter's numeric value, evaluating its dependency
/// chain as needed. No state is kept between calls (a fresh memo cache
/// every call) — mirrors `refresh_sketch_constraints`'s
/// rebuild-from-scratch-per-trigger model (design doc §2): this stage
/// has no document/UI integration yet to decide when a cached value
/// would go stale, so it simply never caches across calls.
pub fn resolve(&self, name: &str) -> Result<f64, ParamError> {
let mut cache = HashMap::new();
let mut stack = Vec::new();
self.resolve_inner(name, &mut cache, &mut stack)
}
/// Resolves every parameter, isolating a failure to just the parameters
/// it actually affects rather than aborting the whole table — the same
/// don't-let-one-bad-item-take-down-everything-else treatment this
/// codebase already gives dangling constraints and similar derived-data
/// failures elsewhere.
pub fn resolve_all(&self) -> HashMap<String, Result<f64, ParamError>> {
let mut out = HashMap::with_capacity(self.parameters.len());
for param in &self.parameters {
out.insert(param.name.clone(), self.resolve(&param.name));
}
out
}
fn resolve_inner(&self, name: &str, cache: &mut HashMap<String, f64>, stack: &mut Vec<String>) -> Result<f64, ParamError> {
if let Some(v) = cache.get(name) {
return Ok(*v);
}
if stack.iter().any(|s| s == name) {
// Defense in depth only — `set` already refuses to create a
// cycle; see `find_path`'s doc comment for how one could still
// end up here.
let mut cyc = stack.clone();
cyc.push(name.to_string());
return Err(ParamError::Cycle(cyc));
}
let param = self.get(name).ok_or_else(|| ParamError::UndefinedReference(name.to_string()))?;
stack.push(name.to_string());
let value = self.eval(&param.expr, cache, stack)?;
stack.pop();
cache.insert(name.to_string(), value);
Ok(value)
}
fn eval(&self, expr: &Expr, cache: &mut HashMap<String, f64>, stack: &mut Vec<String>) -> Result<f64, ParamError> {
Ok(match expr {
Expr::Number(n) => *n,
Expr::Ref(name) => self.resolve_inner(name, cache, stack)?,
Expr::Neg(a) => -self.eval(a, cache, stack)?,
Expr::Add(a, b) => self.eval(a, cache, stack)? + self.eval(b, cache, stack)?,
Expr::Sub(a, b) => self.eval(a, cache, stack)? - self.eval(b, cache, stack)?,
Expr::Mul(a, b) => self.eval(a, cache, stack)? * self.eval(b, cache, stack)?,
Expr::Div(a, b) => {
let denom = self.eval(b, cache, stack)?;
if denom == 0.0 {
return Err(ParamError::DivisionByZero);
}
self.eval(a, cache, stack)? / denom
}
Expr::Pow(a, b) => self.eval(a, cache, stack)?.powf(self.eval(b, cache, stack)?),
Expr::Call(fname, args) => {
let mut values = Vec::with_capacity(args.len());
for arg in args {
values.push(self.eval(arg, cache, stack)?);
}
call_builtin(fname, &values)?
}
})
}
}
impl Scene {
/// The document's named-parameter table (design doc §2: one per
/// document). Read-only accessor; use [`Scene::named_parameters_mut`] to
/// define/redefine/remove a parameter.
pub fn named_parameters(&self) -> &ParameterTable {
&self.named_parameters
}
pub fn named_parameters_mut(&mut self) -> &mut ParameterTable {
&mut self.named_parameters
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn parses_arithmetic_with_ordinary_precedence() {
// 2 + 3 * 4 == 14, not 20 -- proves * binds tighter than +.
assert_eq!(eval_literal("2 + 3 * 4"), 14.0);
// (2 + 3) * 4 == 20 -- parens override precedence.
assert_eq!(eval_literal("(2 + 3) * 4"), 20.0);
// 2 ^ 3 ^ 2 == 2 ^ (3 ^ 2) == 512 -- right-associative power.
assert_eq!(eval_literal("2 ^ 3 ^ 2"), 512.0);
// Unary minus binds tighter than power's left operand is *read*,
// but -2^2 must still equal -(2^2) = -4 under this grammar (unary
// parses before power), matching common calculator convention.
assert_eq!(eval_literal("-2^2"), -4.0);
assert_eq!(eval_literal("2^-2"), 0.25);
assert_eq!(eval_literal("10 / 4"), 2.5);
assert_eq!(eval_literal("-(3 + 4)"), -7.0);
}
fn eval_literal(src: &str) -> f64 {
let table = ParameterTable::new();
let expr = parse(src).unwrap_or_else(|e| panic!("parse '{src}': {e}"));
table.eval(&expr, &mut HashMap::new(), &mut Vec::new()).unwrap_or_else(|e| panic!("eval '{src}': {e}"))
}
#[test]
fn parses_scientific_notation() {
assert_eq!(eval_literal("1e3"), 1000.0);
assert_eq!(eval_literal("1.5e-2"), 0.015);
}
#[test]
fn rejects_malformed_formulas() {
assert!(matches!(parse(""), Err(ParamError::Parse(_))));
assert!(matches!(parse("1 +"), Err(ParamError::Parse(_))));
assert!(matches!(parse("(1 + 2"), Err(ParamError::Parse(_))));
assert!(matches!(parse("1 2"), Err(ParamError::Parse(_))));
assert!(matches!(parse("1 @ 2"), Err(ParamError::Parse(_))));
}
#[test]
fn builtin_functions_evaluate_and_check_arity() {
assert_eq!(eval_literal("sqrt(16)"), 4.0);
assert_eq!(eval_literal("abs(-5)"), 5.0);
assert_eq!(eval_literal("min(3, 7)"), 3.0);
assert_eq!(eval_literal("max(3, 7)"), 7.0);
assert_eq!(eval_literal("pow(2, 10)"), 1024.0);
let table = ParameterTable::new();
let expr = parse("sqrt(1, 2)").unwrap();
let err = table.eval(&expr, &mut HashMap::new(), &mut Vec::new()).unwrap_err();
assert_eq!(err, ParamError::WrongArgCount { function: "sqrt".to_string(), expected: 1, found: 2 });
let expr = parse("bogus(1)").unwrap();
let err = table.eval(&expr, &mut HashMap::new(), &mut Vec::new()).unwrap_err();
assert_eq!(err, ParamError::UnknownFunction("bogus".to_string()));
}
#[test]
fn division_by_zero_is_reported_not_panicked() {
let table = ParameterTable::new();
let expr = parse("1 / 0").unwrap();
assert_eq!(table.eval(&expr, &mut HashMap::new(), &mut Vec::new()), Err(ParamError::DivisionByZero));
}
#[test]
fn set_rejects_invalid_and_reserved_names() {
let mut table = ParameterTable::new();
assert!(matches!(table.set("2bad", "1"), Err(ParamError::InvalidName(_))));
assert!(matches!(table.set("has space", "1"), Err(ParamError::InvalidName(_))));
assert!(matches!(table.set("", "1"), Err(ParamError::InvalidName(_))));
assert!(matches!(table.set("sqrt", "1"), Err(ParamError::InvalidName(_))));
assert!(table.set("_valid1", "1").is_ok());
}
#[test]
fn a_literal_parameter_resolves_to_its_number() {
let mut table = ParameterTable::new();
table.set("plate_len", "42.5").unwrap();
assert_eq!(table.resolve("plate_len"), Ok(42.5));
}
#[test]
fn a_formula_resolves_through_a_reference_chain() {
let mut table = ParameterTable::new();
table.set("hole_dia", "5").unwrap();
table.set("hole_spacing", "2 * hole_dia + 1.5").unwrap();
table.set("row_span", "3 * hole_spacing").unwrap();
assert_eq!(table.resolve("hole_spacing"), Ok(11.5));
assert_eq!(table.resolve("row_span"), Ok(34.5));
}
#[test]
fn resolving_an_undefined_reference_reports_which_name_is_missing() {
let mut table = ParameterTable::new();
table.set("a", "b + 1").unwrap();
assert_eq!(table.resolve("a"), Err(ParamError::UndefinedReference("b".to_string())));
}
#[test]
fn set_rejects_a_direct_self_reference() {
let mut table = ParameterTable::new();
let err = table.set("a", "a + 1").unwrap_err();
assert!(matches!(err, ParamError::Cycle(_)));
assert!(!table.contains("a"), "a rejected formula must not be stored");
}
#[test]
fn set_rejects_an_indirect_cycle_at_the_definition_that_closes_it() {
let mut table = ParameterTable::new();
table.set("a", "b").unwrap();
// b doesn't exist yet -- not a cycle, a is just unresolved for now.
assert_eq!(table.resolve("a"), Err(ParamError::UndefinedReference("b".to_string())));
// Defining b = a closes the loop right here and must be rejected,
// leaving b undefined rather than silently accepted.
let err = table.set("b", "a").unwrap_err();
assert!(matches!(err, ParamError::Cycle(_)));
assert!(!table.contains("b"));
// a's formula (still "b") is untouched -- it stays unresolved, not
// corrupted by the rejected attempt.
assert_eq!(table.resolve("a"), Err(ParamError::UndefinedReference("b".to_string())));
}
#[test]
fn set_allows_redefining_an_existing_parameter_to_break_a_would_be_cycle() {
let mut table = ParameterTable::new();
table.set("a", "1").unwrap();
table.set("b", "a + 1").unwrap();
// Redefining a in terms of b must be rejected (a <- b <- a)...
assert!(table.set("a", "b + 1").is_err());
// ...but redefining a to something unrelated is always fine.
table.set("a", "5").unwrap();
assert_eq!(table.resolve("a"), Ok(5.0));
assert_eq!(table.resolve("b"), Ok(6.0));
}
#[test]
fn resolve_all_isolates_a_failure_to_the_affected_parameters_only() {
let mut table = ParameterTable::new();
table.set("good", "10").unwrap();
table.set("bad", "missing + 1").unwrap();
table.set("depends_on_bad", "bad * 2").unwrap();
let results = table.resolve_all();
assert_eq!(results.get("good"), Some(&Ok(10.0)));
assert_eq!(results.get("bad"), Some(&Err(ParamError::UndefinedReference("missing".to_string()))));
assert_eq!(results.get("depends_on_bad"), Some(&Err(ParamError::UndefinedReference("missing".to_string()))));
}
#[test]
fn remove_drops_a_parameter_and_leaves_dependents_undefined_not_panicking() {
let mut table = ParameterTable::new();
table.set("a", "1").unwrap();
table.set("b", "a + 1").unwrap();
assert!(table.remove("a"));
assert!(!table.remove("a"), "removing an already-removed name reports false");
assert_eq!(table.resolve("b"), Err(ParamError::UndefinedReference("a".to_string())));
}
#[test]
fn set_upserts_an_existing_parameters_formula() {
let mut table = ParameterTable::new();
table.set("a", "1").unwrap();
table.set("a", "2").unwrap();
assert_eq!(table.len(), 1, "redefining must not create a second entry");
assert_eq!(table.resolve("a"), Ok(2.0));
}
#[test]
fn table_round_trips_through_bincode() {
let mut table = ParameterTable::new();
table.set("hole_dia", "5").unwrap();
table.set("hole_spacing", "2 * hole_dia + 1.5").unwrap();
let bytes = bincode::serialize(&table).expect("serialize");
let restored: ParameterTable = bincode::deserialize(&bytes).expect("deserialize");
assert_eq!(restored.len(), 2);
assert_eq!(restored.resolve("hole_spacing"), Ok(11.5));
assert_eq!(restored.get("hole_dia").unwrap().source, "5");
}
#[test]
fn iter_preserves_insertion_order_across_redefinitions() {
let mut table = ParameterTable::new();
table.set("z", "1").unwrap();
table.set("a", "2").unwrap();
table.set("z", "3").unwrap(); // redefine, should not move to the end
let names: Vec<&str> = table.iter().map(|p| p.name.as_str()).collect();
assert_eq!(names, vec!["z", "a"]);
}
}
+201
View File
@@ -0,0 +1,201 @@
//! Save/load integration for [`super::named_parameters::ParameterTable`] —
//! design doc §4 stage 2 (the XRecord persistence spike) plus enough of
//! stage 2's "wire it into open/save" to actually be useful, following
//! [`super::sketch_persist`]'s exact precedent (itself the sketch-constraint
//! project's stage 4/§5.2): the "lazy" model, where the table lives only in
//! `Scene::named_parameters` in-memory during editing and is materialized
//! into `document.objects` as one XRecord right before a save, then read
//! back right after a load.
//!
//! The one structural difference from `sketch_persist`: a
//! [`super::named_parameters::ParameterTable`] is document-wide, not
//! per-[`super::sketch_constraints::SketchScope`], so there is exactly one
//! XRecord for the whole document rather than one per scope owner.
//!
//! **Owner handle, and a real round-trip gap found while building this**:
//! the obvious choice was `document.header.named_objects_dict_handle` (the
//! document's own root Named Objects Dictionary) — `extension_dictionary_
//! handle` in the vendored `cadcodec` source explicitly resolves an
//! extension dictionary for a `Dictionary`-type owner (`ObjectType::
//! Dictionary(value) => value.xdictionary_handle`), and that crate's own
//! tests (e.g. `tests/issue51.rs`) use `named_objects_dict_handle` as an
//! XRecord owner directly. In practice this round-tripped through DWG but
//! **silently lost the XRecord through DXF** (caught by this module's own
//! save/load tests, not assumed): `CadDocument::ensure_xrecord` only writes
//! the owner's *own* `xdictionary_handle` field when `get_entity_mut(owner)`
//! succeeds (a real "entity"); a `Dictionary` owner like the root NOD isn't
//! one, so `ensure_xrecord` falls back to a runtime-only side map
//! (`xdic_by_handle`) instead. The DWG writer consults that side map
//! (`io/dwg/dwg_document_builder.rs` et al.); the DXF writer never does (no
//! `xdic_by_handle` reference anywhere under `io/dxf/writer/`) — so a
//! Dictionary-owned extension dictionary is invisible to a DXF save. This
//! is a gap in the vendored crate for this specific owner-type combination,
//! not something introduced here; per this project's own stated preference
//! to avoid needing changes to `acadrust`/`cadkernel`, the fix is to pick a
//! different, already-proven owner rather than patch it.
//!
//! **Owner used instead**: `document.header.model_space_block_handle` — a
//! real `BlockRecord` entity, the exact anchor `sketch_persist`'s own
//! `SketchScope::ModelSpace` case already proved round-trips cleanly
//! through both DWG and DXF. Model space is not a semantically perfect fit
//! for "the whole document" (a parameter table has nothing to do with model
//! space specifically), but it is guaranteed to exist in every
//! `CadDocument` and is a real entity, which is the property that actually
//! matters here.
use super::named_parameters::ParameterTable;
use super::Scene;
const XRECORD_KEY: &str = "OCS_NAMED_PARAMETERS";
/// Prefixed onto the serialized blob so a future schema change can be
/// detected and gracefully skipped rather than silently misreading bytes —
/// same call-out as `sketch_persist::FORMAT_VERSION`, same caveat that the
/// migration path itself is still undesigned.
const FORMAT_VERSION: u8 = 1;
fn encode(table: &ParameterTable) -> Option<Vec<u8>> {
let mut bytes = vec![FORMAT_VERSION];
bytes.extend(bincode::serialize(table).ok()?);
Some(bytes)
}
fn decode(bytes: &[u8]) -> Option<ParameterTable> {
let (&version, body) = bytes.split_first()?;
if version != FORMAT_VERSION {
return None;
}
bincode::deserialize(body).ok()
}
impl Scene {
/// Writes `named_parameters` into the document's `OCS_NAMED_PARAMETERS`
/// XRecord, ready for whatever save call happens next to serialize
/// `self.document` as-is. Called from the same save entry points as
/// `materialize_sketch_constraints_for_save`
/// (`OpenCADStudio::prepare_native_save`, the wasm save path).
///
/// An empty table is skipped rather than writing an empty XRecord — an
/// unconstrained-by-parameters drawing (the common case, at least until
/// this feature is used) should not gain persisted-but-empty
/// bookkeeping on every save.
pub(crate) fn materialize_named_parameters_for_save(&mut self) {
if self.named_parameters.is_empty() {
return;
}
let Some(bytes) = encode(&self.named_parameters) else { return };
let owner = self.document.header.model_space_block_handle;
if owner.is_null() {
return;
}
self.document.ensure_xrecord(owner, XRECORD_KEY);
if let Some(record) = self.document.xrecord_mut(owner, XRECORD_KEY) {
// Overwrite, not append: a resave must replace the prior blob,
// not accumulate one more Chunk entry every time.
record.entries.clear();
record
.entries
.push(acadrust::objects::XRecordEntry::new(310, acadrust::objects::XRecordValue::Chunk(bytes)));
}
}
/// Populates `named_parameters` from the just-loaded `self.document`'s
/// `OCS_NAMED_PARAMETERS` XRecord, if any — called right after a
/// document open installs its `CadDocument` into this `Scene`, the same
/// call sites `load_sketch_constraints_from_document` already has
/// (`OpenCADStudio::on_file_opened`, the automation `"open"`/`"new"`
/// ops). Replaces whatever was already in `named_parameters` (a fresh
/// table for a real open; `"new"`'s blank document has nothing to find
/// anyway).
pub(crate) fn load_named_parameters_from_document(&mut self) {
self.named_parameters = ParameterTable::new();
let owner = self.document.header.model_space_block_handle;
if owner.is_null() {
return;
}
let Some(record) = self.document.xrecord(owner, XRECORD_KEY) else { return };
let Some(bytes) = record.entries.iter().find_map(|entry| match &entry.value {
acadrust::objects::XRecordValue::Chunk(bytes) => Some(bytes.clone()),
_ => None,
}) else {
return;
};
if let Some(table) = decode(&bytes) {
self.named_parameters = table;
}
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn a_parameter_table_survives_materialize_and_reload() {
let mut scene = Scene::new();
scene.named_parameters_mut().set("hole_dia", "5").unwrap();
scene.named_parameters_mut().set("hole_spacing", "2 * hole_dia + 1.5").unwrap();
scene.materialize_named_parameters_for_save();
// Simulate the load-time side of a save/reload round trip: drop the
// in-memory table (a fresh open starts empty) and repopulate purely
// from what materialize just wrote into `document.objects`.
scene.named_parameters = ParameterTable::new();
scene.load_named_parameters_from_document();
assert_eq!(scene.named_parameters().len(), 2);
assert_eq!(scene.named_parameters().resolve("hole_spacing"), Ok(11.5));
assert_eq!(scene.named_parameters().get("hole_dia").unwrap().source, "5");
}
/// The above test only proves `materialize`/`load` agree with each
/// other on the same in-memory `CadDocument` — this drives a real
/// `save_to_bytes`/`load_bytes` round trip (the same primitives
/// `OpenCADStudio::on_file_opened`'s native save/open path uses),
/// through both supported formats, mirroring
/// `sketch_persist`'s own real-bytes tests.
fn full_bytes_roundtrip(ext: &str) -> ParameterTable {
let mut scene = Scene::new();
scene.named_parameters_mut().set("plate_len", "42.5").unwrap();
scene.named_parameters_mut().set("plate_width", "plate_len / 2").unwrap();
scene.materialize_named_parameters_for_save();
let bytes = crate::io::save_to_bytes(&scene.document, ext, scene.document.version)
.unwrap_or_else(|e| panic!("save to {ext} bytes: {e}"));
let mut reloaded_scene = Scene::new();
reloaded_scene.document = crate::io::load_bytes(&format!("named_params_roundtrip.{ext}"), bytes)
.unwrap_or_else(|e| panic!("reload {ext} bytes: {e}"));
reloaded_scene.load_named_parameters_from_document();
reloaded_scene.named_parameters
}
#[test]
fn a_parameter_table_survives_a_real_dxf_save_and_load_round_trip() {
let restored = full_bytes_roundtrip("dxf");
assert_eq!(restored.len(), 2);
assert_eq!(restored.resolve("plate_width"), Ok(21.25));
}
#[test]
fn a_parameter_table_survives_a_real_dwg_save_and_load_round_trip() {
let restored = full_bytes_roundtrip("dwg");
assert_eq!(restored.len(), 2);
assert_eq!(restored.resolve("plate_width"), Ok(21.25));
}
#[test]
fn an_empty_table_is_not_materialized() {
let mut scene = Scene::new();
scene.materialize_named_parameters_for_save();
let owner = scene.document.header.model_space_block_handle;
assert!(scene.document.xrecord(owner, XRECORD_KEY).is_none(), "an empty table should not create an XRecord");
}
#[test]
fn decode_rejects_a_mismatched_format_version() {
let mut bytes = vec![FORMAT_VERSION.wrapping_add(1)];
bytes.extend(bincode::serialize(&ParameterTable::new()).unwrap());
assert!(decode(&bytes).is_none(), "a future/unknown format version must be rejected, not misread");
}
}
+27 -15
View File
@@ -11,6 +11,7 @@
//! entities and is meant to be re-solved every time referenced geometry
//! changes, not applied once.
use super::named_parameters::DrivingValue;
use acadrust::types::{Handle, Vector3};
use serde::{Deserialize, Serialize};
@@ -90,11 +91,14 @@ pub struct SketchConstraint {
pub id: ConstraintId,
pub kind: ConstraintKind,
pub refs: Vec<SketchRef>,
/// The typed target for a dimensional constraint (a `Distance`'s
/// length, an `Angle`'s degrees, a `Radius`'s radius). `None` for every
/// purely-geometric kind (Coincident, Horizontal, Vertical, Parallel,
/// Perpendicular, Equal, Tangent).
pub driving_param: Option<f64>,
/// The target for a dimensional constraint (a `Distance`'s length, an
/// `Angle`'s degrees, a `Radius`'s radius) — a literal number or a
/// named-parameter reference (`docs/named_parameters_design.md`,
/// resolved through `Scene::named_parameters` at solve time by
/// `sketch_solve::build_constraint`). `None` for every purely-geometric
/// kind (Coincident, Horizontal, Vertical, Parallel, Perpendicular,
/// Equal, Tangent).
pub driving_param: Option<DrivingValue>,
/// Lets a user suppress a constraint without losing it — a re-solve
/// skips a disabled constraint entirely.
pub enabled: bool,
@@ -160,7 +164,7 @@ impl SketchConstraintSet {
}
/// Appends a constraint, assigning it a fresh id unique within this set.
pub fn add(&mut self, kind: ConstraintKind, refs: Vec<SketchRef>, driving_param: Option<f64>) -> ConstraintId {
pub fn add(&mut self, kind: ConstraintKind, refs: Vec<SketchRef>, driving_param: Option<DrivingValue>) -> ConstraintId {
let id = self.next_id;
self.next_id += 1;
self.constraints.push(SketchConstraint { id, kind, refs, driving_param, enabled: true });
@@ -348,9 +352,17 @@ impl ConstraintKind {
/// The full glyph text for one constraint: its symbol, plus the driving
/// value for a dimensional kind (Distance/Angle/Radius).
pub(crate) fn glyph_label(constraint: &SketchConstraint) -> String {
match (constraint.kind, constraint.driving_param) {
(ConstraintKind::Angle, Some(value)) => format!("{} {value:.1}°", constraint.kind.glyph_symbol()),
(_, Some(value)) => format!("{} {value:.2}", constraint.kind.glyph_symbol()),
match (constraint.kind, &constraint.driving_param) {
(ConstraintKind::Angle, Some(DrivingValue::Literal(value))) => format!("{} {value:.1}°", constraint.kind.glyph_symbol()),
(_, Some(DrivingValue::Literal(value))) => format!("{} {value:.2}", constraint.kind.glyph_symbol()),
// A named reference has no single resolved number to show without
// threading `ParameterTable` into every glyph-render call site
// (`src/ui/overlay.rs`) — showing the name itself is enough for now;
// stage 4's parameters panel is the natural place to reconsider this
// once a named `driving_param` can actually be authored through the
// UI (nothing can yet — this arm exists so the match is exhaustive
// and correct ahead of that UI, not because it's reachable today).
(_, Some(DrivingValue::Named(name))) => format!("{} {name}", constraint.kind.glyph_symbol()),
(_, None) => constraint.kind.glyph_symbol().to_string(),
}
}
@@ -415,7 +427,7 @@ impl super::Scene {
if handle_map.is_empty() {
return;
}
let mut to_add: Vec<(usize, ConstraintKind, Vec<SketchRef>, Option<f64>)> = Vec::new();
let mut to_add: Vec<(usize, ConstraintKind, Vec<SketchRef>, Option<DrivingValue>)> = Vec::new();
for (scope_index, set) in self.sketch_constraints.iter().enumerate() {
for c in &set.constraints {
if !c.enabled || !c.refs.iter().all(|r| handle_map.contains_key(&r.entity)) {
@@ -423,7 +435,7 @@ impl super::Scene {
}
let new_refs: Vec<SketchRef> =
c.refs.iter().map(|r| SketchRef { entity: handle_map[&r.entity], marker: r.marker }).collect();
to_add.push((scope_index, c.kind, new_refs, c.driving_param));
to_add.push((scope_index, c.kind, new_refs, c.driving_param.clone()));
}
}
if to_add.is_empty() {
@@ -453,10 +465,10 @@ mod tests {
fn add_assigns_increasing_ids_and_get_finds_them() {
let mut set = SketchConstraintSet::new(SketchScope::ModelSpace);
let a = set.add(ConstraintKind::Horizontal, vec![SketchRef::whole(h(1))], None);
let b = set.add(ConstraintKind::Distance, vec![SketchRef::whole(h(1))], Some(25.0));
let b = set.add(ConstraintKind::Distance, vec![SketchRef::whole(h(1))], Some(DrivingValue::Literal(25.0)));
assert_ne!(a, b);
assert_eq!(set.get(a).unwrap().kind, ConstraintKind::Horizontal);
assert_eq!(set.get(b).unwrap().driving_param, Some(25.0));
assert_eq!(set.get(b).unwrap().driving_param, Some(DrivingValue::Literal(25.0)));
}
#[test]
@@ -521,14 +533,14 @@ mod tests {
fn sketch_constraint_set_round_trips_through_bincode() {
let mut set = SketchConstraintSet::new(SketchScope::Block(h(5)));
set.add(ConstraintKind::Coincident, vec![SketchRef::point(h(1), 0), SketchRef::point(h(2), 1)], None);
set.add(ConstraintKind::Distance, vec![SketchRef::whole(h(3))], Some(12.5));
set.add(ConstraintKind::Distance, vec![SketchRef::whole(h(3))], Some(DrivingValue::Literal(12.5)));
let bytes = bincode::serialize(&set).expect("serialize");
let restored: SketchConstraintSet = bincode::deserialize(&bytes).expect("deserialize");
assert_eq!(restored.scope, set.scope);
assert_eq!(restored.constraints.len(), set.constraints.len());
assert_eq!(restored.constraints[1].driving_param, Some(12.5));
assert_eq!(restored.constraints[1].driving_param, Some(DrivingValue::Literal(12.5)));
assert_eq!(restored.constraints[0].refs, set.constraints[0].refs);
}
}
+16 -4
View File
@@ -48,7 +48,15 @@ fn remove_xrecord(document: &mut CadDocument, owner: Handle, key: &str) {
/// or panicking — design doc §5.2's explicit call-out, and §7's flagged
/// "no migration path designed yet": bumping this is the signal a real
/// migration is needed, not a substitute for one.
const FORMAT_VERSION: u8 = 1;
///
/// Bumped 1 -> 2 by `named_parameters_design.md` stage 3:
/// `SketchConstraint::driving_param` changed from a bare `Option<f64>` to
/// `Option<named_parameters::DrivingValue>` (an enum, with its own
/// discriminant), so a version-1 blob's bytes no longer line up with the
/// current struct shape — exactly the "detected and gracefully skipped"
/// case this version byte exists for, not something to migrate (no
/// migration path exists yet, per the call-out above).
const FORMAT_VERSION: u8 = 2;
fn encode(set: &SketchConstraintSet) -> Option<Vec<u8>> {
let mut bytes = vec![FORMAT_VERSION];
@@ -192,7 +200,11 @@ mod tests {
vec![SketchRef::point(a, 1), SketchRef::point(b, 0)],
None,
);
scene.sketch_constraint_set_mut(SketchScope::ModelSpace).add(ConstraintKind::Distance, vec![SketchRef::whole(b)], Some(12.5));
scene.sketch_constraint_set_mut(SketchScope::ModelSpace).add(
ConstraintKind::Distance,
vec![SketchRef::whole(b)],
Some(crate::scene::named_parameters::DrivingValue::Literal(12.5)),
);
scene.materialize_sketch_constraints_for_save();
let bytes = crate::io::save_to_bytes(&scene.document, ext, scene.document.version)
@@ -213,7 +225,7 @@ mod tests {
assert_eq!(restored.constraints.len(), 2);
assert_eq!(restored.constraints[0].kind, ConstraintKind::Coincident);
assert_eq!(restored.constraints[1].kind, ConstraintKind::Distance);
assert_eq!(restored.constraints[1].driving_param, Some(12.5));
assert_eq!(restored.constraints[1].driving_param, Some(crate::scene::named_parameters::DrivingValue::Literal(12.5)));
}
#[test]
@@ -222,7 +234,7 @@ mod tests {
assert_eq!(restored.constraints.len(), 2);
assert_eq!(restored.constraints[0].kind, ConstraintKind::Coincident);
assert_eq!(restored.constraints[1].kind, ConstraintKind::Distance);
assert_eq!(restored.constraints[1].driving_param, Some(12.5));
assert_eq!(restored.constraints[1].driving_param, Some(crate::scene::named_parameters::DrivingValue::Literal(12.5)));
}
#[test]
+21 -10
View File
@@ -38,6 +38,7 @@ use ocs_gcs::geo::{Circle as GCircle, Line as GLine, Point as GPoint};
use ocs_gcs::solvers::dogleg::solve_dl;
use ocs_gcs::system::System;
use super::named_parameters::ParameterTable;
use super::sketch_constraints::{ConstraintId, ConstraintKind, SketchConstraint, SketchConstraintSet, SketchRef};
use super::{ChangeKind, Scene};
@@ -113,14 +114,21 @@ fn resolve_ref(
/// One [`SketchConstraint`]'s `ocs_gcs` construction, or `None` if it can't
/// be built (an unsupported/not-yet-mapped kind, a dangling ref, a ref
/// shape the kind doesn't expect — e.g. `Parallel` needs two whole-line
/// refs). A constraint that can't be built is simply skipped for this
/// solve, not an error: geometry it would have constrained is left alone,
/// matching how a dangling associative-dimension reference degrades today
/// rather than aborting the whole recompute.
/// refs — or, for a dimensional kind, a `driving_param` that fails to
/// resolve: an undefined named-parameter reference or a division by zero in
/// its formula, design doc `named_parameters_design.md` stage 3). A
/// constraint that can't be built is simply skipped for this solve, not an
/// error: geometry it would have constrained is left alone, matching how a
/// dangling associative-dimension reference degrades today rather than
/// aborting the whole recompute. (A named-parameter *cycle* should be
/// unreachable here — `ParameterTable::set` already refuses to create one —
/// so it gets the same treatment as any other resolve failure rather than a
/// special case.)
fn build_constraint(
document: &acadrust::CadDocument,
sys: &mut System,
cache: &mut HashMap<Handle, EntityGeom>,
params: &ParameterTable,
c: &SketchConstraint,
) -> Option<Rc<dyn Constraint>> {
if !c.enabled {
@@ -181,18 +189,18 @@ fn build_constraint(
ConstraintKind::Distance => {
let [a, b] = c.refs.as_slice() else { return None };
let (pa, pb) = (point_ref(sys, cache, *a)?, point_ref(sys, cache, *b)?);
let target = sys.add_param(c.driving_param?, true);
let target = sys.add_param(c.driving_param.as_ref()?.resolve(params).ok()?, true);
Some(Rc::new(P2PDistance::new(pa, pb, target)))
}
ConstraintKind::Angle => {
let [a, b] = c.refs.as_slice() else { return None };
let (fixed, moving) = (whole_line(sys, cache, *a)?, whole_line(sys, cache, *b)?);
let angle = sys.add_param(c.driving_param?.to_radians(), true);
let angle = sys.add_param(c.driving_param.as_ref()?.resolve(params).ok()?.to_radians(), true);
Some(Rc::new(L2LAngle::new(fixed, moving, angle)))
}
ConstraintKind::Radius => {
let circle = whole_circle(sys, cache, *c.refs.first()?)?;
let target = sys.add_param(c.driving_param?, true);
let target = sys.add_param(c.driving_param.as_ref()?.resolve(params).ok()?, true);
Some(Rc::new(Equal::new(circle.rad, target, 1.0)))
}
ConstraintKind::Tangent => {
@@ -252,6 +260,7 @@ const MOVE_EPS: f64 = 1e-9;
/// on a successful solve.
fn solve_scope(
document: &acadrust::CadDocument,
params: &ParameterTable,
set: &SketchConstraintSet,
) -> Option<(Vec<(Handle, EntityType)>, usize, Vec<(ConstraintId, ocs_gcs::diagnosis::RedundancyKind)>)> {
let mut sys = System::new();
@@ -266,7 +275,7 @@ fn solve_scope(
let mut owner: Vec<(Rc<dyn Constraint>, ConstraintId)> = Vec::new();
for c in &set.constraints {
let Some(constraint) = build_constraint(document, &mut sys, &mut cache, c) else { continue };
let Some(constraint) = build_constraint(document, &mut sys, &mut cache, params, c) else { continue };
owner.push((constraint.clone(), c.id));
sys.add_constraint(constraint);
// Coincident needs both X and Y equal; `build_constraint` returns
@@ -412,7 +421,9 @@ impl Scene {
if !touched {
continue;
}
let Some((solved, dof, conflicts)) = solve_scope(&self.document, &self.sketch_constraints[i]) else { continue };
let Some((solved, dof, conflicts)) = solve_scope(&self.document, &self.named_parameters, &self.sketch_constraints[i]) else {
continue;
};
self.sketch_constraints[i].dof = Some(dof);
self.sketch_constraints[i].conflicts = conflicts;
for (handle, new_entity) in solved {
@@ -470,7 +481,7 @@ impl Scene {
if !is_touched {
continue;
}
let Some((solved, _dof, _conflicts)) = solve_scope(&self.document, set) else { continue };
let Some((solved, _dof, _conflicts)) = solve_scope(&self.document, &self.named_parameters, set) else { continue };
result.extend(solved);
}
result