mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-24 06:35:10 +00:00
bb0293a4e870b8ee2be21ff3aadbad73b48d4aa5
751 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
AustinMroz
|
2717d59451 |
Fix reactivity of vue subgraph price badges (#12029)
When a subgraph contains partner nodes with price badges, those badges are also displayed on the subgraphNode. The reactivity here was spotty: The price badges would fail to display unless the user had navigated into the subgraph on the current page load. Fixing this is performed in 2 steps: - Firing a `node:property:changed` event when the badges contained in a subgraph are updated - Extending the reactivity updates so that badges update in vue mode despite using the litegraph badge getter. This PR also includes a minor styling tweak to fix text alignment on price badges | Before | After | | ------ | ----- | | <img width="360" alt="before" src="https://github.com/user-attachments/assets/56a95cbe-12c9-43b0-8664-34e52b6415ac" /> | <img width="360" alt="after" src="https://github.com/user-attachments/assets/bf4a0d81-21e4-4afc-946e-eba5967f1715" />| Resolves FE-346 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12029-Fix-reactivity-of-vue-subgraph-price-badges-3586d73d3650813cb12fe265090940e4) by [Unito](https://www.unito.io) |
||
|
Dante
|
3b37488eee |
fix: keep node context menu overflow visible when content fits (#12035)
## Summary Stops the Shape submenu (and any other PrimeVue nested submenu) from being clipped behind the node context menu when the menu fits in the viewport. ## Changes - **What**: `constrainMenuHeight` in `NodeContextMenu.vue` now applies `max-height` + `overflow-y: auto` to the root `<ul>` only when `scrollHeight > availableHeight`. The common case keeps `overflow: visible`. - Added `browser_tests/tests/nodeContextMenuShapeSubmenu.spec.ts` regression spec. ## Review Focus Root cause: setting only `overflow-y: auto` on a `<ul>` coerces `overflow-x` to a non-visible value per CSS spec (`If one of overflow-x/overflow-y is visible and the other isn't, the visible value is computed as auto`). PrimeVue `ContextMenuSub` renders submenus in-tree as a nested `<ul>` with `position: absolute; left: 100%`, so the implicit horizontal clip hides them entirely. The pre-existing overflow scenario (#10824 / #10854) is unchanged — when the menu actually overflows, the clamp still applies and `nodeContextMenuOverflow.spec.ts` continues to verify scroll. Submenu clipping in that overflow case is a known limitation, not introduced by this PR. Fixes FE-570 ## screenshot ### AS IS <img width="788" height="505" alt="Screenshot 2026-05-07 at 12 43 26 PM" src="https://github.com/user-attachments/assets/36d34070-0c57-4385-a130-0394f22f282e" /> ### TO BE <img width="779" height="627" alt="Screenshot 2026-05-07 at 12 42 44 PM" src="https://github.com/user-attachments/assets/00956729-763b-4787-822f-209e8ea42331" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12035-fix-keep-node-context-menu-overflow-visible-when-content-fits-3586d73d365081ad9aaec82f220d401c) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
jaeone94
|
fc7e6a0935 |
fix(terminal): resync logs console on backend reconnect (#12270)
## Summary When the built-in logs terminal stayed open during a backend restart, the buffer froze on pre-restart entries and live log streaming silently stopped — only closing and reopening the panel resynced. Listen for the api `reconnected` event and rebuild the terminal contents the same way a fresh open would. ## Changes - **What**: - Extract `useLogsTerminal` composable. The SFC is now a thin shell holding `terminal: shallowRef<Terminal>` and forwarding to the composable, so `onMounted`/`onScopeDispose` no longer rely on the child's emit callback timing. - Subscribe to `api`'s `reconnected` event via `useEventListener`, registered synchronously before any awaits. On reconnect: `terminal.reset()` → refetch raw logs → `scrollToBottom()` → `subscribeLogs(true)` (the backend loses the per-client subscription on restart, so re-subscribe is required for live streaming to resume). - Wrap in-flight resync/mount fetches in AbortControllers. Overlapping reconnects abort the prior resync, and unmount mid-fetch suppresses writes to the disposed xterm. - Hide BaseTerminal whenever `errorMessage` is set so the error layout doesn't expose an empty xterm container behind the message; `loading=false` after both load failure and resync success so a later successful reconnect can clear a stuck spinner. - Migrate the load/resync error strings to vue-i18n (`logsTerminal.loadError`, `logsTerminal.resyncError`). ## Review Focus - **Re-subscribe is the non-obvious half of the fix** — without it, even after the WebSocket reconnects the backend never resumes streaming logs to this client because its subscription state was wiped on restart. The visible "stale buffer" is only one symptom; the silent "no new logs" symptom needed the explicit `subscribeLogs(true)` re-call in resync. - `terminal.reset()` lives after a successful raw-logs fetch (not before) so a failed resync leaves the prior buffer visible instead of blanking it; resync errors surface via the same inline error message the mount path uses. - 8 unit tests around the composable: mount + subscribe, resync ordering (reset → write → scroll → subscribe via `invocationCallOrder`), in-flight resync abort on double reconnect, resync error surfacing, mount-failure-then-recovery, unmount-mid-fetch terminal-write suppression, listener cleanup on unmount. - 2 E2E tests using `ws.close()` on the proxied WebSocket as the reconnect trigger and `subscribeLogs` HTTP fetch count as the sync point (same pattern as `wsReconnectStaleJob.spec.ts`). Red-checked: disabling the `reconnected` listener fails exactly the two new tests, all 8 pre-existing tests stay green. Fixes FE-712 ## Screenshots Before - (After rebooting, the console window does not update from its state before the reboot must remount the console window for it to resync.) https://github.com/user-attachments/assets/b1e49c2c-89a4-4a4a-82b4-064412acee12 After - (The console window syncs automatically after a reboot.) https://github.com/user-attachments/assets/54b582c5-ad42-41c0-9886-18f4495859da ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12270-fix-terminal-resync-logs-console-on-backend-reconnect-3606d73d3650812fb13fd1934c632344) by [Unito](https://www.unito.io) |
||
|
Dante
|
d955625c20 |
fix(model-library): auto-refresh after upload, plus 'r' key fallback (FE-695) (#12257)
## Summary Model Library sidebar now refreshes automatically when an upload completes, and the `r` keybinding refreshes the library in addition to refreshing combo widgets inside graph nodes. ## Changes - **What**: - `modelStore`: new `refreshModelFolder(name)` for surgical reset+reload of one folder, and `refresh()` that re-loads any folders that had been loaded - `ModelLibrarySidebarTab.vue`: watches `assetDownloadStore.lastCompletedDownload` and refreshes the affected folder; the in-panel refresh button now routes through `refresh()` - `Comfy.RefreshNodeDefinitions` (`r` key): also calls `modelStore.refresh()` so the keyboard fallback actually refreshes the Model Library list ## Review Focus - Both `modelStore` and `assetsStore` exist; the upload wizard was only refreshing the latter, which is what caused the bug. Confirm the new watcher path is the right hook (rather than wiring it inside the wizard) — chose this so it also covers completions that happen after the wizard has been closed. - `refreshModelFolder` falls back to `refresh()` (not raw `loadModelFolders()`) for unknown folder types, to avoid dropping other folders' loaded contents. - Generated tab half of the ticket is intentionally **deferred** until BE-885 (cursor pagination on `GET /api/jobs`) lands — AC items around "no duplicates" and "cursor state maintained" depend on it. Fixes FE-695 (Model Library half). ## Screenshots (if applicable) N/A — behavior change verified by unit tests. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12257-fix-model-library-auto-refresh-after-upload-plus-r-key-fallback-3606d73d3650811a8895ef6e3ef2b4b8) 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) |
||
|
Csongor Czezar
|
7160a9ee3f |
fix: QPO progress bar now shows node name in subgraphs (#7688)
## Summary
Resolve the queue progress node label from queued prompt metadata so
subgraph execution IDs show the correct node name without depending on
the live canvas.
## Changes
- **What**: Store a prompt-scoped `executionId -> { title, type }`
lookup from `p.output` when queueing a job, and use that lookup for the
active job's executing node label.
- **What**: Reuse the same job-scoped node info for the browser tab
title so it stays aligned with the queue overlay.
- **What**: Add unit coverage for root and subgraph execution IDs, and
merge the branch forward to current `main`.
## Review Focus
This keeps the fix scoped to the existing singular `activeJobId` path.
It fixes subgraph labels and avoids the workflow-switching regression
from resolving against `app.rootGraph`, but it does not redesign
concurrent multi-job selection yet.
Longer term, the cleaner solution is still prompt-scoped execution
metadata from the backend rather than frontend reconstruction.
## Screenshots (if applicable)
N/A
---------
Co-authored-by: Alexander Brown <drjkl@comfy.org>
Co-authored-by: Benjamin Lu <benjaminlu1107@gmail.com>
|
||
|
Dante
|
1869416185 |
fix: surface error dialog when Open Workflow from Job Queue fails (FE-215) (#12071)
## Summary `openJobWorkflow` (Job Queue → "Open Workflow in New Tab") had no error handling around `workflowService.openWorkflow`. When workflow JSON contained nodes that fail `LiteGraph.configure()` (e.g. rgthree `DisplayAny`, stale `GetNode/SetNode aux_id`), the menu action appeared to do nothing — either an early return after a generic "Load Workflow Error" dialog (`app.ts:1340-1347`) or a context-less toast from the surrounding `wrapWithErrorHandlingAsync`. This PR catches the error inside `openJobWorkflow` and routes it to `dialogService.showErrorDialog` with a Job-Queue-specific `reportType` so users get a clear, actionable message tied to the action they invoked. - Fixes #8841 - Linear: [FE-215](https://linear.app/comfyorg/issue/FE-215/open-workflow-from-frontend-not-working) ## Red-Green Verification | Commit | CI Status | Run | |--------|-----------|-----| | `test: add failing test for openJobWorkflow swallowing load errors` ( |
||
|
jaeone94
|
a3106c4d53 |
fix: open node info panel from context menu (#12205)
## Summary Replaces #12164. Right-clicking a Vue node, using the selection toolbox More Options menu, or clicking the selection toolbox Node Info button now opens the right-side Info tab only when the new-menu UI makes that panel available. Legacy-menu contexts hide the no-op action even when the legacy node library design is selected; node-library help remains isolated to the node library itself. The existing `selection_toolbox_node_info_opened` telemetry fires only after the toolbox button successfully opens node info. No new context-menu telemetry event is added in this PR. ## Changes - **What**: Share the node-info availability/action path across the context menu and selection toolbox, keep legacy-menu state out of the right-side panel public store API, tighten node-info settings tests, and add unit plus E2E regression coverage for new-menu and legacy-menu modes. - **Dependencies**: None ## Review Focus Confirm the node context menu, selection toolbox direct Info button, and selection toolbox More Options entry all respect right-side panel availability, including legacy menu + legacy node library mode, while node-library help behavior remains isolated to the node library. ## Validation - Self-review: checked production path, unit mocks, and Playwright coverage; only gap found was weak E2E coverage for the toolbox direct Info path, now strengthened. - `pnpm test:unit -- src/composables/graph/useSelectionState.test.ts src/components/graph/SelectionToolbox.test.ts src/components/graph/selectionToolbox/InfoButton.test.ts` - `pnpm test:browser:local -- --project=chromium browser_tests/tests/selectionToolboxActions.spec.ts browser_tests/tests/selectionToolboxSubmenus.spec.ts browser_tests/tests/vueNodes/interactions/node/contextMenu.spec.ts --grep "info button opens the right-side info tab|info button is hidden|hides Node Info|should open node info"` - `pnpm typecheck:browser` - `pnpm exec oxlint --type-aware browser_tests/tests/selectionToolboxActions.spec.ts` - `pnpm exec eslint --cache --no-warn-ignored browser_tests/tests/selectionToolboxActions.spec.ts` - `pnpm exec oxfmt --check browser_tests/tests/selectionToolboxActions.spec.ts` - `git diff --check` - Commit hooks: lint-staged + `pnpm typecheck` + `pnpm typecheck:browser` - Push hook: `knip --cache` (existing tag hint only) ## Screenshots (if applicable) Before https://github.com/user-attachments/assets/4b1f6ddb-a01c-4958-81ab-36167f434e59 https://github.com/user-attachments/assets/83433f0d-24f1-46b7-a81d-f0f065812496 After https://github.com/user-attachments/assets/30bd61e5-f8d4-48b7-97e0-26c93e3cb362 https://github.com/user-attachments/assets/afce9f51-a43d-434f-a006-6b357a61ac8f --------- Co-authored-by: github-actions <github-actions@github.com> |
||
|
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) |
||
|
Terry Jia
|
4b5b184cad |
FE-566: fix Painter mask submission edge cases on cloud (#12196)
## Summary Rework the painter always hands the backend a valid asset reference: - Drop the `hasStrokes` flag and the `isCanvasEmpty` check. - `serializeValue` falls back to the existing `modelValue` when the canvas element is transiently unmounted, reuses the cached upload when not dirty and a value is present, and otherwise uploads the current canvas (a fully transparent PNG is a valid no-op mask, Painter's Python `execute()` treats painter_alpha=0 the same as "no mask painted"). - `handleClear` now also clears `modelValue` so a user-initiated clear doesn't resurrect a stale upload on the next serialize. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12196-FE-566-fix-Painter-mask-submission-edge-cases-on-cloud-35e6d73d365081dd8856ddb785952526) 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>
|
||
|
pythongosssss
|
15b8771cc2 |
fix: clear active job on reconnect if no longer in queue (#12067)
## Summary When a socket disconnects messages can be missed and lead to a stale UI state, this updates the state on reconnect and clears the active job if it is no longer running ## Changes - **What**: - add call to update queue on reconnect - clear active job if job not in queue response - tests ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12067-fix-clear-active-job-on-reconnect-if-no-longer-in-queue-3596d73d365081f79d42d73966420c50) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
8f68be5699 |
fix: handle annotated output media paths in missing media scan (#12069)
## Summary
This PR fixes missing-media false positives for annotated media widget
values such as:
```txt
photo.png [output]
clip.mp4 [input]
147257c95a3e957e0deee73a077cfec89da2d906dd086ca70a2b0c897a9591d6e.png [output]
clip.mp4[input] // Cloud compact form
```
The change is intentionally scoped to the missing-media detection
pipeline for:
- `LoadImage`
- `LoadImageMask`
- `LoadVideo`
- `LoadAudio`
It preserves the raw widget value on `MissingMediaCandidate.name` for UI
display, grouping, replacement, and user-facing missing-media rows.
Normalized values are used only as comparison keys during verification.
## Diff Size
`main...HEAD` line diff is currently:
- Production/runtime code: `+478 / -37` (`515` changed lines)
- Unit test code: `+960 / -47` (`1,007` changed lines)
- Total: `+1,438 / -84` (`1,522` changed lines)
The PR looks large mostly because it locks both Cloud and OSS/Core
runtime paths with unit coverage; the production/runtime change is about
one third of the total diff.
## What Changed
- Added missing-media-scoped annotation helpers for detection-only path
normalization.
- Core/OSS recognizes spaced suffixes like `file.png [output]`.
- Cloud also recognizes compact suffixes like `file.png[output]`.
- User-selectable trailing `input` and `output` annotations are
normalized for matching.
- Unknown annotations and middle-of-filename annotations are left
unchanged.
- Added shared file-path helpers in `formatUtil`:
- `joinFilePath(subfolder, filename)`
- `getFilePathSeparatorVariants(filepath)`
- Updated media verification to compare candidates against both raw and
normalized match keys.
- Kept input candidates and generated output candidates in separate
identifier sets so an input asset cannot accidentally satisfy an output
reference with the same name.
- Moved missing-media source loading into `missingMediaAssetResolver` so
`missingMediaScan` remains focused on scan/verification orchestration.
- Updated Cloud generated-media verification to use the Cloud assets API
instead of job history:
- Cloud input candidates use input/public assets.
- Cloud output candidates use `output` tagged assets.
- Kept OSS/Core generated-media verification history-based, matching the
current generated-picker/widget availability model.
## Runtime Verification Paths
### Cloud
Cloud stores generated outputs as asset records. For an annotated output
value, this PR verifies against the `output` asset tag rather than job
history.
```txt
Widget value
"147257...d6e.png [output]"
|
v
Detection keys
"147257...d6e.png [output]"
"147257...d6e.png"
|
v
Cloud asset sources
input candidates -> /api/assets?include_tags=input&include_public=true
output candidates -> /api/assets?include_tags=output&include_public=true
|
v
Match against
asset.name
asset.asset_hash
subfolder/asset.name
subfolder/asset.asset_hash
slash and backslash separator variants
```
Example:
```ts
candidate.name = 'abc123.png [output]'
asset.name = 'ComfyUI_00001_.png'
asset.asset_hash = 'abc123.png'
asset.tags = ['output']
// Result: not missing
```
### OSS / Core
Core widget options for the normal loader nodes are input-folder based.
Annotated output values are resolved by Core through
`folder_paths.get_annotated_filepath()`, but the current generated
picker path is history-backed. This PR keeps OSS generated verification
aligned with that widget availability model instead of treating the full
output folder as the source of truth.
```txt
Widget value
"subfolder/photo.png [output]"
|
v
Detection keys
"subfolder/photo.png [output]"
"subfolder/photo.png"
|
v
OSS generated source
fetchHistoryPage(...)
|
v
History preview_output
filename: "photo.png"
subfolder: "subfolder"
|
v
Generated match keys
"subfolder/photo.png"
"subfolder\\photo.png"
```
This means OSS/Core verification is about whether the generated media is
currently available through the same generated/history-backed path the
widget uses, not a full disk-level executability check across the entire
output directory.
## Why Not Consolidate All Annotated Path Parsers
There are existing annotated-path parsers in image widget, Load3D, and
path creation code. This PR does not replace them.
The helper added here is detection-only: it strips annotations to build
comparison keys for missing-media verification. Parser consolidation
across widget implementations is intentionally left out of scope to keep
this fix narrow.
## Known Follow-Ups / Out Of Scope
- FE-620 tracks the separate video drag-and-drop upload race between
upload completion and missing-media detection.
- Published/shared workflow assets are still not fully represented by
`/api/assets?include_public=true`; that remains a backend/API contract
issue.
- A future backend/API contract that answers “is this workflow media
executable?” would be preferable to stitching together runtime-specific
FE sources.
- OSS/Core full output-folder scanning via `/internal/files/output` was
considered, but that endpoint is internal, shallow (`os.scandir`), and
not the same source currently used by the generated picker flow.
## Validation
- `pnpm test:unit -- missingMediaAssetResolver missingMediaScan
mediaPathDetectionUtil formatUtil`
- touched files `oxfmt`
- touched files `oxlint --fix`
- touched files `eslint --cache --fix --no-warn-ignored`
- `pnpm typecheck`
- pre-commit `pnpm knip --cache`
- pre-push `pnpm knip --cache`
`knip` passes with the existing tag hint:
```txt
Unused tag in src/scripts/metadata/flac.ts: getFromFlacBuffer → @knipIgnoreUnusedButUsedByCustomNodes
```
## Screenshots
Before
https://github.com/user-attachments/assets/50eab565-3160-4a57-a758-87ec2c09071e
After
https://github.com/user-attachments/assets/08adcbbd-c3fc-43f9-b86c-327e4eb5abd8
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-12069-fix-handle-annotated-output-media-paths-in-missing-media-scan-3596d73d365081f4afa3d4dd45cad3da)
by [Unito](https://www.unito.io)
|
||
|
AustinMroz
|
68843967cf |
App Mode tests (#10633)
Adds tests for - Mobile app mode. - Drag and drop operations in app mode - Basic widget interaction in app mode. - The read only state when in builder mode. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10633-App-Mode-tests-3306d73d36508154aa25d8096119a32c) by [Unito](https://www.unito.io) |
||
|
Dante
|
0bc951fd12 |
fix: clarify unsaved-changes modal buttons and fix sign-out 3-state (#11669)
## Summary The dirtyClose modal had three buttons (`Cancel | No | Save`) and the sign-out flow collapsed two distinct outcomes (deny vs. dismiss) into a single early return — so today clicking "No" *cancels* sign-out instead of signing out without saving, and clicking "Save" never actually saves before logging out. This PR drops `Cancel` for `dirtyClose`, gives each caller a context-specific deny label, and fixes the sign-out 3-state handling. - Fixes [FE-419](https://linear.app/comfyorg/issue/FE-419/unsaved-changes-modal-uses-confusing-button-labels) ## Changes - **What**: - `ConfirmationDialogContent.vue`: hide `Cancel` for `type='dirtyClose'`; add `denyLabel?: string` prop; autofocus `Save` (preserves work on Enter). - `dialogService.confirm()`: accept and forward `denyLabel`. - `useAuthActions.logout`: handle `null` (cancel) / `false` (sign out anyway, no save) / `true` (save each modified workflow, then logout) distinctly. Pass `denyLabel: 'Sign out anyway'`. - `workflowService.closeWorkflow`: pass `denyLabel: 'Close anyway'`. - i18n: add `auth.signOut.signOutAnyway` and `sideToolbar.workflowTab.closeAnyway`. - **Breaking**: none. The `denyLabel` prop is optional and falls back to `g.no`. ## Review Focus - The "Save" branch in `useAuthActions.logout` now iterates `workflowStore.modifiedWorkflows` and awaits `useWorkflowService().saveWorkflow(workflow)` for each before calling `authStore.logout()`. The close-tab path (`workflowService.closeWorkflow`) was already correct — only the sign-out path needed the same shape. - `ConfirmationDialogContent` autofocus moves from `Cancel` (gone for `dirtyClose`) to `Save`. The dialog is still dismissable via ESC / outside-click, which routes through `dialogComponentProps.onClose → resolve(null)` — sign-out and close-tab both treat `null` as cancel. - Out of scope: the native browser `beforeunload` warning (`UnloadWindowConfirmDialog.vue`) is a separate flow and never reaches the in-app modal. ## Tests - Unit (`useAuthActions.test.ts`, new): logout handles `null` / `false` / `true` / no-modified-workflows; saves *every* modified workflow before `authStore.logout`; passes `denyLabel='Sign out anyway'`. - Unit (`ConfirmationDialogContent.test.ts`): Cancel hidden for `dirtyClose`; custom `denyLabel` rendered; falls back to `g.no` when omitted. - E2E (`workflowTabs.spec.ts`): modified-tab close shows `Close anyway` (not `No`) and no `Cancel`; clicking `Close anyway` removes the tab; ESC keeps the tab. ## screenshot ### AS IS <img width="816" height="379" alt="Screenshot 2026-04-27 at 5 40 19 PM" src="https://github.com/user-attachments/assets/a8e39403-bf72-455a-8d86-6ceb1f94ac85" /> <img width="923" height="396" alt="Screenshot 2026-04-27 at 5 40 38 PM" src="https://github.com/user-attachments/assets/08031c7c-b3a6-45d7-a4dc-5dcb4e63cfa0" /> ### TO BE <img width="1661" height="872" alt="Screenshot 2026-04-27 at 5 43 40 PM" src="https://github.com/user-attachments/assets/b89d160b-be66-450e-981e-32b1591f6841" /> <img width="1488" height="584" alt="Screenshot 2026-04-27 at 5 44 21 PM" src="https://github.com/user-attachments/assets/b3a141a7-1f3b-4f25-85a9-49529229c28b" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11669-fix-clarify-unsaved-changes-modal-buttons-and-fix-sign-out-3-state-34f6d73d365081bf8afad8e146b3b990) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
1ab9752af8 |
fix: keep Reka overlays above PrimeVue dialogs (#12038)
## Summary Temporarily patch FE-569 by keeping the affected portaled Reka dropdowns and menus above their containing PrimeVue dialogs when PrimeVue auto z-index state has been elevated. ## Changes - **What**: Added a small compatibility helper, `usePrimeVueOverlayChildStyle`, that returns an anchor ref plus a computed inline style for child popover content. The helper finds the nearest PrimeVue dialog mask (`.p-dialog-mask` / `.p-overlay-mask`) from the parent surface and, only when found, applies `parent z-index + 1` to the affected Reka overlay content. - **What**: Applied that helper at the exact PrimeVue parent surfaces where the issue was found. This PR does not add a global overlay policy and does not change every Reka select/dropdown in the app. - **What**: Added optional `contentStyle`/`selectContentStyle` plumbing only where needed so the style reaches the actual portaled Reka overlay root. - **What**: Added focused unit coverage for the helper contract: no PrimeVue parent preserves existing stacking, PrimeVue dialog/overlay masks render child content above the parent, low parent z-index values respect the Reka floor, and invalid z-index values do not inject an inline override. - **Approach**: This is intentionally a minimal, parent-scoped band-aid. It avoids a global PrimeVue overlay scanner because global sampling can be polluted by unrelated persistent PrimeVue roots such as Toast and would turn this fix into a broader layering policy. - **Approach**: The patch targets the confirmed failure mode: a Reka child overlay rendering below its owning PrimeVue dialog after PrimeVue autoZIndex has been elevated. It does not attempt to solve PrimeVue z-index globally. - **Lifecycle**: This is temporary migration compatibility. PrimeVue dialogs and controls are being incrementally migrated to Reka UI, so `usePrimeVueOverlayChildStyle` and the optional style props added for FE-569 should be removed once the affected parent surfaces move to Reka. - **Breaking**: None. New props are optional and no public API contract is changed. - **Dependencies**: None. ## Patched Entry Points This PR pinpoints the six affected user-facing surfaces below. Each patch is applied from the PrimeVue dialog parent and passed only to the Reka child overlay content that can render underneath that parent. https://github.com/user-attachments/assets/d0d1522a-ffc7-4934-9e7a-06b83e20f809 1. **Workflow Template Library filters** - **How to enter**: click the Templates button in the left sidebar, or open the Comfy menu and choose **Browse Templates**. - **Affected elements**: the template filter popovers in `WorkflowTemplateSelectorDialog`: **Model**, **Use case**, **Runs on**, and **Sort by**. - **Patch point**: `WorkflowTemplateSelectorDialog.vue` anchors to the template dialog content filter area and passes `selectContentStyle` to the affected `MultiSelect` / `SingleSelect` controls. https://github.com/user-attachments/assets/3641fa24-da51-4392-a904-9085f8a5a2f4 2. **Manager dialog header controls** - **How to enter**: open Manager from the top/menu Manager entry when the new Manager UI is available. - **Affected elements**: the Manager header controls in `ManagerDialog`: search mode `SingleSelect`, search autocomplete suggestions, and **Sort** `SingleSelect`. - **Patch point**: `ManagerDialog.vue` anchors to the dialog header and passes `selectContentStyle` to those three Reka overlays. https://github.com/user-attachments/assets/cf25cc06-f851-48ef-9d9c-9ec2da8afc06 3. **Asset Browser filter bar** - **How to enter**: open the Asset Browser from an eligible model widget browse action, the Model Library flow, or another `useAssetBrowserDialog` caller. - **Affected elements**: `AssetFilterBar` controls: **File formats**, **Base models**, **Ownership**, and **Sort by**. - **Patch point**: `AssetBrowserModal.vue` anchors to the PrimeVue dialog header and passes the style through `AssetFilterBar` to its `MultiSelect` / `SingleSelect` controls. https://github.com/user-attachments/assets/e27bd805-10c0-4b3b-97f3-9e11faa47021 4. **Asset Browser model info panel** - **How to enter**: open Asset Browser, select an asset, then use the right-side model info panel. - **Affected element**: the **Model type** select in `ModelInfoPanel`. - **Patch point**: `AssetBrowserModal.vue` reuses the same parent-scoped style and passes it to `ModelInfoPanel` as `selectContentStyle`. https://github.com/user-attachments/assets/5e9f7ef0-ebd7-4987-ba1b-2137c034086f 5. **Upload Model confirmation step** - **How to enter**: open Asset Browser, click **Upload**, enter/fetch model metadata, then proceed to the confirmation step. - **Affected element**: the **Model type** `SingleSelect` in `UploadModelConfirmation`. - **Patch point**: `UploadModelConfirmation.vue` anchors within the upload dialog content and passes `selectContentStyle` to the model type selector. https://github.com/user-attachments/assets/ec145f26-8621-455b-915e-bedee47e1cbd 6. **Settings > Keybinding panel controls** - **How to enter**: open Settings from the sidebar/menu, then select the **Keybinding** panel. - **Affected elements**: the keybinding preset select, the preset overflow dropdown menu, and the row context menu inside `KeybindingPanel`. - **Patch point**: `KeybindingPanel.vue` anchors to the settings dialog panel and passes `keybindingOverlayContentStyle` only to those Reka overlay roots. ## Review Focus - Confirm the patch stays narrowly scoped to the six known PrimeVue parent + Reka child overlay surfaces above. - Confirm `contentStyle` reaches the actual portaled Reka overlay content in each patched path. - Confirm the fallback behavior preserves existing stacking when no PrimeVue parent overlay is found; in that case the helper returns an empty style object and leaves existing Tailwind z-index classes alone. - Please avoid expanding this into a larger overlay refactor. The goal is a clean, backport-friendly compatibility patch while the Reka migration continues. Validation performed: - `pnpm exec vitest run src/composables/usePopoverSizing.test.ts` - `pnpm typecheck` - `pnpm lint` (passes with existing unrelated warnings only) - `pnpm format:check` - commit hook lint-staged checks (`oxfmt`, `stylelint`, `oxlint`, `eslint --fix`, `pnpm typecheck`) - pre-push `pnpm knip` Linear: FE-569 ## Bug Screenshots https://github.com/user-attachments/assets/e73761af-9867-4c50-ab0d-4e32e59011e1 https://github.com/user-attachments/assets/145daf4d-3268-428b-9987-1e1afd0b866f ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12038-fix-keep-Reka-overlays-above-PrimeVue-dialogs-3596d73d365081e7af49dbc4d3905962) by [Unito](https://www.unito.io) |
||
|
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) |
||
|
Kelly Yang
|
d78c630d36 |
test(maskeditor): expand useBrushDrawing behavioral coverage (#12001)
Adds targeted behavioral tests for the slimmed `useBrushDrawing`
orchestration composable (Phase E of the brush-drawing refactor).
## Changes
- 5 new tests covering previously untested branches:
- `compositeStroke` receives `isRgb=true` when active layer is `rgb`
- `compositeStroke` receives `isErasing=true` when tool is `eraser`
- Mask canvas opacity is restored after drawing on the mask layer
- `globalCompositeOperation` is set to `destination-out` during
`handleDrawing` when tool is eraser
- `globalCompositeOperation` is set to `destination-out` during
`handleDrawing` when right mouse button is held
## Coverage (useBrushDrawing.ts)
| Metric | Before | After |
|--------|--------|-------|
| Statements | 86.33% | 87.05% |
| Branches | 68.75% | 70.00% |
| Functions | 90.00% | 90.00% |
| Lines | 89.23% | 90.00% |
All 18 tests pass. GPU paths remain `/* c8 ignore */` excluded
(untestable without WebGL).
- Fixes #0
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Low risk: changes are limited to unit tests, adding coverage for
eraser/right-click composition and `drawEnd` GPU compositing/opacity
restoration paths without altering production logic.
>
> **Overview**
> Adds new `useBrushDrawing` test cases to cover previously untested
branches: setting `globalCompositeOperation` to `destination-out` during
`handleDrawing` when erasing (tool or right-click), and verifying
`drawEnd` passes correct `isRgb`/`isErasing` flags to
`gpu.compositeStroke`.
>
> Also asserts mask-layer opacity is restored after `drawEnd`,
increasing behavioral coverage around stroke completion and canvas
visibility cleanup.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
jaeone94
|
0307281ff2 |
fix: highlight missing input slots on Vue nodes (#11950)
## Summary Restores required-input validation highlighting on Vue node input slots. ## Changes - **What**: Passes validation error state from `NodeSlots` to `InputSlot` using node locator IDs, including subgraph and nested subgraph execution IDs. - **What**: Adds unit coverage for root, one-level subgraph, and nested subgraph slot error mapping. - **What**: Adds a Vue Nodes screenshot regression test that asserts the missing required input slot itself receives the error highlight. - **Dependencies**: None. ## Review Focus - Required input errors on Vue-rendered node's slots. - The new Playwright screenshot expectation will need the `New Browser Test Expectation` label for Linux baseline generation. ## Screenshots (if applicable) Before <img width="499" height="324" alt="스크린샷 2026-05-05 오후 3 00 44" src="https://github.com/user-attachments/assets/285fdf91-6d7e-480b-99b9-715705f78914" /> After <img width="482" height="356" alt="스크린샷 2026-05-05 오후 3 01 11" src="https://github.com/user-attachments/assets/51b8db49-eb9c-4155-8aa5-109c0bd7699b" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11950-fix-highlight-missing-input-slots-on-Vue-nodes-3576d73d365081bd85bfd1ea149d45c5) by [Unito](https://www.unito.io) --------- Co-authored-by: github-actions <github-actions@github.com> |
||
|
jaeone94
|
21406dceb1 |
fix: skip nested subgraph containers in replay scan (#11908)
## Summary Fixes the Cloud-only nested subgraph missing-model false positive covered by the stacked regression test in #11907. When returning from an outer subgraph to the root graph, the Vue graph node manager replays `onNodeAdded` for existing graph nodes. The realtime error-clearing hook handled a subgraph container by recursively scanning all interior nodes. For nested subgraphs, that also scanned the nested subgraph container itself. Nested subgraph container widgets are promoted synthetic views of interior widgets. Scanning them as real model-loader nodes is wrong: the container node type is the subgraph UUID, not `UNETLoader`, so Cloud asset resolution can classify an installed promoted model as missing. ## Changes - Skip nested subgraph container nodes during parent subgraph replay scans. - Keep scanning real active interior leaf nodes. - Add unit coverage proving the replay scan visits the `UNETLoader` leaf but not the nested subgraph container. - Remove the `test.fail()` annotation from the Cloud E2E regression test added in #11907. ## Stacked PR This PR is stacked on #11907. After #11907 lands, this branch should be rebased or retargeted onto `main`. ## Verification - `pnpm exec vitest run src/composables/graph/useErrorClearingHooks.test.ts -t "skips nested subgraph containers during parent subgraph replay scan"` - `pnpm exec oxfmt --check src/composables/graph/useErrorClearingHooks.ts src/composables/graph/useErrorClearingHooks.test.ts browser_tests/tests/propertiesPanel/errorsTabCloudMissingModels.spec.ts` - `pnpm exec eslint src/composables/graph/useErrorClearingHooks.ts src/composables/graph/useErrorClearingHooks.test.ts browser_tests/tests/propertiesPanel/errorsTabCloudMissingModels.spec.ts` - `pnpm exec oxlint src/composables/graph/useErrorClearingHooks.ts src/composables/graph/useErrorClearingHooks.test.ts browser_tests/tests/propertiesPanel/errorsTabCloudMissingModels.spec.ts --type-aware` - `pnpm typecheck` - `pnpm typecheck:browser` - `pnpm build:cloud` - `PLAYWRIGHT_LOCAL=1 PLAYWRIGHT_TEST_URL=http://localhost:8188 pnpm exec playwright test browser_tests/tests/propertiesPanel/errorsTabCloudMissingModels.spec.ts --project=cloud` - commit hook: `pnpm typecheck`, `pnpm typecheck:browser` - push hook: `pnpm knip` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11908-fix-skip-nested-subgraph-containers-in-replay-scan-3566d73d3650819c8687d6ab74add1b9) by [Unito](https://www.unito.io) |
||
|
Benjamin Lu
|
3637b61fcd |
Use Reka popover for queue job details (#11540)
## Summary Use ShadCN-style Reka popover primitives for the live queue job list after the unused legacy queue row implementation is removed in #11621. This is the first step in migrating popovers toward the ShadCN UI pattern: local design-system wrappers over Reka UI, rather than ad hoc direct Reka or PrimeVue popovers at each call site. ## Changes - **What**: Added the minimal ShadCN-style popover primitives needed by this fix: `Popover`, `PopoverAnchor`, and `PopoverContent`. - **What**: Migrated `JobAssetsList` job details from manual fixed positioning to these popover primitives with viewport collision handling. - **What**: Removed the obsolete manual hover-position helper after `JobAssetsList` stopped using it. - **Dependencies**: No new dependencies; the primitives wrap the existing `reka-ui` package. - Added browser coverage for bottom-row job details clipping in the queue overlay. ## Review Focus - This PR is stacked on #11621. - The live queue surfaces are `JobAssetsList` consumers: expanded queue progress overlay and job history sidebar. - The new `src/components/ui/popover` files intentionally seed the ShadCN-style migration path, but only include the pieces used here to keep the first PR small. - Follow-up PRs can add `PopoverTrigger` and migrate existing PrimeVue/direct-Reka popovers once there is an actual caller. |
||
|
Kelly Yang
|
0e9a5ecbe9 |
refactor: extract GPU lifecycle into useGPUResources (phase D) (#11784)
## Summary
Phase D of the **useBrushDrawing-refactor plan.md**. Extract `WebGPU`
state management from `useBrushDrawing` into a dedicated
`useGPUResources` composable, reducing `useBrushDrawing` from ~1,160
lines to ~230. This is Phase D of the ongoing `useBrushDrawing`
decomposition (Phases A–C landed in previous PRs).
## Changes
- **What**: Split `useBrushDrawing` along a clean boundary — GPU
device/texture lifecycle moves to `useGPUResources`, stroke
orchestration stays in `useBrushDrawing`. Shared reactive state
(`dirtyRect`, `isSavingHistory`, `previewCanvas`) is now owned by
`useGPUResources` and exposed as refs. A pure
`clampDirtyRect` helper is extracted to `gpuUtils.ts`.
- **Dependencies**: No new dependencies
## Tests
Local test - pass
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Refactors WebGPU initialization, texture management, and readback
paths used during drawing; regressions could affect stroke rendering,
canvas visibility, and undo/redo GPU sync.
>
> **Overview**
> Extracts WebGPU device/texture/renderer lifecycle, watchers (clear,
undo/redo sync, texture recreation), and readback logic out of
`useBrushDrawing` into a new `useGPUResources` composable, with shared
refs (`dirtyRect`, `isSavingHistory`, `previewCanvas`, `hasRenderer`)
now owned by that module.
>
> Updates `useBrushDrawing` to delegate GPU-specific operations
(prepare/render/draw point/composite/readback/cleanup) to
`useGPUResources` while keeping CPU drawing + stroke orchestration, and
adds new pure helpers in `gpuUtils` (`clampDirtyRect`,
`buildStrokePoints`) to centralize dirty-rect clamping and stroke point
resampling.
>
> Adds Vitest coverage for the new helpers, `useGPUResources`
no-op/error behavior when GPU isn’t available, and `useBrushDrawing`
interactions with the extracted GPU API (composition mode selection,
shift-line, history save, and canvas/preview opacity restoration).
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Christian Byrne
|
6ea5a5e32d |
fix(load3d): preserve unknown Model Config fields with spread (#11838)
## Summary Use spread pattern when writing `nodeValue.properties['Model Config']` so future ModelConfig fields are preserved across viewer dialog cancel/apply. ## Changes - **What**: Spread existing `Model Config` before applying known keys in both `restoreInitialState()` and `applyChanges()` in [useLoad3dViewer.ts](src/composables/useLoad3dViewer.ts). Removes the hard-coded `showSkeleton: false` override from `applyChanges()` so it falls through from the existing config. ## Review Focus The change is intentionally minimal and matches the suggestion in the upstream issue. Two regression tests added (one each for restore/apply) verify that an unknown future field on Model Config survives both code paths. Fixes #11346 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11838-fix-load3d-preserve-unknown-Model-Config-fields-with-spread-3546d73d3650819686efc4e1a9799ad9) 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
|
||
|
Christian Byrne
|
1f60f7cfcc |
test: add unit tests for useImageCrop composable (#11138)
## Summary Add 40 unit tests for `useImageCrop` composable (previously 0% coverage, 277 missed lines). ## Changes - **What**: New test file `src/composables/useImageCrop.test.ts` covering: - Crop computed properties (read/write/defaults) - `cropBoxStyle` computation - `selectedRatio` / `isLockEnabled` aspect ratio locking - `applyLockedRatio` with boundary clamping - `resizeHandles` filtering (8 handles unlocked, 4 corners locked) - `handleImageLoad` / `handleImageError` - Drag start/move/end with boundary clamping - Resize from all 4 edges + MIN_CROP_SIZE enforcement - Constrained resize with locked aspect ratio (corner handles) - `getInputImageUrl` with subgraph node resolution - `updateDisplayedDimensions` for landscape/portrait/zero dimensions - `initialize` with `resolveNode` lookup ## Review Focus Test-only change. Mocks `resolveNode`, `useNodeOutputStore`, and `useResizeObserver`. No production code changes. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11138-test-add-unit-tests-for-useImageCrop-composable-33e6d73d365081e6aa06e98b66feb585) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
Christian Byrne
|
d429d481e8 |
test: use real vue-i18n plugin in useReconnectingNotification tests (#11386)
## Summary
Replace `vi.mock('vue-i18n')` stub with a real `createI18n` plugin
instance in `useReconnectingNotification` tests.
## Changes
- **What**: Add `setupComposable()` helper that renders a wrapper
component via `@testing-library/vue` with a real `createI18n` plugin.
Assertions now check translated values
(`'Reconnecting'`/`'Reconnected'`) instead of raw i18n keys. Removes the
brittle `vi.mock('vue-i18n')` stub.
## Review Focus
Straightforward test-only change — the composable requires a component
setup context for `useI18n()`, so we render a thin wrapper via
`@testing-library/vue` with the i18n plugin installed.
Fixes #11153
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11386-test-use-real-vue-i18n-plugin-in-useReconnectingNotification-tests-3476d73d3650814ba70eea6df91c8bbe)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Alexander Brown <drjkl@comfy.org>
|
||
|
pythongosssss
|
4a05d89fdb |
fix: detach DOM widget event listeners on widget removal (#11724)
## Summary Fixes leaked event listeners ## Changes - **What**: - update all listeners to use AbortController to signal removal on widget remove ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11724-fix-detach-DOM-widget-event-listeners-on-widget-removal-3506d73d3650811dae81c034c1098759) by [Unito](https://www.unito.io) --------- Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
Kelly Yang
|
c74e08e244 |
refactor: extract useBrushAdjustment from useBrushDrawing (#11544)
## Summary
Part of the `useBrushDrawing` decomposition plan (PR C). Extracts brush
size/hardness adjustment logic (Alt+drag interaction) into a dedicated
`useBrushAdjustment` composable. No runtime behavior is changed — pure
structural refactor.
## Changes
- **New** `src/composables/maskeditor/useBrushAdjustment.ts` —
encapsulates `startBrushAdjustment` and `handleBrushAdjustment`,
including dead zone filtering, dominant axis suppression, and
size/hardness clamping
- **New** `src/composables/maskeditor/useBrushAdjustment.test.ts` — unit
tests covering: no-op before start, dead zone suppression, size increase
on drag, size/hardness clamping, dominant axis lock
- **Updated** `src/composables/maskeditor/useBrushDrawing.ts` — removes
inlined adjustment state and functions, delegates to
`useBrushAdjustment(initialSettings)`
## Test Functionality
Open ComfyUI and enter the MaskEditor of any image node. On the canvas,
Alt + Right-click Drag:
- Drag Right → Increase brush size - pass
- Drag Left → Decrease brush size - pass
- Drag Up → Increase hardness - pass
- Drag Down → Decrease hardness - pass
https://github.com/user-attachments/assets/273e8383-dab5-4c82-ac7b-0a1534dfd770
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Touches core pointer-interaction logic for brush tuning and changes
adjustment behavior (removes delta saturation and uses initial values),
which could subtly affect UX even though scope is localized to the mask
editor.
>
> **Overview**
> Extracts the Alt-drag brush size/hardness adjustment logic out of
`useBrushDrawing` into a new `useBrushAdjustment` composable, and wires
`useBrushDrawing` to delegate to it.
>
> The extracted logic now bases adjustments off the captured initial
brush size/hardness and removes prior delta capping (no ±100px
saturation), which changes how large/continuous drags affect the final
values. Adds a Vitest suite covering dead-zone behavior, dominant-axis
suppression, clamping, and the no-op-before-start contract.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Benjamin Lu
|
9e16390c33 |
test: assert core command help urls (#11768)
## Summary - Tighten the new `useCoreCommands` help command tests to assert the exact external URL opened for GitHub issues and Discord. ## Testing ```bash pnpm test:unit -- src/composables/useCoreCommands.test.ts pnpm format:check src/composables/useCoreCommands.test.ts ``` Also passed pre-commit `pnpm typecheck` and push hook `pnpm knip`. Stacked on #11748. ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11768-test-assert-core-command-help-urls-3516d73d365081de99d0c71f707d0fb4) by [Unito](https://www.unito.io) --------- Co-authored-by: dante01yoon <bunggl@naver.com> |
||
|
Dante
|
8f011225bf |
test: add unit tests for useCoreCommands canvas/help commands (#11748)
## Summary Adds 8 tests across three new describe blocks for \`src/composables/useCoreCommands.ts\`: - **Canvas view**: \`Comfy.Canvas.ResetView\`, \`Comfy.Canvas.ZoomIn\`, \`Comfy.Canvas.ZoomOut\`. - **Workflow lifecycle**: \`Comfy.OpenClipspace\`, \`Comfy.RefreshNodeDefinitions\`. - **Help**: \`Comfy.Help.OpenComfyUIIssues\`, \`Comfy.Help.OpenComfyOrgDiscord\`, \`Comfy.Help.AboutComfyUI\`. Adds \`vi.hoisted\` mocks for \`useTelemetry\`, \`useSettingsDialog\`, and \`useLitegraphService.resetView\` so they remain isolated from the existing 15-test suite. ## Why this slice \`useCoreCommands.ts\` exports 118 distinct command callbacks (1356 LOC). A single coverage-backfill PR for the whole file would be unwieldy and risk merge conflicts (this file is touched frequently). This PR covers a coherent slice — view/lifecycle/help commands — and follow-up PRs can pick off remaining clusters. ## Testing \`\`\`bash pnpm vitest run src/composables/useCoreCommands.test.ts \`\`\` ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11748-test-add-unit-tests-for-useCoreCommands-canvas-help-commands-3516d73d365081c384ffcc72c15dfd47) by [Unito](https://www.unito.io) |
||
|
AustinMroz
|
1c541d8577 |
Short circuit asset reuploads, simplify node dnd (#11691)
When an output is dragged from the assets panel onto a node, outputs were being reuploaded. This logic has been simplified to instead reference the existing asset by resolving the annotated path. As part of this change, async drop handlers on nodes are also fixed. Rather than placing obligation of event handling on client code, not respecting async handlers, or completely ignoring return types, the vue drop handler will now simply set `app.dragOverNode` and allow the `document` drop handler to resolve node drag/drop operations without any of the difficulty from propagation. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11691-Short-circuit-asset-reuploads-simplify-node-dnd-34f6d73d36508157af86e6cf09229781) by [Unito](https://www.unito.io) --------- Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
Terry Jia
|
fc2a4e82cf |
feat(load3d): bind UI capability gating to ModelAdapterCapabilities (#11711)
> Final piece of the PLY / 3D Gaussian Splatting series. Previous PR made `ModelAdapterCapabilities` load-bearing on the engine side; the UI was still gating off `isSplatModel` / `isPlyModel` proxies. This PR routes the viewer and the viewer-mode dialog through the capability fields directly, so the same source of truth that drives `Load3d` behavior also drives what the user sees. Eighth and last in the series splitting up. ## Summary Snapshot `Load3d.getCurrentModelCapabilities()` into 5 Vue refs on each model load and pipe them through the existing `Load3D` / `Load3DControls` / `Load3dViewerContent` / `ModelControls` / `ViewerModelControls` / `Preview3d` props. Replaces the format-specific `:is-splat-model` / `:is-ply-model` props and the hardcoded "splat → drop light/gizmo/export" subtraction with additive capability gates. No engine behavior changes — capability values are what previous PR already produces; UI now consumes them. ## Changes - **`useLoad3d.ts` / `useLoad3dViewer.ts`**: 5 new refs (`canFitToViewer` / `canUseGizmo` / `canUseLighting` / `canExport` / `materialModes`) refreshed on every load via `load3d.getCurrentModelCapabilities()`. `useLoad3dViewer` extracts the snapshot into a single `captureAdapterFlags(source)` helper because it runs in three places (initializeViewer / initializeStandaloneViewer / loadStandaloneModel). - **`Load3D.vue`**: gate the fit-to-viewer button on `canFitToViewer`; pass capability refs to `Load3DControls` instead of `isSplatModel` / `isPlyModel`. - **`Load3DControls.vue`**: build `availableCategories` additively (`['scene','model','camera']` plus `light` / `gizmo` / `export` if their capability is true) rather than subtracting from a fixed list when `isSplatModel` is true. Forwards `materialModes` to `ModelControls`. - **`Load3dViewerContent.vue`**: gate the light / gizmo / export sidebar sections on the capability refs; pass `materialModes` to `ViewerModelControls`. - **`ModelControls.vue` / `ViewerModelControls.vue`**: drop the local `materialModes` computed (which derived its options from `isPlyModel` and a hardcoded mesh list) and accept `materialModes` as a `readonly MaterialMode[]` prop. An empty array hides the dropdown entirely. - **`Preview3d.vue`** (renderer linearMode): mirror the prop swap on the standalone preview path. ## Review Focus - **Capability prop wiring is the only public-API change for child components**. `ModelControls` and `ViewerModelControls` lost `hideMaterialMode` / `isPlyModel` props. Any extension that imported these components directly will need to migrate, but they're internal `src/components/load3d/controls/**` files and not part of the documented extension surface. - **Empty-`materialModes` semantics**: previously hidden via `:hide-material-mode`; now hidden via `materialModes.length === 0`. `SplatModelAdapter` declares `materialModes: []`, so the splat case keeps the same behavior — the dropdown disappears. PLY adds `'pointCloud'` to the array, so the dropdown picks up that mode automatically without the controls needing an `isPlyModel` branch. - **`captureAdapterFlags` runs after every load completes**, so switching between mesh and splat in the same viewer instance updates the chrome correctly. Verified via the new `Load3D.test.ts` / `Load3dViewerContent.test.ts` cases. - **Capability gating is inclusive of `canFitToViewer`** in this PR even though `Load3DControls` has no fit category — the fit-to-viewer floating button on `Load3D.vue` is what reads it. PLY's `fitToViewer: true` means the button stays visible for PLY users. ## Coverage | File | Stmts | Branch | Funcs | |---|---|---|---| | `Load3D.vue` (modified) | 53.3% | **95.5%** | 83.3% | | `Load3DControls.vue` (modified) | 77.5% | **94.8%** | 86.4% | | `Load3dViewerContent.vue` (modified) | 60.6% | 72.1% | 54.5% | | `controls/ModelControls.vue` (modified) | 16.3% | 0% | 0% | | `controls/viewer/ViewerModelControls.vue` (modified) | **100%** | **100%** | **100%** | | `composables/useLoad3d.ts` (modified) | 78.7% | 64.5% | 71.4% | | `composables/useLoad3dViewer.ts` (modified) | 76.0% | 52.1% | 66.7% | Four new test files (`Load3D.test.ts` / `Load3DControls.test.ts` / `Load3dViewerContent.test.ts` / `controls/viewer/ViewerModelControls.test.ts`) cover the new capability gating directly: each component is rendered with capability flags toggled on/off and the appropriate sidebar / dropdown / button visibility is asserted. Capability prop forwarding from `Load3D.vue` → `Load3DControls.vue` and from `Load3dViewerContent.vue` → `ViewerModelControls.vue` is exercised end-to-end. `controls/ModelControls.vue` is the legacy node-side ModelControls — its existing tests live elsewhere and were not in this PR's scope; the diff line covered (the `v-if="materialModes.length > 0"` swap) is exercised by the new `Load3DControls.test.ts` cases that drive a non-empty / empty `materialModes` through. `Preview3d.vue` (renderer linearMode) has no test file in the project; the prop swap there is the same shape as the `Load3D.vue` swap which is covered. `useLoad3d.ts` / `useLoad3dViewer.ts` percentages are roughly the pre-existing baseline. The diff lines (the 5 new refs and the `captureAdapterFlags` helper) are exercised by the existing composable tests via the mock that now stubs `getCurrentModelCapabilities()`. 73 new component unit tests; 393 total load3d-related tests pass on this branch. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11711-feat-load3d-bind-UI-capability-gating-to-ModelAdapterCapabilities-3506d73d365081b3af68f30e3f728e24) by [Unito](https://www.unito.io) |
||
|
Kelly Yang
|
8b83559402 |
refactor: extract useBrushPersistence from useBrushDrawing (#11543)
## Summary
PR B of this https://github.com/Comfy-Org/ComfyUI_frontend/pull/11388
Part of the `useBrushDrawing` decomposition plan (PR B).
Extracts brush settings persistence logic into a dedicated
`useBrushPersistence` composable, reducing the responsibility surface of
`useBrushDrawing`.
No runtime behavior is changed — this is a pure structural refactor.
## Changes
- **New** `src/composables/maskeditor/useBrushPersistence.ts` —
encapsulates `loadAndApply` (reads brush settings from localStorage and
applies them to the store on init) and `save` (debounced write of
current brush settings to localStorage)
- **New** `src/composables/maskeditor/useBrushPersistence.test.ts` —
unit tests covering load from empty storage, full restore round-trip,
missing `stepSize` fallback, corrupted data resilience, and
save-to-localStorage behavior
- **Updated** `src/composables/maskeditor/useBrushDrawing.ts` — removes
the inlined persistence functions and delegates to `useBrushPersistence`
## Test Locally
1. Adjust brush size and hardness, close MaskEditor, then reopen it —
brush parameters should restore to the previous settings (verifies
`save` + `loadAndApply`) - pass
2. Draw a few strokes and confirm the marks appear correctly (verifies
the `saveBrushSettings` public interface is not broken) - pass
https://github.com/user-attachments/assets/961155d5-6742-4668-a419-51c29b850edf
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Low risk refactor that moves localStorage read/write logic behind a
new composable and adds unit tests; main risk is accidental behavior
drift in when/what brush settings are persisted/restored.
>
> **Overview**
> Refactors mask editor brush settings persistence by extracting the
localStorage load/save (including debounced writes and `stepSize`
fallback) out of `useBrushDrawing` into a new `useBrushPersistence`
composable, while keeping the `saveBrushSettings` public API wired
through.
>
> Adds `useBrushPersistence` unit tests covering empty storage,
round-trip restore, missing field defaults, corrupted JSON handling, and
save semantics.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Dante
|
8c1ea7ae64 |
test: add unit tests for useNodePricing edge cases (#11673)
## Summary
Extends `useNodePricing.test.ts` with three behavioral gaps the existing
suite did not cover: `getNodePricingConfig` does not leak the compiled
JSONata expression, `pricingRevision` ticks after async evaluation
resolves (with a cache-hit path that does not), and
`formatPricingResult` returns `''` for non-finite numeric inputs across
all four result types.
## Changes
- **What**: Adds 9 Vitest cases across two existing `describe` blocks
(`getNodePricingConfig`, `formatPricingResult`) and one new block
(`reactive revision`). Reuses the existing `priceBadge` and
`createMockNodeWithPriceBadge` helpers.
## Review Focus
- The cache-hit assertion checks that `pricingRevision.value` does not
advance after a second `getNodeDisplayPrice` call with the same
signature, exercising the WeakMap cache hit at `useNodePricing.ts:573`.
- Non-finite coverage spans `type:'usd'`, `type:'range_usd'`,
`type:'list_usd'` (empty + all-non-finite + mixed), and the legacy `{
usd }` shape, matching the four `asFiniteNumber` call sites in
`formatPricingResult`.
- The strip-`_compiled` assertion uses `toHaveProperty` so the test
fails loudly if a future refactor accidentally re-exposes the runtime
JSONata instance to debug consumers.
## Testing
\`\`\`bash
pnpm exec vitest run src/composables/node/useNodePricing.test.ts
pnpm format -- src/composables/node/useNodePricing.test.ts
pnpm lint
pnpm typecheck
pnpm knip
\`\`\`
91 tests pass (82 prior + 9 new).
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11673-test-add-unit-tests-for-useNodePricing-edge-cases-34f6d73d365081dab525cdaa71b348a5)
by [Unito](https://www.unito.io)
|
||
|
Kelly Yang
|
88faaf3d86 |
test: complete remaining Painter widget E2E tests (#11613)
## Summary
Implemented E2E test coverage for Levels 6-12 of the Painter Widget
## Changes
Adds the following coverage to complete the test plan:
Level 6 - Input image connection (3 tests, @slow):
- Width/height/bg-color controls hide when input is linked
- Canvas resizes to match input image dimensions after execution
- Drawing over input image produces canvas content
Level 7 - Clear on empty canvas is harmless
Level 8 - Unchanged canvas does not re-upload on second serialization
Level 9 - Settings persistence:
- Tool selection saved to node.properties.painterTool
- Brush size change saved to node.properties.painterBrushSize
Level 10 - Compact layout collapses to grid-cols-1 when node width <
350px
Level 12 - Rapid drawing accumulates all strokes (checks 3 y-positions)
Supporting changes:
- Add data-testid="painter-controls" to controls grid in
WidgetPainter.vue (needed for compact mode class assertion)
- Add browser_tests/assets/widgets/painter_with_input.json workflow
fixture (LoadImage connected to Painter input slot 0)
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Mostly adds/adjusts Playwright and unit test coverage; the only
runtime change is wrapping pointer-capture calls in `try/catch`, which
is low-risk but touches input-handling paths.
>
> **Overview**
> Completes and expands Painter widget browser test coverage, including
new scenarios for clearing an empty canvas, preventing redundant uploads
when serializing an unchanged canvas, persisting tool/brush-size
settings to node properties, compact layout behavior, multi-stroke
accumulation checks, and an input-image-connected workflow (new
`painter_with_input.json`) with execution/resizing/draw-over-image
assertions.
>
> Hardens `usePainter` pointer handling by tolerating
`setPointerCapture`/`releasePointerCapture` failures (e.g., synthetic
events), with corresponding unit tests updated/added to validate the
behavior and serialization expectations.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
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) |
||
|
Christian Byrne
|
56aec1878a |
test: add unit tests for GPUBrushRenderer (#11388)
## Summary Add unit tests for `GPUBrushRenderer`, increasing coverage from ~3.5% to cover constructor initialization, stroke rendering, compositing, preview blitting, readback, and resource cleanup. ## Changes - **What**: 27 unit tests for `GPUBrushRenderer` covering all public methods with comprehensive WebGPU API mocks ## Review Focus Mock factory approach for WebGPU objects — all GPU globals (`GPUBufferUsage`, `GPUTextureUsage`, `GPUShaderStage`) are polyfilled since happy-dom lacks WebGPU support. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11388-test-add-unit-tests-for-GPUBrushRenderer-3476d73d3650814ab0e2c0fb8c424faa) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
b2bba78ce0 |
test: add unit tests for usePanAndZoom composable (#11391)
## Summary Add 29 unit tests for the `usePanAndZoom` composable to improve mask editor test coverage. ## Changes - **What**: New test file `src/composables/maskeditor/usePanAndZoom.test.ts` covering all public API methods ## Review Focus Test coverage spans: - `initializeCanvasPanZoom` — landscape/portrait fit-to-view, panel width accounting, style application - `handlePanStart`/`handlePanMove` — panning state, offset updates, error on missing start - `zoom` — wheel in/out, clamping (0.2–10.0), missing canvas early return, cursor update - `updateCursorPosition` — pan offset application - `invalidatePanZoom` — missing image warning, store container fallback, rgbCanvas dimension sync - Touch handlers — single/two-finger start, double-tap undo, pen blocking, single-touch pan, pinch zoom, touch end states - `addPenPointerId`/`removePenPointerId` — add, dedup, remove, no-op ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11391-test-add-unit-tests-for-usePanAndZoom-composable-3476d73d36508104b447d87471ce021b) by [Unito](https://www.unito.io) --------- Co-authored-by: Terry Jia <terryjia88@gmail.com> Co-authored-by: GitHub Action <action@github.com> |
||
|
Dante
|
c594e30b84 |
test: harden useKeyboard test setup with vi.hoisted and try/finally (#11659)
## Summary - Move `mockCanvasHistory` / `mockStore` into `vi.hoisted()` so the mock state is hoisted before module imports, matching the pattern in `useCanvasTransform.test.ts`. - Wrap the temporary `document.activeElement` override in `try/finally` so the property is restored even if the assertion throws, preventing state leak into subsequent tests. - Fixes #11658 ## Test plan - [x] `pnpm test:unit src/composables/maskeditor/useKeyboard.test.ts` — 17/17 pass - [x] `pnpm typecheck` - [x] `pnpm lint` (no new warnings) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11659-test-harden-useKeyboard-test-setup-with-vi-hoisted-and-try-finally-34f6d73d36508139be2ddc3095ea6952) by [Unito](https://www.unito.io) |
||
|
Terry Jia
|
59ef69f355 |
test: add unit tests for useCoordinateTransform mask editor composable (#11640)
## Summary
Add unit tests for `useCoordinateTransform` mask editor composable,
raising coverage from 2.43% to 100% (statements / branches / functions /
lines).
## Changes
- **What**: Add
`src/composables/maskeditor/useCoordinateTransform.test.ts` (14 tests)
covering both `screenToCanvas` and `canvasToScreen`: identity (display
matches bitmap), uniform downscale (bitmap larger than display),
`pointerZone`-vs-`canvasContainer` offset, non-uniform per-axis scaling,
screen↔canvas round-trip, and the three "element missing" branches
(`pointerZone` / `canvasContainer` / `maskCanvas` null) that should warn
and return `{x:0,y:0}`.
## Review Focus
- Mocked `createSharedComposable` to a pass-through so each test gets a
fresh transform reading the latest `mockStore` refs (otherwise the
shared instance captures stale element references between tests).
- DOM rects are stubbed via `vi.spyOn(el, 'getBoundingClientRect')`
rather than constructing fake DOMRects, so `unref(...)` in the
composable still receives a real `HTMLElement` / `HTMLCanvasElement`.
- Round-trip test (`screenToCanvas` → `canvasToScreen`) verifies the two
functions are mathematical inverses under the offset + scale
combination, which is the actual invariant the rest of the editor relies
on.
- Style aligned with sibling tests: `should ...` naming, `describe`
grouped by public method, explicit `MockStore` type alias, helper
factories `createElementWithRect` / `createCanvasWithRect`.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11640-test-add-unit-tests-for-useCoordinateTransform-mask-editor-composable-34e6d73d3650814d95bdef66e36328e8)
by [Unito](https://www.unito.io)
|
||
|
Terry Jia
|
9ad052467d |
test: add unit tests for useKeyboard mask editor composable (#11639)
## Summary
Add unit tests for `useKeyboard` mask editor composable, raising
coverage from 0% to 100% (statements/lines/functions, 95.65% branch).
## Changes
- **What**: Add `src/composables/maskeditor/useKeyboard.test.ts` (17
tests) covering key tracking (`isKeyDown`), space-key
blur/preventDefault, undo/redo shortcuts (Ctrl/Meta+Z, Ctrl+Shift+Z,
Ctrl+Y), modifier-key edge cases (Alt suppression, no-modifier no-op,
Ctrl+Shift+Y ignored), `window blur` clearing keys, and listener
teardown via `removeListeners`.
## Review Focus
- Mock surface is intentionally minimal — only `useMaskEditorStore` is
mocked because the composable only reaches
`store.canvasHistory.{undo,redo}`.
- `afterEach(keyboard.removeListeners)` is required: the composable
attaches listeners to `document` / `window`, so without teardown earlier
test instances leak handlers and inflate mock call counts in later
tests.
- Tests dispatch real `KeyboardEvent`s via `document.dispatchEvent`
rather than calling the internal handlers directly, so they exercise the
actual `addEventListener` wiring.
- Test style aligned with existing mask editor tests: `should ...`
naming, `describe` grouped by public method, explicit `MockStore` /
`MockCanvasHistory` type aliases.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11639-test-add-unit-tests-for-useKeyboard-mask-editor-composable-34e6d73d36508129b437d0270d9424d8)
by [Unito](https://www.unito.io)
|
||
|
Terry Jia
|
bc16865019 |
test: add unit tests for useToolManager mask editor composable (#11643)
## Summary
Add unit tests for `useToolManager` mask editor composable, raising
coverage from 0% to 100% (statements / functions / lines, 97.53%
branch).
## Changes
- **What**: Add `src/composables/maskeditor/useToolManager.test.ts` (35
tests) covering:
- `switchTool`: store update, layer auto-switch via
`newActiveLayerOnSet`, custom-cursor branch, no-cursor (default
`'none'`) branch, missing-`pointerZone` no-throw guard.
- `setActiveLayer`: rgb-while-mask-only-tool → swap to `PaintPen`,
mask-while-`PaintPen` → swap to `MaskPen`, no-swap path.
- `updateCursor`: same custom-cursor / default-cursor split plus
`brushPreviewGradientVisible = false` post-condition.
- `currentTool` watcher: clears `lastColorSelectPoint` only when leaving
`MaskColorFill`.
- `handlePointerDown`: touch-ignore, pen pointer registration,
middle-button pan, space+left pan, `MaskPen`/`PaintPen` left-button
drawing, `PaintPen` continue-drawing branch (`button !== 0 && buttons
=== 1`), `MaskBucket` flood fill (with coord transform),
`MaskColorFill`, alt+right brush adjustment, right-click drawing for
drawing tools, no-op for non-drawing tools.
- `handlePointerMove`: touch-ignore, cursor position update,
middle-button pan, space+left pan, non-drawing-tool ignore, alt+right
brush adjustment while `isAdjustingBrush`, left/right drag drawing.
- `handlePointerUp`: state cleanup (`isPanning` / `brushVisible` /
`isAdjustingBrush`), pen pointer removal, touch-pointer early bail
before `drawEnd`.
## Review Focus
- Mock store is wrapped in `reactive()` so the `watch(() =>
store.currentTool, ...)` actually fires when tests mutate `currentTool`.
Plain object mocks would silently no-op the watcher branch.
- Each `setup()` runs `useToolManager` inside its own `effectScope`,
stopped in `afterEach`. Without scoping, watchers from previous tests
stay attached to the shared reactive store and accumulate (a single
mutation in test N would call `clearLastColorSelectPoint` N times).
- Mocked `app.extensionManager.setting.get` because `useBrushDrawing`
factory reads two settings synchronously at construction time. The mock
returns deterministic defaults so we don't need `useSettingStore`
plumbing.
- Pointer-event factory builds the minimal shape (`button` / `buttons` /
`pointerType` / `offset*` / `client*` / `altKey` / `pointerId`) — no
jsdom `PointerEvent` constructor noise. `preventDefault` is a `vi.fn()`
because the source calls it unconditionally.
- Style aligned with sibling tests: `should ...` naming, `describe`
grouped by exposed function/watcher, typed `MockStore`, helper
`pointerEvent({ ... })` and `setup()`.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11643-test-add-unit-tests-for-useToolManager-mask-editor-composable-34e6d73d36508184b017ebd04626b29d)
by [Unito](https://www.unito.io)
|
||
|
Terry Jia
|
206a367379 |
test: add unit tests for useMaskEditor composable (#11644)
## Summary
Add unit tests for `useMaskEditor` composable, raising coverage from 0%
to 100% (statements / branches / functions / lines).
## Changes
- **What**: Add `src/composables/maskeditor/useMaskEditor.test.ts` (7
tests) covering `openMaskEditor`:
- Happy path: dialog opened once, `node` forwarded as a prop, header /
content components attached.
- Modal dialog config (`modal` / `maximizable` / `closable` flags)
forwarded to PrimeVue dialog props.
- Acceptance path for nodes with no `imgs` but `previewMediaType ===
'image'`.
- Three guard paths that should log and bail: `node` is null, node with
empty `imgs` and no image preview, node with empty `imgs` and a
non-image preview type (e.g. `'video'`).
## Review Focus
- Mocked `useDialogStore` with a single shared `showDialog` spy — the
only contract under test is "we forwarded these props to the store
action", so instantiating Pinia would just add noise.
- `TopBarHeader.vue` and `MaskEditorContent.vue` are stubbed because
they pull in the full mask-editor render tree; we only assert they're
forwarded as `headerComponent` / `component`, not what they render.
- `console.error` is spied per-test so the bail messages are observable
but don't pollute runner output.
- `nodeWithImage` factory uses a structural `NodeShape` (`{ imgs?,
previewMediaType? }`) rather than `Partial<LGraphNode>` because the real
`LGraphNode` type requires a `LGraphNodeConstructor`-shaped
`constructor` field, which would force every test to construct a full
graph node — irrelevant to the contract being tested.
- Style aligned with sibling tests: `should ...` naming, `describe`
grouped by exposed function (`openMaskEditor`), helper factory.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11644-test-add-unit-tests-for-useMaskEditor-composable-34e6d73d365081e98336db0a92c37ccf)
by [Unito](https://www.unito.io)
|
||
|
Terry Jia
|
b232831441 |
fix: stop duplicate node creation when dropping image on Vue nodes (#11541)
## Summary handleDrop checked `handled === true` to gate stopPropagation, but onDragDrop from useNodeDragAndDrop is async and always returns a Promise, so the check never matched. The drop then bubbled to the document handler in app.ts and spawned a new LoadImage node in addition to the one that accepted the drop. Updated onDragDrop to take an optional claimEvent flag — when true, it calls preventDefault()/stopPropagation() synchronously inside the handler, only after the sync acceptance check passes (valid files / same-origin URI), and before any await. The Vue node now just calls await node.onDragDrop(event, true). Rejected payloads (cross-origin URI, files filtered out, no valid files) skip the claim and bubble to the document fallback as before. The remaining edge case is async URI fetch failures, which we can't sync-detect without speculatively claiming. ## Screenshots (if applicable) before https://github.com/user-attachments/assets/d79a5101-370b-4873-8365-5f9ce188731b after https://github.com/user-attachments/assets/8b787474-eab9-4060-8146-c4d8bb24ff9f ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11541-fix-stop-duplicate-node-creation-when-dropping-image-on-Vue-nodes-34a6d73d36508113b153e31768602933) by [Unito](https://www.unito.io) |
||
|
pythongosssss
|
a6c3ff1a54 |
fix: load3d used wrong i18n key, add test (#11546)
## Summary Toast for Load3D initialization failure was using the wrong key and so showed an untranslated key to the user. ## Changes - **What**: - Update to use correct existing key - Add test that forces init failure ## Screenshots (if applicable) Fixed <img width="482" height="121" alt="image" src="https://github.com/user-attachments/assets/f89eef99-c1a6-463a-a711-7e9c16d0e89a" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11546-fix-load3d-used-wrong-i18n-key-add-test-34a6d73d36508159aab9f042d3e9c4f0) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
Christian Byrne
|
739d4b6136 |
fix: move template distribution filter from v-show to data pipeline (#11418)
*PR Created by the Glary-Bot Agent* --- ## Summary - Moves distribution-based template filtering from a CSS-level `v-show` gate into the `useTemplateFiltering` composable's data pipeline, guaranteeing that templates not meant for the current distribution never reach the view layer - Fixes "Showing 19 of 419" count mismatch when only 2 templates are visible on Cloud with "Wan 2.2" filter active - Derives `availableModels` and `availableUseCases` from distribution-visible templates so filter dropdowns don't show options that only exist on other distributions - Always prunes `activeModels`/`activeUseCases` against available options to prevent stale persisted selections from causing zero-result filtering ## Root Cause The template selector dialog used `v-show="isTemplateVisibleOnDistribution(template)"` to hide templates that don't match the current distribution (cloud/desktop/local). But `filteredCount` and `totalCount` were computed upstream in the pipeline before this visual filter, so the count text showed all matching templates regardless of distribution visibility. ## Changes - **`useTemplateFiltering.ts`**: Added `visibleTemplates` computed that applies distribution filter at the top of the pipeline. All downstream computeds (`fuse`, `availableModels`, `availableUseCases`, `filteredBySearch`, counts) now operate on this distribution-filtered set. `activeModels`/`activeUseCases` always prune against available options. - **`WorkflowTemplateSelectorDialog.vue`**: Passes `distributions` ref to composable, removes `v-show` gate and `isTemplateVisibleOnDistribution` function. - **`useTemplateFiltering.test.ts`**: 10 new unit tests covering distribution filtering, filter composition (search + model + use case + runsOn), stale persisted selections, multi-distribution templates, and Mac distribution. - **`templateFilteringCount.spec.ts`**: 5 new `@cloud` e2e tests verifying count/card consistency, DOM leak prevention, and filter reset behavior with mocked template data. ## Verification - 22 unit tests passing (12 existing + 10 new) - `pnpm typecheck` clean - `pnpm typecheck:browser` clean - `oxlint` + `eslint` clean on all changed files - E2E tests tagged `@cloud` — designed for CI cloud build execution ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11418-fix-move-template-distribution-filter-from-v-show-to-data-pipeline-3476d73d365081c3ba09fc8a42eb4c9b) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> |
||
|
Kelly Yang
|
66409488ce |
Refactor/brush drawing utils (#11531)
## Summary
Phase 1 of this https://github.com/Comfy-Org/ComfyUI_frontend/pull/11388
## Changes
* **`src/composables/maskeditor/brushDrawingUtils.ts` (New)** —
Extracted `premultiplyData`, `formatRgba`, `drawShapeOnContext`,
`createBrushGradient`, `getCachedBrushTexture`, `drawRgbShape`,
`drawMaskShape`, `resetDirtyRect`, and `updateDirtyRect`; also exports
`DirtyRect` / `MaskColor` types.
* **`src/composables/maskeditor/brushDrawingUtils.test.ts` (New)** — 11
unit tests with zero module mocking.
* **`src/composables/maskeditor/useBrushDrawing.ts`** — Replaced logic
with imports; updated all `updateDirtyRect` call sites to use pure
function calls, eliminating redundant calculations in `drawShape`.
## Test locally
1. Draw a few strokes on the canvas — verify brush marks appear
correctly- ok
2. Switch to the eraser tool and erase part of the stroke — verify
erasure works - ok
3. Press Ctrl+Z to undo — verify the canvas state is restored - ok
4. Alt+drag to adjust brush size/hardness — verify the brush parameters
update correctly - ok
https://github.com/user-attachments/assets/ba4ca54d-e1a9-4985-bc46-b996bbf13eee
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Refactors core brush rendering and dirty-rect tracking used during
interactive drawing, so subtle regressions in brush
appearance/performance or cache behavior are possible. Adds new error
paths when brush texture canvas context/radius are invalid.
>
> **Overview**
> Extracts CPU brush rendering utilities into new
`brushDrawingUtils.ts`, including **shape drawing**, **soft brush
gradients/rect textures with an LRU cache**, **alpha
premultiplication**, and **dirty-rect reset/update** helpers.
>
> Updates `useBrushDrawing.ts` to import and use these helpers,
switching dirty-rect tracking to a pure-function style (`dirtyRect.value
= updateDirtyRect(...)`) and simplifying `drawShape` by computing
effective radius/hardness once.
>
> Adds `brushDrawingUtils.test.ts` with focused unit coverage for
premultiplication, dirty-rect bounds behavior, and RGB/mask drawing
paths (including cached soft-rect textures and error handling when a 2D
context can’t be created).
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Dante
|
bea72410fd |
test: add unit tests for utility widgets (#11442)
## Summary Adds 26 unit tests across 3 files covering BatchNavigation, FormSearchInput, and WidgetLayoutField. Part of a widget-test-coverage sequence. ## Changes - **What**: - \`BatchNavigation.test.ts\` (10) — hidden when count ≤ 1, counter formatted as 1-based \`current / total\`, prev/next navigation, disabled states at range boundaries. - \`FormSearchInput.test.ts\` (8) — v-model binding as the user types, clear-button visibility based on trimmed-query, debounced searcher invocation with fake timers (250ms debounce, 1000ms maxWait). - \`WidgetLayoutField.test.ts\` (8) — widget.name vs widget.label preference, empty-name suppression, \`HideLayoutFieldKey\` injection hides label but preserves slot, slot receives \`borderStyle\` scoped prop. ## Review Focus - Fake timers used in FormSearchInput tests for \`refDebounced\` — the debounce assertion depends on the 250ms/1000ms window in the component staying unchanged. - \`HideLayoutFieldKey\` provided via \`global.provide\` using the Symbol key. - No changes to any source component. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11442-test-add-unit-tests-for-utility-widgets-3486d73d365081a891cafe21b09b91c0) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> Co-authored-by: bymyself <cbyrne@comfy.org> |
||
|
Christian Byrne
|
71ca582325 |
fix: reset file input value after selection to allow same-file reupload (#11417)
*PR Created by the Glary-Bot Agent* --- ## Summary Fixes the "choose video to upload" button becoming unresponsive after running a workflow with a subgraph a few times. **Root cause**: The detached input element in `useNodeFileInput` never resets its `value`. The browser's `onchange` only fires when the value *changes* — re-selecting the same file silently drops the event. A page refresh recreates the input with an empty value, which is why refreshing fixes it. ## Changes - `useNodeFileInput.ts`: Reset `fileInput.value` before invoking callbacks so value is cleared even if a callback throws - `useNodeDragAndDrop.ts`: Add `onRemoved` cleanup for installed handlers (only clears own handlers; preserves replacements from extensions) - `useNodePaste.ts`: Add `onRemoved` cleanup for installed `pasteFiles` handler (same reference-safe pattern) - 3 new colocated test files with 26 test cases covering all branches ## Codebase Audit Audited all 11 file upload implementations across the codebase. Found 5 using the ghost/virtual input pattern — 3 with the same missing value-reset bug: - `useNodeFileInput.ts` — fixed in this PR - `scripts/utils.ts` (`uploadFile()`) — one-shot pattern, lower risk - `extensions/core/load3d.ts` — partial reset only The 4 Vue component implementations already reset correctly. ## Future Work VueUse `useFileDialog` composable handles same-file reselection via `reset: true` and provides automatic lifecycle cleanup. A follow-up PR could migrate the ghost input patterns for a centralized solution. ## Test Plan - 26 unit tests across 3 new test files (all pass) - 9 existing useNodeImageUpload tests still pass - Pre-commit hooks pass (oxfmt, oxlint, eslint, typecheck) - Oracle code review addressed ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11417-fix-reset-file-input-value-after-selection-to-allow-same-file-reupload-3476d73d3650814d95efdab602a3852d) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> Co-authored-by: GitHub Action <action@github.com> |