16 commits

Author	SHA1	Message	Date
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	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	14aa0d5017	perf(cad): measure what a frame submits — Phase 0 of the render plan ... Phase 0 of REVIEWS/CAD_RENDER_OPTIMISATION_PLAN.md. No optimisation here; this is the measurement everything after it depends on. 0a first: Makepad does not count draw calls. `Cx::performance_stats` is `PerformanceStats { last_frame_time, max_frame_times }` — frame times only. So the seam had to be built. `render_budget.rs` reports two costs, deliberately not summed: tessellations CPU calls into tessellate_path_stroke draw calls GPU submissions Measured at 1920x1080, now in BENCH_BASELINE.md: zoomed in (0.2 m grid, 170 lines) 2000 parts -> 2170 tess, 2001 draw calls zoomed out (5 m grid, 270 lines) 2000 parts -> 2270 tess, 2001 draw calls The grid column barely moves across a 40x zoom range because the step adapts — that half is already virtualised. The parts column is every part, every frame, on screen or not. **This is not a model of the renderer.** The obvious way to count submissions is to write a second copy of the loop structure and count what it would do, which is exactly how the scene-cache benchmarks ended up timing a function that cannot cache. So `grid_range` owns the decision and `draw_2d_vector_scene` now drives its loops from it: the count and the drawing come from one function and cannot disagree. The ~25 lines of nice-number step arithmetic that were inline in the renderer now live in the pure module, with tests. 11 tests, 100% of the new module, and two of them are there to pin things people get wrong: - the 2D scene is ONE draw call regardless of part count, because DrawVector::end() submits the whole accumulation; - 3D is one draw call per uploaded part, which is where they multiply. One test — culling_shows_up_as_fewer_part_tessellations — asserts the *shape* of Phase 1's improvement before the work starts: parts fall proportionally, the grid column does not move. Verified: 1062 lib tests pass, cad_integration 154, cargo fmt clean, engine coverage 97.20% with every floor met including render_budget at 100%. The benchmark runs host-only under CAD_BENCH=1.	2026-08-20 21:01:52 +00:00
andodeki	70bbff7ecb	fix(cad): the nav pad's zoom buttons were a pixel from their own hit zone ... First test written against the widget layer, and it found something on the way in. The viewport's navigation pad — fit, four pans, zoom in, zoom out — had its layout written twice. `viewport_render.rs` drew each button at `col * (BTN_W + GAP)`. `viewport_input.rs` decided what a click hit with hand-written arithmetic inline in a 630-line event handler: `BTN_W * 3.5 + GAP * 3`. Those agree for whole-numbered columns and disagree at the half column the zoom pair sits in: 3.5 * 24 = 84 drawn, 22 * 3.5 + 2 * 3 = 83 hit-tested. The zoom buttons' clickable area sat one pixel left of the buttons, so their right-hand pixel column did nothing and a pixel of empty space beside them zoomed. `PanRight` was 2px short at its right edge for the same reason. One pixel is not much on its own. The mechanism is what matters, and it is the third instance of it in this module: two copies of one piece of geometry, free to drift, with nothing able to notice. The camera-to-world pair was the first, the scene-cache benchmarks the second. Both callers now read `nav_pad::LAYOUT`. The renderer iterates it; the hit test tests against it; the offsets come from one function. That also turns 88 lines of inline conditionals in the event handler into 32 lines of match, which is a readability win I would not have bothered with on its own. The bounds are now the drawn rectangle exactly — half-open, BTN_W by BTN_H, gaps dead. The old hit zones were 2px larger than the buttons in several places. Being strict is deliberate: a hit area larger than its button is indistinguishable from a misaligned one the next time something looks wrong. 7 tests, 100% of the new module. The one that matters is `drawn_and_hit_zones_agree`: every drawn rectangle must hit-test to its own button at all four corners and the centre. That test fails on the old code, which is the only reason to trust it. Verified with a real compiler, which this environment turns out to have: 1051 lib tests pass (7 new), cad_integration 154 pass, cargo fmt clean, engine coverage 97.16% with every floor met including nav_pad at 100%.	2026-08-17 12:12:34 +00:00
andodeki	ac145bfaab	test(cad): measure tools.rs, which a false comment had ruled out — 0% to 97.07% ... tools.rs opens with: Struct definitions stay in mod.rs (they use #[derive] macros that need the script_mod! context). Only the impl blocks are here. That is not true, and it is the reason 250 lines of pure tool-state logic — tool cycling, work-plane mapping, the inclined-UCS maths — had never been measured or tested. `CadTool`, `WorkPlane`, `AxisLock`, `DrawingState`, `SnapSettings` and `InclinedPlane` each derive some combination of Clone/Copy/Debug/PartialEq/Eq and nothing else, and none is inside the `script_mod!` block. I found it by asking the compiler instead of grepping. Adding each of the eleven unmeasured CAD files to the harness one at a time and reading the errors gives the real dependency, not a guess: tools.rs needed six plain types; viewport_2d.rs needs `CadViewport` and `Cx`; code_editor.rs needs `Cx2d` and `DrawStep`. Only the first of those is a documentation problem rather than a real one. My previous two triage passes used a grep heuristic and were wrong twice, including about this file. The harness now mirrors those six declarations, extracted from mod.rs at run time so they cannot drift, exactly as it already did for ViewMode and SelectionMode. **No production code moved.** Moving the declarations for real is a smaller job than the comment implies but not a free one: DrawingState, SnapSettings and InclinedPlane have private fields that mod.rs and viewport.rs read directly, so their fields need widening first. CadTool and WorkPlane are fieldless and could move today. That is now written in the file for whoever has a compiler for the widget layer. 13 tests, on the properties that break quietly: - Cycling forward visits all 17 tools exactly once and closes the ring. `cycle_next` is a hand-written 17-arm match; a duplicated or skipped arm makes a tool unreachable from the keyboard and nothing else would notice. - Backwards is asserted to be the exact inverse, per tool. Shift-Tab that does not undo Tab reads as "the tool picker jumps". - Labels must be unique — two buttons reading the same is a UI bug with no test otherwise — and every tool needs a description. - `to_kind` maps only the drawing tools; Select, Delete and Measure must return None or they would create geometry on click. - `InclinedPlane::from_3_points` produces a unit normal perpendicular to both edges, and rejects collinear or coincident picks rather than returning a NaN basis from a zero-length cross product. - The plane basis is orthonormal in both branches, including the vertical-normal case that exists because the usual "up" reference is parallel to the normal there. Also fixes a real bug in this script's own drift detector: it tested membership with `case " ${ENGINE_FILES[*]} "`, and `[*]` joins on the first character of IFS, which this script sets to a newline. The pattern could never match, so the note fired for every non-widget file. It was right about tools.rs by accident. Floor: tools.rs 95. Total unchanged at 97.14% over a larger denominator. Verified: 568 tests green, every floor met, hermetic run clean.	2026-08-17 10:16:52 +00:00
andodeki	e46b2c504a	fix(cad): the scene-cache benchmarks were measuring a function that cannot cache ... `REVIEWS/PAY_CAD_IMPLEMENTATION_STATUS.md` has carried the same CAD entry through ten tranches — "unchanged", blocked on "no Cargo toolchain is available in this execution environment to run the required tests/profiles". `profile_benchmarks.rs` imports nothing from Makepad but the math types, so it builds in the same host-only harness the coverage run already uses. `CAD_BENCH=1 ./tools/test-cad-coverage.sh` runs all 16, release, no desktop, self-cleaning. Running them found the drift they exist to catch. `bench_scene_cache_hit_vs_rebuild` reported **1.2×** against the **488×** recorded in BENCH_BASELINE.md, and `bench_scene_cache_scaling` reported 1× at every part count with the warm read scaling linearly — 3.2 µs at 10 parts to 64 µs at 500. That reads as a catastrophic cache regression. It was not. Both called `SceneCache::scene(&[CadNode])`, which is documented as always rebuilding: it takes a bare slice, so it has no generation to compare against and cannot cache. The editor's caching entry point is `scene_for(&PartsStore)`. When the generation-tracked store landed in Phase 5.1 these two benchmarks were not moved with it, so their "warm" sample was a second full rebuild and the printed speedup was allocator noise. Nobody saw it because the benchmarks had not been runnable since. Repointed at `scene_for`, they reproduce the checked-in baseline on different hardware: cold 24.6 µs / warm **48 ns**, **512×** against the recorded 488×, and the warm read is flat at ~55 ns from 10 parts to 500. `bench_scene_cache_hit_vs_rebuild` now asserts `Arc::ptr_eq` across its two samples, so it fails loudly instead of quietly timing two rebuilds if it is ever pointed at a non-caching path again. `SceneCache::scene()` itself is untouched. I started to delete it as a "cacheless method on a cache" and stopped: its docstring says exactly what it does and why, and nine tests use it for precisely that case. The benchmarks were wrong, not the API. Verified: the 16 benchmarks run and reproduce the baseline; the default coverage mode is unchanged at 97.14% with every floor met.	2026-08-17 09:23:32 +00:00
andodeki	4ea1224e49	test(cad): actually exercise the GLB parallel path, 94.83% -> 98.65% ... Correcting my own test from two commits ago. `build_glb` only reaches rayon at 32 or more geometric nodes -- below that it runs an ordinary iterator. Every test in the file used one or two nodes, so: - the `par_iter()` arm, the reason rayon is a dependency at all, had never executed; and - the test named "the sequential fallback matches the parallel builder" was comparing `build_glb`'s SEQUENTIAL branch against `build_glb_sequential`. Two sequential paths. It would have passed with the parallel arm deleted. The coverage report is what showed it: those lines stayed red after a commit whose message claimed to cover them. Three tests: - 40 nodes, crossing the threshold, asserted byte-identical to the sequential builder and with mesh names still in order. `par_iter().filter_map().collect()` preserves order; `par_bridge` or a collect into a map would not, and the symptom is a model whose parts are labelled with each other's names. - The sequential builder walks a SceneVisitor whose per-variant arms are separate code from the parallel path's `collect_node`. Seven variants through it, asserting one mesh each. - Nodes that are geometric but mesh to nothing (two empty CSG results) hit the third error arm, on both paths. Without it the exporter writes a GLB with an empty buffer, which a viewer opens as a blank stage and the user reads as a successful export. Floor: arch_gltf 92 -> 97. Verified with: ./tools/test-cad-coverage.sh (555 tests green, total 97.14%)	2026-08-17 04:41:52 +00:00
andodeki	bd97e68af0	test(cad): opt-in reuse knobs so the coverage loop is usable while writing tests ... A cold run spends about 90 seconds installing a toolchain and another minute compiling printpdf before it measures anything. That is correct for CI and hostile to the person actually writing the tests, who runs it twenty times in an afternoon — and the workaround is to hand-roll a private copy of the harness, which then drifts from the committed one. Both of the last two coverage pushes were done that way. Better to support it. Three opt-in variables, none set by CI: CAD_COV_TOOLCHAIN_HOME reuse RUSTUP_HOME + CARGO_HOME CAD_COV_TARGET_DIR reuse the build cache CAD_COV_MAKEPAD reuse a Makepad checkout (already existed) With all three: 20 seconds instead of three minutes, measured. The default is unchanged and stays the only reproducible mode: everything under one mktemp directory, removed by the trap. A reused directory is deliberately NOT deleted — it lives outside $WORK by definition, and silently removing a path the caller named would be a nasty surprise the first time someone points it at the wrong thing. The toolchain check is now "is there a cargo binary here", and a reused home that was installed without llvm-tools-preview gets a message naming the component and the rustup line to fix it, rather than a "no such file" on llvm-profdata three steps later. Verified both paths against this commit: hermetic cold run and fully-reused run both report 96.86% and meet every floor.	2026-08-17 04:40:16 +00:00
andodeki	723fe019d0	test(cad): the store's generation contract, the STL trait impl, and two NaN guards ... Mop-up of three files whose remaining gaps were small but not empty. scene_holder 96.16 -> 100.00%, arch_stl 96.83 -> 98.88%, construction_geometry 95.08 -> 95.88%. scene_holder — the generation counter is the whole reason PartsStore exists: it moves on its own so `SceneCache::scene_for` can tell whether its cached scene is stale, instead of trusting every caller to remember `mark_dirty()`. That contract is per-method and nothing checked it: - Reads must NOT bump. Eight of them (len, as_slice, iter, get, find_by_raw_id, index_of_raw_id, is_empty) asserted against one generation snapshot. A read that bumps rebuilds the scene every frame -- slow, and invisible. - `get_mut` bumps only on a hit. Bumping on a miss invalidates the cache for a lookup that changed nothing. - `iter_mut` bumps unconditionally, before it knows whether the caller writes. That is the deliberate conservative choice that replaced the `as_mut_vec()` escape hatch, and it is now pinned so nobody "optimises" it into a lie. - The pairing itself: an unchanged store returns the same Arc, a bumped one rebuilds and the rebuilt scene carries the edit. - `PartIdAllocator::default()` must agree with `new(1)`. Defaulting to 0 would hand out an id that reads as "no node". arch_stl — only `build_stl` was covered, so the `Exporter` impl (the path the export buttons and the async worker take) had never run. Both arms now write the same bytes, both report a failed write, and a group node contributes nothing an empty scene would not: meshing it would add an empty solid and shift every later vertex index. construction_geometry — the two non-finite guards in `snap_to_polar_angle` and `normalize_angle_signed`. `rem_euclid` on an infinity is a NaN, so without them an infinite drag delta becomes a NaN heading and every vertex after it is NaN. Also the documented wrap-round contract at the boundary: 370 degrees behaves as 10, -30 snaps to -45 rather than 315, and pi stays pi because the range is (-pi, pi]. Its remaining 20 uncovered lines are `other => panic!(...)` arms inside existing tests. Those only execute when a test fails, so they are uncoverable by construction rather than untested. Floors: construction_geometry 92 -> 95, arch_stl 94 -> 98, scene_holder 93 -> 99, total 95 -> 96. Verified with: ./tools/test-cad-coverage.sh (552 tests green, total 96.86%)	2026-08-17 04:38:04 +00:00
andodeki	aad2a20d43	test(cad): cover the PDF plan projection, 75.92% -> 88.99% ... The last of the four exporters with the same gap: the arch projector classifies each node by LAYER NAME and then by geometry, and only the wall path had ever run. Columns, beams, spheres, generic blocks, polygons, the 2-D primitives and CSG results all reached their own `make_*` and none of them were executed, along with both public entry points. 10 tests: - The plan projection negates Z so north is up. One line, and a sign error there mirrors the entire drawing. - Cylinders become Columns and spheres become Spheres, with centre, radius and height asserted, and the label prefix checked -- the labels carry a per-type counter that the drawing schedule reads. - A box on an unregistered layer falls back to a generic Block rather than vanishing. That fallback is what keeps an unclassified part on the drawing. - A beam keeps length on size.x, plan width on size.z and thickness on size.y. Swapping any two produces a plausible-looking beam of the wrong shape. - Polygons and extruded polygons are drawn as their bounding box centred on the polygon's own centre, not the node origin -- the node is at (2, 3) and the triangle's centre is offset from it, so the test would pass either way if it only checked the size. - An empty vertex list emits nothing, rather than a zero-by-zero block at the plan origin. - `export_scene_to_pdf` writes a real `%PDF-` file into a directory it had to create, and reports a path it cannot create. The tolerances in this module are 1e-6 rather than 1e-9 on purpose: every dimension crosses f32 to f64 on the way in, and 0.3f32 as f64 is 0.30000001192092896. The first draft used 1e-9 and failed on the beam. Floors: arch_pdf 72 -> 86, total 94 -> 95. The remaining 152 lines are the printpdf emitter itself -- page furniture, dimension strings and title-block layout, whose output is only meaningfully checked by opening the file. Verified with: ./tools/test-cad-coverage.sh (540 tests green, total 96.53%)	2026-08-17 04:31:44 +00:00
andodeki	5e864498d5	test(cad): cover the GLB export entry points and every solid it collects, 75.48% -> 94.83% ... Same shape of gap as the SVG exporter, one file over. The mesh collector had an arm per solid and only boxes were ever walked through it; `scene_to_glb`, `export_scene_to_glb`, `export_scene_to_glb_with_cache` and `build_glb_sequential` -- every public entry point except the one the trait impl uses -- were at zero, along with both `From` conversions on the error type. 8 tests: - Nine solids exported one at a time, each asserted to produce a structurally valid GLB: the "glTF" magic, version 2, and a declared length that matches the file. A viewer rejects the file outright if any of those disagree, so checking "some bytes came back" would not have been worth writing. - The sequential fallback is asserted byte-identical to the parallel builder. It is documented as the path for environments without rayon; if it drifts, that fallback silently exports something else and only those environments see it. - An empty scene and a groups-only scene are both refused, with the two distinct messages. A GLB that opens to an empty stage is worse than a refusal, because the user reads it as "the export worked". - `export_scene_to_glb` creates the directory it was pointed at (the user picks the path, its parent may not exist), the shared-cache variant writes identical bytes, and both report a path they cannot create instead of dropping the export. - The error type's Display, plus its io and serde_json `From` conversions -- those exist so `?` works inside the export path, and an unexercised conversion is a `?` that fails to compile the day someone needs it. Floors: arch_gltf 72 -> 92, total 93 -> 94. Verified with: ./tools/test-cad-coverage.sh (530 tests green, total 95.42%)	2026-08-17 04:29:19 +00:00
andodeki	e16a6da5f5	test(cad): cover the real command context and the undo-stack housekeeping, 87.63% -> 96.23% ... The undo/redo tests all ran against a mock. The mock keeps its own Vec, so `CadCommandCtx` -- the implementation the editor actually uses -- had two methods that no test had ever called: `update_node`, the in-place property edit, and `insert_node_at`, the undo of a delete. `UndoRedoStack::clear`, its Debug impl, and the describe/as_any pair on half the command types were also at zero. 12 tests against the real context, with a real PartsStore: - `update_node` edits in place, bumps the store generation, and does NOT reorder the list. Order is what the layer panel and the draw order read; the same class of reordering defect is already called out in the mock's own comment. - `update_node` on a missing id reports NodeNotFound and does not run the edit closure -- otherwise a stale selection edits whatever node happens to be in that slot. - `insert_node_at` puts a deleted node back at its recorded index, not on the end, and clamps an out-of-range index instead of panicking. The index is captured before the delete and other commands may have shortened the list since. - DeleteNode and CreateNode are round-tripped through the real context, including DeleteNode's no-recorded-index arm (appends) and CreateNode's fallback from `assigned_id` to the snapshot id. Plus the trait and stack housekeeping: - The `Command` defaults: the generic "command" label, and `can_merge` returning false. A default of true would silently collapse unrelated undo steps. - `clear()` empties both stacks. The editor calls it when a document is closed; an entry surviving into the next document applies an edit to the wrong model. - The Debug impl prints depths and asserts the command list is NOT dumped -- a derived Debug over two stacks of boxed trait objects would put the whole edit history in a log line. - Every command type's describe/as_any, including that two commands with identical field shapes do not downcast into each other. That downcast is what `can_merge` runs on; a wrong one turns a drag into one undo entry per frame. Floors: commands 85 -> 94, total 92 -> 93. Verified with: ./tools/test-cad-coverage.sh (523 tests green, total 94.31%)	2026-08-17 04:27:54 +00:00
andodeki	44bf7da725	test(cad): cover the shapes the SVG exporter never drew in a test, 82.48% -> 99.02% ... Only boxes were exercised. Cylinders, spheres, circles, arcs, polygons, extruded polygons and CSG results all had their own arm in the SVG visitor and not one of them was executed — 110 uncovered lines, in the exporter that produces the construction drawing. That is the worst shape for an exporter bug: a part whose arm is wrong does not fail anything, it just is not in the drawing. Nothing is red, the file opens, and the column is missing. 13 tests: - Every drawable variant is exported on its own and must produce exactly one path. Nine variants, nine assertions. - A round outline has one point per segment, and a sphere is drawn from segments_u, not segments_v. Both show up visually as a column faceted in the wrong axis rather than as an error. - An arc is sampled inclusively across its 32 segments (33 points) so it closes on the end angle instead of stopping a step short, and a half sweep must not return to its start. - A polygon with two vertices, and an empty CSG result, add no path. An empty `points=""` renders as a stray dot in some viewers. - The `Exporter` impl itself: the cacheless `export`, the cached one (asserted byte-identical), and the write-failure arm, whose message is what the status label shows. Only `build_svg` was covered before, so a broken `export` would have shipped. - A 90 degree yaw must change the projected outline of a polygon. The box arm had rotation covered; the polygonal and round arms use a different projection helper and had none. Floors: arch_svg 79 -> 97, total 89 -> 92. Verified with: ./tools/test-cad-coverage.sh (510 tests green, total 93.44%)	2026-08-17 04:25:04 +00:00
andodeki	a82916b006	test(cad): cover the scene-graph builder API, 81.73% -> 98.53% ... cad_scene.rs is the type every other CAD file is written against, and 388 of its lines had never been executed. The gap was not in exotic corners -- it was the fluent builder the whole editor and the script VM construct scenes through. `NodeBuilder`'s setters, `SceneBuilder`'s starters and domain builders, `push_raw`, `CadTransform`'s helpers, the 2-D solids' size/set_size arms, `ParamHash` over half the variants, `walk_scene`'s dispatch, and the `Exporter` default methods were all at zero. 32 tests, grouped by what they protect: - The setters are checked for what they must NOT touch as well as what they set. `.cube().radius(9.0)` has to be a no-op, not a silent solid swap; `.rotate_y(30).rotate_y(15)` has to be 45 degrees, because every one of these helpers is additive and a helper that assigned instead would drop the earlier call. - The domain sugar is pinned to its documented axes: length/width to size.x, domain_height to size.y, thickness/depth to size.z. Getting one onto the wrong axis gives a wall 0.2 m long and 6 m thick, which reads as a modelling mistake rather than a code one. - The six domain builders are checked for layer, name and their documented default colour. Asserting the colour rather than "not the default material" is deliberate and was found the hard way: Column's grey IS the default colour, so it correctly shares the default material instead of registering a duplicate. - `set_size` is checked on every parametric solid. It is what the properties panel calls, and a missing arm is a control that does nothing -- the same class of defect the by-value-getter CI gate already guards. - `size()` on a CSG or extruded solid is checked against a real mesh bounding box, including the empty-result case. That arm used to return a hardcoded 1x1x1, which made those parts unpickable outside a 1 m box at their origin. - `ParamHash` is checked to move for every field of the 2-D and section variants. It keys the mesh cache AND the viewport's GPU buffers, so a field it does not hash is an edit that leaves stale geometry on screen. - `walk_scene` is checked to route all twelve `CadSolid` variants to their own callback, in order, with `leave_node` always firing. The exporters are all visitors: a variant landing in the wrong arm is a part that silently vanishes from the STL, the SVG or the PDF. A visitor overriding nothing is walked too, so the trait's default bodies are executed rather than assumed. - `export_to_vec`, `export_with_cache` and `spawn_export` -- the default methods an exporter gets for free, all on the async export path -- are driven through a counting stub, with the worker thread joined so the callback assertion is deterministic. Floors raised to lock it in: cad_scene 78 -> 96, total 85 -> 89. Remaining 44 lines are small accessors and defensive arms. Verified with: ./tools/test-cad-coverage.sh (499 tests green, total 92.44%)	2026-08-17 04:23:24 +00:00
andodeki	1c91d6b398	ci(cad): gate the engine coverage, with per-file floors ... The harness measured; nothing enforced. A coverage number nobody gates goes down. tools/test-cad-coverage.sh now exports llvm-cov JSON and fails when the total drops below 85% or any of the fourteen engine files drops below its own floor. The per-file floors are the point: deleting every test in persistence.rs moves the total by under two points, so a single number would wave that through. Each floor sits a couple of points under today's measurement, so refactoring does not trip it and a real loss does. The low floors are the honest ones. arch_pdf (72) and arch_gltf (72) have gaps in byte-layout paths that only a real PDF or GLB consumer reaches; arch_svg (79) and cad_scene (78) have gaps in widget-facing helpers and defensive arms on invariants SceneBuilder already enforces; exporters (88) cannot reach the save-dialog branch without a windowing system. Raising those needs work, not a bigger number here. Also in this commit, from running the script the way CI will rather than with a warm local checkout: - the Makepad fetch is sparse + blobless + depth 1 over the actual path-dependency closure (math, csg and its six siblings, micro_serde, its derive, micro_proc_macro, live_id, id_macros). 29 MB and two seconds instead of a 319 MB checkout of a repository that is mostly shaders, fonts and demos. Two of those crates were found by the run failing at manifest-read time, which is why the script now verifies all thirteen manifests exist before building instead of trusting the sparse pattern. The new cad-engine-coverage job needs no native packages and no GPU -- makepad-math and makepad-csg are dependency-free Rust, which is the whole reason the engine can be measured at all. It installs its own toolchain into a temp dir and deletes everything through a shell trap: nothing cached between runs, nothing left in the workspace. Verified end to end with a cold run: fresh toolchain, fresh sparse fetch, 466 tests green, total 88.75%, all floors met, environment cleaned.	2026-08-16 22:26:42 +00:00
andodeki	24a26f052e	test(cad): host-only coverage harness for the CAD engine ... The CAD module had 411 tests and no way to find out what they miss. `cargo test -p nigig-build` needs the full Makepad desktop stack -- wayland, X11, GL, alsa, polkit -- so nobody had ever run it under instrumentation, and "well tested" was an assertion, not a measurement. Fourteen of the module's twenty-six files are pure: geometry, the scene graph, undo/redo, the four exporters and file I/O. Their only Makepad imports are the math types, the CSG library and two log macros, all of which are dependency-free Rust. This script copies those fourteen into a temporary crate that carries the SAME module path (`nigig_build::construction_frame::pages::workspace::cad::*`), so the sources compile byte-for-byte with no edits, and runs them plus the real tests/cad_integration.rs under `-C instrument-coverage`. Baseline on this commit: 84.41% of lines over the fourteen engine files and the integration suite. math.rs is 20.40% and persistence.rs is 0.00%. Excluded from the report, per the coverage plan: the Makepad checkout (vendored/generated upstream code), the cargo registry and git caches, the rustc sysroot, and the harness's own lib.rs/shim/picker -- the platform-startup stand-ins the script writes itself, which are scaffolding and not CAD code. The exclusion is enforced twice, by -ignore-filename-regex and by an explicit source list, because the regex alone breaks when CAD_COV_MAKEPAD points outside the temp dir. What it does NOT measure, and does not pretend to: mod.rs, viewport*.rs, workspace*.rs, script_bindings.rs, cad_editor_sheet.rs, code_editor.rs, tools.rs and profile_benchmarks.rs. Those need live_design!, Cx and an event loop; the full-crate-check job in nigig-build.yml gates them. Everything -- toolchain, cargo home, target dir, profraw data, the fetched Makepad tree, the report -- lives under one mktemp directory removed by a shell trap on success, failure, interrupt or termination. The two enums the integration suite borrows from the widget-bound mod.rs are extracted from the real file at run time rather than copied, so the harness cannot silently drift from the crate.	2026-08-16 22:16:56 +00:00