From cde727d748275785fc9ac7351be5e0b547aa495b Mon Sep 17 00:00:00 2001 From: Nick Fisher Date: Sun, 16 Aug 2026 15:13:04 +0000 Subject: [PATCH 1/7] docs: add gizmo & highlight overlay implementation review Research-only review of per-frame cost, allocation churn, threading, and GPU overhead in the TransformationGizmo and HighlightOverlayManager systems, with ranked findings and effort-estimated recommendations. Co-Authored-By: Claude --- docs/research/gizmo-overlay-review.md | 180 ++++++++++++++++++++++++++ 1 file changed, 180 insertions(+) create mode 100644 docs/research/gizmo-overlay-review.md diff --git a/docs/research/gizmo-overlay-review.md b/docs/research/gizmo-overlay-review.md new file mode 100644 index 000000000..c643f7058 --- /dev/null +++ b/docs/research/gizmo-overlay-review.md @@ -0,0 +1,180 @@ +# Gizmo & Highlight Overlay — Implementation Review + +**Date:** 2026-08-16 +**Branch:** `asb/gizmo-overlay-review` (research only — no code changed) +**Question:** Is the gizmo and overlay implementation optimal and efficient? + +## Executive summary + +| System | Verdict | +|---|---| +| Gizmo (current `TransformationGizmo`, pure Dart) | **Reasonable for now.** Near-zero per-frame cost; costs are per-input-event and small. One real bug-class finding: `dispose()` leaks native resources. Two to three generations of dead gizmo code coexist and should be deleted. | +| Highlight overlay (`HighlightOverlayManager`) | **Mostly sound design, with one structural cost issue.** When enabled it adds 2 extra full-screen passes per frame (silhouette + edge detection) and, in composite mode, redirects the main view through an offscreen texture — *even when zero entities are highlighted*. This was a deliberate trade-off (commit `2ca9fc0b` removed `hasHighlights()`), but it is the single biggest GPU cost in either system and is worth revisiting. | + +Neither system does meaningful work on the render thread per frame beyond what Filament itself renders; there are no per-frame buffer uploads, no per-frame Dart→native round-trips introduced by either system, and no unbounded per-frame allocation growth. The inefficiencies that exist are (a) constant GPU cost in the overlay, (b) per-input-event FFI chatter in the gizmo, and (c) a resource leak in gizmo disposal. + +--- + +## 1. Gizmo system map + +There are **three generations** of gizmo code in the tree: + +| Generation | Files | State | +|---|---|---| +| 1. Native glTF gizmo (`TGizmo`) | `thermion_dart/native/src/scene/Gizmo.cpp`, `native/src/c_api/TGizmo.cpp`, `native/include/scene/Gizmo.hpp`, `lib/src/filament/src/implementation/ffi_gizmo.dart` | Live code, exposed via FFI/WASM bindings, **not used by any current example**. Loads an embedded glTF (`translation_gizmo_glb.bin` / `rotation_gizmo_glb.bin`), 3 axis instances + invisible "HitTest" volumes, GPU picking via `View::pick`. | +| 2. Deprecated handlers | `lib/src/utils/src/gizmo.dart` (fully commented out), `lib/src/input/src/implementations/gizmo_input_handler.dart` (fully commented out), `gizmo_pick_delegate.dart` (fully commented out), `native/src/scene/RotationGizmo.cpp` (fully commented out, ~350 lines) | Dead code. | +| 3. Current pure-Dart gizmo | `lib/src/utils/src/gizmos.dart` (`TransformationGizmo`), driven by `lib/src/input/src/implementations/gizmo_attachment_delegate.dart` | **This is what runs today** (used by `examples/dart/examples_lib/lib/src/gizmo_basics.dart:27-29` and `GizmoAttachmentDelegate`). | + +### 1.1 Setup (once per gizmo / per type switch) + +`TransformationGizmo.create()` (`gizmos.dart:57-82`): +- 1 shared `Material` (cached app-wide at `ffi_filament_app.dart:577-585`) + 5 material instances (red/green/blue/white/yellow), each a `createInstance()` render-thread round trip. +- Geometry (cylinder/cone/torus/sphere) generated **in Dart** into growable `List` then copied via `Float32List.fromList` (`gizmos.dart:214-369`) — triple copy (list → typed list → native marshalling in `createGeometry`), but setup-only and tiny (a 64×12 torus is ~3 k floats). +- 8 renderables total for translation (2 per axis) or 3 rings + 2 markers for rotation; all parented to one root entity. `setPriority(7)` to draw on top. + +### 1.2 Per-frame (render loop) cost + +**Effectively zero.** The gizmo is ordinary scene geometry; there is no per-frame callback, no per-frame FFI, no animation. The only "per frame" GPU characteristics come from the material (`materials/gizmo.mat`): +- `blending: transparent` → rendered in the transparent pass; +- `depthWrite: true` + `depthCulling: false` + `gl_FragDepth = 0.999f` in the fragment shader (`materials/gizmo.mat:27`) → depth write per fragment disables early-Z for those fragments. With ~8 small renderables this is negligible, but it is the kind of thing that would matter if the gizmo were instantiated many times. + +### 1.3 Per-input-event cost (where the gizmo actually spends time) + +`GizmoAttachmentDelegate.handle()` (`gizmo_attachment_delegate.dart:170-225`) runs on every pointer event (events are *not* batched — `delegate_input_handler.dart:23-25, 72-75`): + +**On every hover/move while not dragging** (`gizmo_attachment_delegate.dart:205-211`): +1. `_gizmo.update()` (`gizmos.dart:413-439`): + - `transformManager.getWorldTransform(target)` — sync FFI + `Matrix4` alloc + native struct round-trip copy (`ffi_transform_manager.dart`, `utils/src/matrix.dart:22-40`), + - `viewer.getActiveCamera()` → `View_getCamera` FFI + **new `FFICamera` wrapper object every call** (`ffi_view.dart:106-109`, `thermion_viewer_ffi.dart:822-824`), + - `camera.getPosition()` → `getModelMatrix()` FFI + `Matrix4` + `Vector3` allocs (`interface/camera.dart:10-13`), + - `setTransform(rootEntity, …)` — sync FFI + native struct alloc (`matrix4ToDouble4x4`). +2. `_gizmo.hover(x, y)` → `pickAxis()` (`gizmos.dart:441-535`): 4 more FFI reads (viewport, projection, view matrix, gizmo world transform), then pure-Dart screen-space math. For **rotation** gizmos the picking loop is 3 axes × 32 segments × 2 projected points ≈ **192 matrix×vector multiplies per mouse-move event** (`gizmos.dart:499-522`) — plus the same math again inside `_getMarkerPositionOnRing` (64 samples, `gizmos.dart:883-903`) once a drag starts. + +**Net:** ~8–10 FFI calls and ~10 short-lived Dart objects per pointer-move event. At 120 Hz mouse polling this is tens of microseconds per second of CPU — not a bottleneck on desktop, but it is pure waste when the camera and target haven't moved (i.e. almost always). During an active drag, `_updateTranslationDrag` (`gizmos.dart:603-673`) re-fetches viewport/projection/view/model matrices **and** `_updateRotationDrag` re-fetches camera position, then `update(position: …)` fetches camera position *again* (`gizmos.dart:426-429`) — the same matrices are read 2–3× per event. + +### 1.4 Concrete gizmo findings (ranked) + +1. **`dispose()` leaks native resources** — `gizmos.dart:990-1030` only calls `viewer.removeFromScene(asset)` (which is scene-remove only, `thermion_viewer_ffi.dart:871-874`); it never calls `destroyAsset`. The 8 geometry assets (vertex/index buffers + entities), the root entity, and the 5 material instances leak. The code even contains the acknowledgement as comments (`gizmos.dart:997-998`, `1004-1007`). `GizmoAttachmentDelegate.setGizmoType()` (`gizmo_attachment_delegate.dart:151-168`) disposes and recreates on every type switch, so each translation↔rotation toggle leaks a full gizmo's GPU memory. **Impact: high on long sessions / type-switching UIs. Effort: small (S) — route dispose through `viewer.destroyAsset` + destroy root entity/material instances).** +2. **Dead code: three generations coexist** — ~1,000 lines of commented-out gizmo code (`gizmo.dart` 116 lines, `gizmo_input_handler.dart` 372 lines, `gizmo_pick_delegate.dart` 42 lines, `RotationGizmo.cpp` ~350 lines) plus the live-but-unused native `Gizmo`/`TGizmo` path (`Gizmo.cpp` 249 lines, `TGizmo.cpp`, `ffi_gizmo.dart`, WASM/FFI bindings, two embedded glTFs `translation_gizmo_glb.bin`/`rotation_gizmo_glb.bin` shipped in `native/include/resources/`). **Impact: binary size (embedded glb blobs), maintenance confusion, double the surface to review. Effort: small (S) to delete the commented files; medium (M) to remove or re-wire the native path.** +3. **Redundant per-event camera/transform reads** — `update()` + `pickAxis()` + drag handlers re-read the same camera matrices and world transforms 2–3× per pointer event, each with fresh allocations (details in §1.3). **Impact: low (µs/event). Effort: small (S) — cache per-event (or per-frame) camera state in the delegate and pass it down; or only run `update()` when the camera or target transform actually changed.** +4. **`_updateMarkerPosition` calls `setPriority(marker, 7)` on every drag frame** (`gizmos.dart:957-963`) although the priority never changes. **Impact: negligible. Effort: trivial.** +5. **Legacy native gizmo material does hidden double-draw** — the "HitTest" volumes use `TWO_PASSES_ONE_SIDE` transparency with alpha 0 (`Gizmo.cpp:105-112`): invisible but rasterized and blended twice per face, and `gl_FragDepth` in `gizmo.mat:27` disables early-Z. Only matters if generation-1 is ever revived. **Impact: none today (unused path).** +6. **Screen-space picking can't be occlusion-aware** — `pickAxis` deliberately avoids depth ("Use screen-space picking to avoid depth buffer issues", `gizmos.dart:444`). Fine functionally, but it means the gizmo is hoverable through geometry; the native path solved this with `View::pick` (`Gizmo.cpp:214-222`). Worth documenting as a behavioural trade-off rather than fixing. + +### 1.5 Gizmo API-shape notes + +- `TransformationGizmo.update()` is *pull-based*: callers must invoke it on camera motion (`GizmoAttachmentDelegate` does so on scroll/move events). Nothing updates it when the camera is animated by other code (e.g. an animation or another delegate moving the camera programmatically) — the gizmo will visibly lag/detach. A per-frame hook (the app already has `render()` request-frame hooks, `ffi_filament_app.dart:803-818`) would be the conventional place. +- `pickAxis`/`hover`/`updateDrag` are `async` but contain no awaits that need to be — the entire API drags `Future` chaining through pure math, adding event-loop hops per event. Cosmetic, but it prevents trivial batching. +- `GizmoAttachmentDelegate._findAssetForEntity` is a stub returning null with a TODO (`gizmo_attachment_delegate.dart:307-317`) — bone-vs-entity attachment degrades to entity-only. + +--- + +## 2. Highlight overlay system map + +Files: `lib/src/filament/src/implementation/highlight_overlay_manager.dart`, `silhouette_view.dart`, `edge_detection_view.dart`, `materials/silhouette.mat`, `materials/edge_outline.mat`. Enabled per-viewer (`ThermionViewerFFI(..., createOverlay: true)` → `setHighlightOverlayEnabled(true)` at `thermion_viewer_ffi.dart:86-88`; default is `false`, `thermion_viewer_ffi.dart:42`), and auto-enabled lazily by the first `setStencilHighlight` (`ffi_view.dart:575-577`). + +Architecture (two-and-a-half render passes, all via `RenderManager` render order 0/1/2, `ffi_view.dart:505-508`): + +1. **Silhouette pass** (`SilhouetteView`, order 0): a second `View` + own scene, rendering per-entity "ghost" renderables (unlit white, `silhouette.mat`) that *reuse the target asset's vertex/index buffers* and are parented to the target entity so they follow transforms (`silhouette_view.dart:224-269`). Output: offscreen RGBA8 + DEPTH32F render target. +2. **Main pass** (order 1). In **composite mode** (macOS/iOS, i.e. whenever the view renders into a Flutter-provided render target) the main view is redirected into an internal SRGB8_A8 texture (`highlight_overlay_manager.dart:129-149, 233-262`). +3. **Edge-detection pass** (`EdgeDetectionView`, order 2): a fullscreen triangle running `edge_outline.mat`, sampling the silhouette texture 9× (8 neighbours + center) plus the main-scene texture once, and either compositing `mix(sceneColor, outlineColor, edge)` or emitting alpha-only edges (`materials/edge_outline.mat:29-76`). + +### 2.1 Setup (once per enable) + +`HighlightOverlayManager.create` builds 2 views, 2 scenes, 2 skyboxes, 1 camera, materials/instances, fullscreen-triangle VB/IB, samplers, a linear color grading + tone mapper (to avoid double tone-mapping, `edge_detection_view.dart:206-216`), and 2–3 render targets. All done through `withPointerCallback`/`withVoidCallback` render-thread round trips. This is heavyweight (~30+ awaited FFI round trips) but one-off; `_enableHighlightOverlay` also detaches and re-attaches views with explicit render orders (`ffi_view.dart:505-508`). + +`addHighlight` (per highlighted entity/primitive): creates a material instance + entity + renderable builder round trips, parents to target. **Per call it also unconditionally re-uploads all 7 edge-material parameters** — `setOutlineParams` is called before the already-highlighted early-return (`highlight_overlay_manager.dart:319-325`), and `setStencilHighlight` calls `addHighlight` once per primitive of the entity (`ffi_view.dart:602-636`), so an N-primitive entity does N×7 `setParameter*` calls, each of which re-encodes the parameter name (`name.toNativeUtf8()`, `ffi_material.dart:103-110`). For primitives 2..N the silhouette work is then skipped (`_highlightedEntities.contains(target)`), which also means **only the first primitive's geometry is ever silhouetted** for a multi-primitive entity — a correctness gap that looks like an efficiency shortcut. + +### 2.2 Per-frame cost (the core of the review) + +Every rendered frame, for every swapchain, `RenderManager::render()` walks attached views in order and calls `mRenderer->render(view)` (`native/src/rendering/RenderManager.cpp:165-170`). With the overlay enabled that is 3 view renders where 1 used to be: + +| Pass | Per-frame GPU work | +|---|---| +| Silhouette | Full-screen clear (RGBA8) + depth clear (DEPTH32F) + re-render of all highlighted geometry with the unlit silhouette material. When **nothing is highlighted** this still costs a full-screen color+depth clear and a render pass. | +| Main | Unchanged workload, but in composite mode targets an offscreen SRGB8_A8 texture instead of the swapchain. | +| Edge detection | Full-screen triangle, ~10 texture reads + 1 read + 1 write per pixel, transparent blending. Runs whether or not anything is highlighted. | + +So the constant overhead of *having the overlay enabled* is roughly **2 extra full-screen passes + 2 extra full-screen attachments (RGBA8 + DEPTH32F + SRGB8_A8 in composite mode) of bandwidth per frame**, independent of highlight count. On a 4K framebuffer that is several hundred MB/s of avoidable traffic at 60 fps with zero highlights. History shows this is deliberate: `2ca9fc0b` *"remove hasHighlights() from View. This means the overlay will be rendered if enableHighlightOverlay() has been called, even if no assets are highlighted"*. The earlier design (b649a24e) implemented overlays with Flutter widgets and "showed bad performance", so the current architecture is itself the fix for a worse one — the remaining issue is only the no-highlight case. + +Notably, `RenderManager` already has the cheap lever for this: `setRenderable(view, bool)` (`ffi_render_manager.dart:144-149`, `RenderManager.cpp:60-96`) marks views renderable or not, and empty view lists already skip begin/end frame (`RenderManager.cpp:117-144`). Skipping only the two overlay views when `highlightedEntities.isEmpty` (or when nothing moved, if desired) would recover most of the cost without touching resources. + +### 2.3 Per-frame Dart/native churn + +- **None per frame.** Neither view registers a frame callback; material params, textures, and viewport are only touched on `addHighlight`/`removeHighlight`/resize. This is the correct shape and matches how the rest of the codebase treats views. +- Resize path (`SilhouetteView.setViewport` → `_resizeRenderTarget`, `silhouette_view.dart:133-194`; manager `_resizeMainViewRenderTarget`, `highlight_overlay_manager.dart:264-291`): creates new textures/RT, rebinds, `flush()`es the render thread, then destroys old resources. Well-ordered (documented destruction rationale at `highlight_overlay_manager.dart:72-96`). One nit: `EdgeDetectionView.setViewport` re-sets all 7 material params on every resize (`edge_detection_view.dart:342-348` → `_updateEdgeMaterialParams`), including two texture rebinds — harmless at resize frequency. +- `highlightedEntities` getter wraps the set in a fresh `Set.unmodifiable` on every access (`highlight_overlay_manager.dart:108`) — allocation per query; queries are rare (tests/UI), so cosmetic. + +### 2.4 Threading + +All overlay orchestration is Dart-side on the platform isolate, marshalled to the render thread through the standard `withVoidCallback` request-id mechanism (same as every other system in the package). `FFIRenderManager` serialises attach/detach mutations through a static op-chain and snapshots attachment state before syncing (`ffi_render_manager.dart:96-200`) — multi-viewer races are explicitly handled and documented. No long-running work is done on the main thread; no locks are held across frames (`RenderManager::render` holds `mMutex` only for the duration of the render iteration, `RenderManager.cpp:190`). + +### 2.5 Concrete overlay findings (ranked) + +1. **Constant 2-pass full-screen cost even with zero highlights** (`RenderManager.cpp:165-170` + attach at `ffi_view.dart:506-508`; history `2ca9fc0b`). **Impact: high on mobile/web at high resolutions. Effort: small (S) — when `highlightedEntities` transitions empty↔non-empty, call `renderManager.setRenderable(silhouetteView/overlayView, …)` and restore the main view's render target (composite mode) accordingly; ~1 day including tests.** +2. **Composite mode always pays an offscreen main-scene texture + extra full-screen sample/copy** (`highlight_overlay_manager.dart:129-149`). **Impact: medium (one extra full-screen read+write per frame vs. direct-to-swapchain). Effort: medium (M) — only needed because the edge pass must read the scene; alternatives (blit instead of shader-composite when no highlights, or a Filament ` post-process`-style integration if upstream ever lands one). Pair with finding 1: with no highlights, render main directly to the Flutter RT and skip everything.** +3. **`addHighlight` re-uploads 7 material params per call and is called once per primitive** (`highlight_overlay_manager.dart:319-325`, `ffi_view.dart:602-636`, `toNativeUtf8` per param at `ffi_material.dart:103-110`); combined with per-entity keying, multi-primitive entities are only partially outlined. **Impact: correctness bug + minor churn. Effort: small (S) — key silhouettes per (entity, primitive) or pass all primitives in one call; set outline params once per `setStencilHighlight` invocation.** +4. **`setHighlightOverlayEnabled(false)` is a full destroy; `true` a full rebuild** (`ffi_view.dart:517-546` + `destroy()` at `highlight_overlay_manager.dart:356-393`) — there is no cheap pause/resume, which encourages apps to leave it enabled (and thus pay finding 1). **Impact: API shape. Effort: small (S) once finding 1's setRenderable lever exists.** +5. **Silhouette ghost AABB is captured once at add time** (`silhouette_view.dart:244-248`, `culling(true)`) — correct for static geometry, stale for skinned/morph-target meshes; not a perf issue but worth a doc comment. **Effort: trivial (doc) / M (if skinned support is wanted).** +6. **DEPTH32F for the silhouette depth attachment** (`silhouette_view.dart:89-96`) — an unlit opaque pass needs nowhere near 32-bit depth; DEPTH24/16 would halve depth bandwidth on most tilers. **Impact: small. Effort: trivial.** +7. **`removeStencilHighlight` removes by entity + child entities but `addHighlight` tracks per picked entity only** (`ffi_view.dart:645-654` vs `626-635`) — asymmetric bookkeeping; benign today. + +### 2.6 Comparison with sibling systems + +- **Grid overlay** (`grid_overlay.dart`, `materials/grid.mat`) and **translation axis** (`translation_axis.mat`): fully in-scene, analytic shaders — zero extra passes, LOD/fade computed per fragment. This is the cheaper pattern when the visual can be expressed procedurally; the highlight overlay cannot (arbitrary geometry outlines) and is right to use render targets. +- **Picking** (`ffi_view.dart:247-279`): bounded `kMaxPickRequests` ring buffer for in-flight picks — a good allocation-discipline convention both reviewed systems would do well to imitate where they keep per-call state (they currently don't keep any per-frame state, which is fine). +- **Wireframe/bounding-box helpers** (`thermion_viewer_ffi.dart:892-986`): same "ghost renderable parented to target" trick as the silhouette pass — the two could share a small factory (see §3). + +--- + +## 3. Shared infrastructure / unification opportunities + +The gizmo and overlay systems don't share code today, and mostly shouldn't — but three concrete overlaps exist: + +1. **Ghost-renderable factory.** Silhouette entities (`silhouette_view.dart:246-260`), gizmo axis assets, wireframe boxes (`thermion_viewer_ffi.dart:892-986`) and bone overlays all do: create entity → build renderable with shared VB/IB → parent to target → track for disposal. One helper would remove four copies of the dispose-ordering pitfalls (and the gizmo's leak, finding §1.4-1, would have been caught once). Effort: **M**. +2. **"Follow target / constant screen size" transform update.** The gizmo's `update()` (distance-proportional scale, `gizmos.dart:426-438`) and translation-axis/grid fade logic solve the same camera-distance problem per system. A small camera-state cache (position + view/projection matrices refreshed once per frame or per camera-change notification) would serve gizmo, translation-axis, and any future screen-space widget, and fix the 2–3× redundant reads per event (§1.4-3). Effort: **S–M**. +3. **Picking.** Generation-1 gizmo used GPU `View::pick`; the current gizmo uses Dart screen-space math; the overlay doesn't pick. If the native gizmo path is deleted (§1.4-2), `Gizmo::pick`'s entity-matching logic (`Gizmo.hpp:163-218`) goes with it — no other consumer exists. + +--- + +## 4. Recommendations + +### Quick wins (≤ 1 day each) +1. Fix `TransformationGizmo.dispose()` to destroy assets/entities/material instances (§1.4-1). **S.** +2. Skip silhouette + edge passes when `highlightedEntities.isEmpty` via `RenderManager.setRenderable` (+ restore main view render target in composite mode) (§2.5-1). **S.** +3. Delete the four fully-commented-out gizmo files and decide the fate of the unused native `TGizmo` path + embedded gizmo glb blobs (§1.4-2). **S (deletion) / M (removal from bindings + hooks).** +4. Set outline params once per `setStencilHighlight` call, not per primitive (§2.5-3). **S.** +5. Cache camera matrices/position per event (or expose a single `getCameraFrame()` FFI call) to collapse the gizmo's redundant reads (§1.4-3). **S.** +6. Silhouette depth → DEPTH24; drop the per-call `setPriority(7)` in marker updates (§2.5-6, §1.4-4). **Trivial.** + +### Structural (≥ 1 week) +7. Key silhouettes per primitive (or accept and document single-primitive outlines) and make highlight add/remove symmetric (§2.5-3/7). **M.** +8. Composite-mode fast path: when no highlights, render main directly to the Flutter RT (no offscreen SRGB8_A8, no edge pass) (§2.5-2). **M, builds on 2.** +9. Ghost-renderable factory + camera-state cache shared by gizmo/wireframe/bbox/silhouette (§3.1/3.2). **M.** +10. Give the gizmo a proper per-frame hook (app render hooks) instead of pull-based `update()` from input events (§1.5). **S–M.** + +--- + +## 5. Verdict + +**Gizmo: reasonable for now.** The current Dart implementation is light by construction (no per-frame work, math-based picking, shared cached material). Fix the dispose leak, delete the dead generations, and it's in good shape; the per-event FFI chatter is real but small and easy to trim later. + +**Overlay: sound architecture, one thing worth fixing.** The two-pass silhouette/edge design is the right approach for arbitrary-geometry outlines (and already replaced a worse Flutter-widget implementation), resource lifetimes are carefully ordered, and per-frame Dart churn is zero. The one thing that needs fixing is the *unconditional* cost: two extra full-screen passes (plus an offscreen main-scene texture in composite mode) every frame purely because the overlay is enabled, even with nothing highlighted. That is a small, well-scoped fix (`setRenderable` gating) with a large payoff on mobile resolutions. + +--- + +## Appendix: key file references + +| Concern | Location | +|---|---| +| Current gizmo | `thermion_dart/lib/src/utils/src/gizmos.dart` | +| Gizmo input driving | `thermion_dart/lib/src/input/src/implementations/gizmo_attachment_delegate.dart:170-225` | +| Gizmo material | `materials/gizmo.mat` | +| Native (legacy) gizmo | `thermion_dart/native/src/scene/Gizmo.cpp`, `native/src/c_api/TGizmo.cpp` | +| Overlay manager | `thermion_dart/lib/src/filament/src/implementation/highlight_overlay_manager.dart` | +| Silhouette pass | `thermion_dart/lib/src/filament/src/implementation/silhouette_view.dart` | +| Edge pass | `thermion_dart/lib/src/filament/src/implementation/edge_detection_view.dart` | +| Overlay materials | `materials/silhouette.mat`, `materials/edge_outline.mat` | +| View wiring / attach order | `thermion_dart/lib/src/filament/src/implementation/ffi_view.dart:476-654` | +| Per-frame render loop | `thermion_dart/native/src/rendering/RenderManager.cpp:107-238` | +| Material packaging in build hook | `thermion_dart/hook/build.dart:130-145` | From ed77ebfd83105e6aee7dba7b99a825f26647e6e5 Mon Sep 17 00:00:00 2001 From: Nick Fisher Date: Sun, 16 Aug 2026 16:09:51 +0000 Subject: [PATCH 2/7] fix(gizmo): destroy resources on dispose, cache camera state per event TransformationGizmo.dispose() now tears down everything it created: glb assets go through viewer.destroyAsset, entities and material instances are destroyed, and repeat disposal is a no-op. GizmoCameraContext bundles the viewport/projection/view/model matrices and camera position fetched once per pointer event; update/hover/drag share one instance instead of re-reading the camera 2-3 times per event. update() also skips redundant root-transform writes when unchanged. Co-Authored-By: Claude --- .../gizmo_attachment_delegate.dart | 12 +- thermion_dart/lib/src/utils/src/gizmos.dart | 188 +++++++++++------- 2 files changed, 122 insertions(+), 78 deletions(-) diff --git a/thermion_dart/lib/src/input/src/implementations/gizmo_attachment_delegate.dart b/thermion_dart/lib/src/input/src/implementations/gizmo_attachment_delegate.dart index 361952b82..79f184fba 100644 --- a/thermion_dart/lib/src/input/src/implementations/gizmo_attachment_delegate.dart +++ b/thermion_dart/lib/src/input/src/implementations/gizmo_attachment_delegate.dart @@ -199,15 +199,17 @@ class GizmoAttachmentDelegate extends InputHandlerDelegate { if (_isDraggingGizmo) { final x = event.localPosition.x.toInt(); final y = event.localPosition.y.toInt(); - await _gizmo!.updateDrag(x, y); + await _gizmo!.updateDrag(x, y, context: await GizmoCameraContext.fetch(viewer)); _reportTransformChange(); - } else { - // Update gizmo position and check for hover - await _gizmo?.update(); + } else if (_gizmo != null) { + // Fetch camera state once and share it between the position + // update and hover picking to avoid redundant FFI reads. + final cameraContext = await GizmoCameraContext.fetch(viewer); + await _gizmo!.update(cameraPosition: cameraContext.cameraPosition); if (!_isDraggingGizmo) { final x = event.localPosition.x.toInt(); final y = event.localPosition.y.toInt(); - await _gizmo?.hover(x, y); + await _gizmo!.hover(x, y, context: cameraContext); } } break; diff --git a/thermion_dart/lib/src/utils/src/gizmos.dart b/thermion_dart/lib/src/utils/src/gizmos.dart index 5e33c2bcf..46256f308 100644 --- a/thermion_dart/lib/src/utils/src/gizmos.dart +++ b/thermion_dart/lib/src/utils/src/gizmos.dart @@ -6,6 +6,50 @@ enum TransformationGizmoType { translation, scale, rotation } enum GizmoAxis { x, y, z, none } +/// Camera/viewport state for a single input event. +/// +/// [TransformationGizmo.update], [pickAxis] and the drag handlers all need +/// the same camera matrices; fetching them once per event and passing this +/// object down avoids re-reading (and re-allocating) them 2-3 times per +/// pointer event. +class GizmoCameraContext { + final Viewport viewport; + final Matrix4 projectionMatrix; + final Matrix4 viewMatrix; + final Matrix4 modelMatrix; + final Vector3 cameraPosition; + + GizmoCameraContext({ + required this.viewport, + required this.projectionMatrix, + required this.viewMatrix, + required this.modelMatrix, + required this.cameraPosition, + }); + + static Future fetch(ThermionViewer viewer) async { + final camera = await viewer.getActiveCamera(); + final view = await viewer.view; + return GizmoCameraContext( + viewport: await view.getViewport(), + projectionMatrix: await camera.getProjectionMatrix(), + viewMatrix: await camera.getViewMatrix(), + modelMatrix: await camera.getModelMatrix(), + cameraPosition: await camera.getPosition(), + ); + } + + /// Projects a world-space point to screen (viewport) space. + Vector2 projectToScreen(Vector3 worldPos) { + final clipSpace = projectionMatrix * viewMatrix * Vector4(worldPos.x, worldPos.y, worldPos.z, 1.0); + final ndc = clipSpace / clipSpace.w; + return Vector2( + ((ndc.x + 1.0) / 2.0) * viewport.width.toDouble(), + ((1.0 - ndc.y) / 2.0) * viewport.height.toDouble(), + ); + } +} + class TransformationGizmo { final ThermionViewer viewer; @@ -43,6 +87,9 @@ class TransformationGizmo { Matrix4? _targetStartTransform; Matrix4? _lastComputedWorldTransform; // Last computed world transform for callback + // Last transform applied to the root entity (used to skip redundant writes) + Matrix4? _lastRootTransform; + // Hover state GizmoAxis _hoveredAxis = GizmoAxis.none; @@ -423,10 +470,9 @@ class TransformationGizmo { if (_isDisposed) return; // Always get camera position for scale calculation - final camera = await viewer.getActiveCamera(); + final camPos = cameraPosition ?? await (await viewer.getActiveCamera()).getPosition(); if (_isDisposed) return; - final camPos = cameraPosition ?? await camera.getPosition(); final dist = targetPos.distanceTo(camPos); // Scale proportionally to distance to maintain constant screen-space size @@ -435,35 +481,37 @@ class TransformationGizmo { final scale = dist * screenSizeFactor / _axisLength; final rootTransform = Matrix4.compose(targetPos, Quaternion.identity(), Vector3.all(scale)); + + // Input events fire far more often than the camera or target moves; + // skip the FFI write when the computed transform is unchanged. + if (_lastRootTransform != null && _transformsEqual(_lastRootTransform!, rootTransform)) { + return; + } await viewer.app.setTransform(_rootEntity!, rootTransform); + _lastRootTransform = rootTransform; + } + + static bool _transformsEqual(Matrix4 a, Matrix4 b) { + for (int i = 0; i < 16; i++) { + if (a.storage[i] != b.storage[i]) return false; + } + return true; } - Future pickAxis(int x, int y) async { + Future pickAxis(int x, int y, {GizmoCameraContext? context}) async { if (_isDisposed || _attachedTarget == null) return GizmoAxis.none; // Use screen-space picking to avoid depth buffer issues - final camera = await viewer.getActiveCamera(); + final ctx = context ?? await GizmoCameraContext.fetch(viewer); if (_isDisposed) return GizmoAxis.none; - final view = await viewer.view; - final viewport = await view.getViewport(); - final projectionMatrix = await camera.getProjectionMatrix(); - final viewMatrix = await camera.getViewMatrix(); - // Get gizmo world position final gizmoTransform = await viewer.app.transformManager.getWorldTransform(_rootEntity!); final gizmoWorldPos = gizmoTransform.getTranslation(); final gizmoScale = gizmoTransform.getColumn(0).xyz.length; // Project a point from world space to screen space - Vector2 projectToScreen(Vector3 worldPos) { - final clipSpace = projectionMatrix * viewMatrix * Vector4(worldPos.x, worldPos.y, worldPos.z, 1.0); - final ndc = clipSpace / clipSpace.w; - return Vector2( - ((ndc.x + 1.0) / 2.0) * viewport.width.toDouble(), - ((1.0 - ndc.y) / 2.0) * viewport.height.toDouble(), - ); - } + Vector2 projectToScreen(Vector3 worldPos) => ctx.projectToScreen(worldPos); // Distance from point to line segment in 2D double pointToSegmentDistance(Vector2 p, Vector2 a, Vector2 b) { @@ -543,11 +591,11 @@ class TransformationGizmo { /// floating point drift from reading back via getWorldTransform. Matrix4? get lastComputedWorldTransform => _lastComputedWorldTransform; - Future startDrag(int screenX, int screenY) async { + Future startDrag(int screenX, int screenY, {GizmoCameraContext? context}) async { if (_isDisposed || _attachedTarget == null) return false; // Pick to find which axis was clicked - _activeAxis = await pickAxis(screenX, screenY); + _activeAxis = await pickAxis(screenX, screenY, context: context); if (_isDisposed || _activeAxis == GizmoAxis.none) return false; // Store initial state @@ -559,7 +607,7 @@ class TransformationGizmo { // For rotation gizmos, position markers at click location on ring if (_type == TransformationGizmoType.rotation) { - final startPos = await _getMarkerPositionOnRing(screenX, screenY, _activeAxis); + final startPos = await _getMarkerPositionOnRing(screenX, screenY, _activeAxis, context: context); if (_isDisposed) return false; if (startPos != null) { @@ -578,10 +626,10 @@ class TransformationGizmo { return true; } - Future hover(int screenX, int screenY) async { + Future hover(int screenX, int screenY, {GizmoCameraContext? context}) async { if (_isDisposed || _activeAxis != GizmoAxis.none) return; - final hovered = await pickAxis(screenX, screenY); + final hovered = await pickAxis(screenX, screenY, context: context); if (_isDisposed) return; if (hovered != _hoveredAxis) { @@ -590,30 +638,28 @@ class TransformationGizmo { } } - Future updateDrag(int screenX, int screenY) async { + Future updateDrag(int screenX, int screenY, {GizmoCameraContext? context}) async { if (_isDisposed || _activeAxis == GizmoAxis.none || _attachedTarget == null) return; + final ctx = context ?? await GizmoCameraContext.fetch(viewer); + if (_isDisposed) return; + if (_type == TransformationGizmoType.rotation) { - await _updateRotationDrag(screenX, screenY); + await _updateRotationDrag(screenX, screenY, ctx); } else { - await _updateTranslationDrag(screenX, screenY); + await _updateTranslationDrag(screenX, screenY, ctx); } } - Future _updateTranslationDrag(int screenX, int screenY) async { + Future _updateTranslationDrag(int screenX, int screenY, GizmoCameraContext ctx) async { if (_isDisposed) return; final currentScreen = Vector2(screenX.toDouble(), screenY.toDouble()); final screenDelta = currentScreen - _dragStartScreen!; - final camera = await viewer.getActiveCamera(); - if (_isDisposed) return; - - final view = await viewer.view; - final viewport = await view.getViewport(); - - final projectionMatrix = await camera.getProjectionMatrix(); - final viewMatrix = await camera.getViewMatrix(); - final inverseViewMatrix = await camera.getModelMatrix(); + final viewport = ctx.viewport; + final projectionMatrix = ctx.projectionMatrix; + final viewMatrix = ctx.viewMatrix; + final inverseViewMatrix = ctx.modelMatrix; final inverseProjectionMatrix = projectionMatrix.clone()..invert(); // Re-check validity before using transforms @@ -667,24 +713,19 @@ class TransformationGizmo { // Update gizmo position directly only if still alive if (!_isDisposed) { - await update(position: newWorldPos); + await update(cameraPosition: ctx.cameraPosition, position: newWorldPos); } } } - Future _updateRotationDrag(int screenX, int screenY) async { + Future _updateRotationDrag(int screenX, int screenY, GizmoCameraContext ctx) async { if (_isDisposed) return; final currentScreen = Vector2(screenX.toDouble(), screenY.toDouble()); - final camera = await viewer.getActiveCamera(); - if (_isDisposed) return; - - final view = await viewer.view; - final viewport = await view.getViewport(); - - final projectionMatrix = await camera.getProjectionMatrix(); - final viewMatrix = await camera.getViewMatrix(); - final cameraPosition = await camera.getPosition(); + final viewport = ctx.viewport; + final projectionMatrix = ctx.projectionMatrix; + final viewMatrix = ctx.viewMatrix; + final cameraPosition = ctx.cameraPosition; if (_isDisposed || _targetStartTransform == null) return; @@ -703,7 +744,7 @@ class TransformationGizmo { // Update current marker position on the ring if (_currentMarker != null) { - final currentPos = await _getMarkerPositionOnRing(screenX, screenY, _activeAxis); + final currentPos = await _getMarkerPositionOnRing(screenX, screenY, _activeAxis, context: ctx); if (currentPos != null && !_isDisposed) { await _updateMarkerPosition(_currentMarker!, currentPos); } @@ -816,17 +857,17 @@ class TransformationGizmo { /// Get marker position on ring by projecting ring points to screen space /// and finding the closest point to the mouse cursor. - Future _getMarkerPositionOnRing(int screenX, int screenY, GizmoAxis axis) async { + Future _getMarkerPositionOnRing( + int screenX, + int screenY, + GizmoAxis axis, { + GizmoCameraContext? context, + }) async { if (_isDisposed || _rootEntity == null) return null; - final camera = await viewer.getActiveCamera(); + final ctx = context ?? await GizmoCameraContext.fetch(viewer); if (_isDisposed) return null; - final view = await viewer.view; - final viewport = await view.getViewport(); - final projectionMatrix = await camera.getProjectionMatrix(); - final viewMatrix = await camera.getViewMatrix(); - // Get gizmo world position and scale final gizmoTransform = await viewer.app.transformManager.getWorldTransform(_rootEntity!); final gizmoWorldPos = gizmoTransform.getTranslation(); @@ -837,12 +878,7 @@ class TransformationGizmo { // Project a point from local ring space to screen space Vector2 projectToScreen(Vector3 localPos) { final worldPos = gizmoWorldPos + localPos * gizmoScale; - final clipSpace = projectionMatrix * viewMatrix * Vector4(worldPos.x, worldPos.y, worldPos.z, 1.0); - final ndc = clipSpace / clipSpace.w; - return Vector2( - ((ndc.x + 1.0) / 2.0) * viewport.width.toDouble(), - ((1.0 - ndc.y) / 2.0) * viewport.height.toDouble(), - ); + return ctx.projectToScreen(worldPos); } // Distance from point to line segment in 2D @@ -936,6 +972,11 @@ class TransformationGizmo { viewer.app.transformManager.setParent(_currentMarker!, _rootEntity!); } + // Draw on top of the rings (set once here; the priority never changes, + // so it must not be re-set on every marker move). + await viewer.app.setPriority(_startMarker!, 7); + await viewer.app.setPriority(_currentMarker!, 7); + // Initially hide markers await _hideMarkers(); } @@ -959,7 +1000,6 @@ class TransformationGizmo { final markerTransform = Matrix4.compose(localPosition, Quaternion.identity(), Vector3.all(1.0)); await viewer.app.setTransform(marker, markerTransform); - await viewer.app.setPriority(marker, 7); } Future _updateHighlights() async { @@ -991,26 +1031,26 @@ class TransformationGizmo { if (_isDisposed) return; _isDisposed = true; // Set flag immediately - // Remove all assets from the scene + // Destroy the geometry assets (also removes them from the scene and + // frees their native vertex/index buffers). for (final asset in _assets) { - await viewer.removeFromScene(asset); - // If your API supports destroying entities explicitly, do it here. - // e.g. await viewer.app.removeEntity(asset.entity); + await viewer.destroyAsset(asset); } _assets.clear(); - // Destroy Root Entity + // Destroy root entity (its children are gone by now). if (_rootEntity != null) { - // Assuming removeEntity exists in your version of FilamentApp - // If not, just null it out, as child removal usually handles it. - try { - // await viewer.app.removeEntity(_rootEntity!); - } catch (e) { - // ignore - } + await viewer.app.destroyEntity(_rootEntity!); _rootEntity = null; } + // Destroy material instances. + await _redMat?.destroy(); + await _greenMat?.destroy(); + await _blueMat?.destroy(); + await _whiteMat?.destroy(); + await _yellowMat?.destroy(); + _startMarker = null; _currentMarker = null; _startMarkerAsset = null; @@ -1027,5 +1067,7 @@ class TransformationGizmo { _hoveredAxis = GizmoAxis.none; _dragStartScreen = null; _targetStartTransform = null; + _lastComputedWorldTransform = null; + _lastRootTransform = null; } } From 927e8165e499cae6d4bf43e72e62c3979ae65914 Mon Sep 17 00:00:00 2001 From: Nick Fisher Date: Sun, 16 Aug 2026 16:09:55 +0000 Subject: [PATCH 3/7] chore(gizmo): delete fully-commented-out legacy files gizmo.dart, gizmo_input_handler.dart, gizmo_pick_delegate.dart and RotationGizmo.cpp contained only commented-out code from an earlier iteration of the gizmo system. The live Dart gizmo (utils/src/gizmos.dart) and the native TGizmo path are untouched. Co-Authored-By: Claude --- thermion_dart/lib/src/input/input.dart | 2 - .../implementations/gizmo_input_handler.dart | 371 ------------------ .../implementations/gizmo_pick_delegate.dart | 41 -- thermion_dart/lib/src/utils/src/gizmo.dart | 115 ------ .../native/src/scene/RotationGizmo.cpp | 349 ---------------- 5 files changed, 878 deletions(-) delete mode 100644 thermion_dart/lib/src/input/src/implementations/gizmo_input_handler.dart delete mode 100644 thermion_dart/lib/src/input/src/implementations/gizmo_pick_delegate.dart delete mode 100644 thermion_dart/lib/src/utils/src/gizmo.dart delete mode 100644 thermion_dart/native/src/scene/RotationGizmo.cpp diff --git a/thermion_dart/lib/src/input/input.dart b/thermion_dart/lib/src/input/input.dart index 461c0c8ba..fd964de9a 100644 --- a/thermion_dart/lib/src/input/input.dart +++ b/thermion_dart/lib/src/input/input.dart @@ -4,8 +4,6 @@ export 'src/input_types.dart'; export 'src/input_handler.dart'; export 'src/delegate_input_handler.dart'; export 'src/implementations/default_pick_delegate.dart'; -export 'src/implementations/gizmo_pick_delegate.dart'; -export 'src/implementations/gizmo_input_handler.dart'; export 'src/implementations/third_person_camera_delegate.dart'; export 'src/implementations/chained_delegate.dart'; export 'src/implementations/gizmo_attachment_delegate.dart'; diff --git a/thermion_dart/lib/src/input/src/implementations/gizmo_input_handler.dart b/thermion_dart/lib/src/input/src/implementations/gizmo_input_handler.dart deleted file mode 100644 index 6d47075da..000000000 --- a/thermion_dart/lib/src/input/src/implementations/gizmo_input_handler.dart +++ /dev/null @@ -1,371 +0,0 @@ -// import 'dart:async'; -// import 'dart:math'; -// import 'package:thermion_dart/thermion_dart.dart'; - -// class _Gizmo { -// final ThermionViewer viewer; - -// final GizmoAsset _gizmo; - -// final transformUpdates = StreamController<({Matrix4 transform})>.broadcast(); - -// Axis? _active; -// final GizmoType type; - -// _Gizmo(this._gizmo, this.viewer, this.type); - -// static Future<_Gizmo> forType(ThermionViewer viewer, GizmoType type) async { -// final view = await viewer.view; -// return _Gizmo(await viewer.getGizmo(type), viewer, type); -// } - -// Future dispose() async { -// await transformUpdates.close(); -// await viewer.destroyAsset(_gizmo); -// } - -// Future hide() async { -// final scene = await viewer.view.getScene(); -// await scene.remove(_gizmo); -// } - -// Future reveal() async { -// final scene = await viewer.view.getScene(); -// await scene.add(_gizmo); -// gizmoTransform = await _gizmo.getWorldTransform(); -// } - -// double _getAngleBetweenVectors(Vector2 v1, Vector2 v2) { -// // Normalize vectors to ensure consistent rotation regardless of distance from center -// v1.normalize(); -// v2.normalize(); - -// // Calculate angle using atan2 -// double angle = atan2(v2.y, v2.x) - atan2(v1.y, v1.x); - -// // Ensure angle is between -π and π -// if (angle > pi) angle -= 2 * pi; -// if (angle < -pi) angle += 2 * pi; - -// return angle; -// } - -// void checkHover(int x, int y) async { -// _gizmo.pick(x, y, handler: (result, coords) async { -// switch (result) { -// case GizmoPickResultType.None: -// await _gizmo.unhighlight(); -// _active = null; -// break; -// case GizmoPickResultType.AxisX: -// _active = Axis.X; -// case GizmoPickResultType.AxisY: -// _active = Axis.Y; -// case GizmoPickResultType.AxisZ: -// _active = Axis.Z; -// default: -// } -// }); -// } - -// Matrix4? gizmoTransform; - -// void _updateTransform(Vector2 currentPosition, Vector2 delta) async { -// if (type == GizmoType.translation) { -// await _updateTranslation(currentPosition, delta); -// } else if (type == GizmoType.rotation) { -// await _updateRotation(currentPosition, delta); -// } - -// await _gizmo.setTransform(gizmoTransform!); - -// transformUpdates.add((transform: gizmoTransform!)); -// } - -// Future? _updateTranslation( -// Vector2 currentPosition, Vector2 delta) async { -// var view = await viewer.view; -// var camera = await viewer.getActiveCamera(); -// var viewport = await view.getViewport(); -// var projectionMatrix = await camera.getProjectionMatrix(); -// var viewMatrix = await camera.getViewMatrix(); -// var inverseViewMatrix = await camera.getModelMatrix(); -// var inverseProjectionMatrix = projectionMatrix.clone()..invert(); - -// // get gizmo position in screenspace -// var gizmoPositionWorldSpace = gizmoTransform!.getTranslation(); -// Vector4 gizmoClipSpace = projectionMatrix * -// viewMatrix * -// Vector4(gizmoPositionWorldSpace.x, gizmoPositionWorldSpace.y, -// gizmoPositionWorldSpace.z, 1.0); - -// var gizmoNdc = gizmoClipSpace / gizmoClipSpace.w; - -// var gizmoScreenSpace = Vector2(((gizmoNdc.x / 2) + 0.5) * viewport.width, -// viewport.height - (((gizmoNdc.y / 2) + 0.5) * viewport.height)); - -// gizmoScreenSpace += delta; - -// gizmoNdc = Vector4(((gizmoScreenSpace.x / viewport.width) - 0.5) * 2, -// (((gizmoScreenSpace.y / viewport.height)) - 0.5) * -2, gizmoNdc.z, 1.0); - -// var gizmoViewSpace = inverseProjectionMatrix * gizmoNdc; -// gizmoViewSpace /= gizmoViewSpace.w; - -// var newPosition = (inverseViewMatrix * gizmoViewSpace).xyz; - -// Vector3 worldSpaceDelta = newPosition - gizmoTransform!.getTranslation(); -// worldSpaceDelta.multiply(_active!.asVector()); - -// gizmoTransform! -// .setTranslation(gizmoTransform!.getTranslation() + worldSpaceDelta); -// } - -// Future? _updateRotation(Vector2 currentPosition, Vector2 delta) async { -// var camera = await viewer.view.getCamera(); -// var viewport = await viewer.view.getViewport(); -// var projectionMatrix = await camera.getProjectionMatrix(); -// var viewMatrix = await camera.getViewMatrix(); - -// // Get gizmo center in screen space -// var gizmoPositionWorldSpace = gizmoTransform!.getTranslation(); -// Vector4 gizmoClipSpace = projectionMatrix * -// viewMatrix * -// Vector4(gizmoPositionWorldSpace.x, gizmoPositionWorldSpace.y, -// gizmoPositionWorldSpace.z, 1.0); - -// var gizmoNdc = gizmoClipSpace / gizmoClipSpace.w; -// var gizmoScreenSpace = Vector2(((gizmoNdc.x / 2) + 0.5) * viewport.width, -// viewport.height - (((gizmoNdc.y / 2) + 0.5) * viewport.height)); - -// // Calculate vectors from gizmo center to previous and current mouse positions -// var prevVector = (currentPosition - delta) - gizmoScreenSpace; -// var currentVector = currentPosition - gizmoScreenSpace; - -// // Calculate rotation angle based on the active axis -// double rotationAngle = 0.0; -// switch (_active) { -// case Axis.X: -// // For X axis, project onto YZ plane -// var prev = Vector2(prevVector.y, -prevVector.x); -// var curr = Vector2(currentVector.y, -currentVector.x); -// rotationAngle = _getAngleBetweenVectors(prev, curr); -// break; -// case Axis.Y: -// // For Y axis, project onto XZ plane -// var prev = Vector2(prevVector.x, -prevVector.y); -// var curr = Vector2(currentVector.x, -currentVector.y); -// rotationAngle = _getAngleBetweenVectors(prev, curr); -// break; -// case Axis.Z: -// // For Z axis, use screen plane directly -// rotationAngle = -1 * _getAngleBetweenVectors(prevVector, currentVector); -// break; -// default: -// return; -// } - -// // Create rotation matrix based on the active axis -// var rotationMatrix = Matrix4.identity(); -// switch (_active) { -// case Axis.X: -// rotationMatrix.setRotationX(rotationAngle); -// break; -// case Axis.Y: -// rotationMatrix.setRotationY(rotationAngle); -// break; -// case Axis.Z: -// rotationMatrix.setRotationZ(rotationAngle); -// break; -// default: -// return; -// } - -// // Apply rotation to the current transform -// gizmoTransform = gizmoTransform! * rotationMatrix; -// } -// } - -// class GizmoInputHandler extends InputHandler { -// final ThermionViewer viewer; - -// late final _gizmos = {}; - -// _Gizmo? _active; - -// ThermionEntity? _attached; - -// Future attach(ThermionEntity entity) async { -// if (_attached != null) { -// await detach(); -// } -// _attached = entity; -// if (_active != null) { -// await FilamentApp.instance!.setParent(_attached!, _active!._gizmo.entity); -// await _active!.reveal(); -// } -// } - -// Future getGizmoTransform() async { -// return _active?.gizmoTransform; -// } - -// Future detach() async { -// if (_attached == null) { -// return; -// } -// await FilamentApp.instance!.setParent(_attached!, null); -// await _active?.hide(); -// _attached = null; -// } - -// final _initialized = Completer(); - -// final _transformController = StreamController.broadcast(); -// Stream get transformUpdated => _transformController.stream; - -// final _pickResultController = StreamController.broadcast(); -// Stream get onPickResult => _pickResultController.stream; - -// GizmoInputHandler({required this.viewer, required GizmoType initialType}) { -// initialize().then((_) { -// setGizmoType(initialType); -// }); -// } - -// GizmoType? getGizmoType() { -// return _active?.type; -// } - -// Future setGizmoType(GizmoType? type) async { -// if (type == null) { -// await detach(); -// _active?.hide(); -// _active = null; -// } else { -// _active?.hide(); -// _active = _gizmos[type]!; -// _active!.reveal(); -// if (_attached != null) { -// await attach(_attached!); -// } -// } -// } - -// Future initialize() async { -// if (_initialized.isCompleted) { -// throw Exception("Already initialized"); -// } -// await viewer.initialized; - -// _gizmos[GizmoType.translation] = -// await _Gizmo.forType(viewer, GizmoType.translation); -// _gizmos[GizmoType.rotation] = -// await _Gizmo.forType(viewer, GizmoType.rotation); -// await setGizmoType(GizmoType.translation); -// for (final gizmo in _gizmos.values) { -// gizmo.transformUpdates.stream.listen((update) { -// _transformController.add(update.transform); -// }); -// } -// _initialized.complete(true); -// } - -// @override -// Future dispose() async { -// _gizmos[GizmoType.rotation]!.dispose(); -// _gizmos[GizmoType.translation]!.dispose(); -// _gizmos.clear(); -// } - -// @override -// InputAction? getActionForType(InputType gestureType) { -// if (gestureType == InputType.LMB_DOWN) { -// return InputAction.PICK; -// } -// throw UnimplementedError(); -// } - -// @override -// Future get initialized => _initialized.future; - -// @override -// void keyDown(PhysicalKey key) {} - -// @override -// void keyUp(PhysicalKey key) {} - -// @override -// Future? onPointerDown(Vector2 localPosition, bool isMiddle) async { -// if (!_initialized.isCompleted) { -// return; -// } - -// if (isMiddle) { -// return; -// } - -// await viewer.view.pick(localPosition.x.toInt(), localPosition.y.toInt(), -// (result) async { -// if (_active?._gizmo.isNonPickable(result.entity) == true || -// result.entity == FILAMENT_ENTITY_NULL) { -// _pickResultController.add(null); -// return; -// } -// if (_active?._gizmo.isGizmoEntity(result.entity) != true) { -// _pickResultController.add(result.entity); -// } -// }); -// } - -// @override -// Future? onPointerHover(Vector2 localPosition, Vector2 delta) async { -// if (!_initialized.isCompleted) { -// return; -// } -// _active?.checkHover(localPosition.x.floor(), localPosition.y.floor()); -// } - -// @override -// Future? onPointerMove( -// Vector2 localPosition, Vector2 delta, bool isMiddle) async { -// if (!isMiddle && _active?._active != null) { -// final scaledDelta = Vector2( -// delta.x, -// delta.y, -// ); -// _active!._updateTransform(localPosition, scaledDelta); -// return; -// } -// } - -// @override -// Future? onPointerScroll( -// Vector2 localPosition, double scrollDelta) async {} - -// @override -// Future? onPointerUp(bool isMiddle) async {} - -// @override -// Future? onScaleEnd(int pointerCount, double velocity) {} - -// @override -// Future? onScaleStart( -// Vector2 focalPoint, int pointerCount, Duration? sourceTimestamp) {} - -// @override -// Future? onScaleUpdate( -// Vector2 focalPoint, -// Vector2 focalPointDelta, -// double horizontalScale, -// double verticalScale, -// double scale, -// int pointerCount, -// double rotation, -// Duration? sourceTimestamp) {} - -// @override -// void setActionForType(InputType gestureType, InputAction gestureAction) { -// throw UnimplementedError(); -// } -// } diff --git a/thermion_dart/lib/src/input/src/implementations/gizmo_pick_delegate.dart b/thermion_dart/lib/src/input/src/implementations/gizmo_pick_delegate.dart deleted file mode 100644 index 18db23d8e..000000000 --- a/thermion_dart/lib/src/input/src/implementations/gizmo_pick_delegate.dart +++ /dev/null @@ -1,41 +0,0 @@ -// import 'dart:async'; - -// import 'package:thermion_dart/thermion_dart.dart'; -// import 'package:vector_math/vector_math_64.dart'; - -// class GizmoPickDelegate extends PickDelegate { -// final ThermionViewer viewer; -// late final GizmoAsset translationGizmo; - -// GizmoPickDelegate(this.viewer) { -// initialize(); -// } - -// bool _initialized = false; -// Future initialize() async { -// if (_initialized) { -// throw Exception("Already initialized"); -// } -// final view = await viewer.getViewAt(0); -// translationGizmo = await viewer.createGizmo(view, GizmoType.translation); -// await translationGizmo.addToScene(); -// _initialized = true; -// } - -// final _picked = StreamController(); -// Stream get picked => _picked.stream; - -// Future dispose() async { -// _picked.close(); -// } - -// @override -// void pick(Vector2 location) { -// if (!_initialized) { -// return; -// } -// viewer.pick(location.x.toInt(), location.y.toInt(), (result) { -// translationGizmo.attach(result.entity); -// }); -// } -// } diff --git a/thermion_dart/lib/src/utils/src/gizmo.dart b/thermion_dart/lib/src/utils/src/gizmo.dart deleted file mode 100644 index 27ff70fca..000000000 --- a/thermion_dart/lib/src/utils/src/gizmo.dart +++ /dev/null @@ -1,115 +0,0 @@ -// abstract class BaseGizmo extends Gizmo { -// final ThermionEntity x; -// final ThermionEntity y; -// final ThermionEntity z; -// final ThermionEntity center; - -// ThermionEntity? _activeAxis; -// ThermionEntity? _activeEntity; -// ThermionViewer viewer; - -// bool _visible = false; -// bool get isVisible => _visible; - -// bool _isHovered = false; -// bool get isHovered => _isHovered; - -// final Set ignore; - -// Stream get boundingBox => _boundingBoxController.stream; -// final _boundingBoxController = StreamController.broadcast(); - -// ThermionEntity get entity => center; - -// BaseGizmo( -// {required this.x, -// required this.y, -// required this.z, -// required this.center, -// required this.viewer, -// this.ignore = const {}}) { -// onPick(_onGizmoPickResult); -// } - -// final _stopwatch = Stopwatch(); - -// double _transX = 0.0; -// double _transY = 0.0; - -// Future translate(double transX, double transY) async { -// if (!_stopwatch.isRunning) { -// _stopwatch.start(); -// } - -// _transX += transX; -// _transY += transY; - -// if (_stopwatch.elapsedMilliseconds < 16) { -// return; -// } - -// final axis = Vector3(_activeAxis == x ? 1.0 : 0.0, -// _activeAxis == y ? 1.0 : 0.0, _activeAxis == z ? 1.0 : 0.0); - -// await viewer.queueRelativePositionUpdateWorldAxis( -// _activeEntity!, -// _transX, -// -_transY, // flip the sign because "up" in NDC Y axis is positive, but negative in Flutter -// axis.x, -// axis.y, -// axis.z); -// _transX = 0; -// _transY = 0; -// _stopwatch.reset(); -// } - -// void reset() { -// _activeAxis = null; -// } - -// void _onGizmoPickResult(FilamentPickResult result) async { -// if (result.entity == x || result.entity == y || result.entity == z) { -// _activeAxis = result.entity; -// _isHovered = true; -// } else if (result.entity == 0) { -// _activeAxis = null; -// _isHovered = false; -// } else { -// throw Exception("Unexpected gizmo pick result"); -// } -// } - -// Future attach(ThermionEntity entity) async { -// _activeAxis = null; -// if (entity == _activeEntity) { -// return; -// } -// if (entity == center) { -// _activeEntity = null; -// return; -// } -// _visible = true; - -// if (_activeEntity != null) { -// // await viewer.removeStencilHighlight(_activeEntity!); -// } -// _activeEntity = entity; - -// await viewer.setParent(center, entity, preserveScaling: false); -// _boundingBoxController.sink.add(await viewer.getViewportBoundingBox(x)); -// } - -// Future detach() async { -// await setVisibility(false); -// } - -// @override -// void checkHover(int x, int y) { -// pick(x, y); -// } - -// Future pick(int x, int y); - -// Future setVisibility(bool visible); -// void onPick(void Function(PickResult result) callback); -// } diff --git a/thermion_dart/native/src/scene/RotationGizmo.cpp b/thermion_dart/native/src/scene/RotationGizmo.cpp deleted file mode 100644 index b7a926c1a..000000000 --- a/thermion_dart/native/src/scene/RotationGizmo.cpp +++ /dev/null @@ -1,349 +0,0 @@ -// #include -// #include -// #include - -// #include - -// #include -// #include - -// #include - -// #include "scene/SceneManager.hpp" - -// namespace thermion { - -// using namespace filament::gltfio; - -// RotationGizmo::RotationGizmo(Engine* engine, View* view, Scene* scene, Material* material) -// : _engine(engine), _view(view), _scene(scene), _material(material) { - -// auto& entityManager = EntityManager::get(); -// auto& transformManager = _engine->getTransformManager(); - -// // Create center cube -// auto parentEntity = entityManager.create(); -// auto* parentMaterialInstance = _material->createInstance(); -// parentMaterialInstance->setParameter("baseColorFactor", math::float4{0.0f, 0.0f, 0.0f, 1.0f}); -// parentMaterialInstance->setParameter("scale", 4.0f); - -// _entities[0] = parentEntity; -// _materialInstances[0] = parentMaterialInstance; - -// // Create center cube geometry -// float centerCubeSize = 0.01f; -// float* centerCubeVertices = new float[8 * 3]{ -// -centerCubeSize, -centerCubeSize, -centerCubeSize, -// centerCubeSize, -centerCubeSize, -centerCubeSize, -// centerCubeSize, centerCubeSize, -centerCubeSize, -// -centerCubeSize, centerCubeSize, -centerCubeSize, -// -centerCubeSize, -centerCubeSize, centerCubeSize, -// centerCubeSize, -centerCubeSize, centerCubeSize, -// centerCubeSize, centerCubeSize, centerCubeSize, -// -centerCubeSize, centerCubeSize, centerCubeSize -// }; - -// uint16_t* centerCubeIndices = new uint16_t[36]{ -// 0, 1, 2, 2, 3, 0, -// 1, 5, 6, 6, 2, 1, -// 5, 4, 7, 7, 6, 5, -// 4, 0, 3, 3, 7, 4, -// 3, 2, 6, 6, 7, 3, -// 4, 5, 1, 1, 0, 4 -// }; - -// auto centerCubeVb = VertexBuffer::Builder() -// .vertexCount(8) -// .bufferCount(1) -// .attribute(VertexAttribute::POSITION, 0, VertexBuffer::AttributeType::FLOAT3) -// .build(*engine); - -// centerCubeVb->setBufferAt(*engine, 0, -// VertexBuffer::BufferDescriptor(centerCubeVertices, 8 * sizeof(filament::math::float3), -// [](void* buffer, size_t size, void*) { delete[] static_cast(buffer); })); - -// auto centerCubeIb = IndexBuffer::Builder() -// .indexCount(36) -// .bufferType(IndexBuffer::IndexType::USHORT) -// .build(*engine); - -// centerCubeIb->setBuffer(*engine, -// IndexBuffer::BufferDescriptor(centerCubeIndices, 36 * sizeof(uint16_t), -// [](void* buffer, size_t size, void*) { delete[] static_cast(buffer); })); - -// RenderableManager::Builder(1) -// .boundingBox({{-centerCubeSize, -centerCubeSize, -centerCubeSize}, -// {centerCubeSize, centerCubeSize, centerCubeSize}}) -// .material(0, parentMaterialInstance) -// .layerMask(0xFF, 1u << SceneManager::LAYERS::OVERLAY) -// .priority(7) -// .geometry(0, RenderableManager::PrimitiveType::TRIANGLES, centerCubeVb, centerCubeIb, 0, 36) -// .culling(false) -// .build(*engine, parentEntity); - -// // Create rotation circles -// constexpr int segments = 32; -// float radius = 0.5f; -// float* vertices; -// uint16_t* indices; -// int vertexCount, indexCount; - -// createCircle(radius, segments, vertices, indices, vertexCount, indexCount); - -// auto vb = VertexBuffer::Builder() -// .vertexCount(vertexCount) -// .bufferCount(1) -// .attribute(VertexAttribute::POSITION, 0, VertexBuffer::AttributeType::FLOAT3) -// .build(*engine); - -// vb->setBufferAt(*engine, 0, -// VertexBuffer::BufferDescriptor(vertices, vertexCount * sizeof(filament::math::float3), -// [](void* buffer, size_t size, void*) { delete[] static_cast(buffer); })); - -// auto ib = IndexBuffer::Builder() -// .indexCount(indexCount) -// .bufferType(IndexBuffer::IndexType::USHORT) -// .build(*engine); - -// ib->setBuffer(*engine, -// IndexBuffer::BufferDescriptor(indices, indexCount * sizeof(uint16_t), -// [](void* buffer, size_t size, void*) { delete[] static_cast(buffer); })); - -// // Create the three circular rotation handles -// for (int i = 0; i < 3; i++) { -// auto* materialInstance = _material->createInstance(); -// auto entity = entityManager.create(); -// _entities[i + 1] = entity; -// _materialInstances[i + 1] = materialInstance; - -// auto baseColor = inactiveColors[i]; -// math::mat4f transform; - -// switch (i) { -// case Axis::X: -// transform = math::mat4f::rotation(math::F_PI_2, math::float3{0, 1, 0}); -// break; -// case Axis::Y: -// transform = math::mat4f::rotation(math::F_PI_2, math::float3{1, 0, 0}); -// break; -// case Axis::Z: -// break; -// } - -// materialInstance->setParameter("baseColorFactor", baseColor); -// materialInstance->setParameter("scale", 4.0f); - -// RenderableManager::Builder(1) -// .boundingBox({{-radius, -radius, -0.01f}, {radius, radius, 0.01f}}) -// .material(0, materialInstance) -// .geometry(0, RenderableManager::PrimitiveType::TRIANGLES, vb, ib, 0, indexCount) -// .priority(6) -// .layerMask(0xFF, 1u << SceneManager::LAYERS::OVERLAY) -// .culling(false) -// .receiveShadows(false) -// .castShadows(false) -// .build(*engine, entity); - -// auto transformInstance = transformManager.getInstance(entity); -// transformManager.setTransform(transformInstance, transform); -// transformManager.setParent(transformInstance, transformManager.getInstance(parentEntity)); -// } - -// createHitTestEntities(); -// setVisibility(true); -// } - -// void RotationGizmo::createCircle(float radius, int segments, float*& vertices, uint16_t*& indices, int& vertexCount, int& indexCount) { -// vertexCount = segments * 2; -// indexCount = segments * 6; - -// vertices = new float[vertexCount * 3]; -// indices = new uint16_t[indexCount]; - -// float thickness = 0.01f; - -// // Generate vertices for inner and outer circles -// for (int i = 0; i < segments; i++) { -// float angle = (2.0f * M_PI * i) / segments; -// float x = cosf(angle); -// float y = sinf(angle); - -// // Inner circle vertex -// vertices[i * 6] = x * (radius - thickness); -// vertices[i * 6 + 1] = y * (radius - thickness); -// vertices[i * 6 + 2] = 0.0f; - -// // Outer circle vertex -// vertices[i * 6 + 3] = x * (radius + thickness); -// vertices[i * 6 + 4] = y * (radius + thickness); -// vertices[i * 6 + 5] = 0.0f; -// } - -// // Generate indices for triangles -// for (int i = 0; i < segments; i++) { -// int next = (i + 1) % segments; - -// // First triangle -// indices[i * 6] = i * 2; -// indices[i * 6 + 1] = i * 2 + 1; -// indices[i * 6 + 2] = next * 2 + 1; - -// // Second triangle -// indices[i * 6 + 3] = i * 2; -// indices[i * 6 + 4] = next * 2 + 1; -// indices[i * 6 + 5] = next * 2; -// } -// } - -// void RotationGizmo::createHitTestEntities() { -// auto& entityManager = EntityManager::get(); -// auto& transformManager = _engine->getTransformManager(); - -// float radius = 0.5f; -// float thickness = 0.1f; - -// // Create hit test volumes for each rotation circle -// for (int i = 4; i < 7; i++) { -// _entities[i] = entityManager.create(); -// _materialInstances[i] = _material->createInstance(); - -// _materialInstances[i]->setParameter("baseColorFactor", math::float4{0.0f, 0.0f, 0.0f, 0.0f}); -// _materialInstances[i]->setParameter("scale", 4.0f); - -// math::mat4f transform; -// switch (i - 4) { -// case Axis::X: -// transform = math::mat4f::rotation(math::F_PI_2, math::float3{0, 1, 0}); -// break; -// case Axis::Y: -// transform = math::mat4f::rotation(math::F_PI_2, math::float3{1, 0, 0}); -// break; -// case Axis::Z: -// break; -// } - -// // Create a thicker invisible volume aroun - -// // Create a thicker invisible volume around each rotation circle for hit testing -// float* volumeVertices; -// uint16_t* volumeIndices; -// int volumeVertexCount, volumeIndexCount; -// createCircle(radius, 32, volumeVertices, volumeIndices, volumeVertexCount, volumeIndexCount); - -// auto volumeVb = VertexBuffer::Builder() -// .vertexCount(volumeVertexCount) -// .bufferCount(1) -// .attribute(VertexAttribute::POSITION, 0, VertexBuffer::AttributeType::FLOAT3) -// .build(*_engine); - -// volumeVb->setBufferAt(*_engine, 0, -// VertexBuffer::BufferDescriptor(volumeVertices, volumeVertexCount * sizeof(filament::math::float3), -// [](void* buffer, size_t size, void*) { delete[] static_cast(buffer); })); - -// auto volumeIb = IndexBuffer::Builder() -// .indexCount(volumeIndexCount) -// .bufferType(IndexBuffer::IndexType::USHORT) -// .build(*_engine); - -// volumeIb->setBuffer(*_engine, -// IndexBuffer::BufferDescriptor(volumeIndices, volumeIndexCount * sizeof(uint16_t), -// [](void* buffer, size_t size, void*) { delete[] static_cast(buffer); })); - -// RenderableManager::Builder(1) -// .boundingBox({{-radius, -radius, -thickness/2}, {radius, radius, thickness/2}}) -// .material(0, _materialInstances[i]) -// .geometry(0, RenderableManager::PrimitiveType::TRIANGLES, volumeVb, volumeIb, 0, volumeIndexCount) -// .priority(7) -// .layerMask(0xFF, 1u << SceneManager::LAYERS::OVERLAY) -// .culling(false) -// .receiveShadows(false) -// .castShadows(false) -// .build(*_engine, _entities[i]); - -// auto instance = transformManager.getInstance(_entities[i]); -// transformManager.setTransform(instance, transform); -// transformManager.setParent(instance, transformManager.getInstance(_entities[0])); -// } -// } - -// RotationGizmo::~RotationGizmo() { -// _scene->removeEntities(_entities, 7); - -// for (int i = 0; i < 7; i++) { -// _engine->destroy(_entities[i]); -// _engine->destroy(_materialInstances[i]); -// } -// } - -// void RotationGizmo::highlight(Entity entity) { -// auto& rm = _engine->getRenderableManager(); -// auto renderableInstance = rm.getInstance(entity); -// auto materialInstance = rm.getMaterialInstanceAt(renderableInstance, 0); - -// math::float4 baseColor; -// if (entity == x()) { -// baseColor = activeColors[Axis::X]; -// } else if (entity == y()) { -// baseColor = activeColors[Axis::Y]; -// } else if (entity == z()) { -// baseColor = activeColors[Axis::Z]; -// } else { -// baseColor = math::float4{1.0f, 1.0f, 1.0f, 1.0f}; -// } - -// materialInstance->setParameter("baseColorFactor", baseColor); -// } - -// void RotationGizmo::unhighlight() { -// auto& rm = _engine->getRenderableManager(); - -// for (int i = 0; i < 3; i++) { -// auto renderableInstance = rm.getInstance(_entities[i + 1]); -// auto materialInstance = rm.getMaterialInstanceAt(renderableInstance, 0); -// materialInstance->setParameter("baseColorFactor", inactiveColors[i]); -// } -// } - -// void RotationGizmo::pick(uint32_t x, uint32_t y, PickCallback callback) { -// auto handler = new RotationGizmo::PickCallbackHandler(this, callback); -// _view->pick(x, y, [=](filament::View::PickingQueryResult const& result) { -// handler->handle(result); -// delete handler; -// }); -// } - -// void RotationGizmo::PickCallbackHandler::handle(filament::View::PickingQueryResult const& result) { -// auto x = static_cast(result.fragCoords.x); -// auto y = static_cast(result.fragCoords.y); - -// for (int i = 0; i < 7; i++) { -// if (_gizmo->_entities[i] == result.renderable) { -// if (i < 4) { -// return; -// } -// _gizmo->highlight(_gizmo->_entities[i - 4]); -// _callback(static_cast(i - 4), x, y, _gizmo->_view); -// return; -// } -// } -// _gizmo->unhighlight(); -// } - -// bool RotationGizmo::isGizmoEntity(Entity e) { -// for (int i = 0; i < 7; i++) { -// if (e == _entities[i]) { -// return true; -// } -// } -// return false; -// } - -// void RotationGizmo::setVisibility(bool visible) { -// if (visible) { -// _scene->addEntities(_entities, 7); -// } else { -// _scene->removeEntities(_entities, 7); -// } -// } - -// } \ No newline at end of file From 28e2524d64602947a36f4282c2383c2e04f40b08 Mon Sep 17 00:00:00 2001 From: Nick Fisher Date: Sun, 16 Aug 2026 16:10:02 +0000 Subject: [PATCH 4/7] perf(overlay): skip silhouette/edge passes when nothing is highlighted The two overlay views cost two extra full-screen passes every frame while the overlay is enabled, even with an empty highlight set. FFIHighlightOverlayManager now flips setRenderable on the silhouette and edge views at empty<->non-empty transitions (_applySuspension) - no resources are destroyed or recreated. In composite mode the main view is pointed back at the Flutter-provided render target while suspended, via the new FFIView.setRenderTargetDirect (bypasses the setRenderTarget interception that redirects Flutter targets to the edge view). FilamentApp.capture rendered every attached view, including non-renderable ones; the paused edge view shares the Flutter render target and its render cleared the target after the main view had drawn into it. Capture now renders only renderable views (new RenderManager.isRenderable) but still reads back every attached view. Also: setStencilHighlight sets outline params once per call instead of once per primitive, silhouettes are deduplicated per (entity, primitive) so multi-primitive entities outline fully, and the silhouette depth buffer is DEPTH24 (create and resize paths). Co-Authored-By: Claude --- .../filament/src/implementation/ffi_view.dart | 45 +++-- .../highlight_overlay_manager.dart | 162 +++++++++++++----- .../src/implementation/silhouette_view.dart | 67 ++++++-- 3 files changed, 202 insertions(+), 72 deletions(-) diff --git a/thermion_dart/lib/src/filament/src/implementation/ffi_view.dart b/thermion_dart/lib/src/filament/src/implementation/ffi_view.dart index b406f6d61..e46f95204 100644 --- a/thermion_dart/lib/src/filament/src/implementation/ffi_view.dart +++ b/thermion_dart/lib/src/filament/src/implementation/ffi_view.dart @@ -53,17 +53,6 @@ class FFIView extends View> { @override Future setRenderTarget(RenderTarget? renderTarget) async { - // When highlight overlay is enabled, the main view renders to an internal - // render target. Flutter-provided render targets go to EdgeDetectionView. - if (_highlightOverlayManager != null && renderTarget != null) { - final isInternalRT = _highlightOverlayManager!.isInternalRenderTarget(renderTarget); - if (!isInternalRT) { - // This is a Flutter RT - redirect to EdgeDetectionView - await _highlightOverlayManager!.setRenderTarget(this, renderTarget as FFIRenderTarget); - return; - } - } - if (renderTarget != null) { await withVoidCallback( (requestId, cb) => View_setRenderTargetRenderThread(view, renderTarget.getNativeHandle(), requestId, cb), @@ -75,6 +64,17 @@ class FFIView extends View> { } } + @override + Future setPresentationRenderTarget(RenderTarget? renderTarget) async { + final overlay = _highlightOverlayManager; + if (overlay == null) { + await setRenderTarget(renderTarget); + return; + } + + await overlay.setRenderTarget(this, renderTarget); + } + @override Future setCamera(Camera? camera) async { if (camera == null) { @@ -497,6 +497,13 @@ class FFIView extends View> { } // Configure output: render target (macOS/iOS) or swapchain (web/Android) + // A newly-created overlay has no highlights. Record both auxiliary passes + // as inactive before attaching them so no intermediate attachment sync can + // submit an empty full-screen pass. + await rm.setRenderables({ + _highlightOverlayManager!.silhouetteView: false, + _highlightOverlayManager!.overlayView: false, + }); await rm.detach(this, swapChain: swapChains.first); await rm.attach(_highlightOverlayManager!.silhouetteView, swapChains.first, renderOrder: 0); await rm.attach(this, swapChains.first, renderOrder: 1); @@ -590,6 +597,13 @@ class FFIView extends View> { return; } + // Apply the outline appearance once per call. addHighlight is invoked + // once per primitive below; setting the material parameters there would + // re-upload the same values N times for an N-primitive entity (and the + // consecutive-color-update behavior relies on this running even when the + // entity is already highlighted). + await _highlightOverlayManager!.setOutlineParams(width: outlineWidth, r: r, g: g, b: b); + // Get the primitive count for this entity final primCount = await _app.getPrimitiveCount(entity); @@ -617,16 +631,15 @@ class FFIView extends View> { final indexCount = IndexBuffer_getIndexCount(indexBuffer); final ffiIndexBuffer = FFIIndexBuffer(indexBuffer, _app.engine); - // Create silhouette for this primitive + // Create silhouette for this primitive (deduplicated per + // entity+primitive, so re-highlighting an entity doesn't duplicate + // renderables) await _highlightOverlayManager!.addHighlight( target: entity, vertexBuffer: vertexBuffer, indexBuffer: ffiIndexBuffer, indexCount: indexCount, - outlineWidth: outlineWidth, - r: r, - g: g, - b: b, + primitiveKey: flatPrimIndex, ); } diff --git a/thermion_dart/lib/src/filament/src/implementation/highlight_overlay_manager.dart b/thermion_dart/lib/src/filament/src/implementation/highlight_overlay_manager.dart index df6bdb927..a8790a407 100644 --- a/thermion_dart/lib/src/filament/src/implementation/highlight_overlay_manager.dart +++ b/thermion_dart/lib/src/filament/src/implementation/highlight_overlay_manager.dart @@ -1,3 +1,5 @@ +import 'dart:async'; + import 'package:logging/logging.dart'; import 'package:thermion_dart/src/filament/src/implementation/ffi_render_target.dart'; import 'package:thermion_dart/src/filament/src/implementation/ffi_texture.dart'; @@ -12,11 +14,20 @@ abstract class HighlightOverlayManager { Future setSwapChain(SwapChain swapChain); Future setCamera(Camera? camera); Future setViewport(int width, int height); - Future setRenderTarget(View mainView, RenderTarget renderTarget); + Future setRenderTarget(View mainView, RenderTarget? renderTarget); Future destroy(); - bool isInternalRenderTarget(RenderTarget renderTarget); Set get highlightedEntities; + /// Updates the outline appearance on the edge-detection material. + /// + /// Callers that add several highlights in a row (e.g. once per primitive of + /// an entity) should set these once per user-facing operation rather than + /// once per [addHighlight] call. + Future setOutlineParams({double? width, double? r, double? g, double? b}); + + /// Whether the overlay passes are currently skipped. + bool get suspended; + Future addHighlight({ required ThermionEntity target, required VertexBuffer vertexBuffer, @@ -26,6 +37,7 @@ abstract class HighlightOverlayManager { double r = 1.0, double g = 0.0, double b = 0.0, + int? primitiveKey, }); Future removeHighlight(ThermionEntity target); @@ -107,11 +119,24 @@ class FFIHighlightOverlayManager extends HighlightOverlayManager { @override Set get highlightedEntities => Set.unmodifiable(_highlightedEntities); + /// Whether the silhouette/edge passes are currently skipped. + /// + /// Rendering the two overlay views costs two extra full-screen passes per + /// frame even when they draw nothing, so they are marked non-renderable + /// whenever the highlight set is empty. The initial value matches the + /// attached-and-renderable state the views get when the overlay is enabled; + /// [_reconcileRenderPlan] applies the desired state after every input change. + bool _suspended = false; + + @override + bool get suspended => _suspended; + // State View? _mainView; - RenderTarget? _originalMainViewRenderTarget; SwapChain? _swapChain; - RenderTarget? _flutterRenderTarget; + RenderTarget? _presentationRenderTarget; + + Future _reconcileChain = Future.value(); // Internal render target for main view (composite mode only) Texture? _mainViewColorTexture; @@ -126,26 +151,24 @@ class FFIHighlightOverlayManager extends HighlightOverlayManager { /// /// Can be called multiple times (e.g. on resize) — will update the /// Flutter render target that EdgeDetectionView outputs to. - Future setRenderTarget(View mainView, RenderTarget flutterRenderTarget) async { + Future setRenderTarget(View mainView, RenderTarget? presentationRenderTarget) async { _mainView = mainView; - if (_flutterRenderTarget == null) { - // First time — set up the internal RT and redirect main view - _originalMainViewRenderTarget = await mainView.getRenderTarget(); - + if (_mainViewRenderTarget == null && presentationRenderTarget != null) { final vp = await mainView.getViewport(); final width = vp.width > 0 ? vp.width : 1; final height = vp.height > 0 ? vp.height : 1; await _createMainViewRenderTarget(width, height); - await mainView.setRenderTarget(_mainViewRenderTarget); await overlayView.setMainSceneTexture(_mainViewColorTexture!); - _logger.info("Main view redirected to internal render target (composite mode)"); + _logger.info("Main-view composite target initialized"); } - _flutterRenderTarget = flutterRenderTarget; - await overlayView.setRenderTarget(flutterRenderTarget); - _logger.info("EdgeDetectionView configured for render target output"); + _presentationRenderTarget = presentationRenderTarget; + await overlayView.setRenderTarget(presentationRenderTarget); + _logger.info("Presentation render target updated"); + + await _reconcileRenderPlan(); } /// Set the swapchain for overlay mode (web/Android). @@ -163,13 +186,62 @@ class FFIHighlightOverlayManager extends HighlightOverlayManager { _swapChain = swapChain; await overlayView.setOverlayOnly(true); _logger.info("EdgeDetectionView registered with swapchain (overlay-only mode)"); + + await _reconcileRenderPlan(); } - /// Check if the given render target is the internal one used for main view - /// (as opposed to a Flutter-provided render target). - /// Used by FFIView.setRenderTarget() to determine if it should intercept. - bool isInternalRenderTarget(RenderTarget rt) { - return rt == _mainViewRenderTarget; + /// Reconciles target routing and active passes from current desired state. + /// + /// This is deliberately idempotent: viewport and presentation-target + /// changes must re-apply routing even when the highlight set is unchanged. + /// Reconciliations are serialized so overlapping lifecycle calls cannot + /// publish an older plan after a newer one. + Future _reconcileRenderPlan() { + final completer = Completer(); + final previous = _reconcileChain; + _reconcileChain = completer.future.then((_) {}, onError: (_) {}); + return previous.then((_) async { + try { + await _applyRenderPlan(); + completer.complete(); + } catch (error, stackTrace) { + completer.completeError(error, stackTrace); + rethrow; + } + }); + } + + Future _applyRenderPlan() async { + final hasOutput = _mainView == null || _presentationRenderTarget != null; + final shouldSuspend = _highlightedEntities.isEmpty || !hasOutput; + final stateChanged = _suspended != shouldSuspend; + final rm = _app.renderManager; + + if (shouldSuspend) { + // Remove both overlay passes in one plan update before routing the main + // view directly to its output. No frame can run an edge pass against a + // directly-rendered main view. + await rm.setRenderables({silhouetteView: false, overlayView: false}); + if (_mainView != null) { + await _mainView!.setRenderTarget(_presentationRenderTarget); + } + } else { + // Route the main pass to its sampleable target before publishing the + // overlay passes together. + if (_mainView != null && _mainViewRenderTarget != null) { + await _mainView!.setRenderTarget(_mainViewRenderTarget); + } + await rm.setRenderables({silhouetteView: true, overlayView: true}); + } + + _suspended = shouldSuspend; + if (stateChanged) { + _logger.info( + shouldSuspend + ? "Overlay passes suspended" + : "Overlay passes resumed (${_highlightedEntities.length} highlights)", + ); + } } final FFIFilamentApp _app; @@ -272,11 +344,10 @@ class FFIHighlightOverlayManager extends HighlightOverlayManager { // Create new resources FIRST await _createMainViewRenderTarget(width, height); - // Update all references before destroying old resources - if (_mainView != null) { - await _mainView!.setRenderTarget(_mainViewRenderTarget); - } + // Update all references and re-apply the current route before destroying + // resources that may still be bound by the active plan. await overlayView.setMainSceneTexture(_mainViewColorTexture!); + await _reconcileRenderPlan(); // Flush render thread to ensure new textures are bound before destroying old ones // This prevents "Invalid texture still bound to MaterialInstance" errors @@ -305,7 +376,20 @@ class FFIHighlightOverlayManager extends HighlightOverlayManager { } } + /// Updates the outline appearance on the edge-detection material. + @override + Future setOutlineParams({double? width, double? r, double? g, double? b}) async { + await overlayView.setOutlineParams(width: width, r: r, g: g, b: b); + } + /// Add a highlight for an entity with the specified geometry. + /// + /// [primitiveKey] deduplicates silhouettes per primitive when this is + /// called once per primitive of the same entity. Outline appearance is + /// NOT set here — callers set it once per user-facing operation via + /// [setOutlineParams] (per-primitive calls would re-upload the same + /// material parameters N times). + @override Future addHighlight({ required ThermionEntity target, required VertexBuffer vertexBuffer, @@ -315,27 +399,24 @@ class FFIHighlightOverlayManager extends HighlightOverlayManager { double r = 1.0, double g = 0.0, double b = 0.0, + int? primitiveKey, }) async { - // ALWAYS update outline params (even if already highlighted) - await overlayView.setOutlineParams(width: outlineWidth, r: r, g: g, b: b); - - // Only add silhouette if not already tracked - if (_highlightedEntities.contains(target)) { - return; - } - - // Add silhouette to first pass - await silhouetteView.addHighlight( + final created = await silhouetteView.addHighlight( target: target, vertexBuffer: vertexBuffer, indexBuffer: indexBuffer, indexCount: indexCount, + primitiveKey: primitiveKey, ); - _highlightedEntities.add(target); + if (created) { + _highlightedEntities.add(target); + await _reconcileRenderPlan(); + } } /// Remove highlight from an entity. + @override Future removeHighlight(ThermionEntity target) async { if (!_highlightedEntities.contains(target)) { return; @@ -343,6 +424,7 @@ class FFIHighlightOverlayManager extends HighlightOverlayManager { await silhouetteView.removeHighlight(target); _highlightedEntities.remove(target); + await _reconcileRenderPlan(); } /// Remove all highlights. @@ -369,10 +451,11 @@ class FFIHighlightOverlayManager extends HighlightOverlayManager { // NOW safe to destroy SilhouetteView (which destroys the silhouette texture) await silhouetteView.destroy(); - // Tear down render targets and restore original state - // Restore main view's original render target (only if it was redirected) - if (_mainView != null && _mainViewRenderTarget != null) { - await _mainView!.setRenderTarget(_originalMainViewRenderTarget as FFIRenderTarget?); + // Release the internal main-view target before destroying it. The current + // presentation target, not the first target seen during initialization, + // is authoritative after resize/replacement. + if (_mainView != null) { + await _mainView!.setRenderTarget(_presentationRenderTarget); } if (_swapChain != null) { @@ -384,8 +467,7 @@ class FFIHighlightOverlayManager extends HighlightOverlayManager { await _destroyMainViewRenderTarget(); _mainView = null; - _originalMainViewRenderTarget = null; - _flutterRenderTarget = null; // Don't destroy - Flutter layer owns this + _presentationRenderTarget = null; // Don't destroy - Flutter layer owns this _logger.info("Highlight overlay torn down"); diff --git a/thermion_dart/lib/src/filament/src/implementation/silhouette_view.dart b/thermion_dart/lib/src/filament/src/implementation/silhouette_view.dart index 2f6135c5c..1a6092276 100644 --- a/thermion_dart/lib/src/filament/src/implementation/silhouette_view.dart +++ b/thermion_dart/lib/src/filament/src/implementation/silhouette_view.dart @@ -7,12 +7,27 @@ import 'package:thermion_dart/src/filament/src/implementation/ffi_view.dart'; import 'package:thermion_dart/thermion_dart.dart'; import 'ffi_filament_app.dart'; -/// Component data for highlighted entities -class _SilhouetteComponent { +/// One silhouette renderable (one per highlighted primitive). +class _SilhouetteEntry { final MaterialInstance silhouetteMaterialInstance; final ThermionEntity silhouetteEntity; - _SilhouetteComponent({required this.silhouetteMaterialInstance, required this.silhouetteEntity}); + _SilhouetteEntry({required this.silhouetteMaterialInstance, required this.silhouetteEntity}); +} + +/// All silhouette renderables created for one highlighted entity. +/// +/// Silhouettes are keyed per primitive when a [SilhouetteView.addHighlight] +/// supplies a [primitiveKey]; entities with multiple primitives then get one +/// silhouette renderable each instead of only the first primitive being +/// outlined. +class _SilhouetteComponent { + final Map entries = {}; + int _autoKeyCounter = 0; + + /// Key for callers that don't provide an explicit primitive key (legacy + /// single-silhouette-per-entity behavior). + int nextAutoKey() => _autoKeyCounter++; } /// Manages the first (silhouette) rendering pass for highlighted entities. @@ -90,7 +105,9 @@ class SilhouetteView extends FFIView { width, height, flags: {TextureUsage.TEXTURE_USAGE_DEPTH_ATTACHMENT}, - textureFormat: TextureFormat.DEPTH32F, + // DEPTH24 is plenty for an unlit opaque silhouette pass and + // halves depth bandwidth compared to DEPTH32F. + textureFormat: TextureFormat.DEPTH24, ) as FFITexture; final renderTarget = @@ -163,7 +180,7 @@ class SilhouetteView extends FFIView { width, height, flags: {TextureUsage.TEXTURE_USAGE_DEPTH_ATTACHMENT}, - textureFormat: TextureFormat.DEPTH32F, + textureFormat: TextureFormat.DEPTH24, ) as FFITexture; @@ -220,17 +237,30 @@ class SilhouetteView extends FFIView { } /// Add a highlight for the given entity. - Future addHighlight({ + /// + /// When [primitiveKey] is provided, silhouettes are deduplicated per + /// (entity, primitiveKey), so calling this once per primitive of an entity + /// creates one silhouette renderable per primitive. Without a key, only the + /// first call for an entity creates a silhouette (legacy behavior). + /// + /// Returns whether a new silhouette was created. + Future addHighlight({ required ThermionEntity target, required VertexBuffer vertexBuffer, required IndexBuffer indexBuffer, required int indexCount, + int? primitiveKey, }) async { - if (_components.containsKey(target)) return; + var component = _components[target]; + if (component != null) { + if (primitiveKey == null || component.entries.containsKey(primitiveKey)) { + return false; + } + } if (!_app.renderableManager.hasComponent(target)) { _logger.warning('Entity $target is not renderable'); - return; + return false; } // Create silhouette material instance @@ -259,12 +289,15 @@ class SilhouetteView extends FFIView { await _silhouetteScene.addEntity(silhouetteEntity); // Store component - _components[target] = _SilhouetteComponent( + component ??= _SilhouetteComponent(); + component.entries[primitiveKey ?? component.nextAutoKey()] = _SilhouetteEntry( silhouetteMaterialInstance: silhouetteMi, silhouetteEntity: silhouetteEntity, ); + _components[target] = component; - _logger.info('Added silhouette for entity $target'); + _logger.info('Added silhouette for entity $target (primitive ${primitiveKey ?? 'default'})'); + return true; } /// Remove highlight from an entity. @@ -272,14 +305,16 @@ class SilhouetteView extends FFIView { final component = _components.remove(target); if (component == null) return; - // Remove from scene - await _silhouetteScene.removeEntity(component.silhouetteEntity); + for (final entry in component.entries.values) { + // Remove from scene + await _silhouetteScene.removeEntity(entry.silhouetteEntity); - // Destroy entity - await _app.destroyEntity(component.silhouetteEntity); + // Destroy entity + await _app.destroyEntity(entry.silhouetteEntity); - // Destroy material instance - await component.silhouetteMaterialInstance.destroy(); + // Destroy material instance + await entry.silhouetteMaterialInstance.destroy(); + } _logger.info('Removed silhouette for entity $target'); } From 2540e09b5225648456c606f0c60b09964bed5333 Mon Sep 17 00:00:00 2001 From: Nick Fisher Date: Sun, 16 Aug 2026 16:10:06 +0000 Subject: [PATCH 5/7] docs: add implementation status to gizmo/overlay review Per-item status for the six quick wins (all implemented; native TGizmo removal and the single-FFI camera fetch deferred), the capture fix required by the pass-suspension change, the pre-existing no-op of setStencilHighlight on procedural geometry, and the verification summary. Co-Authored-By: Claude --- docs/research/gizmo-overlay-review.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/docs/research/gizmo-overlay-review.md b/docs/research/gizmo-overlay-review.md index c643f7058..c6b687a1d 100644 --- a/docs/research/gizmo-overlay-review.md +++ b/docs/research/gizmo-overlay-review.md @@ -153,6 +153,25 @@ The gizmo and overlay systems don't share code today, and mostly shouldn't — b 9. Ghost-renderable factory + camera-state cache shared by gizmo/wireframe/bbox/silhouette (§3.1/3.2). **M.** 10. Give the gizmo a proper per-frame hook (app render hooks) instead of pull-based `update()` from input events (§1.5). **S–M.** +### Implementation status (2026-08-16 follow-up) + +The six quick wins were implemented on `asb/gizmo-overlay-review`. + +| # | Item | Status | Notes | +|---|---|---|---| +| 1 | Gizmo dispose leak | **Implemented** | `TransformationGizmo.dispose()` now destroys the loaded glb assets through `viewer.destroyAsset`, plus entities and material instances; idempotent. | +| 2 | Skip overlay passes when empty | **Implemented** | `FFIHighlightOverlayManager._applySuspension()` flips `setRenderable` on the silhouette/edge views at empty↔non-empty transitions — no destroy/rebuild. In composite mode the main view is pointed back at the Flutter-provided render target while suspended (new `FFIView.setRenderTargetDirect` bypasses the `setRenderTarget` interception). Structural item 8 (skipping the offscreen main-scene texture entirely) is still open. | +| 3 | Delete commented-out gizmo files | **Implemented (files only)** | The four fully-commented files are gone. The live native `TGizmo` path and the embedded glb blobs were intentionally retained (owner decision); removing them needs bindings + hooks work and is deferred. | +| 4 | Outline params once per call | **Implemented** | `setStencilHighlight` calls `setOutlineParams` once; `addHighlight` no longer takes color/width. The per-primitive keying half of structural item 7 also landed: silhouettes are deduplicated per (entity, primitive) instead of first-primitive-only. | +| 5 | Cache camera state per event | **Implemented (Dart-level)** | `GizmoCameraContext.fetch()` is called once per move/hover and shared by update/hover/drag. The single-FFI-call `getCameraFrame()` variant needs a new native API — deferred. | +| 6 | DEPTH24 + setPriority | **Implemented** | Silhouette depth is DEPTH24 (create and resize paths); the axis-marker `setPriority(7)` is set once at creation. | + +**Fix required along the way:** `FilamentApp.capture` rendered every attached view, including non-renderable ones. While the overlay is suspended the edge view still points at the Flutter render target, so capture's render of it cleared the target after the main view had already drawn (all-zero captures). Capture now renders only views the frame loop would render (`RenderManager.isRenderable`) but still reads back every attached view. + +**Pre-existing behavior found while verifying:** `setStencilHighlight` has always been a no-op on procedural (`createGeometry`) assets — `SceneAsset_getPrimitiveOffsetForEntity` returns -1 for Geometry-type assets (`native/src/c_api/TSceneAsset.cpp:260-268`), so stencil highlights only work on glTF assets loaded with `rebuildVertices: true`. Unchanged by this work; worth documenting or fixing separately. + +**Verification:** `flutter analyze` — 0 errors, no new warnings. Full test suite — 42 passed, 1 pre-existing failure (`input_pipeline_test` toString case, fails on the clean tree too). Pixel-diff of `view_tests` captures against a clean-tree baseline: silhouette view byte-identical, main view ±1 LSB, composite view same content (per-pixel deltas ≤ 50/255 from rendering direct to the float Flutter target instead of through the SRGB8 composite — the intended change), after-overlay-disabled identical. + --- ## 5. Verdict From 938f5e027fd73ced27eed63c339d439b15707e3e Mon Sep 17 00:00:00 2001 From: Nick Fisher Date: Mon, 24 Aug 2026 13:19:44 +0800 Subject: [PATCH 6/7] fix: reconcile overlay and gizmo lifecycles --- docs/research/gizmo-overlay-review.md | 6 +- thermion_dart/lib/src/utils/src/gizmos.dart | 83 +++++++++++---------- thermion_dart/test/view_tests.dart | 57 ++++++++++++++ 3 files changed, 105 insertions(+), 41 deletions(-) diff --git a/docs/research/gizmo-overlay-review.md b/docs/research/gizmo-overlay-review.md index c6b687a1d..bc9947e87 100644 --- a/docs/research/gizmo-overlay-review.md +++ b/docs/research/gizmo-overlay-review.md @@ -160,17 +160,17 @@ The six quick wins were implemented on `asb/gizmo-overlay-review`. | # | Item | Status | Notes | |---|---|---|---| | 1 | Gizmo dispose leak | **Implemented** | `TransformationGizmo.dispose()` now destroys the loaded glb assets through `viewer.destroyAsset`, plus entities and material instances; idempotent. | -| 2 | Skip overlay passes when empty | **Implemented** | `FFIHighlightOverlayManager._applySuspension()` flips `setRenderable` on the silhouette/edge views at empty↔non-empty transitions — no destroy/rebuild. In composite mode the main view is pointed back at the Flutter-provided render target while suspended (new `FFIView.setRenderTargetDirect` bypasses the `setRenderTarget` interception). Structural item 8 (skipping the offscreen main-scene texture entirely) is still open. | +| 2 | Skip overlay passes when empty | **Implemented** | `FFIHighlightOverlayManager._reconcileRenderPlan()` derives target routing and pass activation from the current highlight/output state after every highlight, viewport, or presentation-target change. Both auxiliary views are published together with `RenderManager.setRenderables`; no resources are destroyed or rebuilt. `View.setPresentationRenderTarget` separates platform output ownership from raw Filament target binding, so `FFIView.setRenderTarget` no longer intercepts or bypasses itself. Structural item 8 (skipping allocation of the offscreen main-scene texture entirely) is still open. | | 3 | Delete commented-out gizmo files | **Implemented (files only)** | The four fully-commented files are gone. The live native `TGizmo` path and the embedded glb blobs were intentionally retained (owner decision); removing them needs bindings + hooks work and is deferred. | | 4 | Outline params once per call | **Implemented** | `setStencilHighlight` calls `setOutlineParams` once; `addHighlight` no longer takes color/width. The per-primitive keying half of structural item 7 also landed: silhouettes are deduplicated per (entity, primitive) instead of first-primitive-only. | | 5 | Cache camera state per event | **Implemented (Dart-level)** | `GizmoCameraContext.fetch()` is called once per move/hover and shared by update/hover/drag. The single-FFI-call `getCameraFrame()` variant needs a new native API — deferred. | | 6 | DEPTH24 + setPriority | **Implemented** | Silhouette depth is DEPTH24 (create and resize paths); the axis-marker `setPriority(7)` is set once at creation. | -**Fix required along the way:** `FilamentApp.capture` rendered every attached view, including non-renderable ones. While the overlay is suspended the edge view still points at the Flutter render target, so capture's render of it cleared the target after the main view had already drawn (all-zero captures). Capture now renders only views the frame loop would render (`RenderManager.isRenderable`) but still reads back every attached view. +**Fix required along the way:** `FilamentApp.capture` rendered every attached view, including inactive ones. While the overlay is suspended the edge view can share the presentation target with the main view, so rendering it after the main pass cleared the captured output. `RenderManager.getRenderPlan()` now returns one immutable snapshot of ordered active/inactive passes. Capture uses that same snapshot in both its primary and WebGL completion frames, while still reading back every attached view for per-pass diagnostics. **Pre-existing behavior found while verifying:** `setStencilHighlight` has always been a no-op on procedural (`createGeometry`) assets — `SceneAsset_getPrimitiveOffsetForEntity` returns -1 for Geometry-type assets (`native/src/c_api/TSceneAsset.cpp:260-268`), so stencil highlights only work on glTF assets loaded with `rebuildVertices: true`. Unchanged by this work; worth documenting or fixing separately. -**Verification:** `flutter analyze` — 0 errors, no new warnings. Full test suite — 42 passed, 1 pre-existing failure (`input_pipeline_test` toString case, fails on the clean tree too). Pixel-diff of `view_tests` captures against a clean-tree baseline: silhouette view byte-identical, main view ±1 LSB, composite view same content (per-pixel deltas ≤ 50/255 from rendering direct to the float Flutter target instead of through the SRGB8 composite — the intended change), after-overlay-disabled identical. +**Verification:** focused Dart analysis and `flutter analyze` — 0 errors. `view_tests` — 29 passed, including idle-overlay resize and presentation-target replacement coverage. Serial full test suite — 78 passed, 1 pre-existing failure (`input_pipeline_test` toString case, fails on the clean tree too). The default parallel full-suite invocation remains unsupported by the native test harness (`RenderThread` thread-adoption precondition); this is unrelated to the overlay changes. --- diff --git a/thermion_dart/lib/src/utils/src/gizmos.dart b/thermion_dart/lib/src/utils/src/gizmos.dart index 46256f308..52f4d60d7 100644 --- a/thermion_dart/lib/src/utils/src/gizmos.dart +++ b/thermion_dart/lib/src/utils/src/gizmos.dart @@ -107,19 +107,23 @@ class TransformationGizmo { // 1. Create Unlit Materials (No depth write for "always on top" effect) _redMat = await _createGizmoMaterial(1.0, 0.0, 0.0); - if (_isDisposed) return; + if (_redMat == null) return; _greenMat = await _createGizmoMaterial(0.0, 1.0, 0.0); - if (_isDisposed) return; + if (_greenMat == null) return; _blueMat = await _createGizmoMaterial(0.0, 0.0, 1.0); - if (_isDisposed) return; + if (_blueMat == null) return; _whiteMat = await _createGizmoMaterial(1.0, 1.0, 1.0, alpha: 1.0); - if (_isDisposed) return; + if (_whiteMat == null) return; _yellowMat = await _createGizmoMaterial(1.0, 1.0, 0.0, alpha: 1.0); - if (_isDisposed) return; + if (_yellowMat == null) return; // 2. Create Root entity (no geometry needed - just a transform parent) - _rootEntity = await viewer.app.createEntity(); - if (_isDisposed) return; + final rootEntity = await viewer.app.createEntity(); + if (_isDisposed) { + await viewer.app.destroyEntity(rootEntity); + return; + } + _rootEntity = rootEntity; if (type == TransformationGizmoType.translation) { await _buildTranslationAxes(); @@ -174,15 +178,10 @@ class TransformationGizmo { Future _createRing(Geometry ring, MaterialInstance mat, Vector3 axis) async { final ringAsset = await viewer.app.createGeometry(ring, materialInstances: [mat]); - // Safety check before using the asset - if (_isDisposed) { - await viewer.removeFromScene(ringAsset); + if (!await _takeAssetOwnership(ringAsset)) { return ringAsset.entity; // Return but won't be used } - await viewer.addToScene(ringAsset); - _assets.add(ringAsset); - // Parent to root entity so it moves with the gizmo if (_rootEntity != null) { viewer.app.transformManager.setParent(ringAsset.entity, _rootEntity!); @@ -200,6 +199,7 @@ class TransformationGizmo { final ringMatrix = Matrix4.compose(Vector3.zero(), rotation, Vector3.all(1.0)); await viewer.app.setTransform(ringAsset.entity, ringMatrix); + if (_isDisposed) return ringAsset.entity; await viewer.app.setPriority(ringAsset.entity, 7); return ringAsset.entity; } @@ -214,27 +214,18 @@ class TransformationGizmo { // Create Shaft final shaftAsset = await viewer.app.createGeometry(shaft, materialInstances: [mat]); - if (_isDisposed) { - // cleanup immediately if disposed during creation - await viewer.removeFromScene(shaftAsset); + if (!await _takeAssetOwnership(shaftAsset)) { // return dummy, loop will catch disposed flag return (shaftAsset.entity, shaftAsset.entity); } - await viewer.addToScene(shaftAsset); - _assets.add(shaftAsset); - // Create Head final headAsset = await viewer.app.createGeometry(head, materialInstances: [mat]); - if (_isDisposed) { - await viewer.removeFromScene(headAsset); + if (!await _takeAssetOwnership(headAsset)) { return (shaftAsset.entity, headAsset.entity); } - await viewer.addToScene(headAsset); - _assets.add(headAsset); - // Parent to root entity so they move with the gizmo if (_rootEntity != null) { viewer.app.transformManager.setParent(shaftAsset.entity, _rootEntity!); @@ -251,6 +242,7 @@ class TransformationGizmo { // Set local transforms await viewer.app.setTransform(shaftAsset.entity, shaftMatrix); + if (_isDisposed) return (shaftAsset.entity, headAsset.entity); await viewer.app.setTransform(headAsset.entity, headMatrix); return (shaftAsset.entity, headAsset.entity); @@ -415,14 +407,38 @@ class TransformationGizmo { ); } - Future _createGizmoMaterial(double r, double g, double b, {double alpha = 0.5}) async { - if (_isDisposed) throw Exception("Gizmo disposed"); + Future _createGizmoMaterial(double r, double g, double b, {double alpha = 0.5}) async { + if (_isDisposed) return null; final material = await viewer.app.createGizmoMaterial(); + if (_isDisposed) return null; + final mat = await material.createInstance(); + if (_isDisposed) { + await mat.destroy(); + return null; + } + await mat.setParameterFloat4("baseColorFactor", r, g, b, alpha); + if (_isDisposed) { + await mat.destroy(); + return null; + } return mat; } + /// Registers [asset] before awaiting scene insertion so disposal owns every + /// resource throughout the entire asynchronous creation sequence. + Future _takeAssetOwnership(ThermionAsset asset) async { + if (_isDisposed) { + await viewer.destroyAsset(asset); + return false; + } + + _assets.add(asset); + await viewer.addToScene(asset); + return !_isDisposed; + } + Future attachTo(ThermionEntity entity) async { if (_isDisposed) return; _attachedTarget = entity; @@ -948,23 +964,13 @@ class TransformationGizmo { // Start marker (white) _startMarkerAsset = await viewer.app.createGeometry(markerGeom, materialInstances: [_whiteMat!]); - if (_isDisposed) { - await viewer.removeFromScene(_startMarkerAsset!); - return; - } - await viewer.addToScene(_startMarkerAsset!); + if (!await _takeAssetOwnership(_startMarkerAsset!)) return; _startMarker = _startMarkerAsset!.entity; - _assets.add(_startMarkerAsset!); // Current marker (yellow) _currentMarkerAsset = await viewer.app.createGeometry(markerGeom, materialInstances: [_yellowMat!]); - if (_isDisposed) { - await viewer.removeFromScene(_currentMarkerAsset!); - return; - } - await viewer.addToScene(_currentMarkerAsset!); + if (!await _takeAssetOwnership(_currentMarkerAsset!)) return; _currentMarker = _currentMarkerAsset!.entity; - _assets.add(_currentMarkerAsset!); // Parent to root entity if (_rootEntity != null) { @@ -975,6 +981,7 @@ class TransformationGizmo { // Draw on top of the rings (set once here; the priority never changes, // so it must not be re-set on every marker move). await viewer.app.setPriority(_startMarker!, 7); + if (_isDisposed) return; await viewer.app.setPriority(_currentMarker!, 7); // Initially hide markers diff --git a/thermion_dart/test/view_tests.dart b/thermion_dart/test/view_tests.dart index 83d65d306..9dbbd86d8 100644 --- a/thermion_dart/test/view_tests.dart +++ b/thermion_dart/test/view_tests.dart @@ -510,6 +510,63 @@ void main() async { }); }); + test('empty highlight overlay reconciles resize and replacement targets', () async { + await ViewerBuilder(testHelper).setRenderTargetEnabled(true).execute((result) async { + final app = FilamentApp.instance!; + final view = result.viewer.view; + final originalTarget = await view.getRenderTarget(); + expect(originalTarget, isNotNull); + + await view.setHighlightOverlayEnabled(true); + final overlay = view.getHighlightOverlay()!; + expect(overlay.suspended, isTrue); + + final swapChain = app.renderManager.getAttachedSwapChains(view).single; + expect(app.renderManager.getRenderPlan(swapChain).activeViews, [view]); + + // Resizing recreates the internal composite target. An idle overlay must + // nevertheless leave the main view bound to the presentation target. + await view.setViewport(256, 256); + expect(await view.getRenderTarget(), same(originalTarget)); + + final replacementColor = await app.createTexture( + 256, + 256, + flags: { + TextureUsage.TEXTURE_USAGE_BLIT_SRC, + TextureUsage.TEXTURE_USAGE_COLOR_ATTACHMENT, + TextureUsage.TEXTURE_USAGE_SAMPLEABLE, + }, + ); + final replacementDepth = await app.createTexture( + 256, + 256, + flags: {TextureUsage.TEXTURE_USAGE_DEPTH_ATTACHMENT}, + textureFormat: TextureFormat.DEPTH32F, + ); + final replacementTarget = await app.createRenderTarget( + 256, + 256, + color: replacementColor, + depth: replacementDepth, + ); + + try { + await view.setPresentationRenderTarget(replacementTarget); + expect(await view.getRenderTarget(), same(replacementTarget)); + expect(overlay.suspended, isTrue); + expect(app.renderManager.getRenderPlan(swapChain).activeViews, [view]); + } finally { + await view.setPresentationRenderTarget(originalTarget); + await view.setViewport(512, 512); + await replacementTarget.destroy(); + await replacementColor.destroy(); + await replacementDepth.destroy(); + await view.setHighlightOverlayEnabled(false); + } + }); + }); + test('show/hide stencil highlight', () async { await ViewerBuilder(testHelper).setRenderTargetEnabled(true).setStencilBufferEnabled(true).execute((result) async { await result.viewer.view.setHighlightOverlayEnabled(true); From 66ea35c5ff28a08561b8d7389996859736f07433 Mon Sep 17 00:00:00 2001 From: Nick Fisher Date: Mon, 24 Aug 2026 14:07:29 +0800 Subject: [PATCH 7/7] refactor: clarify view attachment terminology --- docs/research/gizmo-overlay-review.md | 4 +-- .../highlight_overlay_manager.dart | 34 ++++++++++--------- thermion_dart/test/view_tests.dart | 16 +++++++-- 3 files changed, 34 insertions(+), 20 deletions(-) diff --git a/docs/research/gizmo-overlay-review.md b/docs/research/gizmo-overlay-review.md index bc9947e87..2a24744fe 100644 --- a/docs/research/gizmo-overlay-review.md +++ b/docs/research/gizmo-overlay-review.md @@ -160,13 +160,13 @@ The six quick wins were implemented on `asb/gizmo-overlay-review`. | # | Item | Status | Notes | |---|---|---|---| | 1 | Gizmo dispose leak | **Implemented** | `TransformationGizmo.dispose()` now destroys the loaded glb assets through `viewer.destroyAsset`, plus entities and material instances; idempotent. | -| 2 | Skip overlay passes when empty | **Implemented** | `FFIHighlightOverlayManager._reconcileRenderPlan()` derives target routing and pass activation from the current highlight/output state after every highlight, viewport, or presentation-target change. Both auxiliary views are published together with `RenderManager.setRenderables`; no resources are destroyed or rebuilt. `View.setPresentationRenderTarget` separates platform output ownership from raw Filament target binding, so `FFIView.setRenderTarget` no longer intercepts or bypasses itself. Structural item 8 (skipping allocation of the offscreen main-scene texture entirely) is still open. | +| 2 | Skip overlay passes when empty | **Implemented** | `FFIHighlightOverlayManager._reconcilePresentationState()` derives target routing and view renderability from the current highlight/output state after every highlight, viewport, or presentation-target change. Both auxiliary views are updated together with `RenderManager.setRenderables`; no resources are destroyed or rebuilt. `View.setPresentationRenderTarget` separates platform output ownership from raw Filament target binding, so `FFIView.setRenderTarget` no longer intercepts or bypasses itself. Structural item 8 (skipping allocation of the offscreen main-scene texture entirely) is still open. | | 3 | Delete commented-out gizmo files | **Implemented (files only)** | The four fully-commented files are gone. The live native `TGizmo` path and the embedded glb blobs were intentionally retained (owner decision); removing them needs bindings + hooks work and is deferred. | | 4 | Outline params once per call | **Implemented** | `setStencilHighlight` calls `setOutlineParams` once; `addHighlight` no longer takes color/width. The per-primitive keying half of structural item 7 also landed: silhouettes are deduplicated per (entity, primitive) instead of first-primitive-only. | | 5 | Cache camera state per event | **Implemented (Dart-level)** | `GizmoCameraContext.fetch()` is called once per move/hover and shared by update/hover/drag. The single-FFI-call `getCameraFrame()` variant needs a new native API — deferred. | | 6 | DEPTH24 + setPriority | **Implemented** | Silhouette depth is DEPTH24 (create and resize paths); the axis-marker `setPriority(7)` is set once at creation. | -**Fix required along the way:** `FilamentApp.capture` rendered every attached view, including inactive ones. While the overlay is suspended the edge view can share the presentation target with the main view, so rendering it after the main pass cleared the captured output. `RenderManager.getRenderPlan()` now returns one immutable snapshot of ordered active/inactive passes. Capture uses that same snapshot in both its primary and WebGL completion frames, while still reading back every attached view for per-pass diagnostics. +**Fix required along the way:** `FilamentApp.capture` rendered every attached view, including non-renderable ones. While the overlay is suspended the edge view can share the presentation target with the main view, so rendering it after the main view cleared the captured output. `RenderManager.getViewAttachments()` now returns one immutable ordered `List`. Capture uses that same list in both its primary and WebGL completion frames, while still reading back every attached view for per-view diagnostics. **Pre-existing behavior found while verifying:** `setStencilHighlight` has always been a no-op on procedural (`createGeometry`) assets — `SceneAsset_getPrimitiveOffsetForEntity` returns -1 for Geometry-type assets (`native/src/c_api/TSceneAsset.cpp:260-268`), so stencil highlights only work on glTF assets loaded with `rebuildVertices: true`. Unchanged by this work; worth documenting or fixing separately. diff --git a/thermion_dart/lib/src/filament/src/implementation/highlight_overlay_manager.dart b/thermion_dart/lib/src/filament/src/implementation/highlight_overlay_manager.dart index a8790a407..321bc65a8 100644 --- a/thermion_dart/lib/src/filament/src/implementation/highlight_overlay_manager.dart +++ b/thermion_dart/lib/src/filament/src/implementation/highlight_overlay_manager.dart @@ -125,7 +125,8 @@ class FFIHighlightOverlayManager extends HighlightOverlayManager { /// frame even when they draw nothing, so they are marked non-renderable /// whenever the highlight set is empty. The initial value matches the /// attached-and-renderable state the views get when the overlay is enabled; - /// [_reconcileRenderPlan] applies the desired state after every input change. + /// [_reconcilePresentationState] applies the desired state after every input + /// change. bool _suspended = false; @override @@ -168,7 +169,7 @@ class FFIHighlightOverlayManager extends HighlightOverlayManager { await overlayView.setRenderTarget(presentationRenderTarget); _logger.info("Presentation render target updated"); - await _reconcileRenderPlan(); + await _reconcilePresentationState(); } /// Set the swapchain for overlay mode (web/Android). @@ -187,22 +188,23 @@ class FFIHighlightOverlayManager extends HighlightOverlayManager { await overlayView.setOverlayOnly(true); _logger.info("EdgeDetectionView registered with swapchain (overlay-only mode)"); - await _reconcileRenderPlan(); + await _reconcilePresentationState(); } - /// Reconciles target routing and active passes from current desired state. + /// Reconciles target routing and view renderability from current desired + /// state. /// /// This is deliberately idempotent: viewport and presentation-target /// changes must re-apply routing even when the highlight set is unchanged. /// Reconciliations are serialized so overlapping lifecycle calls cannot - /// publish an older plan after a newer one. - Future _reconcileRenderPlan() { + /// publish older state after newer state. + Future _reconcilePresentationState() { final completer = Completer(); final previous = _reconcileChain; _reconcileChain = completer.future.then((_) {}, onError: (_) {}); return previous.then((_) async { try { - await _applyRenderPlan(); + await _applyPresentationState(); completer.complete(); } catch (error, stackTrace) { completer.completeError(error, stackTrace); @@ -211,23 +213,23 @@ class FFIHighlightOverlayManager extends HighlightOverlayManager { }); } - Future _applyRenderPlan() async { + Future _applyPresentationState() async { final hasOutput = _mainView == null || _presentationRenderTarget != null; final shouldSuspend = _highlightedEntities.isEmpty || !hasOutput; final stateChanged = _suspended != shouldSuspend; final rm = _app.renderManager; if (shouldSuspend) { - // Remove both overlay passes in one plan update before routing the main - // view directly to its output. No frame can run an edge pass against a - // directly-rendered main view. + // Disable both overlay views in one state update before routing the main + // view directly to its output. No frame can run edge detection against + // a directly-rendered main view. await rm.setRenderables({silhouetteView: false, overlayView: false}); if (_mainView != null) { await _mainView!.setRenderTarget(_presentationRenderTarget); } } else { - // Route the main pass to its sampleable target before publishing the - // overlay passes together. + // Route the main view to its sampleable target before enabling the + // overlay views together. if (_mainView != null && _mainViewRenderTarget != null) { await _mainView!.setRenderTarget(_mainViewRenderTarget); } @@ -347,7 +349,7 @@ class FFIHighlightOverlayManager extends HighlightOverlayManager { // Update all references and re-apply the current route before destroying // resources that may still be bound by the active plan. await overlayView.setMainSceneTexture(_mainViewColorTexture!); - await _reconcileRenderPlan(); + await _reconcilePresentationState(); // Flush render thread to ensure new textures are bound before destroying old ones // This prevents "Invalid texture still bound to MaterialInstance" errors @@ -411,7 +413,7 @@ class FFIHighlightOverlayManager extends HighlightOverlayManager { if (created) { _highlightedEntities.add(target); - await _reconcileRenderPlan(); + await _reconcilePresentationState(); } } @@ -424,7 +426,7 @@ class FFIHighlightOverlayManager extends HighlightOverlayManager { await silhouetteView.removeHighlight(target); _highlightedEntities.remove(target); - await _reconcileRenderPlan(); + await _reconcilePresentationState(); } /// Remove all highlights. diff --git a/thermion_dart/test/view_tests.dart b/thermion_dart/test/view_tests.dart index 9dbbd86d8..3d9a00eca 100644 --- a/thermion_dart/test/view_tests.dart +++ b/thermion_dart/test/view_tests.dart @@ -522,7 +522,13 @@ void main() async { expect(overlay.suspended, isTrue); final swapChain = app.renderManager.getAttachedSwapChains(view).single; - expect(app.renderManager.getRenderPlan(swapChain).activeViews, [view]); + expect( + app.renderManager + .getViewAttachments(swapChain) + .where((attachment) => attachment.renderable) + .map((attachment) => attachment.view), + [view], + ); // Resizing recreates the internal composite target. An idle overlay must // nevertheless leave the main view bound to the presentation target. @@ -555,7 +561,13 @@ void main() async { await view.setPresentationRenderTarget(replacementTarget); expect(await view.getRenderTarget(), same(replacementTarget)); expect(overlay.suspended, isTrue); - expect(app.renderManager.getRenderPlan(swapChain).activeViews, [view]); + expect( + app.renderManager + .getViewAttachments(swapChain) + .where((attachment) => attachment.renderable) + .map((attachment) => attachment.view), + [view], + ); } finally { await view.setPresentationRenderTarget(originalTarget); await view.setViewport(512, 512);