18 commits

Author	SHA1	Message	Date
andodeki	cbce585eb4	perf(cad): complete 2D and 3D LOD -- Phase 5 ... The first Phase 5 tranche correctly reduced sub-pixel 2D outlines, but it stopped short of the plan's full level-of-detail phase. Complete the phase by applying the same measured three-pixel policy to the 3D path. Visible 3D parts below the projected AABB threshold now use one shared unit-cube geometry with an axis-aligned world-bounds transform. Full-detail shape batches, proxy instance rows and proxy draw calls are reported separately. Wireframe and hidden-line overlays use the same policy, and selected or hovered parts remain full detail. Extend the host-testable projection and proxy policy tests, frame benchmark, source guards and documentation. Keep the real GPU/window visual check explicit: compilation, policy arithmetic, submission structure and coverage pass here, but this environment cannot execute Makepad's draw submission.	2026-08-26 16:25:45 +00:00
andodeki	a5ba719d8d	perf(cad): add measured sub-pixel LOD to the 2D path -- Phase 5 ... Phase 4 reduced the number of tessellation calls, but it left the geometry volume unchanged: a zoomed-out 2,000-part plan still queued a full outline for every visible part. At the measured 200 m site view a 1 m part is only about 2.7 logical pixels across, so the remaining geometry was not resolvable detail. Add a pure lod policy that projects plane extents into logical pixels, keeps selected and hovered parts full detail, and conservatively falls back to a full outline for malformed state. The renderer and CadViewport::frame_budget call the same policy. Ordinary sub-pixel parts use one bounded 2x2 filled marker, while FrameBudget reports full outlines, markers, strokes and fills separately. Also fold the remaining 2D 1.2 margins into render_budget::VIEW_MARGIN, extend the structural benchmark and coverage harness, correct the phase documentation, and explicitly leave 3D mesh LOD deferred until a real GPU/window measurement justifies a second geometry policy.	2026-08-26 15:40:02 +00:00
andodeki	4cbb155cb7	perf(cad): stroke per group, not per item -- Phase 4 of the render plan ... A 2,000-part plan view with everything on screen cost 2,270 tessellation calls a frame. It now costs four: two for the grid, two for the parts. `stroke()` tessellates the whole accumulated path and clears it -- `tessellate_path_stroke` ends in `path.clear()` -- so queueing many subpaths and stroking once is one tessellation instead of N. The idiom was already in this file: `queue_dashed_line` has done it for the axis grid since Phase 3.9, guarded by a test. Phase 4 applies it to the two loops that never adopted it. **Base grid: two passes, two strokes.** Minors queued and stroked at 0.55, majors at 1.6 -- the stroke width is the one thing that genuinely needs its own call. `GridRange::has_minor_lines`/`has_major_lines` decide whether a pass runs at all and `frame_budget` counts strokes with the same two predicates, because an empty `stroke()` still enters the tessellator and a budget that assumed two when the renderer made one would be wrong in the direction that hides work. Minors stroke first so majors land on top where they cross; same colour either way, so the only visible difference is that the thicker line wins a crossing, which is the right answer. **Parts grouped by colour.** New `batching::ColorKey` -- the bit pattern, because `f32` is not `Hash` and two colours whose bits differ are two colours -- feeding the same `group_in_first_appearance_order` that Phase 3 groups shapes with. The colour policy moved out of the two draw loops into `constants::part_outline_color`, so the renderer and `frame_budget` cannot disagree about how many groups a frame has; the 2D loop had `vec4(1.0, 0.82, 0.40, 1.0)` written out where `PART_SELECT_COLOR` already existed. **Selected and hovered parts stroke last**, in their own groups, so a highlight is never hidden under a neighbour's outline. They were interleaved in document order before and could be. `FrameBudget` gained `grid_lines` and `part_outlines` beside the call counts. Geometry volume and call count are different numbers now and both are worth reading -- `VectorSubmission { outlines, stroke_calls }` mirrors Phase 3's `MeshSubmission` for the same reason. Measured (bench_frame_submission_budget, 1920x1080, 200 m site): zoom 5 m, 2000 parts: 12 visible outlines -> 4 tessellations (was 2170) zoom 200 m, 2000 parts: 2000 visible outlines -> 4 tessellations (was 2270) The second row is the point, and it is the row Phase 1 could not move: everything is on screen, culling removes nothing, and the frame still costs four calls. WHAT THIS DOES NOT DO: vertex volume is unchanged. The same 2,000 rectangles are tessellated -- in two calls rather than 2,000. What is saved is per-call overhead: tessellator setup, two `std::mem::take`s and an `append_geometry` each time. If a 2,000-part plan view is still slow after this, the remaining cost is triangles, which is Phase 5 and should only happen if a measurement asks for it. One visible-behaviour caveat, stated rather than buried: parts of the same colour are now drawn together, so where two outlines of *different* colours overlap, which is on top can change. They are 1.8 px outlines and the highlight ordering got strictly better, but it is a change to what is drawn, not only to how. Two tests were wrong before the code was, which is becoming this plan's pattern. `constants.rs` fell to 81.82% and the coverage floor caught it -- `part_outline_color` had no tests, and it now has five. And the guard test's first draft looked for a closing brace at a fixed indentation, matched the wrong one, and failed on correct code; it matches braces properly now. Verified: tools/test-cad-coverage.sh green -- total 97.33%, batching.rs 100%, cull.rs 100%, render_budget.rs 99.68%, constants.rs 98.55%, all floors met; cargo check --locked -p nigig-build --lib clean; cargo test --lib 1114 passed (1100 + 14 new); --test cad_integration 154 passed; CAD_BENCH=1 harness green; cargo fmt --check and git diff --check clean.	2026-08-21 05:08:14 +00:00
andodeki	dc1defd7e8	perf(cad): one draw call per shape, not per part -- Phase 3 of the render plan ... A 2,000-part model of six repeated shapes now draws in six calls. It drew in 2,000 before, and in 733 after Phase 1 culling at a working camera distance -- but at 400 m, with the whole site on screen and nothing to cull, it still drew in 2,000. Culling decides which parts are submitted; instancing decides how many calls carry them. They are different axes and this is the one that does not care where the camera is. No shader change was needed and that was the surprise of the phase. `DrawCadMesh`'s `transform`, `color`, `depth_clip` and `display_mode` are `#[live]` fields after `#[deref] draw_vars` in a `#[repr(C)]` struct, which is precisely the per-instance row `DrawVars::as_slice` packs and sends. The loop was already sending instance rows -- one per call. Three methods (`begin_instances`, `push_instance`, `end_instances`), ported from `DrawPbr::begin_many_instances_for_mesh` in the pinned fork, keep the call open across a group instead. New `batching.rs` (pure, 100% covered, floored): group_in_first_appearance_order(items) -> Vec<(K, Vec<T>)> batch_count(keys) -> usize First-appearance order rather than iterating a `HashMap`, deliberately. `HashMap` order is randomised per process, so batch order -- and the order parts reach the GPU -- would differ between runs and after a rehash. For opaque depth-tested geometry that is invisible, which is exactly what makes it a bad thing to depend on: the day someone adds a translucent material it becomes a flicker that reproduces on one machine in five. `FrameBudget` now carries `MeshSubmission { instances, batches }`. Named fields rather than a second positional `usize` because the entire point of the phase is that the two numbers now differ, and a caller that swapped them would report the win backwards. Measured (`bench_frame_submission_budget`, counts): camera 20 m, 2000 parts: 733 visible -> 6 draw calls (was 2000) camera 80 m, 2000 parts: 1459 visible -> 6 draw calls (was 2000) camera 400 m, 2000 parts: 2000 visible -> 6 draw calls (was 2000) ...and with every part a different size, 733/1459/2000 -- the ceiling, which is in the table for the same reason Phase 2's is. WHAT IS NOT VERIFIED, plainly. The instanced submission has never run. There is no GPU, no window and no `Cx` here, and tests/ui.rs still fails at child-build exit 101. What is verified: it compiles against the real Makepad API; the grouping is right (seven tests, including "no item is lost or duplicated", which is the failure mode hardest to see in a screenshot); the budget arithmetic is right; and the batch cannot be left open on any path -- `every_instanced_batch_is_closed_before_the_loop_turns` is a source check in the same style as the redraw_all guard, because a batch left open drops its rows and the parts simply vanish with no error anywhere. Someone with a window needs to open a 3D model and confirm the picture is unchanged. Two things limit the damage if it is not: `begin_instances` returning false falls back to the old one-call-per-part loop (it returns false while the draw shader is still compiling, which happens on the first frames of every window), and batch order is deterministic, so a defect reproduces instead of flickering. Verified: tools/test-cad-coverage.sh green -- batching.rs 100%, cull.rs 100%, total 97.30%, all floors met; cargo check --locked -p nigig-build --lib clean at the 127-warning baseline; cargo test --lib 1100 passed (1091 + 9 new); --test cad_integration 154 passed; CAD_BENCH=1 harness green; cargo fmt --check and git diff --check clean.	2026-08-21 04:41:48 +00:00
andodeki	1e5d014198	perf(cad): one GPU buffer per shape, not per part -- Phase 2 of the render plan ... Two hundred identical columns held two hundred GPU vertex buffers. They build byte-identical triangles -- `CadSolid::build_mesh` is a pure function of the solid payload and returns local-space geometry, and the upload path (`part_mesh_buffers_from_mesh` -> `MeshSpace::Model`) carries no transform and no colour -- so the duplication bought nothing but memory and upload time. `part_geoms` is now `HashMap<ShapeHash, Geometry>`. **ShapeHash**, new in cad_scene.rs: the solid payload and nothing else. No node id, no transform, no material. `ParamHash` is now *defined in terms of it* -- `id` then `ShapeHash` -- rather than repeating the match arm by arm. That is deliberate: last commit corrected a plan written on the assumption that equal `ParamHash` meant "same shape", which the leading `node.id.hash()` quietly made false. With one shared body the two hashes cannot drift apart again, and `param_hash_moves_whenever_the_shape_hash_moves` pins the join. Three consequences worth naming: - **Staleness became structural.** The key IS the content hash, so the draw loop's `.filter(|(hash, _)| *hash == ParamHash::from_node(part))` is gone: an edited part looks up a key that does not exist yet and `ensure_part_geometry` uploads it on the same frame. There is no "entry uploaded from different parameters" state left to guard against, which is the class of bug `subdivide_selected` shipped. - **Eviction is by live shape, not live id.** This is the hazard the change introduces and it is not obvious: deleting one of two hundred identical columns must NOT drop the buffer the other 199 draw from. An eviction written as "remove the deleted node's entry" would blank most of the model. `geometry_is_retained_by_live_shape_not_by_live_id` pins it. - **Uploads deduplicate within the frame.** Without the `queued` set, the first frame of a 200-column scene would call `get_or_build` two hundred times before the map had anything in it. Measured (`bench_geometry_buffers_shared_by_shape`, counts not timings): 200 identical walls 200 parts -> 1 buffer (200x) 420-part repetitive model 420 parts -> 6 buffers (70x) 420 all-distinct parts 420 parts -> 420 buffers (1x) The last row is in the table on purpose. Sharing is a property of the model, not of the code; a scene where every part differs gets nothing from this phase. And draw calls are unchanged -- still one per visible part -- exactly as the plan predicted. Collapsing those is Phase 3, which needed these shared buffers to be possible at all. Estimated at a week, took under a day. Two things the estimate did not know: `MeshCache::get_or_build` was already a pure function of `node.solid`, and the upload path already produced transform-free, colour-free buffers. It assumed both would need untangling; they were built right the first time. Verified: tools/test-cad-coverage.sh green -- total 97.27%, cad_scene.rs 98.56%, cull.rs 100%, all floors met; cargo check --locked -p nigig-build --lib clean (and one warning fewer: the `ParamHash` re-export in mod.rs is no longer needed by the widget layer); cargo test --lib 1091 passed (1085 + 6 new); --test cad_integration 154 passed; CAD_BENCH=1 harness green; cargo fmt --check and git diff --check clean.	2026-08-21 04:18:33 +00:00
andodeki	4aaebafe4e	perf(cad): cull parts against the viewport -- Phase 1 of the render plan ... Before this, `grep -niE "cull|frustum|offscreen|in_view"` over the 2,438 line renderer returned nothing. Every part in the document was tessellated in 2D and issued its own draw call in 3D, on screen or not. At a 5 m drafting zoom over a 200 m site, a 2,000-part model spent 2,000 tessellations and 2,001 draw calls per frame to show twelve parts. New `cull.rs` -- pure, host-testable, 100% covered with a floor in tools/test-cad-coverage.sh: draw_part_2d(centre, half_w, half_h, view, decorated) -> bool Frustum::from_view_projection(view, projection) -> Frustum Frustum::draw_part_3d(aabb, decorated) -> bool world_aabb_from_local_bounds(min, max, model) -> WorldAabb It lives in its own module for the same reason nav_pad.rs and render_budget.rs do: viewport_render.rs is 2,438 lines at 0.00% coverage and needs a Cx, so a predicate written inline there could not be tested at all. Both draw loops AND CadViewport::frame_budget call these same functions, which is what stops the reported budget from drifting away from what is drawn -- the discipline render_budget.rs already applies to the grid loops. Measured (bench_frame_submission_budget, 1920x1080, parts on a 200 m site; full tables in BENCH_BASELINE.md): 2D, 5 m zoom, 2,000 parts: 12 visible. 2,170 -> 182 tessellations, 2,001 -> 13 draw calls. 3D, 20 m camera, 2,000: 733 draw calls, 63% culled. 3D, 80 m camera, 2,000: 1,459 draw calls, 27% culled. Whole site on screen: nothing culled, and nothing should be. That last row is the honest half and it is in the doc too: when the view holds the whole model, all 2,000 parts are genuinely visible and culling cannot help. Phases 2 and 3 (shape-shared geometry, then instancing) are what address that case. Three things came out different from the plan I wrote last commit, and the plan was wrong about each: - **AABB, not bounding sphere.** A sphere around an AABB is looser at the same cost -- a 6 m wall gets a 3 m radius ball. The p-vertex AABB-versus-plane test is strictly tighter. So there is no bounding_sphere helper, and in particular no bounding_sphere FIELD on CadNode: the AABB is already cached by PlacedHash in SceneCache, which invalidates itself on a move or a resize, where a stored field would have to be maintained at 77 construction sites. - **world_aabb_from_local_bounds is shared with pick_part**, replacing the eight-corner transform that was inline there. Two copies would be two chances to disagree about a part's bounds, and picking a part the renderer culled is precisely the bug that disagreement produces. - **The "drawn extent" hazard split into two concrete rules.** The 2D test takes part_to_plane_2d and part_size_on_plane -- the very values the draw uses -- so it tests the drawn rect, not the model extent. And no selected or hovered part is ever culled: the highlight and the tooltip are drawn at the cursor, arbitrarily far from the part. The frustum comes from Gribb-Hartmann row arithmetic on projection * view, deliberately not from mat4_inverse -- that function had a mistyped index in all sixteen cofactors until 2ea5a74, and the terms that were wrong are exactly the ones a perspective matrix makes non-zero. Makepad's Mat4f::perspective is the OpenGL convention (xr/src/scene/xr_root.rs builds the camera with it), so -w <= x,y,z <= w is right; a 0..w depth projection would only make the near plane more permissive, which is the safe direction. Both predicates are conservative by construction: NaN coordinates, NaN AABBs, negative extents and an identity camera all fall through to "draw it". A part kept but invisible costs one submission; a part culled but visible is a bug the user sees. One test of mine was wrong before the code was: the first draft asserted a part 40 m off axis was invisible from a 50 m camera at 60 deg on 16:9. tan(30 deg) * 16/9 is 1.03, so the horizontal half-angle reaches past 45 deg and the part is on screen. The frustum was right; the test is now narrow-fov and says so in its docstring. Verified: tools/test-cad-coverage.sh green -- cull.rs 100.00%, total 97.26% (was 97.20%), all floors met; cargo check --locked -p nigig-build --lib clean; cargo test --locked -p nigig-build --lib 1085 passed (1062 + 23 new); --test cad_integration 154 passed; cargo fmt --check clean; git diff --check clean. Not done here: the two hard-coded 1.2 margins in viewport_render.rs (397-400, 1374-1377) still do not read render_budget::VIEW_MARGIN. cull.rs does, so cull and grid agree by the constant rather than by the code. Folded into Phase 4, which rewrites those loops anyway.	2026-08-20 21:45:45 +00:00
andodeki	01d889c90b	docs(cad): what the first widget extraction actually bought ... 13.15% -> 13.25%, and viewport_input.rs is still at 0.00%. Worth writing down rather than quietly celebrating a fix. Extraction makes logic testable by moving it out of the handler, so the handler shrinks instead of getting covered: nav_pad.rs took 64 lines to 100%, and viewport_input.rs went from 736 lines uncovered to 672 lines uncovered. At ~60 lines an extraction the remaining input handler is ten more of these. The pixel bug found on the first one suggests the yield is real, so it is a reasonable way to spend effort — but anyone expecting the widget number to climb fast should know it will not, and that actually covering the handlers means driving them with an event loop through makepad-test, which tests/cad_ui.rs and spreadsheet-ui already use.	2026-08-17 12:20:21 +00:00
andodeki	0596fc66ed	test(cad): measure the widget layer — 13.15%, and six files at zero ... The number nobody had. The engine harness is structurally blind to viewport*.rs, workspace*.rs and friends, so "97.14% covered" has always been a statement about the smaller half. With the Makepad Linux packages installed, the real crate builds and its tests run under -C instrument-coverage, which makes the other half measurable in about six minutes. 10,714 lines. 13.15% covered. 9,305 never executed by any test. viewport_input.rs 736 lines 0.00% viewport_render.rs 2,083 lines 0.00% workspace_actions.rs 690 lines 0.00% cad_editor_sheet.rs 224 lines 0.00% viewport_2d.rs 138 lines 0.00% code_editor.rs 67 lines 0.00% viewport.rs 3,548 lines 14.37% workspace.rs 2,462 lines 14.18% script_bindings.rs 724 lines 71.27% viewport_input.rs is every click, drag, modifier and keystroke the editor handles, and not one line of it has ever run in a test. viewport_render.rs is every draw call. This reframes the six sessions of engine work above it. 1044 green tests and a 97% engine coexist with an input layer nothing has touched. Those facts were never in tension — they were just never on the same page, because the tool that produced the good number could not see the bad one. A coverage figure that excludes the risky half is not a summary, it is an average with the interesting term deleted. The new script reports the widget files ONLY, on purpose. Folding them into one number would let a 97% engine hide a 0% input layer, which is the arithmetic this exists to prevent. No floors yet, deliberately: a floor at 13% reads as a blessing rather than a debt. The first real input test should set one behind it. Verified: two runs, cold and warm, same numbers; script cleans its profraw data on exit and refuses with a useful message when the native packages or llvm-tools are missing.	2026-08-17 11:59:06 +00:00
andodeki	f1c3c18374	docs(cad): the widget layer was never unbuildable — I never tried ... Every claim I have made about this environment's limits was wrong, and it cost six sessions of work routed around an obstacle that was not there. `cargo test -p nigig-build` needs wayland, X11, GL, alsa and polkit. I turned that into "does not build here" early on, wrote it into commit messages, wrote it into TEST_BASELINE.md, wrote it into the review, and never retested it. In a plain container: bash tools/makepad-native-libs.sh --install # ~8s cargo check --locked -p nigig-build --lib # 2m52s, clean cargo test --locked -p nigig-build --lib # 5m22s, 1044 passed cargo test --locked -p nigig-build --test cad_integration # 154 passed cargo fmt -p nigig-build -- --check # clean The install script existed the whole time, written for exactly this, with a comment explaining that libasound and libpulse are needed to *link* a test binary rather than merely to check it. This file's own "Reproducing" section pointed at it. So did the review's "Required next commands". Two things follow. First, every commit I have pushed to this crate is now verified rather than CI-gated-and-hoped-for, including `refactor(cad): one screen-to-world path, not two`, whose message says plainly that its compile could not be checked locally. It compiles; the suite passes; the gates pass. Second, the honest reading of the last six sessions: I asked once whether to attempt the heavy build, got a reasonable "ship what you can actually test", and then treated that as settled fact rather than a decision worth revisiting when the cost of being wrong kept growing. An assumption made once and never retested is indistinguishable from a fact. That is precisely the criticism this work levelled at the CAD review entry, and I earned it too. The 750-test figure in this file was also stale; it is 1044 now. The host-only harness stays. It needs no apt, no root and no desktop packages, runs in ~20s warm against five minutes for the full crate, and is what the CI coverage gate uses. The full build is what to reach for when a change touches the widget layer, which the harness cannot see.	2026-08-17 11:55:25 +00:00
andodeki	179fe0533d	docs(cad): replace the guess about the widget layer with the compiler's answer ... This file said ~18,000 lines "needing live_design!, Cx and an event loop". That was a grep-shaped guess, and it was wrong twice: it swept in tools.rs, which needed no such thing, and profile_benchmarks.rs, whose only Makepad imports are the math types. Replaced with a per-file table produced by adding each of the eleven files to the harness on its own and reading what the compiler asked for. The answer is unglamorous but now trustworthy: every remaining file is blocked by a widget or platform type — Cx, Cx2d, Event, Widget, LiveId, ScriptVmBase — and there is no cheap seam left. Measuring any of them means a real Cx, or lifting logic out of `impl CadViewport` into free functions first, which needs a compiler for the widget layer. Recording the sizes alongside the blockers so the next person can pick the cheapest one deliberately instead of by feel.	2026-08-17 10:17:46 +00:00
andodeki	de255a617c	docs(cad): correct an overstated impact claim about the mat4_inverse fix ... I wrote that the broken matrix inverse meant "3-D picking resolved clicks to the wrong world point, about one unit off at a 35 degree yaw". That is not true, and I should have checked before writing it. `mat4_inverse` has exactly two callers -- `CadViewport::camera_eye` and `CadViewport::unproject_point` -- and grepping the workspace shows nothing calls either of them. Live 3-D picking runs through `screen_to_ray_3d`, which builds the ray from the camera's yaw, pitch, distance and field of view and never inverts a matrix. So the defect was real and latent, not real and live: a wrong answer waiting for its first caller. The fix and its tests stand; the severity claim does not, and an overstated bug report is its own kind of defect in a file people read to decide what to work on next. It does surface something worth fixing on its own terms, now recorded here: viewport.rs carries two independent camera-to-world implementations -- the dead matrix pair, and the live spherical one that duplicates the 42-degree fov and the axis conventions. Two implementations of the same geometry, one never exercised, is how conventions drift apart.	2026-08-17 05:26:45 +00:00
andodeki	f9f4bda2d8	docs(cad): final coverage table, and the list of what is deliberately not covered ... 96.53% -> 97.14%. Two things worth having in the file rather than only in commit messages. First, the list of what is NOT covered and why: the printpdf emitter (152 lines, checked by opening the file), the save-dialog branch (needs a windowing system), the two persistence wrappers (they write to the real user data directory), the `panic!` arms inside tests, and the one `#[ignore]`d test that pins release-only behaviour. Every one of those is a decision, and a reader who does not know that will either try to "fix" them or quietly lower a floor. Second, a finding about the measurement rather than the code. `build_glb` only reaches rayon at 32+ geometric nodes; every test used one or two, so the parallel arm had never run and the test named "the sequential fallback matches the parallel builder" was comparing two sequential paths. It would have passed with the parallel arm deleted. The only thing that showed it was those lines staying red after a commit whose message claimed to cover them — which is the argument for reading the per-file report rather than watching the total.	2026-08-17 04:42:27 +00:00
andodeki	ede451136b	docs(cad): refresh the coverage table, 88.75% -> 96.53% ... Six tranches since the table was written. It also gains the second finding, which is quieter than the matrix bug and the same shape as it: three of the four exporters had an arm per CadSolid variant and only one variant had ever been walked through them. SVG had boxes, GLB had boxes, the PDF projector had walls. That failure mode is worth writing down rather than leaving in commit messages. A part whose arm is wrong does not fail anything -- the export succeeds, the file opens, and the column is not in it. All four exporters are now driven over every variant they claim to support.	2026-08-17 04:32:52 +00:00
andodeki	2770716516	docs(cad): record the engine coverage baseline and what it excludes ... TEST_BASELINE.md said "750 passed / 0 failed" and stopped there. A pass count says the suite is green; it says nothing about what the suite touches, and this module shipped a broken matrix inverse under 750 green tests. Adds the per-file table, the enforced floors, and -- the part that matters -- the list of what is NOT measured. Twelve files, roughly 18,000 lines of widget code, have no coverage number at all. Reading 88.75% as "the CAD module is 88.75% covered" would be wrong: it is the engine that is, and the engine is the smaller half. Better to write that down than to let the number be quoted without it.	2026-08-16 22:27:22 +00:00
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.	2026-07-28 17:51:51 +00:00
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`.	2026-07-28 17:37:21 +00:00
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).	2026-07-28 17:18:13 +00:00
Arena Agent	b5471e32e3	fix(cad): make the crate buildable, testable and safe to ship ... 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.	2026-07-28 16:49:30 +00:00