18 commits

Author	SHA1	Message	Date
andodeki	ac8f8aa002	chore: sync full working tree to gitdab ... Whole-tree sync: cad-core/cad-ui split sources, nigig-build construction_frame migration, pdf port progress, mpesa/pay/uikit/doc updates, workspace members/profiles/lock, CI workflows and reviews. See individual file history for details.	2026-09-12 07:15:24 +03:00
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	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	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
Arena Agent	d61e215e9e	perf(cad): batch the 2D grid into one stroke (3.9); measure 3.3 and 3.8 ... Phase 3.9 -- the real cost was not the "nice number" step computation the plan named. That is already a cheap if-else chain, no log10/pow. It was draw_dashed_line calling stroke() after every 6px dash, and stroke() tessellates the entire accumulated path each time. A full-screen grid is ~50 lines of ~108 dashes: 5,400 tessellations per frame at 1080p, 14,688 at 4K. Split into queue_dashed_line (appends to the path) and draw_dashed_line (queues, then strokes) so single-line callers are unchanged. The grid queues every dash and strokes once. Bubble markers are collected and drawn after, not inside the loop -- they use draw_text and their own fills, which would otherwise land in the middle of the grid's path. Also one String allocation per grid line instead of two. the_2d_grid_strokes_once_not_once_per_dash pins it. My first version asserted exactly one stroke in the whole function and failed with 3: the work-plane cross below the grid is a separate feature with its own colour and correctly gets its own strokes. Scoped the assertion to the grid rather than weakening it. Negative test: swapping one queue_dashed_line back to draw_dashed_line fails it. --- Phase 3.3 (memoise ParamHash on CadNode): MEASURED, NOT DONE. Box: 50 ns/node -> 25 us/frame at 500 parts Extruded 64-gon: 987 ns/node -> 493 us/frame The Box case does not justify a cached field that every mutation would have to invalidate -- the exact hazard Phase 5.4 removed from part_geoms. The polygon case is the vertex data itself: I tried bulk-hashing the slice as raw bytes and measured 125 us vs 128 us for 500 x 64 verts, i.e. nothing. The only real fix is to give polygons an Arc identity the way Csg already has, which is a data-model change, not a cache. Benchmark kept so the next person starts from numbers. Phase 3.8 (cost estimate instead of node count): MEASURED, NOT DONE. 200 nodes, cold cache: 8.80ms seq / 6.21ms par -> 1.42x 200 nodes, warm cache: 5.81ms seq / 5.73ms par -> 1.01x On a warm cache -- the common case, since the preview renderer has already built every mesh -- parallel neither helps nor hurts. A cost estimate would have to hash every node to count cache misses, in order to choose between two paths that differ by 1% in the case it would most often face. The threshold comment now carries these numbers instead of "can be tuned based on real-world profiling". 783 lib + 154 integration tests pass. All 13 CI gates pass.	2026-08-02 10:13:13 +00:00
Arena Agent	bdf882b817	perf(cad): regenerate the parts script on drag commit, not per frame (3.6) ... `script_dirty` was set on every MouseMove and FingerMove of a part drag. That makes `sync_parts_from_any_dirty_viewport` call `generate_parts_script()` -- formatting every part into a String -- and the workspace then replaces the entire editor document via `set_editor_text_all`. Per motion event. Measured before changing it: 42 us at 50 parts, 170 us at 200, 425 us at 500, and that is the string formatting alone, before the code editor's own work. See bench_parts_script_regeneration_per_drag_frame. The reason it was set mid-drag no longer holds. The comment said it kept the split 2D/3D viewports in sync while dragging -- true when each viewport owned its own parts list, but since Phase 5.2 all three share one CadDocument. A move IS their state the moment it happens; they need a repaint, not a resync, and they get one. Both commit paths already set the flag: the MouseUp arm for mouse drags, and finish_part_drag for touch (reached from three places). So the script still regenerates exactly when it needs to -- once, when the edit is final. a_drag_regenerates_the_script_on_commit_not_per_frame pins it. It walks every arm that calls move_selected and asserts none of them set script_dirty, then asserts the commit paths still exist -- because the failure mode of this change is not "slow", it is "the script never updates at all", and a test that only checked the first half would miss it. Negative test: putting the assignment back fails it with the line number. 782 lib + 154 integration tests pass. All 13 CI gates pass.	2026-08-02 10:05:45 +00:00
Arena Agent	73c4c49cb9	perf(cad): cache the world AABB per placement -- 230us -> 39us (Phase 3.2) ... Measured before building, because I had previously dismissed this item as "smaller" without checking. It is 5.9x at 500 parts, and hover picking runs on mouse-move, so it is a per-frame cost. pick_part's broad phase transformed 8 local corners by the model matrix for every part on every pick. Phase 5.3 had already removed the expensive half (it no longer re-meshes to read bounds), leaving 8 matrix multiplies per part -- cheap individually, 230 us/frame at 500 parts. SceneCache::world_aabb_for now memoises the result. The key is a NEW type, PlacedHash, not the existing ParamHash. This is the whole subtlety of the change: ParamHash deliberately excludes the transform, because a local-space mesh cannot change when a part moves (Phase 5.4). A world-space AABB is exactly the opposite -- moving the part is the entire point. Reusing ParamHash here would serve a stale box after every drag and make parts unpickable at their new position, which is the picking equivalent of the stale part_geoms bug. Making it a distinct type rather than "ParamHash plus a flag" means the two cannot be confused at a call site. a_move_invalidates_the_world_aabb_even_though_it_keeps_the_mesh pins the asymmetry directly: the same move that rebuilds the AABB must still hit the mesh cache. Negative test: making PlacedHash ignore the transform -- i.e. reverting it to ParamHash -- fails that test. Restored and green. retain_world_aabbs is paired with every retain_meshes call site, for the same reason that one exists: the map is keyed by NodeId and nothing drops an entry when its node is deleted, so without it the map grows for the session. 775 lib + 154 integration tests pass. All 13 CI gates pass.	2026-08-02 09:58:23 +00:00
Arena Agent	5ffc515f91	perf(cad): build the command scene snapshot lazily -- 254us -> 0.8us ... CadCommandCtx::new eagerly called scene_cache.scene_for(parts), which is O(nodes): every node cloned, every material re-registered. No production command reads that scene -- MoveNode, ResizeNode, RotateNode, YawNode, ModifyNode, CreateNode and DeleteNode all address a node by id. Only tests and the benchmark call ctx.scene(). Every command mutation bumps the PartsStore generation, so the next context construction was a guaranteed cache miss. move_selected issues one command per selected part per frame, making a drag O(commands x nodes) for work that is O(1) per command. This arrived with Phase 5.1, when the generation counter turned a previously accidental cache hit into a guaranteed miss; the 0.4us baseline predated it. The snapshot is now built on the first scene() call and memoised. CommandContext::scene() returns Arc<CadScene> rather than &CadScene so an implementation can build on demand instead of keeping one alive for the context's lifetime. This was also a latent CORRECTNESS bug, not only waste. The eager snapshot was captured BEFORE the command ran, so a command that mutated and then read scene() saw its own edit missing. Every mutating method now drops the memo; those invalidate_snapshot() calls are placed directly beneath the existing scene_cache.mark_dirty() calls so a new mutator cannot silently miss one. Two tests, both verified to fail against the old code: - a_command_that_never_reads_the_scene_does_not_build_one asserts on SceneCache::rebuild_count (a new #[cfg(test)] counter) rather than wall-clock, so it is deterministic rather than machine-dependent. Fails 20-vs-0 when construction is made eager again. - a_lazily_built_scene_reflects_edits_made_earlier_in_the_same_context reads the scene BEFORE mutating, then again after. Reading only after the mutation passes even with invalidate_snapshot() gutted -- the lazy build simply happens later -- so the first read is what gives the test teeth. I checked: the obvious version of this test was vacuous. The benchmark was also measuring the wrong thing. It called ctx.scene() each iteration to read the start position, which no production path does: move_selected and finish_part_drag both read p.pos() off the parts list. Fixing the lazy build alone moved undo 228us -> 0.5us but left execute at 231us, because the benchmark was timing its own scene read. It now mirrors the production callers. Measured: execute 254us -> 0.8us, undo 246us -> 0.5us (1000 commands over a 1000-node scene), ~300x. No other benchmark regressed. 755 lib + 154 integration tests pass. Test-name list diffed: +2, nothing dropped. All 12 CI gates pass.	2026-08-02 08:57:10 +00:00
Arena Agent	11ef0fbf67	fix(cad): GPU buffers self-invalidate; stop re-meshing on every drag frame ... Phase 5.4. Two defects with one root cause: part_geoms was keyed on the raw node id, so staleness was invisible to the type and correctness rested on seventeen scattered `part_geoms.remove(&id)` calls at the edit sites. All seventeen are deleted; the map is now keyed on (id, ParamHash), the same key MeshCache uses, and an entry whose hash no longer matches its node is simply never read. 1. subdivide_selected drew the wrong geometry. It replaced each selected part's solid with a fresh Csg and invalidated the MESH cache, but never removed the part's part_geoms entry -- so the stale uploaded buffer stayed a hit and the viewport kept drawing the un-subdivided shape. An audit of every mutation path found this was the only edit site missing its manual eviction, which is exactly the failure mode a manual protocol produces. 2. ParamHash covered the transform and the material, so a pure move invalidated the mesh. It should not: build_mesh reads neither. The cached TriMesh is in the node's LOCAL space, and all four consumers -- the draw loop, pick_part, the STL and glTF exporters -- apply the model matrix themselves. Dragging a part therefore re-triangulated it on every frame to produce byte-identical triangles, at up to 886 ns per extruded part per frame, per selected part. The hash now covers the solid parameters and nothing else. Two existing tests asserted the old behaviour and were INVERTED, not deleted -- they described what the code did rather than what it needed to do: cache_detects_transform_edits_via_param_hash -> a_transform_edit_reuses_the_cached_local_space_mesh mesh_cache_self_invalidates_on_parameter_and_transform_edits -> mesh_cache_self_invalidates_on_solid_parameter_edits New: build_mesh_output_does_not_depend_on_the_transform guards the assumption the key now rests on -- if anyone makes build_mesh bake the transform in, it fails and the key must grow it back. Plus mesh_cache_hits_on_a_pure_transform_edit, mesh_cache_hits_on_a_material_edit and an_uploaded_buffer_is_not_reused_after_the_solid_changes. Negative test: restoring the transform to the hash makes the first two fail; restored and green. Deletion is still explicit (`retain` on the live id set) because it is the one case a content hash cannot express -- there is no node left to hash. ParamHash is pub(crate), not pub: it is how the caches agree on staleness, not a consumer contract. Also recorded in BENCH_BASELINE.md, not fixed here: bench_command_execute _overhead is 227 us/cmd against a stale recorded 0.4 us. Verified pre-existing -- 254 us on the commit before this branch, so this work slightly improves it. CadCommandCtx::new eagerly builds a scene snapshot that most commands never read, and every command bumps the generation, so it is O(commands x nodes). The fix is to make that snapshot lazy; it is a separate change and gets its own commit. 753 lib + 154 integration tests pass. Test-name list diffed, not just the count. All 12 CI gates pass.	2026-08-02 08:44:42 +00:00
Arena Agent	8af41b3310	perf(cad): evict dead meshes instead of clearing the whole cache ... ARCHITECTURE.md invariant 2 has said since it was written that the scattered clear_mesh_cache() calls are "unnecessary and expensive", because MeshCache keys on (NodeId, ParamHash) and an edited node invalidates itself. Phase 5.2 removed the first one. This removes the rest: 8 of the 11 live call sites, leaving only invalidate_all (whose contract is exactly that) and the definition. Investigating them turned up something the invariant did not say, and which makes the cleanup a bug fix rather than tidying: MeshCache has NO eviction for deleted nodes. Entries are keyed by NodeId and nothing ever removes one when its node goes away. The wholesale clears were, accidentally, the cache's only garbage collector. Deleting them naively would have converted a performance problem into an unbounded memory leak over a session. So the fix is a targeted replacement, not a deletion: MeshCache::retain, SceneCache::retain_meshes and CadViewport::evict_dead_meshes drop exactly the entries whose node is gone and leave every live one warm. Each call site was then classified rather than pattern-matched: - deletes (delete_selected, delete_part, CommandContext::delete_node) -> evict dead entries - pure additions (create_node, insert_node_at, paste, and the wall / circle / rect / area / column / beam / polygon / polyline tool completions) -> nothing at all. A new node has no cache entry to invalidate and cannot affect any other node's mesh. These were discarding the entire scene's meshes to make room for nothing. - subdivide_selected -> invalidate only the selected ids. It builds a fresh Arc<Solid> per part, so ParamHash already differs; the unselected parts were being thrown away for no reason. Measured: deleting 1 of 100 parts costs 62.1 us with clear+rebuild versus 21.9 us with evict+reuse, a 3x saving on every delete. Two tests, both negative-tested by stubbing out the retain body: retain_nodes_evicts_dead_entries_and_keeps_live_ones checks both directions (dead gone, live still an Arc::ptr_eq hit), and cache_does_not_grow_past_the_live_node_set simulates 50 add/delete rounds and asserts the cache holds 1 entry, not 50. With retain stubbed it holds exactly 50, which is the leak. Also documented a sharp edge found while reading ParamHash: it hashes a Csg node's Arc POINTER, not its geometry. Re-wrapping identical geometry in a new Arc is therefore a safe false-positive, but mutating a Solid behind a shared Arc would go unnoticed. Nothing does that today and ARCHITECTURE.md now says it must stay that way. 624 lib + 154 integration, 0 failed.	2026-07-28 20:12:55 +00:00
Arena Agent	2e0a3e9f81	refactor(cad): route GPU uploads through MeshCache (Phase 5.3) ... The module had two independent paths from a node to triangles. ensure_part_geometry called part.build_solid() directly; the exporters and pick_part went through MeshCache::get_or_build -> CadSolid::build_mesh(). Neither saw the other's work, so a part that had just been exported or hovered was still re-meshed from scratch for the GPU upload. Proved they agree before merging them. pipeline_equivalence_tests builds all 12 CadSolid variants through both paths and compares vertex positions, triangle indices and winding exactly. They match. A second test asserts the variant list has 12 entries, so adding a CadSolid variant without covering it fails the build rather than silently narrowing the guarantee. Both negative-tested: swapping width/height in build_solid's Rect2D arm fails with "Rect2D: vertex 0 differs: (-1.5, -2, 0) vs (-2, -1.5, 0)"; dropping a variant from the list fails the coverage test. The change itself: build_mesh_buffers now takes &TriMesh rather than &Solid -- it only ever read solid.mesh() -- and ensure_part_geometry pulls from the shared MeshCache. Added cad_mesh_data_from_mesh and part_mesh_buffers_from_mesh as the mesh-taking entry points; the Solid-taking ones remain as thin wrappers for callers that hold a Solid. Measured, and the number is smaller than the section title suggests: 2x on a warm cache (boxes 0.27 -> 0.178 us/part, extruded 24-gons 1.11 -> 0.516 us/part), and *negative* on a cold one -- 0.79 vs 0.27 us/part, because a first build now pays a ParamHash and a map insert. That is recorded in BENCH_BASELINE.md with a note not to "optimise" it back by special-casing cheap primitives. The reason to do it anyway is correctness, not speed. Two match arms over the same enum drift, and this drift would have been invisible: no crash, no failing test, just a preview that quietly disagreed with the exported file. Now there is one pipeline and a test that fails if it forks again. build_mesh and build_solid both still exist because a Csg node needs a real Solid to compose with. ARCHITECTURE.md's "Two meshing pipelines" section, which told readers to apply every meshing change twice, is now "One meshing pipeline". 615 lib + 154 integration, 0 failed.	2026-07-28 19:11:45 +00:00
Arena Agent	676dbaf19f	perf(cad): stop resyncing viewports that already agree (Phase 5.2, first increment) ... Measured first. bench_viewport_snapshot_sync_per_frame puts the cost of one sync frame at 98.5 us for 100 parts: 21.2 us of Vec<CadNode> deep copies and 77.2 us of forced scene rebuilds. A generation comparison over the same data is 0.051 us. Dragging a part sets script_dirty on every MouseMove, so that was the steady-state per-frame cost of a drag. Three separate wastes, each removed: 1. The source viewport was written back to. sync took a snapshot from the dirty viewport and then handed it to all three, including the one it came from -- a deep copy plus a full cache reset to install state that viewport already had. It now pushes to the other two only. 2. Destinations were re-pushed unconditionally. They now compare parts_generation() against the source's and skip if equal. During a drag the same list was being reinstalled every frame; a frame where nothing changed now costs three integer comparisons. 3. replace_parts_snapshot cleared part_geoms and the entire mesh cache. The part_geoms.clear() was the expensive one and it is not in the benchmark: it dropped every uploaded GPU buffer, so the next draw had to re-mesh and re-upload the whole scene through ensure_part_geometry. Now retains geometry for ids that survive the replacement. The clear_mesh_cache() was redundant. MeshCache keys on (NodeId, ParamHash) and ParamHash covers the solid discriminant, all its parameters and the full transform, so a node whose geometry changed misses on its own. ARCHITECTURE.md invariant 2 has said this was unnecessary since it was written; this removes the first of them. Three tests, each negative-tested by reintroducing the defect: - mesh_cache_self_invalidates_on_parameter_and_transform_edits proves the claim that justifies dropping the wipe. Deleting the transform hashing from ParamHash makes it fail. - replace_all_bumps_the_generation_so_destinations_can_compare pins the precondition for the skip. Removing the bump makes it fail, which is the failure mode that matters: a stale generation would make the sync silently drop a real edit. - geometry_is_retained_for_surviving_ids_only covers the retain predicate. Geometry needs a live Cx, so this tests the id set logic -- keeping a buffer for a deleted id, or dropping one for a survivor. Also collapsed six copies of the widget-borrow chain into with_viewport, taking a callback rather than returning a guard: the WidgetRef is a temporary, so returning a borrow of it does not compile (E0515, the same shape as the let-else bug fixed in cf29ba7). Not done: the three viewports still hold three copies of the parts list. Sharing one Rc<RefCell<CadDocument>> and deleting the snapshot pair is the rest of 5.2. This bounds the cost of the copy in the meantime, and invariant 4 now says so. BENCH_BASELINE.md records that this benchmark measures the primitives, not the widget method, so its numbers do not move when the sync path is fixed. It sizes the prize; the behaviour tests verify the change. 613 lib + 154 integration, 0 failed.	2026-07-28 18:50:11 +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