Gizmo & highlight overlay quick wins - #247
Open
nmfisher wants to merge 7 commits into
Open
Conversation
nmfisher
force-pushed
the
asb/gizmo-overlay-review
branch
from
August 24, 2026 05:22
a52906d to
8191648
Compare
nmfisher
changed the base branch from
develop
to
codex/render-pipeline-refactor
August 24, 2026 05:29
nmfisher
force-pushed
the
asb/gizmo-overlay-review
branch
from
August 24, 2026 06:08
8191648 to
6cfb8cb
Compare
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
nmfisher
force-pushed
the
asb/gizmo-overlay-review
branch
from
August 24, 2026 06:59
6cfb8cb to
66ea35c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the six approved quick wins from
docs/research/gizmo-overlay-review.md, rebased ontocodex/render-pipeline-refactor.Gizmo
TransformationGizmo.dispose()destroys geometry assets, entities, and material instances; creation paths also destroy resources that complete after disposal.GizmoCameraContextis fetched once per pointer event and shared by update/hover/drag.update()skips redundant root-transform writes.gizmo.dart,gizmo_input_handler.dart,gizmo_pick_delegate.dart,RotationGizmo.cpp). The live Dart gizmo and nativeTGizmopath are untouched.Highlight overlay
RenderManager.setRenderables; resources remain allocated while idle.View.setPresentationRenderTargetseparates platform output ownership from raw Filament view binding.FFIHighlightOverlayManagerserializes idempotent presentation-state reconciliation after highlight, viewport, and output-target changes.List<ViewAttachment>and uses it for both the primary render/readback loop and the WebGL completion frame. Non-renderable targets remain readable without submitting their views.(entity, primitive)so multi-primitive entities outline fully.Notes
setStencilHighlightis a no-op on procedural (createGeometry) assets becauseSceneAsset_getPrimitiveOffsetForEntityreturns-1for Geometry-type assets. Stencil highlights require glTF plusrebuildVertices: true.TGizmopath/glb blobs, single-FFIgetCameraFrame(), and avoiding allocation of the composite target while idle.Verification
flutter analyze: 0 errors.view_tests: 29 passed, including idle-overlay resize and presentation-target replacement.input_pipeline_testtoStringexpectation).RenderThreadthread-adoption limitation.