mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-01-26 19:09:52 +00:00
fix: remove LOD from vue nodes (#6950)
## Summary Refactor to remove LOD from vue nodes. Also, hide Litegraph LOD settings in vue nodes to prevent confusion / stale setting. Keep litegraph LOD the exact same. ## Changes - **What**: settings, all LOD related code in vue nodes ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-6950-fix-remove-LOD-from-vue-nodes-2b76d73d365081568723f8cbc7be7e17) by [Unito](https://www.unito.io) --------- Co-authored-by: github-actions <github-actions@github.com>
This commit is contained in:
Binary file not shown.
|
Before Width: | Height: | Size: 34 KiB After Width: | Height: | Size: 36 KiB |
@@ -1,49 +0,0 @@
|
||||
import { expect } from '@playwright/test'
|
||||
|
||||
import { comfyPageFixture as test } from '../../../fixtures/ComfyPage'
|
||||
|
||||
test.beforeEach(async ({ comfyPage }) => {
|
||||
await comfyPage.setSetting('Comfy.UseNewMenu', 'Disabled')
|
||||
})
|
||||
|
||||
test.describe('Vue Nodes - LOD', () => {
|
||||
test.beforeEach(async ({ comfyPage }) => {
|
||||
await comfyPage.setSetting('Comfy.VueNodes.Enabled', true)
|
||||
await comfyPage.setup()
|
||||
await comfyPage.loadWorkflow('default')
|
||||
await comfyPage.setSetting('LiteGraph.Canvas.MinFontSizeForLOD', 8)
|
||||
})
|
||||
|
||||
test('should toggle LOD based on zoom threshold', async ({ comfyPage }) => {
|
||||
await comfyPage.vueNodes.waitForNodes()
|
||||
|
||||
const initialNodeCount = await comfyPage.vueNodes.getNodeCount()
|
||||
expect(initialNodeCount).toBeGreaterThan(0)
|
||||
|
||||
await expect(comfyPage.canvas).toHaveScreenshot('vue-nodes-default.png')
|
||||
|
||||
const vueNodesContainer = comfyPage.vueNodes.nodes
|
||||
const textboxesInNodes = vueNodesContainer.getByRole('textbox')
|
||||
const comboboxesInNodes = vueNodesContainer.getByRole('combobox')
|
||||
|
||||
await expect(textboxesInNodes.first()).toBeVisible()
|
||||
await expect(comboboxesInNodes.first()).toBeVisible()
|
||||
|
||||
await comfyPage.zoom(120, 10)
|
||||
await comfyPage.nextFrame()
|
||||
|
||||
await expect(comfyPage.canvas).toHaveScreenshot('vue-nodes-lod-active.png')
|
||||
|
||||
await expect(textboxesInNodes.first()).toBeHidden()
|
||||
await expect(comboboxesInNodes.first()).toBeHidden()
|
||||
|
||||
await comfyPage.zoom(-120, 10)
|
||||
await comfyPage.nextFrame()
|
||||
|
||||
await expect(comfyPage.canvas).toHaveScreenshot(
|
||||
'vue-nodes-lod-inactive.png'
|
||||
)
|
||||
await expect(textboxesInNodes.first()).toBeVisible()
|
||||
await expect(comboboxesInNodes.first()).toBeVisible()
|
||||
})
|
||||
})
|
||||
Binary file not shown.
|
Before Width: | Height: | Size: 81 KiB After Width: | Height: | Size: 81 KiB |
@@ -1329,57 +1329,6 @@ audio.comfy-audio.empty-audio-widget {
|
||||
will-change: transform;
|
||||
}
|
||||
|
||||
/* START LOD specific styles */
|
||||
/* LOD styles - Custom CSS avoids 100+ Tailwind selectors that would slow style recalculation when .isLOD toggles */
|
||||
|
||||
.isLOD .lg-node {
|
||||
box-shadow: none;
|
||||
filter: none;
|
||||
backdrop-filter: none;
|
||||
text-shadow: none;
|
||||
mask-image: none;
|
||||
clip-path: none;
|
||||
background-image: none;
|
||||
text-rendering: optimizeSpeed;
|
||||
border-radius: 0;
|
||||
contain: layout style;
|
||||
transition: none;
|
||||
}
|
||||
|
||||
.isLOD .lg-node-header {
|
||||
border-radius: 0;
|
||||
pointer-events: none;
|
||||
}
|
||||
|
||||
.isLOD .lg-node-widgets {
|
||||
pointer-events: none;
|
||||
}
|
||||
|
||||
.lod-toggle {
|
||||
visibility: visible;
|
||||
}
|
||||
|
||||
.isLOD .lod-toggle {
|
||||
visibility: hidden;
|
||||
}
|
||||
|
||||
.lod-fallback {
|
||||
display: none;
|
||||
}
|
||||
|
||||
.isLOD .lod-fallback {
|
||||
display: block;
|
||||
}
|
||||
|
||||
.isLOD .image-preview img {
|
||||
image-rendering: pixelated;
|
||||
}
|
||||
|
||||
.isLOD .slot-dot {
|
||||
border-radius: 0;
|
||||
}
|
||||
/* END LOD specific styles */
|
||||
|
||||
/* ===================== Mask Editor Styles ===================== */
|
||||
/* To be migrated to Tailwind later */
|
||||
#maskEditor_brush {
|
||||
|
||||
@@ -3,7 +3,6 @@ import { shallowRef, watch } from 'vue'
|
||||
|
||||
import { useGraphNodeManager } from '@/composables/graph/useGraphNodeManager'
|
||||
import type { GraphNodeManager } from '@/composables/graph/useGraphNodeManager'
|
||||
import { useRenderModeSetting } from '@/composables/settings/useRenderModeSetting'
|
||||
import { useVueFeatureFlags } from '@/composables/useVueFeatureFlags'
|
||||
import { useVueNodesMigrationDismissed } from '@/composables/useVueNodesMigrationDismissed'
|
||||
import type { LGraphNode } from '@/lib/litegraph/src/litegraph'
|
||||
@@ -26,11 +25,6 @@ function useVueNodeLifecycleIndividual() {
|
||||
|
||||
let hasShownMigrationToast = false
|
||||
|
||||
useRenderModeSetting(
|
||||
{ setting: 'LiteGraph.Canvas.MinFontSizeForLOD', vue: 0, litegraph: 8 },
|
||||
shouldRenderVueNodes
|
||||
)
|
||||
|
||||
const initializeNodeManager = () => {
|
||||
// Use canvas graph if available (handles subgraph contexts), fallback to app graph
|
||||
const activeGraph = comfyApp.canvas?.graph
|
||||
|
||||
@@ -1,42 +0,0 @@
|
||||
import type { ComputedRef } from 'vue'
|
||||
import { ref, watch } from 'vue'
|
||||
|
||||
import { useSettingStore } from '@/platform/settings/settingStore'
|
||||
import type { Settings } from '@/schemas/apiSchema'
|
||||
|
||||
interface RenderModeSettingConfig<TSettingKey extends keyof Settings> {
|
||||
setting: TSettingKey
|
||||
vue: Settings[TSettingKey]
|
||||
litegraph: Settings[TSettingKey]
|
||||
}
|
||||
|
||||
export function useRenderModeSetting<TSettingKey extends keyof Settings>(
|
||||
config: RenderModeSettingConfig<TSettingKey>,
|
||||
isVueMode: ComputedRef<boolean>
|
||||
) {
|
||||
const settingStore = useSettingStore()
|
||||
const vueValue = ref(config.vue)
|
||||
const litegraphValue = ref(config.litegraph)
|
||||
const lastWasVue = ref<boolean | null>(null)
|
||||
|
||||
const load = async (vue: boolean) => {
|
||||
if (lastWasVue.value === vue) return
|
||||
|
||||
if (lastWasVue.value !== null) {
|
||||
const currentValue = settingStore.get(config.setting)
|
||||
if (lastWasVue.value) {
|
||||
vueValue.value = currentValue
|
||||
} else {
|
||||
litegraphValue.value = currentValue
|
||||
}
|
||||
}
|
||||
|
||||
await settingStore.set(
|
||||
config.setting,
|
||||
vue ? vueValue.value : litegraphValue.value
|
||||
)
|
||||
lastWasVue.value = vue
|
||||
}
|
||||
|
||||
watch(isVueMode, load, { immediate: true })
|
||||
}
|
||||
@@ -8,9 +8,11 @@ import {
|
||||
} from '@/platform/settings/settingStore'
|
||||
import type { ISettingGroup, SettingParams } from '@/platform/settings/types'
|
||||
import { normalizeI18nKey } from '@/utils/formatUtil'
|
||||
import { useVueFeatureFlags } from '@/composables/useVueFeatureFlags'
|
||||
|
||||
export function useSettingSearch() {
|
||||
const settingStore = useSettingStore()
|
||||
const { shouldRenderVueNodes } = useVueFeatureFlags()
|
||||
|
||||
const searchQuery = ref<string>('')
|
||||
const filteredSettingIds = ref<string[]>([])
|
||||
@@ -54,7 +56,11 @@ export function useSettingSearch() {
|
||||
const allSettings = Object.values(settingStore.settingsById)
|
||||
const filteredSettings = allSettings.filter((setting) => {
|
||||
// Filter out hidden and deprecated settings, just like in normal settings tree
|
||||
if (setting.type === 'hidden' || setting.deprecated) {
|
||||
if (
|
||||
setting.type === 'hidden' ||
|
||||
setting.deprecated ||
|
||||
(shouldRenderVueNodes.value && setting.hideInVueNodes)
|
||||
) {
|
||||
return false
|
||||
}
|
||||
|
||||
|
||||
@@ -10,6 +10,7 @@ import type { SettingParams } from '@/platform/settings/types'
|
||||
import { isElectron } from '@/utils/envUtil'
|
||||
import { normalizeI18nKey } from '@/utils/formatUtil'
|
||||
import { buildTree } from '@/utils/treeUtil'
|
||||
import { useVueFeatureFlags } from '@/composables/useVueFeatureFlags'
|
||||
|
||||
interface SettingPanelItem {
|
||||
node: SettingTreeNode
|
||||
@@ -31,10 +32,14 @@ export function useSettingUI(
|
||||
const settingStore = useSettingStore()
|
||||
const activeCategory = ref<SettingTreeNode | null>(null)
|
||||
|
||||
const { shouldRenderVueNodes } = useVueFeatureFlags()
|
||||
|
||||
const settingRoot = computed<SettingTreeNode>(() => {
|
||||
const root = buildTree(
|
||||
Object.values(settingStore.settingsById).filter(
|
||||
(setting: SettingParams) => setting.type !== 'hidden'
|
||||
(setting: SettingParams) =>
|
||||
setting.type !== 'hidden' &&
|
||||
!(shouldRenderVueNodes.value && setting.hideInVueNodes)
|
||||
),
|
||||
(setting: SettingParams) => setting.category || setting.id.split('.')
|
||||
)
|
||||
|
||||
@@ -919,7 +919,8 @@ export const CORE_SETTINGS: SettingParams[] = [
|
||||
step: 1
|
||||
},
|
||||
defaultValue: 8,
|
||||
versionAdded: '1.26.7'
|
||||
versionAdded: '1.26.7',
|
||||
hideInVueNodes: true
|
||||
},
|
||||
{
|
||||
id: 'Comfy.Canvas.SelectionToolbox',
|
||||
|
||||
@@ -47,6 +47,7 @@ export interface SettingParams<TValue = unknown> extends FormItem {
|
||||
// sortOrder for sorting settings within a group. Higher values appear first.
|
||||
// Default is 0 if not specified.
|
||||
sortOrder?: number
|
||||
hideInVueNodes?: boolean
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -29,12 +29,6 @@ vi.mock('@/renderer/core/layout/transform/useTransformState', () => {
|
||||
}
|
||||
})
|
||||
|
||||
vi.mock('@/renderer/extensions/vueNodes/lod/useLOD', () => ({
|
||||
useLOD: vi.fn(() => ({
|
||||
isLOD: false
|
||||
}))
|
||||
}))
|
||||
|
||||
function createMockCanvas(): LGraphCanvas {
|
||||
return {
|
||||
canvas: {
|
||||
|
||||
@@ -4,8 +4,7 @@
|
||||
:class="
|
||||
cn(
|
||||
'absolute inset-0 w-full h-full pointer-events-none',
|
||||
isInteracting ? 'transform-pane--interacting' : 'will-change-auto',
|
||||
isLOD && 'isLOD'
|
||||
isInteracting ? 'transform-pane--interacting' : 'will-change-auto'
|
||||
)
|
||||
"
|
||||
:style="transformStyle"
|
||||
@@ -22,7 +21,6 @@ import { computed } from 'vue'
|
||||
import type { LGraphCanvas } from '@/lib/litegraph/src/litegraph'
|
||||
import { useTransformSettling } from '@/renderer/core/layout/transform/useTransformSettling'
|
||||
import { useTransformState } from '@/renderer/core/layout/transform/useTransformState'
|
||||
import { useLOD } from '@/renderer/extensions/vueNodes/lod/useLOD'
|
||||
import { cn } from '@/utils/tailwindUtil'
|
||||
|
||||
interface TransformPaneProps {
|
||||
@@ -31,9 +29,7 @@ interface TransformPaneProps {
|
||||
|
||||
const props = defineProps<TransformPaneProps>()
|
||||
|
||||
const { camera, transformStyle, syncWithCanvas } = useTransformState()
|
||||
|
||||
const { isLOD } = useLOD(camera)
|
||||
const { transformStyle, syncWithCanvas } = useTransformState()
|
||||
|
||||
const canvasElement = computed(() => props.canvas?.canvas)
|
||||
const { isTransforming: isInteracting } = useTransformSettling(canvasElement, {
|
||||
|
||||
@@ -83,20 +83,17 @@
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<div class="relative">
|
||||
<!-- Video Dimensions -->
|
||||
<div class="mt-2 text-center text-xs text-white">
|
||||
<span v-if="videoError" class="text-red-400">
|
||||
{{ $t('g.errorLoadingVideo') }}
|
||||
</span>
|
||||
<span v-else-if="isLoading" class="text-smoke-400">
|
||||
{{ $t('g.loading') }}...
|
||||
</span>
|
||||
<span v-else>
|
||||
{{ actualDimensions || $t('g.calculatingDimensions') }}
|
||||
</span>
|
||||
</div>
|
||||
<LODFallback />
|
||||
<!-- Video Dimensions -->
|
||||
<div class="mt-2 text-center text-xs text-white">
|
||||
<span v-if="videoError" class="text-red-400">
|
||||
{{ $t('g.errorLoadingVideo') }}
|
||||
</span>
|
||||
<span v-else-if="isLoading" class="text-smoke-400">
|
||||
{{ $t('g.loading') }}...
|
||||
</span>
|
||||
<span v-else>
|
||||
{{ actualDimensions || $t('g.calculatingDimensions') }}
|
||||
</span>
|
||||
</div>
|
||||
</div>
|
||||
</template>
|
||||
@@ -110,8 +107,6 @@ import { useI18n } from 'vue-i18n'
|
||||
import { downloadFile } from '@/base/common/downloadUtil'
|
||||
import { useNodeOutputStore } from '@/stores/imagePreviewStore'
|
||||
|
||||
import LODFallback from './components/LODFallback.vue'
|
||||
|
||||
interface VideoPreviewProps {
|
||||
/** Array of video URLs to display */
|
||||
readonly imageUrls: readonly string[] // Named imageUrls for consistency with parent components
|
||||
|
||||
@@ -93,20 +93,17 @@
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<div class="relative">
|
||||
<!-- Image Dimensions -->
|
||||
<div class="mt-2 text-center text-xs text-white">
|
||||
<span v-if="imageError" class="text-red-400">
|
||||
{{ $t('g.errorLoadingImage') }}
|
||||
</span>
|
||||
<span v-else-if="isLoading" class="text-smoke-400">
|
||||
{{ $t('g.loading') }}...
|
||||
</span>
|
||||
<span v-else>
|
||||
{{ actualDimensions || $t('g.calculatingDimensions') }}
|
||||
</span>
|
||||
</div>
|
||||
<LODFallback />
|
||||
<!-- Image Dimensions -->
|
||||
<div class="mt-2 text-center text-xs text-white">
|
||||
<span v-if="imageError" class="text-red-400">
|
||||
{{ $t('g.errorLoadingImage') }}
|
||||
</span>
|
||||
<span v-else-if="isLoading" class="text-smoke-400">
|
||||
{{ $t('g.loading') }}...
|
||||
</span>
|
||||
<span v-else>
|
||||
{{ actualDimensions || $t('g.calculatingDimensions') }}
|
||||
</span>
|
||||
</div>
|
||||
</div>
|
||||
</template>
|
||||
@@ -122,8 +119,6 @@ import { app } from '@/scripts/app'
|
||||
import { useCommandStore } from '@/stores/commandStore'
|
||||
import { useNodeOutputStore } from '@/stores/imagePreviewStore'
|
||||
|
||||
import LODFallback from './LODFallback.vue'
|
||||
|
||||
interface ImagePreviewProps {
|
||||
/** Array of image URLs to display */
|
||||
readonly imageUrls: readonly string[]
|
||||
|
||||
@@ -10,14 +10,13 @@
|
||||
/>
|
||||
|
||||
<!-- Slot Name -->
|
||||
<div class="relative h-full flex items-center min-w-0">
|
||||
<div class="h-full flex items-center min-w-0">
|
||||
<span
|
||||
v-if="!dotOnly"
|
||||
:class="cn('truncate text-xs font-normal lod-toggle', labelClasses)"
|
||||
:class="cn('truncate text-xs font-normal', labelClasses)"
|
||||
>
|
||||
{{ slotData.localized_name || slotData.name || `Input ${index}` }}
|
||||
</span>
|
||||
<LODFallback />
|
||||
</div>
|
||||
</div>
|
||||
</template>
|
||||
@@ -37,7 +36,6 @@ import { useSlotLinkInteraction } from '@/renderer/extensions/vueNodes/composabl
|
||||
import { useExecutionStore } from '@/stores/executionStore'
|
||||
import { cn } from '@/utils/tailwindUtil'
|
||||
|
||||
import LODFallback from './LODFallback.vue'
|
||||
import SlotConnectionDot from './SlotConnectionDot.vue'
|
||||
|
||||
interface InputSlotProps {
|
||||
|
||||
@@ -99,18 +99,14 @@
|
||||
/>
|
||||
</div>
|
||||
|
||||
<!-- Node Body - rendered based on LOD level and collapsed state -->
|
||||
<div
|
||||
class="flex flex-1 flex-col gap-1 pb-2"
|
||||
:data-testid="`node-body-${nodeData.id}`"
|
||||
>
|
||||
<!-- Slots only rendered at full detail -->
|
||||
<NodeSlots :node-data="nodeData" />
|
||||
|
||||
<!-- Widgets rendered at reduced+ detail -->
|
||||
<NodeWidgets v-if="nodeData.widgets?.length" :node-data="nodeData" />
|
||||
|
||||
<!-- Custom content at reduced+ detail -->
|
||||
<div v-if="hasCustomContent" class="min-h-0 flex-1 flex">
|
||||
<NodeContent :node-data="nodeData" :media="nodeMedia" />
|
||||
</div>
|
||||
|
||||
@@ -1,5 +0,0 @@
|
||||
<template>
|
||||
<div
|
||||
class="lod-fallback absolute inset-0 h-full w-full bg-node-component-widget-skeleton-surface"
|
||||
></div>
|
||||
</template>
|
||||
@@ -18,7 +18,7 @@
|
||||
<div class="flex items-center justify-between gap-2.5 min-w-0">
|
||||
<!-- Collapse/Expand Button -->
|
||||
<div class="relative grow-1 flex items-center gap-2.5 min-w-0 flex-1">
|
||||
<div class="lod-toggle flex shrink-0 items-center px-0.5">
|
||||
<div class="flex shrink-0 items-center px-0.5">
|
||||
<IconButton
|
||||
size="fit-content"
|
||||
type="transparent"
|
||||
@@ -44,7 +44,7 @@
|
||||
<!-- Node Title -->
|
||||
<div
|
||||
v-tooltip.top="tooltipConfig"
|
||||
class="lod-toggle flex min-w-0 flex-1 items-center gap-2 text-sm font-bold"
|
||||
class="flex min-w-0 flex-1 items-center gap-2 text-sm font-bold"
|
||||
data-testid="node-title"
|
||||
>
|
||||
<div class="truncate min-w-0 flex-1">
|
||||
@@ -57,10 +57,9 @@
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
<LODFallback />
|
||||
</div>
|
||||
|
||||
<div class="lod-toggle flex shrink-0 items-center justify-between gap-2">
|
||||
<div class="flex shrink-0 items-center justify-between gap-2">
|
||||
<NodeBadge
|
||||
v-for="badge of nodeBadges"
|
||||
:key="badge.text"
|
||||
@@ -112,7 +111,6 @@ import {
|
||||
} from '@/utils/graphTraversalUtil'
|
||||
import { cn } from '@/utils/tailwindUtil'
|
||||
|
||||
import LODFallback from './LODFallback.vue'
|
||||
import type { NodeBadgeProps } from './NodeBadge.vue'
|
||||
|
||||
interface NodeHeaderProps {
|
||||
|
||||
@@ -5,11 +5,10 @@
|
||||
<!-- Slot Name -->
|
||||
<span
|
||||
v-if="!dotOnly"
|
||||
class="lod-toggle text-xs font-normal truncate text-node-component-slot-text"
|
||||
class="text-xs font-normal truncate text-node-component-slot-text"
|
||||
>
|
||||
{{ slotData.localized_name || slotData.name || `Output ${index}` }}
|
||||
</span>
|
||||
<LODFallback />
|
||||
</div>
|
||||
<!-- Connection Dot -->
|
||||
<SlotConnectionDot
|
||||
@@ -35,7 +34,6 @@ import { useSlotElementTracking } from '@/renderer/extensions/vueNodes/composabl
|
||||
import { useSlotLinkInteraction } from '@/renderer/extensions/vueNodes/composables/useSlotLinkInteraction'
|
||||
import { cn } from '@/utils/tailwindUtil'
|
||||
|
||||
import LODFallback from './LODFallback.vue'
|
||||
import SlotConnectionDot from './SlotConnectionDot.vue'
|
||||
|
||||
interface OutputSlotProps {
|
||||
|
||||
@@ -1,141 +0,0 @@
|
||||
# ComfyUI Widget LOD System: Architecture and Implementation
|
||||
|
||||
## Executive Summary
|
||||
|
||||
The ComfyUI widget Level of Detail (LOD) system has evolved from a reactive, Vue-based approach to a CSS-driven, non-reactive implementation. This architectural shift was driven by performance requirements at scale (300-500+ nodes) and a deeper understanding of browser rendering pipelines. The current system prioritizes consistent performance over granular control, leveraging CSS visibility rules rather than component mounting/unmounting.
|
||||
|
||||
## The Two Approaches: Reactive vs. Static LOD
|
||||
|
||||
### Approach 1: Reactive LOD (Original Design)
|
||||
|
||||
The original design envisioned a system where each widget would reactively respond to zoom level changes, controlling its own detail level through Vue's reactivity system. Widgets would import LOD utilities, compute what to show based on zoom level, and conditionally render elements using `v-if` and `v-show` directives.
|
||||
|
||||
**The promise of this approach was compelling:** widgets could intelligently manage their complexity, progressively revealing detail as users zoomed in, much like how mapping applications work. Developers would have fine-grained control over performance optimization.
|
||||
|
||||
### Approach 2: Static LOD with CSS (Current Implementation)
|
||||
|
||||
The implemented system takes a fundamentally different approach. All widget content is loaded and remains in the DOM at all times. Visual simplification happens through CSS rules, primarily using `visibility: hidden` and simplified visual representations (gray rectangles) at distant zoom levels. No reactive updates occur when zoom changes—only CSS rules apply differently.
|
||||
|
||||
**This approach seems counterintuitive at first:** aren't we wasting resources by keeping everything loaded? The answer reveals a deeper truth about modern browser rendering.
|
||||
|
||||
## The GPU Texture Bottleneck
|
||||
|
||||
The key insight driving the current architecture comes from understanding how browsers handle CSS transforms:
|
||||
|
||||
When you apply a CSS transform to a parent element (the "transformpane" in ComfyUI's case), the browser promotes that entire subtree to a compositor layer. This creates a single GPU texture containing all the transformed content. Here's where traditional performance intuitions break down:
|
||||
|
||||
### Traditional Assumption
|
||||
|
||||
"If we render less content, we get better performance. Therefore, hiding complex widgets should improve zoom/pan performance."
|
||||
|
||||
### Actual Browser Behavior
|
||||
|
||||
When all nodes are children of a single transformed parent:
|
||||
|
||||
1. The browser creates one large GPU texture for the entire node graph
|
||||
2. The texture dimensions are determined by the bounding box of all content
|
||||
3. Whether individual pixels are simple (solid rectangles) or complex (detailed widgets) has minimal impact
|
||||
4. The performance bottleneck is the texture size itself, not the complexity of rasterization
|
||||
|
||||
This means that even if we reduce every node to a simple gray rectangle, we're still paying the cost of a massive GPU texture when viewing hundreds of nodes simultaneously. The texture dimensions remain the same whether it contains simple or complex content.
|
||||
|
||||
## Two Distinct Performance Concerns
|
||||
|
||||
The analysis reveals two often-conflated performance considerations that should be understood separately:
|
||||
|
||||
### 1. Rendering Performance
|
||||
|
||||
**Question:** How fast can the browser paint and composite the node graph during interactions?
|
||||
|
||||
**Traditional thinking:** Show less content → render faster
|
||||
**Reality with CSS transforms:** GPU texture size dominates performance, not content complexity
|
||||
|
||||
The CSS transform approach means that zoom, pan, and drag operations are already optimized—they're just transforming an existing GPU texture. The cost is in the initial rasterization and texture upload, which happens regardless of content complexity when texture dimensions are fixed.
|
||||
|
||||
### 2. Memory and Lifecycle Management
|
||||
|
||||
**Question:** How much memory do widget instances consume, and what's the cost of maintaining them?
|
||||
|
||||
This is where unmounting widgets might theoretically help:
|
||||
|
||||
- Complex widgets (3D viewers, chart renderers) might hold significant memory
|
||||
- Event listeners and reactive watchers consume resources
|
||||
- Some widgets might run background processes or animations
|
||||
|
||||
However, the cost of mounting/unmounting hundreds of widgets on zoom changes could create worse performance problems than the memory savings provide. Vue's virtual DOM diffing for hundreds of nodes is expensive, potentially causing noticeable lag during zoom transitions.
|
||||
|
||||
## Design Philosophy and Trade-offs
|
||||
|
||||
The current CSS-based approach makes several deliberate trade-offs:
|
||||
|
||||
### What We Optimize For
|
||||
|
||||
1. **Consistent, predictable performance** - No reactivity means no sudden performance cliffs
|
||||
2. **Smooth zoom/pan interactions** - CSS transforms are hardware-accelerated
|
||||
3. **Simple widget development** - Widget authors don't need to implement LOD logic
|
||||
4. **Reliable state preservation** - Widgets never lose state from unmounting
|
||||
|
||||
### What We Accept
|
||||
|
||||
1. **Higher baseline memory usage** - All widgets remain mounted
|
||||
2. **Less granular control** - Widgets can't optimize their own LOD behavior
|
||||
3. **Potential waste for exotic widgets** - A 3D renderer widget still runs when hidden
|
||||
|
||||
## Open Questions and Future Considerations
|
||||
|
||||
### Should widgets have any LOD control?
|
||||
|
||||
The current system provides a uniform gray rectangle fallback with CSS visibility hiding. This works for 99% of widgets, but raises questions:
|
||||
|
||||
**Scenario:** A widget renders a complex 3D scene or runs expensive computations
|
||||
**Current behavior:** Hidden via CSS but still mounted
|
||||
**Question:** Should such widgets be able to opt into unmounting at distance?
|
||||
|
||||
The challenge is that introducing selective unmounting would require:
|
||||
|
||||
- Maintaining widget state across mount/unmount cycles
|
||||
- Accepting the performance cost of remounting when zooming in
|
||||
- Adding complexity to the widget API
|
||||
|
||||
### Could we reduce GPU texture size?
|
||||
|
||||
Since texture dimensions are the limiting factor, could we:
|
||||
|
||||
- Use multiple compositor layers for different regions (chunk the transformpane)?
|
||||
- Render the nodes using the canvas fallback when 500+ nodes and < 30% zoom.
|
||||
|
||||
These approaches would require significant architectural changes and might introduce their own performance trade-offs.
|
||||
|
||||
### Is there a hybrid approach?
|
||||
|
||||
Could we identify specific threshold scenarios where reactive LOD makes sense?
|
||||
|
||||
- When node count is low (< 50 nodes)
|
||||
- For specifically registered "expensive" widgets
|
||||
- At extreme zoom levels only
|
||||
|
||||
## Implementation Guidelines
|
||||
|
||||
Given the current architecture, here's how to work within the system:
|
||||
|
||||
### For Widget Developers
|
||||
|
||||
1. **Build widgets assuming they're always visible** - Don't rely on mount/unmount for cleanup
|
||||
2. **Use CSS classes for zoom-responsive styling** - Let CSS handle visual changes
|
||||
3. **Minimize background processing** - Assume your widget is always running
|
||||
4. **Consider requestAnimationFrame throttling** - For animations that won't be visible when zoomed out
|
||||
|
||||
### For System Architects
|
||||
|
||||
1. **Monitor GPU memory usage** - The single texture approach has memory implications
|
||||
2. **Consider viewport culling** - Not rendering off-screen nodes could reduce texture size
|
||||
3. **Profile real-world workflows** - Theoretical performance differs from actual usage patterns
|
||||
4. **Document the architecture clearly** - The non-obvious performance characteristics need explanation
|
||||
|
||||
## Conclusion
|
||||
|
||||
The ComfyUI LOD system represents a pragmatic choice: accepting higher memory usage and less granular control in exchange for predictable performance and implementation simplicity. By understanding that GPU texture dimensions—not rasterization complexity—drive performance in a CSS-transform-based architecture, the team has chosen an approach that may seem counterintuitive but actually aligns with browser rendering realities.
|
||||
|
||||
The system works well for the common case of hundreds of relatively simple widgets. Edge cases involving genuinely expensive widgets may need future consideration, but the current approach provides a solid foundation that avoids the performance pitfalls of reactive LOD at scale.
|
||||
|
||||
The key insight—that showing less doesn't necessarily mean rendering faster when everything lives in a single GPU texture—challenges conventional web performance wisdom and demonstrates the importance of understanding the full rendering pipeline when making architectural decisions.
|
||||
@@ -1,34 +0,0 @@
|
||||
/**
|
||||
* Level of Detail (LOD) composable for Vue-based node rendering
|
||||
*
|
||||
* Provides dynamic quality adjustment based on zoom level to maintain
|
||||
* performance with large node graphs. Uses zoom threshold based on DPR
|
||||
* to determine how much detail to render for each node component.
|
||||
* Default minFontSize = 8px
|
||||
* Default zoomThreshold = 0.57 (On a DPR = 1 monitor)
|
||||
**/
|
||||
import { useDevicePixelRatio } from '@vueuse/core'
|
||||
import { computed } from 'vue'
|
||||
|
||||
import { useSettingStore } from '@/platform/settings/settingStore'
|
||||
|
||||
interface Camera {
|
||||
z: number // zoom level
|
||||
}
|
||||
|
||||
export function useLOD(camera: Camera) {
|
||||
const isLOD = computed(() => {
|
||||
const { pixelRatio } = useDevicePixelRatio()
|
||||
const baseFontSize = 14
|
||||
const dprAdjustment = Math.sqrt(pixelRatio.value)
|
||||
|
||||
const settingStore = useSettingStore()
|
||||
const minFontSize = settingStore.get('LiteGraph.Canvas.MinFontSizeForLOD') //default 8
|
||||
const threshold =
|
||||
Math.round((minFontSize / (baseFontSize * dprAdjustment)) * 100) / 100 //round to 2 decimal places i.e 0.86
|
||||
|
||||
return camera.z < threshold
|
||||
})
|
||||
|
||||
return { isLOD }
|
||||
}
|
||||
@@ -5,7 +5,7 @@
|
||||
>
|
||||
<!-- Display mode: Rendered markdown -->
|
||||
<div
|
||||
class="comfy-markdown-content lod-toggle size-full min-h-[60px] overflow-y-auto rounded-lg text-sm"
|
||||
class="comfy-markdown-content size-full min-h-[60px] overflow-y-auto rounded-lg text-sm"
|
||||
:class="isEditing === false ? 'visible' : 'invisible'"
|
||||
v-html="renderedHtml"
|
||||
/>
|
||||
@@ -27,7 +27,6 @@
|
||||
@click.stop
|
||||
@keydown.stop
|
||||
/>
|
||||
<LODFallback />
|
||||
</div>
|
||||
</template>
|
||||
|
||||
@@ -38,8 +37,6 @@ import { computed, nextTick, ref } from 'vue'
|
||||
import type { SimplifiedWidget } from '@/types/simplifiedWidget'
|
||||
import { renderMarkdownToHtml } from '@/utils/markdownRendererUtil'
|
||||
|
||||
import LODFallback from '../../components/LODFallback.vue'
|
||||
|
||||
const { widget } = defineProps<{
|
||||
widget: SimplifiedWidget<string>
|
||||
}>()
|
||||
|
||||
@@ -78,7 +78,6 @@
|
||||
@ended="playback.onPlaybackEnded"
|
||||
@loadedmetadata="playback.onMetadataLoaded"
|
||||
/>
|
||||
<LODFallback />
|
||||
</div>
|
||||
</template>
|
||||
|
||||
@@ -91,7 +90,6 @@ import { t } from '@/i18n'
|
||||
import type { LGraphNode } from '@/lib/litegraph/src/LGraphNode'
|
||||
import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets'
|
||||
import { useToastStore } from '@/platform/updates/common/toastStore'
|
||||
import LODFallback from '@/renderer/extensions/vueNodes/components/LODFallback.vue'
|
||||
import { app } from '@/scripts/app'
|
||||
import { useAudioService } from '@/services/audioService'
|
||||
import type { SimplifiedWidget } from '@/types/simplifiedWidget'
|
||||
|
||||
@@ -3,9 +3,7 @@
|
||||
<Textarea
|
||||
v-model="modelValue"
|
||||
v-bind="filteredProps"
|
||||
:class="
|
||||
cn(WidgetInputBaseClass, 'size-full text-xs lod-toggle resize-none')
|
||||
"
|
||||
:class="cn(WidgetInputBaseClass, 'size-full text-xs resize-none')"
|
||||
:placeholder="placeholder || widget.name || ''"
|
||||
:aria-label="widget.name"
|
||||
:readonly="widget.options?.read_only"
|
||||
@@ -17,7 +15,6 @@
|
||||
@pointerup.capture.stop
|
||||
@contextmenu.capture.stop
|
||||
/>
|
||||
<LODFallback />
|
||||
</div>
|
||||
</template>
|
||||
|
||||
@@ -32,7 +29,6 @@ import {
|
||||
filterWidgetProps
|
||||
} from '@/utils/widgetPropFilter'
|
||||
|
||||
import LODFallback from '../../components/LODFallback.vue'
|
||||
import { WidgetInputBaseClass } from './layout'
|
||||
|
||||
const { widget, placeholder = '' } = defineProps<{
|
||||
|
||||
@@ -132,7 +132,6 @@
|
||||
</template>
|
||||
</TieredMenu>
|
||||
</div>
|
||||
<LODFallback />
|
||||
</div>
|
||||
</template>
|
||||
|
||||
@@ -143,7 +142,6 @@ import { computed, nextTick, onUnmounted, ref, watch } from 'vue'
|
||||
import { useI18n } from 'vue-i18n'
|
||||
|
||||
import type { LGraphNode } from '@/lib/litegraph/src/LGraphNode'
|
||||
import LODFallback from '@/renderer/extensions/vueNodes/components/LODFallback.vue'
|
||||
import { api } from '@/scripts/api'
|
||||
import { app } from '@/scripts/app'
|
||||
import { useNodeOutputStore } from '@/stores/imagePreviewStore'
|
||||
|
||||
@@ -3,8 +3,6 @@ import { noop } from 'es-toolkit'
|
||||
|
||||
import type { SimplifiedWidget } from '@/types/simplifiedWidget'
|
||||
|
||||
import LODFallback from '../../../components/LODFallback.vue'
|
||||
|
||||
defineProps<{
|
||||
widget: Pick<SimplifiedWidget<string | number | undefined>, 'name' | 'label'>
|
||||
}>()
|
||||
@@ -17,23 +15,21 @@ defineProps<{
|
||||
<div class="relative flex h-full min-w-0 items-center">
|
||||
<p
|
||||
v-if="widget.name"
|
||||
class="lod-toggle flex-1 truncate text-xs font-normal text-node-component-slot-text"
|
||||
class="flex-1 truncate text-xs font-normal text-node-component-slot-text"
|
||||
>
|
||||
{{ widget.label || widget.name }}
|
||||
</p>
|
||||
<LODFallback />
|
||||
</div>
|
||||
<!-- basis-full grow -->
|
||||
<div class="relative min-w-0 flex-1">
|
||||
<div
|
||||
class="lod-toggle cursor-default min-w-0"
|
||||
class="cursor-default min-w-0"
|
||||
@pointerdown.stop="noop"
|
||||
@pointermove.stop="noop"
|
||||
@pointerup.stop="noop"
|
||||
>
|
||||
<slot />
|
||||
</div>
|
||||
<LODFallback />
|
||||
</div>
|
||||
</div>
|
||||
</template>
|
||||
|
||||
@@ -1,69 +0,0 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { nextTick, reactive } from 'vue'
|
||||
|
||||
import { useLOD } from '@/renderer/extensions/vueNodes/lod/useLOD'
|
||||
|
||||
const mockSettingStore = reactive({
|
||||
get: vi.fn(() => 8)
|
||||
})
|
||||
|
||||
vi.mock('@/platform/settings/settingStore', () => ({
|
||||
useSettingStore: () => mockSettingStore
|
||||
}))
|
||||
|
||||
describe('useLOD', () => {
|
||||
beforeEach(() => {
|
||||
vi.restoreAllMocks()
|
||||
vi.unstubAllGlobals()
|
||||
|
||||
mockSettingStore.get.mockReturnValue(8)
|
||||
})
|
||||
|
||||
it('should calculate isLOD value based on zoom threshold correctly', async () => {
|
||||
vi.stubGlobal('devicePixelRatio', 1)
|
||||
|
||||
const camera = reactive({ z: 1 })
|
||||
const { isLOD } = useLOD(camera)
|
||||
|
||||
await nextTick()
|
||||
expect(isLOD.value).toBe(false)
|
||||
|
||||
camera.z = 0.55
|
||||
await nextTick()
|
||||
expect(isLOD.value).toBe(true)
|
||||
|
||||
camera.z = 0.87
|
||||
await nextTick()
|
||||
expect(isLOD.value).toBe(false)
|
||||
})
|
||||
|
||||
it('should handle a different devicePixelRatio value', async () => {
|
||||
vi.stubGlobal('devicePixelRatio', 3) //Threshold with 8px minFontsize = 0.19
|
||||
|
||||
const camera = reactive({ z: 1 })
|
||||
const { isLOD } = useLOD(camera)
|
||||
|
||||
await nextTick()
|
||||
expect(isLOD.value).toBe(false)
|
||||
|
||||
camera.z = 0.18
|
||||
await nextTick()
|
||||
expect(isLOD.value).toBe(true)
|
||||
})
|
||||
|
||||
it('should respond to different minFontSize settings', async () => {
|
||||
vi.stubGlobal('devicePixelRatio', 1)
|
||||
|
||||
mockSettingStore.get.mockReturnValue(16) //Now threshold is 1.14
|
||||
|
||||
const camera = reactive({ z: 1 })
|
||||
const { isLOD } = useLOD(camera)
|
||||
|
||||
await nextTick()
|
||||
expect(isLOD.value).toBe(true)
|
||||
|
||||
camera.z = 1.15
|
||||
await nextTick()
|
||||
expect(isLOD.value).toBe(false)
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user