mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-02-19 14:30:07 +00:00
fix: detect video output from data in Nodes 2.0 (#8943)
## Summary - Fixes SaveWebM node showing "Error loading image" in Vue nodes mode - Extracts `isAnimatedOutput`/`isVideoOutput` utility functions from inline logic in `unsafeUpdatePreviews` so both the litegraph canvas renderer and Vue nodes renderer can detect video output directly from execution data - Uses output-based detection in `imagePreviewStore.isImageOutputs` to avoid applying image preview format conversion to video files ## Background In Vue nodes mode, `nodeMedia` relied on `node.previewMediaType` to determine if output is video. This property is only set via `onDrawBackground` → `unsafeUpdatePreviews` in the litegraph canvas path, which doesn't run in Vue nodes mode. This caused webm output to render via `<img>` instead of `<video>`. ## Before https://github.com/user-attachments/assets/36f8a033-0021-4351-8f82-d19e3faa80c2 ## After https://github.com/user-attachments/assets/6558d261-d70e-4968-9637-6c24532e23ac ## Test plan - [x] `pnpm typecheck` passes - [x] `pnpm lint` passes - [x] `pnpm test:unit` passes (4500 tests) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8943-fix-detect-video-output-from-data-in-Vue-nodes-mode-30a6d73d365081e98e91d6d1dcc88785) by [Unito](https://www.unito.io) --------- Co-authored-by: github-actions <github-actions@github.com>
This commit is contained in:
committed by
GitHub
parent
b47414a52f
commit
d3c0e331eb
@@ -0,0 +1,86 @@
|
||||
{
|
||||
"id": "save-image-and-webm-test",
|
||||
"revision": 0,
|
||||
"last_node_id": 12,
|
||||
"last_link_id": 2,
|
||||
"nodes": [
|
||||
{
|
||||
"id": 10,
|
||||
"type": "LoadImage",
|
||||
"pos": [50, 100],
|
||||
"size": [315, 314],
|
||||
"flags": {},
|
||||
"order": 0,
|
||||
"mode": 0,
|
||||
"inputs": [],
|
||||
"outputs": [
|
||||
{
|
||||
"name": "IMAGE",
|
||||
"type": "IMAGE",
|
||||
"links": [1, 2]
|
||||
},
|
||||
{
|
||||
"name": "MASK",
|
||||
"type": "MASK",
|
||||
"links": null
|
||||
}
|
||||
],
|
||||
"properties": {
|
||||
"Node name for S&R": "LoadImage"
|
||||
},
|
||||
"widgets_values": ["example.png", "image"]
|
||||
},
|
||||
{
|
||||
"id": 11,
|
||||
"type": "SaveImage",
|
||||
"pos": [450, 100],
|
||||
"size": [210, 270],
|
||||
"flags": {},
|
||||
"order": 1,
|
||||
"mode": 0,
|
||||
"inputs": [
|
||||
{
|
||||
"name": "images",
|
||||
"type": "IMAGE",
|
||||
"link": 1
|
||||
}
|
||||
],
|
||||
"outputs": [],
|
||||
"properties": {},
|
||||
"widgets_values": ["ComfyUI"]
|
||||
},
|
||||
{
|
||||
"id": 12,
|
||||
"type": "SaveWEBM",
|
||||
"pos": [450, 450],
|
||||
"size": [210, 368],
|
||||
"flags": {},
|
||||
"order": 2,
|
||||
"mode": 0,
|
||||
"inputs": [
|
||||
{
|
||||
"name": "images",
|
||||
"type": "IMAGE",
|
||||
"link": 2
|
||||
}
|
||||
],
|
||||
"outputs": [],
|
||||
"properties": {},
|
||||
"widgets_values": ["ComfyUI", "vp9", 6, 32]
|
||||
}
|
||||
],
|
||||
"links": [
|
||||
[1, 10, 0, 11, 0, "IMAGE"],
|
||||
[2, 10, 0, 12, 0, "IMAGE"]
|
||||
],
|
||||
"groups": [],
|
||||
"config": {},
|
||||
"extra": {
|
||||
"frontendVersion": "1.17.0",
|
||||
"ds": {
|
||||
"offset": [0, 0],
|
||||
"scale": 1
|
||||
}
|
||||
},
|
||||
"version": 0.4
|
||||
}
|
||||
Binary file not shown.
|
Before Width: | Height: | Size: 93 KiB After Width: | Height: | Size: 93 KiB |
42
browser_tests/tests/saveImageAndWebp.spec.ts
Normal file
42
browser_tests/tests/saveImageAndWebp.spec.ts
Normal file
@@ -0,0 +1,42 @@
|
||||
import { expect } from '@playwright/test'
|
||||
|
||||
import { comfyPageFixture as test } from '../fixtures/ComfyPage'
|
||||
|
||||
test.describe(
|
||||
'Save Image and WEBM preview',
|
||||
{ tag: ['@screenshot', '@widget'] },
|
||||
() => {
|
||||
test.beforeEach(async ({ comfyPage }) => {
|
||||
await comfyPage.settings.setSetting('Comfy.VueNodes.Enabled', true)
|
||||
await comfyPage.vueNodes.waitForNodes()
|
||||
})
|
||||
|
||||
test('Can preview both SaveImage and SaveWEBM outputs', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
await comfyPage.workflow.loadWorkflow(
|
||||
'widgets/save_image_and_animated_webp'
|
||||
)
|
||||
await comfyPage.vueNodes.waitForNodes()
|
||||
|
||||
await comfyPage.runButton.click()
|
||||
|
||||
const saveImageNode = comfyPage.vueNodes.getNodeByTitle('Save Image')
|
||||
const saveWebmNode = comfyPage.vueNodes.getNodeByTitle('SaveWEBM')
|
||||
|
||||
// Wait for SaveImage to render an img inside .image-preview
|
||||
await expect(saveImageNode.locator('.image-preview img')).toBeVisible({
|
||||
timeout: 30000
|
||||
})
|
||||
|
||||
// Wait for SaveWEBM to render a video inside .video-preview
|
||||
await expect(saveWebmNode.locator('.video-preview video')).toBeVisible({
|
||||
timeout: 30000
|
||||
})
|
||||
|
||||
await expect(comfyPage.page).toHaveScreenshot(
|
||||
'save-image-and-webm-preview.png'
|
||||
)
|
||||
})
|
||||
}
|
||||
)
|
||||
Binary file not shown.
|
After Width: | Height: | Size: 98 KiB |
Binary file not shown.
|
After Width: | Height: | Size: 89 KiB |
@@ -250,6 +250,7 @@ import { useExecutionStore } from '@/stores/executionStore'
|
||||
import { useNodeOutputStore } from '@/stores/imagePreviewStore'
|
||||
import { useRightSidePanelStore } from '@/stores/workspace/rightSidePanelStore'
|
||||
import { isTransparent } from '@/utils/colorUtil'
|
||||
import { isVideoOutput } from '@/utils/litegraphUtil'
|
||||
import {
|
||||
getLocatorIdFromNodeData,
|
||||
getNodeByLocatorId
|
||||
@@ -663,6 +664,7 @@ const nodeMedia = computed(() => {
|
||||
if (!urls?.length) return undefined
|
||||
|
||||
const type =
|
||||
isVideoOutput(newOutputs) ||
|
||||
node.previewMediaType === 'video' ||
|
||||
(!node.previewMediaType && hasVideoInput.value)
|
||||
? 'video'
|
||||
|
||||
@@ -56,8 +56,10 @@ import { useRightSidePanelStore } from '@/stores/workspace/rightSidePanelStore'
|
||||
import { useWidgetStore } from '@/stores/widgetStore'
|
||||
import { normalizeI18nKey } from '@/utils/formatUtil'
|
||||
import {
|
||||
isAnimatedOutput,
|
||||
isImageNode,
|
||||
isVideoNode,
|
||||
isVideoOutput,
|
||||
migrateWidgetsValues
|
||||
} from '@/utils/litegraphUtil'
|
||||
import { getOrderedInputSpecs } from '@/workbench/utils/nodeDefOrderingUtil'
|
||||
@@ -753,17 +755,9 @@ export const useLitegraphService = () => {
|
||||
if (isNewOutput) this.images = output.images
|
||||
|
||||
if (isNewOutput || isNewPreview) {
|
||||
this.animatedImages = output?.animated?.find(Boolean)
|
||||
this.animatedImages = isAnimatedOutput(output)
|
||||
|
||||
const isAnimatedWebp =
|
||||
this.animatedImages &&
|
||||
output?.images?.some((img) => img.filename?.includes('webp'))
|
||||
const isAnimatedPng =
|
||||
this.animatedImages &&
|
||||
output?.images?.some((img) => img.filename?.includes('png'))
|
||||
const isVideo =
|
||||
(this.animatedImages && !isAnimatedWebp && !isAnimatedPng) ||
|
||||
isVideoNode(this)
|
||||
const isVideo = isVideoOutput(output) || isVideoNode(this)
|
||||
if (isVideo) {
|
||||
useNodeVideo(this, callback).showPreview()
|
||||
} else {
|
||||
|
||||
@@ -9,6 +9,7 @@ import { useNodeOutputStore } from '@/stores/imagePreviewStore'
|
||||
import * as litegraphUtil from '@/utils/litegraphUtil'
|
||||
|
||||
vi.mock('@/utils/litegraphUtil', () => ({
|
||||
isAnimatedOutput: vi.fn(),
|
||||
isVideoNode: vi.fn()
|
||||
}))
|
||||
|
||||
@@ -150,13 +151,14 @@ describe('imagePreviewStore getPreviewParam', () => {
|
||||
beforeEach(() => {
|
||||
setActivePinia(createTestingPinia({ stubActions: false }))
|
||||
vi.clearAllMocks()
|
||||
vi.mocked(litegraphUtil.isAnimatedOutput).mockReturnValue(false)
|
||||
vi.mocked(litegraphUtil.isVideoNode).mockReturnValue(false)
|
||||
})
|
||||
|
||||
it('should return empty string if node.animatedImages is true', () => {
|
||||
it('should return empty string if output is animated', () => {
|
||||
const store = useNodeOutputStore()
|
||||
// @ts-expect-error `animatedImages` property is not typed
|
||||
const node = createMockNode({ animatedImages: true })
|
||||
vi.mocked(litegraphUtil.isAnimatedOutput).mockReturnValue(true)
|
||||
const node = createMockNode()
|
||||
const outputs = createMockOutputs([{ filename: 'img.png' }])
|
||||
expect(store.getPreviewParam(node, outputs)).toBe('')
|
||||
expect(vi.mocked(app).getPreviewFormatParam).not.toHaveBeenCalled()
|
||||
|
||||
@@ -14,7 +14,7 @@ import { app } from '@/scripts/app'
|
||||
import { useExecutionStore } from '@/stores/executionStore'
|
||||
import type { NodeLocatorId } from '@/types/nodeIdentification'
|
||||
import { parseFilePath } from '@/utils/formatUtil'
|
||||
import { isVideoNode } from '@/utils/litegraphUtil'
|
||||
import { isAnimatedOutput, isVideoNode } from '@/utils/litegraphUtil'
|
||||
import {
|
||||
releaseSharedObjectUrl,
|
||||
retainSharedObjectUrl
|
||||
@@ -83,7 +83,7 @@ export const useNodeOutputStore = defineStore('nodeOutput', () => {
|
||||
outputs: ExecutedWsMessage['output']
|
||||
): boolean => {
|
||||
// If animated webp/png or video outputs, return false
|
||||
if (node.animatedImages || isVideoNode(node)) return false
|
||||
if (isAnimatedOutput(outputs) || isVideoNode(node)) return false
|
||||
|
||||
// If no images, return false
|
||||
if (!outputs?.images?.length) return false
|
||||
|
||||
@@ -9,9 +9,12 @@ import type {
|
||||
import type { ISerialisedGraph } from '@/lib/litegraph/src/types/serialisation'
|
||||
import type { IWidget } from '@/lib/litegraph/src/types/widgets'
|
||||
import type { InputSpec } from '@/schemas/nodeDef/nodeDefSchemaV2'
|
||||
import type { ExecutedWsMessage } from '@/schemas/apiSchema'
|
||||
import {
|
||||
compressWidgetInputSlots,
|
||||
createNode,
|
||||
isAnimatedOutput,
|
||||
isVideoOutput,
|
||||
migrateWidgetsValues
|
||||
} from '@/utils/litegraphUtil'
|
||||
|
||||
@@ -199,6 +202,106 @@ describe('migrateWidgetsValues', () => {
|
||||
})
|
||||
})
|
||||
|
||||
function createOutput(
|
||||
overrides: Partial<ExecutedWsMessage['output']> = {}
|
||||
): ExecutedWsMessage['output'] {
|
||||
return { ...overrides }
|
||||
}
|
||||
|
||||
describe('isAnimatedOutput', () => {
|
||||
it('returns false for undefined output', () => {
|
||||
expect(isAnimatedOutput(undefined)).toBe(false)
|
||||
})
|
||||
|
||||
it('returns false when animated array is missing', () => {
|
||||
expect(isAnimatedOutput(createOutput())).toBe(false)
|
||||
})
|
||||
|
||||
it('returns false when all animated values are false', () => {
|
||||
expect(isAnimatedOutput(createOutput({ animated: [false, false] }))).toBe(
|
||||
false
|
||||
)
|
||||
})
|
||||
|
||||
it('returns true when any animated value is true', () => {
|
||||
expect(isAnimatedOutput(createOutput({ animated: [false, true] }))).toBe(
|
||||
true
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe('isVideoOutput', () => {
|
||||
it('returns false for non-animated output', () => {
|
||||
expect(
|
||||
isVideoOutput(
|
||||
createOutput({
|
||||
animated: [false],
|
||||
images: [{ filename: 'video.webm' }]
|
||||
})
|
||||
)
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('returns false for animated webp output', () => {
|
||||
expect(
|
||||
isVideoOutput(
|
||||
createOutput({
|
||||
animated: [true],
|
||||
images: [{ filename: 'anim.webp' }]
|
||||
})
|
||||
)
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('returns false for animated png output', () => {
|
||||
expect(
|
||||
isVideoOutput(
|
||||
createOutput({
|
||||
animated: [true],
|
||||
images: [{ filename: 'anim.png' }]
|
||||
})
|
||||
)
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('returns true for animated webm output', () => {
|
||||
expect(
|
||||
isVideoOutput(
|
||||
createOutput({
|
||||
animated: [true],
|
||||
images: [{ filename: 'output.webm' }]
|
||||
})
|
||||
)
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
it('returns true for animated mp4 output', () => {
|
||||
expect(
|
||||
isVideoOutput(
|
||||
createOutput({
|
||||
animated: [true],
|
||||
images: [{ filename: 'output.mp4' }]
|
||||
})
|
||||
)
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
it('returns true for animated output with no images array', () => {
|
||||
expect(isVideoOutput(createOutput({ animated: [true] }))).toBe(true)
|
||||
})
|
||||
|
||||
it('does not false-positive on filenames containing webp as substring', () => {
|
||||
expect(
|
||||
isVideoOutput(
|
||||
createOutput({
|
||||
animated: [true],
|
||||
images: [{ filename: 'my_webp_file.mp4' }]
|
||||
})
|
||||
)
|
||||
).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe('compressWidgetInputSlots', () => {
|
||||
it('should remove unconnected widget input slots', () => {
|
||||
// Using partial mock - only including properties needed for test
|
||||
|
||||
@@ -5,6 +5,7 @@ import type {
|
||||
LGraph,
|
||||
LGraphCanvas
|
||||
} from '@/lib/litegraph/src/litegraph'
|
||||
import type { ExecutedWsMessage } from '@/schemas/apiSchema'
|
||||
import {
|
||||
LGraphGroup,
|
||||
LGraphNode,
|
||||
@@ -77,6 +78,32 @@ export function isVideoNode(node: LGraphNode | undefined): node is VideoNode {
|
||||
return node.previewMediaType === 'video' || !!node.videoContainer
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if output data indicates animated content (animated webp/png or video).
|
||||
*/
|
||||
export function isAnimatedOutput(
|
||||
output: ExecutedWsMessage['output'] | undefined
|
||||
): boolean {
|
||||
return !!output?.animated?.find(Boolean)
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if output data indicates video content (animated but not webp/png).
|
||||
*/
|
||||
export function isVideoOutput(
|
||||
output: ExecutedWsMessage['output'] | undefined
|
||||
): boolean {
|
||||
if (!isAnimatedOutput(output)) return false
|
||||
|
||||
const isAnimatedWebp = output?.images?.some((img) =>
|
||||
img.filename?.endsWith('.webp')
|
||||
)
|
||||
const isAnimatedPng = output?.images?.some((img) =>
|
||||
img.filename?.endsWith('.png')
|
||||
)
|
||||
return !isAnimatedWebp && !isAnimatedPng
|
||||
}
|
||||
|
||||
export function isAudioNode(node: LGraphNode | undefined): boolean {
|
||||
return !!node && node.previewMediaType === 'audio'
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user