mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-24 14:45:36 +00:00
e20bcd54858e7acb874db41377a07b64bd14b213
557 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Alexander Brown
|
e20bcd5485 | Merge branch 'main' into glary/remove-group-node-creation | ||
|
Terry Jia
|
52d77e6ee0 |
chore: upgrade sparkjs to 2.x and three to 0.184 (#12396)
## Summary - Spark 2.x requires SparkRenderer in scene tree; add it in SceneManager and protect it in clearModel so model reloads don't dispose the splat renderer. - three 0.184 OrbitControls listens on ownerDocument; drop redundant pointermove/up .stop in Load3D containers so the document listener can receive events. - Narrow Texture.image type for 0.184 strict typing. |
||
|
Terry Jia
|
cd2f4677c2 |
FE-719 feat(load3d): add FBX export support (#12323)
## Summary implement fbx export, using our own lib fbx-exporter-three ## Screenshots (if applicable) https://github.com/user-attachments/assets/80012338-d065-4a00-a9a0-0a2e73d67db4 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12323-FE-719-feat-load3d-add-FBX-export-support-3656d73d365081ef901ffe880ae9568a) by [Unito](https://www.unito.io) |
||
|
Glary-Bot
|
f460e105e9 |
feat: remove ability to create Group Nodes
Group Nodes are a legacy feature superseded by Subgraphs. This removes all UI entry points for creating new Group Nodes, while keeping the loading, ungrouping, and management code intact so existing workflows that contain Group Nodes continue to load and can still be unpacked or managed. Removed entry points: - 'Convert selected nodes to group node' command - Alt+G keybinding - 'Convert to Group Node (Deprecated)' canvas and node context menu items - 'Convert to Group Node' option in the Vue selection menu - Associated en locale strings - Browser tests that exercised the creation flow |
||
|
Terry Jia
|
5133ab6cf7 |
FE-416: route 3D node outputs to owner workflow on tab switch (#12295)
## Summary When a workflow queues a 3D job (Preview3D / Load3D / SaveGLB) and the user switches to another tab before the job completes, the WS `executed` payload was silently dropped: `getNodeByExecutionId(app.rootGraph, …)` resolves against the currently-visible workflow's graph, so the node isn't found, `onExecuted` never fires, and the workflow JSON never learns the path of the asset the backend just saved. Worse, `setNodeOutputsByExecutionId` for top-level execution ids returns the id verbatim without consulting rootGraph, so the old handler also wrote the payload into the *active* (wrong) workflow's `app.nodeOutputs`, polluting it and getting mis-attributed by `ChangeTracker` snapshot/restore on subsequent tab switches. This change introduces a single routing point — no new public API, no new in-memory cache layer — that re-uses three pieces of infrastructure that already exist independently: 1. `executionStore.jobIdToSessionWorkflowPath` already records which workflow queued each prompt_id. 2. Each `ComfyWorkflow.ChangeTracker` already snapshots `nodeOutputs` on `deactivate()` and replays it through the `app.nodeOutputs = …` setter on `restore()`, which fires `onNodeOutputsUpdated` for every extension. 3. Audio nodes already implement `onNodeOutputsUpdated` to rehydrate their widget from `nodeOutputs`. They have always been silently broken in this same scenario (no one noticed because missing audio is less visible than a missing 3D model); the fix here repairs audio for free as a side effect. The new behaviour: - `app.ts` looks up the node against the active rootGraph *first*. Only when found do we touch `nodeOutputStore` and call `node.onExecuted` (existing path, unchanged). Moving the `nodeOutputStore` write inside `if (node)` is the part that eliminates the cross-workflow pollution above. - When the node is not in the active rootGraph, the new `routeExecutedToInactiveOwner` helper finds the owner workflow via `jobIdToSessionWorkflowPath`, then writes the payload directly into `owner.changeTracker.nodeOutputs` — the same field that `restore()` already drains on tab switch back. - Preview3D / SaveGLB add `onNodeOutputsUpdated` (mirroring the pattern audio uses), normalise the path, write `node.properties['Last Time Model File']` (already workflow-JSON-serialised), and apply it to the viewer. The legacy `node.onExecuted` chain stays intact for live active-workflow executions — both paths are idempotent. ## Screenshots (if applicable) Before https://github.com/user-attachments/assets/3803af1f-2eb6-41af-87ed-ac885a2eaad6 After https://github.com/user-attachments/assets/72e1bed9-5f94-414d-ac31-fc925651d11b ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12295-FE-416-route-3D-node-outputs-to-owner-workflow-on-tab-switch-3616d73d3650817b908de48a32b1d6bd) by [Unito](https://www.unito.io) |
||
|
Terry Jia
|
861d737041 |
FE-702: rehydrate 3D viewer on subgraph re-entry via persistent ready hook (#12294)
## Summary When a Preview3D / Load3D / SaveGLB node lives inside a subgraph, the 3D viewer correctly displays the model the first time you enter the subgraph but is blank after exiting and re-entering — even though `node.properties['Last Time Model File']` is still populated and the underlying file is on disk. Fix: introduce a persistent companion to `waitForLoad3d` in `useLoad3d.ts`: - `onLoad3dReady(callback)` — registers a callback that fires on *every* (re-)initialization of the `Load3d` instance for a given node, not just the first one. Cleared automatically when the node is removed from the graph (chained into `node.onRemoved` alongside the existing `pendingCallbacks` cleanup). - `waitForLoad3d` keeps its original one-shot semantics so callbacks that install per-node side effects (e.g. wrapping `node.onExecuted`, setting `sceneWidget.serializeValue`) do not chain on remount. - When `onLoad3dReady` is registered after a `Load3d` instance already exists, the callback fires synchronously as well, so the same code path covers both initial setup and subsequent rehydrations. Preview3D / Load3D / SaveGLB move the "reapply state from `node.properties` / `model_file` widget to the Load3d viewer" block from `waitForLoad3d` to `onLoad3dReady`. First mount and every subsequent remount now run identical rehydration code, with `node.properties['Last Time Model File']` (already workflow-JSON-serialised) as the single source of truth. ## Screenshots (if applicable) before https://github.com/user-attachments/assets/e4b0fe6f-c898-4210-b545-7ad6883ed722 after https://github.com/user-attachments/assets/a4a28490-071d-4694-87a8-5eaa501ac168 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12294-FE-702-rehydrate-3D-viewer-on-subgraph-re-entry-via-persistent-ready-hook-3616d73d3650811e93e7dedb32762711) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
95e616b894 |
fix: clear media upload errors via widget change (#12212)
## Summary Clear missing media validation errors after paste/drop media uploads by emitting the existing widget-change event path. ## Changes - **What**: Emit `node.onWidgetChanged` after image/video upload completion updates the file combo widget. - **What**: Emit the same widget-change path after Load Audio upload completion. - **What**: Add unit coverage for upload completion emitting `onWidgetChanged` and for missing media clearing through that existing hook path. - **What**: Add E2E coverage for Load Image drag/drop and paste clearing validation rings, with red/green verified from a fresh `main` base. - **Dependencies**: None. ## Review Focus Please check that paste/drop upload paths now reuse the existing widget-change error-clearing path instead of expanding `widget.callback` patching. Also check the Load Image E2E helper path for synthetic paste/drop behavior. Supersedes #12207. Ref: FE-687 ## Screenshots Before https://github.com/user-attachments/assets/2cee52bc-b1c8-4dff-8a02-5b18a69ae639 After https://github.com/user-attachments/assets/e1ecd147-1d8a-470e-b77d-13345d473ef3 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12212-fix-clear-media-upload-errors-via-widget-change-35f6d73d365081bcb1a0dfc042d417eb) by [Unito](https://www.unito.io) |
||
|
Alexis Rolland
|
5738c7a539 |
Add SaveAudioAdvanced to whitelisted nodes (#12213)
## Summary Add `SaveAudioAdvanced` to whitelisted nodes in order to display the audio player. This PR goes with the core PR: https://github.com/Comfy-Org/ComfyUI/pull/13871 ## Changes - **What**: Display audio player on new node `SaveAudioAdvanced` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12213-Add-SaveAudioAdvanced-to-whitelisted-nodes-35f6d73d3650815783fac52d3d37e1e1) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
e765eb1bb2 |
fix: suppress missing media scan during uploads (#12111)
## Summary - Prevent missing media detection from scanning media loader nodes while a drag/drop, paste, or file-select upload is still in progress. - Align LoadAudio with the existing media upload lifecycle by setting `node.isUploading`, blocking concurrent uploads, and clearing the flag after upload completion. - Keep added-node model and missing-node scans on the original one-microtask path, while deferring added-node media scanning by one extra microtask so upload handlers can mark transient upload state before the scan reads widget values. ## Why Drag/drop and paste can create media loader nodes before the backing upload has settled. During that short window, the widget may contain a local filename that is not yet backend-resolvable, so missing media detection can surface a false missing asset. Refreshing works because the upload has completed by then. ## Follow-up - E2E coverage for this upload race will be handled in a follow-up PR together with E2E coverage for the annotated output-media path changes from #12069. ## Validation - `pnpm format` - `pnpm lint` - `pnpm typecheck` - `pnpm vitest run src/composables/graph/useErrorClearingHooks.test.ts src/platform/missingMedia/missingMediaScan.test.ts src/extensions/core/uploadAudio.test.ts src/composables/node/useNodeImageUpload.test.ts` - Re-ran `pnpm typecheck` after rebasing onto latest `main` - Pre-push `knip` hook passed Fixes FE-620 ## Screenshots Before https://github.com/user-attachments/assets/db7891de-a4b5-4cde-aa76-6340e6cdf7b2 After https://github.com/user-attachments/assets/9b99bb13-0d5b-4ff7-8f52-66eea6e417ec ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12111-fix-suppress-missing-media-scan-during-uploads-35b6d73d365081f3b54eed02874ccaa4) by [Unito](https://www.unito.io) |
||
|
Alexander Brown
|
25c2d828c0 |
test: enable vitest/consistent-each-for and migrate .each → .for (#12161)
*PR Created by the Glary-Bot Agent*
---
Enables the oxlint rule `vitest/consistent-each-for` (configured to
prefer `.for` for `test`, `it`, `describe`, and `suite`) and migrates
every `.each` parameterized test in the repo to `.for`. Using `.for`
avoids accidentally splatting tuple elements into separate callback
arguments and exposes `TestContext` as the second callback argument.
The first commit covers the 38 lint-detected files (88 callsites):
renames `.each` → `.for` and updates callback signatures to destructure
when the data is an array of tuples (objects/primitives already work
unchanged with `.for`).
The follow-up commit addresses code review feedback: oxlint's rule does
not recognize `test.each` on extended test bases
(`baseTest.extend(...)`) and skips files in `ignorePatterns`
(`src/extensions/core/*`). These were converted manually so the policy
is uniform across the codebase.
## Verification
- `node_modules/.bin/oxlint src` — 0 errors, 0 `consistent-each-for`
violations
- `pnpm typecheck` — passes
- `pnpm test:unit` — all modified test files pass; pre-existing
environmental flakes (`GraphView.test.ts`, `ColorWidget.test.ts`, etc.,
unchanged here and flaky on `main` in this sandbox) are unrelated
- `pnpm lint` / `pnpm knip` — clean
- Manual verification: 362 tests across 6 representative converted
suites re-run in an interactive shell — all passing
Manual UI verification (Playwright/screenshots) is not applicable:
changes are test-file-only refactors with no production runtime or UI
behavior change.
## Notes on `.for` semantics
- Array-of-tuples (`[[a, b], ...]`) passes the tuple as a single arg, so
callbacks were changed from `(a, b) => …` to `([a, b]) => …`.
- Array-of-objects (`[{a}, …]`) already used destructuring — unchanged.
- Array-of-primitives (`['a', …]`) — callback signature unchanged.
- A handful of complex cases use a small `type Case = [...]` alias plus
`it.for<Case>([...])` to preserve tuple inference where TS narrowed
unions otherwise broke parameter types.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-12161-test-enable-vitest-consistent-each-for-and-migrate-each-for-35e6d73d3650810c9417e07bdd9f27a2)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com>
|
||
|
Terry Jia
|
653ef1a4f0 |
Handle Load3D "none" model selection in frontend (#11178)
## Summary Load3D now supports panoramic images and HDRI loading, it can serve as a viewer for those without requiring a 3D model. Previously, the node required a model file to execute. Rather than making the input optional (which would break existing workflows that rely on it being required), a "none" option is added to the combo list, allowing users to run Load3D with no model loaded. BE change is https://github.com/Comfy-Org/ComfyUI/pull/13379 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11178-Handle-Load3D-none-model-selection-in-frontend-3416d73d365081e589b3d89bc67f75e7) by [Unito](https://www.unito.io) |
||
|
pythongosssss
|
6a8c453659 |
test: add missing emitModelReady to mock object (#12056)
## Summary
Unblock CI due to missing function in mock
```
⎯⎯⎯⎯ Unhandled Rejection ⎯⎯⎯⎯⎯
TypeError: this.load3d.emitModelReady is not a function
❯ src/extensions/core/load3d/Load3DConfiguration.ts:313:19
311| }
312|
313| this.load3d.emitModelReady()
| ^
314| }
315| }
This error originated in "src/extensions/core/load3d/Load3DConfiguration.test.ts" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
The latest test that might've caused the error is "prefers persisted Scene/Camera/Light Config over settings". It might mean one of the following:
- The error was thrown, while Vitest was running this test.
- If the error occurred after the test had been completed, this was the last documented test before it was thrown.
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Test Files 773 passed (773)
Tests 10417 passed | 8 skipped (10425)
Errors 2 errors
Start at 12:06:06
Duration 446.79s (transform 32.89s, setup 113.26s, import 589.18s, tests 109.19s, environment 228.45s)
```
## Changes
- **What**:
- add `emitModelReady` fn to mock
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-12056-test-add-missing-emitModelReady-to-mock-object-3596d73d36508130a087f5c7713e932f)
by [Unito](https://www.unito.io)
|
||
|
Terry Jia
|
ea277dec4d |
FE-446: test(load3d): cover Load3D/Preview3D extensions and config persistence (#11969)
## Summary Add unit tests for src/extensions/core/load3d.ts (Load3D and Preview3D nodeCreated/onExecuted, beforeRegisterNodeDef, getNodeMenuItems, camera matrix generation guard) and extend Load3DConfiguration tests for the Scene/Camera/Light config persistence + settingStore fallback paths. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11969-FE-446-test-load3d-cover-Load3D-Preview3D-extensions-and-config-persistence-3576d73d365081c2a847e0dc9621a75d) by [Unito](https://www.unito.io) |
||
|
Terry Jia
|
9c62bbc74a |
FE-412: fix(load3d): persist SaveGLB last model so it survives tab switch (#11965)
## Summary Mirror the Preview3D pattern: store the executed file path and folder on node.properties and reload them in nodeCreated so the mesh re-appears when the node is recreated (e.g. after switching workflow tabs). ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11965-FE-412-fix-load3d-persist-SaveGLB-last-model-so-it-survives-tab-switch-3576d73d365081a98bcadbd9a81539d0) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
0446ca7a18 |
fix: route default topbar feedback button to Typeform (#11863)
*PR Created by the Glary-Bot Agent* --- ## Summary PR #10890 routed the legacy action bar feedback button and the Help Center feedback item to the nightly Typeform survey, but the **default topbar feedback button** in `WorkflowTabs.vue` still called `buildFeedbackUrl()` and opened Zendesk. Since `Comfy.UI.TabBarLayout` defaults to `Default` (not `Legacy`), most Cloud/Nightly users were clicking the WorkflowTabs button and never reaching the Typeform survey — explaining the lack of survey responses. ## Changes - Added a shared `buildFeedbackTypeformUrl(source)` helper in `platform/support/config.ts` that tags the survey URL with: - `distribution`: `ccloud` / `oss-nightly` / `oss` (preserves the build-tagging the old `buildFeedbackUrl()` sent to Zendesk so responses stay segmented) - `source`: `topbar` / `action-bar` / `help-center` (identifies which UI entry point launched the survey) Tags are passed via the URL fragment (Typeform's hidden-field convention), so they reach the survey but are never sent to the server in the request line. - `WorkflowTabs.vue`: replaced `buildFeedbackUrl()` with `buildFeedbackTypeformUrl('topbar')`. - `cloudFeedbackTopbarButton.ts` and `HelpCenterMenuContent.vue`: use the shared builder with their respective source labels instead of inline URL literals. - Removed the now-unused `buildFeedbackUrl()` and `ZENDESK_FEEDBACK_FORM_ID` (knip-clean). `buildSupportUrl()` is preserved — `Comfy.ContactSupport` (the Help Center "Help" item) still routes to Zendesk as before. - Added unit tests for the builder, the WorkflowTabs feedback button, the legacy action bar button, and the Help Center feedback item (covering both the Cloud/Nightly Typeform path and the OSS `Comfy.ContactSupport` fallback). ## Verification - `pnpm format`, `pnpm lint`, `pnpm typecheck`, `pnpm knip`: clean (one pre-existing unrelated lint warning in `useWorkspaceBilling.test.ts`) - `pnpm test:unit` (impacted scope): 506/506 passing, including 13 new tests ## Review Focus - Cloud/Nightly gating in `WorkflowTabs.vue` (`v-if="isCloud || isNightly"`) is unchanged and matches PR #10890's gating philosophy. - The Help Center "Help" item and `Comfy.ContactSupport` command intentionally still route to Zendesk — feedback ≠ support. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11863-fix-route-default-topbar-feedback-button-to-Typeform-3556d73d3650815fb446dac33095d4be) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> |
||
|
Terry Jia
|
6ef051f200 |
FE-537: fix(load3d): preserve camera view, fit transform, and first-frame paint after refresh (#11944)
## Summary - Defer thumbnail capture until camera state is restored via new modelReady event so captureThumbnail no longer races with the saved view, fixing the "snap back to default on hover" regression. - Repaint the live scene at the end of captureThumbnail so the canvas is not left with the offscreen mask/normal pass when the render loop is gated. - Persist post-fitToViewer model.scale + model.position into the existing modelConfig.gizmo slot so a refresh reapplies them via the existing applyGizmoConfigToLoad3d path; rotation stays owned by upDirection. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11944-FE-537-fix-load3d-preserve-camera-view-fit-transform-and-first-frame-paint-after-re-3576d73d365081429653ea4740612617) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
666684e6e6 |
fix: stop PreviewAny widgets from triggering re-execution (#12010)
## Summary Preview as Text (`PreviewAny`) nodes were re-executing on every prompt submission because the rendered preview text was being echoed back to the backend as input values, mutating the cache signature. ## Changes - **What**: Set `widget.options.serialize = false` on the three widgets the `Comfy.PreviewAny` extension adds (`preview_markdown`, `preview_text`, `previewMode`) so they are excluded from the API prompt sent to the backend. ## Root cause The extension was setting `widget.serialize = false`, which only controls **workflow JSON** persistence (checked in `LGraphNode.serialize`). The **API prompt** serializer in `executionUtil.graphToPrompt` checks `widget.options.serialize` instead — a distinct property documented in litegraph's `WIDGET_SERIALIZATION` convention. After `onExecuted` writes the rendered text into the widget value, the next `graphToPrompt` call serialized that text into `inputs.preview_text` / `inputs.preview_markdown`. The backend cache signature in `comfy_execution/caching.py` hashes all keys in `node["inputs"]`, so the changing text invalidated the cache and forced a redundant execution every time. ## Review Focus Two commits, red-green TDD: 1. `test:` failing unit + e2e tests asserting the desired behavior. 2. `fix:` adds `options.serialize = false` to make them pass. Tests verify the widgets are excluded from the API prompt; e2e additionally simulates a prior execution populating widget values to mirror the real bug condition. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12010-fix-stop-PreviewAny-widgets-from-triggering-re-execution-3586d73d3650810585cdd077f3ac64f5) by [Unito](https://www.unito.io) |
||
|
Kelly Yang
|
a877ccde94 |
Test/edit attention unit tests (#11301)
## Summary
A follow-up PR of
https://github.com/Comfy-Org/ComfyUI_frontend/issues/11107.
## Changes
Add unit test to `editAttention.ts`
- [x] `Extract pure functions to module level`: **Moved**
`incrementWeight`, `findNearestEnclosure`, and `addWeightToParentheses`
out of the `init()` closure and **promoted** them to module-level
functions with `export` to allow for independent testing.
- [x] `Add unit tests for incrementWeight`: **Added** 6 tests covering
edge cases such as normal increment/decrement, NaN input, negative
weights, and floating-point precision.
- [x] `Add unit tests for findNearestEnclosure`: **Added** 7 tests
covering edge cases including simple brackets, no brackets, cursor
outside, nested brackets (inner/outer), empty strings, and missing
closing brackets.
- [x] `Add unit tests for addWeightToParentheses`: **Added** 6 tests
covering scenarios like adding a default 1.0 weight, retaining existing
weights, no changes when brackets are absent, scientific notation
weights, negative weights, and multi-word tokens.
- [x] `Mock app module`: **Used** `vi.mock('@/scripts/app')` to
intercept side effects from `app.registerExtension`, **preventing** the
triggering of ComfyUI extension registration logic during module import.
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Adjusts token selection/weight-detection logic used during
Ctrl/Cmd+Arrow editing, which could subtly change how prompts are
rewritten in edge cases (nested parens, scientific notation, time-like
text). Adds tests that should reduce regression risk but behavior
changes still warrant verification in the UI.
>
> **Overview**
> Adds a new `vitest` unit test suite for `editAttention` by mocking
`app.registerExtension` side effects and validating `incrementWeight`,
`findNearestEnclosure`, and `addWeightToParentheses` across common and
edge cases.
>
> Refactors those helpers to exported module-level functions and
tightens parsing/selection behavior: `findNearestEnclosure` now handles
the cursor being on an opening `(`, `addWeightToParentheses` improves
trailing weight detection (supports scientific notation/negatives and
avoids misclassifying time-like `12:30`), and the weight-rewrite regex
now matches exponent forms.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Christian Byrne
|
551cf21fb1 |
fix(load3d): reapply up-direction after fitToViewer() transform reset (#11826)
## Summary
`fitToViewer()` in `SceneModelManager` resets the model rotation to
`(0,0,0)` as part of its normalize-and-scale flow, but does not reapply
`currentUpDirection` afterward. This causes a state/view mismatch when
the user has previously selected a non-default up-axis (e.g. `+x`,
`-z`): the model snaps back to its raw orientation while the Up
Direction control still shows the previously selected value.
## Changes
- In `fitToViewer()`, clear `originalRotation` (to avoid compounding
with the stale pre-fit base) then reapply `currentUpDirection` if it is
not `'original'`
- Add 5 unit tests covering: no-op when no model, reapplication of
direction, no rotation compounding on repeated calls, zero rotation for
`'original'` direction, and stale `originalRotation` guard
## Testing
### Automated
- `src/extensions/core/load3d/SceneModelManager.test.ts` — 5 new tests
in `describe('fitToViewer')`
- All 48 unit tests pass
### E2E Verification Steps
1. Open the Load3D viewer with any 3D model
2. Change **Up Direction** to any non-default value (e.g. `+x` or `-z`)
— observe model rotates correctly
3. Click **Fit to Viewer** — model should remain in the chosen
up-direction orientation, not snap back to raw orientation
4. Click **Fit to Viewer** again — rotation should remain stable (no
compounding)
5. Change Up Direction back to `original` then click **Fit to Viewer** —
model should return to neutral orientation `(0,0,0)`
## Review Focus
The key invariant: `fitToViewer()` resets `rotation.set(0,0,0)`
explicitly, so we must clear `originalRotation = null` before calling
`setUpDirection`. Otherwise `setUpDirection` restores the stale pre-fit
rotation as a base and then adds the direction offset on top,
compounding incorrectly.
Fixes #11347
<!-- Pipeline-Ticket: pick-issue-3414 -->
┆Issue is synchronized with this [Notion
page](https://app.notion.com/p/PR-11826-fix-load3d-reapply-up-direction-after-fitToViewer-transform-reset-3546d73d36508166b9bcecc9949c4952)
by [Unito](https://www.unito.io)
|
||
|
Christian Byrne
|
2c8ecd82ec |
fix(load3d): snapshot original materials before reapplying materialMode (#11825)
## Summary
Fixes a bug where models reloaded in wireframe/normal/depth modes would
not restore to their original materials correctly, because the material
snapshot was being taken *after* the mode was applied.
## Changes
- Move `setupModelMaterials(model)` to immediately after
`scene.add(model)` and before `setMaterialMode()` / `setUpDirection()`
in `SceneModelManager.setupModel()`
- Save `materialMode` into `pendingMaterialMode` before calling
`setupModelMaterials()`, since the latter internally calls
`setMaterialMode('original')` which resets `this.materialMode` —
preserving the value ensures the subsequent reapplication guard works
correctly
- Update stale test assertion that reflected the old (incorrect) call
order
- Add regression test: verifies that `originalMaterials` captures the
pre-mutation material and that restoring to `'original'` after a
non-original load gives back the true original mesh material
## Testing
### Automated
- `src/extensions/core/load3d/SceneModelManager.test.ts` — 44 tests, all
pass
- Full load3d test suite — 401 tests, all pass
### E2E Verification Steps
1. Open ComfyUI with a Load3D node
2. Load any GLB/OBJ model
3. Switch Material Mode to **Wireframe** (or Normal/Depth)
4. Load a different model (or reload the same one)
5. Switch Material Mode back to **Original**
6. Verify the model renders with its original diffuse/PBR materials (not
wireframe)
## Review Focus
The key invariant: `setupModelMaterials()` must snapshot mesh materials
in their *unmodified* state. It must run before any `setMaterialMode()`
call that mutates them. The `pendingMaterialMode` variable is needed
because `setupModelMaterials()` internally calls
`setMaterialMode('original')`, which updates `this.materialMode`, making
the subsequent guard `if (this.materialMode !== 'original')` silently
skip reapplication without it.
Fixes #11344
<!-- Pipeline-Ticket: pick-issue-3417 -->
┆Issue is synchronized with this [Notion
page](https://app.notion.com/p/PR-11825-fix-load3d-snapshot-original-materials-before-reapplying-materialMode-3546d73d3650818b9c35fa60c15f17a3)
by [Unito](https://www.unito.io)
|
||
|
Kelly Yang
|
7b59c561ff |
fix(load3d): update renderer pixel ratio on canvas zoom to fix LOD resolution (#11734)
## Summary
Preview 3D and Animation nodes were stuck at the LOD from initial page
load because CSS `scale3d` transforms don't affect
`clientWidth`/`clientHeight` — `handleResize()` always received
layout-space dimensions regardless of zoom level. This fix passes
`ds.scale` as the renderer pixel ratio so the 3D scene renders at the
correct visual resolution when the graph is zoomed in or out.
## Changes
- **What**: In `Load3d.handleResize()`, call
`renderer.setPixelRatio(ds.scale)` before `setSize` so pixel density
scales with canvas zoom. A `getZoomScale` callback is threaded through
`Load3DOptions` → `Load3d` constructor → `handleResize`. In `useLoad3d`,
a watcher on `canvasStore.appScalePercentage` triggers `handleResize`
whenever the zoom level changes.
- **What**: Fix `SceneManager.captureScene()` to save and restore the
renderer's logical size and pixel ratio around capture, so exact-pixel
output is unaffected by the current zoom state.
## Review Focus
- `handleResize` now calls `setPixelRatio` before `setSize`. Three.js
renders at `logicalWidth × pixelRatio` physical pixels while CSS
displays it at `logicalWidth` CSS pixels — this is the standard pattern
for HiDPI but here used to match the visual zoom level.
- `captureScene` must reset `pixelRatio` to 1 so `setSize(w, h)`
produces exactly `w×h` pixel output. It saves and restores both logical
size and pixel ratio via `renderer.getSize()` /
`renderer.getPixelRatio()`.
- The zoom watcher is guarded with `getActivePinia()` to avoid errors in
unit tests and non-Pinia contexts.
## Test
before
https://github.com/user-attachments/assets/9778ad54-7cb2-4fdc-b200-65a683ee8e4d
after
https://github.com/user-attachments/assets/acfaaf7a-43c7-495f-b352-5dd2cdaa94db
## Analysis Report
https://linear.app/comfyorg/issue/FE-401/bug-preview-3d-and-animation-nodes-lod-stuck-at-initial-page-load
## More
- Add `debounce` and pixel ratio limit
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Medium risk because it changes core `Load3d.handleResize()` rendering
behavior (pixel ratio/LOD) and adds a debounced zoom-driven resize
watcher, which could affect performance or visual output across all
Load3D nodes. Capture logic is also refactored to manipulate renderer
size/pixel ratio and camera params, so regressions would show up in
thumbnails/exports.
>
> **Overview**
> Fixes Load3D LOD/render sharpness when the graph canvas is zoomed by
threading a new `getZoomScale` option from `useLoad3d` into `Load3d` and
using it to call `renderer.setPixelRatio()` (clamped) during
`handleResize()`.
>
> Adds a debounced watcher on `canvasStore.appScalePercentage` to
trigger `handleResize()` on zoom changes, and updates
`SceneManager.captureScene()` to temporarily force pixel ratio 1 and
restore renderer size/pixel ratio and camera settings after capture.
Coverage is expanded with new Playwright smoke coverage plus unit tests
for zoom propagation, debouncing, pixel ratio behavior, and capture
state restoration.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Terry Jia
|
b5b502755f |
fix(load3d): parse [output]/[input]/[temp] annotation when loading (#11805)
## Summary The Vue node model picker mixes output assets into the dropdown with a trailing ' [output]' suffix on the value. Forwarding that string to loadModel as a literal filename under the configured input folder caused a 404 and the model never rendered. Strip the trailing folder annotation per call and use the matching folder so picking an output asset loads correctly while plain values keep the configured folder. Output assets stored under a subfolder (e.g. 3d/) were exposed in the Vue node dropdown as just '<filename> [output]', so selection produced an /api/view URL with empty subfolder and a 404. Read the subfolder from the asset's OutputAssetMetadata and prefix it onto the annotated path so the downstream load handler can split it back out and target the correct folder. ## Screenshots (if applicable) https://github.com/user-attachments/assets/463d1071-efdc-46a4-b101-8e1c012d19c7 ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11805-fix-load3d-parse-output-input-temp-annotation-when-loading-3536d73d365081a8ac9cf75d14ae29e6) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
341fef46a9 |
refactor: replace unsafe as Error assertions with type guards (#11845)
## Summary Replaces all 7 production `as Error` type assertions with proper `instanceof Error` narrowing or a new `toError()` helper, and adds an ESLint rule to prevent new ones. First slice of #11429 (the `as Error` category — 9 total occurrences, 7 production + 2 in a test file left untouched). ## Changes - **What**: - New `src/utils/errorUtil.ts` exporting `toError(value: unknown): Error` and `getErrorMessage(value: unknown): string | undefined`. `toError` returns the value unchanged if already an `Error`, otherwise wraps it (handles strings, `undefined`, JSON-serializable objects, and circular refs via `String()` fallback). - Refactored 7 production call sites: - `src/services/gateway/registrySearchGateway.ts` — `toError(error)` for `lastError` assignment in fallback loop - `src/platform/cloud/onboarding/auth.ts` (×2) — `toError(error)` for `captureApiError` Sentry calls - `src/renderer/extensions/vueNodes/widgets/composables/audio/useAudioRecorder.ts` — `toError(err)` before forwarding to `options.onError` - `src/extensions/core/load3d/LoaderManager.ts` — replaced `error as Error & { response?: ... }` cast inside `isNotFoundError` with `'response' in error` + nested narrowing - `apps/desktop-ui/src/stores/maintenanceTaskStore.ts` — inline `error instanceof Error ? error.message : String(error)` - `apps/desktop-ui/src/components/maintenance/TaskListPanel.vue` — inline `error instanceof Error ? error.message : undefined` - New ESLint rule (`no-restricted-syntax` block named `comfy/no-unsafe-error-assertion`) banning `TSAsExpression TSTypeReference[typeName.name='Error']` in `src/**` and `apps/*/src/**`, with test files (`*.test.ts`, `*.spec.ts`) excluded. - 12 unit tests for the new helpers in `src/utils/errorUtil.test.ts`. - **Breaking**: none - **Dependencies**: none ## Review Focus - The lint rule is scoped to non-test source files. Test files retain freedom to use `as Error` for fixture construction; only 2 occurrences exist (in `teamWorkspaceStore.test.ts` and `errorDialog.spec.ts`) and they're intentional. - `toError` is duplicated as inline `instanceof` narrowing in `apps/desktop-ui/` rather than imported, since the desktop-ui workspace doesn't share `@/utils/` with the main app and adding a path mapping for one helper felt heavier than two inline guards. - Remaining `as`-on-DOM categories (HTMLElement ×133, HTMLInputElement ×55, HTMLCanvasElement ×36, KeyboardEvent ×7, Element ×3, MouseEvent ×2, Event ×2) are intentionally left for follow-up PRs to keep this one reviewable. Refs #11429 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11845-refactor-replace-unsafe-as-Error-assertions-with-type-guards-3546d73d36508137a015c4f9e8708f23) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
df2ae6f2d0 |
fix(load3d): dispose THREE.Points GPU resources in clearModel() (#11836)
Fixes #11345 ## Summary `clearModel()` in `SceneModelManager` only traversed and disposed `THREE.Mesh` instances, leaving `THREE.Points` objects (created by `handlePLYModeSwitch()` for point-cloud mode) leaking GPU geometry and material memory on repeated point-cloud loads/clears. ## Changes - `SceneModelManager.ts`: extend the dispose traversal in `clearModel()` to also handle `THREE.Points`, mirroring the pattern already used by `removeAllMainModelsFromScene()`. - `SceneModelManager.test.ts`: add regression test verifying `geometry.dispose()` and `material.dispose()` are called for `THREE.Points` children on `clearModel()`. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11836-fix-load3d-dispose-THREE-Points-GPU-resources-in-clearModel-3546d73d365081718338e824bc3e737d) by [Unito](https://www.unito.io) |
||
|
Kelly Yang
|
e7e1ae25a6 |
fix(load3d): suppress error toast on 404 when loading output model file (#11807)
## Summary
- Adds `silentOnNotFound` option to `LoadModelOptions` interface,
threaded through `Load3d.loadModel` → `LoaderManager.loadModel`
- 404 errors (detected via message text or `response.status`) are
silently swallowed when `silentOnNotFound: true`; all other errors still
surface a toast
- Sets `silentOnNotFound: true` for output-folder loads in `load3d.ts`
and `saveMesh.ts` — covers shared workflows opened on a machine that
never ran them
## Test plan
- [x] `LoaderManager.test.ts` — 40 unit tests covering 404 suppression,
non-404 still toasts, stale load handling
- [x] `Load3DConfiguration.test.ts` — 4 unit tests verifying
`silentOnNotFound` propagates correctly through `configureForSaveMesh`
and `configure`
- [x] `load3d.spec.ts` — 2 E2E tests: 404 → no toast, 500 → toast
appears
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Changes error-handling behavior in the 3D model loading pipeline and
extends method signatures/options; risk is mainly missed call sites or
incorrectly classifying non-404 errors as 404 and hiding real failures.
>
> **Overview**
> Prevents noisy user-facing toasts when an *output* 3D model referenced
by `Preview3D`/`SaveGLB` is missing locally by adding a
`silentOnNotFound` flag and suppressing the "Error loading model" toast
specifically for HTTP 404 failures.
>
> Threads the new `LoadModelOptions` through `Load3d.loadModel` →
`LoaderManager.loadModel` and updates `Load3DConfiguration`/callers to
opt in for output-folder loads, with new unit + Playwright coverage (404
stays silent, non-404 still toasts).
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Terry Jia
|
c88275b2a4 |
test(load3d): add unit tests for SceneManager (#11762)
## Summary Add 35 unit tests for `SceneManager`, the largest remaining gap in the load3d core (452 LOC). Targets only the logic-bearing methods (background mode dispatch, render-mode switching, aspect-ratio scaling, capture pipeline, dispose). Renderer-passthrough internals are intentionally left to E2E. Follow-up to Tier 1 (#11733), Tier 2, Tier 3a, and Tier 3b. ## Changes - **What**: 35 new tests covering construction (main scene + background scene + grid + tiled mesh + default color mode), `toggleGrid`, `setBackgroundColor` (color update, scene-bg cleanup, panorama-demote, prior-texture dispose), `setBackgroundImage` (empty-path fallback, loading-start emit, temp/output subfolder rewrite, /api prefix, tiled-mesh material swap, panorama scene-background promotion, prior-texture dispose, error-path fallback), `removeBackgroundImage`, `setBackgroundRenderMode` (no-op same-mode, color-only emit, image panorama-promote, image tiled-demote), `updateBackgroundSize` (no-texture/no-mesh/no-map guards, wide vs. tall image scaling), `handleResize` (image-bg active vs. color-only), `getCurrentBackgroundInfo`, `captureScene` (returns 3 data URLs + restores renderer state, restores grid visibility, propagates errors), and `dispose` (resource cleanup + scene-background null). ## Review Focus - **Coverage**: `SceneManager.ts` 89.5% lines / 74.2% branches / 89.5% funcs. Uncovered lines are concentrated in `renderBackground` and the deep mesh-traversal loop inside `captureScene` — exactly the renderer-passthrough territory deferred per the Tier 3c plan. - **`THREE.Material.needsUpdate` is a write-only setter** in THREE.js — reading returns `undefined`. The "demote panorama → tiled" test asserts `mat.map === texture` instead of `mat.needsUpdate === true`, with a comment explaining why. - **happy-dom canvas `clientWidth`/`clientHeight` default to 0** — `makeRenderer()` overrides them via `Object.defineProperty` so production code reading `renderer.domElement.clientWidth` gets the test value. - **`THREE.TextureLoader` is mocked via `vi.mock('three', ...)` with `importOriginal`**, matching the pattern in `RecordingManager.test.ts` and `HDRIManager.test.ts`. `mockTextureLoad` is hoisted so each test can resolve/reject the load callback independently. - **`vi.spyOn(manager, 'setBackgroundColor')` in three places** to assert internal delegation (empty-path fallback, error fallback, `removeBackgroundImage`). Defensible because the delegation IS the documented contract for these methods. ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11762-test-load3d-add-unit-tests-for-SceneManager-3516d73d365081628ff4c146defebac1) by [Unito](https://www.unito.io) |
||
|
Terry Jia
|
d2e88011aa |
test(load3d): add unit tests for EventManager, ViewHelperManager, and load3dLazy (#11760)
## Summary Add unit tests for the three remaining small/wrapper modules in the load3d domain (`EventManager` pub/sub, `ViewHelperManager` ViewHelper wrapper, and `load3dLazy` lazy-extension loader). Follow-up to Tier 1 (#11733) and Tier 2. ## Changes - **What**: 28 new unit tests across 3 files covering EventManager add/remove/emit semantics, ViewHelperManager container DOM creation + pointer-event interception + animation-finished camera-state emission + perspective/orthographic zoom snapshotting + dispose ordering, and load3dLazy extension registration + 3D-node-type recognition + Load3D-specific `mesh_upload` injection + `beforeRegisterNodeDef` hook replay for newly registered extensions. ## Review Focus - **Coverage** (lines/branches/funcs): EventManager 100% / 100% / 100%, ViewHelperManager 100% / 83.3% / 72.7%, load3dLazy 95.8% / 80% / 100%. Aggregate: **98.6% / 85.3% / 85.7%**. - **`vi.mock` factory side-effects only fire once per test file** — `load3dLazy.test.ts` originally tried to count dynamic imports of `./load3d` and `./saveMesh` via spies inside the mock factory, but factories aren't re-invoked across `vi.resetModules()`. Switched to verifying observable side effects (`enabledExtensions` getter call counts, `beforeRegisterNodeDef` replay invocations). - **Snapshot-vs-diff `enabledExtensions` queue** in `load3dLazy.test.ts`: production code does `before = new Set(enabledExtensions); await imports; diff = enabledExtensions.filter(!before.has)`. To exercise the replay branch, the mock returns `[]` first (for `before`) and `[newExtension]` second (for the post-import snapshot) via `mockReturnValueOnce` queueing. - **`MockViewHelper` is defined inside the `vi.mock()` factory** rather than `vi.hoisted()`, matching the `GizmoManager.test.ts:15-41` convention (hoisted handles only, classes inside the factory). - **PointerEvent propagation tests** require the production code's `event.stopPropagation()` to actually keep events from bubbling to the parent in happy-dom; the parent listener gets attached and asserted-not-called. ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11760-test-load3d-add-unit-tests-for-EventManager-ViewHelperManager-and-load3dLazy-3516d73d3650814bb2dac3360ab0d2a1) by [Unito](https://www.unito.io) |
||
|
Terry Jia
|
180a0001e8 |
test(load3d): add unit tests for LightingManager, ControlsManager, exportMenuHelper, and ModelExporter (#11758)
## Summary Add unit tests for the four Tier 2 untested logic modules in the load3d domain (`LightingManager`, `ControlsManager`, `exportMenuHelper`, and `ModelExporter`). Follow-up to the Tier 1 PR (#11733). ## Changes - **What**: 43 new unit tests across 4 files covering light setup/intensity scaling/HDRI mode/disposal, OrbitControls construction (including DOM-parent fallback) and camera-state event emission, the export submenu builder (item structure, submenu opening, format dispatch, success/error toasts), and the static `ModelExporter` (URL parsing, direct-URL fast paths for matching extensions, GLB/OBJ/STL serialization branches, error toast paths). ## Review Focus - **Coverage** (lines/branches/funcs): LightingManager 100% / 50% / 90.9%, ControlsManager 100% / 100% / 87.5%, exportMenuHelper 100% / 100% / 100%, ModelExporter 98.4% / 95.7% / 100%. Aggregate: **99.2% / 93.5% / 95.1%**. - **`vi.mock(import('@/lib/litegraph/src/litegraph'), ...)` in `exportMenuHelper.test.ts`** uses the dynamic-import form so `importOriginal()` is auto-typed. Required because `apiSchema.ts` transitively imports `LinkMarkerShape` from the same module — replacing the whole module breaks the build. The mock replaces only `LiteGraph.ContextMenu` in-place on the real singleton. - **`MockContextMenu` must be a class**, not an arrow function — production code does `new LiteGraph.ContextMenu(...)`. Initial arrow-function mock failed with "is not a constructor". - **Fake-timer rejection pattern in `ModelExporter.test.ts`**: rejection assertions are attached *before* `vi.runAllTimersAsync()` (`const assertion = expect(p).rejects.toThrow(...); await drain; await assertion`) to avoid unhandled-rejection warnings. - **Surprising `detectFormatFromURL` behavior**: `detectFormatFromURL('?filename=cube')` returns `'cube'`, not `null`, because `'cube'.split('.').pop()` returns the whole basename when no dot is present. Test documents this rather than asserting an incorrect expectation. - **Two unreachable lines left uncovered**: `LightingManager:65` (`?? 1` fallback in the `setLightIntensity` multiplier lookup — every light is registered in the map at construction, so the fallback is dead) and `ModelExporter:21` (a `try/catch` around `URLSearchParams` whose constructor cannot throw on the inputs the production code passes). ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11758-test-load3d-add-unit-tests-for-LightingManager-ControlsManager-exportMenuHelper-and-3516d73d365081cb96fff33672503822) by [Unito](https://www.unito.io) |
||
|
Terry Jia
|
57d708767a |
test(load3d): add unit tests for AnimationManager, CameraManager, RecordingManager, and load3dService (#11733)
## Summary
Add unit tests for the four largest untested logic-heavy modules in the
load3d domain (`AnimationManager`, `CameraManager`, `RecordingManager`,
and the `load3dService` façade).
## Changes
- **What**: 78 new unit tests across 4 files covering animation
lifecycle (mixer setup, clip switching, play/pause/seek, dispose),
camera state (perspective↔orthographic toggling, FOV gating, state
round-trip, controls rebinding, resize math, model fitting), recording
lifecycle (MediaRecorder wiring, indicator visibility, chunk handling,
export/clear paths, dispose ordering), and the service singleton
(sync/async map access, viewer cache, `handleViewerClose` apply-changes
flow, `handleViewportRefresh` camera-toggle dance).
## Review Focus
- **Coverage**: AnimationManager 100%, CameraManager 88.8%,
RecordingManager 89.1%, load3dService 54.5% lines / 88.9% functions. The
service gap is concentrated in one method — `copyLoad3dState` (lines
217-333) — which is entangled with 3-4 subsystems and is intentionally
deferred to a follow-up PR.
- **happy-dom shims** in `RecordingManager.test.ts`: `MediaRecorder`,
`HTMLCanvasElement.prototype.captureStream`, and `getContext('2d')` are
all stubbed because happy-dom doesn't provide them.
`THREE.TextureLoader` is also mocked because the constructor eagerly
loads an SVG indicator.
- **Singleton state reset** in `load3dService.test.ts`: the service
holds a module-level `viewerInstances` Map that can't be reached from
outside. Tests track every node they create in a `Set` and drain via
`removeViewer` in `beforeEach`. Cleaner than `vi.resetModules()` (which
would re-import the service and break the singleton identity).
- **One subtle THREE behavior**: `AnimationManager.setAnimationTime`
clamps to `duration`, but `AnimationMixer.setTime(duration)` with
`LoopRepeat` wraps `action.time` back to 0. The clamping is therefore
only observable through the emitted progress event, not via
`action.time` directly — the test asserts via the event.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11733-test-load3d-add-unit-tests-for-AnimationManager-CameraManager-RecordingManager-and--3516d73d3650812485a0c91065e161f0)
by [Unito](https://www.unito.io)
|
||
|
Terry Jia
|
f001bc9af3 |
feat: derive Preview3D camera pose from EXTRINSICS/INTRINSICS matrices (#11626)
> Prerequisite work for improved PLY / 3D Gaussian Splatting support — splat workflows (and PLY pipelines that go through SHARP / COLMAP) emit camera pose as extrinsics + intrinsics matrices, and the viewer needs a way to consume them before that end-to-end story can ship. ## Summary Adds the ability to drive the Preview3D camera from a pair of OpenCV-convention extrinsics + intrinsics matrices (as produced by SHARP / COLMAP / other SfM pipelines), so backend nodes that emit such matrices can position the viewer's camera deterministically. Fifth in the series splitting up https://github.com/Comfy-Org/ComfyUI_frontend/pull/11495. Fully self-contained — adds new API surface only, no existing call paths are modified. ## Changes - **What**: - `cameraFromMatrices.ts` (new): pure function `computeCameraFromMatrices(extrinsics, intrinsics)` returning `{ position, target, fovYDegrees }`. No THREE.js dependency. Shape-validates the inputs (4×4 and 3×3) with a clear error. - `Load3d.setCameraFromMatrices(extrinsics, intrinsics)`: wires the utility into Load3d via `setCameraState` + `setFOV`. Preserves the caller's existing `zoom` and `cameraType`. - `load3d.ts` (extension): Preview3D's `onExecuted` extracts `extrinsics`/`intrinsics` from `result[3]`/`result[4]` (when present) and calls `setCameraFromMatrices`. The output tuple type is widened to include the two optional `Matrix` fields. ## Review Focus - **Convention conversion**: OpenCV is Y-down, Z-forward; three.js is Y-up, Z-back. The function flips Y and Z on both `position` and `target` (equivalent to a 180° X-axis rotation of the whole world). The same rotation is applied elsewhere to splats at load time, so a future splat + camera-pose pair lines up. - **FOV math**: vertical FOV (radians) = `2 * atan(cy / fy)`. We expose it in degrees, matching `cameraManager.setFOV`'s contract. - **Camera-state preservation**: `setCameraFromMatrices` reads the current state to keep `zoom` and `cameraType` intact — only `position`, `target`, and `fov` come from the matrices. - **Backwards compatibility**: backend nodes that don't return matrices simply skip the new branch (the `if (extrinsics && intrinsics)` guard). Existing Preview3D workflows are unaffected. - 7 unit tests for the matrix math (identity, rotation, translation, FOV derivation, shape errors, etc.); 1 unit test for the Load3d wiring (verifies the destructured `position`/`target`/`fovYDegrees` reach `setCameraState` + `setFOV` with `zoom`/`cameraType` preserved). ## Coverage | File | Stmts | Branch | Funcs | Lines | |---|---|---|---|---| | `cameraFromMatrices.ts` (new) | **100%** | **100%** | **100%** | **100%** | | `Load3d.ts` (modified) | ~7.6% | 0% | ~14.6% | ~7.7% | | `load3d.ts` (extension, modified) | 0% | 0% | 0% | 0% | The `Load3d.ts` numbers are the pre-existing baseline — `Load3d.test.ts` covers façade methods via prototype injection rather than instantiating the class (the constructor needs `THREE.WebGLRenderer`, which happy-dom can't provide). The new `setCameraFromMatrices` method body is exercised by a new unit test that asserts `setCameraState` receives the destructured matrix output and `setFOV` receives the computed `fovYDegrees`, with the caller's `zoom`/`cameraType` preserved. `load3d.ts` (the extension registration file, 0% on `main` and after this PR) has no unit-test scaffolding in the project — its `onExecuted` handler runs only after a workflow execution and is exercised end-to-end via browser tests. The new `if (extrinsics && intrinsics) load3d.setCameraFromMatrices(...)` branch sits in that path. Net: the matrix math itself, which previously didn't exist, is now 100% covered. The wiring layer relies on the same e2e safety net the surrounding code has always used. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11626-feat-derive-Preview3D-camera-pose-from-EXTRINSICS-INTRINSICS-matrices-34d6d73d3650817297bdf25a83a4f7a2) by [Unito](https://www.unito.io) |
||
|
Terry Jia
|
42ff7b6c62 |
refactor(load3d): drive viewer behavior from ModelAdapter capabilities (#11660)
> Final architectural step in the PLY / 3D Gaussian Splatting series. Previous PR introduced `ModelAdapter` with a dormant `capabilities` field; this PR makes those capabilities load-bearing and replaces the remaining `instanceof SplatMesh` / `instanceof BufferGeometry` switches with adapter-driven dispatch. Together with previous one it removes the last of the format-specific branching from `SceneModelManager` / `Load3d`. Seventh in the series splitting up the https://github.com/Comfy-Org/ComfyUI_frontend/pull/11495. ## Summary Drive viewer behavior (fit-to-viewer, default camera pose, world bounds, GPU dispose, material rebuild) from `ModelAdapterCapabilities` + 3 new optional adapter methods, instead of `SceneModelManager` reflecting on model shape. `Load3d` is rewritten to take its 13 managers as injected `Load3dDeps`; a new `createLoad3d` factory assembles them and threads a single `AdapterRef` between `LoaderManager` (writer) and `SceneModelManager` + `Load3d` (readers). Splat orientation + decoder-race + sizing bugs are fixed as a side effect — splats now render upright, fill the grid, and don't lock the OrbitControls target on first frame. ## Changes - **`ModelAdapter.ts`**: add `AdapterRef = { current: ModelAdapter | null }` shared handle and 3 optional adapter methods — `computeBounds(model)`, `disposeModel(model)`, `defaultCameraPose()`. - **`SplatModelAdapter.ts`**: implement all 3 optional methods; `await splatMesh.initialized` so first-frame bounds are populated (fixes a decoder race that collapsed the OrbitControls target onto the camera); `quaternion.set(1, 0, 0, 0)` to convert sparkjs's OpenCV (Y-down, Z-forward) to three.js (Y-up, Z-back); flip `fitToViewer` back to `true` and bump `fitTargetSize` to `20` so splats fill the 20-unit grid instead of shrinking to 1/4 of it. - **`PointCloudModelAdapter.ts`**: extract `buildPointCloudForMaterialMode` so `SceneModelManager` rebuilds via the same code path the initial load uses; `setPath` so PLYs that reference relative assets resolve correctly. - **`LoaderManager.ts`**: accept optional `AdapterRef`; write through it instead of the internal `_currentAdapter` field. `clearModel()` now runs while the old adapter is still current so its `disposeModel()` can release renderer-owned resources. - **`SceneModelManager.ts`**: accept 4 capability lambdas (`getCurrentCapabilities` / `getBoundsFromAdapter` / `disposeModelViaAdapter` / `getDefaultCameraPose`) with `DEFAULT_MODEL_CAPABILITIES` / null fallbacks. `setupModel`, `fitToViewer`, and material-mode rebuild are now capability-driven; `containsSplatMesh` (30 lines of `instanceof` traversal) and `handlePLYModeSwitch` (90 lines of duplicated PLY rebuild) are gone. - **`Load3d.ts`**: ctor switches from manager-creation to deps-injected (`Load3dDeps`); add `getCurrentModelCapabilities()` reader; gate `setGizmoEnabled` / `setGizmoMode` / `resetGizmoTransform` / `applyGizmoTransform` on `capabilities.gizmoTransform`; `isSplatModel` / `isPlyModel` now read `adapterRef.current?.kind` directly. - **`createLoad3d.ts`** (new): single factory that builds the renderer (`createRenderer`), assembles all 13 managers in dependency order, and threads one shared `AdapterRef` through `LoaderManager` and `SceneModelManager`'s 4 capability lambdas. - **`useLoad3d.ts` / `useLoad3dViewer.ts`**: switch from `new Load3d(container, options)` to `createLoad3d(container, options)`. No other call-site changes. ## Review Focus - **Capability dispatch parity**: walk each former hardcoded branch in `SceneModelManager` and confirm it now falls out of the right capability: - `containsSplatMesh()` → `!capabilities.fitToViewer` + `getDefaultCameraPose()` - `handlePLYModeSwitch()` → `capabilities.requiresMaterialRebuild` + `buildPointCloudForMaterialMode()` - `Box3.setFromObject(model)` for sizing → `getBoundsFromAdapter(model) ?? Box3.setFromObject(model)` - Mesh/Points geometry+material disposal in `clearModel` → still happens, plus `disposeModelViaAdapter(obj)` for adapter-owned resources (sparkjs SplatMesh internal GPU state) - **`AdapterRef` lifecycle**: one ref is created in `createLoad3d`, passed to `LoaderManager` (writer) and `SceneModelManager` (read via 4 closures). `LoaderManager.loadModel` clears via the *old* adapter first (so `disposeModel` runs), then null-resets the ref before picking the new one. Test `keeps the old adapter current while clearModel runs` pins this ordering. - **Splat fixes are user-visible, not pure refactor**: - Orientation: `quaternion.set(1, 0, 0, 0)` matches the sparkjs README convention. Without it splats render upside-down and mirrored on Z. Same rotation is applied to the camera-from-matrices output in PR-E so a future splat + camera-pose pair lines up. - Decoder race: `await splatMesh.initialized` ensures `getBoundingBox` returns a non-zero box on the first call. Without it `setupModel`'s bounds → camera pipeline placed the OrbitControls target on the camera origin, locking the view. - Sizing: `fitTargetSize: 20` (vs. the mesh default of 5) means splat geometry spans the full 20-unit grid footprint instead of ~1/4 of it. Mesh assets are unaffected. - **Gizmo gating**: `setGizmoEnabled(true)` early-returns when `capabilities.gizmoTransform` is false. Internal `setGizmoEnabled(false)` still runs (so we can always disable). `setGizmoMode` / `resetGizmoTransform` / `applyGizmoTransform` no-op when the capability is off. - **`createLoad3d` is the single ctor entry**: `new Load3d(...)` is no longer callable from app code (ctor signature changed to `(container, deps, options)`). All call sites use `createLoad3d`. Test scaffolding still uses `Object.create(Load3d.prototype)` + property injection where it needs to bypass renderer creation. - **Backwards compatibility**: `LoaderManager`'s `adapterRef` and `SceneModelManager`'s 4 capability lambdas all have defaults (`createAdapterRef()` and `() => DEFAULT_MODEL_CAPABILITIES` etc.), so the existing test suites that construct these classes with the old signatures still compile and pass without modification beyond what's in this PR. ## Coverage | File | Stmts | Branch | Funcs | Lines | |---|---|---|---|---| | `ModelAdapter.ts` (modified) | **100%** | **100%** | **100%** | **100%** | | `LoaderManager.ts` (modified) | **100%** | 91.7% | 86.7% | **100%** | | `MeshModelAdapter.ts` (unchanged) | **100%** | **100%** | **100%** | **100%** | | `PointCloudModelAdapter.ts` (modified) | **97.9%** | 69.2% | 71.4% | **97.9%** | | `SplatModelAdapter.ts` (modified) | **100%** | **100%** | **100%** | **100%** | | `SceneModelManager.ts` (modified) | 75.4% | 67.2% | 72.2% | 75.4% | | `Load3d.ts` (modified) | 29.5% | 30.6% | 26.7% | 30.1% | | `createLoad3d.ts` (new) | 83.8% | **100%** | 58.3% | 83.8% | | `useLoad3d.ts` (modified) | 78.2% | 65.1% | 71.4% | 82.2% | | `useLoad3dViewer.ts` (modified) | 75.2% | 52.1% | 65.9% | 79.4% | `SplatModelAdapter.ts` jumps to 100% via 6 new tests covering the orientation set, the `await initialized` decoder wait, `computeBounds` (world-space transform + null fallback), `disposeModel` (per-SplatMesh dispose + no-op on non-splat trees), and `defaultCameraPose`. `createLoad3d.ts` hits 100% branch via a new test file with 12 cases — `WebGLRenderer` config, `Load3DOptions` forwarding, `AdapterRef` identity between `LoaderManager` and `SceneModelManager`, and the 4 capability lambdas in both adapter-null and adapter-published states (each delegates correctly to the adapter's optional methods or falls back to defaults). The remaining func% reflects the inline `gizmoTransformChange` callback — not a deliberate skip, just out of scope for the dispatch-wiring tests. `SceneModelManager.ts` and `Load3d.ts` numbers are the pre-existing baseline — the existing `*.test.ts` files cover façade methods via prototype injection rather than instantiating the classes (`Load3d` constructor needs `THREE.WebGLRenderer`, which happy-dom can't provide; `SceneModelManager` covers the new capability paths via its existing `createManager(overrides)` helper). All new branches (capability gating, capability-driven `setupModel` / `fitToViewer` / rebuild, adapter-driven `isSplatModel` / `isPlyModel`) have dedicated tests. Net diff: **+846 / −370** across 16 files (10 production, 6 test). ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11660-refactor-load3d-drive-viewer-behavior-from-ModelAdapter-capabilities-34f6d73d36508130b0ece884add182b9) by [Unito](https://www.unito.io) |
||
|
Terry Jia
|
6bf75b4cf0 |
refactor(load3d): introduce ModelAdapter abstraction for the loader switch (#11627)
> Prerequisite work for improved PLY / 3D Gaussian Splatting support — the per-format loader logic needs to live behind a stable seam before splat-specific fixes (orientation, async-decoder waits, GPU dispose, custom bounds) and capability-driven UX gating can be added without touching `LoaderManager`'s switch every time. ## Summary Pure refactor. Extracts the per-extension switch inside `LoaderManager` into three `ModelAdapter` implementations and wires the manager to dispatch through them. **No behavior change** — same loader code paths, same outputs, same fallbacks. Sixth in the series splitting up the https://github.com/Comfy-Org/ComfyUI_frontend/pull/11495. ## Changes - **What**: - `ModelAdapter.ts` (new): defines the `ModelAdapter` interface (`kind`, `extensions`, `capabilities`, `load`), a `ModelLoadContext` that exposes only the `SceneModelManager` surface adapters need (`setOriginalModel`, `registerOriginalMaterial`, `standardMaterial`, `materialMode`), and a shared `fetchModelData(path, filename)` helper. - `MeshModelAdapter.ts` (new): owns `stl`, `fbx`, `obj`, `gltf`, `glb`. Each branch is a 1:1 lift of the corresponding `case` from `LoaderManager.loadModelInternal` on `main`. - `PointCloudModelAdapter.ts` (new): owns `ply`. Includes the existing `FastPLYLoader` / `PLYLoader` fallback and the `pointCloud` vs mesh branching logic. - `SplatModelAdapter.ts` (new): owns `spz`, `splat`, `ksplat`. Wraps the `SplatMesh` in a `Group` exactly like the previous `loadSplat` did. - `LoaderManager.ts`: now owns just an adapter array (default = the three above) and a small dispatch path. `pickAdapter` matches by extension and routes PLY → splat when the `Comfy.Load3D.PLYEngine` setting is `sparkjs` (preserving the previous routing). `getCurrentAdapter()` is the new public reader used by `Load3d`. - `Load3d.isSplatModel` / `isPlyModel` now query `loaderManager.getCurrentAdapter()?.kind` instead of doing tree-introspection (`containsSplatMesh`) or `instanceof THREE.BufferGeometry` checks. Same return values, decoupled from the model shape. - `LoaderManagerInterface` no longer exposes the per-format loader fields (`gltfLoader`, `objLoader`, etc.); those are now adapter-internal. - `SceneModelManager` is **unchanged** in this PR. Its existing `containsSplatMesh()` traversal and PLY material-mode rebuild stay put; a follow-up PR refactors them once capability gating is in place. ## Review Focus - **Loader equivalence**: the body of every `case` in `main`'s `LoaderManager.loadModelInternal` is now in the corresponding adapter's `load()` method. Easiest way to verify: diff `main`'s `LoaderManager.loadModelInternal` against the four `load()` bodies and confirm each branch's behavior (file fetch + parse + material wiring + group wrapping) is byte-identical. - **Dispatch parity**: `pickAdapter` produces the same routing as `main` — extension match first, then the PLYEngine === 'sparkjs' override hoisted up from inside the old `loadPLY`. - **Capability fields are dormant**: the `ModelAdapterCapabilities` record (`fitToViewer`, `materialModes`, `fitTargetSize`, …) is declared on every adapter but **not consumed anywhere in this PR**. SceneModelManager / Load3d / Load3DControls still read no capability data. The follow-up PR turns these on. - **`setOriginalModel` / `registerOriginalMaterial`**: adapters now go through the `ModelLoadContext` getter rather than reaching into `modelManager` directly. The context's `standardMaterial` and `materialMode` are exposed via getters so a late-bound `materialMode` is read at the actual call site, not snapshotted at context creation. ## Coverage | File | Stmts | Branch | Funcs | Lines | |---|---|---|---|---| | `ModelAdapter.ts` (new) | **100%** | **100%** | **100%** | **100%** | | `MeshModelAdapter.ts` (new) | **100%** | **100%** | **100%** | **100%** | | `PointCloudModelAdapter.ts` (new) | 97.22% | 61.11% | 75% | 97.22% | | `SplatModelAdapter.ts` (new) | **100%** | **100%** | **100%** | **100%** | | `LoaderManager.ts` (modified) | **100%** | 91.17% | 86.66% | **100%** | | `Load3d.ts` (modified) | 6.63% | 0% | 13.68% | 6.7% | All four new files are at or near 100% via dedicated unit tests for each adapter (load happy path, error propagation, extension declarations, capability shape). `LoaderManager.test.ts` exercises the dispatch logic — extension matching, the `ply → splat` sparkjs override, the stale-load discard, the load-context proxying — across 34 cases. The two changed `Load3d.ts` methods (`isSplatModel`, `isPlyModel`) get dedicated tests verifying they read the current adapter's `kind` and fall back to `false` when none is loaded. `Load3d.ts`'s overall 6.7% number is the pre-existing baseline — the existing `Load3d.test.ts` covers façade methods via prototype injection rather than instantiating the class (the constructor needs `THREE.WebGLRenderer`, which happy-dom can't provide). PR-F's surface in `Load3d.ts` is two method bodies, both covered by the new adapter-driven kind queries test. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11627-refactor-load3d-introduce-ModelAdapter-abstraction-for-the-loader-switch-34d6d73d3650811b8a1ccc55b45100f2) by [Unito](https://www.unito.io) |
||
|
Terry Jia
|
9f4c54eb24 |
refactor: extract Load3d right-click guard to load3dContextMenuGuard (#11625)
## Summary Pull the right-click vs right-drag detection out of `Load3d` into a sibling helper. Mechanical refactor — no behavior change. Third of four small PRs splitting up the [`remove-ply-3dgs-nodes-squashed` mega-commit.](https://github.com/Comfy-Org/ComfyUI_frontend/pull/11495.) ## Changes - **What**: New `load3dContextMenuGuard.ts` exports `attachContextMenuGuard(target, onMenu, { isDisabled, dragThreshold })`. It installs `mousedown` / `mousemove` / `contextmenu` listeners against a single `AbortController` and returns one dispose function. - `Load3d` now calls `attachContextMenuGuard(this.renderer.domElement, (event) => this.onContextMenuCallback?.(event), { isDisabled: () => this.isViewerMode })` and stores the returned disposer in a single field. Drops four private fields (`rightMouseStart`, `rightMouseMoved`, `dragThreshold`, `contextMenuAbortController`) plus the now-redundant `showNodeContextMenu` private method. - The 5px drag threshold and `isViewerMode` gating are preserved. ## Review Focus - The three event handlers (`mousedown`, `mousemove`, `contextmenu`) inside the new helper match the old inline implementations one-for-one — same `e.button === 2` / `e.buttons === 2` checks, same call to `exceedsClickThreshold`, same `preventDefault` + `stopPropagation` ordering. - `isDisabled: () => this.isViewerMode` replaces the inline `if (this.isViewerMode) return` early-out — same gate, just lifted to a callback. - A single `AbortController.abort()` (in the returned disposer) replaces the old four-field teardown in `Load3d.remove()`. - 9 unit tests cover the helper: click vs drag distinction at the threshold, drag-then-click reset, `isDisabled` short-circuit, and the disposer detaching all three listeners. ## Coverage | File | Stmts | Branch | Funcs | Lines | |---|---|---|---|---| | `load3dContextMenuGuard.ts` (new) | **100%** | **93.33%** | **100%** | **100%** | | `Load3d.ts` (modified) | 7.12% | 0% | 13.97% | 7.18% | The single uncovered branch on `load3dContextMenuGuard.ts` (line 22) is the default-parameter fallback for `dragThreshold` when the caller omits it — `Load3d` always passes `{ isDisabled, dragThreshold: 5 }` through `attachContextMenuGuard`'s second-arg destructure, so the default never fires under the production call path. Adding a test that omits `dragThreshold` would push it to 100%; left as-is to avoid a change-detector test for a default value. The `Load3d.ts` numbers are the pre-existing baseline on `main` — `Load3d.test.ts` covers façade methods via prototype injection rather than instantiating the class (the constructor needs `THREE.WebGLRenderer`, which happy-dom can't provide). The `initContextMenu` rewrite and the `remove()` teardown change both sit in those same uninstantiated paths and rely on browser e2e for end-to-end coverage, the same as before. Net: the click-vs-drag logic that previously had no unit test is now ≥93% covered through the extracted helper. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11625-refactor-extract-Load3d-right-click-guard-to-load3dContextMenuGuard-34d6d73d36508162aecef46553a3f50d) by [Unito](https://www.unito.io) |
||
|
Terry Jia
|
b0fa179b87 |
refactor: extract Load3d render loop to load3dRenderLoop (#11623)
## Summary
Pull the `requestAnimationFrame` loop and its activity-gated tick body
out of `Load3d` into a small `startRenderLoop({ tick, isActive })`
helper. Pure mechanical refactor — no behavior change. First of four
small PRs splitting up the
https://github.com/Comfy-Org/ComfyUI_frontend/pull/11495.
## Changes
- **What**: New `load3dRenderLoop.ts` exports `startRenderLoop` (returns
a `{ stop }` handle). `Load3d.startAnimation()` now constructs a loop
through it; `Load3d.remove()` calls `stop()` instead of
`cancelAnimationFrame`. Field `animationFrameId: number | null` becomes
`renderLoop: RenderLoopHandle | null`.
## Review Focus
- The tick body inside `startAnimation()` is byte-identical to the
previous inline body — only the rAF scheduling has moved.
- `isActive()` is now invoked through a `() => this.isActive()` closure
instead of a direct call inside the inline `animate` function, so the
activity check still fires once per frame and reads the same fields.
- The new helper has 4 unit tests covering: ticks while active, skip
while inactive, stop halts ticks, stop is idempotent.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11623-refactor-extract-Load3d-render-loop-to-load3dRenderLoop-34d6d73d3650815c9c4ec7713e912e37)
by [Unito](https://www.unito.io)
|
||
|
Terry Jia
|
ba6dd2a09c |
refactor(load3d): extract viewport math to load3dViewport (#11624)
## Summary Pull two pure helpers out of `Load3d` into a sibling module. Mechanical refactor — no behavior change. Second of four small PRs splitting up the https://github.com/Comfy-Org/ComfyUI_frontend/pull/11495. ## Changes - **What**: New `load3dViewport.ts` exports two pure functions: - `computeLetterboxedViewport({ width, height }, targetAspectRatio)` returns `{ offsetX, offsetY, width, height }` — the aspect-ratio fitting math that was duplicated inline in three places (`renderMainScene`, `setBackgroundImage`, `handleResize`). - `isLoad3dActive(flags)` consumes the activity-flag struct used by the rAF tick gate; `Load3d.isActive()` now delegates to it. ## Review Focus - The three call sites in `Load3d.ts` produce byte-identical viewport rectangles for any `(containerWidth, containerHeight, targetAspect)` triple — same branch on aspect-ratio comparison, same offset derivation. Simplest way to verify: diff the math. - `isLoad3dActive` ORs the same six flags as before in the same order. Old implementation read `this.STATUS_MOUSE_ON_NODE` etc. directly; new one reads them through a flags object built at the call site. - 13 unit tests cover the helpers: the "wider" / "taller" / "matching" aspect cases for letterboxing, and each individual flag flipping `isLoad3dActive` on by itself. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11624-refactor-load3d-extract-viewport-math-to-load3dViewport-34d6d73d365081ab9af5fc445bf5bd5a) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
pythongosssss
|
0052cdadd4 |
test: e2e coverage for node templates (#11564)
## Summary Add E2E tests for node templates ## Changes - **What**: - add tests for save, insert, delete, import export - vue and litegraph - add testid to dialog - update `clickLitegraphMenuItem` to enable clicking children with the same name as parent ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11564-test-e2e-coverage-for-node-templates-34b6d73d365081a39ce5c713f05a2a92) by [Unito](https://www.unito.io) |
||
|
Dante
|
9599a4e00a |
fix cloud frontend runtime guard regressions (#11180)
## Summary - harden cloud frontend runtime paths that were throwing on `cloud.comfy.org` - guard widget value propagation when the source widget is missing - treat nullish executed outputs as empty output during flatten/parsing - ignore stale autogrow disconnect callbacks after an autogrow group is removed ## Root cause This PR bundles three small runtime guard fixes from `cloud-frontend-staging` issues that reproduce on `https://cloud.comfy.org/`: - `CLOUD-FRONTEND-STAGING-429`: widget propagation assumed `this.widgets[0]` always existed and crashed during group-node/widget lifecycle transitions - `CLOUD-FRONTEND-STAGING-3QA` and sibling `3QB`: executed-event parsing assumed `detail.output` was always an object and crashed on nullish output payloads - `CLOUD-FRONTEND-STAGING-42B`: `autogrowInputDisconnected()` could run from a stale `requestAnimationFrame()` callback after its autogrow group had already been removed ## User impact - prevents unhandled frontend exceptions on `cloud.comfy.org` - keeps node output rendering and linear-mode flattening resilient to sparse executed payloads - avoids autogrow disconnect crashes during graph/widget churn ## Changes - extracted shared widget propagation logic into `widgetValuePropagation.ts` - added source-widget guards in custom widget / primitive widget propagation paths - added null guards in result parsing and linear-mode output flattening - added an autogrow-group existence guard in `dynamicWidgets.ts` - added focused regression tests for all three bug shapes ## Red / Green Verification ### Red I ran the new targeted regression suite in a temporary pre-fix worktree with the runtime guards reverted while keeping the new tests. Failing tests in that state: - `src/extensions/core/widgetValuePropagation.test.ts` - `returns early when the source widget is missing` - `src/stores/resultItemParsing.test.ts` - `returns empty array for nullish node output` - `ignores nullish node outputs` - `src/renderer/extensions/linearMode/flattenNodeOutput.test.ts` - `returns empty array for nullish node output` Representative pre-fix errors: - `TypeError: Cannot read properties of undefined (reading 'value')` - `TypeError: Cannot convert undefined or null to object` ### Green On the draft PR branch, the targeted regression suite passes: - `pnpm exec vitest run src/core/graph/widgets/dynamicWidgets.test.ts src/stores/resultItemParsing.test.ts src/renderer/extensions/linearMode/flattenNodeOutput.test.ts src/extensions/core/widgetValuePropagation.test.ts src/extensions/core/customWidgets.test.ts` Result: - `5` test files passed - `49` tests passed ## Validation - `pnpm exec vitest run src/core/graph/widgets/dynamicWidgets.test.ts src/stores/resultItemParsing.test.ts src/renderer/extensions/linearMode/flattenNodeOutput.test.ts src/extensions/core/widgetValuePropagation.test.ts src/extensions/core/customWidgets.test.ts` - `pnpm exec eslint --no-ignore src/core/graph/widgets/dynamicWidgets.ts src/core/graph/widgets/dynamicWidgets.test.ts src/stores/resultItemParsing.ts src/stores/resultItemParsing.test.ts src/renderer/extensions/linearMode/flattenNodeOutput.ts src/renderer/extensions/linearMode/flattenNodeOutput.test.ts src/extensions/core/customWidgets.ts src/extensions/core/widgetInputs.ts src/extensions/core/widgetValuePropagation.ts src/extensions/core/widgetValuePropagation.test.ts` - `pnpm exec oxfmt --check src/core/graph/widgets/dynamicWidgets.ts src/core/graph/widgets/dynamicWidgets.test.ts src/stores/resultItemParsing.ts src/stores/resultItemParsing.test.ts src/renderer/extensions/linearMode/flattenNodeOutput.ts src/renderer/extensions/linearMode/flattenNodeOutput.test.ts src/extensions/core/customWidgets.ts src/extensions/core/widgetInputs.ts src/extensions/core/widgetValuePropagation.ts src/extensions/core/widgetValuePropagation.test.ts` ## Notes - I explicitly skipped the `getCanvas: canvas is null` issue because it is already covered by open PRs `#11173` / `#11174`. - `pnpm typecheck` was not included in validation because the temporary PR worktree used for publication hits local path-resolution issues through the shared dependency install, which is unrelated to the changes in this PR. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11180-codex-fix-cloud-frontend-runtime-guard-regressions-3416d73d365081e0af6ec612c9d0d8aa) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
pythongosssss
|
ac728b92ae |
fix: fix webcam node not showing preview in nodes 2.0 (#11549)
## Summary Adds test coverage for webcam node & fixes issue found in testing where the captured image does not show in nodes 2.0 ## Changes - **What**: - call `setNodePreviewsByNodeId` alongside `node.imgs = [img]` - add tests for general coverage ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11549-fix-fix-webcam-node-not-showing-preview-in-nodes-2-0-34a6d73d3650810c89eee9c25cd07700) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
Christian Byrne
|
0638e8e993 |
test: add unit tests for SceneModelManager (#11392)
## Summary Add 44 unit tests for `SceneModelManager` in the 3D viewer (`src/extensions/core/load3d/`). ## Changes - **What**: New test file `SceneModelManager.test.ts` covering constructor, dispose, createSTLMaterial, addModelToScene, setupModel, setOriginalModel, clearModel, reset, setMaterialMode (all 5 modes), setupModelMaterials, setUpDirection (all 7 directions), hasSkeleton, setShowSkeleton, containsSplatMesh, and PLY mode switching (point cloud, wireframe, vertex colors, cleanup). ## Review Focus - Test coverage of PLY mode switching edge cases (vertex colors, old model cleanup) - Mock strategy for WebGLRenderer (happy-dom cannot instantiate it) - SplatMesh mock leverages the existing global mock in `vitest.setup.ts` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11392-test-add-unit-tests-for-SceneModelManager-3476d73d3650819097f3f6d73d8fbe02) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
b157182a20 |
refactor: inline node footer layout to fix selection bounding box (#10741)
## Summary Refactor node footer from absolute overlay to inline flow layout, fixing the selection bounding box not encompassing footer buttons and collapsed node dimensions. ## Background The node footer (Enter Subgraph, Advanced, Error buttons) was rendered as an absolute overlay (`absolute top-full`) outside the node body. This caused: 1. **Selection bounding box** did not include footer height — the dashed multi-select border cut through footer buttons 2. **Footer offset compensation** required 3 hardcoded computed classes (`footerStateOutlineBottomClass`, `footerRootBorderBottomClass`, `footerResizeHandleBottomClass`) with magic pixel values (31px, 35px, etc.) that had to stay in sync with CSS ## Solution: Inline Footer with `isolate -z-1` The footer is moved into normal document flow (no longer `absolute top-full`). The key challenge was keeping the footer visually behind the body's rounded bottom edge (the "tuck under" effect) without adding `z-index` to the body — because adding `z-index` to the body creates a stacking context that traps slot connection dots, making them appear behind overlay borders. The solution uses CSS `isolation: isolate` combined with `-z-1` on the footer wrapper: - **`isolate`** creates an independent stacking context for the footer, so internal z-index (Error button `z-10` above Enter button) does not leak to the parent - **`-z-1`** places the entire footer behind the body (`z-index: auto`), achieving the visual overlap without touching the body's stacking behavior - **Slot dots remain free** — the body has no explicit z-index, so slots participate in the root stacking context and are never trapped behind overlay borders This eliminates all 3 footer offset computed classes and their hardcoded pixel values. ## Selection Box: `min-height` on root + unified size path Moving `min-h-(--node-height)` from the body (`node-inner-wrapper`) to the root element makes the footer height naturally included in `node.size` via ResizeObserver → layoutStore → litegraph sync. This means `boundingRect` is automatically correct for expanded nodes — no callbacks or overrides needed. For collapsed nodes, a pre-existing issue (since v1.40) caused `_collapsed_width` to fall back to `NODE_COLLAPSED_WIDTH = 80px` because Vue nodes lack a canvas context for text measurement. The fix lets collapsed dimensions flow through the **same** `batchUpdateNodeBounds` path as expanded nodes — no parallel data structure, no separate accessor, no cache: 1. ResizeObserver writes the collapsed DOM dimensions to `layoutStore.size` via `batchUpdateNodeBounds` 2. `useLayoutSync` syncs `layoutStore.size` → `liteNode.size` as it does for any other size change 3. The expanded size survives the collapse→expand round trip via CSS custom properties — the `isCollapsed` watcher in `LGraphNode.vue` swaps `--node-width` to `--node-width-x` on collapse and restores it on expand 4. `measure()` reads `this.size` directly for Vue collapsed nodes via a one-line gate: `if (!this.flags?.collapsed || LiteGraph.vueNodesMode)`. Legacy behavior is unchanged. ## Changes - **NodeFooter.vue**: `absolute top-full` overlay → inline flow with `isolate -z-1` wrappers, Error/Enter button layering via `-mr-5` + DOM order, reactive props destructuring, static `RADIUS_CLASS` lookup for Tailwind scanning, Vue 3.3+ `defineEmits` property syntax - **LGraphNode.vue**: Move `min-h-(--node-height)` from body to root; remove `footerStateOutlineBottomClass`, `footerRootBorderBottomClass`, `footerResizeHandleBottomClass`, `hasFooter` computed; replace dynamic `beforeShapeClass` interpolation with static `bypassOverlayClass`/`mutedOverlayClass` computeds for Tailwind scanning - **LGraphNode.ts**: `measure()` collapsed branch gated by `|| LiteGraph.vueNodesMode` — Vue mode defers to `this.size`; legacy path unchanged - **useVueNodeResizeTracking.ts**: Collapsed and expanded nodes both flow through `batchUpdateNodeBounds`; narrowed `useVueElementTracking` parameter from `MaybeRefOrGetter<string>` to `string`; `deferredElements.delete(element)` on unmount to prevent memory retention - **selectionBorder.ts**: Unchanged — `createBounds` just works because `boundingRect` is now correct - **12 parameterized E2E tests**: Vue mode (subgraph/regular × expanded/collapsed × bottom-left/bottom-right) + legacy mode (expanded/collapsed × bottom-left/bottom-right), driven by `keyboard.collapse()` (Alt+C) - **Unit tests**: `measure()` branching (legacy fallback, Vue `this.size` usage, expanded parity) - **Shared test helpers**: `repositionNodes`, `KeyboardHelper.collapse`, `measureSelectionBounds`, `assertSelectionEncompassesNodes` ## Review Focus - `isolate -z-1` CSS layering pattern — is this acceptable long-term? - `measure()` collapsed branch gated on `LiteGraph.vueNodesMode` — one-line gate to avoid the canvas-ctx-less fallback in Vue mode - Footer button overlap design (`-mr-5` with DOM order for painting) ## Screenshots <img width="1392" height="800" alt="image" src="https://github.com/user-attachments/assets/abaebff5-bb8c-4b5b-8734-8d44fdee4cb9" /> <img width="1493" height="872" alt="image" src="https://github.com/user-attachments/assets/6b9c77f9-e3ae-4d4e-81dc-acfa9a24c768" /> <img width="813" height="515" alt="image" src="https://github.com/user-attachments/assets/ce15bafb-e157-408c-971b-a650088f316a" /> <img width="1031" height="669" alt="image" src="https://github.com/user-attachments/assets/20fdc336-4bc2-4d47-ab7e-c0cbcee0d150" /> <img width="753" height="525" alt="image" src="https://github.com/user-attachments/assets/2dccbe31-7d18-49bc-9ed4-158b1659fddf" /> <img width="730" height="370" alt="image" src="https://github.com/user-attachments/assets/ab87edfa-a4b4-46f7-86ae-4965a4509b42" /> <img width="1132" height="465" alt="image" src="https://github.com/user-attachments/assets/54643f5b-4a31-4c3d-9475-c433f87aedb0" /> <img width="1102" height="449" alt="image" src="https://github.com/user-attachments/assets/9c045df3-e1f5-481e-b1cb-ead1db1626f5" /> --------- Co-authored-by: github-actions <github-actions@github.com> |
||
|
Terry Jia
|
deba72e7a0 |
gizmo controls (#11274)
## Summary Add Gizmo transform controls to load3d - Remove automatic model normalization (scale + center) on load; models now appear at their original transform. The previous auto-normalization conflicted with gizmo controls — applying scale/position on load made it impossible to track and reset the user's intentional transform edits vs. the system's normalization - Add a manual Fit to Viewer button that performs the same normalization on demand, giving users explicit control - Add Gizmo Controls (translate/rotate) for interactive model manipulation with full state persistence across node properties, viewer dialog, and model reloads - Gizmo transform state is excluded from scene capture and recording to keep outputs clean ## Motivation The gizmo system is a prerequisite for these potential features: - Custom cameras — user-placed cameras in the scene need transform gizmos for precise positioning and orientation - Custom lights — scene lighting setup requires the ability to interactively position and aim light sources - Multi-object scene composition — positioning multiple models relative to each other requires per-object transform controls - Pose editor — skeletal pose editing depends on the same transform infrastructure to manipulate individual bones/joints Auto-normalization was removed because it silently mutated model transforms on load, making it impossible to distinguish between the original model pose and user edits. This broke gizmo reset (which needs to know the "clean" state) and would corrupt round-trip transform persistence. ## Screenshots (if applicable) https://github.com/user-attachments/assets/621ea559-d7c8-4c5a-a727-98e6a4130b66 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11274-gizmo-controls-3436d73d365081c38357c2d58e49c558) by [Unito](https://www.unito.io) |
||
|
Alexander Brown
|
da91bdc957 |
fix: persist middle-click reroute node setting across reloads (#11362)
*PR Created by the Glary-Bot Agent* --- ## Summary - Remove hardcoded `LiteGraph.middle_click_slot_add_default_node = true` from `slotDefaults` extension `init()` that unconditionally overrode the user's persisted preference on every page load - Add E2E regression test verifying both the setting store value and the LiteGraph runtime flag persist through page reload ## Root Cause The `Comfy.SlotDefaults` extension's `init()` method (in `slotDefaults.ts`) contained a hardcoded `LiteGraph.middle_click_slot_add_default_node = true` from the original JS→TS conversion (July 2024). When `Comfy.Node.MiddleClickRerouteNode` was later made configurable in v1.3.42, this line was never removed. Since extension `init()` runs **after** `useLitegraphSettings()` syncs the stored value, the hardcoded assignment overwrote the user's preference on every reload. ## Changes | File | Change | |------|--------| | `src/extensions/core/slotDefaults.ts` | Remove line 21 (`LiteGraph.middle_click_slot_add_default_node = true`) | | `browser_tests/tests/dialogs/settingsDialog.spec.ts` | Add reload persistence test asserting both store value and LiteGraph global | The setting default (`true`) is already properly managed by `coreSettings.ts` and reactively synced via `useLitegraphSettings.ts`, so removing the hardcoded line preserves existing default behavior while allowing user overrides to persist. ## Screenshots    ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11362-fix-persist-middle-click-reroute-node-setting-across-reloads-3466d73d365081ef8692dbd0619c8594) by [Unito](https://www.unito.io) Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> |
||
|
Kelly Yang
|
20255da61f |
feat(load3d): add optional HDRI environment lighting to 3D preview nodes (#10818)
## Summary Adds `HDRIManager` to load `.hdr/.exr` files as equirectangular environment maps via **three.js** `RGBELoader/EXRLoader` - Uploads HDRI files to the server via `/upload/image` API so they persist across page reloads - Restores HDRI state (enabled, **intensity**, **background**) from node properties on reload - Auto-enables "**Show as Background**" on successful upload for immediate visual feedback - Hides standard directional lights when HDRI is active; restores them when disabled - Hides the Light Intensity control while HDRI is active (lights have no effect when HDRI overrides scene lighting) - Limits HDRI availability to PBR-capable formats (.gltf, .glb, .fbx, .obj); automatically disables when switching to an incompatible model - Adds intensity slider and "**Show as Background**" toggle to the HDRI panel ## How to Use HDRI Environment Lighting 1. Load a 3D model using a Load3D or Load3DViewer node (supported formats: .gltf, .glb, .fbx, .obj) 2. Open the control panel → go to the Light tab 3. Click the globe icon to open the **HDRI panel** 4. Click Upload HDRI and select a` .hdr` or `.exr` file 5. The environment lighting applies automatically — the scene background also updates to preview the panorama 6. Use the intensity slider to adjust the strength of the environment lighting 7. Toggle Show as Background to show or hide the HDRI panorama behind the model ## Screenshots https://github.com/user-attachments/assets/1ec56ef0-853e-452f-ae2b-2474c9d0d781 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10818-feat-load3d-add-optional-HDRI-environment-lighting-to-3D-preview-nodes-3366d73d365081ea8c7ad9226b8b1e2f) by [Unito](https://www.unito.io) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Adds new HDRI loading/rendering path and persists new `LightConfig.hdri` state, touching Three.js rendering, file uploads, and node property restoration. Risk is moderate due to new async flows and potential compatibility/performance issues with model switching and renderer settings. > > **Overview** > Adds optional **HDRI environment lighting** to Load3D previews, including a new `HDRIManager` that loads `.hdr`/`.exr` files into Three.js environment/background and exposes controls for enable/disable, background display, and intensity. > > Extends `LightConfig` with an `hdri` block that is persisted on nodes and restored on reload; `useLoad3d` now uploads HDRI files, loads them into `Load3d`, maps scene light intensity to HDRI intensity, and auto-disables HDRI when the current model format doesn’t support it. > > Updates the UI to include embedded HDRI controls under the Light panel (with dismissable overlays and icon updates), adjusts light intensity behavior when HDRI is active, and adds tests/strings/utilities (`getFilenameExtension`, `mapSceneLightIntensityToHdri`, new constants) to support the feature. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit b12c9722dc54a227004bdc383e3bf583504ebdce. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Terry Jia <terryjia88@gmail.com> |
||
|
Christian Byrne
|
97fca566fb |
fix: use || instead of ?? and server type in WebcamCapture upload path (#11000)
## Description Fixes the WebcamCapture image upload path construction that was still broken on cloud environments after #10220. ### Root cause The cloud `/upload/image` endpoint returns: ```json { "name": "hash.png", "subfolder": "", "type": "input" } ``` The previous fix used `??` (nullish coalescing), which doesn't catch empty strings: - `subfolder: ""` → `"" ?? "webcam"` = `""` → path becomes `/hash.png` (wrong) - `type` was hardcoded as `[temp]` but cloud stores as `input` → file not found ### Fix - `??` → `||` so empty strings fall back to defaults - Use `data.type` from server response instead of hardcoding `[temp]` ### QA evidence Prod (cloud/1.42): `ImageDownloadError: the input file 'webcam/1775685296883.png [temp]' doesn't exist` Staging (cloud/1.42): `ImageDownloadError: Failed to validate images` ### Related - Fixes the remaining issue from #10220 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11000-fix-use-instead-of-and-server-type-in-WebcamCapture-upload-path-33d6d73d36508156b93cfce0aae8e017) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
8487c13f14 |
feat: integrate Typeform survey into feedback button (#10890)
## Summary Replace Zendesk feedback URLs with Typeform survey (`q7azbWPi`) in the action bar feedback button and Help Center menu for Cloud/Nightly distributions. ## Changes - **What**: - `cloudFeedbackTopbarButton.ts`: Replace `buildFeedbackUrl()` (Zendesk) with direct Typeform survey URL. Remove unused Zendesk import. - `HelpCenterMenuContent.vue`: Feedback menu item now opens Typeform URL for Cloud/Nightly builds; falls back to `Comfy.ContactSupport` (Zendesk) for other distributions. Added external link icon for Cloud/Nightly. - Help menu item and `Comfy.ContactSupport` command unchanged — support flows still route to Zendesk. ## Review Focus - Gating logic: `isCloud || isNightly` correctly limits Typeform redirect to intended distributions - Help item intentionally unchanged (support ≠ feedback) Ticket: COM-17992 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10890-feat-integrate-Typeform-survey-into-feedback-button-33a6d73d36508185abbfe57e7a36b5f7) by [Unito](https://www.unito.io) |
||
|
Dante
|
65d1313443 |
fix: preserve CustomCombo options through clone and paste (#10853)
## Summary - Fix `CustomCombo` copy/paste so the combo keeps its option list and selected value - Scope the fix to `src/extensions/core/customWidgets.ts` instead of changing LiteGraph core deserialization - Replace the previous round-trip test with a regression test that exercises the actual clone/paste lifecycle - Fixes #9927 ## Root Cause `CustomCombo` option widgets override `value` to read from `widgetValueStore`. During `node.clone()` and clipboard paste, `configure()` restores widget values before the new node is added to the graph and before those widgets are registered in the store. That meant the option widgets read back as empty while `updateCombo()` was rebuilding the combo state, so `comboWidget.options.values` became blank on the pasted node. ## Fix Keep a local fallback value for each generated `option*` widget in `customWidgets.ts`. The getter now returns the store-backed value when available and falls back to the locally restored value before store registration. This preserves the option list during `clone().serialize()` and paste without hard-coding `CustomCombo` behavior into `LGraphNode.configure()`. ## Why No E2E Test This regression happens in the internal LiteGraph clipboard lifecycle: `clone() -> serialize() -> createNode() -> configure() -> graph.add()`. The failing state is the transient pre-add relationship between `CustomCombo`'s store-backed option widgets and `comboWidget.options.values`, which is not directly exposed through a stable DOM assertion in the current Playwright suite. A focused unit regression test is the most direct way to cover that lifecycle without depending on brittle canvas interaction timing. ## Test Plan - [x] Regression test covers `clone().serialize() -> createNode() -> configure() -> graph.add()` for `CustomCombo` - [ ] CI on the latest two commits (`81ac6d2ce`, `94147caf1`) - [ ] Manual: create `CustomCombo` -> add `alpha`, `beta`, `gamma` -> select `beta` -> copy/paste -> verify the pasted combo still shows all three options and keeps `beta` selected |
||
|
Dante
|
4411d9a417 |
refactor: extract shared click-vs-drag guard utility (#10357)
## Summary Extract duplicated click-vs-drag detection logic into a shared `useClickDragGuard` composable and `exceedsClickThreshold` pure utility function. ## Changes - **What**: New `useClickDragGuard(threshold)` composable in `src/composables/useClickDragGuard.ts` that stores pointer start position and checks squared distance against a threshold. Also exports `exceedsClickThreshold` for non-Vue contexts. - Migrated `DropZone.vue`, `useNodePointerInteractions.ts`, and `Load3d.ts` to use the shared utility - `CanvasPointer.ts` left as-is (LiteGraph internal) - All consumers now use squared-distance comparison (no `Math.sqrt` or per-axis `Math.abs`) ## Review Focus - The composable uses plain `let` state instead of `ref` since reactivity is not needed for the start position - `Load3d.ts` uses the pure `exceedsClickThreshold` function directly since it is a class, not a Vue component - Threshold values preserved per-consumer: DropZone=5, useNodePointerInteractions=3, Load3d=5 Fixes #10356 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10357-refactor-extract-shared-click-vs-drag-guard-utility-32a6d73d3650816e83f5cb89872fb184) by [Unito](https://www.unito.io) --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
Terry Jia
|
debbdfd1f7 |
test: verify gradient_stops survives object spread (#10408)
## Summary follow up https://github.com/Comfy-Org/ComfyUI_frontend/pull/10406 to add test ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10408-test-verify-gradient_stops-survives-object-spread-32c6d73d3650815da123c88f4b9b86ce) by [Unito](https://www.unito.io) |
||
|
Terry Jia
|
2d88720d80 |
fix: make gradient_stops enumerable so it survives object spread (#10406)
## Summary Object.defineProperty defaults to enumerable: false for new properties. NodeWidgets.vue spreads widget options into a new object, dropping non-enumerable properties — gradient_stops was silently lost, causing the gradient slider to fall back to the default black-to-white gradient. ## Screenshots (if applicable) before <img width="1679" height="1271" alt="image" src="https://github.com/user-attachments/assets/60a20163-56e5-4438-9c36-ea42c23c7495" /> after <img width="1686" height="1421" alt="image" src="https://github.com/user-attachments/assets/5057b3d4-32ee-4abc-9118-cc2c3f85ae85" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10406-fix-make-gradient_stops-enumerable-so-it-survives-object-spread-32c6d73d36508131b7c8c3ab3e93d221) by [Unito](https://www.unito.io) |
||
|
Alexander Brown
|
32bd570855 |
chore: clean up knip config, upgrade to v6, remove unused exports (#10348)
## Summary Clean up stale knip configuration, upgrade to v6, and remove unused exports. ## Changes - **What**: Upgrade knip 5.75.1 → 6.0.1; remove 13 stale/redundant config entries; remove custom CSS compiler (replaced by v6 built-in); move CSS plugin deps to design-system package; fix husky pre-commit binary detection; remove 3 unused exports (`MaintenanceTaskRunner`, `ClipspaceDialog`, `Load3dService`) - **Dependencies**: knip ^5.75.1 → ^6.0.1; tailwindcss-primeui and tw-animate-css moved from root to packages/design-system ## Review Focus - The husky pre-commit change from `pnpm exec lint-staged` to `npx --no-install lint-staged` works around a knip script parser limitation ([knip#743](https://github.com/webpro-nl/knip/issues/743)) - knip v6 drops Node.js 18 support (requires ≥20.19.0) and removes `classMembers` issue type Co-authored-by: Amp <amp@ampcode.com> |
