Agent-built designs: stop redoing whole-design work on every command (#1148)
A design built over MCP is many small commands, and each one did work over the whole design, so building or showing a 1,300-body design took minutes: - moved components were transformed and meshed again after every change; world_state now reuses each placed body whose definition and placement are unchanged - next_d_name searched the design once per candidate number - Document::eval evaluated every parameter for every expression; now only when the expression names one - param_rows' used_by was quadratic; the desktop no longer gathers completions on every change while no text box has focus solidcraft-cli bench-agent builds a generated ~1,300-body bicycle command by command and times it (241 s -> 16 s here). Eval output is unchanged apart from timings. Co-authored-by: Claude Opus 5.5 <[email protected]>
This commit is contained in:
1 parent
37547f8eba
commit
6de8f6db95
11 files changed
+439
-21
No files matched your search
@@ -162,12 +162,12 @@ impl Document {
|
||||
pub fn face_colors(&self, b: &crate::ModelBody) -> Vec<(usize, Look)> {
|
||||
// Face colours the body was imported with, under the design's own.
|
||||
let to8 = |x: f32| (x.clamp(0.0, 1.0) * 255.0).round() as u8;
|
||||
let names = crate::naming::face_names(b);
|
||||
let mut out: Vec<(usize, Look)> = match b.body.paint() {
|
||||
Some(p) => p.faces.iter().map(|f| (f.face, Look::custom(f.color.map(to8), f.opacity as f64))).collect(),
|
||||
// Carried from an earlier version of the body: by face name.
|
||||
// Carried from an earlier version of the body: by face name (worked out only here:
|
||||
// the viewport asks every body for its face colours on every change).
|
||||
None => match &b.imported {
|
||||
Some(imp) => names
|
||||
Some(imp) => crate::naming::face_names(b)
|
||||
.iter()
|
||||
.enumerate()
|
||||
.filter_map(|(i, n)| imp.faces.get(crate::naming::strip_piece(n)).map(|(c, o)| (i, Look::custom(c.map(to8), *o as f64))))
|
||||
|
||||
@@ -2358,8 +2358,14 @@ impl Document {
|
||||
|
||||
/// Evaluate an expression against the document's parameters.
|
||||
pub fn eval(&self, e: &str, kind: Kind) -> Result<f64> {
|
||||
let (vals, _) = self.param_values();
|
||||
Self::eval_in(&vals, e, kind)
|
||||
// The parameters are evaluated only when the expression names one: most expressions an
|
||||
// agent sends are plain values, and evaluating every parameter of a large design for
|
||||
// each of them made every new feature cost more than the last.
|
||||
let vals: std::cell::OnceCell<std::collections::BTreeMap<String, expr::Value>> = std::cell::OnceCell::new();
|
||||
expr::eval_with(e, &|n| {
|
||||
vals.get_or_init(|| self.param_values().0).get(n).copied().ok_or_else(|| DocError::Expr(format!("unknown parameter `{n}`")))
|
||||
})?
|
||||
.to_kind(kind)
|
||||
}
|
||||
|
||||
pub fn eval_in(vals: &std::collections::BTreeMap<String, expr::Value>, e: &str, kind: Kind) -> Result<f64> {
|
||||
|
||||
+56
-2
@@ -4753,9 +4753,61 @@ pub fn state_in_frame_at<'a>(doc: &Document, st: &'a ModelState, comp: u64, fram
|
||||
/// occurrence of it (bodies of the first placement keep their names; further instances get the
|
||||
/// occurrence path in brackets). Without components this is the model itself.
|
||||
pub fn world_state(doc: &Document, st: &ModelState) -> ModelState {
|
||||
world_state_reusing(doc, st, &mut PlacedBodies::default())
|
||||
}
|
||||
|
||||
/// Bodies a [`world_state_reusing`] call placed away from their definition (a moved
|
||||
/// component's bodies), by their definition and placement: the next call reuses each one whose
|
||||
/// definition and placement are unchanged instead of transforming it again, and so keeps its
|
||||
/// display mesh. Without this every change anywhere in a design transformed and meshed every
|
||||
/// body of every moved component again.
|
||||
#[derive(Default)]
|
||||
pub struct PlacedBodies {
|
||||
by_source: std::collections::HashMap<(usize, [u64; 16]), (ModelBody, ModelBody)>,
|
||||
}
|
||||
|
||||
impl PlacedBodies {
|
||||
/// The body `b` placed by `m`, from an earlier call when `b` is the same body as then.
|
||||
fn placed(&self, b: &ModelBody, m: &crate::Mat, name: String) -> ModelBody {
|
||||
let reused = self.by_source.get(&placed_key(b, m)).filter(|(source, _)| {
|
||||
Arc::ptr_eq(&source.mesh, &b.mesh)
|
||||
&& source.body.same(&b.body)
|
||||
&& source.body.color() == b.body.color()
|
||||
&& source.body.paint() == b.body.paint()
|
||||
&& match (source.body.split_lines(), b.body.split_lines()) {
|
||||
(Some(x), Some(y)) => Arc::ptr_eq(x, y),
|
||||
(None, None) => true,
|
||||
_ => false,
|
||||
}
|
||||
});
|
||||
match reused {
|
||||
Some((_, placed)) => {
|
||||
let mut fresh = ModelBody::new(name, placed.body.clone(), b.feature);
|
||||
fresh.mesh = placed.mesh.clone();
|
||||
fresh.plan = placed.plan.clone();
|
||||
fresh.plan_mesh = placed.plan_mesh.clone();
|
||||
fresh
|
||||
}
|
||||
None => ModelBody::new(name, kernel::transform_matrix(&b.body, *m).unwrap_or_else(|_| b.body.clone()), b.feature),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn placed_key(b: &ModelBody, m: &crate::Mat) -> (usize, [u64; 16]) {
|
||||
let mut bits = [0u64; 16];
|
||||
for (slot, v) in bits.iter_mut().zip(m.iter().flatten()) {
|
||||
*slot = v.to_bits();
|
||||
}
|
||||
(Arc::as_ptr(&b.mesh) as usize, bits)
|
||||
}
|
||||
|
||||
/// [`world_state`], reusing the bodies `placed` holds from an earlier call and leaving it holding
|
||||
/// this call's.
|
||||
pub fn world_state_reusing(doc: &Document, st: &ModelState, placed_before: &mut PlacedBodies) -> ModelState {
|
||||
if doc.occurrences.is_empty() {
|
||||
return st.clone();
|
||||
}
|
||||
let mut placed_now = PlacedBodies::default();
|
||||
let comp_of_feature = |id: u64| doc.feature(id).map(|f| f.component).unwrap_or(0);
|
||||
let first_path = |comp: u64| -> Vec<u64> {
|
||||
let mut path = Vec::new();
|
||||
@@ -4785,8 +4837,9 @@ pub fn world_state(doc: &Document, st: &ModelState) -> ModelState {
|
||||
placed.name = name;
|
||||
placed
|
||||
} else {
|
||||
let body = kernel::transform_matrix(&b.body, bm).unwrap_or_else(|_| b.body.clone());
|
||||
ModelBody::new(name, body, b.feature)
|
||||
let placed = placed_before.placed(b, &bm, name);
|
||||
placed_now.by_source.insert(placed_key(b, &bm), (b.clone(), placed.clone()));
|
||||
placed
|
||||
};
|
||||
// World states keep their previous display-only evaluation-key semantics.
|
||||
placed.key = 0;
|
||||
@@ -4834,6 +4887,7 @@ pub fn world_state(doc: &Document, st: &ModelState) -> ModelState {
|
||||
}
|
||||
}
|
||||
}
|
||||
*placed_before = placed_now;
|
||||
out
|
||||
}
|
||||
|
||||
|
||||
@@ -49,9 +49,10 @@ pub use assembly::{
|
||||
};
|
||||
pub use document::*;
|
||||
pub use eval::{
|
||||
EdgeRef, FeatureResult, Model, ModelBody, ModelState, RevolveExtent, SolvedSketch, ThreadInfo, curve_pick_error, edge_names_for, edge_refs,
|
||||
face_point_names, face_point_state, hole_direction_at, object_height, parse_metric_thread, pattern_matrices, revolve_extent,
|
||||
EdgeRef, FeatureResult, Model, ModelBody, ModelState, PlacedBodies, RevolveExtent, SolvedSketch, ThreadInfo, curve_pick_error, edge_names_for,
|
||||
edge_refs, face_point_names, face_point_state, hole_direction_at, object_height, parse_metric_thread, pattern_matrices, revolve_extent,
|
||||
revolve_extent_with_convention, select_profiles, sketch_hole_direction, state_in_frame, state_in_frame_at, through_all_distance, world_state,
|
||||
world_state_reusing,
|
||||
};
|
||||
pub use params::{ParamRow, ParamUser, cycles};
|
||||
pub use solidcraft_kernel as kernel;
|
||||
|
||||
@@ -467,10 +467,14 @@ impl Document {
|
||||
|
||||
/// The next free `dN` name.
|
||||
pub fn next_d_name(&self) -> String {
|
||||
// Every taken name at once: asking `name_taken` for d1, d2, … searched the whole
|
||||
// design once per number, quadratic in the number of named inputs.
|
||||
let taken: std::collections::HashSet<&str> =
|
||||
self.params.iter().map(|p| p.name.as_str()).chain(self.features.iter().flat_map(|f| f.param_names.iter().map(String::as_str))).collect();
|
||||
let mut n = 1;
|
||||
loop {
|
||||
let c = format!("d{n}");
|
||||
if !self.name_taken(&c) {
|
||||
if !taken.contains(c.as_str()) {
|
||||
return c;
|
||||
}
|
||||
n += 1;
|
||||
@@ -657,7 +661,15 @@ impl Document {
|
||||
let (vals, errs) = self.param_values();
|
||||
let all = self.all_param_exprs();
|
||||
let refs: BTreeMap<String, Vec<String>> = all.iter().map(|(n, e, _)| (n.clone(), expr::references(e))).collect();
|
||||
let used_by = |n: &str| -> Vec<String> { refs.iter().filter(|(_, r)| r.iter().any(|x| x == n)).map(|(k, _)| k.clone()).collect() };
|
||||
// Who uses each name, in name order: one pass over the references (a design an agent
|
||||
// built names every feature input, so a search per row was quadratic).
|
||||
let mut users: BTreeMap<&str, Vec<String>> = BTreeMap::new();
|
||||
for (k, r) in &refs {
|
||||
for x in r {
|
||||
users.entry(x.as_str()).or_default().push(k.clone());
|
||||
}
|
||||
}
|
||||
let used_by = |n: &str| -> Vec<String> { users.get(n).cloned().unwrap_or_default() };
|
||||
let value = |n: &str, k: Kind| vals.get(n).and_then(|v| v.to_kind(k).ok()).map(|x| if k == Kind::Angle { x.to_degrees() } else { x });
|
||||
let mut rows = Vec::new();
|
||||
for p in &self.params {
|
||||
|
||||
@@ -173,6 +173,8 @@ struct WorldCache {
|
||||
active_component: u64,
|
||||
active_occurrence_path: Option<Vec<u64>>,
|
||||
world: Arc<solidcraft_doc::ModelState>,
|
||||
/// The moved components' bodies as placed for `world`, reused by the next world state.
|
||||
placed: Arc<std::sync::Mutex<solidcraft_doc::PlacedBodies>>,
|
||||
}
|
||||
|
||||
impl WorldCache {
|
||||
@@ -395,17 +397,27 @@ impl Session {
|
||||
{
|
||||
return cached.world.clone();
|
||||
}
|
||||
let mut w = if self.pending_moves.is_empty() {
|
||||
solidcraft_doc::world_state(&self.doc, &st)
|
||||
} else {
|
||||
let mut d = (*self.doc).clone();
|
||||
for o in &mut d.occurrences {
|
||||
if let Some(m) = self.pending_moves.get(&o.id) {
|
||||
o.transform = *m;
|
||||
// The bodies the last world state placed: unchanged ones are not transformed and meshed
|
||||
// again (a design built command by command changed one body at a time).
|
||||
let placed = self.world_cache.lock().ok().and_then(|c| c.as_ref().map(|c| c.placed.clone())).unwrap_or_default();
|
||||
// Taken out while placing, so a scratch copy of the session sharing it never waits.
|
||||
let mut reuse = placed.lock().map(|mut p| std::mem::take(&mut *p)).unwrap_or_default();
|
||||
let mut w = {
|
||||
if self.pending_moves.is_empty() {
|
||||
solidcraft_doc::world_state_reusing(&self.doc, &st, &mut reuse)
|
||||
} else {
|
||||
let mut d = (*self.doc).clone();
|
||||
for o in &mut d.occurrences {
|
||||
if let Some(m) = self.pending_moves.get(&o.id) {
|
||||
o.transform = *m;
|
||||
}
|
||||
}
|
||||
solidcraft_doc::world_state_reusing(&d, &st, &mut reuse)
|
||||
}
|
||||
solidcraft_doc::world_state(&d, &st)
|
||||
};
|
||||
if let Ok(mut p) = placed.lock() {
|
||||
*p = reuse;
|
||||
}
|
||||
// Sketch definitions are displayed once, at the placement currently being authored.
|
||||
if self.active_occurrence_path.is_some() && self.active_component != 0 {
|
||||
let first = self.occurrence_placement(&self.doc.first_path(self.active_component)).map(|p| p.world).unwrap_or(solidcraft_doc::IDENTITY);
|
||||
@@ -440,6 +452,7 @@ impl Session {
|
||||
active_component: self.active_component,
|
||||
active_occurrence_path: self.active_occurrence_path.clone(),
|
||||
world: w.clone(),
|
||||
placed,
|
||||
});
|
||||
}
|
||||
w
|
||||
|
||||
@@ -175,3 +175,41 @@ fn adopted_scratch_keeps_its_warm_geometry_and_history_stages_invalidate_it() {
|
||||
run(&mut session, "timeline.suppress", json!({"feature":feature,"suppressed":false}));
|
||||
assert!(session.world_state().body("Draft").is_some());
|
||||
}
|
||||
|
||||
/// A moved component's bodies keep their placed geometry and display mesh while the design
|
||||
/// changes elsewhere: each new body used to transform and mesh every body of every moved
|
||||
/// component again. A body that changes, or a placement that moves, is placed afresh.
|
||||
#[test]
|
||||
fn moved_component_bodies_are_not_placed_again_when_something_else_changes() {
|
||||
let mut session = Session::default();
|
||||
let created = run(&mut session, "component.create", json!({"name":"Wheel"}));
|
||||
let occurrence = created["occurrence"].as_u64().unwrap();
|
||||
run(&mut session, "solid.box", json!({"length":60,"width":60,"height":5,"body_name":"Rim"}));
|
||||
run(&mut session, "component.activate", json!({"component":"root"}));
|
||||
run(&mut session, "occurrence.move", json!({"occurrence":occurrence,"translate":[100,0,0]}));
|
||||
let placed = session.world_state();
|
||||
let rim = placed.bodies.iter().find(|b| b.source_name.as_deref() == Some("Rim")).unwrap().clone();
|
||||
let mesh = rim.mesh();
|
||||
assert!(mesh.bounds().min.dist(Vec3::new(100., 0., 0.)) < 1e-6);
|
||||
// A body added elsewhere: the rim is the same placed body, already meshed.
|
||||
run(&mut session, "solid.box", json!({"length":5,"width":6,"height":7,"body_name":"Frame"}));
|
||||
let after = session.world_state();
|
||||
let again = after.body(&rim.name).unwrap();
|
||||
assert!(Arc::ptr_eq(&mesh, &again.mesh()), "an unchanged placed body keeps its mesh");
|
||||
assert!(again.body.same(&rim.body));
|
||||
// The placement moves: placed and meshed again where it now is.
|
||||
run(&mut session, "occurrence.move", json!({"occurrence":occurrence,"translate":[0,50,0]}));
|
||||
let moved = session.world_state();
|
||||
let moved_rim = moved.bodies.iter().find(|b| b.source_name.as_deref() == Some("Rim")).unwrap();
|
||||
assert!(!Arc::ptr_eq(&mesh, &moved_rim.mesh()));
|
||||
assert!(moved_rim.mesh().bounds().min.dist(Vec3::new(100., 50., 0.)) < 1e-6);
|
||||
// The body itself changes: placed again too, and the old snapshot is untouched.
|
||||
run(&mut session, "body.rename", json!({"body":"Frame","name":"Frame tube"}));
|
||||
let source = session.model.state().body("Rim").unwrap().clone();
|
||||
let world = session.world_state();
|
||||
let kept = world.bodies.iter().find(|b| b.source_name.as_deref() == Some("Rim")).unwrap();
|
||||
assert!(Arc::ptr_eq(&moved_rim.mesh(), &kept.mesh()));
|
||||
assert_eq!(kept.source_transform, moved_rim.source_transform);
|
||||
assert!(source.mesh().bounds().min.dist(Vec3::ZERO) < 1e-6);
|
||||
assert!(mesh.bounds().min.dist(Vec3::new(100., 0., 0.)) < 1e-6);
|
||||
}
|
||||
@@ -120,7 +120,12 @@ pub fn open(app: &mut SolveApp) {
|
||||
|
||||
/// Draw the dialog when it is open. Called once per frame.
|
||||
pub fn show(app: &mut SolveApp, ctx: &egui::Context) {
|
||||
publish(app, ctx);
|
||||
// Completions are offered only under a focused text box; gathering them evaluates every
|
||||
// parameter, so a design changing command after command (an agent building over MCP)
|
||||
// does not pay for it on every change while nobody is typing.
|
||||
if app.params.is_some() || ctx.memory(|m| m.focused().is_some()) {
|
||||
publish(app, ctx);
|
||||
}
|
||||
let Some(mut d) = app.params.take() else { return };
|
||||
let t = Tokens::get();
|
||||
let mut open = true;
|
||||
|
||||
Reference in new issue
Block a user