# CAD render optimisation: a phased plan Companion to `REVIEWS/CAD_DRAWCALL_STRATEGY_ANALYSIS.md`, which contains the evidence and one significant correction. Read its section 0 first: the 2D vector scene is **one draw call**, not hundreds, and the plan below is ordered on the corrected facts rather than the original ones. **Revision 2** merges a second, independently written optimisation plan (frustum culling → geometry merging → GPU instancing → octree/LOD) into this one. Section "Merging a second review" below records what that plan got right, what it got wrong, and — the part that matters most — the place where it and *this* document were both wrong about the same thing. ## The three findings this plan acts on 1. **Before Phase 1 there was no viewport culling.** The original `grep -niE "cull|frustum|offscreen|in_view"` over the 2,461-line renderer returned nothing. Every part was submitted every frame, on screen or not — in 2D that was wasted tessellation, in 3D a wasted draw call. Phase 1 now owns the shared visibility predicates. 2. **Before Phases 2–3, 3D issued one draw call per part**, each with its own geometry buffer, transform and colour. `part_geoms` was keyed by part id, so 200 identical columns were 200 uploads and 200 draws. Phases 2–3 now share shape geometry and instance repeated parts. 3. **Before Phase 4, the base grid and parts loop tessellated per item**, where the file's own `queue_dashed_line` idiom — queue segments, stroke once, guarded by a test — showed how not to. Phase 4 now groups those paths; Phase 5 reduces the geometry for sub-pixel parts. --- # Merging a second review Every claim below was checked against the tree at `14aa0d5` before it was accepted or rejected. Line numbers in the incoming plan were a few off (`draw_scene` is at `viewport_render.rs:325`, its parts loop at 345; `DrawCadMesh` is at `mod.rs:152`), which is drift, not error. | Incoming proposal | Verdict | Evidence | |---|---|---| | Frustum-plane culling in 3D | **Adopted** | `SceneState3D { view, projection }` is captured every frame into `self.last_view` / `self.last_proj` (`viewport.rs:3781-3782`) immediately before `draw_scene`. Planes come out of `projection * view` by adding and subtracting rows — no matrix inverse, which matters here (see below). | | Store `bounding_sphere` on `CadNode` | **Rejected as stored, adopted as derived** | See "The field that must not exist". | | Cull only in `draw_scene` | **Widened** | 2D plan view is where drafting happens and where per-part cost is tessellation. Both loops get the predicate. | | "Log skipped-part count per frame" | **Replaced** | Phase 0 already built the counting seam (`render_budget::FrameBudget`, `bench_frame_submission_budget`). A log line nobody reads is a step that cannot fail. | | "Expect 30-60% of parts skipped" | **Not adopted as a target** | Unfalsifiable as written — it depends entirely on zoom. The exit criterion is a number in `BENCH_BASELINE.md` at a stated zoom, not a guess. | | Merge geometry per `PartKind` with transforms baked into vertices | **Rejected** | See "Why merging bakes in a regression". | | GPU instancing, one call per group | **Adopted, and cheaper than the incoming plan thought** | The machinery is already upstream. See "Instancing is not a shader rewrite". | | Octree + LOD above 500 parts | **Still rejected at that threshold** | The cached world-AABB pass costs **20 µs at 500 parts** (`bench_pick_broadphase_world_aabb_recompute_vs_cache`) — 0.12% of a 16.7 ms frame. A tree that replaces 20 µs cannot pay for itself. Revisit with a measurement, not a part count. | | Dependency order 1 → 2 → 3, 4 optional | **Agreed** | Instancing batches per geometry, so shared geometry genuinely gates it. | ## The correction that applies to both plans `ParamHash::from_node` hashes **`node.id` first** (`cad_scene.rs:1871`), before it touches a single geometric parameter. This document previously said, in Phase 2: > Parts with equal `ParamHash` are geometrically identical by definition — > that is what the hash means. True, and useless: equal `ParamHash` also means *the same node*. Two identical columns at different coordinates have different hashes, so re-keying `part_geoms` by `ParamHash` would share nothing at all. The incoming plan inherited the same assumption from the same type name. The id is harmless in the hash's current uses — `MeshCache` and `part_geoms` are keyed by `NodeId` and only ask "is this entry still valid for this node", where the id is a constant — so this is not a bug to fix, it is a second hash to add. `ShapeHash`: the same body with the id omitted. Pinned by `param_hash_is_not_a_shape_key_it_includes_the_node_id` in `cad_scene.rs`, added with this revision so the claim cannot be made a third time by reading the type name. ## The field that must not exist The incoming plan opens with "add `bounding_sphere: (DVec3, f64)` to `CadNode`, computed on `add_part` / update". Three reasons not to: 1. **There are 77 `CadNode { ... }` construction sites** across the crate and its tests. Every one becomes a place to get the sphere wrong, and the compiler only catches the ones that forget the field, not the ones that fill it in stale. 2. **This codebase has already paid for exactly this mistake.** The `part_geoms` staleness bug (`viewport.rs:4629`) was a map that had to be maintained by hand at every edit site; one site was missed and the viewport drew the pre-edit shape. The fix was to make the entry carry a hash that *invalidates itself*. `ParamHash` and `PlacedHash` exist because of that lesson. 3. **The derived version already exists and is already cached.** `SceneCache::world_aabb_for` is keyed by `PlacedHash` — so it is invalidated by a move *and* by a resize — and is benchmarked at 4.72× faster than recomputing (94 µs → 20 µs at 500 parts). A sphere is two lines from an AABB: `centre = (min + max) / 2`, `radius = |max - min| / 2`. So: `bounding_sphere(aabb) -> (DVec3, f64)`, pure, derived, in the cull module. No new field, no new invalidation surface. ## Why merging bakes in a regression Incoming Phase 2 proposes replacing `part_geoms` with one merged buffer per `PartKind`, "with per-part transform baked into vertex positions". `MeshCache` stores **local-space** meshes on purpose. Its own docstring is explicit: *"The transform and the material are NOT hashed. The cached mesh is local-space, so neither can change it."* Baking transforms into vertices inverts that invariant, and the cost lands on the interaction users perform most: - Today, dragging a part re-uploads **that part's** buffer, or nothing at all if only the transform changed — the transform never enters a vertex buffer, it is a per-draw shader value the vertex stage applies (`mod.rs:377`), and `ParamHash` deliberately excludes it for that reason. - With transforms baked in, dragging one column invalidates the merged buffer for **every column in the model**, and re-concatenates and re-uploads all of it, on every frame of the drag. The incoming plan's own draw loop shows the tension: it merges the buffers but still writes *"offset handled via transform uniform set before each draw"* — which is still one draw call per part, so the merge buys nothing it set out to buy. The two halves of that phase contradict each other. Per-part colour is the second casualty. Selection and hover recolour a single part (`viewport_render.rs:358-370`). One merged buffer with one colour cannot express that without per-vertex colour and a full re-upload on every hover. Keep local-space geometry; share it between parts of the same *shape*; vary transform and colour per instance. That is Phase 2 + Phase 3 below, and it is what the hardware wants anyway. ## Instancing is not a shader rewrite The incoming plan rates instancing "high effort, high risk, ~7 days, requires shader work" and hedges with "use `cx.add_instances(...)` if supported". It is supported, and the shader work is already done. In the pinned Makepad fork (`ecf5a572`): - `Cx::begin_many_instances` / `end_many_instances` (`draw/src/draw_list_2d.rs:315,337`) accumulate instance rows into a single draw item. - `DrawPbr` already wraps that for exactly this case: `begin_many_instances_for_mesh(cx, mesh)`, `push_many_instance_with_transform(transform)`, `end_many_instances(cx)` (`draw/src/shader/draw_pbr.rs:2696-2745`) — one geometry, N transforms, one draw call. About thirty lines to mirror onto `DrawCadMesh`. - Per-instance data is simply the `#[live]` fields declared after `#[deref] draw_vars` in a `#[repr(C)]` shader struct (`DrawVars::as_slice` documents that layout). **`DrawCadMesh` already has that shape**: `color`, `transform`, `depth_clip`, `display_mode` (`mod.rs:152-171`), and the vertex shader already reads `self.transform` (`mod.rs:377`). One risk the incoming plan did not raise and one it did not need to: - **Batching reorders drawing.** Safe here: `DrawCadMesh` is declared `alpha_blend: false` (`mod.rs:346`), so the mesh path is opaque and depth-tested. Had it been blended, regrouping would have changed the picture. - **A batch is per geometry.** So Phase 2 really does gate Phase 3. --- # The phases ## Phase 0 — Make it measurable — **done** (`14aa0d5`) `render_budget.rs` (99.75% line-covered, floored in `tools/test-cad-coverage.sh`) owns the grid-loop decision *and* counts it, so the count and the drawing cannot disagree. `grid_range()` drives `draw_2d_vector_scene`'s loops. `bench_frame_submission_budget` records culling, batching and LOD counts at 100/500/2,000 parts across the two original zoom levels plus a far-zoom LOD case in `BENCH_BASELINE.md`. Baseline to beat, 1920×1080, 2,000 parts: **2,170 tessellations, 2,001 draw calls** zoomed in; **2,270 / 2,001** zoomed out. ## Phase 1 — Cull against the viewport — **done** (this commit) New module `cull.rs` (100% covered, floored in `tools/test-cad-coverage.sh`), in the style of `nav_pad.rs` and `render_budget.rs` — pure, host-testable, and called by both draw loops *and* `CadViewport::frame_budget`, so the reported budget and the drawing cannot disagree: ``` 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 ``` Three things came out different from the plan above, and the plan was wrong about each: 1. **AABB, not sphere.** A bounding 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 at all, and nothing to go stale. 2. **`world_aabb_from_local_bounds` is shared with `pick_part`**, whose inline eight-corner transform it replaces. Two copies would be two chances to disagree about a part's bounds, and picking a part the renderer culled is exactly what that disagreement produces. 3. **The "drawn extent" hazard resolved into two concrete rules**, not one margin: the 2D test uses `part_to_plane_2d` and `part_size_on_plane` — the very values the draw uses, so it tests the drawn rect — and *any selected or hovered part is never culled*, because a selection highlight and a tooltip drawn at the cursor extend arbitrarily far from the part. Both tests are conservative by construction: NaN geometry, negative extents and an identity camera matrix 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. **Measured** (`bench_frame_submission_budget`, 1920×1080, parts on a 200 m site — full tables in `BENCH_BASELINE.md`): drafting at a 5 m zoom a 2,000-part scene submits **12 parts instead of 2,000** (2,170 → 182 tessellations, 2,001 → 13 draw calls). In 3D at a 20 m working distance **63%** of a 2,000-part model is off camera. The honest half: at a zoom or camera distance that fits the whole site on screen, culling removes **nothing** — all 2,000 parts are genuinely visible, and that case is what Phases 2 and 3 are for. **Follow-up completed in Phase 5:** the hard-coded `1.2` margins in `viewport_render.rs` now read `render_budget::VIEW_MARGIN`, including the auxiliary axis, story-level and reference-plane paths. The cull, grid and overlays therefore share both the value and the source of truth. ## Phase 2 — Share geometry between parts of the same shape — **done** (this commit) The prerequisite for instancing, and on its own it collapses GPU memory for the models this app is for: architectural drawings are repeated columns, windows and doors. 1. **`ShapeHash`** 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` + `ShapeHash`), so the two cannot drift apart the way the plan's premise and the code did. 2. **`part_geoms` is keyed by `ShapeHash`**, value `Geometry`. The `(hash, Geometry)` pair and the "is this entry still valid" filter in the draw loop are both gone: the key *is* the content hash, so an edited part looks up a key that does not exist and gets a fresh upload. Staleness became structural rather than checked. 3. **Eviction is by live shape, not live id.** This is the one hazard the change introduces and it is not obvious: deleting one of two hundred identical columns must *not* drop the buffer the other 199 are drawing from. `geometry_is_retained_by_live_shape_not_by_live_id` pins it. 4. Geometry stays **local-space**; transform and colour stay per-draw values, exactly as before. Nothing is baked into a vertex buffer. **Measured** (`bench_geometry_buffers_shared_by_shape`): 200 identical walls → **1** buffer. A 420-part repetitive model (columns, three wall lengths, two opening types) → **6** buffers, 70×. 420 all-distinct parts → **420**, 1× — sharing is a property of the model, and that row is in the table so the ceiling is visible. **Draw calls are unchanged**, as predicted: still one per visible part. That is Phase 3's job, and it is now unblocked. **Effort:** under a day, against the week estimated. Two things made it cheap that the estimate did not know: `MeshCache::get_or_build` is already a pure function of `node.solid`, and the upload path (`part_mesh_buffers_from_mesh` → `MeshSpace::Model`) already produced transform-free, colour-free buffers. The estimate assumed those would need untangling; they were built right. ## Phase 3 — Instance the 3D draw loop — **done** (this commit) 1. Three methods on `DrawCadMesh` — `begin_instances`, `push_instance`, `end_instances` — ported from `DrawPbr::begin_many_instances_for_mesh` / `push_many_instance_with_transform` / `end_many_instances` in the pinned fork. **No shader change**, as predicted: `transform` and `color` are `#[live]` fields after `#[deref] draw_vars`, which is exactly the per-instance row `DrawVars::as_slice` sends. The new `#[rust] many_instances` field sits *before* `draw_vars` for the same layout reason. 2. `draw_scene` collects the visible parts as `(ShapeHash, (transform, colour))`, groups them with `batching::group_in_first_appearance_order`, and issues one call per group. 3. `frame_budget` reports `MeshSubmission { instances, batches }` — named fields, because the whole point of the phase is that the two now differ and a caller that swapped two `usize`s would report the win backwards. **Measured** (`bench_frame_submission_budget`): a 2,000-part model of six shapes draws in **6 calls instead of 2,000** — at every camera distance, including 400 m where the whole site is on screen and culling removes nothing. With every part a different size the count falls back to the visible-part count, which is the honest ceiling and is in the table. **What is *not* verified, plainly.** The submission itself has never run: there is no GPU, no window and no `Cx` in this environment, and `tests/ui.rs` still fails at child-build exit 101 (Phase 0 of `REVIEWS/CAD_COVERAGE_100_PLAN.md`). What *is* verified is that it compiles against the real Makepad API, that the grouping is right (`batching.rs`, 100%, seven tests including "no item is lost or duplicated"), that the budget arithmetic is right, and that the batch cannot be left open on any path (`every_instanced_batch_is_closed_before_the_loop_turns`, a source check in the house style). **Someone with a window needs to open a 3D model and confirm the picture is unchanged.** Two things reduce the blast radius if it is not: `begin_instances` returning `false` falls back to the old one-call-per-part loop, and the batch order is deterministic, so a defect will reproduce rather than flicker. **Effort:** a day. **Risk:** the highest of the four phases, for the reason above — not because the change is large. ## Phase 4 — Reduce tessellation calls in 2D — **done** (this commit) The honest small one, and it turned out to be the one that fixed the case Phase 1 could not: a 2,000-part model with everything on screen. `stroke()` tessellates the whole accumulated path and then clears it (`tessellate_path_stroke` ends in `path.clear()`), so queueing many subpaths and stroking once costs one tessellation instead of N. That idiom was already in the file, in `queue_dashed_line` for the axis grid. Phase 4 applies it to the two loops that had not adopted it. 1. **Base grid: two passes, two strokes.** Minor lines queued and stroked at 0.55, majors at 1.6 — the width is what genuinely needs a separate call. `GridRange::has_minor_lines` / `has_major_lines` decide whether a pass runs, and `frame_budget` counts strokes with the same two predicates, so an empty group is not charged for. Minors are stroked first so majors land on top where they cross. 2. **Parts grouped by colour.** `batching::ColorKey` (bit-pattern key, exact round trip back to the colour) plus the same `group_in_first_appearance_order` Phase 3 groups shapes with. The colour policy moved to `constants::part_outline_color` so the renderer and `frame_budget` cannot disagree about how many groups a frame has. 3. **Selected and hovered parts stroke last**, in their own groups, so a highlight is never hidden under a neighbour's outline. Before, they were interleaved in document order and could be. 4. `FrameBudget` gained `grid_lines` and `part_outlines` alongside the call counts: geometry volume and call count are now different numbers and both are worth reading. **Measured** (`bench_frame_submission_budget`, 1920×1080, 200 m site): | Zoom | Parts | Visible outlines | Tessellations | Before | |---|---|---|---|---| | 5 m | 2,000 | 12 | **4** | 2,170 | | 200 m | 2,000 | 2,000 | **4** | 2,270 | The second row is the point. Everything is on screen, culling removes nothing, and the frame still costs four tessellation calls. **What this does not do at the Phase 4 boundary:** vertex volume is unchanged. The same 2,000 rectangles are tessellated; they are tessellated in two calls instead of 2,000. What is saved is per-call overhead — tessellator setup, two `std::mem::take`s and an `append_geometry` each time. The Phase 5 gate below measures whether that remaining geometry volume is actually unresolvable at the current zoom. **Guard:** `the_base_grid_and_the_parts_loop_stroke_per_group` in `viewport.rs` matches braces to check that neither `stroke()` sits inside an item loop. Its first draft looked for a closing brace at a fixed indent, matched the wrong one, and failed on correct code — the test was wrong before the code was, for the second time in this plan. **Effort:** a day. **Risk:** low, with one visible-behaviour caveat worth stating: parts of the same colour are now drawn together, so where two outlines of *different* colours overlap, which one is on top can change. Outlines are 1.8 px and the highlight ordering got strictly better, but it is a change to what is drawn, not only to how. ## Phase 5 — Level of detail — **done** (this commit) The Phase 4 budget separated geometry volume from tessellation-call count, which made the gate answerable. In 2-D, a 1 m part is only about 2.7 logical pixels across at the 200 m site view and about 1.1 pixels at 500 m. In 3-D, the 400 m camera case leaves all 2,000 parts visible, but 1,620 of them have a projected AABB below three pixels. Both are measured cases where a full representation carries shape detail the display cannot resolve. Phase 5 now covers both rendering paths: 1. `lod.rs` projects 2-D plane extents and 3-D world-AABB corners into logical pixels. The shared threshold is strictly below 3 pixels. Invalid view, extent, clip-space, or proxy-bound data falls back to the full representation, never to hidden geometry. 2. In 2-D, ordinary sub-pixel parts become one bounded 2×2 logical-pixel filled marker. Full outlines remain colour-batched. Selected and hovered parts always stay full detail, so the interaction affordance is not replaced by a generic marker. 3. In 3-D, ordinary sub-pixel parts become instances of one shared unit cube whose transform is built from the part's world AABB. This keeps the approximate footprint and colour while replacing expensive cylinder, sphere, CSG, section, or otherwise high-resolution meshes. The proxy is stored separately from `part_geoms`, so document shape cache eviction cannot remove it. The original local-space geometry is not rewritten or re-uploaded. 4. The filled 3-D path and the wireframe/hidden-line edge overlay use the same 3-D LOD policy. A proxy marker that cannot be projected falls back to the full edge path rather than disappearing. 5. `FrameBudget` reports full 3-D shape instances and simplified proxy instances separately, and counts the shared proxy batch. The budget cannot mistake a reduction in full-shape batches for a reduction in visible parts. 6. The remaining hard-coded `1.2` view margins in the auxiliary 2-D drawing paths now use `render_budget::VIEW_MARGIN`, so cull, grid and overlays share both the value and its source of truth. **Measured** (`bench_frame_submission_budget`, 1920×1080, 200 m site): | Camera/zoom | Parts | Visible | Full detail | LOD representation | Calls | |---|---:|---:|---:|---:|---:| | 2-D, 5 m | 2,000 | 12 | 12 outlines | 0 markers | 4 tessellations | | 2-D, 200 m | 2,000 | 2,000 | 1 outline | 1,999 markers | 4 tessellations | | 3-D, 80 m | 2,000 | 1,459 | 1,459 meshes | 0 proxy instances | 6 shape calls | | 3-D, 400 m | 2,000 | 2,000 | 380 meshes | 1,620 proxy instances | 7 calls | At 400 m with six catalogue shapes, the full-detail groups still use six calls and the simplified parts use one shared proxy call: **7 calls instead of 2,000**. With every part distinct, the same row is 381 calls (380 full shapes plus one proxy batch), which is the honest remaining ceiling. The 2-D rows retain one vector draw call; LOD reduces geometry volume rather than claiming another draw-call change there. The implementation is intentionally a proxy LOD, not a second set of parametric meshes. It avoids a per-shape resolution cache and keeps transitions deterministic. A live GPU/window visual check is still required for release confidence; this environment can compile and execute the policy and submission arithmetic, but cannot execute Makepad's draw submission. **Effort:** under two days. **Risk:** low-to-medium, with an intentional visual-fidelity boundary below three pixels, conservative malformed-state fallbacks, and the existing first-frame instancing fallback preserved. ## Effort and risk, merged | Phase | Effort | Risk | Benefit | |---|---|---|---| | 0 Measurement | done | — | Every claim below is now falsifiable | | 1 Culling (2D rect + 3D frustum) | done | Low | Submissions scale with visible parts | | 2 Shape-shared geometry | done | Medium | 200 walls → 1 buffer; 70× on a mixed model | | 3 Instancing | done | Medium | 2,000 parts, 6 shapes → 6 draw calls | | 4 2D tessellation batching | done | Low | 2,000 parts on screen: 2,270 → 4 tessellations | | 5 2-D/3-D LOD | done | Medium | 1,999 outlines and 1,620 meshes → bounded markers/proxies | ## What this plan deliberately does not do - **No BVH or octree.** At 500–5,000 parts a linear pass over cached AABBs is 20–200 µs. A tree earns its complexity somewhere past ~50k parts and nothing suggests that is the target. If a benchmark ever shows the linear pass in the frame budget, revisit — with the number. - **No `bounding_sphere` field on `CadNode`.** Derived from the cached AABB instead. See above. - **No merged vertex buffers with baked transforms.** They convert an O(1) move into an O(parts-of-that-kind) re-upload per drag frame, and they cannot express per-part selection colour. See above. - **No render-path rewrite.** Each phase is a local change behind a tested predicate or a key change. `viewport_render.rs` is 2,749 lines and remains outside the host-only coverage harness; a rewrite there without the Phase 0 seam would be unverifiable. ## Order and why Phase 0 first because everything after it is otherwise unfalsifiable, and because it doubles as the first test coverage the render path has ever had. Phase 1 next: cheapest, largest, and it shrinks the input to every later phase. Phase 2 exists to make Phase 3 possible and is the invasive one, so it goes after the cheap wins are banked. Phase 3 is the real draw-call win and is now a port rather than a design. Phase 4 is small and honest about being small. Phase 5 is intentionally gated on a measured sub-pixel case and now covers both 2-D markers and 3-D bounding-box proxies. The remaining release check is visual GPU verification, not another speculative geometry cache.