4 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Arena Agent
|
4371374bbb |
refactor(cad): track part mutations by generation (Phase 5.1, first increment)
`SceneCache` decided whether its cached `CadScene` was stale by comparing `scene.node_count()` against `parts.len()`. That is a guess, and its own doc comment called it "a safety net for missing `mark_dirty()` calls" — the real contract was "call `mark_dirty()` after every mutation", enforced only by discipline across ~15 write sites and already violated by the drawing tools, which push nodes directly. The guess has a blind spot: delete one node and add another in the same frame and the length is unchanged, so the cache reports clean and hands back a scene describing the previous contents. Exports and picking then run on stale geometry. `CadViewport.parts` is now a `PartsStore` — the `Vec<CadNode>` plus a monotonic `generation` that every mutating method bumps. You cannot get `&mut` to the nodes without going through a method that bumps it, so `SceneCache::scene_for(&store)` can compare generations exactly. Both the length heuristic and the manual-`mark_dirty` requirement are gone as a class, not fixed case by case. `scene(&[CadNode])` is kept for callers that hold no store (tests, the export path's detached copies) and now always rebuilds: without a generation there is nothing to compare, and guessing is what caused the bug. Migration is incremental by design. `as_mut_vec()` is an escape hatch that bumps unconditionally; three call sites still use it rather than turning this into one 118-site commit. Sharing one document across the three viewports (5.2) and retiring the remaining `mark_dirty` calls are separate changes. Also converted 10 duplicated `vp.parts.iter_mut().find(..)` blocks in workspace.rs to the checked accessor. Tests: 741 -> 750 passing, 0 failing. 9 new, including the delete-plus-add case the old heuristic missed and an in-place edit with no `mark_dirty()`; both were verified to fail against the restored length check. |
||
|
Arena Agent
|
bf280c0fb6 |
refactor(cad): delete the dead Legacy command system and duplicate types
Phase 4 of CAD_ASSESSMENT_AND_PLAN.md: remove the second copy of things the module carried alongside their replacements. No behaviour change. Removed, after confirming each has no live caller: - The entire LegacyCommand system: the trait, LegacyCommandCtx, LegacyUndoRedoStack and the six LegacyMovePart/Resize/Rotate/Add/Delete/ ModifyPart structs, plus their tests. The editor has used the Command / UndoRedoStack pair since v19; the legacy set was still exported and still maintained, so grepping for a command type returned two answers. - CadViewport::legacy_ctx(), which nothing called. - CommandError::Legacy, which was never constructed. - The duplicate DofConstraint in mod.rs and its two bridge From impls. Production reads cad_scene::DofConstraint exclusively; the legacy struct existed only so a conversion test could round-trip it. - Three dead functions in exporters.rs. The dead write_3d_viewer also carried a bug the live path does not: it wrote an absolute filesystem path into the exported viewer's <model-viewer src>, which breaks the moment the folder is copied anywhere. Renamed LegacyCommandCtxHolder -> CommandBorrows. Despite the name it holds the *new* UndoRedoStack and is the split-borrow helper for the live command path. Also refreshed the v10/v13/v18b archaeology comments that referenced the deleted types, and ported bench_command_execute_overhead to the live Command/CadCommandCtx path so the measurement survives. 1,588 lines removed: commands.rs 1863 -> 1318, tests.rs 3754 -> 2840, exporters.rs 186 -> 93. Tests: 728 -> 714 passing, 0 failing. The 29 removed all exercised the deleted system; verified by diffing the full test-name list before and after, which also caught 16 live-system tests that merely *mentioned* a Legacy type in passing and had to be kept. Rebasing onto origin/main also required repairing that branch, which did not compile (4 errors) and so had never run its own tests: - lookup.rs: an over-eager .clone() removal left *bt.category, deref'ing a String field to an unsized str and breaking the key type. - estimate_store.rs: two different tests shared one name; renamed both to say what they assert. - Once building, 3 tests failed. EstimateStore::dispatch took an undo snapshot and emitted UndoRedoChanged even when the command was a no-op, so a rejected out-of-range edit still consumed an undo slot and cleared the redo stack. Command::Initialize also never cleared the room list, so re-initialising kept the previous run's rooms. - spreadsheet-ui/workspace.rs: missing `use crate::model::WorkspaceModel`. |
||
|
Arena Agent
|
3a910b9aa7 |
perf(cad): cut hover-pick cost and stop full relayouts during drags
Phase 3 of CAD_ASSESSMENT_AND_PLAN.md. Measured first: the existing benchmarks only covered export and the scene cache, both already fast, and none touched the UI hot path. Added three that do, and recorded numbers in BENCH_BASELINE.md. pick_part broad phase: 2.1x less work per frame The per-part AABB was built from CadNode::size(). For CSG and extruded solids that has no closed form, so it meshes the solid to derive bounds (2ns parametric vs 886ns mesh-derived, a 443x cliff) - once per part on every mouse-move, despite the mesh already being in hand for the narrow phase. Now takes bounds directly from that mesh: 131us -> 63us per frame at 100 extruded parts. This also fixes a latent correctness bug. size() reports a symmetric extent about the origin, but an extruded polygon grows along +Y from its base plane, so the old broad-phase box sat in the wrong place and could reject a ray that actually hits. Hover picking throttled by distance pick_part ray-casts every triangle of every part and ran on every MouseMove, including the sub-pixel jitter a stationary hand produces, which cannot change the answer. Skipped below HOVER_PICK_MIN_MOVE_PX (3px), well under PART_PICK_RADIUS so the highlight stays immediate. Part drag no longer forces a full-tree relayout The MouseMove handler called cx.redraw_all() on top of area.redraw() for every motion event. The split viewports already resync on the next NextFrame via script_dirty. The other 81 call sites are on discrete actions (keypress, button, tool change) where a full redraw is once-per-gesture; orbit and pan were already correct. Tests: 720 -> 728 passing, 0 failing. 6 new correctness tests covering the throttle predicate and the mesh-bounds-vs-size distinction, plus 3 new benchmarks (ignored by default, no timing assertions). |
||
|
Arena Agent
|
b5471e32e3 |
fix(cad): make the crate buildable, testable and safe to ship
Some checks failed
nigig-build (CAD) / supply-chain (push) Has been cancelled
nigig-build (CAD) / cad-module (push) Has been cancelled
nigig-build (CAD) / full-crate-check (push) Has been cancelled
PDF engine / engine (push) Has been cancelled
PDF engine / makepad-integration (push) Has been cancelled
PDF engine / fuzz (push) Has been cancelled
Phases 0-2 of CAD_ASSESSMENT_AND_PLAN.md. The crate did not compile and no
test had ever run; it now builds clean with a green suite.
Build and CI (Phase 0)
- Pin all 33 git dependency manifests to an explicit rev. A branch
dependency re-resolves on every build and is a code-execution path into
CI if force-pushed.
- Commit Cargo.lock (540 packages). Producing it required fixing three
resolution failures the workspace had always had: a non-existent
makepad-widgets feature, two rusqlite versions both linking sqlite3, and
four missed CellId call sites in spreadsheet-ui.
- Add .forgejo/workflows/nigig-build.yml.
- Replace five stale CAD docs that contradicted the code with one
ARCHITECTURE.md; add PHASE0/1/2_STATUS.md and TEST_BASELINE.md.
Correctness (Phase 1)
- Rotation units: transform_point bound sin_cos() backwards, transposed X
and Z, and applied axes in reverse order, so every exported STL was wrong
even at zero rotation. It now shares the renderer's matrix helpers.
- GLB quaternions had norm 0.125 (half-angle applied to cos/sin, degrees
read as radians) - invalid per the glTF spec.
- PDF wall/door/window yaw fed degrees to cos/sin.
- Fix a TOCTOU unwrap in touch picking; viewport.rs now has no unwrap().
- CommandContext gains update_node/insert_node_at/node_index: resize and
modify were delete+create, silently moving nodes to the end of the scene.
- Wire MAX_UNDO_LEVELS (defined, exported, never read) and switch the undo
stack to VecDeque; this also made the existing drag-merge logic reachable.
- CadNode::size() returned a fake 1x1x1 for CSG and extruded solids, making
them unpickable outside a 1x1x1 box at their origin.
- Reject non-finite script input; makepad_csg clamps NaN rather than
propagating it, so bad input produced silently wrong geometry.
Test baseline: 0 -> 722 passing, 0 failing
- 17 pre-existing failures fixed: 10 real defects (dependency-cycle
detection, over-allocation of unassigned tasks, quote/backslash
corruption on save, default rooms lost for all but the first region,
RGA text ordering) and 7 tests that were themselves wrong, each checked
against its production caller first.
Security (Phase 2)
- env!("CARGO_MANIFEST_DIR") was used as a runtime path in three places,
including as the AI agent's working directory. All runtime data now goes
under app_data_dir().
- Remove the hardcoded LAN LLM endpoint. It is now opt-in via
NIGIG_CAD_LOCAL_OPENAI_URL/_MODEL and refuses plaintext HTTP to anything
but loopback.
- Bound and content-sniff AI image attachments (8 MB cap, magic bytes);
the MIME type came from the filename extension.
- Escape SVG/HTML output, and add SRI to the exported viewer's script tag.
The pinned model-viewer@3.5.1 does not exist, so every exported viewer
was silently broken; now 4.0.0 with a verified hash.
- Stop embedding $USER in exported PDFs and logging document content in
release builds.
- CI now rejects reintroducing the runtime-path and hardcoded-endpoint
classes; both gates were verified to fail on a reintroduced defect.
Add system_prompt.md and embed it with include_str!. The file was missing
from the repository, so the agent silently used a one-line fallback.
|