140 commits

Author	SHA1	Message	Date
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	42b1e7152a	docs(cad): merge a second render review into the plan, and correct the hash both reviews got wrong ... A second optimisation plan arrived (frustum culling, geometry merging, GPU instancing, octree/LOD). Rather than run two plans, every claim in it was checked against the tree at 14aa0d5 and merged into REVIEWS/CAD_RENDER_OPTIMISATION_PLAN.md with a verdict and the evidence behind it. Three of its ideas change what the phases will do; two would have made things worse; one of them found nothing, but reading it found something in mine. The correction that matters, and it lands on my own plan: ParamHash::from_node hashes node.id FIRST (cad_scene.rs:1871), before a single geometric parameter. My Phase 2 said "parts with equal ParamHash are geometrically identical by definition -- that is what the hash means". True and vacuous: equal ParamHash also means the same node. Two identical columns hash differently, so re-keying part_geoms by ParamHash -- the central move of that phase -- would have shared exactly nothing. The incoming plan inherited the same assumption from the same type name, which is the kind of coincidence that argues for a test rather than a paragraph, so there is now one: param_hash_is_not_a_shape_key_it_includes_the_node_id. The id is not a bug. MeshCache and part_geoms are both keyed by NodeId and only ask "is this entry still valid for this node", where the id is a constant. Sharing geometry needs a second hash, ShapeHash: the same body without the id. Phase 2 now carries that instead. Adopted from the incoming plan: - Frustum culling in 3D. SceneState3D { view, projection } is already captured into self.last_view/last_proj every frame (viewport.rs:3781) immediately before draw_scene, so the planes are a row add/subtract away -- and deliberately not via mat4_inverse, which had a mistyped index in all sixteen cofactors until 2ea5a74. My Phase 1 had only an AABB-versus-rect test, which is right for 2D and wrong for perspective. - Instancing, upgraded from "confirm the machinery exists" to "here it is". DrawPbr::begin_many_instances_for_mesh / push_many_instance_with_transform / end_many_instances (draw/src/shader/draw_pbr.rs:2696-2745 in the pinned fork) do exactly this for a shared mesh geometry, and per-instance data is just the #[live] fields after #[deref] draw_vars -- the layout DrawCadMesh already has (color, transform, depth_clip, display_mode). No shader rewrite; ~30 lines ported. The incoming plan rated it 7 days and high risk with "if supported"; it is neither. DrawCadMesh is also declared alpha_blend: false, so regrouping draw order is safe here. - Its dependency graph: instancing batches per geometry, so shared geometry genuinely gates it. Rejected, with reasons in the doc: - bounding_sphere as a stored field on CadNode. 77 construction sites, and this codebase has already paid for hand-maintained derived state once -- the part_geoms staleness bug (viewport.rs:4629) drew the pre-edit shape because one edit site forgot to invalidate. The sphere is two lines from SceneCache::world_aabb_for, which is keyed by PlacedHash so it invalidates itself on a move or a resize, and is benchmarked at 4.72x. - Merged vertex buffers per PartKind with transforms baked in. MeshCache stores local-space meshes on purpose ("the transform and the material are NOT hashed"); baking transforms inverts that and turns dragging one column into a full re-concatenate and re-upload of every column, every frame of the drag. It also cannot express per-part selection colour. That phase's own draw loop still sets a transform per part, which is still N draw calls -- the two halves contradict each other. - Octree above 500 parts. The cached world-AABB pass is 20 us at 500 parts, 0.12% of a 16.7 ms frame. A tree that replaces 20 us cannot pay for itself. Revisit on a measurement, not a part count. - "Log skipped-part count per frame" as verification. Phase 0 already built the counting seam; a log line nobody reads is a step that cannot fail. Also corrected REVIEWS/CAD_DRAWCALL_STRATEGY_ANALYSIS.md, which is where the ParamHash claim originated: section 0 is now two corrections rather than one. Verified: tools/test-cad-coverage.sh green, total 97.20%, all floors met, cad_scene.rs 98.53%; rustfmt clean; git diff --check clean.	2026-08-20 21:17:07 +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
arena-agent	b478945c34	ci(doc): gate the doc-workspace coverage floor on every push ... Adds the doc-workspace-coverage job to the nigig-build workflow, mirroring the CAD gate: checkout, then ./tools/test-doc-workspace-coverage.sh, which installs its own instrumented toolchain into a shell-trap-cleaned temp dir and fails if the total floor (92%) or any per-file floor is not met. The script joins the workflow's push/PR path filters next to tools/test-cad-coverage.sh so edits to the harness itself re-run the gate. The doc README gains the milestone section recording the 28.55% -> 96.76% line measurement, the honest exclusions (widget layer, persistence write-path wrappers, defensive traversal guards) and the behavior pins and defect fixes the drive surfaced.	2026-08-17 10:25:27 +00:00
arena-agent	949cf24189	test(doc): doc-workspace coverage harness with enforced floors ... tools/test-doc-workspace-coverage.sh measures line+region coverage of the doc module's pure layer the same way the CAD gate does: it copies the dependency-free sources (advanced_json, crdt_bridge, mobile_gesture, persistence, projection_layout, projection_session, and the collaboration/, editing/, layout/, model/, plugins/ trees) plus tests_pure.rs into a temporary host-only crate with the real module path, satisfies the five makepad-math symbols the pure layer uses through a 30-line makepad-widgets shim, runs the suite under -C instrument-coverage with a toolchain it installs itself, and enforces a total floor plus a per-file floor for every instrumented file. The per-file floors are the point: a lone total waves through the silent loss of one whole file's tests. Measurement moved from a 28.55% line baseline to 96.76% (6170 lines) with the tranche in the parent commit; floors sit a few points under per file, except persistence.rs (55%), whose three write-path entry points write into the host's real application-data directory and are covered through their path-injected seams instead -- the honest exclusions, the exact table, and the two defect fixes this drive surfaced (ReplaceBlockRange validation order, dead RgaText::visit_children) are written down in the module's new COVERAGE.md. Everything the script touches -- pinned toolchain, cargo home, target dir, fetched Makepad tree, profraw data -- lives under one mktemp dir removed by a shell trap on every exit path; nothing lands in the repo or $HOME unless KEEP_COVERAGE=1 is set for a debugging run. DOC_WS_COVERAGE_REPORT_ONLY=1 measures without gating.	2026-08-17 10:25:27 +00:00
arena-agent	71c31cd19f	test(doc): host-only suite split + pure-layer coverage tranche ... Split the doc module's tests in two so the dependency-free majority can also run under coverage instrumentation on a host-only crate: * tests_pure.rs (new) holds every test that needs no Cx -- model, layout, editing, collaboration, advanced JSON, projection layout/session, CRDT bridge, persistence seams, mobile gestures -- and is the file tools/test-doc-workspace-coverage.sh copies byte-for-byte into its harness. * tests.rs keeps the widget-runtime and boot tests; shared helpers (projection_table_engine, only_table) live in tests_pure so both suites use them. On top of the split, this tranche adds ~80 tests covering the pure layer's real gaps: every Command apply arm and its inverse (text, atoms, blocks by stable id, image properties, block ranges, table cells/rows/columns, merges/splits/restores, node insert/delete/ replace), the DocumentController's CRDT typing lifecycle (insert/replace/delete ranges, backspace/forward delete, position sync), remote operation classification (Applied/Duplicate/Deferred/Rejected), tombstone compaction at a peer-acknowledged frontier, a two-peer MemoryTransport conversation, typing coalescing and history limits, the cell-text editing and multi-line geometry helpers in projection_layout, table cell cursor/range/merge queries, and the small model/session/selection behaviour surface. Two defects found while writing the tests, fixed with pins: * Command::ReplaceBlockRange validated a caller-supplied block_ids length AFTER draining blocks and legacy ids out of the document, so a malformed (remote) command destroyed content before reporting failure. Validation now runs before any mutation; a test proves a rejected replace leaves blocks, ids and order untouched. * model::crdt::RgaText::visit_children was dead code -- a String-collecting duplicate of visit_atoms with no callers. Removed. Two semantics that were folklore are now pinned with inline reasoning: a multi-peer conversation only converges when each peer owns a distinct document.crdt.local_actor (the two-controller sync test assigns alice/bob), and a mid-range replace_range_crdt renders its replacement after the tombstoned subtree it replaced, because RGA sibling order walks by atom id ("hello" -> "hloY" is asserted, not assumed). cargo test -p nigig-build --lib: 1031 passed, 0 failed.	2026-08-17 10:25:27 +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	33ef24cee5	refactor(cad): one screen-to-world path, not two ... viewport.rs carried two independent implementations of the same geometry. `screen_to_ray_3d` and `screen_to_world_3d` derive the camera basis from yaw, pitch, distance and a 42-degree fov; `camera_eye` and `unproject_point` did the same job by inverting `last_view` and `last_proj`. The matrix pair was dead — nothing in the workspace called either of them, and no script binding registers them by name. Two implementations of one piece of geometry, one of them never executed, is exactly how axis conventions drift apart. The evidence is in the deleted code: `camera_eye` carried its own spherical "fallback" that was a copy of `compute_eye`, complete with a "FIXED: was +cp*cy (must match makepad XR convention)" note about a convention the live copy had already been corrected for. A second copy of a convention is a second place to forget to fix it. So the dead pair goes, and a breadcrumb comment in its place says where screen-to-world actually lives — the question someone will have when they find `mat4_inverse` and wonder why nothing calls it. `math::mat4_inverse` stays. It is correct and covered now, and using the matrices the renderer actually drew with is the better way to unproject than re-deriving the camera basis from Euler angles — that is a real improvement for whoever wants it, and they should start from a version that works. Its doc comment no longer claims callers it does not have. VERIFICATION, stated plainly: `cargo check -p nigig-build` needs the Makepad desktop stack and does not run in the environment this was written in. What did run: rustfmt parses both files (a syntax error would be a parse failure, not a diff); a brace/paren delta count over the deletion (10 opens, 10 closes; 31 parens each way); a repo-wide grep for both names across every file type, which finds only comments; and the engine coverage suite, unchanged at 97.14% with every floor met. The compile is gated by full-crate-check in CI, which is where a missed reference would surface — loudly, and immediately.	2026-08-17 09:12:03 +00:00
arena-agent	dd8cc17b75	fix(doc): re-float the clipboard menu when a selection-handle drag ends ... On mobile the native menu floated at long-press word-select (or select-all) but dragging either handle afterwards re-anchored nothing, leaving the platform toolbar over the previous word — or already dismissed — once the selection had moved. The gesture router's end() only distinguishes PendingLongPress, so the Stop arm now samples the router state first: when the ended gesture was AdjustingStartHandle or AdjustingEndHandle and the session is in Edit mode, the menu re-floats on lift-off through the same cx.show_clipboard_actions request the long-press arm sends, with rect = the adjusted selection's handle union (the existing clipboard_menu_rect). Mid-drag stays quiet (the TextInput cadence DEVICE_VERIFICATION 3.3 documents) and View mode keeps handle drags as pure highlight/merge surface. Tests (2 new): a runtime drive of long-press 'hello' -> end-handle drag onto 'w' in 'world' -> lift-off asserts no request mid-drag, a fresh request on Stop whose rect covers more than the stale word rect, and the focus atom on the dragged-to glyph; the View-mode twin asserts the span adjusts while clipboard_menu stays empty. DEVICE_VERIFICATION gains row 3.6 for the hardware pass.	2026-08-17 06:28:44 +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	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
arena-agent	228bc2c81f	ci(doc-engine): gate the engine coverage, and note it in the doc README ... New coverage job runs tools/test-doc-engine-coverage.sh on changes to crates/apps/doc/**, the script itself, or the workflow. A coverage number nobody gates goes down; the floors (total plus per-file) are the enforcement. The doc workspace README records the milestone and the two CRDT-tolerance behaviors the new tests pin.	2026-08-17 04:33:08 +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
arena-agent	d62cc13d34	style(nigig-build): cargo fmt the two test targets left unformatted ... tests/ui.rs and tests/cost_estimator.rs drifted out of rustfmt shape in the recent feature/merge series, failing the whole-crate fmt gate (cargo fmt -p nigig-build -- --check). Mechanical reformat only.	2026-08-17 04:17:36 +00:00
arena-agent	ff0cba1b80	fix(project): replace retired SpreadsheetGrid::select_cell with set_selection_pair ... spreadsheet-ui 6a3c467 removed the direct selection helper, leaving apply_spreadsheet_op calling a method that no longer exists — the nigig-build lib (and therefore every cargo gate) stopped compiling. set_selection_pair((r, c), (r, c)) is the API the same migration series already adopted (8fe7db2); both call sites redraw the view right after, covering the paint the removed helper used to trigger.	2026-08-17 04:17:36 +00:00
arena-agent	5e5adf962d	fix(doc): boot the CRDT editor with content and migrate persistence to the app-data store ... The Android APK (pageflipnav) booted the doc workspace to a blank page. Two compounding causes, both invisible to sandbox gates: - CrdtDocEditor (the active editor since the navigation switch) had no boot init: it starts from DocumentController::default() and only the legacy DocEditor seeded the showcase document behind its initialized gate. The first event on a factory-fresh editor now runs init_document: load the on-disk save when it decodes as #MP_CRDT_V1 wire (initial_document_source gates that so classic-format saves stay with the legacy workspace's first-edit migration), otherwise seed_demo_doc builds a CRDT mirror of demo_doc_blocks() -- styled headings, accent runs, divider, image node, the 4x3 table with bold header, and the closing hint. set_engine flips the same flag so a host-installed document is never overwritten. - persistence.rs resolved its save file under env!("CARGO_MANIFEST_DIR"), baking the build machine's absolute source path into the binary; on device that path does not exist, so Open read nothing and Save wrote nowhere (the errors were swallowed), and on desktop the app polluted its own checkout. Writes now go only to app_data_dir()/nigig_build_store/generated/current.doc.json (the crate-wide convention the CAD store already uses); reads keep a one-way fallback to the legacy source-tree file so an unreplicated developer save is honored once. Boot and migration emit [DOC_TRACE] lines so a device logcat session names the branch that fired. Tests (8 new): boot-source gate (CRDT wire boots verbatim; classic JSON and None route to the demo seed), runtime boot on first event (source-agnostic non-empty projection + flag), host-installed-engine no-overwrite guard, full structural assertion of the seeded showcase, and four temp-dir persistence tests (round trip, store-beats-manifest precedence, manifest fallback, empty-file rejection). DEVICE_VERIFICATION.md gains the matching section-9 hardware rows (9.0 fresh-install demo boot, 9.3 classic-save coexistence).	2026-08-17 04:17:36 +00:00
andodeki	d15a034797	test(cad): cover the async export path and the model-name env read ... exporters.rs 76.30% -> 91.17%, constants.rs 82.69% -> 97.30%. `spawn_export_to_target` was 0% -- the entire function. It exists because every export used to serialise on the UI thread into a fixed path, so a 1,000-part GLB froze the editor and the second export of a session destroyed the first. The replacement had never been executed by a test: not the worker thread, not the BufWriter flush, not the callback. Four tests, driven by a stub Exporter over an empty scene, each blocking on the callback through a channel so the assertion is that `on_done` actually fires on the worker thread: - success: the bytes land at the path, and the status message names the byte count and the destination. The old fixed-path exports had nothing to assert here -- the destination was not a parameter. - exporter failure: reported as "export failed: ...", and NO file is created. A half-written export that reports success is worse than no export. - undeliverable: parent is a regular file, so create_dir_all fails and the message says so. - a directory in the file's place: the other side of the create_dir_all guard, where the write itself fails. Still uncovered in exporters.rs: the ExportTarget::Prompt arm, 28 lines. It raises a native save dialog; there is no windowing system in a coverage run and the harness's picker stub deliberately refuses rather than faking a save, so those lines are reported as uncovered instead of being reached by a test that proves nothing. constants.rs: local_openai_model was the one endpoint-configuration reader with no test, while local_openai_url next to it had five. A blank model name now has to be None -- passing "" to the endpoint produces a rejected request that surfaces to the user as an AI failure rather than as missing configuration. Verified with: ./tools/test-cad-coverage.sh (466 tests green)	2026-08-16 22:22:26 +00:00
andodeki	34d47f4479	test(cad): cover persistence.rs, 0.00% -> 94.51% of lines ... persistence.rs owns every byte the CAD editor writes: the saved script, the baked OBJ mesh, the thread-local slot the script VM uses to hand a Solid back to the UI, and the pending-image mutex. It had no tests. Not low coverage -- zero. It could not have had any. `save_cad_state` and `load_saved_cad_script` resolve their directory through `cad_data_dir()`, which is the real per-user application data directory. A test that called them would write into the developer's (or the CI runner's) actual data dir, and two tests would fight over the same file. So the save/load pair is split: save_cad_state_in(dir, source, solid) load_saved_cad_script_in(dir) with the existing public functions delegating to them through `cad_generated_dir_path()`. No caller changes, no behaviour changes -- viewport.rs, workspace.rs and workspace_actions.rs keep calling exactly what they called before. 14 tests, on the failures rather than the happy path: - A save that cannot create its directory returns the "could not create generated directory" error. This is the path that used to be `.ok();` at the call site, which is how a "Saved" label appeared over a write that never landed. - A failed OBJ write still leaves the script on disk, and the test asserts the script is readable back afterwards. The write order is load-bearing: losing the baked mesh costs a rebuild, losing the source costs the user's session. - An empty or whitespace-only script file loads as None, not as Some(""). Some("") would open the editor blank and then overwrite a script the user still had. - The script-output slot must empty on take. A stale Solid re-applied on the next tick would silently undo whatever the user did in between. - The generated paths are asserted to hang off the runtime data dir and to contain no build-time source path -- the CI gate for that rule greps for one macro, this pins the actual result. Uncovered: 6 lines, the two public wrappers. Calling them means writing to the real data dir, which is the thing this commit is avoiding. Verified with: ./tools/test-cad-coverage.sh (459 tests green)	2026-08-16 22:20:55 +00:00
andodeki	2ea5a7424e	fix(cad): mat4_inverse was wrong for every matrix that rotates and translates ... 3-D picking has been unprojecting clicks to the wrong world point. The coverage run in the previous commit left seven lines uncovered -- the success tail of mat4_inverse -- and the round-trip test written to reach them failed on element 12. mat4_inverse is a cofactor expansion transcribed from the standard MESA gluInvertMatrix. Comparing all sixteen expressions term by term against the reference, EVERY ONE of them had at least one mistyped index: inv[0] a[9]*a[11]*a[14] should be a[9]*a[7]*a[14] inv[12] a[8]*a[10]*a[13] should be a[8]*a[6]*a[13] inv[13] a[8]*a[10]*a[13] should be a[8]*a[2]*a[13] inv[14] a[0]*a[7]*a[13] should be a[0]*a[6]*a[13] inv[14] a[4]*a[7]*a[13] should be a[4]*a[2]*a[13] ... and one each in the other eleven. The wrong terms all carry a[3], a[7], a[11] or a[15] -- the bottom row. For a pure translation or a pure rotation those are 0, 0, 0, 1 and the mistyped products cancel, which is why the function looks correct in isolation and why nothing caught this. It stops cancelling the moment a matrix rotates AND translates. Which is what the two callers pass in: - CadViewport::camera_eye inverts self.last_view. - CadViewport::unproject_point inverts self.last_proj, whose element 11 is -1 for a perspective camera, and then self.last_view. Measured on a view matrix with a 35 deg yaw and eye (3, -1, 2), the old code's inv * m came back with 0.4698 and -0.7988 in the translation row instead of zero: a click resolved to a point roughly one unit away from where the user clicked, growing with camera distance. Selection, snap and the measure tool all read that point. Fixed by transcribing the reference again, this time verified: the tests assert inv*m AND m*inv against the identity for a translation, a rotation, translate*rot_y, translate*rot_zyx, a perspective matrix, an orthographic matrix, and a dense matrix with no zero entries -- the last because a wrong index cannot cancel when nothing is zero. math.rs is now 99.60% of lines. The three remaining are the `det.abs() < 1e-8` early return's own arm, covered by mat4_inverse_of_a_singular_matrix_is_none but not attributed to it. Verified with: ./tools/test-cad-coverage.sh (445 tests green)	2026-08-16 22:18:57 +00:00
andodeki	500489c2f0	test(cad): cover math.rs, 20.40% -> 99.00% of lines ... math.rs had no tests at all. Every other engine file in the CAD module carries its own #[cfg(test)] block; this one -- the file that decides where a click lands in 3-D, whether a point is inside a picked polygon, and how a part's model matrix is built -- had none, and the coverage harness added in the previous commit put a number on it: 20.40%. 39 tests, written against the behaviour that is easy to get wrong rather than the happy path: - The two normalize functions disagree on purpose. DVec3::normalize returns -Z for a degenerate vector, vec3_normalize returns zero. Both are pinned, because "fixing" either to match the other would change picking behaviour silently. - point_in_polygon is exercised on a concave L-shape, not just a square. The picking path projects a bounding box to screen space and can produce a concave outline; a convex-only test passes on a ray-casting implementation that is broken for exactly that case. - point_on_segment_nearest is checked past both endpoints, where the projection parameter is clamped, and on a zero-length segment, where the 1e-12 guard is the only thing between the caller and a NaN. - ray_triangle_intersect is checked on each rejection branch separately: parallel, u < 0, v < 0, u + v > 1, and a triangle behind the origin. - ray_aabb_intersect is checked from outside, from inside (where it returns the exit parameter, not the entry), behind the ray, parallel to a slab both inside and outside it, with a negative direction component (the t1/t2 swap), and on a diagonal miss -- which is the only way to reach the `tmin > tmax` return, since an axis-aligned miss leaves through the parallel-slab branch first. - segment_intersection is checked parallel, crossing, and crossing off the end of one segment and of both. Remaining uncovered: 7 lines, the success tail of mat4_inverse. The next commit reaches them, and finds out why they were never reached. Run: ./tools/test-cad-coverage.sh	2026-08-16 22:18:16 +00:00
nigig-ci	34fecf1924	build: pin every git dependency to a full 40-character SHA (Phase 0.2) ... The repo has a CI gate requiring full-length revs, added deliberately in 5e71457 with a comment explaining that an abbreviated rev resolves only while no other object shares its prefix -- a property of the repository's current object count, not a guarantee. Git's abbreviation length grows as a repo grows, so a short pin silently becomes ambiguous, and an attacker able to push to the fork can try to manufacture a colliding prefix. That gate has been failing. 42 declarations across 34 crates used abbreviated revs: 41x rev = "ecf5a572" (the current makepad pin) 1x rev = "5efe6e24c" (map/tests/makepad_test_app, left behind by the ce0eaae bump) Resolved both against the remote and rewrote them: ecf5a572 -> ecf5a572ab62a1c1598909971f602f99083671cc 5efe6e24c -> 5efe6e24c9f732e9f11b783757f196f4f1c402b2 Verified this changes the LABEL and not the dependency: Cargo.lock holds exactly one makepad commit id and zero references to the old one, so nothing was silently upgraded. The stray makepad_test_app pin did move to the current rev, which is the intent -- it pointed at a stale branch head. Cargo.lock also picks up unrelated churn (brotli et al in, makepad-android-state/jni-sys out). That staleness is PRE-EXISTING, not caused by this change: confirmed by stashing every edit and running `cargo metadata` on a pristine tree, which produces the identical diff. Gate now passes: $ grep -rn 'rev = ' --include=Cargo.toml . | grep -vE 'rev = "[0-9a-f]{40}"' (no output)	2026-08-16 18:35:39 +00:00
andodeki	ce0eaae935	fix(build): bump makepad pin to ecf5a572, restoring the test feature ... 86c9595 synced the fork to upstream/dev at abd70f4, which dropped three fork-local optional dependencies from widgets/Cargo.toml and their re-exports from lib.rs. They were fork additions, so the merge lost them. Every Makepad UI target then failed to resolve: package `nigig-pdf-makepad` depends on `makepad-widgets` with feature `test` but `makepad-widgets` does not have that feature. help: available features: default, serde failed to select a version for `makepad-widgets` The "available features" list is misleading: with no `test` feature on widgets 2.0.0, cargo falls back to the stale old/widgets copy, which is 1.0.0 and offers only default and serde. Same fallback that produced the bogus makepad-fonts-chinese-bold error in an earlier sync. libs/makepad_test was never removed - only the manifest entries and the re-export. The fork's ecf5a572 restores both. This bumps all 34 crates. Verified against the real fork, not a local copy: TEST_TARGET=pdf-ui 682 passing (was: failed to resolve) TEST_TARGET=pdf 637 passing Pin bump only: every hunk changes the rev and nothing else.	2026-08-16 17:39:48 +00:00
andodeki	86c9595729	chore: update makepad fork to latest upstream/dev (abd70f4) ... Updated makepad fork to include all latest APIs needed by map widget: - pack_vector_vertices and VECTOR_PACKED_FLOATS_PER_VERTEX - TileArchiveReader for MKMap archive support - get_tile_decoded method on MbtilesReader - set_trust_fill_winding and fill_fringe_into on Tessellator - retain_queued method on TagThreadPool - set_camera_delta method on DrawRotatedText This resolves all compilation errors in the map widget code.	2026-08-16 17:18:31 +00:00
andodeki	c66ffcb303	test: add comprehensive CAD UI tests for all implemented features ... Added 50+ UI tests covering: - Toolbar buttons (tools, export, zoom, rotation, grid, visibility) - Tool selection via click and keyboard - Drawing creation (rect, circle, wall, column, beam) - Undo/redo roundtrip - Selection and deletion - View manipulation (plane toggle, rotation, zoom, workplane rotation) - Snap/ortho/polar toggles - Grid and reference plane buttons - Export buttons (STL, SVG, PDF, OBJ, 3D, CLI) - PDF preview tab switching - Code editor visibility and content - Cost estimation screen - AI pane widgets - File operations - Splitter toggles - Properties panel - Status label text verification - Mobile editor tabs - All CAD tool buttons (arc, polyline, area, quad, polygon, triplane, extend, chamfer) - Render mode dropdown - View toggle button	2026-08-16 13:17:30 +03:00
andodeki	9d647cec8c	build(deps): bump makepad fork rev to 5efe6e24c (makepad-test enabled) ... All 35 Cargo.toml pins move from d82756a to 5efe6e24c on the gitdab fork (portallist base + makepad_test Android adb / standalone terminal wiring). Lockfile regenerated; pdf crates compile against the new rev.	2026-08-16 08:38:18 +03:00
andodeki	8fe7db2621	fix: update SpreadsheetGrid selection API in project/mod.rs ... Replace deprecated selection_anchor/selection_head field access with set_selection_pair() method call after spreadsheet-ui refactor.	2026-08-16 03:32:51 +03:00
andodeki	f8446fe041	feat: nigig-build cost estimator, pay security prefs, location/sync pipeline, pdf parity docs ... - nigig-build: cost_estimator tests + makepad-test dev-dep for UI parity suites - nigig-pay-ui: file-backed SecurePreferenceStore (security_prefs) for biometric opt-in, wired into shared pay sheet + payments frame - nigig-core: rewrite location.rs subscriber model (drop robius_location Manager sendable wrapper), real Nominatim parser, expanded syncing pipeline - nigig-uikit: camera widget layout rework for permission flow - map/rider: drop makepad 'maps' feature (fork map module doesn't compile at pinned rev); i_tree 0.19.0 pin - pdf-cos: remove debug-only xref round-trip test - docs: NIGIG_PDF_FEATURE_PARITY_PLAN.md (10 phases, dart-pdf test inventory, scale table), workflow.md makepad fork-sync + pdf context sections, THIRD_PARTY_NOTICES.md for dart-pdf attribution - pageflipnav: NDK toolchain env notes for android builds	2026-08-16 02:34:01 +03:00
andodeki	54ac36c0f7	refactor(map): use makepad-widgets map feature instead of custom copy ... Updated makepad fork to d82756a which includes latest map improvements: - Baked fills/faces support - Enhanced 3D building rendering - Improved road geometry and elevation - Better theme matching and styling Removed tile_makepad.rs (12k+ lines) and reverted to using makepad-widgets map functionality directly. This avoids maintaining a separate copy and ensures we get all upstream improvements automatically. Changes: - Updated all Cargo.toml files to use makepad fork d82756a - Removed crates/apps/map/src/tile_makepad.rs - Removed tile_makepad module from lib.rs - Reverted tile_disk.rs to use mbtiles_tile_to_overpass_response	2026-08-04 11:22:05 +00:00
andodeki	6e5a16f661	fix: remove invalid DSL property overrides ... - Remove 'visible: false' from RobrixTextInput in shared_pay_sheet.rs (visible is not a valid property on this widget type) - Remove metric_title and metric_value overrides in cost_estimate_screen.rs (these are child widgets, not overridable properties) These were causing runtime DSL errors that prevented proper widget rendering.	2026-08-04 07:53:28 +00:00
nigig-ci	f1a5203b27	style(nigig-build): apply cargo fmt, no other change ... Mechanical `cargo fmt -p nigig-build`. Nothing but formatting is in this commit, deliberately: it is 89 files and would bury any real change made alongside it. The cad-module job has failed on every run since a runner was first registered. It is one step -- `cargo fmt -p nigig-build -- --check` -- and it reported 1,559 diffs. Note the scope. The step is named "Formatting (CAD module)" but `-p nigig-build` covers the whole crate: the largest offenders are doc/widgets/doc_widget.rs (166 hunks), doc/tests.rs (149) and project_management/mod.rs (128); CAD proper is a minority. The name is misleading and the fix is crate-wide. The changes are what rustfmt does: wrapping long signatures and call chains, exploding single-line struct literals, adding trailing commas, and `use makepad_widgets::{Vec4f}` -> `use makepad_widgets::Vec4f`. Verified inert, since a reformat that changes behaviour is the whole risk here: cargo test -p nigig-build --lib before 794 passed; 0 failed; 19 ignored after 794 passed; 0 failed; 19 ignored cargo test --locked -p nigig-build --test cad_integration after 154 passed; 0 failed All 12 source-scanning gates in the supply-chain job still pass. That check matters more than it looks: several are regex-based and match on line shape, so moving code across line boundaries could have silently defeated them. It did not. `cargo fmt -p nigig-build -- --check` now exits 0.	2026-08-04 06:46:48 +00:00
arena-agent	c854624838	docs(doc): device verification runbook for the hardware-only batch ... One roadmap box legitimately cannot execute in the sandbox — ScrollYView parent handoff verification on Android/iOS — and with it the class of platform-owned behaviors deferred across the touch milestones (IME opening, native clipboard-menu placement, touch arbitration on real event streams, the GPU-bound painting/clipping sweep scoped here by the legacy perf-box retirement). This writes DEVICE_VERIFICATION.md so a hardware session becomes checklist execution: - prereqs: cargo_makepad build/run commands for Android (adb) and iOS (run-device with provisioning), per the fork's tool help; - nine sections covering interaction mode (View/Edit), IME input including autocorrect commits into cells, long-press selection with handles and the clipboard menu, table gestures (touch-only cell-range spanning, merge/split), the scroll-handoff box on BOTH workspaces (crdt_body and the legacy body_scroll), system-clipboard round trips of raw vs RFC-4180-quoted tabular payloads, multi-line cell rendering, the visual painting/clipping sweep with the layout-cache perf smoke check, and persistence; - every row names the code mechanism under test (10 px / 24-frame arbitration, show_text_ime + the NextFrame reassert, show_clipboard_actions keyboard_shift passthrough, the start/extend cell-range path, quoting round trips, grown-row layout) with expected outcomes and explicit fail criteria — including which failures must be filed rather than waved through; - a sign-off table that gates closing the roadmap box on both editor columns passing. Documentation only; no code changes. The roadmap box gains a pointer to the runbook for the hardware session.	2026-08-02 14:41:14 +00:00
arena-agent	2055b6dfb4	perf(doc): cache the projection layout by document state; retire legacy perf boxes ... The legacy roadmap carried four open boxes whose foundations had landed long ago: incremental page/block reflow execution, draw-time fragment-payload reuse, command-to-block-revision wiring, and the renderer draw-pass integration test. It also carried an unmeasured cost on the ACTIVE path: CrdtDocEditor recomputed the whole ProjectionLayoutTree in every event handler (~20 sites) and on every draw — several full O(blocks + glyphs) passes per keystroke. Decision per box (DocWorkspace/DocEditor is the fallback path; the CRDT-native editor ships): - Command->revision wiring: retired. Change detection keys on the engine's op version-vector sum, bumped exactly once per mutating op (edit, undo, redo, peer import) — no per-command revision plumbing needed on the active path. - Incremental reflow execution: retired for the legacy pipeline; answered on the CRDT path by a document-keyed cache in CrdtDocEditor::layout_tree — an unchanged document serves an Rc clone of the previous tree for every consumer, and the first consumer after any op recomputes once. Whole-tree granularity by design: per-block re-layout buys nothing until a profile asks. - Draw-time fragment reuse: retired for the legacy renderer; the CRDT draw walk reuses the same cached tree — the glyph/rect payloads are the cache, not a second draw-only structure. - Renderer draw-pass integration test: resolved by scoping. All non-GPU draw logic (geometry, rects, hit tests, event flows) is covered by the real-Cx runtime harness with Area::Rect stubs; painting/clipping visual verification stays GPU/Studio-bound and lands with the device-verification batch. set_engine drops the cache slot outright so a swapped engine can never inherit another document's tree under a colliding key; the RefCell slot never escapes a call (several consumers hold &self). Tests pin pointer-identity reuse, edit/undo invalidation with fresh geometry, and no stale-tree inheritance across engine replacement. README roadmap boxes annotated and the decision section documents the rationale and residuals.	2026-08-02 14:29:22 +00:00
arena-agent	7d1a7316d2	feat(doc): render multi-line cell text on grown rows ... Cell values holding newlines (legacy strings, or fresh ones the RFC-4180 quoting round-trip now produces) rendered collapsed inline; the text round-tripped but every display line squeezed onto one band. One shared line model now threads layout, renderer, caret, highlight, hit test, and the keyboard surface: - layout_projected_table grows a row by one 18px text line height per extra display line of its tallest visible cell over the 28px baseline; the table rect and the block flow below follow. Column widths stay fixed and single-line tables lay out byte-identical (control assertions pin both). Merge composition: a covered cell's hidden text never inflates its row, and a vertical merge anchor sums the grown heights of the rows it spans. - The renderer draws styled runs segment by segment: an embedded newline in a run resets x to the inset and advances one line, keeping the whole text block vertically centered so single-line cells draw exactly where they did. - table_cell_caret, cell_text_span_rects (one band per covered display line, replacing the single-rect helper), and the point-based cell_char_offset_at (y picks the band, x midpoint- splits within it) all resolve through one cell_text_line_col / cell_text_offset_at pair whose round-trip is unit-tested at every boundary, including empty lines and the newline's own offset. - ArrowUp/ArrowDown, previously dead in cell mode, step between display lines keeping the visual column (clamped per line), Shift extending the in-cell selection; they stay inert at the first and last line and on single-line cells, so no implicit row exit and no half-moved cell ranges. Defect fixed in-phase: an in-cell character span covering a newline copied as a raw slice, so a paste re-distributed it across cells. The in-cell copy branch now quotes through the same quote_tabular_field as every other tabular payload; the Shift+ArrowDown runtime test pins the quoted payload end to end. Tests: line-math boundaries, row growth with block flow and merge composition, multi-line caret rects, per-line selection bands, point hit-testing clamps, vertical-arrow step/inertness/collapse, a real tap parking on the tapped display line, and the quoted span copy via copyable_selection_text and the TextCopy hit.	2026-08-02 10:36:28 +00:00
Arena Agent	e4fbd71d78	fix(cad): finish 1.9 and 4.6, found by auditing the whole plan ... Asked whether every phase was complete, I checked each row against the code instead of against my own record. Phases 0-5 were done except two leftovers that had been reported as finished and were not. 1.9 -- the dead binding was still there: let rzyx = makepad_widgets::Mat4f::identity(); // Simplified — use transform directly let rzyx = mat4_mul(...); // immediately shadows it Harmless to execution, but it reads as though the rotation is being skipped, in the one function that builds the model matrix -- in a module where a rotation bug has already shipped four times. Deleted, with the real computation formatted so the Z*Y*X order is legible and the degrees contract stated. (The other half of 1.9, add_part's placement, was genuinely done: the slot comes from the monotonic id, not parts.len().) 4.6 -- 10 `v18b rev2:` prefixes survived the archaeology sweep, in arch_gltf, arch_pdf, viewport and workspace. Same treatment as the other 73: keep what the code does, drop which internal revision introduced it. Now zero. Also marked the 29 Phase 0/1/2/4 rows that were complete but never recorded as such, with the specifics rather than a bare "DONE" -- 0.2 notes the lockfile is at the workspace root (a per-crate one would be ignored, since nigig-build is a member); 1.1 notes the rotation contract settled on DEGREES, not the radians the plan proposed; 4.3 notes it was superseded by Phase 5.4 rather than done as written. Every numbered row in the plan is now DONE, or REJECTED with the measurement or counter-example that closed it. 783 lib + 154 integration tests pass. All 13 CI gates pass.	2026-08-02 10:27:45 +00:00
Arena Agent	583fbbd092	docs(cad): record Phase 2 and 3 outcomes, including three rejections ... Phase 2 is complete (2.6 was the last open item). Phase 3 is complete in the sense that every item has been either done or measured and closed with a reason. Done: 3.2 world-AABB cache (5.9x), 3.4 redraw_all removal (53 calls), 3.6 script regeneration on commit (425 us/frame at 500 parts), 3.7 async exports, 3.9 grid batching (5,400 -> 1 tessellation per frame at 1080p), 2.6 save dialogs. 3.1, 3.5 were already done in earlier phases. Measured and rejected, with the numbers in the table: 3.3 ParamHash memoisation. 50 ns/node for a Box. The polygon case is real (987 ns) but it is the vertex data, and bulk-hashing measured no faster; the fix would be a data-model change. 3.8 Cost-estimate parallel threshold. 1.01x on a warm cache, which is the common case. Two of the completed items were not what the plan described, and the table now says so rather than quietly claiming the original wording: 3.9 the plan blamed the "nice number" step computation. That is already a cheap if-else chain. The cost was stroke()-per-dash. 3.2 the plan said to key the AABB cache on ParamHash. Doing that would have served a stale box after every drag, because ParamHash deliberately excludes the transform. Also documents the export architecture in ARCHITECTURE.md 2d, including why the 3D viewer and Bake stay directory-based -- both write companion file pairs that reference each other by name.	2026-08-02 10:14:24 +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	84ed7d2e43	perf(cad): drop 53 redundant cx.redraw_all() calls (Phase 3.4) ... cx.redraw_all() sets a flag that repaints every widget in the application. Every one of the 52 calls in viewport.rs, and the 1 in viewport_2d.rs, sat DIRECTLY after `self.area.redraw(cx)` -- the targeted redraw was already there and the full-app repaint added nothing. Verified mechanically before deleting: a scan for `cx.redraw_all()` not preceded by `area.redraw(cx)` returns zero hits in both files. On a drag this ran per motion event: a whole-application relayout to move one part. What I did NOT touch, and why: workspace.rs (15) cross-widget coordination. Both viewport sync paths end in a redraw of the OTHER viewports, and that is what makes removing the viewport's own calls safe. Removing these would be a different change with a different argument. viewport_input.rs (28) event paths; 13 are not paired with an area.redraw at all, so each needs reading on its own terms rather than a bulk edit. cad_editor_sheet.rs (2) not the viewport. The risk here is a missed repaint, which no test can see, so I checked the mechanism rather than relying on the suite staying green: cross- viewport repaint runs through sync_parts_from_any_dirty_viewport (which calls vp.redraw on each destination) and sync_view_from_any_dirty_viewport (which ends in its own redraw_all). Both live in workspace.rs and are untouched. the_viewport_does_not_ask_the_whole_app_to_repaint pins it as a source check, because asserting on repaints needs a live Cx the suite does not have. Negative test: reintroducing one pairing in viewport_2d.rs fails it with the file and line named. 781 lib + 154 integration tests pass. All 13 CI gates pass.	2026-08-02 10:02:20 +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	54e1148b39	feat(doc): round-trip tabular clipboard payloads with RFC-4180 quoting ... Cells holding tabs, newlines, CRs, or quotes re-distributed across the grid on a copy/paste cycle — the documented caveat of every tabular clipboard milestone. This closes it on both sides: - table_grid_tsv quotes such fields on the way out (wrapped in double quotes, inner quotes doubled) via a new quote_tabular_field helper; plain and empty fields stay raw, so payloads stay byte-compatible with spreadsheets and plain text editors. The cell-range payload and the block-span document payload share the one builder, so both inherit the quoting at once. - paste_table_payload replaces its split('\n')/split('\t') walk with split_tabular_payload, an RFC-4180-style tokenizer: quotes open only at field start (mid-field quotes are literal), doubled quotes read as one, tabs/newlines/CRs inside quotes are literal field text, CRLF rows outside quotes keep their tolerance, an unterminated quote reads to the end as best effort, and a single trailing newline adds no phantom row (a deliberate empty row survives). - Caret parking, no-op skipping, empty-field clears, and the one-undo grouped write semantics of the raw paste milestone are unchanged; the caret offset in a multi-line value counts the newline too. Tests: unit coverage for the writer and the tokenizer (every quoting rule plus the quote/split round-trip property), runtime coverage of copy quoting, paste restoring embedded tab/newline values verbatim with one-undo and redo, and an end-to-end copy-cut-paste cycle; a doc-engine materialize test pins special-character cell text surviving peer sync and undo/redo verbatim. README caveats updated and the milestone documented.	2026-08-02 09:54:40 +00:00
Arena Agent	07cef07dfb	feat(cad): async exports with a save dialog (Phase 2.6 + 3.7) ... Both plan items are the same call, so doing them separately would mean writing the plumbing twice: the file picker's `save_data` takes owned bytes, and serialising off the UI thread is what produces owned bytes. Before: every export serialised on the UI thread straight into a File at a hardcoded path -- generated/stl/model.stl, generated/3d/model.glb, generated/floor_plan.pdf. A large scene froze the editor for the whole serialise, and the second export of a session silently destroyed the first. Now: PDF, STL spawn_export_to_target -> worker thread -> save dialog SVG stays sync (the preview widget needs the bytes on the UI thread, so there is nothing to move) but gains the dialog CLI already a String in memory; gains the dialog 3D viewer async, but stays directory-based ON PURPOSE: it writes TWO files and viewer.html references model.glb by relative name, so renaming the GLB through a picker would break it Bake stays directory-based: writes the parts.obj/parts.cad pair, a workspace artefact rather than a document, and Solid::write_obj takes a path not a writer Results come back through an mpsc channel drained on NextFrame, the same mechanism the rebuild worker already uses -- not a second bespoke one. The status says "exporting…" while in flight, and the completion message for a dialog export says "ready — choose where to save" rather than claiming the file is written, because at that point it is not. Reporting success before the write is the exact bug fixed in the Save buttons earlier. The 3D companion HTML is now only written if the GLB actually landed. Previously a truncated GLB still got a viewer.html beside it, which is how "export succeeded" turned into a blank page. ExportTarget::suggest gives each export a distinct default name (<stem>-<project>-<counter>.<ext>) so successive exports do not propose to overwrite each other. sanitize_file_stem exists because a project name is user text that ends up in a save dialog. It can contain a path separator, "..", a NUL, a leading dash, or 300 characters of emoji. The test found a real bug in my first version: replacing "/" with "_" turns "../../etc/passwd" into "_.._.._etc_passwd", so trimming leading dots BEFORE the replacement leaves "_.." behind. Trim after, and include "_". Deleted in the same commit as their cause: write_floor_plan_pdf and export_to_file, both now unreachable. Verified no callers remain anywhere in crates/. New tests: bytes land at the requested path; a failed write is reported rather than swallowed (parent is a regular file, so create_dir_all cannot succeed); an empty export still produces a file; the hostile project-name corpus; successive suggestions do not collide. 772 lib + 154 integration tests pass. All 13 CI gates pass.	2026-08-02 09:52:27 +00:00