feat(camera-info): 3D viewport widget for CreateCameraInfo node - #13741
Open
jtydhr88 wants to merge 1 commit into
Open
feat(camera-info): 3D viewport widget for CreateCameraInfo node#13741jtydhr88 wants to merge 1 commit into
jtydhr88 wants to merge 1 commit into
Conversation
dosubot
Bot
added
the
size:XXL
🎭 Playwright: ✅ 1716 passed, 0 failed · 1 flaky📊 Browser Reports
🎨 Storybook: ✅ Built — View Storybook📦 Bundle: 7.94 MB gzip 🔴 +24.7 kBDetailsSummary
Category Glance App Entry Points — 47.3 kB (baseline 47.3 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.25 MB (baseline 1.25 MB) • 🔴 +30 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 109 kB (baseline 109 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 13 added / 13 removed / 1 unchanged Panels & Settings — 551 kB (baseline 551 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 11 added / 11 removed / 15 unchanged User & Accounts — 29.1 kB (baseline 29.1 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 7 added / 7 removed / 3 unchanged Editors & Dialogs — 121 kB (baseline 121 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 6 added / 6 removed UI Components — 64.7 kB (baseline 64.7 kB) • ⚪ 0 BReusable component library chunks
Status: 13 added / 13 removed / 1 unchanged Data & Services — 225 kB (baseline 270 kB) • 🟢 -45 kBStores, services, APIs, and repositories
Status: 14 added / 14 removed / 2 unchanged Utilities & Hooks — 3.41 MB (baseline 3.41 MB) • 🔴 +735 BHelpers, composables, and utility bundles
Status: 20 added / 20 removed / 16 unchanged Vendor & Third-Party — 15.8 MB (baseline 15.7 MB) • 🔴 +48.9 kBExternal libraries and shared vendor chunks
Status: 2 added / 2 removed / 14 unchanged Other — 12 MB (baseline 11.9 MB) • 🔴 +118 kBBundles that do not match a named category
Status: 104 added / 99 removed / 63 unchanged ⚡ Performance Report
Show regressions
All metrics
Historical variance (last 15 runs)
Trend (last 15 commits on main)
Raw data{
"timestamp": "2026-07-19T18:20:38.005Z",
"gitSha": "228ceefdd64807923cab311c51449f554ded1dfb",
"branch": "feat/create-camera-info-preview",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2094.2469999999958,
"styleRecalcs": 6,
"styleRecalcDurationMs": 6.339999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 506.85,
"heapDeltaBytes": -21950060,
"heapUsedBytes": 46966040,
"domNodes": -265,
"jsHeapTotalBytes": 19275776,
"scriptDurationMs": 20.334,
"eventListeners": -133,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-idle",
"durationMs": 2087.6380000000268,
"styleRecalcs": 10,
"styleRecalcDurationMs": 16.118000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 539.38,
"heapDeltaBytes": -22517248,
"heapUsedBytes": 46494324,
"domNodes": -264,
"jsHeapTotalBytes": 19275776,
"scriptDurationMs": 26.561,
"eventListeners": -131,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1814.7789999999873,
"styleRecalcs": 75,
"styleRecalcDurationMs": 38.777,
"layouts": 12,
"layoutDurationMs": 3.19,
"taskDurationMs": 789.721,
"heapDeltaBytes": -1282300,
"heapUsedBytes": 67548836,
"domNodes": 60,
"jsHeapTotalBytes": 20983808,
"scriptDurationMs": 124.906,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1876.2189999999919,
"styleRecalcs": 74,
"styleRecalcDurationMs": 44.543,
"layouts": 12,
"layoutDurationMs": 3.8850000000000002,
"taskDurationMs": 902.2110000000001,
"heapDeltaBytes": -15867976,
"heapUsedBytes": 53288452,
"domNodes": -266,
"jsHeapTotalBytes": 21110784,
"scriptDurationMs": 133.415,
"eventListeners": -133,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1741.591000000028,
"styleRecalcs": 30,
"styleRecalcDurationMs": 19.576,
"layouts": 6,
"layoutDurationMs": 0.7549999999999998,
"taskDurationMs": 364.80899999999997,
"heapDeltaBytes": 7807456,
"heapUsedBytes": 76783148,
"domNodes": 77,
"jsHeapTotalBytes": 19410944,
"scriptDurationMs": 25.755,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1756.5610000000333,
"styleRecalcs": 32,
"styleRecalcDurationMs": 20.067,
"layouts": 6,
"layoutDurationMs": 0.637,
"taskDurationMs": 378.51599999999996,
"heapDeltaBytes": 7748904,
"heapUsedBytes": 76687804,
"domNodes": 78,
"jsHeapTotalBytes": 19673088,
"scriptDurationMs": 26.428,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "dom-widget-clipping",
"durationMs": 582.7369999999803,
"styleRecalcs": 12,
"styleRecalcDurationMs": 10.088999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 371.53999999999996,
"heapDeltaBytes": -11688868,
"heapUsedBytes": 57402928,
"domNodes": 20,
"jsHeapTotalBytes": 19935232,
"scriptDurationMs": 61.402,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "dom-widget-clipping",
"durationMs": 612.2879999999782,
"styleRecalcs": 12,
"styleRecalcDurationMs": 12.095,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 393.79900000000004,
"heapDeltaBytes": -11617768,
"heapUsedBytes": 57407736,
"domNodes": 20,
"jsHeapTotalBytes": 19935232,
"scriptDurationMs": 66.636,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-idle",
"durationMs": 2019.6109999999976,
"styleRecalcs": 9,
"styleRecalcDurationMs": 9.305000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 577.067,
"heapDeltaBytes": 4879500,
"heapUsedBytes": 64620048,
"domNodes": -267,
"jsHeapTotalBytes": 4665344,
"scriptDurationMs": 103.17699999999999,
"eventListeners": -129,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-idle",
"durationMs": 2026.9539999999893,
"styleRecalcs": 10,
"styleRecalcDurationMs": 10.667,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 710.0840000000002,
"heapDeltaBytes": 5107140,
"heapUsedBytes": 64643880,
"domNodes": -265,
"jsHeapTotalBytes": 4665344,
"scriptDurationMs": 116.33500000000001,
"eventListeners": -129,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "large-graph-pan",
"durationMs": 2132.738999999958,
"styleRecalcs": 69,
"styleRecalcDurationMs": 15.363999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1162.3600000000001,
"heapDeltaBytes": -4016124,
"heapUsedBytes": 56413472,
"domNodes": -270,
"jsHeapTotalBytes": 5451776,
"scriptDurationMs": 401.05199999999996,
"eventListeners": -129,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-pan",
"durationMs": 2196.9119999999975,
"styleRecalcs": 69,
"styleRecalcDurationMs": 18.077999999999996,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1242.575,
"heapDeltaBytes": -4763876,
"heapUsedBytes": 56038140,
"domNodes": -269,
"jsHeapTotalBytes": 5451776,
"scriptDurationMs": 425.145,
"eventListeners": -131,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-zoom",
"durationMs": 3182.0319999999924,
"styleRecalcs": 66,
"styleRecalcDurationMs": 16.845000000000002,
"layouts": 60,
"layoutDurationMs": 7.797,
"taskDurationMs": 1406.113,
"heapDeltaBytes": -1799492,
"heapUsedBytes": 60009208,
"domNodes": -273,
"jsHeapTotalBytes": 8073216,
"scriptDurationMs": 507.629,
"eventListeners": -133,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-zoom",
"durationMs": 3297.582000000034,
"styleRecalcs": 65,
"styleRecalcDurationMs": 17.318,
"layouts": 60,
"layoutDurationMs": 8.008,
"taskDurationMs": 1494.52,
"heapDeltaBytes": 2399608,
"heapUsedBytes": 64712876,
"domNodes": -274,
"jsHeapTotalBytes": 7811072,
"scriptDurationMs": 531.626,
"eventListeners": -131,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "minimap-idle",
"durationMs": 2008.813000000032,
"styleRecalcs": 8,
"styleRecalcDurationMs": 8.495,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 660.5989999999999,
"heapDeltaBytes": 4755776,
"heapUsedBytes": 66127376,
"domNodes": -267,
"jsHeapTotalBytes": 5451776,
"scriptDurationMs": 119.149,
"eventListeners": -129,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "minimap-idle",
"durationMs": 2030.3299999999354,
"styleRecalcs": 8,
"styleRecalcDurationMs": 8.528999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 672.811,
"heapDeltaBytes": 5548084,
"heapUsedBytes": 67078828,
"domNodes": -269,
"jsHeapTotalBytes": 5451776,
"scriptDurationMs": 112.02900000000001,
"eventListeners": -129,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 609.4679999999926,
"styleRecalcs": 48,
"styleRecalcDurationMs": 12.862,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 395.32800000000003,
"heapDeltaBytes": -10757044,
"heapUsedBytes": 58264720,
"domNodes": 22,
"jsHeapTotalBytes": 20983808,
"scriptDurationMs": 126.684,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 618.0650000000014,
"styleRecalcs": 47,
"styleRecalcDurationMs": 15.581,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 417.00200000000007,
"heapDeltaBytes": -11160564,
"heapUsedBytes": 57959152,
"domNodes": 20,
"jsHeapTotalBytes": 20983808,
"scriptDurationMs": 132.48299999999998,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-idle",
"durationMs": 2039.1500000000065,
"styleRecalcs": 10,
"styleRecalcDurationMs": 9.927,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 499.895,
"heapDeltaBytes": -18099252,
"heapUsedBytes": 51025164,
"domNodes": -266,
"jsHeapTotalBytes": 19275776,
"scriptDurationMs": 23.016000000000002,
"eventListeners": -131,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-idle",
"durationMs": 2018.0030000000215,
"styleRecalcs": 11,
"styleRecalcDurationMs": 9.802999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 428.52899999999994,
"heapDeltaBytes": 4038984,
"heapUsedBytes": 72987776,
"domNodes": 22,
"jsHeapTotalBytes": 19673088,
"scriptDurationMs": 20.058,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1689.799999999991,
"styleRecalcs": 75,
"styleRecalcDurationMs": 37.278,
"layouts": 16,
"layoutDurationMs": 4.468,
"taskDurationMs": 745.13,
"heapDeltaBytes": -19014096,
"heapUsedBytes": 50115568,
"domNodes": -265,
"jsHeapTotalBytes": 19537920,
"scriptDurationMs": 96.82,
"eventListeners": -133,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1740.0569999999789,
"styleRecalcs": 78,
"styleRecalcDurationMs": 42.903000000000006,
"layouts": 16,
"layoutDurationMs": 4.493,
"taskDurationMs": 789.632,
"heapDeltaBytes": -18380656,
"heapUsedBytes": 50764960,
"domNodes": -264,
"jsHeapTotalBytes": 20586496,
"scriptDurationMs": 99.929,
"eventListeners": -133,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-transition-enter",
"durationMs": 1199.2819999999824,
"styleRecalcs": 19,
"styleRecalcDurationMs": 31.686000000000007,
"layouts": 15,
"layoutDurationMs": 13.661999999999997,
"taskDurationMs": 852.2030000000001,
"heapDeltaBytes": 44060,
"heapUsedBytes": 72940996,
"domNodes": 13673,
"jsHeapTotalBytes": 15204352,
"scriptDurationMs": 37.242999999999995,
"eventListeners": 2371,
"totalBlockingTimeMs": 153,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "viewport-pan-sweep",
"durationMs": 8159.999999999968,
"styleRecalcs": 250,
"styleRecalcDurationMs": 41.751999999999995,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4139.603,
"heapDeltaBytes": 16502872,
"heapUsedBytes": 77934072,
"domNodes": -264,
"jsHeapTotalBytes": 7491584,
"scriptDurationMs": 1307.4699999999998,
"eventListeners": -113,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "viewport-pan-sweep",
"durationMs": 8178.574000000026,
"styleRecalcs": 250,
"styleRecalcDurationMs": 40.701,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4479.986999999999,
"heapDeltaBytes": 17018576,
"heapUsedBytes": 76358000,
"domNodes": -267,
"jsHeapTotalBytes": 8015872,
"scriptDurationMs": 1471.9119999999998,
"eventListeners": -113,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-idle",
"durationMs": 13442.434999999989,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 13425.333999999999,
"heapDeltaBytes": -71531148,
"heapUsedBytes": 154291372,
"domNodes": -8311,
"jsHeapTotalBytes": -12783616,
"scriptDurationMs": 555.2550000000001,
"eventListeners": -16382,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.219999999999953,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "vue-large-graph-idle",
"durationMs": 13490.035000000034,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 13474.231,
"heapDeltaBytes": -36911036,
"heapUsedBytes": 170603284,
"domNodes": -8311,
"jsHeapTotalBytes": -9895936,
"scriptDurationMs": 553.356,
"eventListeners": -16388,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.776666666666642,
"p95FrameDurationMs": 16.80000000000291
},
{
"name": "vue-large-graph-pan",
"durationMs": 15955.720999999983,
"styleRecalcs": 83,
"styleRecalcDurationMs": 14.085999999999988,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 15930.654,
"heapDeltaBytes": -41911804,
"heapUsedBytes": 167806000,
"domNodes": -8311,
"jsHeapTotalBytes": -12259328,
"scriptDurationMs": 852.0840000000001,
"eventListeners": -16382,
"totalBlockingTimeMs": 21,
"frameDurationMs": 17.220000000000073,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 16461.150999999973,
"styleRecalcs": 93,
"styleRecalcDurationMs": 15.274999999999983,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 16437.497,
"heapDeltaBytes": -53295824,
"heapUsedBytes": 157373368,
"domNodes": -8311,
"jsHeapTotalBytes": -13832192,
"scriptDurationMs": 933.5749999999999,
"eventListeners": -16380,
"totalBlockingTimeMs": 50,
"frameDurationMs": 17.776666666666642,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "workflow-execution",
"durationMs": 467.2640000000001,
"styleRecalcs": 17,
"styleRecalcDurationMs": 23.378999999999998,
"layouts": 3,
"layoutDurationMs": 1.392,
"taskDurationMs": 128.34699999999998,
"heapDeltaBytes": -15743664,
"heapUsedBytes": 52502724,
"domNodes": 154,
"jsHeapTotalBytes": 6565888,
"scriptDurationMs": 11.202,
"eventListeners": 67,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "workflow-execution",
"durationMs": 484.5189999999775,
"styleRecalcs": 12,
"styleRecalcDurationMs": 22.272999999999996,
"layouts": 2,
"layoutDurationMs": 0.47400000000000003,
"taskDurationMs": 120.49100000000001,
"heapDeltaBytes": -15860172,
"heapUsedBytes": 52523128,
"domNodes": 115,
"jsHeapTotalBytes": 6041600,
"scriptDurationMs": 9.345,
"eventListeners": 67,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
}
]
} |
🎨 Storybook: 🚧 Building... |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe pull request adds a camera-info widget with interactive Three.js overlays, orbit and transform gizmos, look-through editing, widget synchronization, viewport render callbacks, and custom camera-up controls. ChangesCamera info and camera controls
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant CameraInfoVue
participant useCameraInfo
participant CameraInfoViewport
participant CameraInfoOverlay
participant LiteGraphWidgets
CameraInfoVue->>useCameraInfo: initialize(container)
useCameraInfo->>LiteGraphWidgets: read camera state
useCameraInfo->>CameraInfoViewport: create viewport
CameraInfoViewport->>CameraInfoOverlay: apply subject camera state
CameraInfoVue->>useCameraInfo: setLookThrough / setGizmosVisible
useCameraInfo->>CameraInfoViewport: update interaction state
CameraInfoViewport->>useCameraInfo: emit field updates
useCameraInfo->>LiteGraphWidgets: write changed values
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Collaborator
Author
|
Note: there's a known bug where the node height resets to its default value after a refresh. This comes from dynamicInput, isn't specific to this PR, and try to fix in another PR #13456 . |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/cameraInfo/CameraInfo.vue`:
- Around line 165-190: Update the mode-change handling near
transformGizmoOptions so transformGizmoMode is reset to the supported fallback
when the current selection becomes disabled, such as switching from
quaternion/camera-rotate to orbit. Reconcile the selection against the computed
enabled options and preserve valid selections; ensure the viewport receives the
fallback rather than retaining an invalid gizmo mode.
In `@src/composables/useCameraInfo.ts`:
- Around line 69-72: Update the initialization error handling in useCameraInfo
to obtain the user-facing alert text through vue-i18n instead of hardcoding it.
Add the corresponding translation entry to src/locales/en/main.json and pass the
translated message to useToastStore().addAlert, while leaving the console error
unchanged.
- Around line 53-79: Update cleanup and initialization around wrappedSet,
unwireWidgets, and wireNodeMouseStatus to fully unwind prior hooks before
reinitializing. Remove each unwired widget from wrappedSet, retain the original
node mouse handlers so cleanup restores them, and clean any existing or
partially initialized viewport before creating a new one in initialize.
In `@src/extensions/core/cameraInfo/CameraInfoOverlay.test.ts`:
- Around line 90-109: Update the “camera type swap” test to assert that the
rebuilt CameraHelper references the newly active camera returned by
getSubjectCamera(), rather than only checking that its object identity differs
from the initial helper. Keep the behavior-focused assertions and avoid
change-detector or implementation-identity checks.
In `@src/extensions/core/cameraInfo/CameraInfoViewport.ts`:
- Around line 3-31: Separate type-only imports in CameraInfoViewport.ts: import
Viewport3d, CameraHandle, OrbitHandles, and other runtime values normally, while
importing Load3DOptions, CameraHandleMode, CameraHandleTransform,
OrbitHandleType, and camera-info types through dedicated import type
declarations. In CameraHandle.test.ts, keep the CameraHandle value import
separate from dedicated type imports for CameraHandleMode and
CameraHandleTransform.
- Around line 481-484: Update CameraInfoViewport.onPostRender so it never
synchronously calls viewport.forceRender from the post-render hook. Move
fitSubjectAspect synchronization before the main render, or defer the required
follow-up render until the current frame completes, while preserving
aspect-change handling and avoiding re-entry into Viewport3d.renderView.
In `@src/extensions/core/cameraInfo/cameraTransform.test.ts`:
- Around line 129-143: The roll test should verify the actual rotation behavior
rather than only checking that the quaternion changed. Update the “rotates
around the view axis without moving the camera position” test to assert that the
forward direction remains unchanged and that the up direction rotates by exactly
45 degrees around that forward axis, using deterministic vector comparisons.
In `@src/extensions/core/cameraInfo/handles/handlePicking.test.ts`:
- Around line 67-78: Adjust the test for pickHandleAtPointer so the ray does not
intersect either handle directly, forcing the screen-space fallback
nearest-candidate branch while keeping both handles within tolerance. Preserve
the assertion that the closer projected handle, “yaw”, is selected.
In `@src/extensions/core/cameraInfo/handles/rollDragMath.ts`:
- Around line 15-24: Update the roll orientation setup around the backward
vector normalization to detect coincident cameraPos and target positions and
choose a stable nonzero fallback direction before computing right and up.
Preserve the existing orientation behavior for distinct positions, and add a
regression test covering coincident inputs to verify the roll drag math remains
nondegenerate.
In `@src/extensions/core/cameraInfo/lookThroughDragMath.ts`:
- Around line 199-204: Update the look-at dolly calculation in the visible
drag-math flow to clamp nextDistance to both MIN_DISTANCE and the shared
maximum-distance constant used by orbit dolly and handle dragging. Preserve the
existing exponential factor and ensure the clamped distance is used when
computing the next position.
- Around line 131-139: Update the quaternion rotation logic near yawQ and pitchQ
so yaw uses the camera’s quaternion-derived local up axis rather than global Y,
and apply both rotations in local space instead of premultiplying yawQ. Preserve
normalization and add a regression covering horizontal movement from a rolled
starting quaternion.
In `@src/extensions/core/cameraInfo/widgetBridge.ts`:
- Around line 34-36: Update num in widgetBridge.ts to apply camera-specific
domain bounds when reading zoom, FOV, distance, and pitch, matching the limits
enforced by the drag controls. Keep finite numeric values only when they fall
within each field’s supported range; otherwise return the provided fallback
before values reach Three.js calculations.
- Around line 1-6: Separate type-only imports from value imports across the
camera info files: update src/extensions/core/cameraInfo/widgetBridge.ts lines
1-6 and src/extensions/core/cameraInfo/CameraInfoOverlay.ts lines 5-10 to use
dedicated import type statements, and update
src/extensions/core/cameraInfo/cameraTransform.test.ts lines 3-4,
src/extensions/core/cameraInfo/lookThroughDragMath.test.ts lines 4-5,
src/extensions/core/cameraInfo/widgetBridge.test.ts lines 3-8, and
src/extensions/core/cameraInfo/CameraInfoOverlay.test.ts lines 4-5 so
CameraInfoState or NodeWithWidgets is imported separately from value symbols.
In `@src/extensions/core/load3d/Viewport3d.ts`:
- Around line 198-210: Update runPostRenderCallbacks to iterate over a stable
snapshot of postRenderCallbacks rather than the live array, so disposals during
dispatch do not skip callbacks and newly registered callbacks wait until the
next render.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6c3d8342-c4a2-4b59-a099-69711fb2fb37
📒 Files selected for processing (42)
src/components/cameraInfo/CameraInfo.vuesrc/components/load3d/Load3DControls.vuesrc/components/load3d/controls/CameraControls.vuesrc/composables/useCameraInfo.test.tssrc/composables/useCameraInfo.tssrc/composables/useLoad3d.tssrc/extensions/core/cameraInfo.tssrc/extensions/core/cameraInfo/CameraInfoOverlay.test.tssrc/extensions/core/cameraInfo/CameraInfoOverlay.tssrc/extensions/core/cameraInfo/CameraInfoViewport.test.tssrc/extensions/core/cameraInfo/CameraInfoViewport.tssrc/extensions/core/cameraInfo/cameraTransform.test.tssrc/extensions/core/cameraInfo/cameraTransform.tssrc/extensions/core/cameraInfo/handles/CameraHandle.test.tssrc/extensions/core/cameraInfo/handles/CameraHandle.tssrc/extensions/core/cameraInfo/handles/OrbitHandles.test.tssrc/extensions/core/cameraInfo/handles/OrbitHandles.tssrc/extensions/core/cameraInfo/handles/RollHandle.test.tssrc/extensions/core/cameraInfo/handles/RollHandle.tssrc/extensions/core/cameraInfo/handles/TargetHandle.tssrc/extensions/core/cameraInfo/handles/handlePicking.test.tssrc/extensions/core/cameraInfo/handles/handlePicking.tssrc/extensions/core/cameraInfo/handles/orbitDragMath.test.tssrc/extensions/core/cameraInfo/handles/orbitDragMath.tssrc/extensions/core/cameraInfo/handles/rollDragMath.test.tssrc/extensions/core/cameraInfo/handles/rollDragMath.tssrc/extensions/core/cameraInfo/handles/types.tssrc/extensions/core/cameraInfo/lookThroughDragMath.test.tssrc/extensions/core/cameraInfo/lookThroughDragMath.tssrc/extensions/core/cameraInfo/types.tssrc/extensions/core/cameraInfo/widgetBridge.test.tssrc/extensions/core/cameraInfo/widgetBridge.tssrc/extensions/core/load3d/CameraManager.tssrc/extensions/core/load3d/Load3d.test.tssrc/extensions/core/load3d/Viewport3d.test.tssrc/extensions/core/load3d/Viewport3d.tssrc/extensions/core/load3d/createViewport3d.tssrc/extensions/core/load3d/interfaces.tssrc/extensions/core/load3d/nodeTypes.tssrc/extensions/core/load3dLazy.tssrc/locales/en/main.jsonsrc/renderer/extensions/vueNodes/widgets/registry/widgetRegistry.ts
💤 Files with no reviewable changes (1)
- src/extensions/core/load3d/Viewport3d.test.ts
jtydhr88
force-pushed
the
feat/create-camera-info-preview
branch
from
July 18, 2026 03:33
e893bf0 to
32b51dc
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
src/composables/useCameraInfo.ts (1)
59-63: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClean up before returning for a missing node.
If
nodeRefbecomes null after initialization, Line 61 returns before removing the existing viewport or restoring the old node’s callbacks. Move cleanup before the guard and add a null-transition regression test.Proposed fix
const initialize = (container: HTMLElement): void => { + if (viewport) cleanup() const raw = toRaw(node.value) if (!raw || !container) return - if (viewport) cleanup()As per coding guidelines, implement proper cleanup and error handling.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/composables/useCameraInfo.ts` around lines 59 - 63, Update initialize in useCameraInfo so cleanup runs before the missing-node guard, ensuring an existing viewport and the previous node’s callbacks are released when nodeRef becomes null. Preserve the early return after cleanup, and add a regression test covering the transition from an initialized node to a null node.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/load3d/controls/CameraControls.vue`:
- Around line 32-41: Update the icon class composition in the CameraControls
template to replace the font-size utility text-lg with the appropriate size-*
utility, while preserving the existing icon selection and text-base-foreground
classes.
In `@src/extensions/core/cameraInfo/CameraInfoViewport.test.ts`:
- Around line 343-362: Extend the camera handle change tests around
CameraInfoViewport to cover quaternion rotation mode: select the camera-rotate
mode, dispatch objectChange on controlsOf(viewport.cameraHandle), and verify
onHandleDrag is called for mode.quat_x, mode.quat_y, mode.quat_z, and
mode.quat_w with numeric values. Preserve the existing translation-mode
coverage.
In `@src/extensions/core/cameraInfo/CameraInfoViewport.ts`:
- Around line 175-185: Update setLookThrough in CameraInfoViewport to save the
controls’ prior enabled state and disable standard controls when look-through is
activated. Restore that saved state when look-through is deactivated, ensuring
custom mouse-look input cannot simultaneously move the existing orbit controls.
- Around line 214-226: Update the handle visibility logic around wantTarget and
wantCamera so both targetHandle and cameraHandle are also gated by gizmosOn.
Preserve the existing transform mode applicability checks, but ensure selecting
“Hide gizmos” makes these handles invisible.
In `@src/extensions/core/cameraInfo/handles/CameraHandle.test.ts`:
- Around line 46-72: Extend the CameraHandle tests to cover emitted drag
behavior by dispatching objectChange and dragging-changed events on the handle’s
controlled object. Assert that the resulting callback emits the updated
transform, current mode, and dragging boolean, while preserving the existing
state and proxy tests.
In `@src/extensions/core/load3d/CameraManager.ts`:
- Around line 177-179: Update the toggleCamera method to copy the current
camera’s up vector to the newly created camera alongside position and rotation,
preserving custom-up orientation and allowing subsequent setUseCustomUp behavior
to remain consistent.
In `@src/extensions/core/load3d/Viewport3d.test.ts`:
- Around line 480-518: Extend the “render callback dispatch” tests to exercise
the real render pipeline through renderView(), recording events from a
pre-render callback, the main scene-render step, and a post-render callback.
Assert the ordering is exactly pre → main scene → post, without directly
invoking runPreRenderCallbacks or runPostRenderCallbacks and without adding
change-detector coverage.
In `@src/locales/en/main.json`:
- Line 2270: Update the failedToInitializeCameraInfoViewer locale message to
include the recovery step “Try refreshing the node,” while preserving the
existing initialization failure context.
---
Duplicate comments:
In `@src/composables/useCameraInfo.ts`:
- Around line 59-63: Update initialize in useCameraInfo so cleanup runs before
the missing-node guard, ensuring an existing viewport and the previous node’s
callbacks are released when nodeRef becomes null. Preserve the early return
after cleanup, and add a regression test covering the transition from an
initialized node to a null node.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8c572bbe-8769-4a93-a4a1-0e077d6f9930
📒 Files selected for processing (45)
src/components/cameraInfo/CameraInfo.test.tssrc/components/cameraInfo/CameraInfo.vuesrc/components/load3d/Load3DControls.vuesrc/components/load3d/controls/CameraControls.vuesrc/composables/useCameraInfo.test.tssrc/composables/useCameraInfo.tssrc/composables/useLoad3d.tssrc/extensions/core/cameraInfo.tssrc/extensions/core/cameraInfo/CameraInfoOverlay.test.tssrc/extensions/core/cameraInfo/CameraInfoOverlay.tssrc/extensions/core/cameraInfo/CameraInfoViewport.test.tssrc/extensions/core/cameraInfo/CameraInfoViewport.tssrc/extensions/core/cameraInfo/cameraTransform.test.tssrc/extensions/core/cameraInfo/cameraTransform.tssrc/extensions/core/cameraInfo/handles/CameraHandle.test.tssrc/extensions/core/cameraInfo/handles/CameraHandle.tssrc/extensions/core/cameraInfo/handles/OrbitHandles.test.tssrc/extensions/core/cameraInfo/handles/OrbitHandles.tssrc/extensions/core/cameraInfo/handles/RollHandle.test.tssrc/extensions/core/cameraInfo/handles/RollHandle.tssrc/extensions/core/cameraInfo/handles/TargetHandle.test.tssrc/extensions/core/cameraInfo/handles/TargetHandle.tssrc/extensions/core/cameraInfo/handles/handlePicking.test.tssrc/extensions/core/cameraInfo/handles/handlePicking.tssrc/extensions/core/cameraInfo/handles/orbitDragMath.test.tssrc/extensions/core/cameraInfo/handles/orbitDragMath.tssrc/extensions/core/cameraInfo/handles/rollDragMath.test.tssrc/extensions/core/cameraInfo/handles/rollDragMath.tssrc/extensions/core/cameraInfo/handles/types.tssrc/extensions/core/cameraInfo/lookThroughDragMath.test.tssrc/extensions/core/cameraInfo/lookThroughDragMath.tssrc/extensions/core/cameraInfo/types.tssrc/extensions/core/cameraInfo/widgetBridge.test.tssrc/extensions/core/cameraInfo/widgetBridge.tssrc/extensions/core/load3d/CameraManager.test.tssrc/extensions/core/load3d/CameraManager.tssrc/extensions/core/load3d/Load3d.test.tssrc/extensions/core/load3d/Viewport3d.test.tssrc/extensions/core/load3d/Viewport3d.tssrc/extensions/core/load3d/createViewport3d.tssrc/extensions/core/load3d/interfaces.tssrc/extensions/core/load3d/nodeTypes.tssrc/extensions/core/load3dLazy.tssrc/locales/en/main.jsonsrc/renderer/extensions/vueNodes/widgets/registry/widgetRegistry.ts
jtydhr88
force-pushed
the
feat/create-camera-info-preview
branch
from
July 18, 2026 12:58
32b51dc to
2162a93
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/composables/useCameraInfo.test.ts`:
- Around line 140-145: Remove the “ignores toolbar actions before
initialization” test from the useCameraInfo test suite, including its
optional-chaining no-throw assertion; retain tests that verify observable camera
behavior instead.
In `@src/extensions/core/cameraInfo/CameraInfoOverlay.ts`:
- Around line 110-119: Update CameraInfoOverlay.dispose() to detach
referenceGroup, subjectPerspective, and subjectOrthographic from the scene
before disposing their resources. Preserve the existing idempotency guard and
cleanup behavior, ensuring a reused scene cannot traverse these disposed
objects.
In `@src/extensions/core/cameraInfo/CameraInfoViewport.test.ts`:
- Around line 351-370: Update the camera-handle change tests around
CameraInfoViewport to assert the complete callback contract: verify position_x,
position_y, and position_z, plus target_x, target_y, and target_z, with numeric
values for each emitted coordinate. Cover both the position and target callback
cases so missing y/z widget writes fail the tests.
In `@src/extensions/core/cameraInfo/CameraInfoViewport.ts`:
- Around line 307-310: Update CameraInfoViewport.updatePointer to convert
pointer coordinates through viewport.clientPointToNdc() instead of calculating
NDC from the full canvas bounds. When fitting the subject/look-through camera,
use Viewport3d’s effective letterboxed render-viewport aspect rather than the
canvas aspect, ensuring both picking and projection calculations match the
rendered sub-viewport.
In `@src/extensions/core/cameraInfo/cameraTransform.test.ts`:
- Around line 70-79: Extend the “uses explicit position” test around
computeSubjectTransform to verify orientation as well as position. Use a
non-axis-aligned lookAt target, transform the camera’s local -Z direction by the
returned quaternion, and assert the resulting direction numerically points
toward the target while preserving the existing position assertion.
In `@src/extensions/core/cameraInfo/handles/OrbitHandles.test.ts`:
- Around line 33-63: Add a nonzero-pitch test case for OrbitHandles.update() and
validate the world-space positions of both the pitch and distance handles
against the expected camera direction, including deterministic edge/permutation
coverage for the pitch branch. Keep assertions focused on resulting geometry
rather than implementation details.
- Around line 84-89: Remove the child mesh visibility assertion from the test
“isVisible reflects the root group, not individual handle meshes” in
OrbitHandles.test.ts. Keep the handles.update setup and the handles.isVisible()
expectation, which validates the observable OrbitHandles behavior without
asserting Three.js child-visibility internals.
In `@src/extensions/core/cameraInfo/handles/rollDragMath.test.ts`:
- Around line 18-21: Expand the rollBasis tests in rollDragMath.test.ts to fully
validate the straight-down fallback by asserting its complete orthonormal basis,
not just right.x. Add a corresponding straight-up camera case and assert the
complete expected basis there as well, covering both vertical-pole fallback
directions and their numerical geometry.
In `@src/extensions/core/cameraInfo/handles/RollHandle.test.ts`:
- Around line 54-77: Add a deterministic test in the roll-handle positioning
suite that sets roll to 90 degrees and verifies the pickable handle’s world
position relative to the target and camera basis. Exercise the handle-position
calculation used by the roll handle, such as through the existing handle.update
flow, and assert the resulting coordinates numerically rather than only checking
the default target placement.
In `@src/extensions/core/cameraInfo/lookThroughDragMath.test.ts`:
- Around line 82-105: Strengthen the quaternion coverage in the
rotateSubjectByDrag tests around rotateSubjectByDrag by adding deterministic
yaw-only and pitch-only cases that assert the resulting transformed forward and
up vectors. Include a rolled starting quaternion to verify yaw rotates around
world Y, and cover the relevant axis/sign permutations so incorrect axis
selection or multiplication order fails while preserving the existing
unit-quaternion assertions.
In `@src/extensions/core/cameraInfo/widgetBridge.test.ts`:
- Around line 20-43: Update the test for readStateFromWidgets to set camera_type
to 'orthographic' and assert that state.cameraType is 'orthographic'. Keep the
existing widget values and assertions unchanged so the test exercises the
non-default camera-type behavior.
In `@src/extensions/core/load3d/Viewport3d.ts`:
- Around line 202-216: Update addPreRenderCallback and addPostRenderCallback to
store a unique registration wrapper rather than the raw callback, and have each
returned disposer remove only that wrapper. Ensure repeated disposer calls are
idempotent and do not remove another registration of the same callback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8a1cbf2f-7497-4b51-8bdd-39aa8182c66f
📒 Files selected for processing (45)
src/components/cameraInfo/CameraInfo.test.tssrc/components/cameraInfo/CameraInfo.vuesrc/components/load3d/Load3DControls.vuesrc/components/load3d/controls/CameraControls.vuesrc/composables/useCameraInfo.test.tssrc/composables/useCameraInfo.tssrc/composables/useLoad3d.tssrc/extensions/core/cameraInfo.tssrc/extensions/core/cameraInfo/CameraInfoOverlay.test.tssrc/extensions/core/cameraInfo/CameraInfoOverlay.tssrc/extensions/core/cameraInfo/CameraInfoViewport.test.tssrc/extensions/core/cameraInfo/CameraInfoViewport.tssrc/extensions/core/cameraInfo/cameraTransform.test.tssrc/extensions/core/cameraInfo/cameraTransform.tssrc/extensions/core/cameraInfo/handles/CameraHandle.test.tssrc/extensions/core/cameraInfo/handles/CameraHandle.tssrc/extensions/core/cameraInfo/handles/OrbitHandles.test.tssrc/extensions/core/cameraInfo/handles/OrbitHandles.tssrc/extensions/core/cameraInfo/handles/RollHandle.test.tssrc/extensions/core/cameraInfo/handles/RollHandle.tssrc/extensions/core/cameraInfo/handles/TargetHandle.test.tssrc/extensions/core/cameraInfo/handles/TargetHandle.tssrc/extensions/core/cameraInfo/handles/handlePicking.test.tssrc/extensions/core/cameraInfo/handles/handlePicking.tssrc/extensions/core/cameraInfo/handles/orbitDragMath.test.tssrc/extensions/core/cameraInfo/handles/orbitDragMath.tssrc/extensions/core/cameraInfo/handles/rollDragMath.test.tssrc/extensions/core/cameraInfo/handles/rollDragMath.tssrc/extensions/core/cameraInfo/handles/types.tssrc/extensions/core/cameraInfo/lookThroughDragMath.test.tssrc/extensions/core/cameraInfo/lookThroughDragMath.tssrc/extensions/core/cameraInfo/types.tssrc/extensions/core/cameraInfo/widgetBridge.test.tssrc/extensions/core/cameraInfo/widgetBridge.tssrc/extensions/core/load3d/CameraManager.test.tssrc/extensions/core/load3d/CameraManager.tssrc/extensions/core/load3d/Load3d.test.tssrc/extensions/core/load3d/Viewport3d.test.tssrc/extensions/core/load3d/Viewport3d.tssrc/extensions/core/load3d/createViewport3d.tssrc/extensions/core/load3d/interfaces.tssrc/extensions/core/load3d/nodeTypes.tssrc/extensions/core/load3dLazy.tssrc/locales/en/main.jsonsrc/renderer/extensions/vueNodes/widgets/registry/widgetRegistry.ts
jtydhr88
force-pushed
the
feat/create-camera-info-preview
branch
from
July 19, 2026 11:53
2162a93 to
f64c8fb
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/extensions/core/load3d/Viewport3d.test.ts (1)
442-517: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider exporting
LetterboxDimmerfor test reuse instead of re-declaring its shape.
DimmerInternals.letterboxDimmerduplicates the privateLetterboxDimmertype fromViewport3d.ts. Exporting it would let the test import the real type and avoid type drift if the shape changes.♻️ Proposed fix
-type LetterboxDimmer = { +export type LetterboxDimmer = { scene: THREE.Scene camera: THREE.OrthographicCamera geometry: THREE.PlaneGeometry material: THREE.MeshBasicMaterial }Then in the test:
import type { LetterboxDimmer } from '@/extensions/core/load3d/Viewport3d'and use it directly inDimmerInternals.Based on learnings: "avoid duplicating interface/type definitions... Import real type definitions from the component modules under test."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/extensions/core/load3d/Viewport3d.test.ts` around lines 442 - 517, Export the existing LetterboxDimmer type from Viewport3d.ts, then update the test to import it and use it for DimmerInternals.letterboxDimmer instead of redeclaring the scene, camera, geometry, and material shape; leave the other test-only internals unchanged.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/composables/useCameraInfo.ts`:
- Around line 59-80: Update the catch block in initialize so any
CameraInfoViewport constructed before wireWidgetsToOverlay or
wireNodeMouseStatus fails is disposed via cleanup(). Preserve the existing error
logging and toast behavior while ensuring partial initialization cannot leave
viewport resources active.
In `@src/extensions/core/cameraInfo/CameraInfoOverlay.test.ts`:
- Around line 106-121: Update the camera-type swap test around applyState and
the newHelper lookup to assert that the scene contains exactly one
THREE.CameraHelper after switching to orthographic mode, while preserving the
existing active-camera and helper-target assertions.
In `@src/extensions/core/cameraInfo/handles/CameraHandle.test.ts`:
- Around line 53-55: Update the tests for CameraHandle visibility, including the
cases around “starts hidden with controls disabled” and “controls.enabled
together,” to assert controls().enabled alongside isVisible(). Use the existing
controls() helper so both visibility and control-enabled state are verified.
In `@src/extensions/core/cameraInfo/handles/OrbitHandles.test.ts`:
- Around line 33-45: Strengthen the yaw-handle assertion in the test around
handles.update by verifying yawHandle.position.z is close to the expected
RING_RADIUS at yaw=0, while retaining the x-position assertion. Replace the
sign-only z check with a magnitude check that directly validates the ring
placement.
In `@src/extensions/core/cameraInfo/handles/OrbitHandles.ts`:
- Around line 35-61: Extract the shared handle/glow mesh builders and constants
into a common handle-visuals module. Update
src/extensions/core/cameraInfo/handles/OrbitHandles.ts lines 35-61 to import and
use the shared makeHandle, makeGlow, BASE_GLOW_OPACITY, HOVER_GLOW_OPACITY,
HOVER_SCALE, HANDLE_RADIUS, and GLOW_RADIUS symbols; apply the same replacement
to src/extensions/core/cameraInfo/handles/RollHandle.ts lines 19-32, removing
its duplicated builders and glow construction.
In `@src/extensions/core/cameraInfo/handles/rollDragMath.test.ts`:
- Around line 18-35: Strengthen both vertical-pole fallback tests for rollBasis
by asserting the exact deterministic up vector: straight-down must produce (0,
0, -1), and straight-up must produce (0, 0, 1). Add the missing right.x
assertion to the straight-up test while preserving the existing length and
orthogonality checks.
---
Outside diff comments:
In `@src/extensions/core/load3d/Viewport3d.test.ts`:
- Around line 442-517: Export the existing LetterboxDimmer type from
Viewport3d.ts, then update the test to import it and use it for
DimmerInternals.letterboxDimmer instead of redeclaring the scene, camera,
geometry, and material shape; leave the other test-only internals unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a73a55da-cb4c-4c3d-b0a9-de10b1ef9b70
📒 Files selected for processing (45)
src/components/cameraInfo/CameraInfo.test.tssrc/components/cameraInfo/CameraInfo.vuesrc/components/load3d/Load3DControls.vuesrc/components/load3d/controls/CameraControls.vuesrc/composables/useCameraInfo.test.tssrc/composables/useCameraInfo.tssrc/composables/useLoad3d.tssrc/extensions/core/cameraInfo.tssrc/extensions/core/cameraInfo/CameraInfoOverlay.test.tssrc/extensions/core/cameraInfo/CameraInfoOverlay.tssrc/extensions/core/cameraInfo/CameraInfoViewport.test.tssrc/extensions/core/cameraInfo/CameraInfoViewport.tssrc/extensions/core/cameraInfo/cameraTransform.test.tssrc/extensions/core/cameraInfo/cameraTransform.tssrc/extensions/core/cameraInfo/handles/CameraHandle.test.tssrc/extensions/core/cameraInfo/handles/CameraHandle.tssrc/extensions/core/cameraInfo/handles/OrbitHandles.test.tssrc/extensions/core/cameraInfo/handles/OrbitHandles.tssrc/extensions/core/cameraInfo/handles/RollHandle.test.tssrc/extensions/core/cameraInfo/handles/RollHandle.tssrc/extensions/core/cameraInfo/handles/TargetHandle.test.tssrc/extensions/core/cameraInfo/handles/TargetHandle.tssrc/extensions/core/cameraInfo/handles/handlePicking.test.tssrc/extensions/core/cameraInfo/handles/handlePicking.tssrc/extensions/core/cameraInfo/handles/orbitDragMath.test.tssrc/extensions/core/cameraInfo/handles/orbitDragMath.tssrc/extensions/core/cameraInfo/handles/rollDragMath.test.tssrc/extensions/core/cameraInfo/handles/rollDragMath.tssrc/extensions/core/cameraInfo/handles/types.tssrc/extensions/core/cameraInfo/lookThroughDragMath.test.tssrc/extensions/core/cameraInfo/lookThroughDragMath.tssrc/extensions/core/cameraInfo/types.tssrc/extensions/core/cameraInfo/widgetBridge.test.tssrc/extensions/core/cameraInfo/widgetBridge.tssrc/extensions/core/load3d/CameraManager.test.tssrc/extensions/core/load3d/CameraManager.tssrc/extensions/core/load3d/Load3d.test.tssrc/extensions/core/load3d/Viewport3d.test.tssrc/extensions/core/load3d/Viewport3d.tssrc/extensions/core/load3d/createViewport3d.tssrc/extensions/core/load3d/interfaces.tssrc/extensions/core/load3d/nodeTypes.tssrc/extensions/core/load3dLazy.tssrc/locales/en/main.jsonsrc/renderer/extensions/vueNodes/widgets/registry/widgetRegistry.ts
Comment on lines
+33
to
+45
| it('positions the yaw handle on the +Z side of the ring at yaw=0', () => { | ||
| handles.update({ | ||
| ...DEFAULT_CAMERA_INFO_STATE, | ||
| mode: 'orbit', | ||
| orbit: { yaw: 0, pitch: 0, distance: 5 } | ||
| }) | ||
|
|
||
| const yawHandle = handles | ||
| .pickableMeshes() | ||
| .find((m) => m.userData.handleType === 'yaw')! | ||
| expect(yawHandle.position.x).toBeCloseTo(0) | ||
| expect(yawHandle.position.z).toBeGreaterThan(0) | ||
| }) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Strengthen the yaw-handle position assertion.
Only the sign of z is checked; the expected magnitude (RING_RADIUS) is never verified, so a scaling regression in the yaw-handle placement would slip through.
🧪 Proposed tightening
expect(yawHandle.position.x).toBeCloseTo(0)
- expect(yawHandle.position.z).toBeGreaterThan(0)
+ expect(yawHandle.position.z).toBeCloseTo(1.5)As per path instructions, ensure new geometry behaviors have meaningful assertions rather than weak change-detector-style checks.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('positions the yaw handle on the +Z side of the ring at yaw=0', () => { | |
| handles.update({ | |
| ...DEFAULT_CAMERA_INFO_STATE, | |
| mode: 'orbit', | |
| orbit: { yaw: 0, pitch: 0, distance: 5 } | |
| }) | |
| const yawHandle = handles | |
| .pickableMeshes() | |
| .find((m) => m.userData.handleType === 'yaw')! | |
| expect(yawHandle.position.x).toBeCloseTo(0) | |
| expect(yawHandle.position.z).toBeGreaterThan(0) | |
| }) | |
| it('positions the yaw handle on the +Z side of the ring at yaw=0', () => { | |
| handles.update({ | |
| ...DEFAULT_CAMERA_INFO_STATE, | |
| mode: 'orbit', | |
| orbit: { yaw: 0, pitch: 0, distance: 5 } | |
| }) | |
| const yawHandle = handles | |
| .pickableMeshes() | |
| .find((m) => m.userData.handleType === 'yaw')! | |
| expect(yawHandle.position.x).toBeCloseTo(0) | |
| expect(yawHandle.position.z).toBeCloseTo(1.5) | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/extensions/core/cameraInfo/handles/OrbitHandles.test.ts` around lines 33
- 45, Strengthen the yaw-handle assertion in the test around handles.update by
verifying yawHandle.position.z is close to the expected RING_RADIUS at yaw=0,
while retaining the x-position assertion. Replace the sign-only z check with a
magnitude check that directly validates the ring placement.
Source: Path instructions
jtydhr88
force-pushed
the
feat/create-camera-info-preview
branch
from
July 19, 2026 15:30
f64c8fb to
d167e6a
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/composables/useCameraInfo.test.ts`:
- Around line 86-234: Restore a test in the useCameraInfo suite covering toolbar
actions before initialization: call setGizmosVisible, setTransformGizmoMode, and
setLookThrough without invoking initialize, then assert instances remains empty
to preserve the lazy-initialization guard.
In `@src/extensions/core/cameraInfo/CameraInfoViewport.test.ts`:
- Around line 386-388: Update the quaternion assertion in the camera drag test
around controlsOf(viewport.cameraHandle) to iterate over x, y, z, and w,
asserting onHandleDrag is called with each corresponding mode.quat_${axis} key
and a numeric value. Preserve the existing event dispatch and ensure all four
quaternion component emits are covered.
In `@src/extensions/core/cameraInfo/handles/CameraHandle.test.ts`:
- Around line 62-68: Update the setMode test around CameraHandle.setMode to
assert the underlying TransformControls instance’s mode after switching between
rotate and translate, rather than relying on handle.getMode(). Preserve coverage
of both mode transitions and verify the observable controls behavior.
In `@src/extensions/core/cameraInfo/handles/OrbitHandles.ts`:
- Around line 257-258: Change the module-level pure helper vec from a const
arrow function to a function declaration, preserving its Vector3Like input and
THREE.Vector3 output behavior. Leave buildArcCurve, makeHandle, and makeGlow
unchanged.
In `@src/extensions/core/cameraInfo/handles/RollHandle.ts`:
- Around line 112-118: Remove the redundant cameraVec object literals in both
update() and dragPlane(), and pass the cameraPos returned by
computeSubjectTransform(state) directly to rollBasis(). Preserve the existing
basis calculations and behavior.
In `@src/extensions/core/cameraInfo/lookThroughDragMath.test.ts`:
- Around line 185-209: Add a null-edge-case test for dollySubjectByWheel in
look_at mode, using stateWith to set lookAt.position equal to target and
asserting the result is null. Place it alongside the existing look_at dolly
tests and mirror the established null-path coverage pattern from
rotateSubjectByDrag.
In `@src/extensions/core/load3d/CameraManager.ts`:
- Around line 202-220: Update setCameraState so applying a state quaternion does
not unconditionally re-enable custom-up after setUseCustomUp(false). Respect the
existing usingCustomUp state when deciding whether to apply the
quaternion-derived activeCamera.up, customUp, and cameraUpStateChange update,
preserving explicit user-disabled custom-up behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 62f69acd-fea0-4971-be2c-4a71e1f961c1
📒 Files selected for processing (45)
src/components/cameraInfo/CameraInfo.test.tssrc/components/cameraInfo/CameraInfo.vuesrc/components/load3d/Load3DControls.vuesrc/components/load3d/controls/CameraControls.vuesrc/composables/useCameraInfo.test.tssrc/composables/useCameraInfo.tssrc/composables/useLoad3d.tssrc/extensions/core/cameraInfo.tssrc/extensions/core/cameraInfo/CameraInfoOverlay.test.tssrc/extensions/core/cameraInfo/CameraInfoOverlay.tssrc/extensions/core/cameraInfo/CameraInfoViewport.test.tssrc/extensions/core/cameraInfo/CameraInfoViewport.tssrc/extensions/core/cameraInfo/cameraTransform.test.tssrc/extensions/core/cameraInfo/cameraTransform.tssrc/extensions/core/cameraInfo/handles/CameraHandle.test.tssrc/extensions/core/cameraInfo/handles/CameraHandle.tssrc/extensions/core/cameraInfo/handles/OrbitHandles.test.tssrc/extensions/core/cameraInfo/handles/OrbitHandles.tssrc/extensions/core/cameraInfo/handles/RollHandle.test.tssrc/extensions/core/cameraInfo/handles/RollHandle.tssrc/extensions/core/cameraInfo/handles/TargetHandle.test.tssrc/extensions/core/cameraInfo/handles/TargetHandle.tssrc/extensions/core/cameraInfo/handles/handlePicking.test.tssrc/extensions/core/cameraInfo/handles/handlePicking.tssrc/extensions/core/cameraInfo/handles/orbitDragMath.test.tssrc/extensions/core/cameraInfo/handles/orbitDragMath.tssrc/extensions/core/cameraInfo/handles/rollDragMath.test.tssrc/extensions/core/cameraInfo/handles/rollDragMath.tssrc/extensions/core/cameraInfo/handles/types.tssrc/extensions/core/cameraInfo/lookThroughDragMath.test.tssrc/extensions/core/cameraInfo/lookThroughDragMath.tssrc/extensions/core/cameraInfo/types.tssrc/extensions/core/cameraInfo/widgetBridge.test.tssrc/extensions/core/cameraInfo/widgetBridge.tssrc/extensions/core/load3d/CameraManager.test.tssrc/extensions/core/load3d/CameraManager.tssrc/extensions/core/load3d/Load3d.test.tssrc/extensions/core/load3d/Viewport3d.test.tssrc/extensions/core/load3d/Viewport3d.tssrc/extensions/core/load3d/createViewport3d.tssrc/extensions/core/load3d/interfaces.tssrc/extensions/core/load3d/nodeTypes.tssrc/extensions/core/load3dLazy.tssrc/locales/en/main.jsonsrc/renderer/extensions/vueNodes/widgets/registry/widgetRegistry.ts
Comment on lines
+86
to
+234
| describe('useCameraInfo', () => { | ||
| it('constructs the viewport from widget state and exposes the mode', () => { | ||
| const node = makeNode({ mode: 'look_at', 'mode.distance': 7 }) | ||
| const container = document.createElement('div') | ||
| const camera = useCameraInfo(nodeRef(node)) | ||
|
|
||
| camera.initialize(container) | ||
|
|
||
| expect(ViewportMock).toHaveBeenCalledOnce() | ||
| const [ctorContainer, initialState] = instances[0].ctorArgs as [ | ||
| HTMLElement, | ||
| { mode: string; orbit: { distance: number } } | ||
| ] | ||
| expect(ctorContainer).toBe(container) | ||
| expect(initialState.mode).toBe('look_at') | ||
| expect(initialState.orbit.distance).toBe(7) | ||
| expect(camera.mode.value).toBe('look_at') | ||
| }) | ||
|
|
||
| it('does nothing when the node is null', () => { | ||
| const camera = useCameraInfo(ref(null)) | ||
| camera.initialize(document.createElement('div')) | ||
|
|
||
| expect(ViewportMock).not.toHaveBeenCalled() | ||
| }) | ||
|
|
||
| it('alerts and does not throw when the viewport fails to construct', () => { | ||
| ViewportMock.mockImplementationOnce(() => { | ||
| throw new Error('webgl unavailable') | ||
| }) | ||
| const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) | ||
| const camera = useCameraInfo(nodeRef(makeNode({ mode: 'orbit' }))) | ||
|
|
||
| expect(() => camera.initialize(document.createElement('div'))).not.toThrow() | ||
| expect(addAlert).toHaveBeenCalledOnce() | ||
|
|
||
| consoleError.mockRestore() | ||
| }) | ||
|
|
||
| it('forwards toolbar actions to the viewport', () => { | ||
| const camera = useCameraInfo(nodeRef(makeNode({ mode: 'orbit' }))) | ||
| camera.initialize(document.createElement('div')) | ||
|
|
||
| camera.setGizmosVisible(false) | ||
| camera.setTransformGizmoMode('camera-rotate') | ||
| camera.setLookThrough(true) | ||
|
|
||
| expect(instances[0].setGizmosVisible).toHaveBeenCalledWith(false) | ||
| expect(instances[0].setTransformGizmoMode).toHaveBeenCalledWith( | ||
| 'camera-rotate' | ||
| ) | ||
| expect(instances[0].setLookThrough).toHaveBeenCalledWith(true) | ||
| }) | ||
|
|
||
| it('re-applies state to the viewport when a widget changes, keeping the original callback', () => { | ||
| const node = makeNode({ mode: 'orbit', target_x: 0 }) | ||
| const original = vi.fn() | ||
| widget(node, 'target_x').callback = original | ||
| const camera = useCameraInfo(nodeRef(node)) | ||
| camera.initialize(document.createElement('div')) | ||
|
|
||
| const targetX = widget(node, 'target_x') | ||
| targetX.value = 3 | ||
| targetX.callback!(3) | ||
|
|
||
| expect(original).toHaveBeenCalledWith(3) | ||
| const applied = instances[0].applyState.mock.lastCall?.[0] as { | ||
| target: { x: number } | ||
| } | ||
| expect(applied.target.x).toBe(3) | ||
| }) | ||
|
|
||
| it('updates the mode ref when the mode widget changes', () => { | ||
| const node = makeNode({ mode: 'orbit' }) | ||
| const camera = useCameraInfo(nodeRef(node)) | ||
| camera.initialize(document.createElement('div')) | ||
|
|
||
| const modeWidget = widget(node, 'mode') | ||
| modeWidget.value = 'quaternion' | ||
| modeWidget.callback!('quaternion') | ||
|
|
||
| expect(camera.mode.value).toBe('quaternion') | ||
| }) | ||
|
|
||
| it('routes node hover into the viewport status flags', () => { | ||
| const node = makeNode({ mode: 'orbit' }) | ||
| const camera = useCameraInfo(nodeRef(node)) | ||
| camera.initialize(document.createElement('div')) | ||
|
|
||
| camera.handleMouseEnter() | ||
| camera.handleMouseLeave() | ||
| node.onMouseEnter?.() | ||
|
|
||
| expect(instances[0].viewport.updateStatusMouseOnScene).toHaveBeenCalledWith( | ||
| true | ||
| ) | ||
| expect(instances[0].viewport.updateStatusMouseOnScene).toHaveBeenCalledWith( | ||
| false | ||
| ) | ||
| expect(instances[0].viewport.updateStatusMouseOnNode).toHaveBeenCalledWith( | ||
| true | ||
| ) | ||
| }) | ||
|
|
||
| it('removes the viewport and restores widget callbacks on cleanup', () => { | ||
| const node = makeNode({ mode: 'orbit', target_x: 0 }) | ||
| const original = vi.fn() | ||
| widget(node, 'target_x').callback = original | ||
| const camera = useCameraInfo(nodeRef(node)) | ||
| camera.initialize(document.createElement('div')) | ||
|
|
||
| camera.cleanup() | ||
|
|
||
| expect(instances[0].remove).toHaveBeenCalledOnce() | ||
| expect(widget(node, 'target_x').callback).toBe(original) | ||
| }) | ||
|
|
||
| it('restores the node mouse handlers on cleanup', () => { | ||
| const node = makeNode({ mode: 'orbit' }) | ||
| const originalEnter = vi.fn() | ||
| const originalLeave = vi.fn() | ||
| node.onMouseEnter = originalEnter | ||
| node.onMouseLeave = originalLeave | ||
| const camera = useCameraInfo(nodeRef(node)) | ||
| camera.initialize(document.createElement('div')) | ||
|
|
||
| expect(node.onMouseEnter).not.toBe(originalEnter) | ||
|
|
||
| camera.cleanup() | ||
|
|
||
| expect(node.onMouseEnter).toBe(originalEnter) | ||
| expect(node.onMouseLeave).toBe(originalLeave) | ||
| }) | ||
|
|
||
| it('re-wires widgets on a second initialize after cleanup', () => { | ||
| const node = makeNode({ mode: 'orbit', target_x: 0 }) | ||
| const camera = useCameraInfo(nodeRef(node)) | ||
| camera.initialize(document.createElement('div')) | ||
| camera.cleanup() | ||
| camera.initialize(document.createElement('div')) | ||
|
|
||
| const targetX = widget(node, 'target_x') | ||
| targetX.value = 5 | ||
| targetX.callback!(5) | ||
|
|
||
| expect(instances).toHaveLength(2) | ||
| expect(instances[1].applyState).toHaveBeenCalled() | ||
| }) | ||
| }) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Restore the "toolbar actions before initialize()" regression test.
Based on learnings, a prior discussion on this exact file concluded that the test asserting instances stays empty when setGizmosVisible/setTransformGizmoMode/setLookThrough are called before camera.initialize() should be retained, since it verifies the lazy-initialization contract (not just a no-throw check). That test no longer appears in this revision — the line range it previously occupied (140-145) now holds an unrelated test. Please restore coverage for this guard, e.g.:
it('ignores toolbar actions before initialization', () => {
const camera = useCameraInfo(nodeRef(makeNode({ mode: 'orbit' })))
camera.setGizmosVisible(false)
camera.setTransformGizmoMode('camera-rotate')
camera.setLookThrough(true)
expect(instances).toHaveLength(0)
})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/composables/useCameraInfo.test.ts` around lines 86 - 234, Restore a test
in the useCameraInfo suite covering toolbar actions before initialization: call
setGizmosVisible, setTransformGizmoMode, and setLookThrough without invoking
initialize, then assert instances remains empty to preserve the
lazy-initialization guard.
Source: Learnings
Comment on lines
+386
to
+388
| controlsOf(viewport.cameraHandle).dispatchEvent({ type: 'objectChange' }) | ||
|
|
||
| expect(onHandleDrag).toHaveBeenCalledWith('mode.quat_w', expect.any(Number)) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert all four quaternion components, not just quat_w.
This mirrors the position/target tests (which already loop over x/y/z). As written, a regression that drops the mode.quat_x/y/z widget writes in handleCameraDrag would still pass. Loop over ['x','y','z','w'] and assert each mode.quat_${axis} emit.
As per path instructions, tests must cover the full behavioral contract for new logic.
💚 Proposed fix
- expect(onHandleDrag).toHaveBeenCalledWith('mode.quat_w', expect.any(Number))
+ for (const axis of ['x', 'y', 'z', 'w']) {
+ expect(onHandleDrag).toHaveBeenCalledWith(
+ `mode.quat_${axis}`,
+ expect.any(Number)
+ )
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| controlsOf(viewport.cameraHandle).dispatchEvent({ type: 'objectChange' }) | |
| expect(onHandleDrag).toHaveBeenCalledWith('mode.quat_w', expect.any(Number)) | |
| controlsOf(viewport.cameraHandle).dispatchEvent({ type: 'objectChange' }) | |
| for (const axis of ['x', 'y', 'z', 'w']) { | |
| expect(onHandleDrag).toHaveBeenCalledWith( | |
| `mode.quat_${axis}`, | |
| expect.any(Number) | |
| ) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/extensions/core/cameraInfo/CameraInfoViewport.test.ts` around lines 386 -
388, Update the quaternion assertion in the camera drag test around
controlsOf(viewport.cameraHandle) to iterate over x, y, z, and w, asserting
onHandleDrag is called with each corresponding mode.quat_${axis} key and a
numeric value. Preserve the existing event dispatch and ensure all four
quaternion component emits are covered.
Source: Path instructions
Comment on lines
+62
to
+68
| it('setMode switches translate <-> rotate', () => { | ||
| handle.setMode('rotate') | ||
| expect(handle.getMode()).toBe('rotate') | ||
|
|
||
| handle.setMode('translate') | ||
| expect(handle.getMode()).toBe('translate') | ||
| }) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Assert the underlying TransformControls mode, not just the wrapper's mirrored field.
handle.getMode() only echoes this.mode; it doesn't verify that controls.setMode() actually propagated to the real TransformControls instance. A regression that updates the local field but breaks the call to this.controls.setMode(mode) would pass this test.
🧪 Proposed fix
it('setMode switches translate <-> rotate', () => {
handle.setMode('rotate')
expect(handle.getMode()).toBe('rotate')
+ expect(controls().mode).toBe('rotate')
handle.setMode('translate')
expect(handle.getMode()).toBe('translate')
+ expect(controls().mode).toBe('translate')
})As per path instructions, avoid change-detector tests that assert implementation state instead of observable side effects.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('setMode switches translate <-> rotate', () => { | |
| handle.setMode('rotate') | |
| expect(handle.getMode()).toBe('rotate') | |
| handle.setMode('translate') | |
| expect(handle.getMode()).toBe('translate') | |
| }) | |
| it('setMode switches translate <-> rotate', () => { | |
| handle.setMode('rotate') | |
| expect(handle.getMode()).toBe('rotate') | |
| expect(controls().mode).toBe('rotate') | |
| handle.setMode('translate') | |
| expect(handle.getMode()).toBe('translate') | |
| expect(controls().mode).toBe('translate') | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/extensions/core/cameraInfo/handles/CameraHandle.test.ts` around lines 62
- 68, Update the setMode test around CameraHandle.setMode to assert the
underlying TransformControls instance’s mode after switching between rotate and
translate, rather than relying on handle.getMode(). Preserve coverage of both
mode transitions and verify the observable controls behavior.
Source: Path instructions
Comment on lines
+257
to
+258
| const vec = (v: THREE.Vector3Like): THREE.Vector3 => | ||
| new THREE.Vector3(v.x, v.y, v.z) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Use a function declaration for the pure vec helper.
vec is a pure, non-callback helper but is defined as a const arrow function. Based on learnings, this repo prefers function declarations for pure functions (all other module-level helpers here — buildArcCurve, makeHandle, makeGlow — already follow this convention).
♻️ Proposed fix
-const vec = (v: THREE.Vector3Like): THREE.Vector3 =>
- new THREE.Vector3(v.x, v.y, v.z)
+function vec(v: THREE.Vector3Like): THREE.Vector3 {
+ return new THREE.Vector3(v.x, v.y, v.z)
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const vec = (v: THREE.Vector3Like): THREE.Vector3 => | |
| new THREE.Vector3(v.x, v.y, v.z) | |
| function vec(v: THREE.Vector3Like): THREE.Vector3 { | |
| return new THREE.Vector3(v.x, v.y, v.z) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/extensions/core/cameraInfo/handles/OrbitHandles.ts` around lines 257 -
258, Change the module-level pure helper vec from a const arrow function to a
function declaration, preserving its Vector3Like input and THREE.Vector3 output
behavior. Leave buildArcCurve, makeHandle, and makeGlow unchanged.
Source: Learnings
Comment on lines
+185
to
+209
| it('look_at: moves the position toward the target, preserving direction', () => { | ||
| const state = stateWith({ | ||
| mode: 'look_at', | ||
| lookAt: { position: { x: 0, y: 0, z: 5 } }, | ||
| target: { x: 0, y: 0, z: 0 } | ||
| }) | ||
| const result = dollySubjectByWheel(state, -100) | ||
| const pos = result!.nextState.lookAt.position | ||
|
|
||
| expect(pos.z).toBeGreaterThan(0) | ||
| expect(pos.z).toBeLessThan(5) | ||
| expect(pos.x).toBeCloseTo(0) | ||
| expect(pos.y).toBeCloseTo(0) | ||
| }) | ||
|
|
||
| it('look_at: clamps distance to the shared maximum on scroll-out', () => { | ||
| const state = stateWith({ | ||
| mode: 'look_at', | ||
| lookAt: { position: { x: 0, y: 0, z: 5 } }, | ||
| target: { x: 0, y: 0, z: 0 } | ||
| }) | ||
| const pos = dollySubjectByWheel(state, 100000)!.nextState.lookAt.position | ||
|
|
||
| expect(Math.hypot(pos.x, pos.y, pos.z)).toBeCloseTo(100) | ||
| }) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Missing null-edge-case test for dollySubjectByWheel in look_at mode.
dollyLookAt returns null when camera position coincides with the target (distance < 1e-6), mirroring the already-tested null path in rotateSubjectByDrag - look_at (Line 71-79). That branch has no coverage here.
🧪 Proposed test addition
it('look_at: clamps distance to the shared maximum on scroll-out', () => {
...
})
+
+ it('look_at: returns null when the camera sits on the target', () => {
+ const state = stateWith({
+ mode: 'look_at',
+ lookAt: { position: { x: 1, y: 1, z: 1 } },
+ target: { x: 1, y: 1, z: 1 }
+ })
+
+ expect(dollySubjectByWheel(state, -100)).toBeNull()
+ })As per path instructions, "ensuring edge cases/error/null scenarios are covered where behavior exists."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('look_at: moves the position toward the target, preserving direction', () => { | |
| const state = stateWith({ | |
| mode: 'look_at', | |
| lookAt: { position: { x: 0, y: 0, z: 5 } }, | |
| target: { x: 0, y: 0, z: 0 } | |
| }) | |
| const result = dollySubjectByWheel(state, -100) | |
| const pos = result!.nextState.lookAt.position | |
| expect(pos.z).toBeGreaterThan(0) | |
| expect(pos.z).toBeLessThan(5) | |
| expect(pos.x).toBeCloseTo(0) | |
| expect(pos.y).toBeCloseTo(0) | |
| }) | |
| it('look_at: clamps distance to the shared maximum on scroll-out', () => { | |
| const state = stateWith({ | |
| mode: 'look_at', | |
| lookAt: { position: { x: 0, y: 0, z: 5 } }, | |
| target: { x: 0, y: 0, z: 0 } | |
| }) | |
| const pos = dollySubjectByWheel(state, 100000)!.nextState.lookAt.position | |
| expect(Math.hypot(pos.x, pos.y, pos.z)).toBeCloseTo(100) | |
| }) | |
| it('look_at: moves the position toward the target, preserving direction', () => { | |
| const state = stateWith({ | |
| mode: 'look_at', | |
| lookAt: { position: { x: 0, y: 0, z: 5 } }, | |
| target: { x: 0, y: 0, z: 0 } | |
| }) | |
| const result = dollySubjectByWheel(state, -100) | |
| const pos = result!.nextState.lookAt.position | |
| expect(pos.z).toBeGreaterThan(0) | |
| expect(pos.z).toBeLessThan(5) | |
| expect(pos.x).toBeCloseTo(0) | |
| expect(pos.y).toBeCloseTo(0) | |
| }) | |
| it('look_at: clamps distance to the shared maximum on scroll-out', () => { | |
| const state = stateWith({ | |
| mode: 'look_at', | |
| lookAt: { position: { x: 0, y: 0, z: 5 } }, | |
| target: { x: 0, y: 0, z: 0 } | |
| }) | |
| const pos = dollySubjectByWheel(state, 100000)!.nextState.lookAt.position | |
| expect(Math.hypot(pos.x, pos.y, pos.z)).toBeCloseTo(100) | |
| }) | |
| it('look_at: returns null when the camera sits on the target', () => { | |
| const state = stateWith({ | |
| mode: 'look_at', | |
| lookAt: { position: { x: 1, y: 1, z: 1 } }, | |
| target: { x: 1, y: 1, z: 1 } | |
| }) | |
| expect(dollySubjectByWheel(state, -100)).toBeNull() | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/extensions/core/cameraInfo/lookThroughDragMath.test.ts` around lines 185
- 209, Add a null-edge-case test for dollySubjectByWheel in look_at mode, using
stateWith to set lookAt.position equal to target and asserting the result is
null. Place it alongside the existing look_at dolly tests and mirror the
established null-path coverage pattern from rotateSubjectByDrag.
Source: Path instructions
jtydhr88
force-pushed
the
feat/create-camera-info-preview
branch
from
July 19, 2026 18:08
d167e6a to
5ab0757
Compare
christian-byrne
left a comment
christian-byrne
left a comment
Contributor
There was a problem hiding this comment.
Can you follow domain-driven organization of code modules and files rather than technical layers like sr/ccomponents and src/composables, etc.?
See
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
4 participants
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.
Summary
Vue viewport widget for the CreateCameraInfo node, bound to the CAMERA_INFO_STATE input. It renders the configured camera in a Load3D-style 3D scene so the camera can be posed directly instead of by typing numbers into the yaw/pitch/position/quaternion widgets; every drag writes back to those widgets, which stay the source of truth.
Shared viewer changes (load3d):
BE Comfy-Org/ComfyUI#14964
Screenshots (if applicable)
2026-07-15.23-31-59.mp4